diff --git a/docs/requirements/003-operator-workbench.md b/docs/requirements/003-operator-workbench.md index f31449d..9a0c0d4 100644 --- a/docs/requirements/003-operator-workbench.md +++ b/docs/requirements/003-operator-workbench.md @@ -20,7 +20,9 @@ changed and what it costs, and a gate means something again. 3. **Diff preview**: every review card shows its diff expanded; the requesting agent's script ships it with the review (the hub reads no git, 001); one block per file, a file over 400 changed lines collapsed, 2 MiB cap with the - rest listed by stat; lines MEDUSA's secret rules match are masked. + rest listed by stat; a file whose old or new content holds an armour + line or matches a MEDUSA secret rule (every file when no rule loads) + ships only path, status, mode and counts with the note "content withheld". Rejected: the hub reads git. Every Ask for a `git commit` shows the staged diff the same way (with `-a` or `--all` the diff against HEAD); the permission hook on the host @@ -145,29 +147,41 @@ flowchart TD ### W1 Operator tasks -- [ ] The top navigation has no Inbox entry; the Tasks entry shows the count of +- [x] The top navigation has no Inbox entry; the Tasks entry shows the count of waiting, unsnoozed operator tasks, hidden at 0. -- [ ] The Stream has no inbox button; `#stream/inbox` opens `#tasks/operator`. -- [ ] `#tasks/operator` shows each operator task as a Stream card (command, +- [x] The Stream has no inbox button; `#stream/inbox` opens `#tasks/operator`. +- [x] `#tasks/operator` shows each operator task as a Stream card (command, directory, reason, run, deny, copy, snooze, pin) with a checkbox; the bulk bar acts on the checked cards. -- [ ] The Stream keeps its operator cards with their actions. -- [ ] The bulk approve counts and approves every checked card, asks, reviews +- [x] The Stream keeps its operator cards with their actions. +- [x] The bulk approve counts and approves every checked card, asks, reviews and operator commands alike, with one passkey. -- [ ] Approving or denying an Ask in the hub also settles the agent's native +- [x] Approving or denying an Ask in the hub also settles the agent's native permission dialog where the runtime allows it (the permission hook waits for the hub's decision); where it does not, the limit is recorded here. -- [ ] A review card shows its diff expanded: one unified block per file, + Limit: Claude Code runs the PermissionRequest hook before it shows its + own dialog, so a hub decision within the hook's `--timeout` settles the + ask and no native dialog appears. After the timeout, with the hub down + or an answer whose signature does not verify, the hook answers nothing + and Claude Code shows its own dialog, which a later hub decision no + longer settles. `make orchestrator-hook` installs the hook for Claude + Code only; other runtimes keep their native prompt and post no ask. +- [x] A review card shows its diff expanded: one unified block per file, additions and deletions coloured, a file over 400 changed lines collapsed, a binary file named only. -- [ ] The review request of `approval.py` (kvb-git-diff) carries +- [x] The review request of `approval.py` (kvb-git-diff) carries `git diff --cached` of the exact staged tree; past 2 MiB the rest is listed per file with its stat. -- [ ] An Ask for a `git commit` carries the staged diff of its working +- [x] An Ask for a `git commit` carries the staged diff of its working directory (with `-a` or `--all` the diff against HEAD), attached by `permission_hook.py`, and shows it like a review, feedback included. -- [ ] A diff line MEDUSA's secret rules match is shown masked. +- [x] A file whose content holds an armour line or matches MEDUSA's secret + rules, and every file when no rule loads, is shown by stat with the + note "content withheld". +- [x] An allow of a commit Ask is bound to the staged tree `git write-tree` + prints (with `-a` also the working tree); a different tree at approval + turns it into a deny "staged tree changed since the preview". - [ ] Selecting lines in a diff or a command and choosing Feedback (right-click menu, key `f`, or the button at the selection) posts one radio `post` to the requesting agent with path, line range, quote and text; the feedback @@ -317,10 +331,12 @@ The work packages meet only through these interfaces: `--drop-after `, `--prio ` or `--unpin`; every changed row carries `updated` (ISO time), which Stale reads. - **Review diff**: the review request of `approval.py` gains `diff`, a list of - `{path, status, additions, deletions, patch, masked}`: status one of A, M, - D, R or B (binary), patch the unified text (empty for binary files and - past the 2 MiB cap), masked the line numbers shown masked. An Ask for a - `git commit` carries the same `diff` from `permission_hook.py`. + `{path, status, mode, additions, deletions, patch, note}`: status one of + A, M, D, R or B (binary), mode the git file mode (`old => new` when it + changed), patch the unified text (empty for binary files, past the 2 MiB + cap and for withheld content), note empty or "content withheld". An Ask + for a `git commit` carries the same `diff` from `permission_hook.py`; a + body past the hub's limit goes without it. - **Feedback**: a feedback is a message to the requesting agent through the existing signed message flow, text `Feedback on :-: "" `. diff --git a/orchestrator/Dockerfile b/orchestrator/Dockerfile index 6508c6f..3597835 100644 --- a/orchestrator/Dockerfile +++ b/orchestrator/Dockerfile @@ -36,7 +36,7 @@ RUN chmod 0755 /opt/voices /opt/whisper COPY server.py /app/server.py COPY kvb_orchestrator /app/kvb_orchestrator -COPY client/signing.py client/passkey.py client/verifier.py /app/client/ +COPY client/hub_limits.py client/signing.py client/passkey.py client/verifier.py /app/client/ COPY templates /app/templates COPY vendor /app/vendor diff --git a/orchestrator/client/approval.py b/orchestrator/client/approval.py index bfa087e..96dea40 100755 --- a/orchestrator/client/approval.py +++ b/orchestrator/client/approval.py @@ -9,10 +9,21 @@ fetches the review's signature from the Agent Orchestrator on 127.0.0.1: and checks it against the repository, worktree, branch and the tree git write-tree prints, as this process sees them. + request --port --title --page --notes + --gate --score --agent [--rules ]: + run in the worktree; asks the operator to approve the staged tree, + with the repository, worktree, branch and tree as git prints them + here, the notes file's text (refused past NOTES_LIMIT characters), + and the diff of the tree git write-tree prints against HEAD + (staged_diff.tree_diff, content withheld per the MEDUSA rules under + , by default those of the pipx install); without the diff when + the body would pass DIFF_BODY_LIMIT. Returns: pin: exit code 0 and the pinned credential on stdout; 1 and the reason on stderr when the blob is no valid registration. + request: exit code 0 and the hub's {"id", "status", "url"} on stdout; + 1 and the reason on stderr. verify: exit code 0 when a pinned key signed this exact commit; 1 and the reason on stderr otherwise. Commit only on 0. """ @@ -21,28 +32,24 @@ from __future__ import annotations import argparse import json -import subprocess import sys import time import urllib.request from pathlib import Path +import staged_diff from hub_api import GUARD, SLACK, base_url, load +from hub_limits import DIFF_BODY_LIMIT, NOTES_LIMIT from passkey import attested, b64decode from signing import ALGORITHMS, ID, Refused, origin_of, review_item from verifier import Trust, check, pin -GIT_TIMEOUT = 30 - def git(cwd: Path, args: list[str]) -> str: try: - done = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True, timeout=GIT_TIMEOUT, check=False) - except (OSError, subprocess.TimeoutExpired) as error: - raise Refused(f"git {args[0]} did not run: {error}") from error - if done.returncode != 0: - raise Refused(f"git {' '.join(args)} failed: {done.stderr.strip()}") - return done.stdout.strip() + return staged_diff.git(cwd, args, None).decode("utf-8", "surrogateescape").strip() + except staged_diff.DiffError as error: + raise Refused(str(error)) from error def staged(cwd: Path, review_id: str) -> dict: @@ -72,6 +79,38 @@ def verify_commit(args: argparse.Namespace) -> str: return f"verified: the operator signed review {args.review}, tree {item['tree']} on {item['branch']} in {item['worktree']}" +def review_body(arguments: dict) -> bytes: + call = {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": "review_request", "arguments": arguments}} + return json.dumps(call, ensure_ascii=False).encode("utf-8") + + +def request_review(args: argparse.Namespace) -> str: + cwd = Path.cwd() + item = staged(cwd, "0" * 16) + try: + notes = args.notes.read_text(encoding="utf-8") + diff = staged_diff.tree_diff(cwd, item["tree"], args.rules or staged_diff.rules_root()) + except (OSError, ValueError, staged_diff.DiffError) as error: + raise Refused(f"the review cannot be prepared: {error}") from error + if len(notes) > NOTES_LIMIT: + raise Refused(f"the notes hold {len(notes)} characters, the hub takes at most {NOTES_LIMIT}") + arguments = {key: item[key] for key in ("repo", "worktree", "branch", "tree")} + arguments.update(title=args.title, page=str(args.page), notes=notes, gate=args.gate, score=args.score, agent=args.agent) + body = review_body({**arguments, "diff": diff}) + if len(body) > DIFF_BODY_LIMIT: + print(f"approval: the review goes without its diff, its {len(body)} bytes pass the hub's {DIFF_BODY_LIMIT}", file=sys.stderr) + body = review_body(arguments) + headers = {"Content-Type": "application/json", "Accept": "application/json, text/event-stream"} + try: + answer = load(urllib.request.Request(f"{base_url(args.port)}/mcp", data=body, headers=headers, method="POST"), SLACK) + except (OSError, ValueError) as error: + raise Refused(f"the Agent Orchestrator gave no answer: {error}") from error + result = answer.get("result") if isinstance(answer, dict) else None + if not isinstance(result, dict) or result.get("isError") or not isinstance(result.get("structuredContent"), dict): + raise Refused(f"the Agent Orchestrator refused the review: {json.dumps(answer)[:500]}") + return json.dumps(result["structuredContent"]) + + def pin_credential(args: argparse.Namespace) -> str: try: credential = json.loads(b64decode(args.credential, "credential")) @@ -100,9 +139,18 @@ def main() -> int: verifying.add_argument("--replay", type=Path, required=True) verifying.add_argument("--review", type=review_id, required=True) verifying.add_argument("--gate", choices=("pass", "fail"), required=True) + requesting = commands.add_parser("request") + requesting.add_argument("--port", type=int, required=True) + for name in ("--title", "--score", "--agent"): + requesting.add_argument(name, required=True) + requesting.add_argument("--page", type=Path, required=True) + requesting.add_argument("--notes", type=Path, required=True) + requesting.add_argument("--gate", choices=("pass", "fail"), required=True) + requesting.add_argument("--rules", type=Path) args = parser.parse_args() + run = {"pin": pin_credential, "verify": verify_commit, "request": request_review}[args.command] try: - print(pin_credential(args) if args.command == "pin" else verify_commit(args)) + print(run(args)) except (Refused, OSError) as error: print(f"refused: {error}", file=sys.stderr) return 1 diff --git a/orchestrator/client/hub_limits.py b/orchestrator/client/hub_limits.py new file mode 100644 index 0000000..215eba0 --- /dev/null +++ b/orchestrator/client/hub_limits.py @@ -0,0 +1,20 @@ +"""Returns: + DIFF_LIMIT: bytes of JSON-encoded patch text one review diff carries, in + the client that builds it and the hub that takes it. + DIFF_BODY_LIMIT: bytes of request body the hub reads for an ask or a + review request; the client sends a larger one without its diff. + FILE_CAP: files one review diff lists; the client builds no diff of more. + PATH_LIMIT: characters of a path the hub takes, a diff path included; + the client builds no diff with a longer one. + NOTES_LIMIT: characters of review notes the hub takes; the client refuses + longer notes instead of cutting them. + WITHHELD: the note of a diff entry that ships without its patch because + its content may hold a secret. +""" + +DIFF_LIMIT = 2 << 20 +DIFF_BODY_LIMIT = 4 << 20 +FILE_CAP = 5000 +PATH_LIMIT = 4096 +NOTES_LIMIT = 100_000 +WITHHELD = "content withheld" diff --git a/orchestrator/client/permission_hook.py b/orchestrator/client/permission_hook.py index 4d75bc3..ee97f6b 100755 --- a/orchestrator/client/permission_hook.py +++ b/orchestrator/client/permission_hook.py @@ -16,7 +16,14 @@ Returns: reason only when a pinned passkey signed it. Nothing on timeout, error, a missing or invalid signature, a value longer than the hub takes or a hub that is down, so Claude Code shows its own dialog. - The exit code is 0. + The exit code is 0. An ask for a Bash git commit carries the diff + staged_diff.py builds of the tree its directory stages (of the + working tree against HEAD with -a or --all), content withheld per + the MEDUSA rules of the pipx install; none when it cannot be built + or the body would pass DIFF_BODY_LIMIT. An allow of such an ask + turns into a deny with CHANGED when what the commit records + (staged_diff.binding) differs right before answering from the + preview's, and with UNBOUND when either cannot be read. """ from __future__ import annotations @@ -30,17 +37,22 @@ import urllib.request from pathlib import Path import hub_api +import hub_limits import signing +import staged_diff import verifier POLL = 25.0 NAME_LIMIT = 200 COMMAND_LIMIT = 4000 -PATH_LIMIT = 4096 +PATH_LIMIT = hub_limits.PATH_LIMIT LIMITS = {"tool": NAME_LIMIT, "command": COMMAND_LIMIT, "cwd": PATH_LIMIT, "session": NAME_LIMIT, "agent": NAME_LIMIT} DENIED = "The operator denied this in the Agent Orchestrator." +CHANGED = "staged tree changed since the preview" +UNBOUND = "the staged tree cannot be read, so the allow is not bound to it" REASON_LIMIT = 2000 CONTROL = re.compile(r"[\x00-\x1f\x7f]") +FAILURES = (OSError, ValueError, staged_diff.DiffError) def summary(tool_input: object) -> str: @@ -61,10 +73,67 @@ def facts(hook: dict) -> dict: } -def create(base: str, ask: dict, timeout: float) -> str: - payload = {**ask, "timeout": timeout} +def commit_target(ask: dict) -> tuple[Path, bool] | None: + return staged_diff.commit_of(ask["command"], ask["cwd"]) if ask["tool"] == "Bash" and ask["cwd"] else None + + +def commit_diff(ask: dict, rules: Path | None) -> tuple[str | None, dict]: + """Returns: + For a commit ask, what the commit records (staged_diff.binding, None + when it cannot be read) and {"diff": the preview of exactly that}, + {} when it cannot be built; None and {} for any other ask. + """ + target = commit_target(ask) + if target is None: + return None, {} + try: + pinned = staged_diff.binding(*target) + except FAILURES as error: + print(f"kvb-orchestrator: the ask goes without its diff and an allow of it turns into a deny: {error}", file=sys.stderr) + return None, {} + try: + return pinned, {"diff": staged_diff.bound_diff(*target, pinned, rules)} + except FAILURES as error: + print(f"kvb-orchestrator: the ask goes without its diff: {error}", file=sys.stderr) + return pinned, {} + + +def clean(text: str) -> str: + return CONTROL.sub("?", text.strip())[:REASON_LIMIT] + + +def bound(chosen: dict | None, ask: dict, pinned: str | None) -> dict | None: + """Args: + pinned: what the commit recorded when the ask was made, None when + that could not be read. + + Returns: + chosen, or for an allow of a commit ask a deny with CHANGED when + what it records now differs from pinned, with UNBOUND and the error + when either cannot be read. + """ + if chosen != {"behavior": "allow"} or commit_target(ask) is None: + return chosen + try: + now = staged_diff.binding(*commit_target(ask)) + except FAILURES as error: + return {"behavior": "deny", "message": f"{UNBOUND}: {clean(str(error))}"} + if pinned is None: + return {"behavior": "deny", "message": UNBOUND} + return chosen if now == pinned else {"behavior": "deny", "message": CHANGED} + + +def encoded(payload: dict) -> bytes: + return json.dumps(payload).encode() + + +def create(base: str, ask: dict, timeout: float, shown: dict) -> str: + data = encoded({**ask, "timeout": timeout, **shown}) + if len(data) > hub_limits.DIFF_BODY_LIMIT: + print(f"kvb-orchestrator: the ask goes without its diff, its {len(data)} bytes pass the hub's {hub_limits.DIFF_BODY_LIMIT}", file=sys.stderr) + data = encoded({**ask, "timeout": timeout}) headers = {"Content-Type": "application/json", **hub_api.GUARD} - request = urllib.request.Request(f"{base}/api/asks", data=json.dumps(payload).encode(), headers=headers, method="POST") + request = urllib.request.Request(f"{base}/api/asks", data=data, headers=headers, method="POST") ask_id = hub_api.call(request, hub_api.SLACK)["id"] if not isinstance(ask_id, str) or not signing.ID.fullmatch(ask_id): raise ValueError("the hub answered no ask id") @@ -87,7 +156,7 @@ def denied(answer: dict, item: dict, trust: verifier.Trust) -> str: except (signing.Refused, OSError) as error: print(f"kvb-orchestrator: the reason is not signed, the agent gets the denial without it: {error}", file=sys.stderr) return DENIED - return f"{DENIED} Reason: {CONTROL.sub('?', reason.strip())[:REASON_LIMIT]}" + return f"{DENIED} Reason: {clean(reason)}" def decision(answer: dict, item: dict, trust: verifier.Trust) -> dict | None: @@ -120,13 +189,14 @@ def main() -> int: if too_long: print(f"kvb-orchestrator: {', '.join(too_long)} too long to sign in full, Claude Code asks instead", file=sys.stderr) return 0 - ask_id = create(base, ask, args.timeout) + pinned, shown = commit_diff(ask, staged_diff.rules_root()) + ask_id = create(base, ask, args.timeout, shown) answer = {"status": "pending"} while answer["status"] == "pending" and (left := deadline - time.monotonic()) > 0: answer = status(base, ask_id, min(left, POLL)) except (OSError, ValueError, KeyError, TypeError, AttributeError): return 0 - chosen = decision(answer, signing.ask_item(ask_id, ask["tool"], ask["command"], ask["cwd"], ask["session"]), trust) + chosen = bound(decision(answer, signing.ask_item(ask_id, ask["tool"], ask["command"], ask["cwd"], ask["session"]), trust), ask, pinned) if chosen is not None: print(json.dumps({"hookSpecificOutput": {"hookEventName": "PermissionRequest", "decision": chosen}})) return 0 diff --git a/orchestrator/client/sandbox_paths.py b/orchestrator/client/sandbox_paths.py new file mode 100644 index 0000000..9ffef50 --- /dev/null +++ b/orchestrator/client/sandbox_paths.py @@ -0,0 +1,167 @@ +"""Args: + top: the working tree; its .claude folder and that of $CLAUDE_PROJECT_DIR join the merge. + +Returns: + denied_roots: FLOOR with every sandbox.filesystem.denyRead path and every + permissions.deny Read(...) rule of the settings Claude Code merges + (managed, user, project), as absolute paths or globs (a **/ also + matching no folder), also resolved + through symlinks; a relative one lies under its scope (managed: its + folder, user: the config dir, project: the project root). + denied: whether a path is, matches or lies under one of those. + +Raises: + Unreadable: when a settings file or managed-settings.d exists but cannot be read or parsed. +""" + +from __future__ import annotations + +import fnmatch +import itertools +import json +import os +import re +import stat +from pathlib import Path + +MANAGED = Path("/etc/claude-code") +SETTINGS = ("settings.json", "settings.local.json") +SETTINGS_LIMIT = 1 << 20 +GLOB = re.compile(r"[*?\[]") +FLOOR =("~/.ssh", "~/.gnupg", "~/.aws", "~/.kube", "~/.netrc", "~/.git-credentials", "~/.config/gcloud", "~/.docker/config.json", "~/.claude") +READ_RULE = re.compile(r"\s*Read(?:\((.*)\))?\s*") + + +class Unreadable(Exception): + pass + + +def project_roots(top: Path) -> list[Path]: + session = os.environ.get("CLAUDE_PROJECT_DIR") + return [top, *([Path(session)] if session else [])] + + +def settings_files(top: Path) -> list[tuple[Path, Path]]: + """Returns: + Every settings file Claude Code merges, with the folder its relative + paths start from. + """ + user = Path(os.environ.get("CLAUDE_CONFIG_DIR") or Path.home() / ".claude") + scoped = [(MANAGED / "managed-settings.json", MANAGED), *((file, MANAGED) for file in managed_parts())] + scoped += [(user / name, user) for name in SETTINGS] + return scoped + [(root / ".claude" / name, root) for root in project_roots(top) for name in SETTINGS] + + +def managed_parts() -> list[Path]: + folder = MANAGED / "managed-settings.d" + try: + names = sorted(os.listdir(folder)) + except FileNotFoundError: + return [] + except OSError as error: + raise Unreadable(f"settings folder {folder} unreadable: {type(error).__name__}") from error + return [folder / name for name in names if name.endswith(".json")] + + +def settings_text(file: Path) -> str | None: + """Returns: + The text of a settings file, None when it does not exist. + + Raises: + Unreadable: when it is no regular file of at most SETTINGS_LIMIT bytes. + OSError, ValueError: when it cannot be opened or is no UTF-8. + """ + try: + fd = os.open(file, os.O_RDONLY | os.O_NONBLOCK) + except FileNotFoundError: + return None + try: + info = os.fstat(fd) + if not stat.S_ISREG(info.st_mode) or info.st_size > SETTINGS_LIMIT: + raise Unreadable(f"settings {file} is no regular file of at most {SETTINGS_LIMIT} bytes") + with open(fd, "rb", closefd=False) as handle: + return handle.read(SETTINGS_LIMIT + 1).decode("utf-8") + finally: + os.close(fd) + + +def settings_data(file: Path) -> object: + try: + text = settings_text(file) + return json.loads(text) if text and text.strip() else {} + except (OSError, ValueError) as error: + raise Unreadable(f"settings {file} unreadable: {type(error).__name__}") from error + + +def strings_at(data: object, keys: tuple[str, ...]) -> list[str]: + for key in keys: + data = data.get(key) if isinstance(data, dict) else None + if data is None: + return [] + if not (isinstance(data, list) and all(isinstance(entry, str) for entry in data)): + raise Unreadable(f"{'.'.join(keys)} is no list of strings") + return data + + +def rule_paths(rules: list[str], scope: Path, folders: list[Path]) -> list[str]: + """Returns: + The paths the permissions.deny Read rules name, read as Claude Code + reads them: //x absolute, ~/x under home, /x under scope, any other + under scope and every one of folders, a name without / at any depth; + a bare Read denies /. + """ + found = [] + for entry in rules: + match = READ_RULE.fullmatch(entry) + if match: + found += rule_path(match.group(1) or "//", scope, folders) + return found + + +def rule_path(spec: str, scope: Path, folders: list[Path]) -> list[str]: + if spec.startswith("//"): + return ["/" + spec.lstrip("/")] + if spec == "~" or spec.startswith("~/"): + return [os.path.expanduser(spec)] + if spec.startswith("/"): + return [f"{scope}{spec}"] + name = spec.removeprefix("./") + bases = [scope, *folders] + if "/" in name.rstrip("/"): + return [f"{base}/{name}" for base in bases] + return [entry for base in bases for entry in (f"{base}/{name}", f"{base}/*/{name}")] + + +def denied_roots(top: Path) -> list[str]: + folders = project_roots(top) + entries = [(entry, Path.home()) for entry in FLOOR] + for file, scope in settings_files(top): + data = settings_data(file) + entries += [(entry, scope) for entry in strings_at(data, ("sandbox", "filesystem", "denyRead"))] + entries += [(entry, scope) for entry in rule_paths(strings_at(data, ("permissions", "deny")), scope, folders)] + return [root for entry, scope in entries for root in resolved(entry, scope)] + + +def resolved(entry: str, base: Path) -> set[str]: + found = set() + for path in zero_folders("/" + os.path.normpath(os.path.join(base, os.path.expanduser(entry))).lstrip("/")): + cut = GLOB.search(path) + head = os.path.dirname(path[: cut.start()]) if cut else path + found |= {path, os.path.realpath(head) + path[len(head) :]} + return found + + +def zero_folders(path: str) -> set[str]: + """Returns: + path and every spelling of it with some of its **/ dropped, as a + gitignore ** also matches no folder at all. + """ + parts = path.split("**/") + return {"".join(part + cut for part, cut in zip(parts, cuts, strict=False)) + parts[-1] for cuts in itertools.product(("**/", ""), repeat=len(parts) - 1)} + + +def denied(path: str, roots: list[str]) -> bool: + if any(path == root or path.startswith(f"{root.rstrip('/')}/") for root in roots if not GLOB.search(root)): + return True + chain = [path, *map(str, Path(path).parents)] + return any(fnmatch.fnmatchcase(link, root) for root in roots if GLOB.search(root) for link in chain) diff --git a/orchestrator/client/staged_diff.py b/orchestrator/client/staged_diff.py new file mode 100644 index 0000000..25073fa --- /dev/null +++ b/orchestrator/client/staged_diff.py @@ -0,0 +1,583 @@ +"""Args: + cwd: the working directory whose repository the diff is taken of. + everything: False for the staged tree against HEAD (git diff --cached), + True for what git commit -a records (every index path as it lies in + the working tree) against HEAD. + tree: for tree_diff, the tree object (git write-tree) diffed against + HEAD instead of the live index. + rules: the MEDUSA rules directory, None when MEDUSA is not installed. + pinned: for bound_diff, what binding returned for the same cwd and everything. + +Returns: + review_diff, tree_diff, bound_diff: the review diff of requirement 003, + one {path, status, mode, additions, deletions, patch, note} per + changed path, sorted by path, status A, M, D or B (binary); path is + the file name with bytes that are no UTF-8 written as backslash + escapes; mode the git file mode, "old => new" when it changed (with + everything, the executable bit on disk). patch holds the unified + hunks; it is empty for a binary file, a gitlink, a side over + BLOB_LIMIT bytes (counted 0/0), a path sandbox_paths denies (its + FLOOR and the settings' deny lists) and every file past DIFF_LIMIT + bytes of JSON-encoded patch text. A file whose old or new content + holds an ARMOUR line or matches a MEDUSA secret rule for its name, + and every file when no secret rule pattern loads, ships no patch, + only its counts and the note WITHHELD. With everything, a working + tree file over BLOB_LIMIT or under a denied path is not read and so + always counts as changed. The diff fails when more than FILE_CAP + paths changed or one is longer than PATH_LIMIT, a settings file + sandbox_paths reads is unreadable, the working tree lies under a + denied path, HEAD exists but its tree cannot be listed, HEAD is + neither a commit nor unborn, or for bound_diff, the working tree + changed while it was read. + binding: what a commit records, for the permission hook to bind an + allow to: the tree git write-tree prints; with everything, joined by + the digest of every index path's mode and content id as it lies in + the working tree, a file not read by its size, inode and times. + commit_of: the directory and -a mode of a single git commit whose + preview matches what it records, None for any other command. + +Only git plumbing runs (rev-parse, symbolic-ref, ls-tree, ls-files, cat-file, +write-tree) with core.fsmonitor and lazy fetching off, and the diff itself is computed here: +the permission hook calls this outside the sandbox in a directory the agent +chose, where a planted repository config, filter, diff driver or promisor +remote must start no program, and a working tree path must not lead the read +through a symlinked directory, out of the working tree or into a path the +sandbox denies reading. +""" + +from __future__ import annotations + +import difflib +import fnmatch +import hashlib +import json +import os +import re +import shlex +import stat +import subprocess +from pathlib import Path + +import hub_limits +import sandbox_paths + +DIFF_LIMIT = hub_limits.DIFF_LIMIT +FILE_CAP = hub_limits.FILE_CAP +PATH_LIMIT = hub_limits.PATH_LIMIT +WITHHELD = hub_limits.WITHHELD +BLOB_LIMIT = 2 << 20 +BINARY_PROBE = 8000 +CONTEXT = 3 +GIT_TIMEOUT = 30 +GITLINK = "160000" +SYMLINK = "120000" +REGULAR = "100644" +EXECUTABLE = "100755" +SECRET_CWE = re.compile(r"CWE-(?:259|321|540|798)\b") +RULES_HOME = Path.home() / ".local" / "pipx" / "venvs" / "medusa-security" / "lib" +RULES_TAIL = "python3*/site-packages/medusa/rules" +CACHE = Path(os.environ.get("XDG_CACHE_HOME") or Path.home() / ".cache") / "kvb-orchestrator" / "secret-rules.json" +UNREAD = "unread" +SAFE = ("git", "--no-pager", "--no-lazy-fetch", "-c", "core.fsmonitor=false") +COMMIT_LONGS = { + "quiet", "verbose", "file", "author", "date", "message", "reedit-message", "reuse-message", "fixup", "squash", "reset-author", "trailer", "signoff", + "template", "edit", "cleanup", "status", "gpg-sign", "all", "include", "interactive", "patch", "unified", "inter-hunk-context", "only", "no-verify", + "dry-run", "short", "branch", "ahead-behind", "porcelain", "long", "null", "amend", "no-post-rewrite", "untracked-files", "pathspec-from-file", + "pathspec-file-nul", "allow-empty", "allow-empty-message", "verify", "post-rewrite", "no-quiet", "no-verbose", "no-file", "no-author", "no-date", + "no-message", "no-reedit-message", "no-reuse-message", "no-fixup", "no-squash", "no-reset-author", "no-trailer", "no-signoff", "no-template", "no-edit", + "no-cleanup", "no-status", "no-gpg-sign", "no-all", "no-include", "no-interactive", "no-patch", "no-only", "no-dry-run", "no-short", "no-branch", + "no-ahead-behind", "no-porcelain", "no-long", "no-null", "no-amend", "no-untracked-files", "no-pathspec-from-file", "no-pathspec-file-nul", "no-allow-empty", + "no-allow-empty-message", +} +LONG_ARGS = {"message", "file", "reuse-message", "reedit-message", "template", "author", "date", "cleanup", "fixup", "squash", "trailer", "unified", "inter-hunk-context"} +NARROWING = {"only", "include", "interactive", "patch", "pathspec-from-file", "no-all"} +SHORT_ARGS = set("mFcCtuSU") +KEPT_VARS = {"GIT_EDITOR", "GIT_SEQUENCE_EDITOR", "GIT_PAGER", "GIT_TERMINAL_PROMPT", "GIT_AUTHOR_NAME", "GIT_AUTHOR_EMAIL", "GIT_AUTHOR_DATE", "GIT_COMMITTER_NAME", "GIT_COMMITTER_EMAIL", "GIT_COMMITTER_DATE"} +ASSIGNMENT = re.compile(r"([A-Za-z_][A-Za-z0-9_]*)=.*") +ARMOUR = re.compile(r"-----BEGIN [A-Z0-9 ]+-----") +Patterns = list[re.Pattern] +Rules = list[tuple[tuple[str, ...], Patterns]] + + +class DiffError(Exception): + pass + + +def git(cwd: Path, args: list[str], data: bytes | None) -> bytes: + try: + done = subprocess.run([*SAFE, *args], cwd=cwd, input=data, capture_output=True, timeout=GIT_TIMEOUT, check=False, env={**os.environ, "GIT_OPTIONAL_LOCKS": "0", "GIT_NO_LAZY_FETCH": "1"}) + except (OSError, subprocess.TimeoutExpired) as error: + raise DiffError(f"git {args[0]} failed to start: {error}") from error + if done.returncode != 0: + raise DiffError(f"git {' '.join(args)} failed: {done.stderr.decode('utf-8', 'replace').strip()}") + return done.stdout + + +def listed(raw: bytes, object_field: int) -> dict[str, tuple[str, str]]: + found = {} + for record in raw.split(b"\0"): + head, _, path = record.partition(b"\t") + if path: + fields = head.decode("ascii").split() + found[path.decode("utf-8", "surrogateescape")] = (fields[0], fields[object_field]) + return found + + +def blob_sizes(cwd: Path, oids: set[str]) -> dict[str, int]: + order = sorted(oids) + lines = git(cwd, ["cat-file", "--batch-check"], "".join(f"{oid}\n" for oid in order).encode()).decode("ascii").splitlines() if order else [] + fields = {oid: line.split() for oid, line in zip(order, lines, strict=True)} + missing = sorted(oid for oid, found in fields.items() if len(found) != 3) + if missing: + raise DiffError(f"objects missing from the repository: {' '.join(missing)}") + return {oid: int(found[2]) for oid, found in fields.items()} + + +def blobs(cwd: Path, oids: set[str]) -> dict[str, bytes]: + order = sorted(oids) + if not order: + return {} + out = git(cwd, ["cat-file", "--batch"], "".join(f"{oid}\n" for oid in order).encode()) + found, at = {}, 0 + for oid in order: + end = out.index(b"\n", at) + size = int(out[at:end].split()[2]) + found[oid] = out[end + 1 : end + 1 + size] + at = end + 2 + size + return found + + +def worktree(top: Path, path: str, mode: str, algorithm: str, roots: list[str]) -> tuple[str, str, bytes | None] | None: + """Args: + top: the working tree, resolved through symlinks. + roots: the paths the sandbox denies reading (sandbox_paths.denied_roots). + + Returns: + The mode (for a regular file its executable bit on disk), blob id and + content of an index path as it lies in the working tree; for a file + over BLOB_LIMIT or under one of roots, which is not read, content + None and in place of the blob id UNREAD with its size, inode and + times; None when it is gone, lies behind a symlinked directory or is + neither a regular file nor a symlink. + + Raises: + DiffError: for a path with an empty, . or .. component. + """ + parts = path.split("/") + if any(part in ("", ".", "..") for part in parts): + raise DiffError(f"index path {path!r} leaves the working tree") + try: + if sandbox_paths.denied(os.path.join(top, *parts), roots): + return mode, unread(os.lstat(os.path.join(top, *parts))), None + found = on_disk(top, parts) + except OSError: + return None + if found is None: + return None + mode, data, info = found + if data is None: + return mode, unread(info), None + return mode, hashlib.new(algorithm, b"blob %d\0" % len(data) + data).hexdigest(), data + + +def unread(info: os.stat_result) -> str: + return f"{UNREAD}:{info.st_size}:{info.st_ino}:{info.st_mtime_ns}:{info.st_ctime_ns}" + + +def on_disk(top: Path, parts: list[str]) -> tuple[str, bytes | None, os.stat_result] | None: + """Returns: + The mode, content (None past BLOB_LIMIT) and status of top/parts, + every folder from / on opened without following a symlink, None for + an entry that is neither a regular file nor a symlink. + """ + folder = os.open("/", os.O_RDONLY | os.O_DIRECTORY) + try: + for part in [*top.parts[1:], *parts[:-1]]: + inner = os.open(part, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=folder) + os.close(folder) + folder = inner + return leaf(folder, parts[-1]) + finally: + os.close(folder) + + +def leaf(folder: int, name: str) -> tuple[str, bytes | None, os.stat_result] | None: + info = os.stat(name, dir_fd=folder, follow_symlinks=False) + if stat.S_ISLNK(info.st_mode): + return SYMLINK, os.readlink(name, dir_fd=folder).encode("utf-8", "surrogateescape"), info + fd = os.open(name, os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=folder) + try: + info = os.fstat(fd) + if not stat.S_ISREG(info.st_mode): + return None + mode = EXECUTABLE if info.st_mode & stat.S_IXUSR else REGULAR + if info.st_size > BLOB_LIMIT: + return mode, None, info + with open(fd, "rb", closefd=False) as handle: + data = handle.read(BLOB_LIMIT + 1) + return mode, data if len(data) <= BLOB_LIMIT else None, info + finally: + os.close(fd) + + +def toplevel(cwd: Path) -> Path: + return Path(os.path.realpath(git(cwd, ["rev-parse", "--show-toplevel"], None).decode("utf-8", "surrogateescape").strip())) + + +def binding(cwd: Path, everything: bool) -> str: + top = toplevel(cwd) + tree = git(top, ["write-tree"], None).decode("ascii").strip() + if not everything: + return tree + new, _ = working_side(top, denied_roots(top)) + return f"{tree} {hashlib.sha256(json.dumps(sorted(new.items())).encode('utf-8', 'surrogateescape')).hexdigest()}" + + +def bound_diff(cwd: Path, everything: bool, pinned: str, rules: Path | None) -> list[dict]: + if not everything: + return tree_diff(cwd, pinned, rules) + diff = review_diff(cwd, True, rules) + if binding(cwd, True) != pinned: + raise DiffError("the working tree changed while its preview was read") + return diff + + +def sides(cwd: Path, everything: bool, tree: str | None) -> tuple[dict, dict, dict[str, bytes | None]]: + top = toplevel(cwd) + roots = denied_roots(top) + old = head_tree(top) + if tree is not None: + new, contents = listed(git(top, ["ls-tree", "-r", "-z", "--full-tree", tree], None), 2), {} + elif everything: + new, contents = working_side(top, roots) + else: + new, contents = listed(git(top, ["ls-files", "-s", "-z"], None), 1), {} + contents.update(dict.fromkeys(side[path][1] for side in (old, new) for path in side if sandbox_paths.denied(os.path.join(top, path), roots))) + return old, new, contents + + +def denied_roots(top: Path) -> list[str]: + try: + roots = sandbox_paths.denied_roots(top) + except sandbox_paths.Unreadable as error: + raise DiffError(str(error)) from error + if sandbox_paths.denied(str(top), roots): + raise DiffError(f"the working tree {top} lies under a path the sandbox denies reading") + return roots + + +def head_tree(top: Path) -> dict[str, tuple[str, str]]: + """Returns: + The paths of HEAD's tree; none for an unborn HEAD, a symbolic ref + to a branch that does not exist yet. + + Raises: + DiffError: when HEAD resolves but its tree cannot be listed, or + neither resolves nor is a symbolic ref. + """ + try: + git(top, ["rev-parse", "-q", "--verify", "HEAD"], None) + except DiffError: + git(top, ["symbolic-ref", "-q", "HEAD"], None) + return {} + return listed(git(top, ["ls-tree", "-r", "-z", "--full-tree", "HEAD"], None), 2) + + +def working_side(top: Path, roots: list[str]) -> tuple[dict, dict[str, bytes | None]]: + algorithm = git(top, ["rev-parse", "--show-object-format"], None).decode("ascii").strip() + new, contents = {}, {} + for path, (mode, oid) in listed(git(top, ["ls-files", "-s", "-z"], None), 1).items(): + found = (mode, oid, None) if mode == GITLINK else worktree(top, path, mode, algorithm, roots) + if found is not None: + new[path] = found[:2] + contents[found[1]] = found[2] + return new, contents + + +def text_lines(text: str) -> list[str]: + lines = text.split("\n") + return lines[:-1] if lines[-1] == "" else lines + + +def secret_rules(root: Path | None) -> Rules | None: + """Returns: + Per MEDUSA rule whose cwe names hard-coded credentials, its file + globs and compiled patterns (flags as MEDUSA compiles them); cached + in CACHE until a rule file changes. None without rules or PyYAML, + or when a rule file cannot be read or parsed. + """ + if root is None or not root.is_dir(): + return None + files = sorted(root.rglob("*.yaml")) + signature = hashlib.sha256(json.dumps([[str(file.relative_to(root)), file.stat().st_size, file.stat().st_mtime_ns] for file in files]).encode()).hexdigest() + try: + cached = json.loads(CACHE.read_text(encoding="utf-8")) + if cached.get("signature") == signature: + return loaded(cached["rules"]) + except (OSError, ValueError, AttributeError, KeyError, TypeError): + pass + raw = parsed_rules(files) + if raw is None: + return None + try: + CACHE.parent.mkdir(parents=True, exist_ok=True) + CACHE.write_text(json.dumps({"signature": signature, "rules": raw}), encoding="utf-8") + except OSError: + pass + return loaded(raw) + + +def loaded(raw: list) -> Rules: + return [(tuple(map(str, globs)), compiled(list(map(str, patterns)))) for globs, patterns in raw] + + +def parsed_rules(files: list[Path]) -> list[list] | None: + try: + import yaml + except ImportError: + return None + found = [] + for file in files: + try: + text = file.read_text(encoding="utf-8", errors="replace") + data = (yaml.safe_load(text) or {}) if SECRET_CWE.search(text) else {} + except (OSError, RecursionError, yaml.YAMLError): + return None + if not isinstance(data, list | dict): + return None + for rule in data if isinstance(data, list) else data.get("rules") or []: + if isinstance(rule, dict) and SECRET_CWE.match(str(rule.get("cwe") or "")) and rule.get("patterns"): + types = rule.get("file_types") or ["*"] + globs = [str(kind) if "*" in str(kind) else f"*.{str(kind).lstrip('.')}" for kind in types] + found.append([globs, [str(pattern) for pattern in rule["patterns"]]]) + return found + + +def compiled(patterns: list[str]) -> Patterns: + found = [] + for pattern in patterns: + try: + found.append(re.compile(pattern, re.IGNORECASE | re.MULTILINE)) + except re.error: + continue + return found + + +def rules_root() -> Path | None: + found = sorted(RULES_HOME.glob(RULES_TAIL)) + return found[-1] if found else None + + +def rule_patterns(path: str, rules: Rules) -> Patterns: + name = Path(path).name.lower() + return [pattern for globs, found in rules if any(fnmatch.fnmatchcase(name, glob.lower()) for glob in globs) for pattern in found] + + +def withheld(path: str, texts: tuple[str, str], rules: Rules | None) -> bool: + if not rules or not any(patterns for _, patterns in rules): + return True + patterns = [ARMOUR, *rule_patterns(path, rules)] + return any(pattern.search(text) for text in texts for pattern in patterns) + + +def entry(head: dict, before: bytes | None, after: bytes | None, rules: Rules | None) -> tuple[dict, list[str]]: + """Args: + head: the entry's path, status and mode. + + Returns: + The diff entry without its patch, and the patch lines; no lines for + a binary side, one over BLOB_LIMIT or content withheld. + """ + found = {**head, "additions": 0, "deletions": 0, "note": ""} + if before is None or after is None: + return found, [] + if b"\0" in before[:BINARY_PROBE] or b"\0" in after[:BINARY_PROBE]: + return {**found, "status": "B"}, [] + texts = (before.decode("utf-8", "replace"), after.decode("utf-8", "replace")) + lines = list(difflib.unified_diff(*map(text_lines, texts), lineterm="", n=CONTEXT))[2:] + found.update(additions=sum(1 for line in lines if line.startswith("+")), deletions=sum(1 for line in lines if line.startswith("-"))) + if withheld(head["path"], texts, rules): + return {**found, "note": WITHHELD}, [] + return found, lines + + +def content_of(side: tuple[str, str] | None, gitlink: bool, contents: dict[str, bytes | None]) -> bytes | None: + if side is None: + return b"" + return None if gitlink else contents.get(side[1]) + + +def review_diff(cwd: Path, everything: bool, rules: Path | None) -> list[dict]: + return built(cwd, sides(cwd, everything, None), rules) + + +def tree_diff(cwd: Path, tree: str, rules: Path | None) -> list[dict]: + return built(cwd, sides(cwd, False, tree), rules) + + +def fetched(cwd: Path, old: dict, new: dict, changed: list[str], contents: dict[str, bytes | None]) -> None: + needed = {side[path][1] for side in (old, new) for path in changed if path in side and side[path][0] != GITLINK and side[path][1] not in contents} + sizes = blob_sizes(cwd, needed) + contents.update(blobs(cwd, {oid for oid in needed if sizes[oid] <= BLOB_LIMIT})) + + +def shown_path(path: str) -> str: + shown = path.encode("utf-8", "surrogateescape").decode("utf-8", "backslashreplace") + if len(shown) > PATH_LIMIT: + raise DiffError(f"a changed path runs past {PATH_LIMIT} characters, the most a preview takes") + return shown + + +def path_entry(path: str, before: tuple[str, str] | None, after: tuple[str, str] | None, contents: dict[str, bytes | None], rules: Rules | None) -> tuple[dict, list[str]]: + status = "A" if before is None else "D" if after is None else "M" + gitlink = any(side is not None and side[0] == GITLINK for side in (before, after)) + modes = list(dict.fromkeys(side[0] for side in (before, after) if side is not None)) + head = {"path": shown_path(path), "status": status, "mode": " => ".join(modes)} + return entry(head, content_of(before, gitlink, contents), content_of(after, gitlink, contents), rules) + + +def built(cwd: Path, found_sides: tuple[dict, dict, dict[str, bytes | None]], rules: Path | None) -> list[dict]: + old, new, contents = found_sides + changed = sorted(path for path in {*old, *new} if old.get(path) != new.get(path)) + if len(changed) > FILE_CAP: + raise DiffError(f"{len(changed)} paths changed, a preview lists at most {FILE_CAP}") + for path in changed: + shown_path(path) + fetched(cwd, old, new, changed, contents) + secrets = secret_rules(rules) + diff, spent, full = [], 0, False + for path in changed: + found, lines = path_entry(path, old.get(path), new.get(path), contents, secrets) + patch = "\n".join(lines) + size = len(json.dumps(patch, ensure_ascii=False).encode("utf-8")) + full = full or spent + size > DIFF_LIMIT + if full: + patch = "" + else: + spent += size + diff.append({**found, "patch": patch}) + return diff + + +def commit_of(command: str, cwd: str) -> tuple[Path, bool] | None: + """Returns: + The directory git commit runs in and whether it records every + tracked change (-a or --all), for a command that is one git commit, + optionally behind variable assignments, env -C and git -C; None for + any other command and for one whose commit the preview would not + match (a pathspec, -o, -i, -p, --no-all, an unknown or ambiguous long + option, git -c, or a GIT_ variable outside KEPT_VARS). + """ + try: + words = shlex.split(command) + except ValueError: + return None + if any(word in {";", "&&", "||", "|", "&"} for word in words): + return None + place, words = launched(Path(cwd), words) + if not words or Path(words[0]).name != "git": + return None + place, rest = past_git_options(place, words[1:]) + mode = commit_mode(rest[1:]) if rest else None + return None if mode is None else (place, mode) + + +def launched(place: Path, words: list[str]) -> tuple[Path, list[str]]: + """Returns: + The directory and the words of the program a command starts, past + variable assignments and env with -C or --chdir; no words when an + assignment names a GIT_ variable outside KEPT_VARS or env takes + another option. + """ + words = past_assignments(words) + if words and Path(words[0]).name == "env": + place, words = past_env_options(place, words[1:]) + words = past_assignments(words) + return place, words + + +def past_assignments(words: list[str]) -> list[str]: + while words and (assigned := ASSIGNMENT.fullmatch(words[0])): + if assigned.group(1).startswith("GIT_") and assigned.group(1) not in KEPT_VARS: + return [] + words = words[1:] + return words + + +def past_env_options(place: Path, words: list[str]) -> tuple[Path, list[str]]: + while words and words[0].startswith("-"): + if words[0] in ("-C", "--chdir") and len(words) > 1: + place, words = place / words[1], words[2:] + elif words[0].startswith("--chdir="): + place, words = place / words[0].partition("=")[2], words[1:] + else: + return place, [] + return place, words + + +def past_git_options(place: Path, rest: list[str]) -> tuple[Path, list[str]]: + while rest and rest[0] != "commit": + if rest[0] == "-C" and len(rest) > 1: + place, rest = place / rest[1], rest[2:] + elif rest[0] in ("--no-pager", "-P", "--no-optional-locks"): + rest = rest[1:] + else: + return place, [] + return place, rest + + +def commit_mode(words: list[str]) -> bool | None: + """Returns: + True for -a or --all, False for the staged tree, None when the + commit records something else (NARROWING, -o, -i, -p or a pathspec). + """ + skip, everything = False, False + for at, word in enumerate(words): + if skip: + skip = False + continue + if word == "--": + return None if words[at + 1 :] else everything + found = option(word) + if found is None: + return None + everything, skip = everything or found[0], found[1] + return everything + + +def option(word: str) -> tuple[bool, bool] | None: + """Returns: + Whether a git commit option word is -a or --all and whether the next + word is its argument, a long option named in full or by a prefix of + exactly one of COMMIT_LONGS (what git 2.56 lists for git commit + --git-completion-helper-all); None for NARROWING, -o, -i, -p, a + pathspec and a long option it names no single one of. + """ + if word.startswith("--"): + name = long_option(word[2:].partition("=")[0]) + return None if name is None or name in NARROWING else (name == "all", name in LONG_ARGS and "=" not in word) + if word.startswith("-") and word != "-": + return cluster(word[1:]) + return None + + +def long_option(name: str) -> str | None: + if name in COMMIT_LONGS: + return name + found = [known for known in COMMIT_LONGS if known.startswith(name)] + return found[0] if len(found) == 1 else None + + +def cluster(letters: str) -> tuple[bool, bool] | None: + """Returns: + Whether a short option cluster holds -a and whether the next word is + its argument; None when it holds -o, -i or -p. + """ + everything = False + for at, letter in enumerate(letters): + if letter in "oip": + return None + everything = everything or letter == "a" + if letter in SHORT_ARGS: + return everything, at == len(letters) - 1 and letter in "mFcCtU" + return everything, False diff --git a/orchestrator/kvb_orchestrator/asking.py b/orchestrator/kvb_orchestrator/asking.py index 49ec1c5..8086832 100644 --- a/orchestrator/kvb_orchestrator/asking.py +++ b/orchestrator/kvb_orchestrator/asking.py @@ -3,6 +3,7 @@ from __future__ import annotations import time from .choosing import EVERYONE, answer_text +from .diffs import store_diff, valid_diff from .errors import Conflict, Missing, ToolError from .fields import ( COMMAND_LIMIT, @@ -60,12 +61,16 @@ def option_of(option: object) -> dict: class Asking: def ask(self, body: bytes) -> dict: payload = decode(body) + diff = valid_diff(payload.pop("diff")) if "diff" in payload else None expect(payload, ("tool", *ASK_TEXTS, "timeout")) values = {name: bounded(payload, name, limit) for name, limit in ASK_TEXTS.items()} lifetime = seconds(payload, "timeout", ASK_LIFETIME) created = time.time() - fields = {**values, "tool": field(payload, "tool").strip(), "status": "pending", "decided": None, "proof": None} - record = self.asks.add({**fields, "created_ts": created, "expires": created + lifetime}) + fields = {**values, "tool": field(payload, "tool").strip(), "status": "pending", "decided": None, "proof": None, "diff_files": None if diff is None else len(diff)} + ask_id = self.asks.fresh() + if diff is not None: + store_diff(self.reviews.pages, "ask", ask_id, diff) + record = self.asks.insert(ask_id, {**fields, "created_ts": created, "expires": created + lifetime}) self.automate("ask", record) return {"id": record["id"], "status": record["status"]} diff --git a/orchestrator/kvb_orchestrator/diffs.py b/orchestrator/kvb_orchestrator/diffs.py new file mode 100644 index 0000000..b801ae8 --- /dev/null +++ b/orchestrator/kvb_orchestrator/diffs.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +import json +import re +from pathlib import Path + +from .errors import Missing, ToolError +from .records import read_json, write_atomic +from .signed import limits + +DIFF_LIMIT = limits.DIFF_LIMIT +DIFF_BODY_LIMIT = limits.DIFF_BODY_LIMIT +FILE_CAP = limits.FILE_CAP +PATH_LIMIT = limits.PATH_LIMIT +STATUSES = ("A", "M", "D", "R", "B") +DIFF_KEYS = ("path", "status", "mode", "additions", "deletions", "patch", "note") +NOTES = ("", limits.WITHHELD) +MODE = re.compile(r"[0-7]{6}(?: => [0-7]{6})?") +DIFF_SUFFIX = {"review": ".review-diff.json", "ask": ".ask-diff.json"} + + +def counted(value: object) -> bool: + return isinstance(value, int) and not isinstance(value, bool) and value >= 0 + + +def diff_entry(entry: object) -> dict: + if not isinstance(entry, dict) or set(entry) != set(DIFF_KEYS): + raise ToolError(f"every diff entry must be an object with exactly {', '.join(DIFF_KEYS)}") + path, patch, mode = entry["path"], entry["patch"], entry["mode"] + if not isinstance(path, str) or not 0 < len(path) <= PATH_LIMIT or entry["status"] not in STATUSES: + raise ToolError(f"a diff path must be 1 to {PATH_LIMIT} characters and its status one of {', '.join(STATUSES)}") + if not isinstance(mode, str) or not MODE.fullmatch(mode): + raise ToolError("mode must be a git file mode, or old => new for a changed one") + if not counted(entry["additions"]) or not counted(entry["deletions"]) or not isinstance(patch, str) or (entry["status"] == "B" and patch): + raise ToolError("additions and deletions must be counts, patch text, empty for a binary file") + if entry["note"] not in NOTES or (entry["note"] and patch): + raise ToolError(f"note must be empty or {limits.WITHHELD!r}, the latter with an empty patch") + return {key: entry[key] for key in DIFF_KEYS} + + +def valid_diff(value: object) -> list[dict]: + """Returns: + The review diff of requirement 003, checked: at most FILE_CAP + entries, and at most DIFF_LIMIT bytes of JSON-encoded patch text in + all, the bound the requesting script keeps. + """ + if not isinstance(value, list) or len(value) > FILE_CAP: + raise ToolError(f"diff must be a list of at most {FILE_CAP} files") + entries = [diff_entry(entry) for entry in value] + if sum(len(json.dumps(entry["patch"], ensure_ascii=False).encode("utf-8")) for entry in entries) > DIFF_LIMIT: + raise ToolError(f"the patches of a diff must stay within {DIFF_LIMIT} bytes") + return entries + + +def diff_bytes(diff: list[dict]) -> bytes: + return json.dumps({"diff": diff}, ensure_ascii=False).encode("utf-8") + + +def store_diff(folder: Path, kind_name: str, item_id: str, diff: list[dict]) -> None: + write_atomic(folder / f"{item_id}{DIFF_SUFFIX[kind_name]}", diff_bytes(diff)) + + +def stored_diff(folder: Path, kind_name: str, item_id: str) -> dict: + data = read_json(folder / f"{item_id}{DIFF_SUFFIX[kind_name]}") if kind_name in DIFF_SUFFIX else None + if not isinstance(data, dict) or not isinstance(data.get("diff"), list): + raise Missing(item_id) + return {"kind": kind_name, "id": item_id, "diff": data["diff"]} diff --git a/orchestrator/kvb_orchestrator/fields.py b/orchestrator/kvb_orchestrator/fields.py index 492e8ef..9c5bda1 100644 --- a/orchestrator/kvb_orchestrator/fields.py +++ b/orchestrator/kvb_orchestrator/fields.py @@ -5,14 +5,15 @@ import math import re from .errors import ToolError +from .signed import limits WAIT_CAP = 300 TEXT_LIMIT = 20_000 COMMAND_LIMIT = 4000 -PATH_LIMIT = 4096 +PATH_LIMIT = limits.PATH_LIMIT NAME_LIMIT = 120 COUNT = re.compile(r"[0-9]{1,12}") -PATH = re.compile(r"/[^\x00-\x1f\x7f]{0,4095}") +PATH = re.compile(rf"/[^\x00-\x1f\x7f]{{0,{PATH_LIMIT - 1}}}") LINE = r"(?=[^\x00-\x1f\x7f]*\S)[^\x00-\x1f\x7f]{{1,{0}}}" ORDER_NAME = re.compile(r"(?![\s-])[^,\x00-\x1f\x7f]{1,200}") FIELDS = { diff --git a/orchestrator/kvb_orchestrator/handler.py b/orchestrator/kvb_orchestrator/handler.py index 5b35909..8580a67 100644 --- a/orchestrator/kvb_orchestrator/handler.py +++ b/orchestrator/kvb_orchestrator/handler.py @@ -6,6 +6,7 @@ from collections.abc import Callable from http import HTTPStatus from urllib.parse import urlsplit +from .diffs import DIFF_BODY_LIMIT from .engines import STT_LIMIT from .exchange import PLAIN_CSP, Exchange @@ -56,6 +57,7 @@ BOARD_SIGN_ROUTE = "/api/board/sign" SETTINGS_SIGN_ROUTE = "/api/settings/sign" CHOICES_ROUTE = "/api/choices" CHOICES_SIGN_ROUTE = "/api/choices/sign" +DIFF_ROUTE = re.compile(r"/api/diffs/(review|ask)/([0-9a-f]{16})") WAV_POSTS = (STT_ROUTE, VOICE_ROUTE) ACTION_POSTS = ("/api/rules", "/api/nl", SETTINGS_ROUTE, TTS_ROUTE, STT_ROUTE, SIGN_ROUTE, RULE_SIGN_ROUTE, CLEANUP_ROUTE, OFFICES_ROUTE, TASKS_ROUTE, VOICE_ROUTE, MESSAGE_SIGN_ROUTE, SETTINGS_SIGN_ROUTE, CHOICES_ROUTE, CHOICES_SIGN_ROUTE) @@ -83,10 +85,13 @@ class Handler(Exchange): cleanup = CLEANUP_STATE.fullmatch(path) office = OFFICE_STATE.fullmatch(path) voice = VOICE_ITEM.fullmatch(path) + diff = DIFF_ROUTE.fullmatch(path) if self.host() is None: self.send_text(HTTPStatus.FORBIDDEN, "forbidden host") elif path == "/": self.index() + elif diff: + self.answer(lambda: hub.diff_of(diff.group(1), diff.group(2)), HTTPStatus.OK) elif vendored and vendored.group(1) in VENDOR: self.vendored(*VENDOR[vendored.group(1)]) elif voice and voice.group(2): @@ -237,7 +242,7 @@ class Handler(Exchange): def do_POST(self) -> None: path = urlsplit(self.path).path - length = self.body_length({UPLOADS: UPLOAD_LIMIT, STT_ROUTE: STT_LIMIT, VOICE_ROUTE: STT_LIMIT}.get(path, BODY_LIMIT)) + length = self.body_length({UPLOADS: UPLOAD_LIMIT, STT_ROUTE: STT_LIMIT, VOICE_ROUTE: STT_LIMIT, "/mcp": DIFF_BODY_LIMIT, "/api/asks": DIFF_BODY_LIMIT}.get(path, BODY_LIMIT)) if length is None: return host = self.host() diff --git a/orchestrator/kvb_orchestrator/inbox.py b/orchestrator/kvb_orchestrator/inbox.py index d4b8467..5f016f8 100644 --- a/orchestrator/kvb_orchestrator/inbox.py +++ b/orchestrator/kvb_orchestrator/inbox.py @@ -19,16 +19,17 @@ SNOOZE_LIMIT = 8 * 86400 class Inbox: def inbox(self, now: float, tasks: dict[str, dict], directory: Directory) -> list[dict]: claims = read_claims(self.council) - base = {"agent": "", "to": "", "detail": "", "green": False, "gate": "", "score": "", "warnings": [], "options": [], "recommended": "", "command": "", "directory": "", "sha256": "", "runnable": False, "deniable": False, "job": None, "rule": None, "dry_run": None} + base = {"agent": "", "to": "", "detail": "", "green": False, "gate": "", "score": "", "warnings": [], "options": [], "recommended": "", "command": "", "directory": "", "sha256": "", "runnable": False, "deniable": False, "job": None, "rule": None, "dry_run": None, "diff": None} items = [] for review in self.reviews.pending(): created = iso_epoch(review["created"]) or now gate = review.get("gate", "") - fields = {"title": review["title"], "detail": review["branch"], "green": gate == "pass", "gate": gate, "score": review.get("score", ""), "directory": review["worktree"]} + agent = review.get("agent", "") + fields = {"title": review["title"], "detail": review["branch"], "green": gate == "pass", "gate": gate, "score": review.get("score", ""), "directory": review["worktree"], "agent": agent, "to": requester_of(claims, agent)[0], "diff": review.get("diff_files")} items.append({**base, "kind": "review", "id": review["id"], "ts": created, **fields}) for ask in (ask for ask in self.asks.pending() if ask["expires"] > now): warnings = signing.warnings(ask["command"]) - fields = {"title": f"{ask['tool']}: {ask['command'][:200]}", "agent": directory.now(ask["session"]), "to": requester_of(claims, ask["session"])[0], "green": not warnings, "warnings": warnings, "deniable": True} + fields = {"title": f"{ask['tool']}: {ask['command'][:200]}", "agent": directory.now(ask["session"]), "to": requester_of(claims, ask["session"])[0], "green": not warnings, "warnings": warnings, "deniable": True, "diff": ask.get("diff_files")} items.append({**base, "kind": "ask", "id": ask["id"], "ts": ask["created_ts"], **fields, "command": ask["command"], "directory": ask["cwd"]}) for decision in self.decisions.pending(): recommended = decision.get("recommended", "") diff --git a/orchestrator/kvb_orchestrator/ledger.py b/orchestrator/kvb_orchestrator/ledger.py index 1c8def1..3d85724 100644 --- a/orchestrator/kvb_orchestrator/ledger.py +++ b/orchestrator/kvb_orchestrator/ledger.py @@ -110,11 +110,17 @@ class Reviews(Ledger): def stored(self, review_id: str, suffix: str) -> Path: return self.pages / f"{review_id}{suffix}" - def submit(self, fields: dict[str, str], page: bytes, files: list[dict]) -> dict: + def submit(self, fields: dict[str, str], page: bytes, files: list[dict], attached: dict[str, bytes]) -> dict: + """Args: + attached: further files written before the review shows, by + suffix. + """ with self.changed: review_id = self.fresh() write_atomic(self.stored(review_id, ".html"), page) write_atomic(self.stored(review_id, ".json"), json.dumps({"files": files}).encode("utf-8")) + for suffix, data in attached.items(): + write_atomic(self.stored(review_id, suffix), data) return self.insert(review_id, {**fields, "files": len(files), "status": "pending", "decided": None}) diff --git a/orchestrator/kvb_orchestrator/reviewing.py b/orchestrator/kvb_orchestrator/reviewing.py index dea568a..fa5fa99 100644 --- a/orchestrator/kvb_orchestrator/reviewing.py +++ b/orchestrator/kvb_orchestrator/reviewing.py @@ -6,16 +6,18 @@ import stat import time from pathlib import Path +from .diffs import DIFF_SUFFIX, diff_bytes, stored_diff, valid_diff from .errors import Conflict, Missing, ToolError from .fields import WAIT_CAP, bounded, decode, expect, field, seconds from .review_page import review_files -from .signed import review_signed +from .signed import limits, review_signed PAGE_LIMIT = 32 << 20 -NOTES_LIMIT = 100_000 +NOTES_LIMIT = limits.NOTES_LIMIT SCORE_LIMIT = 500 GATES = ("pass", "fail") REQUEST = ("repo", "worktree", "branch", "tree", "title", "page", "notes", "gate", "score") +OPTIONAL = ("agent", "diff") LISTED = ("id", "created", "worktree", "branch", "tree", "title") STATE = ("id", "status", "tree", "decided") @@ -55,7 +57,7 @@ class Reviewing: return data def request(self, arguments: dict) -> dict: - expect(arguments, REQUEST) + expect(arguments, (*REQUEST, *(name for name in OPTIONAL if name in arguments))) fields = {name: field(arguments, name) for name in ("repo", "worktree", "branch", "tree", "title")} notes = arguments["notes"] if not isinstance(notes, str) or len(notes) > NOTES_LIMIT: @@ -63,12 +65,18 @@ class Reviewing: if arguments["gate"] not in GATES: raise ToolError("gate must be pass or fail") score = " ".join(bounded(arguments, "score", SCORE_LIMIT).split()) + agent = field(arguments, "agent").strip() if "agent" in arguments else "" + diff = valid_diff(arguments["diff"]) if "diff" in arguments else None page = self.read_page(field(arguments, "page")) - extra = {"notes": notes, "gate": arguments["gate"], "score": score} - review = self.reviews.submit({**fields, "title": fields["title"].strip(), **extra}, page, review_files(page)) + extra = {"notes": notes, "gate": arguments["gate"], "score": score, "agent": agent, "diff_files": None if diff is None else len(diff)} + attached = {} if diff is None else {DIFF_SUFFIX["review"]: diff_bytes(diff)} + review = self.reviews.submit({**fields, "title": fields["title"].strip(), **extra}, page, review_files(page), attached) self.automate("review", review) return {"id": review["id"], "status": review["status"], "url": self.url()} + def diff_of(self, kind_name: str, item_id: str) -> dict: + return stored_diff(self.reviews.pages, kind_name, item_id) + def review_proof(self, review_id: str) -> dict: review = self.reviews.get(review_id) if review is None: diff --git a/orchestrator/kvb_orchestrator/signed.py b/orchestrator/kvb_orchestrator/signed.py index 6994501..a356069 100644 --- a/orchestrator/kvb_orchestrator/signed.py +++ b/orchestrator/kvb_orchestrator/signed.py @@ -30,6 +30,7 @@ def client_module(name: str, uses: dict[str, ModuleType]) -> ModuleType: return module +limits = client_module("hub_limits", {}) signing = client_module("signing", {}) passkey = client_module("passkey", {"signing": signing}) verifier = client_module("verifier", {"signing": signing, "passkey": passkey}) diff --git a/orchestrator/kvb_orchestrator/view.py b/orchestrator/kvb_orchestrator/view.py index 9c65776..f6a57e2 100644 --- a/orchestrator/kvb_orchestrator/view.py +++ b/orchestrator/kvb_orchestrator/view.py @@ -51,7 +51,7 @@ INDEX_CSP = ( def badges(inbox: list[dict], claims: list[dict], jobs: list[dict], now: float) -> dict[str, int]: return { - "inbox": sum(1 for item in inbox if not item["until"]), + "tasks": sum(1 for item in inbox if not item["until"]), "agents": len({claim["agent"] for claim in claims if claim["beat"] is not None and claim["beat"] >= now - ACTIVE}), "artifacts": sum(1 for job in jobs if job["status"] in JOB_OPEN), } diff --git a/orchestrator/templates/diffs.js b/orchestrator/templates/diffs.js new file mode 100644 index 0000000..fb6ed13 --- /dev/null +++ b/orchestrator/templates/diffs.js @@ -0,0 +1,60 @@ +const Diffs = (() => { + "use strict"; + const COLLAPSE = 400; + const QUOTE_LIMIT = 2000; + const HUNK = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/; + const KINDS = {"+": "add", "-": "del", " ": "ctx"}; + + function rows(patch) { + const found = []; + let [old, now] = [0, 0]; + String(patch).split("\n").forEach((text, index) => { + const line = index + 1; + const hunk = HUNK.exec(text); + if (hunk) { + [old, now] = [Number(hunk[1]), Number(hunk[2])]; + found.push({kind: "hunk", line, old: "", new: "", text}); + return; + } + const kind = KINDS[text[0]]; + if (!kind) { + return; + } + const row = {kind, line, old: kind === "add" ? "" : old, new: kind === "del" ? "" : now, text: text.slice(1)}; + old += kind === "add" ? 0 : 1; + now += kind === "del" ? 0 : 1; + found.push(row); + }); + return found; + } + + const collapsed = (file) => file.additions + file.deletions > COLLAPSE; + + function span(numbers) { + const known = numbers.filter((value) => Number.isInteger(value) && value > 0); + return known.length ? [Math.min(...known), Math.max(...known)] : null; + } + + function lineSpan(text, quote) { + const full = String(text); + const at = full.indexOf(quote); + if (!quote || at < 0) { + return [1, full.split("\n").length]; + } + const from = full.slice(0, at).split("\n").length; + return [from, from + quote.split("\n").length - 1]; + } + + function quoted(text) { + const clean = String(text).trim(); + return clean.length > QUOTE_LIMIT ? `${clean.slice(0, QUOTE_LIMIT)}…` : clean; + } + + const feedback = (ref, path, from, to, quote, words) => `Feedback on ${ref} ${path}:${from}-${to}: "${quoted(quote)}" ${String(words).trim()}`; + const feedbackOf = (messages, ref) => messages.filter((message) => String(message.text).startsWith(`Feedback on ${ref} `)); + + return {COLLAPSE, QUOTE_LIMIT, rows, collapsed, span, lineSpan, quoted, feedback, feedbackOf}; +})(); +if (typeof module === "object" && module.exports) { + module.exports = Diffs; +} diff --git a/orchestrator/templates/index.html.j2 b/orchestrator/templates/index.html.j2 index f3640e2..c128870 100644 --- a/orchestrator/templates/index.html.j2 +++ b/orchestrator/templates/index.html.j2 @@ -4,7 +4,7 @@ -{% if state.badges.inbox %}({{ state.badges.inbox }}) {% endif %}Agent Orchestrator +{% if state.badges.tasks %}({{ state.badges.tasks }}) {% endif %}Agent Orchestrator @@ -455,6 +457,8 @@ pre.notes { margin: 0; padding: 8px 10px; background: var(--sunken); border-radi {% include "bubbles.js" %} +{% include "diffs.js" %} + {% include "nav.js" %} {% include "popup.js" %} @@ -502,6 +506,8 @@ pre.notes { margin: 0; padding: 8px 10px; background: var(--sunken); border-radi {% include "page-review.js" %} +{% include "page-diff.js" %} + {% include "page-filters.js" %} {% include "page-stream.js" %} diff --git a/orchestrator/templates/nav.js b/orchestrator/templates/nav.js index 5550599..9ae050c 100644 --- a/orchestrator/templates/nav.js +++ b/orchestrator/templates/nav.js @@ -1,7 +1,8 @@ const Nav = (() => { "use strict"; const TABS = [["stream", "Stream"], ["agents", "Agents"], ["artifacts", "Artifacts"], ["tasks", "Tasks"]]; - const SUBS = {stream: [["inbox", "Inbox"], ["rules", "Firewall"]]}; + const SUBS = {stream: [["rules", "Firewall"]]}; + const PANES = {inbox: ["tasks", "operator"]}; const VIEWS = [["orbit", "Orbit"], ["chord", "Chord"], ["swimlane", "Swimlane"], ["tree", "Tree"], ["3d", "3D"]]; const TASK_VIEWS = [["list", "List"], ["board", "Board"], ["rasci", "RASCI"], ["gantt", "Gantt"], ["burn", "Burndown and burnup"], ["3d", "3D"]]; const OWN = {agents: [["hierarchy", "Hierarchy"]]}; @@ -15,10 +16,13 @@ const Nav = (() => { const known = (screen, view) => viewsOf(screen).some(([id]) => id === view); function tabOf(screen) { - return Object.keys(SUBS).find((tab) => SUBS[tab].some(([id]) => id === screen)) || screen; + return Object.hasOwn(PANES, screen) ? PANES[screen][0] : Object.keys(SUBS).find((tab) => SUBS[tab].some(([id]) => id === screen)) || screen; } function hashOf(screen, view) { + if (Object.hasOwn(PANES, screen)) { + return `#${PANES[screen].join("/")}`; + } return tabOf(screen) === screen ? (view ? `#${screen}/${view}` : `#${screen}`) : `#${tabOf(screen)}/${screen}`; } @@ -27,6 +31,10 @@ const Nav = (() => { if (head === "stream" && sub === "radio") { return {screen: "stream", view: null, redirect: "#stream", spot: true}; } + const pane = Object.keys(PANES).find((screen) => PANES[screen][0] === head && PANES[screen][1] === sub); + if (pane || (head === "stream" && sub === "inbox")) { + return {screen: pane || "inbox", view: null, redirect: pane ? null : hashOf("inbox", null)}; + } if (Object.hasOwn(OLD, head)) { const [screen, fallback] = OLD[head]; const view = known(screen, sub) ? sub : fallback; @@ -41,7 +49,7 @@ const Nav = (() => { return {screen: "stream", view: null, redirect: null}; } - return {TABS, SUBS, VIEWS, TASK_VIEWS, ICONS, viewsOf, route, hashOf, tabOf}; + return {TABS, SUBS, PANES, VIEWS, TASK_VIEWS, ICONS, viewsOf, route, hashOf, tabOf}; })(); if (typeof module === "object" && module.exports) { module.exports = Nav; diff --git a/orchestrator/templates/page-boot.js b/orchestrator/templates/page-boot.js index 4e9f936..dd99b0d 100644 --- a/orchestrator/templates/page-boot.js +++ b/orchestrator/templates/page-boot.js @@ -27,6 +27,8 @@ document.addEventListener("keydown", context, {once: true}); document.addEventListener("pointerdown", unlock); document.addEventListener("keydown", keyboard); + document.addEventListener("contextmenu", feedbackMenu); + document.addEventListener("selectionchange", feedbackButton); document.addEventListener("visibilitychange", refresh); window.addEventListener("focus", refresh); window.addEventListener("hashchange", apply); diff --git a/orchestrator/templates/page-constants.js b/orchestrator/templates/page-constants.js index 8db3dcc..355bd72 100644 --- a/orchestrator/templates/page-constants.js +++ b/orchestrator/templates/page-constants.js @@ -72,7 +72,7 @@ direct: [[740, 0, 0.2]], radio: [[520, 0, 0.07]], }; - const KEYS = [["Ctrl+K", "Commands, views, agents and AI command"], ["1 · 2 · 3 · 4", "Stream · Agents · Artifacts · Tasks"], ["j / k", "Inbox: next / previous entry"], ["x", "Inbox: select entry"], ["a", "Inbox: grant entry (3 s to undo)"], ["Enter", "Inbox: open entry"], ["/", "Search the stream"], ["h", "Look at the sky: drag turns the view, wheel or two fingers zoom, Esc ends"], ["Arrows · + / − · 0", "Sky (focused): turn · zoom · reset"], ["Drag · wheel", "3D: turn, right drag pans, wheel zooms"],["🎙 hold", "Voice input into the agent field or the command line, never sent automatically"], ["?", "This help"], ["Esc", "Close"]]; + const KEYS = [["Ctrl+K", "Commands, views, agents and AI command"], ["1 · 2 · 3 · 4", "Stream · Agents · Artifacts · Tasks"], ["j / k", "Waits on Operator: next / previous entry"], ["x", "Waits on Operator: select entry"], ["a", "Waits on Operator: grant entry (3 s to undo)"], ["Enter", "Waits on Operator: open entry"], ["f", "Feedback on the selected lines of a diff or a command"], ["/", "Search the stream"], ["h", "Look at the sky: drag turns the view, wheel or two fingers zoom, Esc ends"], ["Arrows · + / − · 0", "Sky (focused): turn · zoom · reset"], ["Drag · wheel", "3D: turn, right drag pans, wheel zooms"],["🎙 hold", "Voice input into the agent field or the command line, never sent automatically"], ["?", "This help"], ["Esc", "Close"]]; const calm = window.matchMedia("(prefers-reduced-motion: reduce)"); const relative = new Intl.RelativeTimeFormat("en", {numeric: "auto"}); const absolute = new Intl.DateTimeFormat("en-GB", {dateStyle: "medium", timeStyle: "short"}); diff --git a/orchestrator/templates/page-diff.js b/orchestrator/templates/page-diff.js new file mode 100644 index 0000000..e7426d0 --- /dev/null +++ b/orchestrator/templates/page-diff.js @@ -0,0 +1,161 @@ + const DIFF_TONES = {A: "ok", M: "info", D: "bad", R: "violet", B: "neutral"}; + const diffView = {cache: new Map(), version: 0, float: null, picked: null, timer: 0}; + + function diffOf(item) { + if (!item || typeof item.diff !== "number" || !["review", "ask"].includes(item.kind)) { + return null; + } + const key = keyOf(item); + if (!diffView.cache.has(key)) { + diffView.cache.set(key, "loading"); + fetch(`/api/diffs/${item.kind}/${encodeURIComponent(item.id)}`, {cache: "no-store"}) + .then((response) => (response.ok ? response.json() : {diff: null})) + .then((data) => diffView.cache.set(key, Array.isArray(data.diff) ? data.diff : null)) + .catch(() => diffView.cache.set(key, null)) + .then(() => { + diffView.version += 1; + drawInbox(); + drawStream(); + }); + } + return diffView.cache.get(key); + } + + function patchRow(row) { + const number = row.new || row.old || null; + if (row.kind === "hunk") { + return h("tr", {class: "hunk"}, h("td", {colspan: 3}), h("td", {class: "code"}, row.text)); + } + return h("tr", {class: row.kind, "data-line": number}, + h("td", {class: "num"}, row.old), h("td", {class: "num"}, row.new), h("td", {class: "sign"}, SIGNS[row.kind] || " "), h("td", {class: "code"}, row.text)); + } + + function diffBlock(file) { + const tone = DIFF_TONES[file.status] || "neutral"; + const head = [h("span", {class: "letter", "data-tone": tone}, file.status), h("code", {class: "path"}, file.path), + file.mode && file.mode !== "100644" ? h("code", {class: "mode", title: `mode: ${file.mode}`}, file.mode) : null, h("span", {class: "spacer"}), + file.note ? h("span", {class: "note", title: file.note}, icon("bi-eye-slash"), file.note) : null, + file.status === "B" ? icon("bi-file-earmark-binary") : h("span", {class: "stat"}, h("span", {class: "add"}, `+${file.additions}`), h("span", {class: "del"}, `-${file.deletions}`))]; + if (file.status === "B" || !file.patch) { + return h("section", {class: "diff-file", "data-path": file.path, "data-status": file.status, "data-patch": "none"}, h("div", {class: "diff-head"}, head)); + } + const table = h("table", {class: "diff"}, h("tbody", {}, Diffs.rows(file.patch).map(patchRow))); + return h("section", {class: "diff-file", "data-path": file.path, "data-status": file.status}, + h("details", {open: !Diffs.collapsed(file)}, h("summary", {class: "diff-head"}, head), h("div", {class: "diff-scroll"}, table))); + } + + function diffBlocks(item) { + const found = diffOf(item); + if (found === null || found === undefined) { + return null; + } + if (found === "loading") { + return h("p", {class: "muted"}, "…"); + } + return h("div", {class: "diff-files", "aria-label": `Diff: ${found.length}`}, found.map(diffBlock)); + } + + function feedbackList(ref) { + const sent = Diffs.feedbackOf(state.messages, ref); + return sent.length ? h("ul", {class: "feedback-list", "aria-label": `Feedback: ${sent.length}`}, sent.map((message) => h("li", {}, icon("bi-chat-left-text"), h("span", {class: "text"}, message.text), when(message.created_ts)))) : null; + } + + function selectionFeedback() { + const selection = window.getSelection(); + if (!selection || selection.isCollapsed || !selection.rangeCount || !selection.toString().trim()) { + return null; + } + const range = selection.getRangeAt(0); + const elementOf = (node) => (node instanceof Element ? node : node.parentElement); + const region = elementOf(range.startContainer) && elementOf(range.startContainer).closest("[data-path]"); + const card = region && region.closest("[data-ref]"); + if (!card || !card.dataset.to || !elementOf(range.endContainer) || elementOf(range.endContainer).closest("[data-path]") !== region) { + return null; + } + const quote = selection.toString(); + const code = region.querySelector("code"); + const lines = region.classList.contains("command") && code + ? Diffs.lineSpan(code.textContent, quote) + : Diffs.span([...region.querySelectorAll("tr[data-line]")].filter((row) => range.intersectsNode(row)).map((row) => Number(row.dataset.line))); + if (!lines) { + return null; + } + return {ref: card.dataset.ref, to: card.dataset.to, path: region.dataset.path, from: lines[0], until: lines[1], quote, rect: range.getBoundingClientRect()}; + } + + function feedbackSheet(picked) { + hideFeedback(); + const words = h("textarea", {class: "field", rows: 4, "aria-label": "Feedback", title: "Feedback"}); + const send = iconBtn("bi-send", "Send feedback", "btn primary", async () => { + if (!words.value.trim()) { + words.focus(); + return; + } + send.disabled = true; + const sent = await radioSend({to: picked.to, text: Diffs.feedback(picked.ref, picked.path, picked.from, picked.until, picked.quote, words.value), upload: "", cls: "post", voice: ""}); + send.disabled = false; + if (sent) { + closeSheet(); + } + }, `Send feedback: a post to ${picked.to}`); + openSheet("Feedback", [ + h("div", {class: "who"}, icon("bi-chat-left-text"), person(picked.to), h("code", {}, `${picked.path}:${picked.from}-${picked.until}`)), + h("pre", {class: "notes"}, Diffs.quoted(picked.quote)), + words, + h("div", {class: "actions"}, h("span", {class: "spacer"}), send)], false); + words.focus(); + } + + function hideFeedback() { + if (diffView.float) { + diffView.float.hidden = true; + } + diffView.picked = null; + } + + function showFeedback(picked, x, y, menu) { + if (!diffView.float) { + diffView.float = h("div", {class: "feedback-float", hidden: true}, + h("button", {type: "button", class: "btn small primary icon-only", ...named("Feedback", "Feedback (f): a post with the selected lines to the requesting agent"), on: { + mousedown: (event) => event.preventDefault(), + click: () => diffView.picked && feedbackSheet(diffView.picked), + }}, icon("bi-chat-left-text"))); + document.body.append(diffView.float); + } + diffView.picked = picked; + diffView.float.setAttribute("role", menu ? "menu" : "toolbar"); + diffView.float.firstChild.setAttribute("role", menu ? "menuitem" : "button"); + diffView.float.style.left = `${Math.max(Math.min(x, window.innerWidth - 44), 4)}px`; + diffView.float.style.top = `${Math.max(Math.min(y, window.innerHeight - 44), 4)}px`; + diffView.float.hidden = false; + } + + function feedbackButton() { + window.clearTimeout(diffView.timer); + diffView.timer = window.setTimeout(() => { + const picked = selectionFeedback(); + if (picked) { + showFeedback(picked, picked.rect.right + 4, picked.rect.bottom + 4, false); + } else if (!diffView.float || !diffView.float.contains(document.activeElement)) { + hideFeedback(); + } + }, 120); + } + + function feedbackMenu(event) { + const picked = selectionFeedback(); + if (!picked || !(event.target instanceof Element) || !event.target.closest("[data-path]")) { + return; + } + event.preventDefault(); + showFeedback(picked, event.clientX, event.clientY, true); + diffView.float.firstChild.focus(); + } + + function feedbackKey() { + const picked = selectionFeedback(); + if (picked) { + feedbackSheet(picked); + } + return Boolean(picked); + } diff --git a/orchestrator/templates/page-inbox.js b/orchestrator/templates/page-inbox.js index 40a2f6e..6f633ec 100644 --- a/orchestrator/templates/page-inbox.js +++ b/orchestrator/templates/page-inbox.js @@ -152,32 +152,35 @@ function inboxItem(item, index) { const red = item.kind === "review" && item.gate !== "pass"; - const classes = ["item", isGreen(item) ? "green" : "", red ? "red" : "", item.held ? "held" : "", index === ui.cursor ? "cursor" : ""].filter(Boolean).join(" "); + const commanded = item.kind === "ask" || item.kind === "operator"; + const classes = ["item", "entry", `k-${item.kind}`, "waiting", isGreen(item) ? "green" : "", red ? "red" : "", item.held ? "held" : "", index === ui.cursor ? "cursor" : ""].filter(Boolean).join(" "); const box = h("input", {type: "checkbox", "aria-label": `Select ${item.title}`, checked: ui.selected.has(keyOf(item)), disabled: !pickable(item), on: {change: () => toggle(item, true)}}); - return h("li", {class: classes, "data-index": index}, + return h("li", {class: classes, "data-index": index, "data-ref": keyOf(item), "data-to": item.to || null}, box, - h("div", {}, - h("div", {class: "head-row"}, - pill(ITEM_LABELS[item.kind], KIND_TONES[item.kind]), - item.kind === "review" ? pill(item.gate === "pass" ? "Gate green" : "Gate red", item.gate === "pass" ? "ok" : "bad") : null, - item.warnings.map((warning) => pill(warning, "bad")), - item.pinned ? pill("pinned", "violet") : null, - item.until ? pill(`until ${absolute.format(stampOf(item.until))}`, "neutral") : null, - h("span", {class: "spacer"}), when(item.ts)), - h("button", {type: "button", class: "linkish", title: "Open", on: {click: () => openItem(item)}}, item.title), - item.agent ? h("div", {class: "muted"}, "from ", person(item.agent)) : null, + h("div", {class: "msg-meta"}, + when(item.ts), + pill(ITEM_LABELS[item.kind], KIND_TONES[item.kind]), + item.kind === "review" ? pill(item.gate === "pass" ? "Gate green" : "Gate red", item.gate === "pass" ? "ok" : "bad") : null, + item.warnings.map((warning) => pill(warning, "bad")), + item.pinned ? pill("pinned", "violet") : null, + item.until ? pill(`until ${absolute.format(stampOf(item.until))}`, "neutral") : null), + h("div", {class: "entry-body"}, + h("div", {class: "route"}, + h("button", {type: "button", class: "linkish headline", title: "Open", on: {click: () => openItem(item)}}, item.kind === "operator" ? item.id : item.kind === "ask" ? item.title.split(": ")[0] : item.title), + item.agent ? [face(item.agent, false), person(item.agent)] : null), item.kind === "review" ? h("div", {class: "muted"}, item.detail, item.score ? ` · ${item.score}` : "") : null, - item.kind === "operator" && item.title !== item.command ? h("div", {class: "command"}, h("code", {}, item.command)) : null, - item.kind === "operator" ? h("div", {class: "muted"}, `in ${dash(item.directory)}`, item.detail && item.detail !== "-" ? ` · ${item.detail}` : "") : null, + commanded ? h("div", {class: "command", "data-path": item.directory || "-"}, h("code", {}, item.command), copyButton(item.command)) : null, + commanded ? h("span", {class: "muted"}, `in ${dash(item.directory)}`) : null, + item.kind === "operator" && item.detail && item.detail !== "-" ? h("p", {class: "text reason"}, item.detail) : null, + diffBlocks(item), item.job ? consoleOf(item.job) : null, item.kind === "rule" ? ruleRequestBody(item) : null, - item.kind === "ask" ? h("div", {class: "muted"}, `in ${dash(item.directory)}`) : null, item.kind === "decision" ? h("div", {class: "options"}, item.options.map((option) => h("button", { type: "button", class: `option${option.label === item.recommended ? " recommended" : ""}`, title: `${option.label}: your passkey signs this answer, a post to ${item.agent}`, disabled: item.held, on: {click: () => chooseOne(item, option.label)}, - }, h("strong", {}, option.label), option.description ? h("span", {}, option.description) : null))) : null), - h("div", {class: "item-actions"}, + }, h("strong", {}, option.label), option.description ? h("span", {}, option.description) : null))) : null, + feedbackList(keyOf(item))), + h("div", {class: "item-actions line-actions"}, actionsFor(item), - item.kind === "operator" ? copyButton(item.command) : null, h("span", {class: "spacer"}), iconBtn("bi-hourglass-split", "1 h", "btn small ghost", () => mark(item, nowSeconds() + 3600, item.pinned), "1 h: snooze for one hour"), iconBtn("bi-sunrise", "tomorrow", "btn small ghost", () => mark(item, tomorrow(), item.pinned), "tomorrow: snooze until 8 am tomorrow"), @@ -202,7 +205,7 @@ ui.selected.delete(key); } } - const sig = JSON.stringify([items.map(withoutOutput), [...ui.selected], ui.cursor, ui.snoozed, ui.busy, Math.floor(nowSeconds() / 60)]); + const sig = JSON.stringify([items.map(withoutOutput), [...ui.selected], ui.cursor, ui.snoozed, ui.busy, Math.floor(nowSeconds() / 60), diffView.version, state.messages.map((message) => message.id)]); if (sig === inboxSig) { repaintConsoles(items); return; @@ -221,7 +224,7 @@ const deny = h("button", {type: "button", class: "btn small danger", ...named(`Deny selection (${deniable.length})`, `Deny selection (${deniable.length}): each requester gets its denial by post, one passkey signature, 3 s to undo`), disabled: !deniable.length || ui.busy, on: {click: () => bulk(chosen, "Deny selection", true)}}, icon("bi-x-square"), String(deniable.length)); const all = h("button", {type: "button", class: "btn small primary", ...named(`Grant all green (${green.length})`), disabled: !green.length || ui.busy, on: {click: () => bulk(green, "Grant all green", false)}}, icon("bi-check2-all"), String(green.length)); inboxDom.replaceChildren( - h("div", {class: "inbox-head"}, h("h2", {class: "group-title"}, "Inbox", counter(waitingCount())), h("span", {class: "spacer"}), + h("div", {class: "inbox-head"}, h("h2", {class: "group-title"}, "Waits on Operator", counter(waitingCount())), h("span", {class: "spacer"}), hidden ? h("label", {class: "check"}, snoozed, `${hidden} snoozed`) : null), h("div", {class: "inbox-actions"}, choose, deny, all), onLocalhost() ? "" : h("p", {class: "muted"}, "The passkey signs grants only under ", localLink()), diff --git a/orchestrator/templates/page-keyboard.js b/orchestrator/templates/page-keyboard.js index 3269825..5cd1d56 100644 --- a/orchestrator/templates/page-keyboard.js +++ b/orchestrator/templates/page-keyboard.js @@ -1,6 +1,6 @@ function helpSheet() { openSheet("Keyboard shortcuts", [h("dl", {class: "keys"}, KEYS.map(([key, text]) => [h("dt", {}, key), h("dd", {}, text)])), - h("p", {class: "muted"}, "Control room: full screen with large type for a second screen, under ⚙ Ambience. j, k, x, a and Enter work in the inbox.")], false); + h("p", {class: "muted"}, "Control room: full screen with large type for a second screen, under ⚙ Ambience. j, k, x, a and Enter work in Waits on Operator.")], false); } function overlayOpen() { @@ -68,6 +68,8 @@ if (current && !current.held) { approveOne(current); } + } else if (event.key === "f" && feedbackKey()) { + event.preventDefault(); } else if (event.key === "Enter" && current && target === document.body) { event.preventDefault(); openItem(current); diff --git a/orchestrator/templates/page-screens.js b/orchestrator/templates/page-screens.js index 4de18a5..d5d6dad 100644 --- a/orchestrator/templates/page-screens.js +++ b/orchestrator/templates/page-screens.js @@ -60,7 +60,7 @@ function render(force) { for (const [id, {button}] of navButtons) { - button.setAttribute("aria-current", ui.screen === id ? "page" : "false"); + button.setAttribute("aria-current", ui.screen === id || (!navButtons.has(ui.screen) && Nav.tabOf(ui.screen) === id) ? "page" : "false"); } const sig = signature(); if (force || drawn.screen !== ui.screen || drawn.sig !== sig) { @@ -98,13 +98,13 @@ const {badge} = navButtons.get(id); badge.textContent = String(count); badge.hidden = count === 0; - badge.className = `badge${id === "inbox" && count ? " urgent" : ""}`; + badge.className = `badge${id === "tasks" && count ? " urgent" : ""}`; } const banner = document.getElementById("mayday"); const silence = state.silence; banner.hidden = !silence; banner.replaceChildren(...(silence ? [h("p", {class: "text"}, `MAYDAY · ${dash(silence.by)}: ${silence.text}`), iconBtn("bi-x-octagon", "cancel", "btn small", () => confirmAct("clear-mayday", []), "cancel: end the mayday (radio.py clear)")] : [])); - document.title = `${state.badges.inbox ? `(${state.badges.inbox}) ` : ""}Agent Orchestrator`; + document.title = `${state.badges.tasks ? `(${state.badges.tasks}) ` : ""}Agent Orchestrator`; window.setTimeout(measure, 0); } diff --git a/orchestrator/templates/page-stream.js b/orchestrator/templates/page-stream.js index 0e919f5..73f40a4 100644 --- a/orchestrator/templates/page-stream.js +++ b/orchestrator/templates/page-stream.js @@ -112,7 +112,7 @@ } if (entry.kind === "ask" || entry.kind === "operator") { const where = entry.kind === "ask" ? entry.ref.cwd : entry.ref.directory; - return [h("div", {class: "command"}, h("code", {}, entry.kind === "ask" ? entry.text : entry.title), copyButton(entry.kind === "ask" ? entry.text : entry.title)), h("span", {class: "muted"}, `in ${dash(where)}`), entry.kind === "operator" && entry.text !== "-" ? h("p", {class: "text"}, entry.text) : null]; + return [h("div", {class: "command", "data-path": where || "-"}, h("code", {}, entry.kind === "ask" ? entry.text : entry.title), copyButton(entry.kind === "ask" ? entry.text : entry.title)), h("span", {class: "muted"}, `in ${dash(where)}`), entry.kind === "operator" && entry.text !== "-" ? h("p", {class: "text"}, entry.text) : null]; } if (entry.kind === "review") { return [entry.text ? h("pre", {class: "notes"}, entry.text) : null, h("div", {class: "line-actions"}, iconBtn("bi-eye", "View diff", "btn small soft", () => reviewSheet(entry.ref.id)))]; @@ -134,11 +134,13 @@ const renamed = entry.now && entry.now !== entry.agent ? h("span", {title: `now: ${entry.now}`}, person(entry.agent)) : person(entry.agent); const who = entry.agent ? [face(entry.agent, false), renamed] : entry.session ? h("strong", {title: entry.session}, entry.session.slice(0, 8)) : null; const headline = entry.kind === "transcript" || entry.kind === "ask" || entry.kind === "operator" ? null : h("span", {class: "headline"}, entry.title); - return h("li", {class: `entry k-${entry.kind}${entry.waiting ? " waiting" : ""}${alarm ? " alarm" : ""}`}, + return h("li", {class: `entry k-${entry.kind}${entry.waiting ? " waiting" : ""}${alarm ? " alarm" : ""}`, "data-ref": item ? keyOf(item) : null, "data-to": item && item.to ? item.to : null}, h("div", {class: "msg-meta"}, when(entry.ts), kindPill(entry), extras(entry)), h("div", {}, h("div", {class: "route"}, who, entry.sub ? h("button", {type: "button", class: "linkish muted", on: {click: () => focusSub(entry.session, entry.sub)}}, `› Subagent ${entry.sub.slice(0, 12)}`) : null, entry.kind === "ask" ? h("strong", {}, entry.title) : null, headline), body(entry), + item ? diffBlocks(item) : null, + item ? feedbackList(keyOf(item)) : null, item ? h("div", {class: "line-actions"}, actionsFor(item)) : null)); } diff --git a/orchestrator/templates/style-diff.css b/orchestrator/templates/style-diff.css new file mode 100644 index 0000000..28fa926 --- /dev/null +++ b/orchestrator/templates/style-diff.css @@ -0,0 +1,23 @@ +.inbox-list > .item.entry { grid-template-columns: auto 150px minmax(0, 1fr); align-items: start; padding: var(--sp-3); border: 1px solid var(--line); } +.inbox-list > .item.entry.waiting { box-shadow: inset 3px 0 0 var(--warn); } +.inbox-list > .item.entry.green { box-shadow: inset 3px 0 0 var(--ok); } +.inbox-list > .item.entry.red { box-shadow: inset 3px 0 0 var(--bad); } +.inbox-list > .item.entry > .entry-body { display: grid; gap: var(--sp-1); min-width: 0; } +.inbox-list > .item.entry .reason { margin: 0; } +@media (max-width: 640px) { + .inbox-list > .item.entry { grid-template-columns: auto minmax(0, 1fr); } + .inbox-list > .item.entry > .entry-body { grid-column: 1 / -1; } +} +.diff-files { display: grid; gap: var(--sp-2); min-width: 0; margin-top: var(--sp-1); } +.diff-file { min-width: 0; overflow: hidden; border: 1px solid var(--line); border-radius: 10px; background: var(--panel); } +.diff-file > .diff-head, .diff-file summary.diff-head { padding: 6px 10px; cursor: default; } +.diff-file summary.diff-head { cursor: pointer; list-style: none; } +.diff-file summary.diff-head::-webkit-details-marker { display: none; } +.diff-file .diff-scroll { max-height: min(60vh, 640px); } +.diff-file .diff-head .mode { color: var(--warn); font-size: var(--fs-xs); } +.diff-file .diff-head .note .bi { margin-right: 4px; vertical-align: -2px; } +.diff-file .diff-head .note { display: inline-flex; align-items: center; color: var(--warn); font-size: var(--fs-xs); } +.feedback-list { display: grid; gap: 4px; margin: var(--sp-1) 0 0; padding: 0; list-style: none; } +.feedback-list li { display: flex; align-items: baseline; gap: 6px; padding: 4px 8px; border-radius: 8px; background: var(--info-soft); font-size: var(--fs-sm); } +.feedback-list .text { flex: 1; min-width: 0; overflow-wrap: anywhere; white-space: pre-wrap; } +.feedback-float { position: fixed; z-index: 40; padding: 2px; border-radius: 10px; background: var(--glass-card); box-shadow: var(--shadow); } diff --git a/skills/kvb-git-diff/SKILL.md b/skills/kvb-git-diff/SKILL.md index ca82686..d48eb4d 100644 --- a/skills/kvb-git-diff/SKILL.md +++ b/skills/kvb-git-diff/SKILL.md @@ -16,16 +16,21 @@ once the operator's pinned passkey signature over it verifies here. pkgmgr review --out /tmp/kvb-git-diff/-.html --max-file-lines 400 --base --note ``` -2. Call the kvb-orchestrator MCP tool `review_request` with `repo` (what - `git rev-parse --path-format=absolute --git-common-dir` prints), - `worktree` (what `git rev-parse --show-toplevel` prints), `branch` (what - `git symbolic-ref --short HEAD` prints), `tree` (what `git write-tree` - prints), `title` (the commit subject), `page` (the `--out` path), `notes` - (the commit table), `gate` (`pass` when every gate the calling skill ran is green, else - `fail`) and `score` (the kvb-check-score line). Only a `pass` review can - be approved in bulk or by a rule. Tell the operator in one line that the - review waits in the hub at the `url` it returns, written out as plain - text: chat links do not open for this operator. +2. Request the review in the worktree; it ships the staged diff, every file + MEDUSA's secret rules or an armour line flag by stat only, so the hub + shows it on the card. `` is + `pass` when every gate the calling skill ran is green, else `fail`; + `` your agent name, which inline feedback is posted to: + + ```bash + python3 ~/.local/share/kvb-orchestrator/client/approval.py request --port 8737 --title "" --page /tmp/kvb-git-diff/-.html --notes
--gate --score "" --agent "" + ``` + + Only a `pass` review can be approved in bulk or by a rule. Tell the + operator in one line that the review waits in the hub at the `url` it + prints, written out as plain text: chat links do not open for this + operator. Without the script, the MCP tool `review_request` takes the + same fields without the diff. 3. Call `review_wait` with its `id` and `timeout_seconds` 240 until the status is no longer `pending`. 4. `approved`: in the worktree, verify the operator's passkey signature diff --git a/tests/e2e/test_orchestrator_e2e.py b/tests/e2e/test_orchestrator_e2e.py index 950f774..99d1df7 100644 --- a/tests/e2e/test_orchestrator_e2e.py +++ b/tests/e2e/test_orchestrator_e2e.py @@ -35,8 +35,8 @@ OUT = Path(os.environ.get("E2E_OUT", "/tmp/orchestrator-phase4-shots")) WIDTHS = ((1920, 1080), (1366, 768), (390, 844)) VIEWS = ("orbit", "chord", "swimlane", "tree", "3d") TASK_VIEWS = ("list", "board", "rasci", "gantt", "burn", "3d") -SCREENS = ["stream", "stream/inbox", "stream/rules", *(f"{tab}/{view}" for tab in ("agents", "artifacts") for view in VIEWS), "agents/hierarchy", *(f"tasks/{view}" for view in TASK_VIEWS)] -SUBS = ["Inbox", "Firewall"] +SCREENS = ["stream", "tasks/operator", "stream/rules", *(f"{tab}/{view}" for tab in ("agents", "artifacts") for view in VIEWS), "agents/hierarchy", *(f"tasks/{view}" for view in TASK_VIEWS)] +SUBS = ["Firewall"] TABS = ["Stream", "Agents", "Artifacts", "Tasks"] HELPERS = """ const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); @@ -189,7 +189,7 @@ AFTER_TAKEOVER = """(async () => {@HELPERS@ INBOX_CHIP = """(async () => {@HELPERS@ const chip = (id) => document.querySelector(`#main [aria-label="Task filters"] button[data-filter="${id}"]`); chip("operator").click(); - await until(() => location.hash === "#stream/inbox" && document.querySelector("#main .inbox-list"), 4000); + await until(() => location.hash === "#tasks/operator" && document.querySelector("#main .inbox-list"), 4000); const inbox = {hash: location.hash, items: all("#main .inbox-list > li").length, counted: Number(text(chip("operator").querySelector(".count"))), pressed: chip("operator").getAttribute("aria-pressed"), current: all("#tabs button[aria-current=page]").map(name)}; chip("grant").click(); await until(() => location.hash.startsWith("#tasks/"), 4000); @@ -1062,13 +1062,13 @@ INBOX_PICK = """(async () => {@HELPERS@ const last = all("#main .inbox-list > li").filter((li) => li.textContent.includes("echo row")).pop(); last.scrollIntoView({block: "center"}); await sleep(300); - const title = text(last.querySelector(".linkish")); + const title = text(last.querySelector(".command code")); const before = scroller.scrollTop; last.querySelector("input").click(); await sleep(300); const after = scroller.scrollTop; const holder = document.activeElement && document.activeElement.closest("li"); - const focused = Boolean(holder && text(holder.querySelector(".linkish")) === title && document.activeElement.checked); + const focused = Boolean(holder && text(holder.querySelector(".command code")) === title && document.activeElement.checked); all("#main .inbox-list > li").find((li) => li.textContent.includes("pick a name")).querySelector("input").click(); await sleep(300); const buttons = all("#main .inbox-actions button").map(name); @@ -1785,7 +1785,7 @@ class TestOrchestratorE2E(HubCase): self.assertEqual((probe["title"], probe["h1"]), ("(2) Agent Orchestrator", "Agent Orchestrator")) self.assertEqual(probe["header"], ["radio-open", "bubbles-toggle", "palette-open", "settings-open"]) self.assertEqual((probe["tabs"], probe["subs"]), (TABS, SUBS)) - self.assertEqual(probe["badges"], {"tab-stream": None, "tab-inbox": "2", "tab-rules": None, "tab-agents": "1", "tab-artifacts": None, "tab-tasks": None}) + self.assertEqual(probe["badges"], {"tab-stream": None, "tab-rules": None, "tab-agents": "1", "tab-artifacts": None, "tab-tasks": "2"}) self.assertEqual(probe["banner"], "MAYDAY · Docs: disk full") self.assertFalse(probe["rail"]) self.assertEqual((probe["time"], probe["replay"]), ("24 h ▾", True)) @@ -1799,7 +1799,7 @@ class TestOrchestratorE2E(HubCase): stream = by_hash["#stream"] self.assertEqual([probe["chips"] for probe in probes if probe["hash"] == "#stream"], [["For you", "Radio", "Work", "Transcripts"]] * len(WIDTHS)) self.assertEqual((stream["menus"], stream["custom"]), (["Filter"], True)) - self.assertEqual(by_hash["#stream/inbox"]["inbox"], 2) + self.assertEqual(by_hash["#tasks/operator"]["inbox"], 2) for tab, own in (("agents", ["Hierarchy"]), ("artifacts", [])): with self.subTest(tab=tab): self.assertEqual(by_hash[f"#{tab}/orbit"]["views"], ["Orbit", "Chord", "Swimlane", "Tree", "3D", *own]) @@ -1841,7 +1841,7 @@ class TestOrchestratorE2E(HubCase): self.assertEqual(listed["flows"], ["file", "file"]) self.assertEqual(by_hash["#tasks/board"]["columns"], [["queued", ["queue/gate-3"]], ["running", ["work/feature", "work/stage"]], ["blocked", ["work/docs"]], ["done", ["work/old", "operator/push"]], ["stopped", []]]) self.assertEqual(by_hash["#tasks/rasci"]["matrix"], [6, 6, "CIRARASRARAR"]) - self.assertEqual([probe["taskFilters"][3][1] for probe in probes if probe["hash"] == "#stream/inbox"], ["true"] * len(WIDTHS)) + self.assertEqual([probe["taskFilters"][3][1] for probe in probes if probe["hash"] == "#tasks/operator"], ["true"] * len(WIDTHS)) extra = data["results"][len(probes) :] self.assertEqual([probe["hash"] for probe in extra[:3]], ["#agents/orbit", "#agents/chord", "#artifacts/tree"]) actions = extra[3] @@ -1857,7 +1857,7 @@ class TestOrchestratorE2E(HubCase): self.assertEqual(actions["centred"], [True, "none"]) calm, chips = extra[4:] self.assertRegex(calm["note"], r"Force · without motion \(prefers-reduced-motion\)$") - self.assertEqual({key: value for key, value in chips["inbox"].items() if key != "counted"}, {"hash": "#stream/inbox", "items": chips["inbox"]["counted"], "pressed": "true", "current": ["Inbox"]}) + self.assertEqual({key: value for key, value in chips["inbox"].items() if key != "counted"}, {"hash": "#tasks/operator", "items": chips["inbox"]["counted"], "pressed": "true", "current": ["Tasks"]}) self.assertGreaterEqual(chips["inbox"]["items"], 1) self.assertEqual(chips["back"], {"hash": "#tasks/list", "rows": [["queue/gate-3", "Queue job"]]}) diff --git a/tests/e2e/test_workbench_w1_e2e.py b/tests/e2e/test_workbench_w1_e2e.py new file mode 100644 index 0000000..31424cc --- /dev/null +++ b/tests/e2e/test_workbench_w1_e2e.py @@ -0,0 +1,224 @@ +from __future__ import annotations + +import json +import os +import shutil +import sys +import unittest +from pathlib import Path +from unittest import mock + +HERE = Path(__file__).resolve().parent +sys.path[:0] = [str(HERE.parent), str(HERE)] + +import test_orchestrator_e2e as base +from hub_case import HubCase +from hub_support import ASK, PAGE +from kvb_orchestrator import batches, skill_scripts + +OUT = Path(os.environ.get("E2E_OUT", "/tmp/workbench-w1-shots")) +WITHHELD = "content withheld" +DIFF = [ + {"path": "src/app.py", "status": "M", "mode": "100644", "additions": 2, "deletions": 1, "patch": "@@ -1,3 +1,4 @@\n keep = 1\n-old = 2\n+new = 3\n+more = 4\n tail = 4", "note": ""}, + {"path": "logo.png", "status": "B", "mode": "100644", "additions": 0, "deletions": 0, "patch": "", "note": ""}, + {"path": "big.txt", "status": "M", "mode": "100644", "additions": 401, "deletions": 0, "patch": "@@ -0,0 +1,401 @@\n" + "\n".join(f"+line {n}" for n in range(401)), "note": ""}, + {"path": "key.txt", "status": "A", "mode": "100755", "additions": 3, "deletions": 0, "patch": "", "note": WITHHELD}, +] +OPERATOR = """(async () => {@HELPERS@ + navigator.credentials.get = @SIGNS@ ? navigator.credentials.get.bind(navigator.credentials) : () => Promise.reject(new DOMException("The operator canceled.", "NotAllowedError")); + window.e2ePosted = []; + const send = window.fetch.bind(window); + window.fetch = (path, init) => { + if (String(path).startsWith("/api/") && init && init.method === "POST") { + window.e2ePosted.push([String(path), JSON.parse(init.body)]); + } + return send(path, init); + }; + const list = await until(() => location.hash === "#tasks/operator" && document.querySelector("#main .inbox-list") && document.querySelector("#main .diff-file"), 10000); + if (!list) { + return {error: text(document.getElementById("main")), hash: location.hash}; + } + const mine = (words) => all("#main .inbox-list > li").find((li) => li.textContent.includes(words)); + const cards = ["@TAG@ review", "@TAG@ ship", "@TAG@ commit"].map(mine); + const tasks = document.getElementById("tab-tasks"); + const badge = tasks.querySelector(".badge"); + const review = cards[0]; + const files = all(".diff-file", review).map((file) => ({ + path: file.dataset.path, status: file.dataset.status, open: file.querySelector("details") ? file.querySelector("details").open : null, + rows: all("tr.add, tr.del", file).length, note: all(".note", file).map(text), mode: all(".mode", file).map(text), + colours: [...new Set(all("tr.add, tr.del", file).map((row) => getComputedStyle(row).backgroundColor))].length, + })); + const operator = cards[1]; + const found = { + hash: location.hash, tabs: all("#tabs > .tab-group > button").map(name), subs: all("#tabs .subtabs button").map(name), + badge: badge && !badge.hidden ? text(badge) : null, current: all("#tabs button[aria-current=page]").map(name), streamInbox: Boolean(document.getElementById("tab-inbox")), + kinds: cards.map((card) => (card ? card.className.split(" ").filter((name) => name.startsWith("k-")).join("") : null)), + boxes: cards.map((card) => Boolean(card && card.querySelector('input[type="checkbox"]'))), + operator: operator ? {command: text(operator.querySelector(".command code")), place: operator.querySelector(".command").dataset.path, text: text(operator), buttons: all("button", operator).map(name)} : null, + files, wide: document.documentElement.scrollWidth > window.innerWidth + 1, + }; + cards.forEach((card) => card.querySelector('input[type="checkbox"]').click()); + await sleep(300); + const grant = all("#main .inbox-actions button").find((button) => name(button).startsWith("Grant selection")); + found.grant = name(grant); + grant.click(); + await until(() => document.querySelector(".sheet-box .actions"), 4000); + const confirm = all(".sheet-box .actions button").find((button) => text(button).startsWith("Grant ")); + found.confirm = text(confirm); + found.listed = all(".sheet-box ul.mini > li").length; + confirm.click(); + await until(() => window.e2ePosted.some(([path]) => path === "/api/sign"), 8000); + await sleep(800); + found.posted = window.e2ePosted.filter(([path]) => ["/api/sign", "/api/inbox/approve"].includes(path)).map(([path, body]) => [path, body.items.map((item) => item.kind).sort()]); + return found; +})()""" +FEEDBACK = """(async () => {@HELPERS@ + const card = await until(() => all("#main .inbox-list > li").find((li) => li.textContent.includes("@TAG@ second") && li.querySelector(".diff-file tr.add")), 10000); + if (!card) { + return {error: text(document.getElementById("main"))}; + } + const select = (from, to) => { + const range = document.createRange(); + range.setStart(from, 0); + range.setEnd(to, to.childNodes.length); + const selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + return range.getBoundingClientRect(); + }; + window.e2ePosted = []; + const send = window.fetch.bind(window); + window.fetch = (path, init) => { + if (String(path).startsWith("/api/messages")) { + window.e2ePosted.push([String(path), JSON.parse(init.body).text]); + } + return send(path, init); + }; + navigator.credentials.get = @SIGNS@ ? navigator.credentials.get.bind(navigator.credentials) : () => Promise.reject(new DOMException("The operator canceled.", "NotAllowedError")); + const rows = all(".diff-file[data-path='src/app.py'] tr[data-line]", card); + const box = select(rows[1].querySelector("td.code"), rows[2].querySelector("td.code")); + await sleep(300); + const floating = document.querySelector(".feedback-float"); + const nearby = Boolean(floating && !floating.hidden); + rows[1].querySelector("td.code").dispatchEvent(new MouseEvent("contextmenu", {bubbles: true, cancelable: true, clientX: box.left + 4, clientY: box.top + 4})); + await sleep(200); + const menu = floating && floating.getAttribute("role"); + floating.querySelector("button").click(); + const sheet = await until(() => document.querySelector(".sheet-box textarea"), 4000); + const title = text(document.querySelector(".sheet-head .panel-title")); + const range = text(document.querySelector(".sheet-box .who code")); + sheet.value = "right-click note"; + all(".sheet-box button").find((button) => name(button) === "Send feedback").click(); + await until(() => window.e2ePosted.length >= (@SIGNS@ ? 2 : 1), 8000); + await sleep(600); + const fresh = () => all("#main .inbox-list > li").find((li) => li.textContent.includes("@TAG@ second")); + const shown = @SIGNS@ ? await until(() => fresh() && all(".feedback-list li", fresh()).map(text).find((entry) => entry.includes("right-click note")), 15000) : null; + const close = document.querySelector(".sheet-head button"); + if (close) { + close.click(); + } + const ask = all("#main .inbox-list > li").find((li) => li.textContent.includes("@TAG@ keyed")); + const code = ask.querySelector(".command code"); + select(code, code); + document.body.dispatchEvent(new KeyboardEvent("keydown", {key: "f", bubbles: true, cancelable: true})); + const keyed = await until(() => document.querySelector(".sheet-box textarea"), 4000); + const keyRange = text(document.querySelector(".sheet-box .who code")); + keyed.value = "key f note"; + all(".sheet-box button").find((button) => name(button) === "Send feedback").click(); + await sleep(1500); + return {nearby, menu, title, range, keyRange, shown: shown || "", posted: window.e2ePosted, wide: document.documentElement.scrollWidth > window.innerWidth + 1}; +})()""" + + +def probe(template: str, tag: str, signs: bool) -> str: + return template.replace("@HELPERS@", base.HELPERS).replace("@TAG@", tag).replace("@SIGNS@", "true" if signs else "false") + + +@unittest.skipUnless(shutil.which(os.environ.get("CHROMIUM", "chromium")), "no Chromium") +class TestWorkbenchOperatorTasks(HubCase): + drive = base.TestOrchestratorE2E.drive + authenticator = base.TestOrchestratorE2E.authenticator + + def setUp(self): + super().setUp() + OUT.mkdir(parents=True, exist_ok=True) + self.claim("s2", "Skills") + rows = {} + self.reviews = {} + for width, _ in base.WIDTHS: + tag = f"w{width}" + for word in ("review", "second"): + arguments = {**self.arguments(f"feat: {tag} {word}", "", self.page(f"{tag}-{word}.html", PAGE)), "agent": "Skills", "diff": DIFF} + result = self.tool("review_request", arguments) + self.assertFalse(result["isError"], result) + self.reviews[(tag, word)] = result["structuredContent"]["id"] + for word in ("commit", "keyed"): + status, _, body = self.post_json("/api/asks", {**ASK, "command": f"git commit -m '{tag} {word}'", "session": "s2", "diff": DIFF}, self.local()) + self.assertEqual(status, 201, body) + rows[f"operator/{tag}"] = {"title": f"make {tag} ship", "workdir": "/repo", "description": f"{tag} ship the build", "check": "manual", "status": "queued", "roles": {"Skills": "A"}} + self.write_council({"rasci.json": rows}) + self.base = f"http://localhost:{self.port}/" + + def steps(self, browser: str, signs: bool) -> list[dict]: + steps = [] + for width, height in base.WIDTHS: + tag = f"w{width}" + shot = str(OUT / f"{browser}-{width}-operator") + steps.append({"width": width, "height": height, "url": f"{self.base}?{width}#stream/inbox", "wait": 1200, "probe": probe(OPERATOR, tag, signs), "shot": f"{shot}-tasks.png"}) + steps.append({"width": width, "height": height, "url": None, "wait": 0, "probe": probe(FEEDBACK, tag, signs), "shot": f"{shot}-feedback.png"}) + return steps + + def check(self, data: dict, browser: str, signs: bool) -> None: + results = iter(data["results"]) + for width, _ in base.WIDTHS: + tag = f"w{width}" + tasks, feedback = next(results), next(results) + with self.subTest(browser=browser, width=width): + self.assertNotIn("error", tasks, tasks) + self.assertEqual((tasks["hash"], tasks["tabs"], tasks["subs"], tasks["streamInbox"], tasks["current"]), ("#tasks/operator", base.TABS, ["Firewall"], False, ["Tasks"])) + self.assertRegex(tasks["badge"] or "", r"^[1-9][0-9]*$") + self.assertEqual((tasks["kinds"], tasks["boxes"]), (["k-review", "k-operator", "k-ask"], [True, True, True])) + operator = tasks["operator"] + self.assertEqual((operator["command"], operator["place"]), (f"make {tag} ship", "/repo")) + self.assertIn(f"{tag} ship the build", operator["text"]) + for label in ("Run", "Deny", "Copy", "1 h", "tomorrow", "pin"): + self.assertIn(label, operator["buttons"]) + self.assertEqual([(entry["path"], entry["status"], entry["open"]) for entry in tasks["files"]], [("src/app.py", "M", True), ("logo.png", "B", None), ("big.txt", "M", False), ("key.txt", "A", None)]) + self.assertEqual((tasks["files"][0]["rows"], tasks["files"][0]["note"], tasks["files"][0]["colours"]), (3, [], 2)) + self.assertEqual((tasks["files"][3]["note"], tasks["files"][3]["mode"]), ([WITHHELD], ["100755"])) + self.assertEqual((tasks["grant"], tasks["confirm"], tasks["listed"]), ("Grant selection (3)", "Grant 3", 3)) + kinds = ["ask", "operator", "review"] + self.assertEqual(tasks["posted"], [["/api/sign", kinds], *([["/api/inbox/approve", kinds]] if signs else [])]) + self.assertFalse(tasks["wide"]) + self.assertNotIn("error", feedback, feedback) + self.assertEqual((feedback["nearby"], feedback["menu"], feedback["title"]), (True, "menu", "Feedback")) + self.assertEqual((feedback["range"], feedback["keyRange"]), ("src/app.py:2-2", "/repo:1-1")) + ref = f"review:{self.reviews[(tag, 'second')]}" + texts = [text for path, text in feedback["posted"] if path == "/api/messages/sign"] + self.assertEqual(texts[0].split(": ", 1)[0], f"Feedback on {ref} src/app.py:2-2") + self.assertTrue(texts[0].endswith(" right-click note"), texts) + if signs: + self.assertIn("right-click note", feedback["shown"]) + self.assertTrue(any(text and text.endswith(" key f note") for _, text in feedback["posted"]), feedback["posted"]) + self.assertFalse(feedback["wide"]) + radio = [" ".join(args) for script, args in self.scripts.calls if script == skill_scripts.RADIO] + reached = [entry for entry in radio if "Feedback on review:" in entry] + self.assertEqual(len(reached), 2 * len(base.WIDTHS) if signs else 0, radio) + + def test_chromium_shows_operator_tasks_with_a_badge_diffs_bulk_grants_and_sends_feedback_at_three_widths(self): + with mock.patch.object(batches, "UNDO", 0.3): + data = self.drive(self.steps("chromium", True), self.authenticator()) + (OUT / "chromium-operator.json").write_text(json.dumps(data, indent=1), encoding="utf-8") + self.assertEqual([error for error in data["errors"] if "/favicon.ico" not in error], []) + self.check(data, "chromium", True) + + @unittest.skipUnless(shutil.which(os.environ.get("FIREFOX", "firefox")), "no Firefox") + def test_firefox_shows_operator_tasks_with_a_badge_diffs_and_feedback_without_signing_at_three_widths(self): + data = self.drive(self.steps("firefox", False), None, "bidi.mjs", os.environ.get("FIREFOX", "firefox")) + (OUT / "firefox-operator.json").write_text(json.dumps(data, indent=1), encoding="utf-8") + self.assertEqual([error for error in data["errors"] if "/favicon.ico" not in error], []) + self.check(data, "firefox", False) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_hub_console.py b/tests/test_hub_console.py index 4f99fc1..f0a27bb 100644 --- a/tests/test_hub_console.py +++ b/tests/test_hub_console.py @@ -57,7 +57,7 @@ class TestHubConsole(HubCase): kept = self.item("operator/greet") self.assertEqual((kept["kind"], kept["title"], kept["command"], kept["directory"], kept["runnable"], kept["deniable"]), ("operator", COMMAND, COMMAND, str(self.outside), False, False)) self.assertEqual(kept["job"]["output"], "line 1\nline 2\n") - self.assertEqual(self.snapshot()["badges"]["inbox"], 1) + self.assertEqual(self.snapshot()["badges"]["tasks"], 1) for payload in ({"id": "nope"}, {"id": job["id"], "extra": 1}, {}, [job["id"]]): with self.subTest(payload=payload): self.assertEqual(self.dismiss(payload, self.guard(self.host)), 400) diff --git a/tests/test_hub_nav.py b/tests/test_hub_nav.py index 74a4a4a..0132aa6 100644 --- a/tests/test_hub_nav.py +++ b/tests/test_hub_nav.py @@ -42,7 +42,8 @@ const N = require(process.argv[1]); const hashes = ["", "#stream", "#stream/inbox", "#stream/rules", "#stream/nope", "#agents/chord", "#agents/3d", "#agents", "#agents/nope", "#artifacts/tree", "#artifacts/orbit", "#graph", "#graph/swimlane", "#graph/orbit", "#tree", "#bogus", "#agents%2Fswimlane", "#agents/hierarchy", "#artifacts/hierarchy", "#graph/hierarchy", "#tree/hierarchy", "#stream/radio", "#radio", - "#tasks", "#tasks/board", "#tasks/rasci", "#tasks/orbit", "#agents/board", "#tasks/hierarchy", "#tasks/gantt", "#tasks/burn", "#agents/gantt", "#tasks/3d"]; + "#tasks", "#tasks/board", "#tasks/rasci", "#tasks/orbit", "#agents/board", "#tasks/hierarchy", "#tasks/gantt", "#tasks/burn", "#agents/gantt", "#tasks/3d", + "#tasks/operator", "#tasks%2Foperator", "#stream/operator", "#agents/operator"]; console.log(JSON.stringify({ tabs: N.TABS.map(([id, label]) => [id, label, (N.SUBS[id] || []).map(([sub, name]) => [sub, name])]), views: N.VIEWS.map(([id, label]) => [id, label]), @@ -64,9 +65,10 @@ class TestNavModule(unittest.TestCase): done = subprocess.run(["node", "-e", NAV_PROBE, str(NAV)], capture_output=True, text=True, timeout=60, check=True) cls.data = json.loads(done.stdout) - def test_the_navigation_has_four_entries_and_stream_has_inbox_and_rules(self): - stream = ["stream", "Stream", [["inbox", "Inbox"], ["rules", "Firewall"]]] + def test_the_navigation_has_four_entries_and_no_inbox_and_stream_keeps_only_the_firewall(self): + stream = ["stream", "Stream", [["rules", "Firewall"]]] self.assertEqual(self.data["tabs"], [stream, ["agents", "Agents", []], ["artifacts", "Artifacts", []], ["tasks", "Tasks", []]]) + self.assertNotIn("inbox", json.dumps(self.data["tabs"])) def test_agents_and_artifacts_offer_five_views_in_order_and_tasks_six(self): shared = [["orbit", "Orbit"], ["chord", "Chord"], ["swimlane", "Swimlane"], ["tree", "Tree"], ["3d", "3D"]] @@ -83,7 +85,7 @@ class TestNavModule(unittest.TestCase): self.assertEqual(self.data["routes"], [ stream, stream, - {"screen": "inbox", "view": None, "redirect": None}, + {"screen": "inbox", "view": None, "redirect": "#tasks/operator"}, {"screen": "rules", "view": None, "redirect": None}, stream, {"screen": "agents", "view": "chord", "redirect": None}, @@ -114,11 +116,15 @@ class TestNavModule(unittest.TestCase): {"screen": "tasks", "view": "burn", "redirect": None}, {"screen": "agents", "view": None, "redirect": None}, {"screen": "tasks", "view": "3d", "redirect": None}, + {"screen": "inbox", "view": None, "redirect": None}, + {"screen": "inbox", "view": None, "redirect": None}, + stream, + {"screen": "agents", "view": None, "redirect": None}, ]) - def test_every_screen_has_its_own_hash_and_tab(self): - self.assertEqual(self.data["hashes"], ["#stream", "#stream/inbox", "#stream/rules", "#agents/chord", "#artifacts/3d", "#tasks/board"]) - self.assertEqual(self.data["tabOf"], ["stream", "stream", "stream", "agents", "artifacts", "tasks"]) + def test_every_screen_has_its_own_hash_and_tab_and_operator_tasks_live_under_tasks(self): + self.assertEqual(self.data["hashes"], ["#stream", "#tasks/operator", "#stream/rules", "#agents/chord", "#artifacts/3d", "#tasks/board"]) + self.assertEqual(self.data["tabOf"], ["stream", "tasks", "stream", "agents", "artifacts", "tasks"]) class TestScreenTransition(unittest.TestCase): diff --git a/tests/test_hub_operator_tasks.py b/tests/test_hub_operator_tasks.py new file mode 100644 index 0000000..718a830 --- /dev/null +++ b/tests/test_hub_operator_tasks.py @@ -0,0 +1,838 @@ +from __future__ import annotations + +import ast +import importlib +import json +import os +import shlex +import subprocess +import sys +import tempfile +import threading +import unittest +from pathlib import Path +from unittest import mock + +from hub_case import HubCase +from hub_support import APPROVAL, ASK, GIT_ENV, HUB, PAGE +from kvb_orchestrator import diffs, fields, reviewing, signed + +permission_hook = importlib.import_module("permission_hook") +staged_diff = importlib.import_module("staged_diff") +sandbox_paths = importlib.import_module("sandbox_paths") +hub_limits = importlib.import_module("hub_limits") +approval = importlib.import_module("approval") +TEMPLATES = HUB / "templates" +DIFFS = TEMPLATES / "diffs.js" +PAGE_MODULES = ["page-constants.js", "page-dom.js", "page-diff.js", "page-inbox.js", "page-stream.js", "page-screens.js", "page-keyboard.js"] +SECRET = 'api_key = "plain-example-words"' +ARMOURED = "before\n-----BEGIN EXAMPLE-----\nplain words only\n-----END EXAMPLE-----\nafter\n" +WITHHELD = "content withheld" +ALLOW = {"behavior": "allow"} +RULES = """ +rules: +- id: TEST-SECRET-1 + cwe: 'CWE-798: Use of Hard-coded Credentials' + patterns: + - api_key\\s*=\\s*["'][^"']{8,}["'] + file_types: + - '*.py' +- id: TEST-OTHER-1 + cwe: CWE-200 + patterns: + - print + file_types: + - '*.py' +""" +DIFFS_PROBE = """ +const D = require(process.argv[1]); +const patch = ["@@ -3,3 +3,4 @@", " keep", "-old", "+new", "+more", " tail"].join("\\n"); +console.log(JSON.stringify({ + rows: D.rows(patch).map((row) => [row.kind, row.line, row.old, row.new, row.text]), + collapsed: [[400, 0], [200, 201], [0, 401]].map(([additions, deletions]) => D.collapsed({additions, deletions})), + span: [D.span([7, 3, 5]), D.span([]), D.span([0, 2])], + lineSpan: [D.lineSpan("make\\n deploy\\n now", "deploy\\n now"), D.lineSpan("one line", "nope"), D.lineSpan("a\\nb", "")], + feedback: D.feedback("review:0123456789abcdef", "src/app.py", 4, 5, " -old\\n+new ", " use the helper "), + quoted: [D.quoted("x".repeat(D.QUOTE_LIMIT + 5)).length, D.QUOTE_LIMIT], + picked: D.feedbackOf([{text: "Feedback on review:abc a:1-1: \\"q\\" t"}, {text: "Feedback on review:abcd a:1-1"}, {text: "hello"}], "review:abc").length, +})); +""" +PAGE_PROBE = r""" +const fs = require("fs"); +const vm = require("vm"); +const [dir, names] = [process.argv[1], JSON.parse(process.argv[2])]; +class Node {} +class El extends Node { + constructor(tag) { super(); Object.assign(this, {tag, attrs: {}, children: [], listeners: {}, hidden: false, className: "", style: {}, dataset: {}, disabled: false, value: ""}); } + setAttribute(key, value) { + this.attrs[key] = String(value); + if (key === "class") { this.className = String(value); } + if (key === "hidden") { this.hidden = true; } + if (key === "disabled") { this.disabled = true; } + if (key.startsWith("data-")) { this.dataset[key.slice(5).replace(/-(\w)/g, (_, c) => c.toUpperCase())] = String(value); } + } + getAttribute(key) { return Object.hasOwn(this.attrs, key) ? this.attrs[key] : null; } + removeAttribute(key) { delete this.attrs[key]; } + addEventListener(name, run) { (this.listeners[name] ||= []).push(run); } + append(...nodes) { this.children.push(...nodes); } + replaceChildren(...nodes) { this.children = nodes; } + get firstChild() { return this.children[0] || null; } + get textContent() { return this.children.map((child) => (typeof child === "string" ? child : child.textContent)).join(""); } + set textContent(value) { this.children = [String(value)]; } + focus() {} + contains() { return false; } + querySelector() { return null; } + querySelectorAll() { return []; } + click() { for (const run of this.listeners.click || []) { run({target: this, preventDefault() {}}); } } +} +const elements = new Map(); +const document = { + currentScript: null, title: "", body: new El("body"), activeElement: null, + createElement: (tag) => new El(tag), createElementNS: (ns, tag) => new El(tag), + getElementById: (id) => { + if (!elements.has(id)) { const made = new El("div"); made.hidden = ["palette", "sheet"].includes(id); elements.set(id, made); } + return elements.get(id); + }, + querySelectorAll: () => [], +}; +document.activeElement = document.body; +const all = (node) => (typeof node === "string" ? [] : [node, ...node.children.flatMap(all)]); +const classes = (node) => String(node.className || node.attrs.class || "").split(" ").filter(Boolean); +const labels = (node) => all(node).filter((el) => el.tag === "button").map((el) => el.getAttribute("aria-label") || el.textContent); +const calls = {proofs: [], posts: [], sheets: [], radio: []}; +const now = 1_800_000_000; +const base = {agent: "Skills", to: "Skills", detail: "", green: true, gate: "", score: "", warnings: [], options: [], recommended: "", command: "", directory: "", sha256: "", runnable: false, deniable: false, job: null, rule: null, dry_run: null, diff: null, pinned: false, until: 0, held: false, ts: now - 60}; +const operator = {...base, kind: "operator", id: "operator/ship", title: "make ship", command: "make ship", directory: "/repo", detail: "release the build", runnable: true, deniable: true, green: false, sha256: "abc"}; +const review = {...base, kind: "review", id: "0123456789abcdef", title: "feat: x", detail: "feature/x", gate: "pass", score: "🎯 95", directory: "/repo", diff: 4}; +const ask = {...base, kind: "ask", id: "1123456789abcdef", title: "Bash: git commit -m x", command: "git commit -m x", directory: "/repo", deniable: true}; +const decision = {...base, kind: "decision", id: "2123456789abcdef", title: "Merge?", options: [{label: "Now", description: ""}], recommended: "Now"}; +const context = { + console, Node, document, JSON, Math, Date, Intl, Promise, Set, Map, Number, String, Array, Object, RegExp, Boolean, + window: {location: {hash: "", hostname: "localhost", port: "8737", pathname: "/"}, setTimeout: () => 0, clearTimeout: () => {}, setInterval: () => 0, clearInterval: () => {}, getSelection: () => null, innerWidth: 1366, innerHeight: 768, matchMedia: () => ({matches: false})}, + Nav: require(`${dir}/nav.js`), Diffs: require(`${dir}/diffs.js`), Faces: {of: () => ({parts: []}), SIZE: 64}, + state: {badges: {tasks: 3, agents: 0, artifacts: 1}, inbox: [operator, review, ask, decision], messages: [{id: "m1", text: "Feedback on review:0123456789abcdef a.py:1-2: \"x\" fix", created_ts: now}], people: {}, graph: {nodes: []}, silence: null, presence: {}, kinds: {operator: "Operator"}}, + ui: {screen: "inbox", views: {tasks: "list"}, selected: new Set(), cursor: 0, snoozed: false, busy: false, review: null, file: 0}, + KINDS: [["operator", "Operator"], ["review", "Review"]], speech: {current: null}, look: {hidden: new Set()}, settings: {}, known: {batches: new Set()}, + relative: new Intl.RelativeTimeFormat("en"), absolute: new Intl.DateTimeFormat("en"), calm: {matches: true}, drawn: {screen: "", sig: ""}, + proofFor: async (entries, label) => { calls.proofs.push({entries, label}); return {proof: {payload: "p", assertion: "a"}}; }, + sendJson: async (path, body) => { calls.posts.push({path, body}); return {ok: true, data: {id: "b1", due: now + 3}}; }, + radioSend: async (message) => { calls.radio.push(message); return true; }, + load: async () => {}, toast: () => {}, fail: () => [], undoToast: () => {}, + jobOpen: () => false, dismissJob: () => {}, consoleOf: () => null, settleConsoles: () => {}, repaintConsoles: () => {}, + withoutOutput: (item) => item, ruleRequestBody: () => null, acceptRule: () => null, reviewSheet: () => {}, openAgent: () => {}, confirmAct: () => {}, measure: () => {}, + openSheet: (title, body) => calls.sheets.push({title, body}), closeSheet: () => {}, emptyState: () => new El("div"), onLocalhost: () => true, localLink: () => null, + focusSub: () => {}, openPalette: () => {}, closePalette: () => {}, skyLook: () => {}, popupDialog: () => ({open: false}), closePopup: () => {}, closeMenus: () => {}, helpSheet: () => {}, + space: {look: false}, stream: {events: [], entries: [], dom: null}, filter: {}, inboxDom: null, inboxSig: "", reviewDom: null, graphDom: null, tree: {}, taskView: {}, g3: {}, +}; +context.denySplit = () => { + const button = new El("button"); + button.setAttribute("aria-label", "Deny"); + return button; +}; +const source = names.map((name) => fs.readFileSync(`${dir}/${name}`, "utf8")).join("\n"); +const probe = ` +openSheet = (title, body) => calls.sheets.push({title, body}); +const operatorCard = inboxItem(operator, 0); +const streamCard = eventRow({id: "e1", kind: "operator", waiting: true, ts: now, agent: "Skills", title: "make ship", text: "release the build", ref: {task: "operator/ship", directory: "/repo", status: "queued"}}); +const plainCard = eventRow({id: "e2", kind: "operator", waiting: false, ts: now, agent: "Skills", title: "make other", text: "-", ref: {task: "operator/other", directory: "/repo", status: "done"}}); +diffView.cache.set("review:0123456789abcdef", [ + {path: "src/app.py", status: "M", mode: "100644", additions: 2, deletions: 1, patch: "@@ -1,2 +1,3 @@\\n keep\\n-old\\n+new\\n+more", note: ""}, + {path: "logo.png", status: "B", mode: "100644", additions: 0, deletions: 0, patch: "", note: ""}, + {path: "big.txt", status: "M", mode: "100644 => 100755", additions: 300, deletions: 101, patch: "@@ -1 +1 @@\\n-a\\n+b", note: ""}, + {path: "late.txt", status: "A", mode: "100644", additions: 9, deletions: 0, patch: "", note: "content withheld"}, +]); +const reviewCard = inboxItem(review, 1); +diffView.cache.set("ask:1123456789abcdef", [{path: "src/app.py", status: "M", mode: "100644", additions: 1, deletions: 0, patch: "@@ -1 +1,2 @@\\n keep\\n+new", note: ""}]); +const askCard = inboxItem({...ask, diff: 1}, 2); +const sections = all(reviewCard).filter((el) => classes(el).includes("diff-file")); +const read = (section) => { + const details = all(section).find((el) => el.tag === "details") || null; + const rows = all(section).filter((el) => el.tag === "tr"); + return {path: section.dataset.path, status: section.dataset.status, open: details ? details.getAttribute("open") !== null : null, table: rows.length > 0, + rows: rows.map((row) => [classes(row).join(" "), row.dataset.line || null]), + note: all(section).filter((el) => classes(el).includes("note")).map((el) => el.textContent), mode: all(section).filter((el) => classes(el).includes("mode")).map((el) => el.textContent), + stat: all(section).filter((el) => classes(el).includes("add") || classes(el).includes("del")).filter((el) => el.tag === "span").map((el) => el.textContent)}; +}; +drawStatus(); +const tasksBadge = navButtons.get("tasks").badge; +const badgeState = {keys: [...navButtons.keys()], text: tasksBadge.textContent, hidden: tasksBadge.hidden, urgent: tasksBadge.className, title: document.title}; +state.badges = {tasks: 0, agents: 0, artifacts: 1}; +drawStatus(); +badgeState.zero = [tasksBadge.hidden, document.title]; +const results = { + operator: {classes: classes(operatorCard), checkbox: all(operatorCard).filter((el) => el.tag === "input" && el.attrs.type === "checkbox").length, ref: operatorCard.dataset.ref, to: operatorCard.dataset.to, + command: all(operatorCard).filter((el) => classes(el).includes("command")).map((el) => [el.dataset.path, all(el).find((inner) => inner.tag === "code").textContent]), + text: operatorCard.textContent, buttons: labels(operatorCard)}, + stream: {buttons: labels(streamCard), actions: all(streamCard).filter((el) => classes(el).includes("line-actions")).length, ref: streamCard.dataset.ref || null, plain: [labels(plainCard), plainCard.dataset.ref || null]}, + review: {ref: reviewCard.dataset.ref, files: sections.map(read), feedback: all(reviewCard).filter((el) => classes(el).includes("feedback-list")).map((el) => el.textContent)}, + status: badgeState, + ask: {ref: askCard.dataset.ref, to: askCard.dataset.to, files: all(askCard).filter((el) => classes(el).includes("diff-file")).map((el) => [el.dataset.path, all(el).filter((inner) => inner.tag === "tr").length]), + regions: all(askCard).filter((el) => el.dataset.path !== undefined).map((el) => el.dataset.path)}, +}; +bulk([operator, review, ask, decision], "Grant selection", false); +const sheet = calls.sheets.pop(); +const confirm = all({children: sheet.body}).filter((el) => el.tag === "button").pop(); +results.bulk = {label: confirm.textContent, shown: all({children: sheet.body}).filter((el) => el.tag === "li").length}; +confirm.click(); +feedbackSheet({ref: "review:0123456789abcdef", to: "Skills", path: "src/app.py", from: 2, until: 4, quote: "-old\\n+new"}); +const form = calls.sheets.pop(); +const area = all({children: form.body}).find((el) => el.tag === "textarea"); +area.value = " call the helper instead "; +const send = all({children: form.body}).filter((el) => el.tag === "button").pop(); +results.feedback = {title: form.title, send: [send.getAttribute("aria-label"), send.getAttribute("title")]}; +send.click(); +let prevented = false; +feedbackKey = () => true; +keyboard({key: "f", target: document.body, altKey: false, ctrlKey: false, metaKey: false, isComposing: false, preventDefault: () => { prevented = true; }}); +results.key = prevented; +setTimeout(() => { + results.proofs = calls.proofs.map((call) => call.entries.map((entry) => [entry.kind, entry.verb, entry.label])); + results.posts = calls.posts.map((call) => [call.path, call.body.items.length, call.body.proof !== null]); + results.radio = calls.radio; + console.log(JSON.stringify(results)); +}, 20); +`; +vm.runInNewContext(`${source}\n${probe}`, {...context, setTimeout, Element: El, HTMLElement: El, now, operator, review, ask, decision, calls, all, classes, labels}); +""" + + +def git(cwd: Path, *args: str) -> str: + done = subprocess.run(["git", *args], cwd=cwd, env={**os.environ, **GIT_ENV}, check=True, capture_output=True, text=True) + return done.stdout + + +def scratch_repo(root: Path) -> Path: + repo = root / "repo" + repo.mkdir() + git(repo, "init", "-q", "-b", "main") + (repo / "app.py").write_text("keep = 1\nold = 2\n", encoding="utf-8") + (repo / "gone.txt").write_text("bye\n", encoding="utf-8") + (repo / "logo.png").write_bytes(b"\x89PNG\x00\x01") + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", "base") + (repo / "app.py").write_text(f"keep = 1\nnew = 3\n{SECRET}\nprint(1)\n", encoding="utf-8") + (repo / "added.txt").write_text("fresh\n", encoding="utf-8") + (repo / "logo.png").write_bytes(b"\x89PNG\x00\x02") + (repo / "gone.txt").unlink() + git(repo, "add", "-A") + return repo + + +def bytes_read() -> int: + counters = dict(line.split(": ") for line in Path("/proc/self/io").read_text(encoding="ascii").splitlines()) + return int(counters["rchar"]) + + +def deny(entries: object) -> dict: + return {"sandbox": {"filesystem": {"denyRead": entries}}} + + +def write_settings(file: Path, content: object) -> None: + file.parent.mkdir(parents=True, exist_ok=True) + file.unlink(missing_ok=True) + if content is not None: + file.write_bytes(content if isinstance(content, bytes) else json.dumps(content).encode()) + + +def rules_dir(root: Path) -> Path: + folder = root / "rules" / "secrets" + folder.mkdir(parents=True) + (folder / "test.yaml").write_text(RULES, encoding="utf-8") + return root / "rules" + + +class TestDiffsModule(unittest.TestCase): + @classmethod + def setUpClass(cls): + done = subprocess.run(["node", "-e", DIFFS_PROBE, str(DIFFS)], capture_output=True, text=True, timeout=60, check=True) + cls.data = json.loads(done.stdout) + + def test_a_patch_parses_into_unified_rows_with_both_line_numbers(self): + self.assertEqual(self.data["rows"], [ + ["hunk", 1, "", "", "@@ -3,3 +3,4 @@"], + ["ctx", 2, 3, 3, "keep"], + ["del", 3, 4, "", "old"], + ["add", 4, "", 4, "new"], + ["add", 5, "", 5, "more"], + ["ctx", 6, 5, 6, "tail"], + ]) + + def test_a_file_over_400_changed_lines_is_collapsed(self): + self.assertEqual(self.data["collapsed"], [False, True, True]) + + def test_selected_rows_and_command_quotes_give_a_line_range(self): + self.assertEqual(self.data["span"], [[3, 7], None, [2, 2]]) + self.assertEqual(self.data["lineSpan"], [[2, 3], [1, 1], [1, 2]]) + + def test_the_feedback_text_follows_the_contract(self): + self.assertEqual(self.data["feedback"], 'Feedback on review:0123456789abcdef src/app.py:4-5: "-old\n+new" use the helper') + self.assertEqual(self.data["quoted"], [2001, 2000]) + self.assertEqual(self.data["picked"], 1) + + +class TestOperatorPage(unittest.TestCase): + @classmethod + def setUpClass(cls): + done = subprocess.run(["node", "-e", PAGE_PROBE, str(TEMPLATES), json.dumps(PAGE_MODULES)], capture_output=True, text=True, timeout=60, check=False) + if done.returncode != 0: + raise AssertionError(done.stderr[-4000:]) + cls.data = json.loads(done.stdout) + + def test_the_tasks_entry_carries_the_waiting_count_hidden_at_zero_and_no_inbox_entry_exists(self): + status = self.data["status"] + self.assertEqual(status["keys"], ["stream", "rules", "agents", "artifacts", "tasks"]) + self.assertEqual((status["text"], status["hidden"], status["urgent"], status["title"]), ("3", False, "badge urgent", "(3) Agent Orchestrator")) + self.assertEqual(status["zero"], [True, "Agent Orchestrator"]) + + def test_an_operator_task_is_a_stream_card_with_a_checkbox_and_every_action(self): + card = self.data["operator"] + self.assertTrue({"item", "entry", "k-operator", "waiting"} <= set(card["classes"]), card["classes"]) + self.assertEqual((card["checkbox"], card["ref"], card["to"]), (1, "operator:operator/ship", "Skills")) + self.assertEqual(card["command"], [["/repo", "make ship"]]) + for text in ("in /repo", "release the build"): + with self.subTest(text=text): + self.assertIn(text, card["text"]) + for label in ("Run", "Deny", "Copy", "1 h", "tomorrow", "pin"): + with self.subTest(label=label): + self.assertIn(label, card["buttons"]) + + def test_the_stream_keeps_its_operator_card_with_its_actions(self): + stream = self.data["stream"] + self.assertEqual((stream["actions"], stream["ref"]), (1, "operator:operator/ship")) + self.assertIn("Run", stream["buttons"]) + self.assertIn("Deny", stream["buttons"]) + self.assertEqual(stream["plain"], [["Skills", "Copy"], None]) + + def test_a_review_card_shows_one_unified_block_per_file_expanded_collapsed_past_400_lines_and_binary_named_only(self): + files = self.data["review"]["files"] + self.assertEqual([(entry["path"], entry["status"], entry["open"], entry["table"]) for entry in files], [ + ("src/app.py", "M", True, True), ("logo.png", "B", None, False), ("big.txt", "M", False, True), ("late.txt", "A", None, False), + ]) + self.assertEqual(files[0]["rows"], [["hunk", None], ["ctx", "1"], ["del", "2"], ["add", "2"], ["add", "3"]]) + self.assertEqual((files[0]["note"], files[0]["mode"], files[0]["stat"]), ([], [], ["+2", "-1"])) + self.assertEqual((files[2]["mode"], files[3]["note"], files[3]["stat"]), (["100644 => 100755"], [WITHHELD], ["+9", "-0"])) + + def test_a_commit_ask_shows_its_diff_like_a_review_and_its_command_and_files_take_feedback(self): + ask = self.data["ask"] + self.assertEqual((ask["ref"], ask["to"], ask["files"]), ("ask:1123456789abcdef", "Skills", [["src/app.py", 3]])) + self.assertEqual(ask["regions"], ["/repo", "src/app.py"]) + + def test_sent_feedback_shows_under_its_card(self): + self.assertEqual(len(self.data["review"]["feedback"]), 1) + self.assertIn('a.py:1-2: "x" fix', self.data["review"]["feedback"][0]) + + def test_the_bulk_bar_counts_and_grants_asks_reviews_operator_commands_and_choices_with_one_signature(self): + self.assertEqual(self.data["bulk"], {"label": "Grant 4", "shown": 4}) + self.assertEqual(self.data["proofs"], [[["operator", "approve", ""], ["review", "approve", ""], ["ask", "approve", ""], ["decision", "approve", "Now"]]]) + self.assertEqual(self.data["posts"], [["/api/inbox/approve", 4, True]]) + + def test_feedback_is_one_signed_post_to_the_requesting_agent_in_the_contract_format_and_the_key_f_opens_it(self): + self.assertEqual(self.data["feedback"]["title"], "Feedback") + self.assertEqual(self.data["feedback"]["send"], ["Send feedback", "Send feedback: a post to Skills"]) + self.assertEqual(self.data["radio"], [{"to": "Skills", "text": 'Feedback on review:0123456789abcdef src/app.py:2-4: "-old\n+new" call the helper instead', "upload": "", "cls": "post", "voice": ""}]) + self.assertTrue(self.data["key"]) + + +class TestStagedDiff(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = Path(os.path.realpath(tmp.name)) + self.repo = scratch_repo(self.root) + self.rules = rules_dir(self.root) + for patch in ( + mock.patch.object(staged_diff, "CACHE", self.root / "cache" / "secret-rules.json"), + mock.patch.object(sandbox_paths, "MANAGED", self.root / "managed"), + mock.patch.dict(os.environ, {"HOME": str(self.root), "CLAUDE_CONFIG_DIR": str(self.root / ".claude"), "CLAUDE_PROJECT_DIR": str(self.root / "project")}), + ): + patch.start() + self.addCleanup(patch.stop) + + def test_the_staged_diff_lists_each_file_once_and_a_file_matching_a_secret_rule_ships_only_its_counts_and_the_note(self): + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, False, self.rules)} + self.assertEqual(sorted(found), ["added.txt", "app.py", "gone.txt", "logo.png"]) + self.assertEqual({path: (entry["status"], entry["mode"], entry["additions"], entry["deletions"], entry["note"]) for path, entry in found.items()}, { + "added.txt": ("A", "100644", 1, 0, ""), "app.py": ("M", "100644", 3, 1, WITHHELD), "gone.txt": ("D", "100644", 0, 1, ""), "logo.png": ("B", "100644", 0, 0, ""), + }) + self.assertEqual([found[path]["patch"] for path in ("added.txt", "app.py", "gone.txt", "logo.png")], ["@@ -0,0 +1 @@\n+fresh", "", "@@ -1 +0,0 @@\n-bye", ""]) + self.assertNotIn("plain-example", json.dumps(list(found.values()))) + self.assertFalse(hasattr(staged_diff, "masked")) + self.assertEqual(diffs.valid_diff(list(found.values())), [found[path] for path in sorted(found)]) + + def test_an_armour_line_in_the_old_or_the_new_blob_withholds_the_whole_file(self): + (self.repo / "key.txt").write_text(ARMOURED, encoding="utf-8") + (self.repo / "plain.txt").write_text("plain\n", encoding="utf-8") + git(self.repo, "add", "key.txt", "plain.txt") + git(self.repo, "commit", "-q", "-m", "armour") + (self.repo / "key.txt").write_text("before\nafter\n", encoding="utf-8") + (self.repo / "plain.txt").write_text("plain\n-----BEGIN lowercase is no armour-----\n", encoding="utf-8") + (self.repo / "fresh.txt").write_text(ARMOURED, encoding="utf-8") + git(self.repo, "add", "key.txt", "plain.txt", "fresh.txt") + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, False, self.rules)} + self.assertEqual([(found[path]["patch"], found[path]["note"], found[path]["deletions"]) for path in ("key.txt", "fresh.txt")], [("", WITHHELD, 3), ("", WITHHELD, 0)]) + self.assertEqual((found["plain.txt"]["note"], found["plain.txt"]["additions"]), ("", 1)) + self.assertNotIn("plain words only", json.dumps(list(found.values()))) + + def test_past_the_cap_the_rest_is_listed_by_stat_only(self): + with mock.patch.object(staged_diff, "DIFF_LIMIT", 40): + found = staged_diff.review_diff(self.repo, False, self.rules) + self.assertEqual([(entry["path"], bool(entry["patch"]), entry["note"]) for entry in found], [("added.txt", True, ""), ("app.py", False, WITHHELD), ("gone.txt", False, ""), ("logo.png", False, "")]) + self.assertEqual([(entry["additions"], entry["deletions"]) for entry in found], [(1, 0), (3, 1), (0, 1), (0, 0)]) + + def test_without_secret_rules_or_with_a_broken_rule_file_every_text_file_is_withheld(self): + broken = self.root / "broken-rules" + broken.mkdir() + (broken / "bad.yaml").write_text("cwe: CWE-798\nrules: [unclosed\n", encoding="utf-8") + for rules in (None, self.root / "missing", broken): + with self.subTest(rules=str(rules)): + found = staged_diff.review_diff(self.repo, False, rules) + self.assertEqual([(entry["patch"], entry["note"]) for entry in found], [("", WITHHELD)] * 3 + [("", "")]) + self.assertIsNone(staged_diff.secret_rules(broken)) + + def test_the_diff_fails_past_the_file_cap_or_the_path_limit_instead_of_the_hub_refusing_it(self): + for name, value in (("FILE_CAP", 3), ("PATH_LIMIT", 8)): + with self.subTest(limit=name), mock.patch.object(staged_diff, name, value), self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, False, self.rules) + self.assertEqual((staged_diff.FILE_CAP, staged_diff.PATH_LIMIT, diffs.FILE_CAP, diffs.PATH_LIMIT), (hub_limits.FILE_CAP, hub_limits.PATH_LIMIT) * 2) + + def test_commit_all_shows_the_executable_bit_on_disk(self): + (self.repo / "added.txt").chmod(0o755) + everything = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual((everything["added.txt"]["mode"], everything["app.py"]["mode"]), ("100755", "100644")) + git(self.repo, "commit", "-q", "-m", "next") + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual((found["added.txt"]["status"], found["added.txt"]["mode"], found["added.txt"]["additions"]), ("M", "100644 => 100755", 0)) + self.assertEqual(staged_diff.review_diff(self.repo, False, self.rules), []) + + def test_commit_all_diffs_the_tracked_working_tree_against_head_without_untracked_files(self): + (self.repo / "added.txt").write_text("fresh\nlater\n", encoding="utf-8") + (self.repo / "untracked.txt").write_text("not mine\n", encoding="utf-8") + staged = {entry["path"]: entry["additions"] for entry in staged_diff.review_diff(self.repo, False, self.rules)} + everything = {entry["path"]: entry["additions"] for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual((staged["added.txt"], everything["added.txt"]), (1, 2)) + self.assertNotIn("untracked.txt", everything) + + def test_a_planted_fsmonitor_or_clean_filter_starts_no_program(self): + marker = self.root / "ran" + git(self.repo, "config", "core.fsmonitor", f"touch {marker}") + git(self.repo, "config", "filter.evil.clean", f"touch {marker}; cat") + (self.repo / ".gitattributes").write_text("* filter=evil\n", encoding="utf-8") + for everything in (False, True): + staged_diff.review_diff(self.repo, everything, self.rules) + self.assertFalse(marker.exists()) + + def test_a_planted_promisor_remote_fetches_no_missing_object_and_starts_no_program(self): + marker = self.root / "fetched" + oid = git(self.repo, "rev-parse", "HEAD:app.py").strip() + (self.repo / ".git" / "objects" / oid[:2] / oid[2:]).unlink() + promisor = { + "core.repositoryformatversion": "1", "extensions.partialClone": "origin", "remote.origin.url": str(self.root), + "remote.origin.promisor": "true", "remote.origin.uploadpack": f"touch {shlex.quote(str(marker))} && false", + } + for key, value in promisor.items(): + git(self.repo, "config", key, value) + for everything in (False, True): + with self.subTest(everything=everything), self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, everything, self.rules) + self.assertFalse(marker.exists()) + + def test_commit_all_reads_no_file_behind_a_symlinked_parent_directory(self): + outside = self.root / "outside" + outside.mkdir() + (outside / "key").write_text("stolen = 1\n", encoding="utf-8") + for folder in ("k", "d"): + (self.repo / folder).mkdir() + (self.repo / folder / "key").write_text("mine = 1\n", encoding="utf-8") + (self.repo / "d" / "link").symlink_to("key") + git(self.repo, "add", "k", "d") + (self.repo / "k" / "key").unlink() + (self.repo / "k").rmdir() + (self.repo / "k").symlink_to(outside) + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual([found[path]["patch"] for path in ("d/key", "d/link")], ["@@ -0,0 +1 @@\n+mine = 1", "@@ -0,0 +1 @@\n+key"]) + self.assertNotIn("k/key", found) + self.assertNotIn("stolen", json.dumps(list(found.values()))) + + def test_an_index_path_leaving_the_working_tree_is_refused(self): + (self.root / "outside.txt").write_text("stolen = 1\n", encoding="utf-8") + for path in ("../outside.txt", "d/../../outside.txt", str(self.root / "outside.txt")): + with self.subTest(path=path), self.assertRaises(staged_diff.DiffError): + staged_diff.worktree(self.repo, path, "100644", "sha1", []) + + def test_a_fifo_in_the_working_tree_is_skipped_without_blocking(self): + os.mkfifo(self.repo / "pipe") + found = [] + reader = threading.Thread(target=lambda: found.append(staged_diff.worktree(self.repo, "pipe", "100644", "sha1", [])), daemon=True) + reader.start() + reader.join(10) + self.assertEqual(found, [None]) + + def test_a_working_tree_file_over_the_blob_limit_is_not_read(self): + with (self.repo / "big.bin").open("wb") as handle: + handle.truncate(4 * staged_diff.BLOB_LIMIT) + before = bytes_read() + found = staged_diff.worktree(self.repo, "big.bin", "100644", "sha1", []) + self.assertLess(bytes_read() - before, staged_diff.BLOB_LIMIT) + self.assertEqual((found[0], found[2]), ("100644", None)) + + def test_commit_all_reads_no_file_under_a_deny_read_path_and_still_reads_its_sibling(self): + (self.repo / "vault").mkdir() + (self.repo / "vault" / "key").write_text("stolen = 1\n", encoding="utf-8") + (self.repo / "open.txt").write_text("seen = 1\n", encoding="utf-8") + git(self.repo, "add", "vault", "open.txt") + write_settings(self.repo / ".claude" / "settings.local.json", deny(["~/repo/vault"])) + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual([found[path]["patch"] for path in ("vault/key", "open.txt")], ["", "@@ -0,0 +1 @@\n+seen = 1"]) + self.assertNotIn("stolen", json.dumps(list(found.values()))) + + def test_commit_all_without_any_settings_file_diffs_normal_files(self): + self.assertEqual([file for file, _ in sandbox_paths.settings_files(self.repo) if file.exists()], []) + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual((found["added.txt"]["patch"], found["app.py"]["additions"]), ("@@ -0,0 +1 @@\n+fresh", 3)) + + def test_a_planted_empty_deny_read_list_still_leaves_the_floor_unread(self): + (self.repo / ".ssh").mkdir() + (self.repo / ".ssh" / "id_ed25519").write_text("stolen = 1\n", encoding="utf-8") + (self.repo / "notes.txt").write_text("seen = 1\n", encoding="utf-8") + git(self.repo, "add", ".ssh", "notes.txt") + write_settings(self.repo / ".claude" / "settings.json", deny([])) + with mock.patch.dict(os.environ, {"HOME": str(self.repo)}): + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual([found[path]["patch"] for path in (".ssh/id_ed25519", "notes.txt")], ["", "@@ -0,0 +1 @@\n+seen = 1"]) + self.assertNotIn("stolen", json.dumps(list(found.values()))) + + def test_commit_all_fails_closed_on_an_unreadable_deny_read_setting_or_in_a_denied_working_tree(self): + cases = {"unparsable": b"{", "no utf-8": b"\xff", "no paths": deny("~/x"), "denied top": deny(["~"])} + for case, content in cases.items(): + with self.subTest(case=case): + write_settings(self.root / ".claude" / "settings.json", content) + for everything in (True, False): + with self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, everything, self.rules) + + def test_the_staged_diff_ships_no_content_under_a_denied_path(self): + (self.repo / "vault").mkdir() + (self.repo / "vault" / "key").write_text("stolen = 1\n", encoding="utf-8") + git(self.repo, "add", "vault") + write_settings(self.repo / ".claude" / "settings.json", deny(["vault"])) + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, False, self.rules)} + self.assertEqual((found["vault/key"]["status"], found["vault/key"]["patch"], found["added.txt"]["patch"]), ("A", "", "@@ -0,0 +1 @@\n+fresh")) + + def test_the_session_project_settings_and_read_rules_add_denies(self): + (self.repo / "vault").mkdir() + for name in ("vault/key", "notes.txt", "deep.pem", "open.txt"): + (self.repo / name).write_text("stolen = 1\n" if name != "open.txt" else "seen = 1\n", encoding="utf-8") + git(self.repo, "add", "vault", "notes.txt", "deep.pem", "open.txt") + write_settings(self.root / "project" / ".claude" / "settings.local.json", deny(["~/repo/vault"])) + write_settings(self.repo / ".claude" / "settings.json", {"permissions": {"deny": ["Read(/notes.txt)", "Bash(rm:*)"]}}) + write_settings(self.root / ".claude" / "settings.json", {"permissions": {"deny": ["Read(*.pem)"]}}) + found = {entry["path"]: entry["patch"] for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual([found[path] for path in ("vault/key", "notes.txt", "deep.pem", "open.txt")], ["", "", "", "@@ -0,0 +1 @@\n+seen = 1"]) + + def test_each_settings_scope_resolves_relative_entries_from_its_own_folder_and_read_rules_like_claude_code(self): + managed = self.root / "managed" + write_settings(managed / "managed-settings.json", deny(["held"])) + write_settings(self.root / ".claude" / "settings.json", {**deny(["mine"]), "permissions": {"deny": ["Read(//abs/x)", "Read(~/h)", "Read(/s)", "Read(./r/q)", "Read"]}}) + write_settings(self.repo / ".claude" / "settings.json", deny(["vault"])) + roots = set(sandbox_paths.denied_roots(self.repo)) + expected = {managed / "held", self.root / ".claude" / "mine", self.repo / "vault", Path("/abs/x"), self.root / "h", self.root / ".claude" / "s", self.repo / "r" / "q", Path("/")} + self.assertLessEqual({str(path) for path in expected}, roots) + (managed / "managed-settings.d").write_text("no folder", encoding="utf-8") + with self.assertRaises(sandbox_paths.Unreadable): + sandbox_paths.denied_roots(self.repo) + + def test_a_double_star_read_rule_also_matches_no_folder_at_all(self): + write_settings(self.repo / ".claude" / "settings.json", {"permissions": {"deny": ["Read(**/.env)", "Read(conf/**/secret/**/key)"]}}) + roots = sandbox_paths.denied_roots(self.repo) + for path in (".env", "deep/er/.env", "conf/secret/key", "conf/a/secret/key", "conf/secret/b/key", "conf/a/secret/b/key"): + with self.subTest(path=path): + self.assertTrue(sandbox_paths.denied(str(self.repo / path), roots)) + self.assertFalse(sandbox_paths.denied(str(self.repo / "env.txt"), roots)) + (self.repo / ".env").write_text("hidden = 1\n", encoding="utf-8") + git(self.repo, "add", ".env") + found = {entry["path"]: entry for entry in staged_diff.review_diff(self.repo, True, self.rules)} + self.assertEqual((found[".env"]["patch"], found["added.txt"]["patch"]), ("", "@@ -0,0 +1 @@\n+fresh")) + + def test_an_allow_turns_into_a_deny_when_the_staged_tree_changed_since_the_preview_even_in_a_file_it_withholds(self): + ask = {**ASK, "tool": "Bash", "command": "git commit -m x", "cwd": str(self.repo)} + changed = {"behavior": "deny", "message": "staged tree changed since the preview"} + edits = { + "withheld content": lambda: (self.repo / "app.py").write_text(f"{SECRET}\nslipped in\n", encoding="utf-8"), + "mode only": lambda: (self.repo / "added.txt").chmod(0o755), + "binary": lambda: (self.repo / "logo.png").write_bytes(b"\x89PNG\x00\x03"), + } + for case, edit in edits.items(): + with self.subTest(case=case): + git(self.repo, "add", "-A") + pinned, shown = permission_hook.commit_diff(ask, self.rules) + self.assertEqual((pinned, len(shown["diff"])), (git(self.repo, "write-tree").strip(), len(shown["diff"]))) + self.assertEqual(permission_hook.bound(ALLOW, ask, pinned), ALLOW) + edit() + self.assertEqual(permission_hook.bound(ALLOW, ask, pinned), ALLOW) + git(self.repo, "add", "-A") + self.assertEqual(permission_hook.bound(ALLOW, ask, pinned), changed) + self.assertEqual(permission_hook.bound({"behavior": "deny", "message": "no"}, ask, pinned), {"behavior": "deny", "message": "no"}) + + def test_an_allow_of_commit_all_is_bound_to_the_working_tree_too(self): + ask = {**ASK, "tool": "Bash", "command": "git commit -am x", "cwd": str(self.repo)} + pinned, shown = permission_hook.commit_diff(ask, self.rules) + self.assertEqual([entry["path"] for entry in shown["diff"]], ["added.txt", "app.py", "gone.txt", "logo.png"]) + self.assertEqual(permission_hook.bound(ALLOW, ask, pinned), ALLOW) + (self.repo / "app.py").write_text("unstaged edit\n", encoding="utf-8") + self.assertEqual(permission_hook.bound(ALLOW, ask, pinned)["behavior"], "deny") + + def test_an_allow_fails_closed_with_a_reason_when_the_tree_cannot_be_read(self): + ask = {**ASK, "tool": "Bash", "command": "git commit -m x", "cwd": str(self.repo)} + pinned, _ = permission_hook.commit_diff(ask, self.rules) + self.assertEqual(permission_hook.bound(ALLOW, ask, None), {"behavior": "deny", "message": permission_hook.UNBOUND}) + for error in (staged_diff.DiffError("git write-tree failed: broken\x07index"), OSError("gone"), ValueError("no ascii")): + with self.subTest(error=type(error).__name__), mock.patch.object(staged_diff, "binding", side_effect=error): + chosen = permission_hook.bound(ALLOW, ask, pinned) + self.assertEqual(chosen["behavior"], "deny") + self.assertTrue(chosen["message"].startswith(f"{permission_hook.UNBOUND}: "), chosen) + self.assertNotIn("\x07", chosen["message"]) + with mock.patch.object(staged_diff, "binding", side_effect=staged_diff.DiffError("unmerged")): + self.assertEqual(permission_hook.commit_diff(ask, self.rules), (None, {})) + self.assertEqual(permission_hook.bound(ALLOW, {**ASK, "command": "make deploy"}, None), ALLOW) + + def test_the_tree_diff_shows_the_written_tree_not_the_live_index(self): + tree = git(self.repo, "write-tree").strip() + before = staged_diff.tree_diff(self.repo, tree, self.rules) + (self.repo / "added.txt").write_text("fresh\nslipped in\n", encoding="utf-8") + git(self.repo, "add", "added.txt") + self.assertEqual(staged_diff.tree_diff(self.repo, tree, self.rules), before) + self.assertNotEqual(staged_diff.review_diff(self.repo, False, self.rules), before) + + def test_without_a_secret_pattern_no_patch_ships(self): + empty = self.root / "empty-rules" + empty.mkdir() + (empty / "none.yaml").write_text("rules: []\n", encoding="utf-8") + self.assertEqual({(entry["patch"], entry["note"]) for entry in staged_diff.review_diff(self.repo, False, empty)}, {("", WITHHELD), ("", "")}) + + def test_a_file_name_that_is_no_utf8_ships_as_backslash_escapes(self): + (self.repo / os.fsdecode(b"caf\xe9.txt")).write_text("x\n", encoding="utf-8") + git(self.repo, "add", "-A") + found = staged_diff.review_diff(self.repo, False, self.rules) + self.assertIn("caf\\xe9.txt", [entry["path"] for entry in found]) + json.dumps(found, ensure_ascii=False).encode("utf-8") + + def test_only_an_unborn_head_counts_as_empty_and_lines_split_on_newlines_only(self): + tree = git(self.repo, "rev-parse", "HEAD^{tree}").strip() + (self.repo / "added.txt").write_text("one\x0ctwo\n", encoding="utf-8") + git(self.repo, "add", "added.txt") + self.assertEqual(next(entry for entry in staged_diff.review_diff(self.repo, False, self.rules) if entry["path"] == "added.txt")["additions"], 1) + (self.repo / ".git" / "objects" / tree[:2] / tree[2:]).unlink() + with self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, False, self.rules) + + def test_a_head_naming_a_missing_commit_or_a_broken_ref_fails_and_only_an_unborn_one_is_empty(self): + fresh = self.root / "fresh" + fresh.mkdir() + git(fresh, "init", "-q", "-b", "main") + (fresh / "one.txt").write_text("one\n", encoding="utf-8") + git(fresh, "add", "one.txt") + self.assertEqual([(entry["path"], entry["status"]) for entry in staged_diff.review_diff(fresh, False, self.rules)], [("one.txt", "A")]) + commit = git(self.repo, "rev-parse", "HEAD").strip() + (self.repo / ".git" / "objects" / commit[:2] / commit[2:]).unlink() + with self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, False, self.rules) + (self.repo / ".git" / "refs" / "heads" / "main").write_text("not a ref\n", encoding="utf-8") + with self.assertRaises(staged_diff.DiffError): + staged_diff.review_diff(self.repo, False, self.rules) + + def test_the_rules_are_cached_until_a_rule_file_changes(self): + first = staged_diff.secret_rules(self.rules) + with mock.patch.object(staged_diff, "parsed_rules", side_effect=AssertionError("parsed again")): + self.assertEqual([[globs, [p.pattern for p in found]] for globs, found in staged_diff.secret_rules(self.rules)], [[globs, [p.pattern for p in found]] for globs, found in first]) + self.assertEqual([globs for globs, _ in first], [("*.py",)]) + + def test_a_changed_rule_file_or_a_broken_cache_parses_the_rules_again(self): + staged_diff.secret_rules(self.rules) + rule_file = self.rules / "secrets" / "test.yaml" + rule_file.write_text(RULES.replace("'*.py'", "'*.txt'"), encoding="utf-8") + self.assertEqual([globs for globs, _ in staged_diff.secret_rules(self.rules)], [("*.txt",)]) + for broken in ('{"signature": "x", "rules": 1}', "[]", "{", json.dumps({"signature": json.loads(staged_diff.CACHE.read_text(encoding="utf-8"))["signature"], "rules": 1})): + with self.subTest(cache=broken): + staged_diff.CACHE.write_text(broken, encoding="utf-8") + self.assertEqual([globs for globs, _ in staged_diff.secret_rules(self.rules)], [("*.txt",)]) + + def test_only_a_single_git_commit_names_its_directory_and_all_mode(self): + cases = { + "git commit -m 'feat: x'": ("/w", False), + "git commit -am 'feat: x'": ("/w", True), + "git commit -m all -a": ("/w", True), + "git commit --all --no-verify -m x": ("/w", True), + "git commit -ma": ("/w", False), + "git -C sub commit -m x": ("/w/sub", False), + "GIT_EDITOR=true git commit": ("/w", False), + "env -C sub git commit -m x": ("/w/sub", False), + "env --chdir=sub GIT_EDITOR=true git -C deeper commit -am x": ("/w/sub/deeper", True), + "git commit -m x -- -a": None, + "git commit -m x file.txt": None, + "git commit -a -m x file.txt": None, + "git commit -o -m x": None, + "git commit --only -m x": None, + "git commit -i -m x": None, + "git commit --include -m x": None, + "git commit -p": None, + "git commit --pathspec-from-file=list": None, + "git commit --pathspec-from=list": None, + "git commit --pathspec-f=list": None, + "git commit --inc -m x": None, + "git commit --on -m x": None, + "git commit --inter -m x": None, + "git commit -a --no-all -m x": None, + "git commit --al -m x": None, + "git commit --frobnicate -m x": None, + "git commit --mess 'feat: x' --no-veri": ("/w", False), + "git commit --all-m": None, + "git commit -U 3 -m x": ("/w", False), + "git commit --unified 3 -am x": ("/w", True), + "GIT_INDEX_FILE=/tmp/other git commit -m x": None, + "GIT_DIR=/tmp/other git commit -m x": None, + "env GIT_WORK_TREE=/tmp/other git commit -m x": None, + "env -i git commit -m x": None, + "git -c core.worktree=/tmp/other commit -m x": None, + "git status": None, + "cd x && git commit -a": None, + "git commit -m 'x' ; rm -rf /": None, + "echo git commit": None, + "git commit -m 'unterminated": None, + } + for command, expected in cases.items(): + with self.subTest(command=command): + found = staged_diff.commit_of(command, "/w") + self.assertEqual(None if found is None else (str(found[0]), found[1]), expected) + + +class TestHubOperatorTasks(HubCase): + def scratch(self) -> tuple[Path, Path, dict]: + root = self.outside / "w1" + root.mkdir() + home = root / "home" + home.mkdir() + claude = {"HOME": str(home), "CLAUDE_CONFIG_DIR": str(home / ".claude"), "CLAUDE_PROJECT_DIR": str(root)} + return scratch_repo(root), rules_dir(root), {**os.environ, **GIT_ENV, **claude, "XDG_CACHE_HOME": str(home / ".cache")} + + def test_approval_request_ships_the_masked_staged_diff_with_the_review_and_the_card_reads_it(self): + repo, rules, env = self.scratch() + notes = self.outside / "table.txt" + notes.write_text("🎯 95 💥 3", encoding="utf-8") + page = self.page("w1.html", PAGE) + command = [sys.executable, str(APPROVAL), "request", "--port", str(self.port), "--title", "feat: w1", "--page", page, "--notes", str(notes), "--gate", "pass", "--score", "🎯 95 💥 3", "--agent", "Skills", "--rules", str(rules)] + done = subprocess.run(command, cwd=repo, env=env, capture_output=True, text=True, timeout=60, check=False) + self.assertEqual(done.returncode, 0, done.stderr) + answer = json.loads(done.stdout) + self.assertEqual((answer["status"], answer["url"]), ("pending", f"{self.origin}/")) + item = next(item for item in self.snapshot()["inbox"] if item["id"] == answer["id"]) + self.assertEqual((item["kind"], item["diff"], item["agent"], item["to"]), ("review", 4, "Skills", "Skills")) + status, _, body = self.get(f"/api/diffs/review/{answer['id']}") + self.assertEqual(status, 200, body) + shipped = json.loads(body)["diff"] + self.assertEqual([(entry["path"], entry["status"], entry["note"]) for entry in shipped], [("added.txt", "A", ""), ("app.py", "M", WITHHELD), ("gone.txt", "D", ""), ("logo.png", "B", "")]) + self.assertNotIn(b"plain-example", body) + self.assertEqual(self.tool("review_status", {"id": answer["id"]})["structuredContent"]["tree"], git(repo, "write-tree").strip()) + for path in ("/api/diffs/review/0123456789abcdef", f"/api/diffs/ask/{answer['id']}", f"/api/diffs/decision/{answer['id']}"): + with self.subTest(path=path): + self.assertEqual(self.get(path)[0], 404) + + def test_a_review_request_takes_a_diff_past_the_old_body_limit_and_refuses_a_broken_one(self): + big = [{"path": f"f{index}.txt", "status": "A", "mode": "100644", "additions": 1, "deletions": 0, "patch": "@@ -0,0 +1 @@\n+" + "x" * 300_000, "note": ""} for index in range(5)] + arguments = {**self.arguments("feat: big", "", self.page("big.html", PAGE)), "agent": "Skills", "diff": big} + result = self.tool("review_request", arguments) + self.assertFalse(result["isError"], result) + self.assertEqual(len(json.loads(self.get(f"/api/diffs/review/{result['structuredContent']['id']}")[2])["diff"]), 5) + good = {"path": "a", "status": "M", "mode": "100644 => 100755", "additions": 1, "deletions": 0, "patch": "@@ -1 +1 @@\n+x", "note": ""} + self.assertFalse(self.tool("review_request", {**arguments, "diff": [good, {**good, "path": "b", "patch": "", "note": WITHHELD}]})["isError"]) + for broken in ([{**good, "status": "X"}], [{**good, "status": "B"}], [{**good, "mode": "755"}], [{**good, "note": "masked"}], [{**good, "note": WITHHELD}], [{**good, "masked": []}], + [{**good, "extra": 1}], [{**good, "additions": -1}], {"a": 1}, [{**good, "path": "p" * (hub_limits.PATH_LIMIT + 1)}], [good] * (hub_limits.FILE_CAP + 1), + [{**good, "patch": "x" * (diffs.DIFF_LIMIT + 1)}]): + with self.subTest(broken=str(broken)[:60]): + self.assertTrue(self.tool("review_request", {**arguments, "diff": broken})["isError"]) + + def test_approval_request_refuses_notes_past_the_hub_limit_instead_of_cutting_them(self): + repo, rules, env = self.scratch() + notes = self.outside / "long.txt" + notes.write_text("x" * (hub_limits.NOTES_LIMIT + 1), encoding="utf-8") + command = [sys.executable, str(APPROVAL), "request", "--port", str(self.port), "--title", "feat: w1", "--page", self.page("long.html", PAGE), "--notes", str(notes), "--gate", "pass", "--score", "🎯 95 💥 3", "--agent", "Skills", "--rules", str(rules)] + done = subprocess.run(command, cwd=repo, env=env, capture_output=True, text=True, timeout=60, check=False) + self.assertEqual((done.returncode, done.stdout), (1, "")) + self.assertIn(f"at most {hub_limits.NOTES_LIMIT}", done.stderr) + + def test_the_hook_attaches_the_staged_diff_to_a_git_commit_ask_and_nothing_to_other_commands(self): + repo, rules, env = self.scratch() + with mock.patch.object(staged_diff, "CACHE", self.outside / "cache.json"), mock.patch.dict(os.environ, {key: env[key] for key in ("HOME", "CLAUDE_CONFIG_DIR", "CLAUDE_PROJECT_DIR")}): + commit = {**ASK, "command": "git commit -m 'feat: w1'", "cwd": str(repo)} + pinned, attached = permission_hook.commit_diff(commit, rules) + self.assertEqual(([entry["path"] for entry in attached["diff"]], pinned), (["added.txt", "app.py", "gone.txt", "logo.png"], git(repo, "write-tree").strip())) + self.assertEqual(permission_hook.commit_diff({**commit, "command": "make deploy"}, rules), (None, {})) + self.assertEqual(permission_hook.commit_diff({**commit, "tool": "Write"}, rules), (None, {})) + self.assertEqual(permission_hook.commit_diff({**commit, "cwd": str(self.outside / "nowhere")}, rules), (None, {})) + ask = {key: commit[key] for key in ("tool", "command", "cwd", "session", "agent")} + ask_id = permission_hook.create(f"http://{self.host}", ask, 60, permission_hook.commit_diff(ask, rules)[1]) + item = next(item for item in self.snapshot()["inbox"] if item["id"] == ask_id) + self.assertEqual((item["kind"], item["diff"]), ("ask", 4)) + status, _, body = self.get(f"/api/diffs/ask/{ask_id}") + self.assertEqual((status, [entry["status"] for entry in json.loads(body)["diff"]]), (200, ["A", "M", "D", "B"])) + plain = self.create_ask(ASK) + self.assertIsNone(next(item for item in self.snapshot()["inbox"] if item["id"] == plain)["diff"]) + self.assertEqual(self.post_json("/api/asks", {**ASK, "diff": [{"path": "a"}]}, self.local())[0], 400) + + def test_an_ask_whose_encoded_body_passes_the_hub_limit_goes_without_its_diff_instead_of_a_413(self): + wide = "\U0001f600" * (hub_limits.DIFF_LIMIT // 4 - 16) + diff = [{"path": "wide.txt", "status": "A", "mode": "100644", "additions": 1, "deletions": 0, "patch": f"@@ -0,0 +1 @@\n+{wide}", "note": ""}] + self.assertEqual(diffs.valid_diff(diff), diff) + ask = {key: ASK[key] for key in ("tool", "command", "cwd", "session", "agent")} + self.assertGreater(len(permission_hook.encoded({**ask, "timeout": 60, "diff": diff})), hub_limits.DIFF_BODY_LIMIT) + ask_id = permission_hook.create(f"http://{self.host}", ask, 60, {"diff": diff}) + self.assertIsNone(next(item for item in self.snapshot()["inbox"] if item["id"] == ask_id)["diff"]) + small = permission_hook.create(f"http://{self.host}", ask, 60, {"diff": [{**diff[0], "patch": "@@ -0,0 +1 @@\n+\U0001f600"}]}) + self.assertEqual(next(item for item in self.snapshot()["inbox"] if item["id"] == small)["diff"], 1) + + def test_approval_request_with_a_broken_rule_file_withholds_every_text_file_and_exits_0(self): + repo, rules, env = self.scratch() + (rules / "secrets" / "test.yaml").write_text("cwe: CWE-798\nrules: [unclosed\n", encoding="utf-8") + notes = self.outside / "broken.txt" + notes.write_text("notes", encoding="utf-8") + command = [sys.executable, str(APPROVAL), "request", "--port", str(self.port), "--title", "feat: w1", "--page", self.page("broken.html", PAGE), "--notes", str(notes), "--gate", "pass", "--score", "🎯 95 💥 3", "--agent", "Skills", "--rules", str(rules)] + done = subprocess.run(command, cwd=repo, env=env, capture_output=True, text=True, timeout=60, check=False) + self.assertEqual(done.returncode, 0, done.stderr) + self.assertNotIn("Traceback", done.stderr) + shipped = json.loads(self.get(f"/api/diffs/review/{json.loads(done.stdout)['id']}")[2])["diff"] + self.assertEqual([(entry["patch"], entry["note"]) for entry in shipped], [("", WITHHELD)] * 3 + [("", "")]) + + +class TestSharedLimits(unittest.TestCase): + def test_the_client_and_the_hub_take_each_limit_from_hub_limits(self): + self.assertEqual(Path(signed.limits.__file__).resolve(), (HUB / "client" / "hub_limits.py").resolve()) + self.assertEqual((staged_diff.DIFF_LIMIT, diffs.DIFF_LIMIT, reviewing.NOTES_LIMIT), (hub_limits.DIFF_LIMIT, hub_limits.DIFF_LIMIT, hub_limits.NOTES_LIMIT)) + self.assertEqual((diffs.DIFF_BODY_LIMIT, diffs.FILE_CAP, diffs.PATH_LIMIT, fields.PATH_LIMIT, permission_hook.PATH_LIMIT), (hub_limits.DIFF_BODY_LIMIT, hub_limits.FILE_CAP, *[hub_limits.PATH_LIMIT] * 3)) + self.assertEqual((staged_diff.WITHHELD, diffs.NOTES), (WITHHELD, ("", WITHHELD))) + self.assertTrue(fields.PATH.fullmatch("/" + "p" * (hub_limits.PATH_LIMIT - 1))) + self.assertFalse(fields.PATH.fullmatch("/" + "p" * hub_limits.PATH_LIMIT)) + + def test_the_encoded_worst_case_body_without_a_diff_fits_the_hub_body_limit_non_ascii_included(self): + chars = ("x", "\x01", "é", "
", "\U0001f600") + widest = {ascii_only: max(chars, key=lambda char: len(json.dumps(char, ensure_ascii=ascii_only).encode("utf-8"))) for ascii_only in (True, False)} + self.assertEqual((widest[True], widest[False]), ("\U0001f600", "\x01")) + ask = {name: widest[True] * limit for name, limit in permission_hook.LIMITS.items()} + self.assertLessEqual(len(permission_hook.encoded({**ask, "timeout": float(1 << 60)})), hub_limits.DIFF_BODY_LIMIT) + wide, path = widest[False], "/" + widest[False] * (hub_limits.PATH_LIMIT - 1) + arguments = {"repo": path, "worktree": path, "branch": "b" * 255, "tree": "0" * 64, "title": wide * 200, "page": path, "notes": wide * hub_limits.NOTES_LIMIT, + "gate": "pass", "score": wide * reviewing.SCORE_LIMIT, "agent": wide * 200} + self.assertLessEqual(len(approval.review_body(arguments)), hub_limits.DIFF_BODY_LIMIT) + patch = "+" + widest[True] * (hub_limits.DIFF_LIMIT // 4 - 2) + self.assertLessEqual(len(json.dumps(patch, ensure_ascii=False).encode("utf-8")), hub_limits.DIFF_LIMIT) + self.assertGreater(len(permission_hook.encoded({"diff": [{"patch": patch}]})), hub_limits.DIFF_BODY_LIMIT) + + def test_the_hub_image_copies_every_client_module_signed_loads(self): + tree = ast.parse((HUB / "kvb_orchestrator" / "signed.py").read_text(encoding="utf-8")) + loaded = {f"{node.args[0].value}.py" for node in ast.walk(tree) if isinstance(node, ast.Call) and getattr(node.func, "id", "") == "client_module"} + copies = [line.split()[1:] for line in (HUB / "Dockerfile").read_text(encoding="utf-8").splitlines() if line.startswith("COPY ")] + copied = {Path(source).name for words in copies if words[-1] == "/app/client/" for source in words[:-1]} + self.assertIn("hub_limits.py", loaded) + self.assertLessEqual(loaded, copied) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_hub_surface.py b/tests/test_hub_surface.py index 7ac6b0f..95fc311 100644 --- a/tests/test_hub_surface.py +++ b/tests/test_hub_surface.py @@ -24,16 +24,16 @@ class TestHubSurface(HubCase): self.assertNotIn(gone, html) self.assertIn('