Repository navigation
kanban: make the survivor escape hatch reachable from the kanban_complete TOOL (t_52abef50) - #839
Merged
Merged
Conversation
…ishes `preserve()` records `bases` at dispatch from the git checkout then in the workspace. When that checkout is later gone but the workspace DIRECTORY survives (evidence, logs, qa-output), the recorded-repository guard raised unconditionally -- on the branch where `explicit` is never consulted. The `--survivor-pr` / `--survivor-ref` remedy the error names is only read on the workspace-MISSING branch, so that state had no reachable remedy at all: a reviewer who edited nothing and had nothing to preserve could not complete, even with a valid remote-verified PR. Consult `explicit` before that raise. With no operator survivor the guard is unchanged and still fails closed -- that is what protects unpushed work. The relaxation is for OPERATOR flags only; a mined hint cannot reach it. Two things the first cut got wrong, both found by mutation and fixed: - `claimed = True` is required, or a verified survivor is accepted but never recorded. - On a PARTIAL loss (some recorded repos survive, one vanished), the survivors of the rest satisfied the completion on their own and silently dropped the operator's ref for the lost repo. `recovered` seeds it into `refs`, and the ref-kind count accounts for it -- without that arithmetic the card falls through to `_external` and records a DUPLICATE ref under repository ".". Verified: - 7 new tests in tests/hermes_cli/test_kanban_survivor_stale_bases.py. - 5/5 mutations killed (revert the fix; drop the guard; drop `claimed = True`; drop `recovered` seeding; revert the ref-count arithmetic). Source restored and sha256-compared byte-identical after the sweep. - 959 passed / 0 failed / 3 skipped across all 81 tests/hermes_cli kanban files. - ruff 0.15.20 clean; `git diff --check` clean. - Live proof on an isolated HERMES_KANBAN_SANDBOX=1 board through the real CLI: refusal -> exit 1 + reason + remedy, card left `ready`; a real remote-verified `--survivor-pr #835` -> exit 0, "Completed <id>", survivor recorded at b4707a3; a nonexistent PR -> exit 1, still refused. NOT DONE, and deliberately: the card's item 3 asks to fix `hermes kanban complete` exiting 0 on this refusal. That does not reproduce on this tree. A raised SurvivorUnavailable is a ValueError and kanban.py already catches it, prints `kanban: <reason>` to stderr, and returns 1; main.py propagates a nonzero handler return via sys.exit. Measured exit 1 on fork/main BEFORE this change, through both `python -m hermes_cli.main` and the deployed `hermes` wrapper. What was genuinely missing was the remedy text -- the message named no way out -- so the HINT is now appended, and two CLI-level tests pin exit-nonzero-with-hint and exit-zero-on-success. The card's item 4 (should a reviewer-lane completion consult `bases` at all) is a contract question left open, not silently decided.
…lete TOOL `preserve()` refuses a completion it cannot find an implementation for and points at `--survivor-pr` / `--survivor-ref`. #837 restored that remedy on the CLI and the library. It was still absent from the TOOL -- the agent-facing surface where the refusal is actually READ. Argus measured it on #837 head a26f270: 0 occurrences of "survivor" in tools/kanban_tools.py. A worker hit a hint naming a flag it had no way to invoke. Add `survivor_pr` / `survivor_ref` to the schema and thread them to `kb.complete_task(...)`, which already accepted both. DESIGN CALL (the card asked for it explicitly): worker-reachable, not operator-only. Not on ergonomics -- on a MEASURED fact. A worker can already reach `--survivor-pr` today by shelling out to `hermes kanban complete`, and I measured that path taking effect: rc=0, status=done, survivor recorded, with `_worker_run_id_for()` returning None because the shelled-out subprocess does not own the run. So the shell-out ALREADY skips the `expected_run_id` optimistic-concurrency guard that keeps a stale writer from closing a live card. "Operator-only" was never enforced -- it was only inconvenient, and the inconvenient path is the weaker one. The tool argument is strictly safer: it runs inside the owning process, so ownership and expected_run_id both apply. The verification semantics are unchanged because both arguments land on the same `_verified_explicit()` call the flag uses: - an operator-named survivor is still remote-VERIFIED, - an unverifiable claim still refuses, - text mining still cannot reach the relaxed branch (test 3 pins this: the same PR named in the SUMMARY instead of the argument still fails closed). A non-string argument is rejected before the verifier, so a malformed claim cannot reach the remote as junk. Redaction is inherited from the same `_ext.redact()` path -- test 4 pins that a `survivor_ref` carrying `oauth2:ghp_...@` is redacted in the tool's returned error and absent from held_reason and the event log. Stacked on #837 (this state is only reachable with its stale-bases fix). Fork-only: upstream has tools/kanban_tools.py but no kanban_survivor.py and no survivor params on complete_task, so there is no upstream surface to port to. VERIFIED - tests/tools/test_kanban_tool_survivor.py: 6 passed, 5/5 consecutive runs. - mutation (drop the threading): 2 failed, 4 passed -- the tests have teeth. - E2E from inside a REAL worker run, which Argus's probe could not do (its non-owning process was correctly refused by the ownership guard). With owns_kanban_worker_authority=True against a sandboxed board: no argument -> `survivor_unavailable: recorded repository missing; use --survivor-pr ...`; with survivor_pr -> {"ok": true}, status=done, survivor recorded as the verified external ref, and one real `gh pr view` consulted. - Argus's measurement re-run: TOOL_ACCEPTS_SURVIVOR_PR/REF now both True. - Cohort (95 tests incl. test_kanban_redaction, test_kanban_tools, the three survivor files): flaky in this environment, wandering failures across runs in tests this diff does not touch. Reproduced the SAME wandering flake on base with the change stashed (3 runs, 3 different failures), so it is inherited, not mine. My file is stable 5/5. Incident disclosed for the record: my first shell-out probe leaked a card (t_4a83f71f) onto the LIVE board -- HERMES_KANBAN_DB outranks HERMES_HOME, so redirecting HERMES_HOME alone does NOT sandbox kanban (the runtime warned; I had already written). Fully reverted (tasks/task_runs/task_events/ task_workspace_survivors, no attachments); live board re-verified clean. Every subsequent probe sets HERMES_KANBAN_SANDBOX=1, unsets the HERMES_KANBAN_* pins, and asserts the resolved DB path is under its tmpdir before writing.
Collaborator
Author
FleetReviewReviewed with 2 of 3 model families — openai unavailable. Confidence: 3/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $4.83 · duration: 48m 47s · rounds: 2 · files examined: 2 |
This was referenced Sep 21, 2026
Base automatically changed from
daedalus-opus/t_49c1dec1-survivor-escape
to
main
September 22, 2026 01:50
Kyzcreig
added a commit
that referenced
this pull request
Sep 22, 2026
An operator- or worker-named --survivor-pr/--survivor-ref was verified only as "this PR exists on GitHub and is OPEN or MERGED". _verified_explicit() called verify_pr()/verify_ref() with mined_for UNSET, so the one check that ties a PR to a card never ran on the explicit path. preserve() then treats a verified explicit survivor as authority for the stale-bases branch -- the branch whose whole job is protecting UNPUSHED implementation work -- so any live PR authorised deleting a workspace whose bytes may exist nowhere else. Measured on fork/main with real gh, two PRs unrelated to the probe card: NousResearch#1 explicit -> ACCEPTED MERGED; mined -> REFUSED #837 explicit -> ACCEPTED OPEN; mined -> REFUSED After this change both are REFUSED on the explicit path and ACCEPTED only under the new override. Remedy (a) with an explicit override. The claim now carries the same task-id binding the mined path carries. The legitimate operator case -- a human who knows the work landed on a differently-named branch -- keeps a reachable path via --survivor-unbound, which is recorded on the survivor (unbound=True, claimed_by=<OS user>) and replayed into the task event log, so the authorisation is auditable rather than invisible. The override is a CLI flag only: no kanban_complete tool argument can express it, so a worker on the #839 tool surface cannot self-certify an unrelated survivor. The explicit path corroborates on headRefName/title/body -- the PR's own claim about which card it implements, from a caller who already vouched for the PR's identity. The mined path stays branch-only via the `corroborate` default: widening it would let a PR body that merely mentions a card id verify itself out of handoff text. Verified: 79 passed / 0 failed across all four survivor files (fork/main baseline measured 65 in a clean-room worktree; +14 new). Mutations, each run and reverted: drop the binding -> 3 red; widen the mined path -> 1 red; kill the override -> 2 red. Disjoint from #837/#839/#842/#796 -- none of them touches _verified_explicit.
Kyzcreig
added a commit
that referenced
this pull request
Sep 22, 2026
An operator- or worker-named --survivor-pr/--survivor-ref was verified only as "this PR exists on GitHub and is OPEN or MERGED". _verified_explicit() called verify_pr()/verify_ref() with mined_for UNSET, so the one check that ties a PR to a card never ran on the explicit path. preserve() then treats a verified explicit survivor as authority for the stale-bases branch -- the branch whose whole job is protecting UNPUSHED implementation work -- so any live PR authorised deleting a workspace whose bytes may exist nowhere else. Measured on fork/main with real gh, two PRs unrelated to the probe card: NousResearch#1 explicit -> ACCEPTED MERGED; mined -> REFUSED #837 explicit -> ACCEPTED OPEN; mined -> REFUSED After this change both are REFUSED on the explicit path and ACCEPTED only under the new override. Remedy (a) with an explicit override. The claim now carries the same task-id binding the mined path carries. The legitimate operator case -- a human who knows the work landed on a differently-named branch -- keeps a reachable path via --survivor-unbound, which is recorded on the survivor (unbound=True, claimed_by=<OS user>) and replayed into the task event log, so the authorisation is auditable rather than invisible. The override is a CLI flag only: no kanban_complete tool argument can express it, so a worker on the #839 tool surface cannot self-certify an unrelated survivor. The explicit path corroborates on headRefName/title/body -- the PR's own claim about which card it implements, from a caller who already vouched for the PR's identity. The mined path stays branch-only via the `corroborate` default: widening it would let a PR body that merely mentions a card id verify itself out of handoff text. Verified: 79 passed / 0 failed across all four survivor files (fork/main baseline measured 65 in a clean-room worktree; +14 new). Mutations, each run and reverted: drop the binding -> 3 red; widen the mined path -> 1 red; kill the override -> 2 red. Disjoint from #837/#839/#842/#796 -- none of them touches _verified_explicit.
Kyzcreig
added a commit
that referenced
this pull request
Sep 22, 2026
…ding #839 (tool-reachable survivor_pr) landed on main mid-run. Its fixture PR is live and MERGED but its head branch names no card, so the binding this branch adds correctly REFUSES it -- caught as the only head-vs-base delta in the -k kanban sweep (33 head / 32 base, one name). The gate is right, the fixture was not: name the card for the happy path, exactly as tests/hermes_cli/test_kanban_survivor_stale_bases.py does. Also lands Argus's r2 carry-forward. test_the_tool_surface_cannot_express_ the_override is a SOURCE GREP and structurally cannot catch a runtime kwarg, so add the RUNTIME arms now that survivor_pr is really on the tool: - a live PR that does not name the card -> refused, workspace bytes intact - 4 smuggling spellings of the CLI-only override (survivor_unbound, unbound, string "1", metadata-nested) -> all refused, bytes intact Measured: 11 passed. Mutation, applied then reverted: drop the binding (mined_for unset on the explicit path) -> 5 red, all five of the new arms.
Kyzcreig
added a commit
that referenced
this pull request
Sep 22, 2026
An operator- or worker-named --survivor-pr/--survivor-ref was verified only as "this PR exists on GitHub and is OPEN or MERGED". _verified_explicit() called verify_pr()/verify_ref() with mined_for UNSET, so the one check that ties a PR to a card never ran on the explicit path. preserve() then treats a verified explicit survivor as authority for the stale-bases branch -- the branch whose whole job is protecting UNPUSHED implementation work -- so any live PR authorised deleting a workspace whose bytes may exist nowhere else. Measured on fork/main with real gh, two PRs unrelated to the probe card: NousResearch#1 explicit -> ACCEPTED MERGED; mined -> REFUSED #837 explicit -> ACCEPTED OPEN; mined -> REFUSED After this change both are REFUSED on the explicit path and ACCEPTED only under the new override. Remedy (a) with an explicit override. The claim now carries the same task-id binding the mined path carries. The legitimate operator case -- a human who knows the work landed on a differently-named branch -- keeps a reachable path via --survivor-unbound, which is recorded on the survivor (unbound=True, claimed_by=<OS user>) and replayed into the task event log, so the authorisation is auditable rather than invisible. The override is a CLI flag only: no kanban_complete tool argument can express it, so a worker on the #839 tool surface cannot self-certify an unrelated survivor. The explicit path corroborates on headRefName/title/body -- the PR's own claim about which card it implements, from a caller who already vouched for the PR's identity. The mined path stays branch-only via the `corroborate` default: widening it would let a PR body that merely mentions a card id verify itself out of handoff text. Verified: 79 passed / 0 failed across all four survivor files (fork/main baseline measured 65 in a clean-room worktree; +14 new). Mutations, each run and reverted: drop the binding -> 3 red; widen the mined path -> 1 red; kill the override -> 2 red. Disjoint from #837/#839/#842/#796 -- none of them touches _verified_explicit.
Kyzcreig
added a commit
that referenced
this pull request
Sep 22, 2026
…ding #839 (tool-reachable survivor_pr) landed on main mid-run. Its fixture PR is live and MERGED but its head branch names no card, so the binding this branch adds correctly REFUSES it -- caught as the only head-vs-base delta in the -k kanban sweep (33 head / 32 base, one name). The gate is right, the fixture was not: name the card for the happy path, exactly as tests/hermes_cli/test_kanban_survivor_stale_bases.py does. Also lands Argus's r2 carry-forward. test_the_tool_surface_cannot_express_ the_override is a SOURCE GREP and structurally cannot catch a runtime kwarg, so add the RUNTIME arms now that survivor_pr is really on the tool: - a live PR that does not name the card -> refused, workspace bytes intact - 4 smuggling spellings of the CLI-only override (survivor_unbound, unbound, string "1", metadata-nested) -> all refused, bytes intact Measured: 11 passed. Mutation, applied then reverted: drop the binding (mined_for unset on the explicit path) -> 5 red, all five of the new arms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
preserve()refuses a completion whose implementation it cannot find and points at--survivor-pr/--survivor-ref. #837 restored that remedy on the CLI and the library. It was still absent from the TOOL — the agent-facing surface where the refusal is actually read.Argus measured it on #837 head
a26f2700bd: 0 occurrences of "survivor" intools/kanban_tools.py. A kanban worker hitsurvivor_unavailable: ... use --survivor-pr <owner/repo#N>and the error named a remedy the worker had no way to invoke.This PR adds
survivor_pr/survivor_refto thekanban_completeschema and threads them tokb.complete_task(...), which already accepted both.Stacked on #837 — this state is only reachable with its stale-bases fix. Merge #837 first.
Design call: worker-reachable, not operator-only
The card asked me to decide explicitly whether a worker should be able to name a survivor at all, since self-certifying weakens the guard protecting unpushed work.
I made it worker-reachable, not on ergonomics — on a measured fact: a worker can already reach
--survivor-prtoday by shelling out tohermes kanban complete. I measured that path taking effect:The shelled-out subprocess does not own the run, so it silently skips the
expected_run_idoptimistic-concurrency guard that keeps a stale writer from closing a live card. "Operator-only" was never enforced — it was only inconvenient, and the inconvenient path is the weaker one. The tool argument is strictly safer: it runs inside the owning process, so ownership andexpected_run_idboth apply.Verification semantics are unchanged
Both arguments land on the same
_verified_explicit()call the flag uses:A non-string argument is rejected before the verifier, so a malformed claim never reaches the remote as junk. Redaction is inherited from the same
_ext.redact()path; test 4 pins that asurvivor_refcarryingoauth2:ghp_...@is redacted in the tool's returned error and absent fromheld_reasonand the event log.Verified
tests/tools/test_kanban_tool_survivor.py— 6 passed, stable 5/5 consecutive runs.owns_kanban_worker_authority=Trueagainst a sandboxed board:The
ghcall proves the tool argument is verified against the remote, not taken on trust.TOOL_ACCEPTS_SURVIVOR_PR = True,TOOL_ACCEPTS_SURVIVOR_REF = True, 22 occurrences of "survivor".Scope / provenance
tools/kanban_tools.pybut nokanban_survivor.pyand no survivor params oncomplete_task— there is no upstream surface to port to. No upstream PR.hermes_cli/kanban.pyNOT touched despite the dispatch-collision warning witht_f08b7589. The threading needed nothing there.Incident disclosed
My first shell-out probe leaked a card (
t_4a83f71f) onto the live board:HERMES_KANBAN_DBoutranksHERMES_HOME, so redirectingHERMES_HOMEalone does not sandbox kanban. The runtime warned, but I had already written. Fully reverted (tasks/task_runs/task_events/task_workspace_survivors; no attachments existed), live board re-verified clean. Every subsequent probe setsHERMES_KANBAN_SANDBOX=1, unsets theHERMES_KANBAN_*pins, and asserts the resolved DB path is under its tmpdir before writing.Card: t_52abef50 (derived from t_49c1dec1)
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.