diff --git a/plugins/source-control/.claude-plugin/plugin.json b/plugins/source-control/.claude-plugin/plugin.json index 215123d375..2644103235 100644 --- a/plugins/source-control/.claude-plugin/plugin.json +++ b/plugins/source-control/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "source-control", - "version": "0.26.12", + "version": "0.28.0", "description": "Git and GitHub delivery workflow: /commit (Conventional Commits + Co-Authored-By trailer via safe heredoc mechanics), /pull-request (prep, create, CI monitoring, review-comment triage, merge, CI-log fetch), /babysit-prs (self-pacing fleet loop — safe by default; opt-in worker/autopilot tiers add gate-checked merge and thread resolution behind a deterministic Python engine), /babysit-loop (the loop-lane merge lane: a standing or drain loop that invokes babysit-prs per cycle, configured through repo-scoped babysit_loop_* keys on the layered source-control.md seam, with merge authority human-only until the target repo's tracked config adopts the lane, a gate-proven C2-mechanical baseline once adopted, and merge-rung raises binding from the team-tracked layer only), /worktree (create, status, cleanup, audit for parallel-session isolation), /setup (check the effective commit-subject / PR-title convention merged across its config layers and the babysit-prs config, or apply — interview the repo and write the convention config to a chosen layer), and /resolve-conflicts (intent-first merge/rebase conflict resolution with a semantic-conflict sweep — never --abort). The commit-subject / PR-title convention is configurable via a source-control.md config written by a re-runnable setup skill, layered across a ~/.claude user-global file, the tracked team file, and a gitignored .claude/source-control.local.md personal overlay merged per key; Conventional Commits is the default when no convention is declared.", "author": { "name": "Melodic Software", diff --git a/plugins/source-control/CHANGELOG.md b/plugins/source-control/CHANGELOG.md index ee567d45a7..23767b6a9f 100644 --- a/plugins/source-control/CHANGELOG.md +++ b/plugins/source-control/CHANGELOG.md @@ -3,6 +3,37 @@ All notable changes to the `source-control` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.28.0] + +### Added + +- **`babysit-prs` guard semantics are now an executable contract (`#1265`).** The facts a host + permission classifier has to know about this lane — which entry points mutate, which flags gate + which guard, where a refusal is enforced, and how a mutation is actually performed — were + restated in prose by every consumer and had nothing detecting drift. They are now a table in + `skills/babysit-prs/scripts/tests/guard_contract.py`, executed row by row against the real entry + points by `test_guards.py`, and rendered to a citable + `skills/babysit-prs/reference/guard-contract.md`. Every row carries the prose claim it backs, so + a changed guard fails CI with a message naming the downstream claim that just became false. Five + binding kinds: refusals (invoked, exit code and message asserted), predicates (the classifier + called directly, because `--autonomous`'s `isOutdated` requirement is a condition over fetched + API data that no argument shape expresses), effects (run offline against a throwaway state dir — + this is what proves `manage_babysit_lease.py acquire` writes with no `--apply`, contrary to what + its flag names suggest), mechanisms (`refresh_pr_branch.py` uses GitHub's server-side + `update-branch` and never pushes), and documented command lines (every `bin/`-path wrapper + command spelled in `reference/safety.md` and `reference/orchestration.md` is checked against the + backing CLI's own parser). Catalogue gates fail when a new entry point, wrapper, or + command-spelling document arrives without a row — including the plugin-level + `scripts/babysit-readiness-gate.sh`, the one lane entry point outside the skill's scripts + directory. Each binding asserts the specific claim rather than a proxy for it: a row claiming + the refusal precedes every network call is replayed against a recording `gh` shim and fails if + the shim ran at all, an effect row records which way the state directory's file set moved so a + rewrite cannot pass as a deletion, and documented flags are checked against the parser's usage + block rather than scraped `--help` prose that names flags the CLI rejects. What CI does not + bind is stated in the generated doc's "Not covered here" section rather than left to inference: + the entry-point **Class** column cannot be proven for the four entry points whose mutation is a + GitHub write, because every row runs without network access. + ## [0.26.12] ### Fixed diff --git a/plugins/source-control/skills/babysit-prs/reference/guard-contract.md b/plugins/source-control/skills/babysit-prs/reference/guard-contract.md new file mode 100644 index 0000000000..5483c5984d --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/reference/guard-contract.md @@ -0,0 +1,128 @@ +# Guard contract + + + +What a host permission classifier, an orchestration prompt, or a downstream +consumer may assume about this lane's entry points. Rows in the refusal, +predicate, effect, and documented-command tables are executed as assertions by +`scripts/tests/test_guards.py`; a guard change that falsifies one fails CI with +a message naming the claim. The columns listed under "Not covered here" are +rendered from the same data but are not asserted — read those as annotation, not +as proof. + +Cite a row by its ID. IDs are stable; rows are removed only when the behavior is. + +Source anchors are `file::symbol`, deliberately not line numbers. + +## Entry points: what mutates + +This table scopes `mutation` to DOMAIN state -- GitHub, the queue state file, worktrees, and leases -- and `gate` is the condition under which that domain mutation happens at all. It is deliberately not a filesystem-write audit: a state-touching script performs housekeeping under `--state-dir` whatever its flags say, so entering `state_lock` creates the state directory and a `.lock` sibling, and `load_state` quarantines an already-corrupt state file by renaming it. A classifier granting a gate-less invocation therefore still needs the script to be able to write inside `--state-dir`; what the gate withholds is the domain mutation, not every byte. A blank wrapper column means the entry point has no `bin/` wrapper and is invoked through the interpreter. + +| Entry point | Wrapper | Class | Mutates | Gate | Claim | Backed by | +| --- | --- | --- | --- | --- | --- | --- | +| `skills/babysit-prs/scripts/babysit_merge.py` | `bin/source-control-babysit-merge` | conditionally mutating | merges the PR on GitHub | --merge (absent: readiness check only, exit 0 ready / 10 not ready) | Without --merge this is a readiness reporter. With it, the TOCTOU guard requires --expected-head unless --allow-unpinned-head is passed -- and the bin/ wrapper refuses that override outright. | `merge.allowlist-absent`, `merge.unpinned-head-refused-by-wrapper`, `merge.abbreviated-unpinned-head-refused-by-wrapper`, `merge.abbreviation-is-not-resolved-by-the-cli`, `merge.wrapper-filters-unpinned-head-in-bash`, `merge.parser-refuses-abbreviation` | +| `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `bin/source-control-babysit-resolve-thread` | conditionally mutating | marks review threads resolved on GitHub | --resolve (absent: classification report only) | Which threads it may touch is decided per fetched thread by the classifier, not by the argument shape -- see the predicate rows. The wrapper adds no refusal of its own. | `resolve.allowlist-absent`, `resolve.autonomous-bulk-refused`, `classify.interactive-does-not-require-outdated`, `resolve.wrapper-carries-no-filter`, `resolve.parser-refuses-abbreviation` | +| `skills/babysit-prs/scripts/manage_babysit_lease.py` | -- | mutating | writes, renews, and deletes lease files under --state-dir | none for acquire / heartbeat / release -- they write unconditionally; --apply gates reap's deletion ONLY | Local filesystem writes only, no GitHub write. The --apply flag does not make this script read-only in its other three actions. | `lease.acquire-writes-without-apply`, `lease.reap-without-apply-is-a-dry-run` | +| `skills/babysit-prs/scripts/manage_feedback_ledger.py` | -- | conditionally mutating | records feedback dispositions and fix rounds in queue state | --apply, additionally requiring --lease-token | Local state only, no GitHub write. The lease token requirement means an --apply invocation still refuses without proof of lease ownership. | `ledger.apply-requires-lease-token` | +| `skills/babysit-prs/scripts/prune_babysit_worktrees.py` | -- | conditionally mutating | runs `git worktree remove` on eligible worktrees under --root | --apply (absent: dry run) | The most destructive local helper in the lane. --root is required, so it cannot sweep a directory the caller did not name. | `prune.root-is-required`, `prune.removes-worktrees-via-git` | +| `skills/babysit-prs/scripts/refresh_pr_branch.py` | -- | conditionally mutating | asks GitHub to update the PR branch server-side; writes queue state | --apply | Mutates the PR branch on GitHub WITHOUT pushing: the update is a pinned server-side update-branch API call. | `refresh.uses-server-side-update-branch` | +| `skills/babysit-prs/scripts/request_review.py` | -- | conditionally mutating | POSTs an issue comment requesting an AI re-review; writes queue state | --apply | A GitHub write despite the read-sounding name. | `review.posts-a-comment` | +| `skills/babysit-prs/scripts/pr_queue_snapshot.py` | -- | conditionally mutating | writes the queue snapshot to local state | --write-state (absent: reports the snapshot without persisting it) | No GitHub write on any path. The only domain-state mutation is the local snapshot file. | `snapshot.write-state-gates-the-only-write` | +| `skills/babysit-prs/scripts/babysit_findings.py` | -- | read-only | nothing | n/a | Pure classification over supplied input; no write of any kind. | -- | +| `scripts/babysit-readiness-gate.sh` | -- | read-only | nothing | n/a | The lane's one entry point outside the skill's own scripts directory, invoked by name from SKILL.md. It counts findings and classification rows over fetched comments and reports READINESS_OK / READINESS_BLOCKED; it writes no file and performs no GitHub write. | -- | + +## Refusals: guards that fire on argument shape alone + +Each row is executed. `Refused by` says which layer rejected the invocation. On a `bin/` entry point the distinction is asserted, using the observable discriminator: a bash-wrapper refusal never reaches the interpreter, so it emits plain stderr and no JSON envelope. On a CLI entry point the layer is always `python-cli` and only the exit code and message are asserted. + +`No gh` marks the rows whose claim is that the refusal precedes every network call. Those are replayed a second time with a recording `gh` shim as the only executable on `PATH`, and the assertion is that the shim was never invoked -- merely removing `gh` from `PATH` would let a swallowed lookup failure fall through to the same exit code and read as proof. + +| ID | Entry point | Invocation | Exit | Refused by | No gh | Error names | Enforced at | Claim | +| --- | --- | --- | --- | --- | --- | --- | --- | --- | +| `merge.allowlist-absent` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1` | 3 | python-cli | asserted | `allowed-owners` | `babysit_merge.py::main` | The merge gate is fail-closed on scope: with no --allowed-owners it refuses every PR at exit 3 rather than defaulting to the current repo's owner. | +| `merge.owner-out-of-scope` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners someone-else` | 3 | python-cli | asserted | -- | `babysit_merge.py::main` | An owner outside --allowed-owners is refused at exit 3, so a permission rule may assume the gate never acts on a repository the operator did not name. | +| `merge.owner-check-precedes-self-login-resolution` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners someone-else --self-logins @me` | 3 | python-cli | asserted | -- | `babysit_merge.py::main` | The owner check runs before '@me' self-login resolution, which is a network call: an out-of-scope owner refuses at exit 3 with no gh invocation at all. | +| `merge.short-expected-head` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners owner --merge --expected-head abc` | 2 | python-cli | -- | `expected-head` | `babysit_merge.py::main` | --expected-head must be a long-enough SHA prefix; a short value is a usage error at exit 2, never a loose prefix match against the live head. | +| `merge.malformed-ref` | `skills/babysit-prs/scripts/babysit_merge.py` | `not-a-ref --allowed-owners owner` | 2 | python-cli | -- | -- | `babysit_merge.py::main` | The PR reference must parse as owner/repo#number or a PR URL; anything else is a usage error at exit 2. | +| `merge.autopilot-tier-without-required-sets` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners owner --autopilot-merge-tier` | 3 | python-cli | asserted | `--lane-logins`, `--approver-bot-logins`, `--block-labels` | `babysit_merge.py::main` | The autopilot merge tier is fail-closed: the umbrella flag alone, with none of --lane-logins, --approver-bot-logins, or --block-labels, refuses at exit 3 before any network access. | +| `merge.autopilot-tier-partial-config` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners owner --autopilot-merge-tier --lane-logins lane --approver-bot-logins bot` | 3 | python-cli | asserted | `--block-labels` | `babysit_merge.py::main` | A partially configured autopilot tier still refuses at exit 3, naming only the missing set -- there is no partial-credit tier. | +| `merge.tier-params-without-umbrella` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners owner --lane-logins lane` | 2 | python-cli | -- | `--autopilot-merge-tier` | `babysit_merge.py::main` | Tier parameter sets without --autopilot-merge-tier are a usage error at exit 2, never a silent no-op that reads as configured. | +| `merge.unpinned-head-refused-by-wrapper` | `bin/source-control-babysit-merge` | `owner/repo#1 --merge --allow-unpinned-head` | 2 | bash-wrapper | -- | `--allow-unpinned-head` | `bin/source-control-babysit-merge (argument filter loop)` | The bin/ wrapper refuses --allow-unpinned-head in bash, before Python runs: no allow-rule-covered invocation of the bare command can merge an unvetted head. The refusal prints plain text to stderr and emits no JSON envelope, which is how a caller can tell the wrapper -- not the CLI -- rejected it. | +| `merge.abbreviated-unpinned-head-refused-by-wrapper` | `bin/source-control-babysit-merge` | `owner/repo#1 --merge --allow-unpinned-hea` | 2 | bash-wrapper | -- | `--allow-unpinned-hea` | `bin/source-control-babysit-merge (argument filter loop)` | The wrapper refusal covers every long-option PREFIX of --allow-unpinned-head, not the exact spelling alone. An equality-only filter would let `--allow-unpinned-hea` through to an abbreviation-resolving parser and reinstate the override the wrapper exists to remove, so the prefix family is refused as one. | +| `merge.abbreviation-is-not-resolved-by-the-cli` | `skills/babysit-prs/scripts/babysit_merge.py` | `owner/repo#1 --allowed-owners owner --merge --allow-unpinned-hea` | 2 | python-cli | -- | `--allow-unpinned-hea` | `babysit_merge.py::main` | The wrapper's prefix filter is belt to the CLI's braces: the parser sets allow_abbrev=False, so an abbreviated flag reaching Python directly is an unrecognized argument at exit 2 rather than a silently resolved override. | +| `merge.wrapper-reaches-failclosed-cli` | `bin/source-control-babysit-merge` | `owner/repo#1` | 3 | python-cli | -- | `allowed-owners` | `babysit_merge.py::main` | The wrapper adds no capability of its own: with no --allowed-owners it hands off to the CLI, which refuses at exit 3 exactly as a direct invocation does. | +| `resolve.allowlist-absent` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1` | 3 | python-cli | asserted | -- | `babysit_resolve_thread.py::main` | The thread resolver is fail-closed on scope the same way the merge gate is: no --allowed-owners means exit 3. | +| `resolve.count-pin-without-thread-id` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1 --allowed-owners owner --expected-comment-count 3` | 2 | python-cli | -- | `thread-id` | `babysit_resolve_thread.py::main` | --expected-comment-count without --thread-id is a usage error at exit 2: a pin with no thread to pin would silently fall through to resolving every eligible thread. | +| `resolve.last-updated-pin-without-thread-id` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1 --allowed-owners owner --expected-last-updated 2026-07-17T10:00:00Z` | 2 | python-cli | -- | `thread-id` | `babysit_resolve_thread.py::main` | --expected-last-updated without --thread-id is a usage error at exit 2, for the same silent-bulk-fallthrough reason as the count pin. | +| `resolve.thread-id-without-pins` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1 --allowed-owners owner --resolve --thread-id PRRT_abc` | 2 | python-cli | -- | `--expected-comment-count`, `--expected-last-updated` | `babysit_resolve_thread.py::main` | A --thread-id resolve requires BOTH TOCTOU pins; supplying neither is a usage error at exit 2 naming both missing flags. | +| `resolve.autonomous-bulk-refused` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1 --allowed-owners owner --autonomous --resolve` | 2 | python-cli | -- | `thread-id`, `bulk-resolve` | `babysit_resolve_thread.py::main` | --autonomous --resolve without --thread-id is refused at exit 2: --autonomous means SINGLE PINNED THREAD, never bulk. A worker's own push marks a thread isOutdated, so a bulk autonomous resolve would clear threads with no proof the finding was addressed. | +| `resolve.autonomous-allow-unpinned-refused` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | `owner/repo#1 --allowed-owners owner --autonomous --resolve --thread-id PRRT_abc --allow-unpinned-thread` | 2 | python-cli | -- | `allow-unpinned-thread` | `babysit_resolve_thread.py::main` | There is no unpinned autonomous resolve: --allow-unpinned-thread is an interactive-only override and is refused at exit 2 under --autonomous even with a single --thread-id. | +| `resolve.wrapper-reaches-failclosed-cli` | `bin/source-control-babysit-resolve-thread` | `owner/repo#1` | 3 | python-cli | -- | -- | `babysit_resolve_thread.py::main` | The resolve wrapper is a pure passthrough: no --allowed-owners refuses at exit 3 from the CLI, unchanged by the wrapper. | +| `resolve.wrapper-filters-nothing` | `bin/source-control-babysit-resolve-thread` | `owner/repo#1 --allowed-owners owner --autonomous --resolve --thread-id PRRT_abc --allow-unpinned-thread` | 2 | python-cli | -- | `allow-unpinned-thread` | `babysit_resolve_thread.py::main` | ASYMMETRY: unlike the merge wrapper, the resolve wrapper filters no argument. Its --allow-unpinned-thread refusal comes from Python -- proven by the JSON envelope on stdout -- so an operator may not assume a bash-level refusal exists on this entry point just because one exists on the other. | +| `ledger.apply-requires-lease-token` | `skills/babysit-prs/scripts/manage_feedback_ledger.py` | `dispose --pr owner/repo#1 --expected-head-sha 0123456789abcdef0123456789abcdef01234567 --feedback-id F1 --reason contract-row --state-dir {state_dir} --apply` | 2 | python-cli | -- | `lease token` | `manage_feedback_ledger.py::main` | A ledger write is lease-gated: --apply without --lease-token refuses at exit 2 rather than writing an unowned entry. | +| `prune.root-is-required` | `skills/babysit-prs/scripts/prune_babysit_worktrees.py` | `--state-dir {state_dir}` | 2 | python-cli | -- | `--root` | `prune_babysit_worktrees.py::main` | prune_babysit_worktrees.py has no default root: omitting --root is a usage error at exit 2, so it can never sweep an unintended directory. | +| `lease.state-dir-is-required` | `skills/babysit-prs/scripts/manage_babysit_lease.py` | `reap` | 2 | python-cli | -- | `--state-dir` | `manage_babysit_lease.py::main` | State-dir resolution is flag-only with no environment fallback: omitting --state-dir is a usage error at exit 2. | +| `lease.scope-required-for-non-reap` | `skills/babysit-prs/scripts/manage_babysit_lease.py` | `acquire --state-dir {state_dir}` | 2 | python-cli | -- | `--scope` | `manage_babysit_lease.py::main` | --scope is required for acquire, heartbeat, and release; only reap runs without it. | + +## Predicates: guards over runtime data, not flags + +These decide per fetched thread, so no argument shape can express them and no argparse introspection can recover them. `thread` is the projected thread record; `flags` are the classifier's keyword arguments. + +| ID | Thread | Flags | Result | Enforced at | Claim | +| --- | --- | --- | --- | --- | --- | +| `classify.human-thread-refused-by-default` | isResolved=False, botOnly=False, isOutdated=True | autonomous=False, only_outdated=False, include_human=False | `skipped-human-thread` | `babysit_resolve_thread.py::classify` | Bot-only is a bright line, not a heuristic: a thread with ANY human participant is skipped whenever --include-human is absent, regardless of every other flag. | +| `classify.include-human-opts-in` | isResolved=False, botOnly=False, isOutdated=True | autonomous=False, only_outdated=False, include_human=True | `eligible` | `babysit_resolve_thread.py::classify` | --include-human is the ONLY thing that lifts the bot-only line. A permission rule asserting 'bot threads only' is false for any invocation carrying it. | +| `classify.autonomous-requires-outdated` | isResolved=False, botOnly=True, isOutdated=False | autonomous=True, only_outdated=False, include_human=False | `skipped-not-outdated` | `babysit_resolve_thread.py::classify` | --autonomous requires the deterministic isOutdated 'addressed' signal: a still-current bot thread is skipped so an unattended worker cannot resolve a live finding and self-satisfy the merge gate. | +| `classify.only-outdated-requires-outdated` | isResolved=False, botOnly=True, isOutdated=False | autonomous=False, only_outdated=True, include_human=False | `skipped-not-outdated` | `babysit_resolve_thread.py::classify` | --only-outdated imposes the same isOutdated requirement as --autonomous, independently of it. | +| `classify.interactive-does-not-require-outdated` | isResolved=False, botOnly=True, isOutdated=False | autonomous=False, only_outdated=False, include_human=False | `eligible` | `babysit_resolve_thread.py::classify` | THE CLAIM CONSUMERS GET WRONG: 'resolves only outdated bot threads' is TRUE ONLY under --autonomous or --only-outdated. With neither flag, a current (not outdated) bot thread is eligible. | +| `classify.include-human-does-not-lift-outdated` | isResolved=False, botOnly=False, isOutdated=False | autonomous=True, only_outdated=False, include_human=True | `skipped-not-outdated` | `babysit_resolve_thread.py::classify` | --include-human lifts only the bot-only line. Under --autonomous the isOutdated requirement still applies, so the two guards are independent and neither flag opens the other. | +| `classify.already-resolved-precedes-every-guard` | isResolved=True, botOnly=False, isOutdated=False | autonomous=True, only_outdated=False, include_human=False | `skipped-already-resolved` | `babysit_resolve_thread.py::classify` | An already-resolved thread is skipped before any other guard is consulted, so a re-run never re-resolves and never reports a human thread it declined. | + +## Effects: what reaches disk + +Executed offline against a throwaway state directory seeded with `Fixture`. `State delta` is the observed before/after movement of that directory's file set, not a reading of the flag names, and it is directional rather than a changed/unchanged boolean: a row claiming deletion asserts that the file set strictly shrank, so a reap that rewrote the expired lease -- or touched some unrelated file -- fails it instead of passing on "something changed". The assertion is over the file set, deliberately not over named paths: a path literal here would couple this contract to the lease writer's internal layout. Advisory `.lock` siblings are excluded from the comparison. + +| ID | Entry point | Invocation | Fixture | Exit | State delta | Claim | +| --- | --- | --- | --- | --- | --- | --- | +| `lease.acquire-writes-without-apply` | `skills/babysit-prs/scripts/manage_babysit_lease.py` | `acquire --scope worker --pr owner/repo#1 --state-dir {state_dir}` | empty | 0 | files added | THE TRAP: manage_babysit_lease.py's --apply help reads 'reap only', but acquire writes a lease file unconditionally, with no --apply and no network. A permission rule that derives read-only from flag names classifies acquire wrong. | +| `lease.reap-without-apply-is-a-dry-run` | `skills/babysit-prs/scripts/manage_babysit_lease.py` | `reap --state-dir {state_dir}` | expired-worker-lease | 0 | unchanged | --apply gates reap alone: a reap without it reports which lease files are expired and deletes none of them. | +| `lease.reap-with-apply-deletes` | `skills/babysit-prs/scripts/manage_babysit_lease.py` | `reap --state-dir {state_dir} --apply` | expired-worker-lease | 0 | files removed | reap --apply deletes expired worker lease files. This is the one action on this script the --apply flag actually gates. | + +## Mechanisms: how a mutation is performed + +Asserted against the entry point's source, because the mechanism -- not just the fact of mutation -- decides which permission rule is the relevant one. + +| ID | Entry point | Claim | +| --- | --- | --- | +| `refresh.uses-server-side-update-branch` | `skills/babysit-prs/scripts/refresh_pr_branch.py` | refresh_pr_branch.py never pushes. It asks GitHub to update the branch server-side via PUT repos/{repo}/pulls/{number}/update-branch, pinned with expected_head_sha. A consumer reasoning about push guards for this script is reasoning about the wrong mechanism -- it needs no local write access to the branch at all. | +| `review.posts-a-comment` | `skills/babysit-prs/scripts/request_review.py` | request_review.py --apply performs a GitHub write: it POSTs an issue comment carrying the review-trigger phrase. It is a mutating helper despite its read-sounding name. | +| `prune.removes-worktrees-via-git` | `skills/babysit-prs/scripts/prune_babysit_worktrees.py` | prune_babysit_worktrees.py --apply runs `git worktree remove`, a destructive local filesystem operation. Its dry-run default is what makes the flagless form safe, not anything about the name. | +| `snapshot.write-state-gates-the-only-write` | `skills/babysit-prs/scripts/pr_queue_snapshot.py` | pr_queue_snapshot.py's only queue-state write is the local snapshot, and --write-state is the only thing that reaches it. Nothing on any path writes to GitHub, so a flagless invocation reads the queue and persists nothing of it. Housekeeping is out of scope per the entry-point table's mutation definition: this script loads state unconditionally, so load_state may quarantine an already-corrupt state file whatever the flags say. | +| `merge.wrapper-filters-unpinned-head-in-bash` | `bin/source-control-babysit-merge` | The merge wrapper's refusal is a bash argument loop over "$@" that exits before the interpreter shim is sourced -- it is not, and must not become, an argparse flag on the Python side. | +| `merge.parser-refuses-abbreviation` | `skills/babysit-prs/scripts/babysit_merge.py` | babysit_merge.py builds its parser with allow_abbrev=False. Argparse's default would resolve any unambiguous prefix, so a permission rule written against a flag's exact spelling would not cover the prefixes that reach the same flag. | +| `resolve.parser-refuses-abbreviation` | `skills/babysit-prs/scripts/babysit_resolve_thread.py` | babysit_resolve_thread.py sets allow_abbrev=False for the same reason, and it carries more weight here: the resolve wrapper filters nothing, so the parser is the only layer refusing an abbreviated --allow-unpinned-thread. | +| `resolve.wrapper-carries-no-filter` | `bin/source-control-babysit-resolve-thread` | ASYMMETRY, stated at the source: the resolve wrapper contains no argument filter loop. Adding one here without a matching row is the drift this contract exists to catch. | + +## Documented command lines + +Documents that spell out copyable `bin/`-path wrapper commands. Every wrapper path is checked to resolve and every flag against the backing CLI's own parser, so a renamed or removed flag fails here instead of in an operator's terminal. + +Checked against the WRAPPER boundary, not the parser alone. A wrapper's accepted set is the narrower of the two, so a flag the CLI registers but the wrapper refuses -- `bin/source-control-babysit-merge` and `--allow-unpinned-head`, including its prefixes -- is a documented command that always exits 2, and the parser check alone would bless it. Refusals are the source: every flag listed below is one a `bash-wrapper` refusal row proves that wrapper rejects. + +| Wrapper | Refuses | +| --- | --- | +| `bin/source-control-babysit-merge` | `--allow-unpinned-head` (and prefixes) | +| `bin/source-control-babysit-resolve-thread` | nothing | + +| ID | Document | Claim | +| --- | --- | --- | +| `safety.pinned-command-degradation` | `skills/babysit-prs/reference/safety.md` | reference/safety.md hard-codes fully-argument-pinned bin/-path wrapper command lines for the operator to run when the runtime denies a mutation the gate already proved ready. Every wrapper path in it resolves, and every flag it names is one the backing CLI's parser accepts. | +| `orchestration.worker-dispatch-commands` | `skills/babysit-prs/reference/orchestration.md` | reference/orchestration.md spells out the same bin/-path wrapper commands for the dispatched-worker and thread-resolve paths. It is a second copy of the same argument shapes and drifts independently of safety.md. | + +## Not covered here + +- Flag names, types, and defaults are not catalogued. A behavior row exercises the flags it names; a rename or default change elsewhere is not detected. See the deferral note in `scripts/tests/guard_contract.py`. +- Anything requiring a live GitHub response. Every row above runs with no network access, which is what keeps the suite free of a `gh` stub. +- Five rendered columns are annotation rather than assertion, and a rule must not be written against them as if CI proved them: the entry-point table's **Class**, **Mutates**, **Gate**, and **Claim**, and the refusal table's **Enforced at**. The entry-point rows assert the path and the wrapper; nothing reads the prose in those columns. `Predicate.enforced_at` *is* checked -- `test_anchor_symbol_exists` resolves it -- but `Refusal.enforced_at` is not, so a refusal's location may drift without failing CI. +- **Class** carries two partial bindings and one real gap, so read it precisely. Bound: a non-read-only row must name at least one backing row, and a `read-only` row may not be cited by any effect row that moves state or by any mechanism row -- so a read-only entry point that grows mutation evidence fails CI. Not bound: that a `mutating` or `conditionally mutating` row's cited evidence actually demonstrates the class it declares. It cannot be, for the four entry points whose mutation is a GitHub write -- the merge gate, the thread resolver, `refresh_pr_branch.py`, and `request_review.py`. Every row in this contract runs with no network access, by design, so no offline assertion can witness their mutation; binding the column would mean either a `gh` stub (asserting the stub, not the behavior) or a live-credential CI suite. An entry point that turns from conditional to unconditional mutation therefore still passes. Trigger to revisit: the first time a class changes without a matching row change, or the first offline-observable mutation appearing on a GitHub-writing entry point. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 1297b7c027..b45cb044ce 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -375,6 +375,13 @@ the wrapper gate has already proven ready and in-tier. That denial is an environ ceiling this skill's own contract has no authority over — a normal, expected outcome to plan for, not a bug in this skill, a stalled worker, or a reason to retry with broader permissions. +Configuring that host layer means deciding which of this lane's entry points mutate, which flags +gate which guard, and where each refusal is enforced. Those facts are in +[reference/guard-contract.md](guard-contract.md), generated from the table +`scripts/tests/test_guards.py` executes against the real entry points — so a rule written against +a row cannot silently outlive the guard it cites. Cite a row ID; do not restate the behavior in +the consuming configuration. + ### Pinned-Command Degradation When the runtime denies a guarded mutation that this skill's own gate already proved ready — diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py b/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py new file mode 100644 index 0000000000..226dbf2db1 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/guard_contract.py @@ -0,0 +1,1284 @@ +"""The guard contract: every lane fact a consumer is allowed to rely on. + +This module is the single source of truth for two artifacts that must never +disagree: the executable assertions in `test_guards.py` and the citable prose in +`../reference/guard-contract.md`. A consumer configuring a host permission +classifier cites the markdown; CI proves the markdown is what the code does. + +Each row carries the prose claim it backs, so a changed guard predicate fails +with a message naming the downstream documentation claim that just became false +rather than `AssertionError: 3 != 2`. + +Regenerate the markdown after editing any table: + + python tests/guard_contract.py --emit + +Source anchors are `file.py::symbol`, never line numbers -- a line number in a +drift-detection artifact is the drift it exists to catch. + +TODO(#1265): add an argparse flag catalogue -- every flag's name, type, and +default per entry point -- which would additionally catch a renamed flag or a +changed default that no behavior row exercises. Deferred because it needs a +`build_parser()` extraction across all nine entry points, whose parsers are +built inside `main()`. Trigger: the first time a flag rename or default change +ships without a matching update here. +""" + +from __future__ import annotations + +import argparse +import pathlib +import sys +from dataclasses import dataclass, field + +PLUGIN_ROOT = pathlib.Path(__file__).resolve().parents[4] +SCRIPTS = pathlib.Path(__file__).resolve().parent.parent +GENERATED_DOC = SCRIPTS.parent / "reference" / "guard-contract.md" + +MERGE_CLI = "skills/babysit-prs/scripts/babysit_merge.py" +RESOLVE_CLI = "skills/babysit-prs/scripts/babysit_resolve_thread.py" +LEASE_CLI = "skills/babysit-prs/scripts/manage_babysit_lease.py" +LEDGER_CLI = "skills/babysit-prs/scripts/manage_feedback_ledger.py" +PRUNE_CLI = "skills/babysit-prs/scripts/prune_babysit_worktrees.py" +REFRESH_CLI = "skills/babysit-prs/scripts/refresh_pr_branch.py" +REVIEW_CLI = "skills/babysit-prs/scripts/request_review.py" +SNAPSHOT_CLI = "skills/babysit-prs/scripts/pr_queue_snapshot.py" +FINDINGS_CLI = "skills/babysit-prs/scripts/babysit_findings.py" +READINESS_GATE = "scripts/babysit-readiness-gate.sh" +MERGE_WRAPPER = "bin/source-control-babysit-merge" +RESOLVE_WRAPPER = "bin/source-control-babysit-resolve-thread" + +# Where a refusal is enforced. The distinction is observable: a bash-wrapper +# refusal never reaches Python, so it prints plain text to stderr and emits no +# JSON; a python-cli refusal emits the engine's JSON envelope on stdout. +BASH_WRAPPER = "bash-wrapper" +PYTHON_CLI = "python-cli" + + +def plugin_path(relative: str) -> pathlib.Path: + return PLUGIN_ROOT / relative + + +@dataclass(frozen=True) +class Refusal: + """A guard that rejects on argument shape alone, before any network call.""" + + id: str + claim: str + entry_point: str + argv: tuple[str, ...] + exit_code: int + error_contains: tuple[str, ...] + refused_by: str + enforced_at: str + # Envelope fields a caller may branch on, asserted by identity. A consumer + # reading `inScope` needs it present and false, not merely falsy-or-absent. + envelope_fields: tuple[tuple[str, object], ...] = () + # Rows claiming the refusal precedes every network call. These are replayed + # against a recording `gh` shim that is the only executable on PATH, and the + # shim must never be invoked -- an emptied PATH alone would let a swallowed + # `gh` failure fall through to the same exit code and read as proof. + gh_free: bool = False + + +@dataclass(frozen=True) +class Predicate: + """A guard whose condition is a boolean over runtime API data, not flags. + + No argument shape can reach these -- they decide per fetched thread -- so + they are asserted by calling the classifier directly with a projected + thread record. + """ + + id: str + claim: str + enforced_at: str + thread: dict[str, object] + flags: dict[str, bool] + expected: str + + +@dataclass(frozen=True) +class Effect: + """An observable filesystem effect, run offline against a temp state dir. + + These exist because a flag's name does not say whether an invocation + mutates: `manage_babysit_lease.py --apply` reads "reap only", yet `acquire` + writes a lease file with no `--apply` at all. + """ + + id: str + claim: str + entry_point: str + argv: tuple[str, ...] + fixture: str + # WHICH way state moved, not merely that it moved: a boolean lets a reap + # that rewrites the expired lease -- or touches an unrelated file -- pass a + # row whose claim is deletion. See DELTA_KINDS. + delta: str + exit_code: int + + +@dataclass(frozen=True) +class Mechanism: + """A source-level claim about HOW an entry point mutates. + + A consumer reasoning about the wrong mechanism writes the wrong permission + rule -- `refresh_pr_branch.py` never pushes, so a push guard is irrelevant + to it. + """ + + id: str + claim: str + entry_point: str + must_contain: tuple[str, ...] + must_not_contain: tuple[str, ...] = () + + +@dataclass(frozen=True) +class EntryPoint: + """The mutating / read-only classification a permission rule may assume.""" + + path: str + wrapper: str | None + mutation: str + mutates_what: str + gate: str + claim: str + backed_by: tuple[str, ...] = field(default_factory=tuple) + + +@dataclass(frozen=True) +class DocCommandSource: + """A document that spells out wrapper command lines a reader may copy. + + Every `bin/`-path wrapper command fenced in these files is validated: the + wrapper file exists, and every flag it names is a flag the backing CLI's + parser actually accepts. + """ + + id: str + claim: str + doc: str + + +# -------------------------------------------------------------------------- +# Refusals -- fail-closed guards, asserted by invoking the entry point. +# -------------------------------------------------------------------------- + +REFUSALS: tuple[Refusal, ...] = ( + Refusal( + id="merge.allowlist-absent", + claim=( + "The merge gate is fail-closed on scope: with no --allowed-owners it " + "refuses every PR at exit 3 rather than defaulting to the current repo's owner." + ), + entry_point=MERGE_CLI, + argv=("owner/repo#1",), + exit_code=3, + error_contains=("allowed-owners",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + envelope_fields=(("inScope", False),), + gh_free=True, + ), + Refusal( + id="merge.owner-out-of-scope", + claim=( + "An owner outside --allowed-owners is refused at exit 3, so a permission rule " + "may assume the gate never acts on a repository the operator did not name." + ), + entry_point=MERGE_CLI, + argv=("owner/repo#1", "--allowed-owners", "someone-else"), + exit_code=3, + error_contains=(), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + envelope_fields=(("inScope", False),), + gh_free=True, + ), + Refusal( + id="merge.owner-check-precedes-self-login-resolution", + claim=( + "The owner check runs before '@me' self-login resolution, which is a network " + "call: an out-of-scope owner refuses at exit 3 with no gh invocation at all." + ), + entry_point=MERGE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "someone-else", + "--self-logins", + "@me", + ), + exit_code=3, + error_contains=(), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + envelope_fields=(("inScope", False),), + gh_free=True, + ), + Refusal( + id="merge.short-expected-head", + claim=( + "--expected-head must be a long-enough SHA prefix; a short value is a usage " + "error at exit 2, never a loose prefix match against the live head." + ), + entry_point=MERGE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--merge", + "--expected-head", + "abc", + ), + exit_code=2, + error_contains=("expected-head",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + ), + Refusal( + id="merge.malformed-ref", + claim=( + "The PR reference must parse as owner/repo#number or a PR URL; anything else " + "is a usage error at exit 2." + ), + entry_point=MERGE_CLI, + argv=("not-a-ref", "--allowed-owners", "owner"), + exit_code=2, + error_contains=(), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + ), + Refusal( + id="merge.autopilot-tier-without-required-sets", + claim=( + "The autopilot merge tier is fail-closed: the umbrella flag alone, with none " + "of --lane-logins, --approver-bot-logins, or --block-labels, refuses at exit 3 " + "before any network access." + ), + entry_point=MERGE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--autopilot-merge-tier", + ), + exit_code=3, + error_contains=("--lane-logins", "--approver-bot-logins", "--block-labels"), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + gh_free=True, + ), + Refusal( + id="merge.autopilot-tier-partial-config", + claim=( + "A partially configured autopilot tier still refuses at exit 3, naming only " + "the missing set -- there is no partial-credit tier." + ), + entry_point=MERGE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--autopilot-merge-tier", + "--lane-logins", + "lane", + "--approver-bot-logins", + "bot", + ), + exit_code=3, + error_contains=("--block-labels",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + gh_free=True, + ), + Refusal( + id="merge.tier-params-without-umbrella", + claim=( + "Tier parameter sets without --autopilot-merge-tier are a usage error at " + "exit 2, never a silent no-op that reads as configured." + ), + entry_point=MERGE_CLI, + argv=("owner/repo#1", "--allowed-owners", "owner", "--lane-logins", "lane"), + exit_code=2, + error_contains=("--autopilot-merge-tier",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + ), + Refusal( + id="merge.unpinned-head-refused-by-wrapper", + claim=( + "The bin/ wrapper refuses --allow-unpinned-head in bash, before Python runs: " + "no allow-rule-covered invocation of the bare command can merge an unvetted " + "head. The refusal prints plain text to stderr and emits no JSON envelope, " + "which is how a caller can tell the wrapper -- not the CLI -- rejected it." + ), + entry_point=MERGE_WRAPPER, + argv=("owner/repo#1", "--merge", "--allow-unpinned-head"), + exit_code=2, + error_contains=("--allow-unpinned-head",), + refused_by=BASH_WRAPPER, + enforced_at="bin/source-control-babysit-merge (argument filter loop)", + ), + Refusal( + id="merge.abbreviated-unpinned-head-refused-by-wrapper", + claim=( + "The wrapper refusal covers every long-option PREFIX of " + "--allow-unpinned-head, not the exact spelling alone. An " + "equality-only filter would let `--allow-unpinned-hea` through to an " + "abbreviation-resolving parser and reinstate the override the wrapper " + "exists to remove, so the prefix family is refused as one." + ), + entry_point=MERGE_WRAPPER, + argv=("owner/repo#1", "--merge", "--allow-unpinned-hea"), + exit_code=2, + error_contains=("--allow-unpinned-hea",), + refused_by=BASH_WRAPPER, + enforced_at="bin/source-control-babysit-merge (argument filter loop)", + ), + Refusal( + id="merge.abbreviation-is-not-resolved-by-the-cli", + claim=( + "The wrapper's prefix filter is belt to the CLI's braces: the parser " + "sets allow_abbrev=False, so an abbreviated flag reaching Python " + "directly is an unrecognized argument at exit 2 rather than a silently " + "resolved override." + ), + entry_point=MERGE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--merge", + "--allow-unpinned-hea", + ), + exit_code=2, + error_contains=("--allow-unpinned-hea",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + ), + Refusal( + id="merge.wrapper-reaches-failclosed-cli", + claim=( + "The wrapper adds no capability of its own: with no --allowed-owners it hands " + "off to the CLI, which refuses at exit 3 exactly as a direct invocation does." + ), + entry_point=MERGE_WRAPPER, + argv=("owner/repo#1",), + exit_code=3, + error_contains=("allowed-owners",), + refused_by=PYTHON_CLI, + enforced_at="babysit_merge.py::main", + ), + Refusal( + id="resolve.allowlist-absent", + claim=( + "The thread resolver is fail-closed on scope the same way the merge gate is: " + "no --allowed-owners means exit 3." + ), + entry_point=RESOLVE_CLI, + argv=("owner/repo#1",), + exit_code=3, + error_contains=(), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + envelope_fields=(("inScope", False),), + gh_free=True, + ), + Refusal( + id="resolve.count-pin-without-thread-id", + claim=( + "--expected-comment-count without --thread-id is a usage error at exit 2: a " + "pin with no thread to pin would silently fall through to resolving every " + "eligible thread." + ), + entry_point=RESOLVE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--expected-comment-count", + "3", + ), + exit_code=2, + error_contains=("thread-id",), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.last-updated-pin-without-thread-id", + claim=( + "--expected-last-updated without --thread-id is a usage error at exit 2, for " + "the same silent-bulk-fallthrough reason as the count pin." + ), + entry_point=RESOLVE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--expected-last-updated", + "2026-07-17T10:00:00Z", + ), + exit_code=2, + error_contains=("thread-id",), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.thread-id-without-pins", + claim=( + "A --thread-id resolve requires BOTH TOCTOU pins; supplying neither is a " + "usage error at exit 2 naming both missing flags." + ), + entry_point=RESOLVE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--resolve", + "--thread-id", + "PRRT_abc", + ), + exit_code=2, + error_contains=("--expected-comment-count", "--expected-last-updated"), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.autonomous-bulk-refused", + claim=( + "--autonomous --resolve without --thread-id is refused at exit 2: --autonomous " + "means SINGLE PINNED THREAD, never bulk. A worker's own push marks a thread " + "isOutdated, so a bulk autonomous resolve would clear threads with no proof " + "the finding was addressed." + ), + entry_point=RESOLVE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--autonomous", + "--resolve", + ), + exit_code=2, + error_contains=("thread-id", "bulk-resolve"), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.autonomous-allow-unpinned-refused", + claim=( + "There is no unpinned autonomous resolve: --allow-unpinned-thread is an " + "interactive-only override and is refused at exit 2 under --autonomous even " + "with a single --thread-id." + ), + entry_point=RESOLVE_CLI, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--autonomous", + "--resolve", + "--thread-id", + "PRRT_abc", + "--allow-unpinned-thread", + ), + exit_code=2, + error_contains=("allow-unpinned-thread",), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.wrapper-reaches-failclosed-cli", + claim=( + "The resolve wrapper is a pure passthrough: no --allowed-owners refuses at " + "exit 3 from the CLI, unchanged by the wrapper." + ), + entry_point=RESOLVE_WRAPPER, + argv=("owner/repo#1",), + exit_code=3, + error_contains=(), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="resolve.wrapper-filters-nothing", + claim=( + "ASYMMETRY: unlike the merge wrapper, the resolve wrapper filters no argument. " + "Its --allow-unpinned-thread refusal comes from Python -- proven by the JSON " + "envelope on stdout -- so an operator may not assume a bash-level refusal " + "exists on this entry point just because one exists on the other." + ), + entry_point=RESOLVE_WRAPPER, + argv=( + "owner/repo#1", + "--allowed-owners", + "owner", + "--autonomous", + "--resolve", + "--thread-id", + "PRRT_abc", + "--allow-unpinned-thread", + ), + exit_code=2, + error_contains=("allow-unpinned-thread",), + refused_by=PYTHON_CLI, + enforced_at="babysit_resolve_thread.py::main", + ), + Refusal( + id="ledger.apply-requires-lease-token", + claim=( + "A ledger write is lease-gated: --apply without --lease-token refuses at " + "exit 2 rather than writing an unowned entry." + ), + entry_point=LEDGER_CLI, + argv=( + "dispose", + "--pr", + "owner/repo#1", + "--expected-head-sha", + "0123456789abcdef0123456789abcdef01234567", + "--feedback-id", + "F1", + "--reason", + "contract-row", + "--state-dir", + "{state_dir}", + "--apply", + ), + exit_code=2, + error_contains=("lease token",), + refused_by=PYTHON_CLI, + enforced_at="manage_feedback_ledger.py::main", + ), + Refusal( + id="prune.root-is-required", + claim=( + "prune_babysit_worktrees.py has no default root: omitting --root is a usage " + "error at exit 2, so it can never sweep an unintended directory." + ), + entry_point=PRUNE_CLI, + argv=("--state-dir", "{state_dir}"), + exit_code=2, + error_contains=("--root",), + refused_by=PYTHON_CLI, + enforced_at="prune_babysit_worktrees.py::main", + ), + Refusal( + id="lease.state-dir-is-required", + claim=( + "State-dir resolution is flag-only with no environment fallback: omitting " + "--state-dir is a usage error at exit 2." + ), + entry_point=LEASE_CLI, + argv=("reap",), + exit_code=2, + error_contains=("--state-dir",), + refused_by=PYTHON_CLI, + enforced_at="manage_babysit_lease.py::main", + ), + Refusal( + id="lease.scope-required-for-non-reap", + claim=( + "--scope is required for acquire, heartbeat, and release; only reap runs " + "without it." + ), + entry_point=LEASE_CLI, + argv=("acquire", "--state-dir", "{state_dir}"), + exit_code=2, + error_contains=("--scope",), + refused_by=PYTHON_CLI, + enforced_at="manage_babysit_lease.py::main", + ), +) + + +# -------------------------------------------------------------------------- +# Predicates -- guards over runtime API data, asserted against the classifier. +# -------------------------------------------------------------------------- + +CLASSIFY_ANCHOR = "babysit_resolve_thread.py::classify" + + +def _thread(*, resolved: bool, bot_only: bool, outdated: bool) -> dict[str, object]: + return {"isResolved": resolved, "botOnly": bot_only, "isOutdated": outdated} + + +PREDICATES: tuple[Predicate, ...] = ( + Predicate( + id="classify.human-thread-refused-by-default", + claim=( + "Bot-only is a bright line, not a heuristic: a thread with ANY human " + "participant is skipped whenever --include-human is absent, regardless of " + "every other flag." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=False, outdated=True), + flags={"autonomous": False, "only_outdated": False, "include_human": False}, + expected="skipped-human-thread", + ), + Predicate( + id="classify.include-human-opts-in", + claim=( + "--include-human is the ONLY thing that lifts the bot-only line. A permission " + "rule asserting 'bot threads only' is false for any invocation carrying it." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=False, outdated=True), + flags={"autonomous": False, "only_outdated": False, "include_human": True}, + expected="eligible", + ), + Predicate( + id="classify.autonomous-requires-outdated", + claim=( + "--autonomous requires the deterministic isOutdated 'addressed' signal: a " + "still-current bot thread is skipped so an unattended worker cannot resolve " + "a live finding and self-satisfy the merge gate." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=True, outdated=False), + flags={"autonomous": True, "only_outdated": False, "include_human": False}, + expected="skipped-not-outdated", + ), + Predicate( + id="classify.only-outdated-requires-outdated", + claim=( + "--only-outdated imposes the same isOutdated requirement as --autonomous, " + "independently of it." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=True, outdated=False), + flags={"autonomous": False, "only_outdated": True, "include_human": False}, + expected="skipped-not-outdated", + ), + Predicate( + id="classify.interactive-does-not-require-outdated", + claim=( + "THE CLAIM CONSUMERS GET WRONG: 'resolves only outdated bot threads' is TRUE " + "ONLY under --autonomous or --only-outdated. With neither flag, a current " + "(not outdated) bot thread is eligible." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=True, outdated=False), + flags={"autonomous": False, "only_outdated": False, "include_human": False}, + expected="eligible", + ), + Predicate( + id="classify.include-human-does-not-lift-outdated", + claim=( + "--include-human lifts only the bot-only line. Under --autonomous the " + "isOutdated requirement still applies, so the two guards are independent and " + "neither flag opens the other." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=False, bot_only=False, outdated=False), + flags={"autonomous": True, "only_outdated": False, "include_human": True}, + expected="skipped-not-outdated", + ), + Predicate( + id="classify.already-resolved-precedes-every-guard", + claim=( + "An already-resolved thread is skipped before any other guard is consulted, " + "so a re-run never re-resolves and never reports a human thread it declined." + ), + enforced_at=CLASSIFY_ANCHOR, + thread=_thread(resolved=True, bot_only=False, outdated=False), + flags={"autonomous": True, "only_outdated": False, "include_human": False}, + expected="skipped-already-resolved", + ), +) + + +# -------------------------------------------------------------------------- +# Effects -- what actually touches disk, run offline against a temp state dir. +# -------------------------------------------------------------------------- + +EMPTY_STATE = "empty" +EXPIRED_WORKER_LEASE = "expired-worker-lease" + +# Observed movement of the state directory's file set, asserted over the +# fingerprint's keys rather than named paths -- a path literal here would couple +# the contract to the lease writer's internal layout, which is exactly the drift +# this module refuses to encode. +UNCHANGED = "unchanged" +ADDED = "files added" +REMOVED = "files removed" +REWRITTEN = "same files, new contents" +DELTA_KINDS = (UNCHANGED, ADDED, REMOVED, REWRITTEN) + +EFFECTS: tuple[Effect, ...] = ( + Effect( + id="lease.acquire-writes-without-apply", + claim=( + "THE TRAP: manage_babysit_lease.py's --apply help reads 'reap only', but " + "acquire writes a lease file unconditionally, with no --apply and no network. " + "A permission rule that derives read-only from flag names classifies acquire " + "wrong." + ), + entry_point=LEASE_CLI, + argv=( + "acquire", + "--scope", + "worker", + "--pr", + "owner/repo#1", + "--state-dir", + "{state_dir}", + ), + fixture=EMPTY_STATE, + delta=ADDED, + exit_code=0, + ), + Effect( + id="lease.reap-without-apply-is-a-dry-run", + claim=( + "--apply gates reap alone: a reap without it reports which lease files are " + "expired and deletes none of them." + ), + entry_point=LEASE_CLI, + argv=("reap", "--state-dir", "{state_dir}"), + fixture=EXPIRED_WORKER_LEASE, + delta=UNCHANGED, + exit_code=0, + ), + Effect( + id="lease.reap-with-apply-deletes", + claim=( + "reap --apply deletes expired worker lease files. This is the one action on " + "this script the --apply flag actually gates." + ), + entry_point=LEASE_CLI, + argv=("reap", "--state-dir", "{state_dir}", "--apply"), + fixture=EXPIRED_WORKER_LEASE, + delta=REMOVED, + exit_code=0, + ), +) + + +# -------------------------------------------------------------------------- +# Mechanisms -- HOW an entry point mutates, asserted against its source. +# -------------------------------------------------------------------------- + +MECHANISMS: tuple[Mechanism, ...] = ( + Mechanism( + id="refresh.uses-server-side-update-branch", + claim=( + "refresh_pr_branch.py never pushes. It asks GitHub to update the branch " + "server-side via PUT repos/{repo}/pulls/{number}/update-branch, pinned with " + "expected_head_sha. A consumer reasoning about push guards for this script is " + "reasoning about the wrong mechanism -- it needs no local write access to the " + "branch at all." + ), + entry_point=REFRESH_CLI, + must_contain=('pulls/{number}/update-branch', 'expected_head_sha='), + must_not_contain=('"push"', "'push'"), + ), + Mechanism( + id="review.posts-a-comment", + claim=( + "request_review.py --apply performs a GitHub write: it POSTs an issue comment " + "carrying the review-trigger phrase. It is a mutating helper despite its " + "read-sounding name." + ), + entry_point=REVIEW_CLI, + must_contain=('"--method"', '"POST"'), + ), + Mechanism( + id="prune.removes-worktrees-via-git", + claim=( + "prune_babysit_worktrees.py --apply runs `git worktree remove`, a destructive " + "local filesystem operation. Its dry-run default is what makes the flagless " + "form safe, not anything about the name." + ), + entry_point=PRUNE_CLI, + must_contain=('"worktree", "remove"',), + ), + Mechanism( + id="snapshot.write-state-gates-the-only-write", + claim=( + "pr_queue_snapshot.py's only queue-state write is the local snapshot, and " + "--write-state is the only thing that reaches it. Nothing on any path writes " + "to GitHub, so a flagless invocation reads the queue and persists nothing of " + "it. Housekeeping is out of scope per the entry-point table's mutation " + "definition: this script loads state unconditionally, so load_state may " + "quarantine an already-corrupt state file whatever the flags say." + ), + entry_point=SNAPSHOT_CLI, + must_contain=("if args.write_state:", "save_state("), + ), + Mechanism( + id="merge.wrapper-filters-unpinned-head-in-bash", + claim=( + "The merge wrapper's refusal is a bash argument loop over \"$@\" that exits " + "before the interpreter shim is sourced -- it is not, and must not become, " + "an argparse flag on the Python side." + ), + entry_point=MERGE_WRAPPER, + must_contain=('for arg in "$@"', "--allow-unpinned-head"), + ), + Mechanism( + id="merge.parser-refuses-abbreviation", + claim=( + "babysit_merge.py builds its parser with allow_abbrev=False. Argparse's " + "default would resolve any unambiguous prefix, so a permission rule " + "written against a flag's exact spelling would not cover the prefixes " + "that reach the same flag." + ), + entry_point=MERGE_CLI, + must_contain=("allow_abbrev=False",), + ), + Mechanism( + id="resolve.parser-refuses-abbreviation", + claim=( + "babysit_resolve_thread.py sets allow_abbrev=False for the same reason, " + "and it carries more weight here: the resolve wrapper filters nothing, " + "so the parser is the only layer refusing an abbreviated " + "--allow-unpinned-thread." + ), + entry_point=RESOLVE_CLI, + must_contain=("allow_abbrev=False",), + ), + Mechanism( + id="resolve.wrapper-carries-no-filter", + claim=( + "ASYMMETRY, stated at the source: the resolve wrapper contains no argument " + "filter loop. Adding one here without a matching row is the drift this " + "contract exists to catch." + ), + entry_point=RESOLVE_WRAPPER, + must_contain=("babysit_python",), + must_not_contain=('for arg in "$@"',), + ), +) + + +# -------------------------------------------------------------------------- +# Entry points -- the mutating / read-only catalogue a permission rule cites. +# -------------------------------------------------------------------------- + +READ_ONLY = "read-only" +MUTATING = "mutating" +CONDITIONAL = "conditionally mutating" + +ENTRY_POINTS: tuple[EntryPoint, ...] = ( + EntryPoint( + path=MERGE_CLI, + wrapper=MERGE_WRAPPER, + mutation=CONDITIONAL, + mutates_what="merges the PR on GitHub", + gate="--merge (absent: readiness check only, exit 0 ready / 10 not ready)", + claim=( + "Without --merge this is a readiness reporter. With it, the TOCTOU guard " + "requires --expected-head unless --allow-unpinned-head is passed -- and the " + "bin/ wrapper refuses that override outright." + ), + backed_by=( + "merge.allowlist-absent", + "merge.unpinned-head-refused-by-wrapper", + "merge.abbreviated-unpinned-head-refused-by-wrapper", + "merge.abbreviation-is-not-resolved-by-the-cli", + "merge.wrapper-filters-unpinned-head-in-bash", + "merge.parser-refuses-abbreviation", + ), + ), + EntryPoint( + path=RESOLVE_CLI, + wrapper=RESOLVE_WRAPPER, + mutation=CONDITIONAL, + mutates_what="marks review threads resolved on GitHub", + gate="--resolve (absent: classification report only)", + claim=( + "Which threads it may touch is decided per fetched thread by the classifier, " + "not by the argument shape -- see the predicate rows. The wrapper adds no " + "refusal of its own." + ), + backed_by=( + "resolve.allowlist-absent", + "resolve.autonomous-bulk-refused", + "classify.interactive-does-not-require-outdated", + "resolve.wrapper-carries-no-filter", + "resolve.parser-refuses-abbreviation", + ), + ), + EntryPoint( + path=LEASE_CLI, + wrapper=None, + mutation=MUTATING, + mutates_what="writes, renews, and deletes lease files under --state-dir", + gate=( + "none for acquire / heartbeat / release -- they write unconditionally; " + "--apply gates reap's deletion ONLY" + ), + claim=( + "Local filesystem writes only, no GitHub write. The --apply flag does not " + "make this script read-only in its other three actions." + ), + backed_by=( + "lease.acquire-writes-without-apply", + "lease.reap-without-apply-is-a-dry-run", + ), + ), + EntryPoint( + path=LEDGER_CLI, + wrapper=None, + mutation=CONDITIONAL, + mutates_what="records feedback dispositions and fix rounds in queue state", + gate="--apply, additionally requiring --lease-token", + claim=( + "Local state only, no GitHub write. The lease token requirement means an " + "--apply invocation still refuses without proof of lease ownership." + ), + backed_by=("ledger.apply-requires-lease-token",), + ), + EntryPoint( + path=PRUNE_CLI, + wrapper=None, + mutation=CONDITIONAL, + mutates_what="runs `git worktree remove` on eligible worktrees under --root", + gate="--apply (absent: dry run)", + claim=( + "The most destructive local helper in the lane. --root is required, so it " + "cannot sweep a directory the caller did not name." + ), + backed_by=("prune.root-is-required", "prune.removes-worktrees-via-git"), + ), + EntryPoint( + path=REFRESH_CLI, + wrapper=None, + mutation=CONDITIONAL, + mutates_what="asks GitHub to update the PR branch server-side; writes queue state", + gate="--apply", + claim=( + "Mutates the PR branch on GitHub WITHOUT pushing: the update is a pinned " + "server-side update-branch API call." + ), + backed_by=("refresh.uses-server-side-update-branch",), + ), + EntryPoint( + path=REVIEW_CLI, + wrapper=None, + mutation=CONDITIONAL, + mutates_what="POSTs an issue comment requesting an AI re-review; writes queue state", + gate="--apply", + claim="A GitHub write despite the read-sounding name.", + backed_by=("review.posts-a-comment",), + ), + EntryPoint( + path=SNAPSHOT_CLI, + wrapper=None, + mutation=CONDITIONAL, + mutates_what="writes the queue snapshot to local state", + gate="--write-state (absent: reports the snapshot without persisting it)", + claim=( + "No GitHub write on any path. The only domain-state mutation is the local " + "snapshot file." + ), + backed_by=("snapshot.write-state-gates-the-only-write",), + ), + EntryPoint( + path=FINDINGS_CLI, + wrapper=None, + mutation=READ_ONLY, + mutates_what="nothing", + gate="n/a", + claim="Pure classification over supplied input; no write of any kind.", + backed_by=(), + ), + EntryPoint( + path=READINESS_GATE, + wrapper=None, + mutation=READ_ONLY, + mutates_what="nothing", + gate="n/a", + claim=( + "The lane's one entry point outside the skill's own scripts directory, " + "invoked by name from SKILL.md. It counts findings and classification " + "rows over fetched comments and reports READINESS_OK / " + "READINESS_BLOCKED; it writes no file and performs no GitHub write." + ), + backed_by=(), + ), +) + + +# -------------------------------------------------------------------------- +# Documents that spell out copyable wrapper command lines. +# -------------------------------------------------------------------------- + +DOC_COMMAND_SOURCES: tuple[DocCommandSource, ...] = ( + DocCommandSource( + id="safety.pinned-command-degradation", + claim=( + "reference/safety.md hard-codes fully-argument-pinned bin/-path wrapper " + "command lines for the operator to run when the runtime denies a mutation the " + "gate already proved ready. Every wrapper path in it resolves, and every flag " + "it names is one the backing CLI's parser accepts." + ), + doc="skills/babysit-prs/reference/safety.md", + ), + DocCommandSource( + id="orchestration.worker-dispatch-commands", + claim=( + "reference/orchestration.md spells out the same bin/-path wrapper commands " + "for the dispatched-worker and thread-resolve paths. It is a second copy of " + "the same argument shapes and drifts independently of safety.md." + ), + doc="skills/babysit-prs/reference/orchestration.md", + ), +) + + +# -------------------------------------------------------------------------- +# Markdown emission. +# -------------------------------------------------------------------------- + +WRAPPER_BACKING_CLI = { + MERGE_WRAPPER: MERGE_CLI, + RESOLVE_WRAPPER: RESOLVE_CLI, +} + +# Flags the backing CLI accepts but the wrapper refuses, so a documented wrapper +# command naming one is a command that always exits 2. The wrapper boundary is +# narrower than the parser, and validating documented commands against the +# parser alone would bless exactly the invocation the wrapper exists to prevent. +# Matched as prefix families, mirroring the wrapper's own bash test. +WRAPPER_DENIED_FLAGS: dict[str, tuple[str, ...]] = { + MERGE_WRAPPER: ("--allow-unpinned-head",), + RESOLVE_WRAPPER: (), +} + + +def wrapper_denies(wrapper: str, flag: str) -> bool: + """Whether `flag`, as spelled, cannot survive `wrapper`'s argument filter.""" + return any(denied.startswith(flag) for denied in WRAPPER_DENIED_FLAGS[wrapper]) + +_PREAMBLE = """# Guard contract + + + +What a host permission classifier, an orchestration prompt, or a downstream +consumer may assume about this lane's entry points. Rows in the refusal, +predicate, effect, and documented-command tables are executed as assertions by +`scripts/tests/test_guards.py`; a guard change that falsifies one fails CI with +a message naming the claim. The columns listed under "Not covered here" are +rendered from the same data but are not asserted — read those as annotation, not +as proof. + +Cite a row by its ID. IDs are stable; rows are removed only when the behavior is. + +Source anchors are `file::symbol`, deliberately not line numbers. +""" + + +def _cell(text: str) -> str: + return " ".join(text.split()).replace("|", "\\|") + + +def _argv(argv: tuple[str, ...]) -> str: + return "`" + " ".join(argv) + "`" if argv else "(none)" + + +def render_markdown() -> str: + lines = [_PREAMBLE.rstrip(), ""] + + lines += [ + "## Entry points: what mutates", + "", + "This table scopes `mutation` to DOMAIN state -- GitHub, the queue state file," + " worktrees, and leases -- and `gate` is the condition under which that domain" + " mutation happens at all. It is deliberately not a filesystem-write audit: a" + " state-touching script performs housekeeping under `--state-dir` whatever its" + " flags say, so entering `state_lock` creates the state directory and a `.lock`" + " sibling, and `load_state` quarantines an already-corrupt state file by renaming" + " it. A classifier granting a gate-less invocation therefore still needs the" + " script to be able to write inside `--state-dir`; what the gate withholds is the" + " domain mutation, not every byte. A blank wrapper column means the entry point" + " has no `bin/` wrapper and is invoked through the interpreter.", + "", + "| Entry point | Wrapper | Class | Mutates | Gate | Claim | Backed by |", + "| --- | --- | --- | --- | --- | --- | --- |", + ] + for entry in ENTRY_POINTS: + wrapper = f"`{entry.wrapper}`" if entry.wrapper else "--" + backed = ", ".join(f"`{row}`" for row in entry.backed_by) or "--" + lines.append( + f"| `{entry.path}` | {wrapper} | {entry.mutation} | {_cell(entry.mutates_what)} " + f"| {_cell(entry.gate)} | {_cell(entry.claim)} | {backed} |" + ) + + lines += [ + "", + "## Refusals: guards that fire on argument shape alone", + "", + "Each row is executed. `Refused by` says which layer rejected the invocation." + " On a `bin/` entry point the distinction is asserted, using the observable" + " discriminator: a bash-wrapper refusal never reaches the interpreter, so it" + " emits plain stderr and no JSON envelope. On a CLI entry point the layer is" + " always `python-cli` and only the exit code and message are asserted.", + "", + "`No gh` marks the rows whose claim is that the refusal precedes every network" + " call. Those are replayed a second time with a recording `gh` shim as the only" + " executable on `PATH`, and the assertion is that the shim was never invoked --" + " merely removing `gh` from `PATH` would let a swallowed lookup failure fall" + " through to the same exit code and read as proof.", + "", + "| ID | Entry point | Invocation | Exit | Refused by | No gh | Error names" + " | Enforced at | Claim |", + "| --- | --- | --- | --- | --- | --- | --- | --- | --- |", + ] + for row in REFUSALS: + names = ", ".join(f"`{token}`" for token in row.error_contains) or "--" + lines.append( + f"| `{row.id}` | `{row.entry_point}` | {_argv(row.argv)} | {row.exit_code} " + f"| {row.refused_by} | {'asserted' if row.gh_free else '--'} | {names} " + f"| `{row.enforced_at}` | {_cell(row.claim)} |" + ) + + lines += [ + "", + "## Predicates: guards over runtime data, not flags", + "", + "These decide per fetched thread, so no argument shape can express them and no" + " argparse introspection can recover them. `thread` is the projected thread" + " record; `flags` are the classifier's keyword arguments.", + "", + "| ID | Thread | Flags | Result | Enforced at | Claim |", + "| --- | --- | --- | --- | --- | --- |", + ] + for row in PREDICATES: + thread = ", ".join(f"{k}={v}" for k, v in row.thread.items()) + flags = ", ".join(f"{k}={v}" for k, v in row.flags.items()) + lines.append( + f"| `{row.id}` | {thread} | {flags} | `{row.expected}` " + f"| `{row.enforced_at}` | {_cell(row.claim)} |" + ) + + lines += [ + "", + "## Effects: what reaches disk", + "", + "Executed offline against a throwaway state directory seeded with `Fixture`." + " `State delta` is the observed before/after movement of that directory's file" + " set, not a reading of the flag names, and it is directional rather than a" + " changed/unchanged boolean: a row claiming deletion asserts that the file set" + " strictly shrank, so a reap that rewrote the expired lease -- or touched some" + " unrelated file -- fails it instead of passing on \"something changed\". The" + " assertion is over the file set, deliberately not over named paths: a path" + " literal here would couple this contract to the lease writer's internal" + " layout. Advisory `.lock` siblings are excluded from the comparison.", + "", + "| ID | Entry point | Invocation | Fixture | Exit | State delta | Claim |", + "| --- | --- | --- | --- | --- | --- | --- |", + ] + for row in EFFECTS: + lines.append( + f"| `{row.id}` | `{row.entry_point}` | {_argv(row.argv)} | {row.fixture} " + f"| {row.exit_code} | {row.delta} | {_cell(row.claim)} |" + ) + + lines += [ + "", + "## Mechanisms: how a mutation is performed", + "", + "Asserted against the entry point's source, because the mechanism -- not just" + " the fact of mutation -- decides which permission rule is the relevant one.", + "", + "| ID | Entry point | Claim |", + "| --- | --- | --- |", + ] + for row in MECHANISMS: + lines.append(f"| `{row.id}` | `{row.entry_point}` | {_cell(row.claim)} |") + + lines += [ + "", + "## Documented command lines", + "", + "Documents that spell out copyable `bin/`-path wrapper commands. Every wrapper" + " path is checked to resolve and every flag against the backing CLI's own" + " parser, so a renamed or removed flag fails here instead of in an operator's" + " terminal.", + "", + "Checked against the WRAPPER boundary, not the parser alone. A wrapper's" + " accepted set is the narrower of the two, so a flag the CLI registers but the" + " wrapper refuses -- `bin/source-control-babysit-merge` and" + " `--allow-unpinned-head`, including its prefixes -- is a documented command" + " that always exits 2, and the parser check alone would bless it. Refusals are" + " the source: every flag listed below is one a `bash-wrapper` refusal row" + " proves that wrapper rejects.", + "", + "| Wrapper | Refuses |", + "| --- | --- |", + ] + for wrapper, denied in WRAPPER_DENIED_FLAGS.items(): + names = ", ".join(f"`{flag}` (and prefixes)" for flag in denied) or "nothing" + lines.append(f"| `{wrapper}` | {names} |") + + lines += [ + "", + "| ID | Document | Claim |", + "| --- | --- | --- |", + ] + for row in DOC_COMMAND_SOURCES: + lines.append(f"| `{row.id}` | `{row.doc}` | {_cell(row.claim)} |") + + lines += [ + "", + "## Not covered here", + "", + "- Flag names, types, and defaults are not catalogued. A behavior row exercises" + " the flags it names; a rename or default change elsewhere is not detected. See" + " the deferral note in `scripts/tests/guard_contract.py`.", + "- Anything requiring a live GitHub response. Every row above runs with no" + " network access, which is what keeps the suite free of a `gh` stub.", + "- Five rendered columns are annotation rather than assertion, and a rule" + " must not be written against them as if CI proved them: the entry-point" + " table's **Class**, **Mutates**, **Gate**, and **Claim**, and the refusal" + " table's **Enforced at**. The entry-point rows assert the path and the" + " wrapper; nothing reads the prose in those columns." + " `Predicate.enforced_at` *is* checked -- `test_anchor_symbol_exists`" + " resolves it -- but `Refusal.enforced_at` is not, so a refusal's location" + " may drift without failing CI.", + "- **Class** carries two partial bindings and one real gap, so read it" + " precisely. Bound: a non-read-only row must name at least one backing" + " row, and a `read-only` row may not be cited by any effect row that moves" + " state or by any mechanism row -- so a read-only entry point that grows" + " mutation evidence fails CI. Not bound: that a `mutating` or" + " `conditionally mutating` row's cited evidence actually demonstrates the" + " class it declares. It cannot be, for the four entry points whose" + " mutation is a GitHub write -- the merge gate, the thread resolver," + " `refresh_pr_branch.py`, and `request_review.py`. Every row in this" + " contract runs with no network access, by design, so no offline assertion" + " can witness their mutation; binding the column would mean either a `gh`" + " stub (asserting the stub, not the behavior) or a live-credential CI" + " suite. An entry point that turns from conditional to unconditional" + " mutation therefore still passes. Trigger to revisit: the first time a" + " class changes without a matching row change, or the first offline-" + "observable mutation appearing on a GitHub-writing entry point.", + "", + ] + return "\n".join(lines) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--emit", + action="store_true", + help="write the generated markdown to reference/guard-contract.md", + ) + args = parser.parse_args() + rendered = render_markdown() + if args.emit: + GENERATED_DOC.write_text(rendered, encoding="utf-8", newline="\n") + print(f"wrote {GENERATED_DOC}") + else: + sys.stdout.write(rendered) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_guards.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_guards.py index a4dd5062cf..5cf0d030dd 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_guards.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_guards.py @@ -1,161 +1,605 @@ -"""Security-boundary tests for the two guarded mutation CLIs. +"""Executes the guard contract in `guard_contract.py` against the real entry points. -Every assertion here exercises a refusal that must fire BEFORE any network call, -so the tests need no gh stub: a fail-closed guard rejects on argument shape -alone. These encode the trust contract -- an allowlist that is absent fails -closed, an ambiguous head pin is refused, a thread resolve without its TOCTOU -pins is refused -- that nothing else in the suite covers. +Every assertion here exercises a fact a consumer is told to rely on, and fails +with the prose claim that just became false rather than a bare exit-code +mismatch. The refusal rows need no gh stub: a fail-closed guard rejects on +argument shape alone, before any network call. The effect rows run offline +against a throwaway state directory, and the predicate rows call the classifier +directly because their conditions are boolean expressions over fetched API data +that no argument shape can express. + +The contract's markdown rendering in `../reference/guard-contract.md` is what +consumers cite; `GeneratedDocIsCurrent` is what keeps it honest. """ import json +import os import pathlib +import re +import shutil import subprocess import sys +import tempfile import unittest +from datetime import UTC, datetime, timedelta + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) -SCRIPTS = pathlib.Path(__file__).resolve().parent.parent -MERGE = SCRIPTS / "babysit_merge.py" -RESOLVE = SCRIPTS / "babysit_resolve_thread.py" +import babysit_resolve_thread # noqa: E402 +import guard_contract as contract # noqa: E402 +BASH = shutil.which("bash") -def run(script, *args): - proc = subprocess.run( - [sys.executable, str(script), *args], + +def invoke(entry_point: str, argv: tuple[str, ...]) -> subprocess.CompletedProcess[str]: + """Run an entry point the way an operator would -- wrapper via bash, CLI via Python.""" + target = contract.plugin_path(entry_point) + command = ( + [BASH, str(target), *argv] + if entry_point.startswith("bin/") + else [sys.executable, str(target), *argv] + ) + return subprocess.run( + command, capture_output=True, text=True, + # A foreign cwd proves the wrappers' self-location, and keeps a stray + # repo-relative default from being read as a guard holding. + cwd=tempfile.gettempdir(), ) - payload = {} - if proc.stdout.strip(): - try: - payload = json.loads(proc.stdout) - except json.JSONDecodeError: - payload = {} - return proc.returncode, payload - - -class MergeGuardFailsClosed(unittest.TestCase): - def test_absent_allowlist_refuses_exit_3(self): - code, payload = run(MERGE, "owner/repo#1") - self.assertEqual(code, 3) - self.assertFalse(payload.get("inScope")) - self.assertIn("allowed-owners", payload.get("error", "")) - - def test_owner_out_of_scope_refuses_exit_3(self): - code, payload = run(MERGE, "owner/repo#1", "--allowed-owners", "someone-else") - self.assertEqual(code, 3) - self.assertFalse(payload.get("inScope")) - - def test_self_logins_at_me_refuses_out_of_scope_before_resolving(self): - # '@me' resolution is a network call; passing it with an out-of-scope - # owner must still refuse at exit 3 (owner check precedes resolution), - # so this runs with no gh available and must not hang or error. - code, payload = run( - MERGE, "owner/repo#1", "--allowed-owners", "someone-else", - "--self-logins", "@me", - ) - self.assertEqual(code, 3) - self.assertFalse(payload.get("inScope")) - def test_short_expected_head_is_usage_error_exit_2(self): - code, payload = run( - MERGE, "owner/repo#1", "--allowed-owners", "owner", - "--merge", "--expected-head", "abc", - ) - self.assertEqual(code, 2) - self.assertIn("expected-head", payload.get("error", "")) - - def test_malformed_ref_is_usage_error_exit_2(self): - code, _ = run(MERGE, "not-a-ref", "--allowed-owners", "owner") - self.assertEqual(code, 2) - - def test_autopilot_tier_without_required_sets_refuses_exit_3(self): - # The tier's fail-closed core: the umbrella flag alone, with none of its - # three required sets, refuses before any network access. - code, payload = run( - MERGE, "owner/repo#1", "--allowed-owners", "owner", - "--autopilot-merge-tier", + +def envelope(proc: subprocess.CompletedProcess[str]) -> dict[str, object]: + """The engine's JSON refusal envelope, or {} when the refusal never reached Python.""" + if not proc.stdout.strip(): + return {} + try: + parsed = json.loads(proc.stdout) + except json.JSONDecodeError: + return {} + return parsed if isinstance(parsed, dict) else {} + + +def state_fingerprint(state_dir: pathlib.Path) -> dict[str, bytes]: + """Durable state contents, ignoring the advisory .lock files every run creates.""" + return { + str(path.relative_to(state_dir)): path.read_bytes() + for path in sorted(state_dir.rglob("*")) + if path.is_file() and path.suffix != ".lock" + } + + +def observed_delta(before: dict[str, bytes], after: dict[str, bytes]) -> str: + """Which way the state directory's file set moved. + + Directional, because a boolean cannot tell deletion from a rewrite: a reap + that rewrote the expired lease instead of removing it would satisfy + `before != after` and pass a row whose claim is deletion. Compared over the + key set rather than named paths -- a path literal in a drift-detection + artifact couples it to the writer's internal layout. + """ + if before == after: + return contract.UNCHANGED + kept, arrived = set(before), set(after) + if arrived > kept: + return contract.ADDED + if arrived < kept: + return contract.REMOVED + if arrived == kept: + return contract.REWRITTEN + return f"mixed: +{sorted(arrived - kept)} -{sorted(kept - arrived)}" + + +def seed(fixture: str, state_dir: pathlib.Path) -> None: + if fixture == contract.EMPTY_STATE: + return + if fixture == contract.EXPIRED_WORKER_LEASE: + # Acquire a real lease so the file shape stays the engine's, then age it + # past expiry -- a hand-written record would drift from the writer. + proc = invoke( + contract.LEASE_CLI, + ( + "acquire", + "--scope", + "worker", + "--pr", + "owner/repo#1", + "--state-dir", + str(state_dir), + ), ) - self.assertEqual(code, 3) - error = payload.get("error", "") - for flag in ("--lane-logins", "--approver-bot-logins", "--block-labels"): - self.assertIn(flag, error) - - def test_autopilot_tier_partial_config_refuses_exit_3(self): - # Two of three supplied still refuses, naming only the missing set. - code, payload = run( - MERGE, "owner/repo#1", "--allowed-owners", "owner", - "--autopilot-merge-tier", "--lane-logins", "lane", - "--approver-bot-logins", "bot", + if proc.returncode != 0: + raise AssertionError(f"lease fixture setup failed: {proc.stderr}") + lease_path = pathlib.Path(str(json.loads(proc.stdout)["path"])) + record = json.loads(lease_path.read_text(encoding="utf-8")) + stale = (datetime.now(UTC) - timedelta(days=1)).isoformat() + record.update({"expires_at": stale, "updated_at": stale}) + lease_path.write_text(json.dumps(record), encoding="utf-8") + return + raise AssertionError(f"unknown fixture: {fixture}") + + +def write_gh_shim(directory: pathlib.Path, sentinel: pathlib.Path) -> None: + """A `gh` that records the fact it ran and then fails. + + Exit 127 mimics the not-found status a caller is most likely to swallow, so + a guard that resolved `@me` before checking scope reaches the same refusal + exit code -- and is caught by the sentinel rather than by the exit code. + + The interpreter is named absolutely. A `/usr/bin/env bash` shebang resolves + `bash` through PATH, which the replay below deliberately narrows to this + directory alone: the shim would fail to exec, record nothing, and every row + would pass without proving anything. + """ + posix = directory / "gh" + posix.write_text( + # as_posix(), because `shutil.which` hands back a backslash path under + # Git Bash on Windows and a shebang is read by the kernel, not a shell. + f'#!{pathlib.Path(BASH).as_posix()}\nprintf called > "{sentinel.as_posix()}"\nexit 127\n', + encoding="utf-8", + newline="\n", + ) + posix.chmod(0o755) + if os.name == "nt": + # Windows resolves a bare `gh` through PATHEXT, never the extensionless + # file above, so the shim needs a .cmd sibling to be reachable at all. + (directory / "gh.cmd").write_text( + f'@echo off\r\n(echo called) > "{sentinel}"\r\nexit /b 127\r\n', + encoding="utf-8", + newline="", ) - self.assertEqual(code, 3) - self.assertIn("--block-labels", payload.get("error", "")) - - def test_tier_params_without_umbrella_are_usage_error_exit_2(self): - # The parameter sets are meaningless without the umbrella flag; supplying - # them alone is a usage error, never a silent no-op. - code, payload = run( - MERGE, "owner/repo#1", "--allowed-owners", "owner", - "--lane-logins", "lane", + + +def because(row_id: str, claim: str, detail: str = "") -> str: + suffix = f"\n observed: {detail}" if detail else "" + return ( + f"\nGUARD CONTRACT ROW `{row_id}` NO LONGER HOLDS." + f"\n claim: {claim}" + f"\n Fix the guard, or update the row in scripts/tests/guard_contract.py and" + f" regenerate reference/guard-contract.md." + f"{suffix}" + ) + + +class RefusalsFireOnArgumentShape(unittest.TestCase): + def test_every_refusal_row(self) -> None: + for row in contract.REFUSALS: + with self.subTest(row=row.id): + if row.entry_point.startswith("bin/") and BASH is None: + self.skipTest("bash unavailable; wrapper rows need it") + with tempfile.TemporaryDirectory() as tmp: + argv = tuple(arg.format(state_dir=tmp) for arg in row.argv) + proc = invoke(row.entry_point, argv) + payload = envelope(proc) + message = str(payload.get("error", "")) + combined = "\n".join((message, proc.stdout, proc.stderr)) + self.assertEqual( + proc.returncode, + row.exit_code, + because(row.id, row.claim, f"exit {proc.returncode}: {combined[:400]}"), + ) + for token in row.error_contains: + self.assertIn( + token, combined, because(row.id, row.claim, combined[:400]) + ) + for key, value in row.envelope_fields: + self.assertIn( + key, payload, because(row.id, row.claim, f"no `{key}` in the envelope") + ) + self.assertEqual( + payload[key], + value, + because(row.id, row.claim, f"{key}={payload[key]!r}"), + ) + if row.entry_point.startswith("bin/"): + # The observable bash-vs-Python discriminator: a wrapper-level + # refusal never reaches the interpreter, so it emits no envelope. + if row.refused_by == contract.BASH_WRAPPER: + self.assertEqual( + payload, + {}, + because(row.id, row.claim, "a JSON envelope was emitted"), + ) + self.assertTrue( + proc.stderr.strip(), + because(row.id, row.claim, "no stderr from the wrapper"), + ) + else: + self.assertNotEqual( + payload, + {}, + because(row.id, row.claim, "no JSON envelope: bash refused"), + ) + + def test_the_recording_shim_is_reachable(self) -> None: + # Guards the guard. The replay below asserts a NEGATIVE -- the sentinel + # was never written -- so a shim the isolated PATH cannot execute makes + # every row pass while proving nothing. Resolve and invoke it through + # the lane's own subprocess seam, so a change to how `gh` is located + # cannot leave this probe agreeing with a shim the product would miss. + if BASH is None: + self.skipTest("bash unavailable; the gh shim needs it") + with tempfile.TemporaryDirectory() as tmp: + shim_dir = pathlib.Path(tmp) + sentinel = shim_dir / "gh-was-called" + write_gh_shim(shim_dir, sentinel) + probe = subprocess.run( + [ + sys.executable, + "-c", + "import sys; sys.path.insert(0, sys.argv[1]);" + " import babysit_gh; babysit_gh.gh_capture(['--version'])", + str(contract.SCRIPTS), + ], + capture_output=True, + text=True, + env=dict(os.environ, PATH=str(shim_dir)), + cwd=tempfile.gettempdir(), + ) + recorded = sentinel.exists() + self.assertTrue( + recorded, + "the recording gh shim is unreachable under the isolated PATH, so" + " test_scope_refusal_precedes_every_network_call cannot detect a gh" + f" call at all: {probe.stderr[:400]}", ) - self.assertEqual(code, 2) - self.assertIn("--autopilot-merge-tier", payload.get("error", "")) + def test_scope_refusal_precedes_every_network_call(self) -> None: + # The rows claiming "no gh invocation at all" are replayed against a + # recording shim rather than an emptied PATH. Emptying PATH proves only + # that the refusal survives gh being unreachable -- a swallowed lookup + # failure falling through to the same exit code would pass it while the + # claim stayed false. The shim is the only executable on PATH, so any + # attempt to resolve `@me` leaves a sentinel behind. + rows = [row for row in contract.REFUSALS if row.gh_free] + self.assertTrue(rows, "no refusal row claims network-free refusal") + for row in rows: + with self.subTest(row=row.id): + if BASH is None: + self.skipTest("bash unavailable; the gh shim needs it") + with tempfile.TemporaryDirectory() as tmp: + shim_dir = pathlib.Path(tmp) + sentinel = shim_dir / "gh-was-called" + write_gh_shim(shim_dir, sentinel) + env = dict(os.environ, PATH=str(shim_dir)) + proc = subprocess.run( + [ + sys.executable, + str(contract.plugin_path(row.entry_point)), + *row.argv, + ], + capture_output=True, + text=True, + env=env, + cwd=tempfile.gettempdir(), + ) + called = sentinel.exists() + self.assertFalse( + called, + because(row.id, row.claim, "the gh shim was invoked"), + ) + self.assertEqual( + proc.returncode, + row.exit_code, + because(row.id, row.claim, f"exit {proc.returncode}: {proc.stderr[:400]}"), + ) + payload = envelope(proc) + for key, value in row.envelope_fields: + self.assertEqual( + payload.get(key), + value, + because(row.id, row.claim, f"{key}={payload.get(key)!r}"), + ) -class ResolveGuardFailsClosed(unittest.TestCase): - def test_absent_allowlist_refuses_exit_3(self): - code, payload = run(RESOLVE, "owner/repo#1") - self.assertEqual(code, 3) - self.assertFalse(payload.get("inScope")) - def test_count_pin_without_thread_id_is_usage_error(self): - code, payload = run( - RESOLVE, "owner/repo#1", "--allowed-owners", "owner", - "--expected-comment-count", "3", +class PredicatesHoldOverRuntimeData(unittest.TestCase): + def test_every_predicate_row(self) -> None: + for row in contract.PREDICATES: + with self.subTest(row=row.id): + observed = babysit_resolve_thread.classify(dict(row.thread), **row.flags) + self.assertEqual( + observed, + row.expected, + because(row.id, row.claim, f"classify returned {observed!r}"), + ) + + def test_anchor_symbol_exists(self) -> None: + # The anchors are the contract's substitute for line numbers; a rename + # must fail here rather than leave the doc pointing at nothing. + for row in contract.PREDICATES: + module, _, symbol = row.enforced_at.partition("::") + with self.subTest(row=row.id): + self.assertEqual(module, "babysit_resolve_thread.py") + self.assertTrue( + hasattr(babysit_resolve_thread, symbol), + because(row.id, row.claim, f"no symbol named {symbol!r}"), + ) + + +class EffectsReachDiskAsClaimed(unittest.TestCase): + def test_every_effect_row(self) -> None: + for row in contract.EFFECTS: + with self.subTest(row=row.id): + with tempfile.TemporaryDirectory() as tmp: + state_dir = pathlib.Path(tmp) + seed(row.fixture, state_dir) + before = state_fingerprint(state_dir) + argv = tuple(arg.format(state_dir=tmp) for arg in row.argv) + proc = invoke(row.entry_point, argv) + after = state_fingerprint(state_dir) + self.assertEqual( + proc.returncode, + row.exit_code, + because(row.id, row.claim, f"exit {proc.returncode}: {proc.stderr[:400]}"), + ) + self.assertEqual( + observed_delta(before, after), + row.delta, + because( + row.id, + row.claim, + f"observed {observed_delta(before, after)!r}", + ), + ) + + +class MechanismsMatchTheSource(unittest.TestCase): + def test_every_mechanism_row(self) -> None: + for row in contract.MECHANISMS: + with self.subTest(row=row.id): + source = contract.plugin_path(row.entry_point).read_text(encoding="utf-8") + for token in row.must_contain: + self.assertIn( + token, source, because(row.id, row.claim, f"{token!r} is absent") + ) + for token in row.must_not_contain: + self.assertNotIn( + token, source, because(row.id, row.claim, f"{token!r} appeared") + ) + + +class EntryPointCatalogueIsComplete(unittest.TestCase): + def test_every_executable_script_is_classified(self) -> None: + # A new entry point must arrive with its mutation classification, or a + # consumer's permission rule silently has no row to cite for it. + catalogued = {entry.path for entry in contract.ENTRY_POINTS} + present = { + f"skills/babysit-prs/scripts/{path.name}" + for path in contract.SCRIPTS.glob("*.py") + if "__main__" in path.read_text(encoding="utf-8") + } + # The lane is not confined to the skill's own scripts directory: SKILL.md + # invokes the plugin-level readiness gate by name, and a shell entry point + # there would otherwise evade this gate entirely. + present |= { + f"scripts/{path.name}" + for path in (contract.PLUGIN_ROOT / "scripts").glob("babysit-*.sh") + if not path.name.endswith(".test.sh") + } + self.assertEqual( + present - catalogued, + set(), + "entry points with no guard-contract classification; add them to" + " ENTRY_POINTS in scripts/tests/guard_contract.py", ) - self.assertEqual(code, 2) - self.assertIn("thread-id", payload.get("error", "")) + self.assertEqual(catalogued - present, set(), "classified scripts that no longer exist") - def test_last_updated_pin_without_thread_id_is_usage_error(self): - code, payload = run( - RESOLVE, "owner/repo#1", "--allowed-owners", "owner", - "--expected-last-updated", "2026-07-17T10:00:00Z", + def test_every_wrapper_is_classified(self) -> None: + wrappers = { + f"bin/{path.name}" + for path in (contract.PLUGIN_ROOT / "bin").iterdir() + if path.is_file() and path.name.startswith("source-control-babysit-") + } + self.assertEqual( + wrappers, + {entry.wrapper for entry in contract.ENTRY_POINTS if entry.wrapper}, + "a babysit wrapper is missing from ENTRY_POINTS", ) - self.assertEqual(code, 2) - self.assertIn("thread-id", payload.get("error", "")) - def test_resolve_thread_id_without_pins_is_refused(self): - code, payload = run( - RESOLVE, "owner/repo#1", "--allowed-owners", "owner", - "--resolve", "--thread-id", "PRRT_abc", + def test_classifications_cite_only_real_rows(self) -> None: + known = {row.id for row in contract.REFUSALS} + known |= {row.id for row in contract.PREDICATES} + known |= {row.id for row in contract.EFFECTS} + known |= {row.id for row in contract.MECHANISMS} + for entry in contract.ENTRY_POINTS: + for row_id in entry.backed_by: + with self.subTest(entry=entry.path, row=row_id): + self.assertIn(row_id, known, "backed_by names a row that does not exist") + + def test_row_ids_are_unique(self) -> None: + ids = [ + row.id + for table in ( + contract.REFUSALS, + contract.PREDICATES, + contract.EFFECTS, + contract.MECHANISMS, + contract.DOC_COMMAND_SOURCES, + ) + for row in table + ] + self.assertEqual(sorted(ids), sorted(set(ids)), "row IDs are citation anchors") + + def test_every_mutating_entry_point_is_backed(self) -> None: + for entry in contract.ENTRY_POINTS: + if entry.mutation == contract.READ_ONLY: + continue + with self.subTest(entry=entry.path): + self.assertTrue( + entry.backed_by, + f"`{entry.path}` is classified {entry.mutation} with nothing asserting it", + ) + + def test_read_only_entry_points_cite_no_mutation_evidence(self) -> None: + # The one direction of the Class column CI can bind offline. The reverse + # -- proving a mutating class -- cannot be bound for the four entry + # points whose mutation is a GitHub write, because every row here runs + # without network access; the generated doc's "Not covered here" section + # states that gap rather than letting the column read as proven. + moves_state = {row.id for row in contract.EFFECTS if row.delta != contract.UNCHANGED} + moves_state |= {row.id for row in contract.MECHANISMS} + for entry in contract.ENTRY_POINTS: + if entry.mutation != contract.READ_ONLY: + continue + with self.subTest(entry=entry.path): + self.assertEqual( + set(entry.backed_by) & moves_state, + set(), + f"`{entry.path}` is classified read-only but cites rows asserting" + " a mutation; reclassify it or move the evidence", + ) + + +WRAPPER_COMMAND = re.compile( + r'bash "\$\{CLAUDE_PLUGIN_ROOT\}/(bin/source-control-babysit-[a-z-]+)"' +) + + +def documented_commands(text: str) -> list[tuple[str, str]]: + """Wrapper commands in a document, each paired with its argument tail. + + A command runs to the end of its inline-code span or fenced block, which + prose wraps across lines, so the tail is truncated at the first blank line + rather than the first newline. The truncation is done here instead of in the + pattern: expressing the stop set as an alternation backtracks + catastrophically on a document this size. A fenced block may hold several + commands, so a tail also stops at the next wrapper invocation -- one greedy + tail swallowing its successors would check the later commands' flags against + the first command's CLI, or skip them entirely. + """ + matches = list(WRAPPER_COMMAND.finditer(text)) + commands = [] + for index, match in enumerate(matches): + limit = matches[index + 1].start() if index + 1 < len(matches) else len(text) + tail = text[match.end() : limit].split("`", 1)[0].split("\n\n", 1)[0] + commands.append((match.group(1), " ".join(tail.split()))) + return commands + + +class DocumentedCommandsMatchTheParsers(unittest.TestCase): + def _accepted_flags(self, cli: str) -> set[str]: + """The parser's registered long options, read from its usage block only. + + Scraping the whole `--help` output would treat any flag NAMED IN PROSE as + accepted: every one of these CLIs describes the broad `gh pr merge + --admin` grant it exists to replace, so `--admin` would be admitted here + while the parser exits 2 on it, and a document carrying that stale flag + would pass. Argparse renders the usage block from the registered option + strings and nothing else, so it -- not the description, not the help + text -- is the registry. It runs from `usage:` to the first blank line. + """ + proc = subprocess.run( + [sys.executable, str(contract.plugin_path(cli)), "--help"], + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, f"{cli} --help failed: {proc.stderr}") + self.assertTrue( + proc.stdout.startswith("usage:"), + f"{cli} --help does not open with a usage block; the registry moved", ) - self.assertEqual(code, 2) - self.assertIn("expected-comment-count", payload.get("error", "")) - self.assertIn("expected-last-updated", payload.get("error", "")) - - def test_autonomous_bulk_resolve_without_thread_id_is_refused(self): - # The bug this guard closes: an unattended worker's own push marks a - # thread isOutdated, and a bulk (no --thread-id) autonomous resolve would - # clear it with no proof the finding was addressed. Refused before any - # network fetch -- the fix-closed contract the docs already describe. - code, payload = run( - RESOLVE, "owner/repo#1", "--allowed-owners", "owner", - "--autonomous", "--resolve", + usage = proc.stdout.split("usage:", 1)[1].split("\n\n", 1)[0] + flags = set(re.findall(r"(? None: + # The regression this parse exists to prevent, pinned: `--admin` appears + # in the merge gate's description and is rejected by its parser. + self.assertNotIn("--admin", self._accepted_flags(contract.MERGE_CLI)) + + def test_every_documented_wrapper_command(self) -> None: + accepted: dict[str, set[str]] = {} + for row in contract.DOC_COMMAND_SOURCES: + text = contract.plugin_path(row.doc).read_text(encoding="utf-8") + commands = documented_commands(text) + self.assertTrue( + commands, + because(row.id, row.claim, f"no wrapper command lines found in {row.doc}"), + ) + for wrapper, tail in commands: + with self.subTest(doc=row.doc, wrapper=wrapper): + self.assertTrue( + contract.plugin_path(wrapper).is_file(), + because(row.id, row.claim, f"{wrapper} does not exist"), + ) + cli = contract.WRAPPER_BACKING_CLI[wrapper] + if cli not in accepted: + accepted[cli] = self._accepted_flags(cli) + for flag in re.findall(r"(? None: + # The denial table is data; a refusal row is the evidence. A flag listed + # as wrapper-refused with nothing invoking the wrapper to prove it would + # be an unbacked claim, which is the shape this contract exists to catch. + for wrapper, denied in contract.WRAPPER_DENIED_FLAGS.items(): + for flag in denied: + with self.subTest(wrapper=wrapper, flag=flag): + self.assertTrue( + any( + row.entry_point == wrapper + and row.refused_by == contract.BASH_WRAPPER + and flag in row.argv + for row in contract.REFUSALS + ), + f"`{flag}` is listed as refused by {wrapper} with no" + " bash-wrapper refusal row invoking it", + ) + + def test_every_doc_naming_a_wrapper_is_covered(self) -> None: + # A second document growing its own copy of a command line must be added + # to DOC_COMMAND_SOURCES, not left unchecked. + covered = {row.doc for row in contract.DOC_COMMAND_SOURCES} + # The whole plugin, not just the babysit skill: a command line copied + # into a command, agent, or sibling skill drifts exactly the same way. + naming = { + str(path.relative_to(contract.PLUGIN_ROOT)).replace("\\", "/") + for path in contract.PLUGIN_ROOT.rglob("*.md") + if WRAPPER_COMMAND.search(path.read_text(encoding="utf-8")) + } + self.assertEqual( + naming - covered, + set(), + "documents spelling out wrapper command lines with no DOC_COMMAND_SOURCES row", ) - self.assertEqual(code, 2) - self.assertIn("thread-id", payload.get("error", "")) - self.assertIn("bulk-resolve", payload.get("error", "")) - - def test_autonomous_allow_unpinned_thread_is_refused(self): - # There is no unpinned autonomous resolve: --allow-unpinned-thread is an - # interactive-only override and must not open a bypass around the pins in - # unattended mode, even with a single --thread-id. - code, payload = run( - RESOLVE, "owner/repo#1", "--allowed-owners", "owner", - "--autonomous", "--resolve", "--thread-id", "PRRT_abc", - "--allow-unpinned-thread", + + +class GeneratedDocIsCurrent(unittest.TestCase): + def test_reference_doc_matches_the_tables(self) -> None: + # splitlines(), not bytes: the checkout's line endings are not the claim. + expected = contract.render_markdown().splitlines() + actual = contract.GENERATED_DOC.read_text(encoding="utf-8").splitlines() + self.assertEqual( + actual, + expected, + "reference/guard-contract.md is stale -- regenerate it with" + " `python tests/guard_contract.py --emit`", ) - self.assertEqual(code, 2) - self.assertIn("allow-unpinned-thread", payload.get("error", "")) if __name__ == "__main__":