fix(kanban): close the 4 P1 FleetReview recorded on #848's head 0757be86 - #886
Merged
Merged
Conversation
#848 merged (squash 8300bd7) one hour past a trusted 5-member record on that exact head. Two of the four findings re-opened the hole #848 was written to close. 1. _verified_explicit: a MENTION no longer satisfies the completion path. A title/body match was marked `unbound` and still RETURNED as an explicit survivor. `_reusable` guards the recorded row on the `cleanup=True` pass only, and `_external` takes its `explicit` arm before the `cleanup` arm -- so "follow-up to t_...; does not address it" authorised a completion, and `preserve` treats a verified explicit survivor as authority over unpushed work. A mention now takes the same refusal an unrelated live claim takes, behind the same documented override. The two paths answer the question identically. 2. --survivor-unbound is PER-CLAIM. It was one parameter applied uniformly (`mined_for=None if unbound else task_id`), so an operator overriding one repository's claim silently accepted every sibling with no check that it names the card, and stamped `unbound=True` on all of them -- costing the correctly-bound ones their reclamation authority. New `_unbound_keys` normalises to (bare, keys); a bare flag covers a single-claim invocation and is refused on a multi-claim one; an override naming no claim is refused. CLI flag is now `append`/`nargs="?"`. 3. verify_ref refuses an ambiguous SHA instead of picking a tip. `matches` was keyed by oid, so later ls-remote lines overwrote earlier ones: a commit that is the tip of both a topic branch and `main` (or carries a tag) collapsed to whichever refname was emitted last, and refname order puts refs/heads/main and refs/tags/* after refs/heads/kanban/<tid>-*. Keep every tip, narrow by the binding, refuse if still >1. `branch` is persisted provenance; a guess there is a false recovery-index entry. 4. _reusable restores the isinstance(ref, dict) guard every other previous["refs"] reader applies (_unshrunk, the cleanup carry-forward, _merge_refs, _vouched_repositories). AttributeError is in NEITHER preserve()'s nor remove_workspace_dir()'s except tuple, so a non-dict entry escaped past _hold() and skipped the fail-closed contract. Hoisted into `_bound()` so completion and reclamation cannot diverge again. A malformed entry is treated as UNBOUND, not skipped: fail closed on junk. Verified: - tests/hermes_cli/test_kanban_survivor_848_findings.py (new, 17 tests): 17 passed. Re-run with hermes_cli/ stashed back to 8300bd7: 11 failed, 4 passed -- the 4 are the anti-vacuity controls, green in both directions by design. Every substantive test is RED on the merged head. - Each expectation is computed independently of the function under test: the remote is a subprocess.run fixture fixed by construction, consequences are bytes written and read back in the test, and finding 4's core test asserts an ORDER-INVARIANCE (both ls-remote orderings must agree) rather than a snapshot of the current answer. - 5 pre-existing tests in test_kanban_survivor_binding.py encoded the mention-completes-the-card behaviour finding 3 declares wrong; rewritten to the new contract, with the operator-override arm kept as anti-vacuity. - Full kanban selection, env-scrubbed, this branch: 18 failed, 1241 passed, 9 skipped, 40 errors. Clean-room worktree at base a309b9d: 18 failed, 1222 passed, 9 skipped, 40 errors. The 58 FAILED/ERROR names diff EMPTY between the two -- every red is inherited from main; this branch adds +19 passing and zero new failures.
…open it Argus round 1 on t_99d93499 blocked #886 with one new P1 introduced by the finding-4 fix. verify_ref narrowed `tips` by the binding and refused on `len(tips) != 1`. On the UNBOUND path (`mined_for=None`) nothing narrows, so EVERY multi-tip SHA was refused -- and that path is the one documented remedy for exactly this shape: a fast-forward that left the topic branch alive beside main. Argus measured 86 of 2009 distinct OIDs (4.3%) on this project's own remote carrying more than one tip, 80 of them with 2+ refs/heads. Two harms, both closed: 1. NO REMEDY. The operator had already played --survivor-unbound and had no further move: bare --survivor-ref, --survivor-ref --survivor-unbound and the qualified repo=<claim> + --survivor-unbound repo form all refused. That is the unreachable-remedy bug _qualified_hint exists to prevent. The unbound path now RESOLVES a multi-tip SHA: there is no binding to be a guess about, the operator supplied the relevance, the override records the uid that authorised it, and every tip is the same commit. `branch` takes the lexicographically first tip -- deterministic, not emission-ordered -- and the ambiguity is RECORDED in a new `tips` field rather than hidden, so the recovery-index entry stays auditable. `tips` is absent when there is none. 2. FALSE DIAGNOSTIC. The remote answered with two lines, but the refusal said "could not verify against the remote", and the discrimination at kanban_survivor.py:603 re-called verify_ref without mined_for, which also returned None -- so the honest "is live but does not name <card>" branch never fired and override_hint was never attached. The inverse of the t_de2e348e class this module already refuses: there a non-answer reported as a verdict, here a verdict reported as a non-answer. The bound path's refusal (2+ tips that BOTH name the card) is unchanged in substance and correct; only its shape moved. It now raises a new AmbiguousRef -- a THIRD outcome beside None and RemoteUnavailable, because collapsing it into either one loses the only honest thing the caller can say. AmbiguousRef subclasses ValueError so a call site that ever forgets to handle it still HOLDs fail-closed through preserve()'s except tuple instead of escaping past _hold(), which is the escape class this card's finding 1 was about. discover() catches it and continues: mining states no reason, so an ambiguity there is just "not this candidate". The refusal now names the tips and keeps the hint, because dropping the binding IS the remedy for an ambiguity the binding created. VERIFIED: - tests/hermes_cli/test_kanban_survivor_848_findings.py: 24 passed (17 from round 1 + 7 new, all driving the UNBOUND arm Argus asked for -- the original 17 all drove mined_for=<tid> or verify_pr, which is why 184 green survivor tests and 38 green CI checks missed the regression). - Stashing hermes_cli/ back to the blocked head 3278e5e and re-running: 5 failed, 19 passed. The new tests that stay green there are the anti-vacuity/regression controls (RemoteUnavailable still reported as unreachable; an unambiguous unbound claim records no tips; discover skips), green both directions by design. - tests/hermes_cli -k survivor: 191 passed. - Full tests/hermes_cli, branch: 156 failed, 8707 passed, 10 errors. Clean-room worktree at base a309b9d: 156 failed, 8681 passed, 10 errors. The 156 FAILED name sets diff EMPTY; the 10 teardown-ERROR sets diff EMPTY. Zero new failures, +26 passing. Touches kanban_external_survivor.py and kanban_survivor.py only -- kept off kanban_db.py per the t_4b247923 collision warning, as Argus asked.
Collaborator
Author
|
🤖 merged-by: apollo · lane: fr-pause-0922 · gate: BYPASS: FR PAUSED by Ace ruling 2026-09-22 (state/fleetreview-pause-20260922.md); t_99d93499 approved · why: argus APPROVED r2 EXECUTION (close the 4 P1 FleetReview record findings); CI green; Apollo merge pass 2026-09-22 |
Kyzcreig
added a commit
that referenced
this pull request
Sep 23, 2026
fork/main advanced 83 commits since 0dc715c, including the survivor stack (#837 #842 #848 #856 #872 #879 #886 #888) which rewrote kanban_survivor.py from 475 to 1516 lines. Four conflicts, all in kanban_survivor.py, resolved toward main's shapes: _git union: main's input= (needed by _present_commits/_rev_list) plus this PR's timeout= (needed by _content_advisory's 120s fetch). _capture kept main's extraction; folded this PR's canonical-tree fallback + mirror_hint advisory INTO it, so the _explain_broken_object_store classifier still wraps every object-reading step. ref arm kept main's 'not bundles and len(refs) == len(repos) + len(carried)' (the #842/#848 carried-survivor accounting) and this PR's canonical sidecar. The pre-#848 unbound-claim path is NOT reintroduced: _verified_explicit, _unbound_keys, _reusable and _bound are main's, untouched. The landed arm in preserve() and _verify_landed/_landed_contains_history merged without conflict. Verified: 11 survivor files (4 from this PR + 7 landed since): 255 passed, 0 failed the PR's own 4 files: 98 passed, 0 failed mutation, dirty-tree guard neutered: 16 passed, 2 FAILED mutation, history binding neutered: 15 passed, 3 FAILED ruff on kanban_survivor.py + kanban_db.py: clean git diff --check: clean
Kyzcreig
added a commit
that referenced
this pull request
Sep 23, 2026
…requires (t_c70dac5c) #875 was a 6-commit stack whose first five were older copies of what #842/#848/#886 landed; only c0340b1 (the complete TOOL accepting a qualified --survivor-pr claim) was new. Re-ported that one commit linearly onto main (clean cherry-pick, 2 files, +192/-13). Its remote_multi fixture predates #848's rule that a live PR must name the card it vouches for, so 3 of its 5 new tests failed on main with "is live but does not name t_..."; the stub now answers headRefName=operator/<card>-landed-elsewhere like the single-repo fixture in the same file. tests/tools/test_kanban_tool_survivor.py 16/16. Apollo merge pass 2026-09-23.
This was referenced Sep 23, 2026
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
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.
Close-out for the four unresolved P1 a trusted 5-member FleetReview record reported on #848's head
0757be86(recorded 15:06:04Z), one hour before #848 merged as squash8300bd7513. New branch off main; #848 is not reopened.Two of the four re-opened the exact hole #848 was written to close.
What changed
1 — a MENTION no longer satisfies the completion path (
_verified_explicit)A title/body match was marked
unboundand still returned as an explicit survivor._reusableguards the recorded row on thecleanup=Truepass only, and_externaltakes itsexplicitarm before thecleanuparm — so_reusablestructurally never saw it. "follow-up to t_…; does not address it" authorised a completion, andpreservetreats a verified explicit survivor as authority over unpushed work. A mention now takes the same refusal an unrelated live claim takes, behind the same documented override. Completion and reclamation now answer the question identically.2 —
--survivor-unboundis per-claimIt was one parameter applied uniformly (
mined_for=None if unbound else task_id), so an operator overriding one repository's claim silently accepted every sibling with no check that it names the card — and stampedunbound=Trueon all of them, costing the correctly-bound ones their reclamation authority. New_unbound_keysnormalises to(bare, keys): a bare flag covers a single-claim invocation, is refused on a multi-claim one, and an override naming no claim is refused. CLI flag is nowappend/nargs="?".3 —
verify_refrefuses an ambiguous SHA instead of picking a tipmatcheswas keyed by oid, so laterls-remotelines overwrote earlier ones. A commit that is the tip of both a topic branch andmain(an ordinary fast-forward with the branch left alive), or that carries a tag, collapsed to whichever refname was emitted last — and refname order putsrefs/heads/mainandrefs/tags/*afterrefs/heads/kanban/<tid>-*. Now: keep every tip, narrow by the binding, refuse if still >1.branchis persisted provenance; a guess there is a false recovery-index entry.4 —
_reusablerestores theisinstance(ref, dict)guardEvery other
previous["refs"]reader applies it (_unshrunk, the cleanup carry-forward,_merge_refs,_vouched_repositories).AttributeErroris in neitherpreserve()'s norremove_workspace_dir()'sexcepttuple, so a non-dict entry escaped past_hold()and skipped the fail-closed contract entirely. Hoisted into_bound()so the two paths cannot diverge again. A malformed entry is treated as UNBOUND rather than skipped: fail closed on junk.Verification
tests/hermes_cli/test_kanban_survivor_848_findings.py(new, 17 tests) — 17 passed.Re-run with
hermes_cli/stashed back to8300bd7513: 11 failed, 4 passed. The 4 are the anti-vacuity controls, green in both directions by design. Every substantive test is RED on the merged head.Each expectation is computed independently of the function under test: the remote is a
subprocess.runfixture fixed by construction, consequences are bytes written and read back in the test, and finding 3's core test asserts an order-invariance (bothls-remoteorderings must agree) rather than a snapshot of the current answer.5 pre-existing tests in
test_kanban_survivor_binding.pyencoded the mention-completes-the-card behaviour finding 1 declares wrong; rewritten to the new contract, with the operator-override arm kept as anti-vacuity.Inherited-red proof. Full kanban selection, env-scrubbed:
a309b9d8f1The 58
FAILED/ERRORtest names diff empty between the two. Every red is inherited from main; this branch adds +19 passing and zero new failures.Note for whoever lands it
#875 and #845 are dirty against main after #842/#848; they rebase before review. #866 is still stacked on t_1bcd8aec's branch.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.