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.
…upstream raise
On the PARTIAL-LOSS path (one recorded repo published, one recorded-but-
vanished) the branch performs NO verification of its own. It builds `recovered`
straight out of `explicit` and feeds it into the `len(refs) == len(repos) + 1`
arithmetic that decides completion. The ONLY thing between an operator's
CLOSED/fake `--survivor-pr` and a durably stored survivor is the single
`_verified_explicit()` raise upstream at kanban_survivor.py:324.
Nothing pinned that dependency. The existing coverage misses exactly this
intersection: test_partial_loss_keeps_both_the_surviving_repo_and_the_operator_ref
tests partial loss with a VERIFIED survivor;
test_stale_bases_with_an_unverifiable_survivor_pr_still_refuses tests an
unverifiable survivor on the ALL-GONE path.
Add three refusal pins (CLOSED-unmerged PR, nonexistent PR, unresolvable
--survivor-ref) plus a verified-ref completion as anti-vacuity teeth, and
extend the `remote` fixture to stub `gh` failure and `git ls-remote` tips so
no test reaches the network. The refusal assertions check SHAPE, not just
outcome: no survivor row persisted, no run metadata survivor, and in
particular no `repository: "."` entry from a fall-through to the external path.
DECISION -- test-only, no defense-in-depth duplication in the branch.
`_verified_explicit()` is the correct single choke point: it is the one place
both flags are converted from claim to authority, and it guards BOTH the
workspace-MISSING and the partial-loss branch. Re-verifying inside the branch
would mean a second remote round-trip against the same claim, with two copies
of the "is this authority" rule free to drift apart -- and the drift would be
silent in the safe direction only by luck. The real defect was that the
dependency was load-bearing and unpinned; a test that goes RED when the raise
moves fixes exactly that, at zero runtime cost and zero new verification
surface. The branch keeps its correctness stake honestly: it now has a test
that fails if its upstream guarantee is withdrawn.
Verified:
* 76 passed / 0 failed across all four survivor files, 5/5 consecutive runs
(baseline was 72; +4 new). Run under .venv/bin/python -P with PYTHONPATH.
* RED-PROVED. Mutating `_verified_explicit` to the presence-only behaviour
(return an UNVERIFIED ref instead of raising) turns all 3 refusal pins RED:
PINS_KILLED_MUTANT= 3/3 VERDICT= PINS THE MECHANISM
kanban_survivor.py restored byte-identical (sha256 d8d416beb97262bf before
and after). The pins pass on today's tree only because the raise is there.
* No implementation file touched; no existing guard weakened or relaxed.
* ruff 0.15.10 clean; git diff --check clean.
Collaborator
Author
FleetReview
Reviewed with 2 of 3 model families — openai unavailable. Confidence: 1/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $5.23 · duration: 13m 58s · rounds: 1 · files examined: 1 |
Base automatically changed from
daedalus-opus/t_49c1dec1-survivor-escape
to
main
September 22, 2026 01:50
Collaborator
Author
|
Superseded by #852. #849 was branched off #837's branch before that PR was squash-merged as |
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.
Residue of ARGUS round 2 on #837 (card t_ac0e595c). FleetReview's P1 consequence does NOT reproduce on today's tree — but the structural concern under it is real, and this is the pin for it.
The gap, measured
On the PARTIAL-LOSS path (one recorded repo published + on disk, one recorded-but-vanished) the branch performs no verification of its own. It builds
recoveredstraight out ofexplicitand feeds it into thelen(refs) == len(repos) + 1arithmetic that decides completion. The only thing between an operator's CLOSED/fake--survivor-prand a durably stored survivor is the single_verified_explicit()raise upstream atkanban_survivor.py:324.Nothing pinned that dependency. The existing coverage misses exactly the intersection:
test_partial_loss_keeps_both_the_surviving_repo_and_the_operator_reftest_stale_bases_with_an_unverifiable_survivor_pr_still_refusesReproduced first, not inferred. Monkeypatching
_verified_explicitto the presence-only world FleetReview described completes a partial-loss card and durably stores a CLOSED PR as the lost repo's survivor:What this PR adds
Three refusal pins on the intersection — CLOSED-unmerged PR, nonexistent PR, unresolvable
--survivor-ref(both flags) — plus a verified-ref completion as anti-vacuity teeth, proving the refusals are about verification and not about the flag being inert on this branch.Assertions check shape, not just outcome: no survivor row persisted, no run-metadata survivor, and specifically no
repository: "."entry from a fall-through to the external path.The
remotefixture gainsmissing(gh lookup fails the way a nonexistent PR does) andtips(what a baregit ls-remote <url>advertises, empty by default). Both are stubbed so no test reaches the network — the gate must not pass by relying on a network failure.RED-PROOF — the load-bearing part
A pin that passes on today's tree for a reason unrelated to the partial-loss branch is vacuous. Mutating
_verified_explicitto return an UNVERIFIED ref instead of raising:3/3. The pins pass today only because the raise is there — which is exactly the property that was unpinned.
DECISION: test-only, no defense-in-depth duplication
The card asked for this to be stated, not left implicit.
_verified_explicit()is the correct single choke point: it is the one place both flags are converted from claim to authority, and it already guards both the workspace-MISSING and the partial-loss branch. Re-verifying inside the branch means a second remote round-trip against the same claim, with two copies of the "is this authority" rule free to drift — and drift would land in the safe direction only by luck.The actual defect was never missing verification; it was that a load-bearing dependency was unpinned. A test that goes RED the moment the raise moves, narrows, inlines, or short-circuits fixes precisely that, at zero runtime cost and zero new verification surface. The branch keeps its correctness stake honestly: it now has a test that fails if its upstream guarantee is withdrawn.
Verification
.venv/bin/python -PwithPYTHONPATH— barepython3resolves to a 3.11 without yaml and gives a false "no tests collected" red.ruff 0.15.10clean;git diff --checkclean.Pre-existing flake, attributed not hand-waved
An intermittent 1-test failure appeared with a different victim each run. Attributed by measurement rather than assumed:
a26f2700bd, serial: 8/8 green; single victim test solo: 20/20 greena26f2700bdwith 4 concurrent copies: 4/4 runs degraded (1 failed / 2 errors / 4 errors / 1 error), zero of my code presentConclusion: inherited, load-induced git-subprocess contention under parallel pytest. Not introduced here. Serial runs on this branch are 5/5 clean at 76.
Stacking
Branched from
a26f2700bd(#837's head) and targets that branch, since the partial-loss branch it pins only exists in #837.hermes_cli/kanban_survivor.pyis a multi-way hotspot (#837, #796, #842, #839) — whoever lands last re-runs all four survivor files on the merged tree.Upstream:
kanban_survivor.pyis absent fromNousResearch/main(fork-only) — verified, so there is no upstream surface for this.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.