Skip to content

test(kanban): pin the cleanup relaxation's coverage decision per input shape - #845

Closed
Kyzcreig wants to merge 6 commits into
mainfrom
survivor/t_1bcd8aec-cleanup-coverage
Closed

Kyzcreig wants to merge 6 commits into
mainfrom
survivor/t_1bcd8aec-cleanup-coverage

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #842 (which is stacked on #837). Tests only — git diff hermes_cli/ is empty.

Found by @argus during round 1 of #842. #842's shipped behaviour is CORRECT on every point below;
this is a test-completeness gap. missing <= _vouched_repositories(previous) is the whole of what
lets a reaper DELETE a workspace whose recorded repository is gone, and it was pinned for exactly
one cell of its input matrix (ref-shaped survivor, total loss). Three mutations of the other cells
left all 78 survivor tests green.

What's pinned now

shape / loss pinned by
ref / total test_reclamation_consults_the_recorded_survivor_… (pre-existing)
ref / partial test_cleanup_partial_loss_keeps_the_recorded_ref_… (new)
bundle / total test_cleanup_accepts_a_bundle_shaped_survivor_… (new)
patch / any test_cleanup_rejects_a_patch_shaped_survivor_… (new)
none / wrong repo test_reclamation_still_holds_when_no_survivor_… (pre-existing)
  • ref/partial — if repos: carried = [...] (kanban_survivor.py:380-386) was reached by exactly
    one pre-existing test, which reads the COMPLETION snapshot and passes with the branch disabled.
    Disabled, the surviving repo satisfies the completion alone and the recorded ref for the vanished
    repo is silently dropped — preserve() SUCCEEDS with the lost work pointed at nothing.
  • patch/any — _vouched_repositories() excludes repositories because a patch is keyed by base
    SHA against a checkout. That was prose in a docstring. Widening the helper turns a fail-CLOSED hold
    into a reapable workspace whose only survivor is a patch against a base SHA that exists nowhere.
  • bundle/total — no test exercised a bundle-shaped recorded survivor at cleanup, so dropping
    bundles from the helper converts every bundle-backed reclamation into a permanent HOLD.

Measured

CPython 3.11.15 / pytest 9.1.1, isolated --basetemp, all four survivor files:

baseline #842 head 3ca4b600a3 : 78 passed
with these tests              : 81 passed
mutant  if repos -> if False  : 1 failed, 80 passed   (ref/partial test)
mutant  +repositories         : 1 failed, 80 passed   (patch test)
mutant  -bundles              : 1 failed, 80 passed   (bundle test)
mutant  -bundles, no new tests: 78 passed             <- the gap was real

Each mutant is caught by exactly its own test and nothing else. ruff clean.

Scope note — a live defect, carded not fixed

Enumerating the matrix surfaced a genuine behaviour defect on pristine code, not a test gap:
a bundle-shaped survivor under PARTIAL loss. carried collects only previous["refs"] while
_vouched_repositories() accepts bundles too — so the bundle passes the coverage test, carries
nothing, is erased from the record by _record(), and held_reason clears. Measured:

bases={"kept":<head>,"gone":<stale>}, survivor={"kind":"bundle","bundles":[{"repository":"gone",…}]}
  -> out = {"kind":"ref","refs":[{"repository":"kept",…}]}   # bundle for `gone` gone
  -> held = None                                              # workspace reapable

Filed as t_e41d3dd4 rather than fixed here (this card is explicitly test-only), and deliberately
NOT pinned — pinning current behaviour would freeze the bug.

Refs: t_1bcd8aec, t_a49e8a28


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 3/5

Findings

  • P1 hermes_cli/kanban_survivor.py:380 — Cleanup relaxation carries only refs, so a bundle-vouched vanished repo loses its survivor on partial loss
  • P1 hermes_cli/kanban_survivor.py:414 — A single operator ref releases the fail-closed guard for N vanished repos but is attributed to only one
  • P1 tests/hermes_cli/test_kanban_survivor_stale_bases.py:461 — Bundle-shaped test pins reapability on a fabricated bundle that was never stored
  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:434 — assert ws.is_dir() cannot fail: preserve() never deletes a workspace

FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $4.31 · duration: 23m 54s · rounds: 1 · files examined: 1

@Kyzcreig
Kyzcreig force-pushed the survivor/t_1bcd8aec-cleanup-coverage branch from 24145d2 to 67c99a5 Compare September 22, 2026 00:18
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 4/5

No actionable findings.


FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $3.83 · duration: 32m 35s · rounds: 3 · files examined: 1

Daedalus and others added 3 commits September 21, 2026 19:38
…ving from stale bases

A card that completed successfully via a verified --survivor-pr was left
status=done with its workspace permanently HELD. `_record()` writes the
survivor and NULLs `held_reason`; the workspace-removal path then re-enters
`preserve(cleanup=True)`, which re-ran the same `set(bases) - keys` check.
`bases` is never rewritten on success and a reaper passes no `explicit`, so
the check re-raised on a claim that had already been honoured and `_hold()`
rewrote the reason `_record()` had just cleared. The card cited the very
remedy the operator had used.

Remedy (b) of the two the card offered: on the cleanup pass, consult the
recorded survivor before re-deriving from `bases`. A survivor recorded at
completion is newer and better evidence than a checkout that has since
vanished. Chose (b) over (a) because clearing `bases` would destroy the
dispatch-time claim that an empty in-tree capture must still route through
`_loose_files()`; (b) leaves that claim intact and is keyed on coverage.

Not a `_hold` suppression: the relaxation applies only where the recorded
survivor actually vouches for the missing repository key (refs/bundles; a
patch is keyed by base SHA to a checkout, so it does not count). No survivor,
a survivor for a different repo, or a discoverable-but-unrecorded one all
still HOLD.

Verified:
- 78 passed across all four survivor test files on #837's head (baseline 72).
- mutation: revert the impl -> the 4 new assertions go red.
- adversarial mutation: the forbidden naive fix (suppress on `cleanup` alone)
  is caught red by 2 regression tests.
- PR #795's `_loose_files()` protection intact: loose non-repo evidence is
  still not reapable on a satisfied claim; the hold now names the loose files.
P1 #1 -- partial-loss reclamation deleted loose, unvouched evidence.
`_loose_files()` had exactly one call site, in the `elif claimed:` arm --
only reachable when the in-tree capture produced nothing. On a PARTIAL
loss `repos` is non-empty, the surviving repo resolves its own ref, and
`refs` is pre-seeded with `carried`, so the ref branch at line 457 is
satisfied and control returns before that arm. `_record()` then cleared
`held_reason` and `remove_workspace_dir` rmtree'd reviewer evidence that
pre-diff had forced a HOLD. Fixed at the CLASS, not the instance: the
unvouched-evidence check now sits at the top of the relaxation itself, so
every exit that can reach `rmtree` passes it, not just one arm.

P1 #2 -- a bundle-vouched missing repo was dropped from the rewritten
survivor. `_vouched_repositories()` deliberately counts `bundles`, but the
carry-forward copied only `previous["refs"]`. A repo whose only survivor
was a stored bundle satisfied `missing <= vouched`, `missing` emptied, and
`_record()` rewrote the row as `kind: "ref"` -- the recovery index then
claimed "all work is on a remote" for a card whose unpushed history lived
in an orphaned bundle attachment. Carry `bundles` alongside `refs`, and
gate the ref-kind branch on `not bundles` so a survivor still holding
unpushed history is never relabelled.

Verified (workspace worktree, isolated --basetemp; module path printed to
prove the workspace bytes loaded, not the live tree):
- all four survivor files: 81 passed / 0 failed (78 baseline + 3 new)
- revert impl, keep tests: 2 failed / 79 passed -- both new P1 tests RED
- mutation 5/5 KILLED:
  M1 `if missing <= vouched:` -> `if True:`            2 RED
  M2 delete the #795-routing `claimed = True`          2 RED
  M3 new partial-loss `_loose_files()` -> `if False:`  1 RED
  M4 drop the bundle carry-forward                     1 RED
  M5 revert the `not bundles` ref-branch guard         1 RED
- ruff clean; `git diff --check` clean
…3xP1 on 3afa908)

One class, three instances. The cleanup re-capture can only see repositories
still on disk, and `_record()` overwrites the survivor row with exactly what
it captured -- so anything the RECORDED survivor vouched for that is absent
from disk is dropped from the recovery index unless carried forward. The
previous fix keyed that carry-forward on `set(bases) - keys`, and `bases` is
written once, before dispatch.

P1 a (:392) -- a repo CLONED AFTER DISPATCH is in the survivor and in no
`bases` entry, so `missing` was empty, the relaxation never ran, and the
rewrite orphaned the bundle holding its only unpushed history. Re-keyed on
`absent = _vouched_repositories(previous) - keys`: what the survivor vouches
for and disk no longer holds. `missing <= absent` stays the coverage test, so
a survivor vouching for some other repository still buys nothing.

P1 b (:490) -- the no-survivor `else` wrote NULL over a recorded survivor and
returned a survivor-less verdict, letting the caller rmtree loose evidence the
recorded ref never covered. Reclamation now keeps `previous` there; only the
completion pass may legitimately record None.

P1 c (:432) -- the operator carry stamped ONE `--survivor-pr` onto
`sorted(missing)[0]` while the completeness test counted it as covering all of
them. One flag names one remote; stamping it on each records provenance false
for all but one. A multi-repository loss is now an incomplete claim and fails
closed, naming the repositories.

Verified (workspace worktree, isolated --basetemp, module path printed):
- all four survivor files: 87 passed / 0 failed (81 baseline + 6 new)
- 4 of the 6 new tests RED before the impl; 2 are anti-vacuity controls
- ruff clean; git diff --check clean

Not adopted: the reporter's remedy for P1 c (carry the flag onto every missing
repo) would write false provenance deliberately. Refusing is the smaller
blast radius and keeps the single-loss remedy intact (test asserts both).
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 4/5

Findings

  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:504 — test_cleanup_partial_loss_keeps_the_recorded_ref_... duplicates an existing test; its docstring's premise is factually wrong
  • P1 tests/hermes_cli/test_kanban_survivor_stale_bases.py:593 — Bundle-shaped reclaim test pins the ABSENCE of any attachment verification
  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:584 — assert ws.is_dir() cannot fail; "must not be reapable" is never actually exercised
  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:489 — _seed_repo duplicates the existing _seed_published_repo helper

FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $4.63 · duration: 13m 42s · rounds: 1 · files examined: 1

@Kyzcreig
Kyzcreig force-pushed the daedalus-opus/t_a49e8a28-survivor-hold branch from 3afa908 to 7afe1b9 Compare September 22, 2026 02:52
@Kyzcreig
Kyzcreig force-pushed the survivor/t_1bcd8aec-cleanup-coverage branch from 006d395 to f4467cf Compare September 22, 2026 02:55
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

PARTIAL — ensemble escalated: judge transient failure

This review did not reach a trusted verdict, so it is not a gate pass and the findings below may be incomplete. They are posted so they can be read rather than lost in a terminal record.

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 1/5

Findings

  • P1 hermes_cli/kanban_survivor.py:432 — Partial carry
  • P1 hermes_cli/kanban_survivor.py:303 — A recorded bundle vouches for a vanished repo without checking the bundle still exists — reclamation then rmtrees the only other copy
  • P1 hermes_cli/kanban_survivor.py:392 — Uncarried survivor
  • P1 hermes_cli/kanban_survivor.py:460 — Dropped refs
  • P1 tests/hermes_cli/test_kanban_survivor_stale_bases.py:49 — Total-loss completion is only pinned for the base key "." — the one key that accidentally matches the hardcoded repository: ".", so the headline "done + permanently HELD" regression stays reachable and green

FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $14.69 · duration: 1h 36m 09s · rounds: 3 · files examined: 2

daedalus-opus and others added 3 commits September 21, 2026 21:47
…t shape

`missing <= _vouched_repositories(previous)` is the whole of what lets a reaper
DELETE a workspace whose recorded repository is gone. It was pinned for ONE cell
of its input matrix: a ref-shaped survivor under total loss. Three mutations of
the other cells left all 78 survivor tests green.

Tests only; no production change (`git diff hermes_cli/` is empty).

  ref/partial  -- `if repos: carried = [...]` (kanban_survivor.py:380-386) was
    reached by exactly one PRE-EXISTING test, which reads the COMPLETION
    snapshot and passes with the branch disabled. Disabled, the surviving repo
    satisfies the completion alone and the RECORDED ref for the vanished repo is
    silently dropped -- preserve() SUCCEEDS with the lost work pointed at
    nothing. Now driven through the CLEANUP path, asserting both refs survive.

  patch/any -- `_vouched_repositories()` excludes `repositories` because a patch
    is keyed by base SHA against a checkout. That was prose in a docstring.
    Widening the helper to collect `repositories` turns a fail-CLOSED hold into
    a reapable workspace whose only survivor is a patch against a base SHA that
    exists nowhere. Now pinned, with a direct shape assertion as teeth.

  bundle/total -- no test exercised a bundle-shaped recorded survivor at
    cleanup, so dropping `bundles` from the helper converts every bundle-backed
    reclamation into a permanent HOLD. Verified: that mutant passes 78/78 on the
    unmodified suite. Now pinned.

Verified (CPython 3.11.15, pytest 9.1.1, isolated --basetemp, four survivor
files):

  baseline #842 head 3ca4b60 : 78 passed
  with these tests             : 81 passed
  mutant if repos -> if False  : 1 failed, 80 passed (ref/partial test)
  mutant +repositories         : 1 failed, 80 passed (patch test)
  mutant -bundles              : 1 failed, 80 passed (bundle test)
  mutant -bundles, no new tests: 78 passed  <- the gap was real

Each mutant is caught by exactly its own test and nothing else. ruff clean.

Scope note: enumerating the matrix surfaced a LIVE defect, not a test gap --
bundle-shaped survivor under PARTIAL loss. `carried` collects only
`previous["refs"]` while `_vouched_repositories()` accepts bundles too, so the
bundle passes the coverage test, carries nothing, gets erased from the record,
and the hold clears. Measured on pristine code; carded as t_e41d3dd4 rather than
fixed here, and deliberately NOT pinned -- pinning it would freeze the bug.

Refs: t_1bcd8aec (found by argus on t_a49e8a28 / PR #842 round 1)
…exits

Round-2 class sweep on the cleanup relaxation, re-derived over THREE axes
(survivor shape x loss extent x workspace content) after #842 r2 landed both
FleetReview P1 fixes. The omitted axis in round 1 was workspace content.

Measured, not inferred: a 30-cell probe over
{ref,bundle,patch,mixed,none} x {total,partial,partial-dirty} x {clean,loose}
at 3afa908 found one unpinned family -- the `_loose_files()` guard is
exercised only by REF-shaped survivors. Waiving it for `kind == "bundle"` at
the relaxation's own exit flips four cells (bundle|partial|loose,
mixed|partial|loose, and both partial-dirty variants) from fail-closed HOLD to
a reclaim that rmtree's `qa-output/verdict.md`, and leaves all 84 tests GREEN.

Two tests added:
  test_partial_loss_holds_for_loose_evidence_under_a_bundle_shaped_survivor
  test_a_bundle_shaped_survivor_does_not_buy_a_delete_for_loose_evidence

Verified: 86 passed across the four survivor files. Mutation gate
(mutation_gate_r2.sh) -- M4 (relaxation exit waived for bundles) kills the
partial test; M5 (the `elif claimed:` arm waived) is an EQUIVALENT mutant,
traced with sys.settrace: the total-loss bundle+loose case raises at the first
site and never reaches the second, so it flips zero cells alone. The combined
M4+M5 mutant kills both tests. Source restored pristine after every arm.

Test-only: zero lines of hermes_cli/kanban_survivor.py changed.
…ping

`missing <= vouched` is the whole of what lets a reaper delete a workspace
whose recorded repository is gone. Every existing test loses exactly ONE
repository, where the subset test and an intersection test agree — so
relaxing it to `missing & vouched` (or `missing & vouched or missing <=
vouched`) left all 86 tests on the four survivor files, and all 151 on the
wider survivor surface, GREEN while flipping two cells from fail-closed HOLD
to reapable.

Adds the discriminating shape: two recorded repos gone, the recorded survivor
vouching for only one. The unvouched repo has no ref, no bundle and no patch
anywhere, so under the mutant its unpushed work is pointed at nothing after
the reap. Pinned on both the total and partial loss arms, plus a full-coverage
teeth case proving the assertion keys on TOTAL coverage rather than on
multi-repo loss itself.

Measured (mutation_gate_r3.sh, source restored + `git diff --quiet` verified
after every arm):
  P1 `missing & vouched or missing <= vouched` -> 2 failed, 1 passed
  P2 `missing & vouched`                       -> 2 failed, 1 passed
  P0 pristine                                   -> 3 passed
Four survivor files: 89 passed. Wider surface (7 files): 154 passed.
ruff clean. Zero lines of kanban_survivor.py changed — behaviour untouched.
@Kyzcreig
Kyzcreig force-pushed the survivor/t_1bcd8aec-cleanup-coverage branch from f4467cf to 7c3e0b6 Compare September 22, 2026 04:53
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

FleetReview

Reviewed with 2 of 3 model families — openai unavailable.

Confidence: 3/5

Findings

  • P1 hermes_cli/kanban_survivor.py:414 — Reclamation still shrinks the recovery index for a patch/sidecar-shaped survivor; the new "patch / any" test only pins the fail-closed half
  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:705 — test_cleanup_partial_loss_... duplicates an existing test; its docstring rationale is factually wrong
  • P3 tests/hermes_cli/test_kanban_survivor_stale_bases.py:690 — _seed_repo duplicates the existing _seed_published_repo helper

FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $5.20 · duration: 11m 51s · rounds: 1 · files examined: 1

@Kyzcreig
Kyzcreig force-pushed the daedalus-opus/t_a49e8a28-survivor-hold branch from 7afe1b9 to 0062a34 Compare September 22, 2026 07:33
@Kyzcreig
Kyzcreig force-pushed the daedalus-opus/t_a49e8a28-survivor-hold branch 2 times, most recently from 1e11414 to 1a009ae Compare September 22, 2026 13:08
Base automatically changed from daedalus-opus/t_a49e8a28-survivor-hold to main September 22, 2026 13:50
Kyzcreig added a commit that referenced this pull request Sep 22, 2026
The carded defect (t_e41d3dd4) is ALREADY FIXED: #842 r2 (a61dda5) added
`carried_bundles` and gated the ref branch on `not bundles`. Both shapes the
card measured -- bundle-only partial loss and mixed partial loss -- were driven
through `preserve(cleanup=True)` on pristine #845 head and both already carry
the bundle and record `kind: "bundle"`. No behaviour change is needed.

What IS missing is a gate. `carried` and `carried_bundles` are independent
collections read from the same recorded survivor, and every existing
partial-loss test loses a repository vouched for by exactly ONE of them. So a
mutant that keeps the bundle carry-forward but disables it whenever a ref is
also carried -- `carried_bundles = [] if carried else [...]` -- leaves all 30
tests on this file GREEN while the MIXED shape drops the bundle and relabels
the record `kind: "ref"`, telling an operator the work is pushed when its only
copy is an orphaned bundle attachment.

Measured on this tree, pre-existing suite only:
  M1 `carried_bundles = []`                -> 3 failed / 28 passed (already gated)
  M2 `[] if carried else [...]`            -> 0 failed / 30 passed  SURVIVES
With this test: M1 3 failed, M2 1 failed (killed only by the new test).

Test-only: zero lines of hermes_cli/kanban_survivor.py; impl byte-identical to
the pristine base. Four survivor files at this head: 95 passed before, 96 after.

(cherry picked from commit 1e6339b)
Kyzcreig added a commit that referenced this pull request Sep 23, 2026
The carded defect (t_e41d3dd4) is ALREADY FIXED: #842 r2 (a61dda5) added
`carried_bundles` and gated the ref branch on `not bundles`. Both shapes the
card measured -- bundle-only partial loss and mixed partial loss -- were driven
through `preserve(cleanup=True)` on pristine #845 head and both already carry
the bundle and record `kind: "bundle"`. No behaviour change is needed.

What IS missing is a gate. `carried` and `carried_bundles` are independent
collections read from the same recorded survivor, and every existing
partial-loss test loses a repository vouched for by exactly ONE of them. So a
mutant that keeps the bundle carry-forward but disables it whenever a ref is
also carried -- `carried_bundles = [] if carried else [...]` -- leaves all 30
tests on this file GREEN while the MIXED shape drops the bundle and relabels
the record `kind: "ref"`, telling an operator the work is pushed when its only
copy is an orphaned bundle attachment.

Measured on this tree, pre-existing suite only:
  M1 `carried_bundles = []`                -> 3 failed / 28 passed (already gated)
  M2 `[] if carried else [...]`            -> 0 failed / 30 passed  SURVIVES
With this test: M1 3 failed, M2 1 failed (killed only by the new test).

Test-only: zero lines of hermes_cli/kanban_survivor.py; impl byte-identical to
the pristine base. Four survivor files at this head: 95 passed before, 96 after.

(cherry picked from commit 1e6339b)
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

apollo/merge-pass 2026-09-23: re-ported LINEARLY onto main — this branch was a stack whose lower commits were older copies of work already landed (#842/#848/#886 or #879/#892); only the commit(s) genuinely new to main were cherry-picked (-x) onto origin/main. No content change to those commits. Landing via fleet-merge on the FR-pause lane once CI is green.

@Kyzcreig
Kyzcreig force-pushed the survivor/t_1bcd8aec-cleanup-coverage branch 2 times, most recently from 2750dba to 7c3e0b6 Compare September 23, 2026 12:25
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

apollo/merge-pass 2026-09-23: CLOSED as landed-by-content. Measured against origin/main: every one of the 37 test functions this branch defines in tests/hermes_cli/test_kanban_survivor_stale_bases.py already exists on main by name (0 missing); the 6 whose bodies differ are OLDER shapes of fixtures/tests that #848/#886 since rewrote (the remote stub now names the card). My earlier linear re-port attempt appended them and main's shadowed-definition lint correctly refused (slice 15/16). Branch head restored to the reviewed 7c3e0b6 so the card's survivor ref stays valid. Card t_1bcd8aec is done.

@Kyzcreig Kyzcreig closed this Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant