Conversation
…ed escapes)
A home-clone workspace could never complete. ~/.hermes's only durable remote
is the hermes-home mirror, written by the isolated remote sync, which
REPUBLISHES EVERY COMMIT UNDER A NEW SHA (live: local 9f25d7cce ==
remote e747d18db, same subject, different hash). _remote_survivor() compares
shas, so it can never match; the ref path is unreachable by construction, the
capture falls to patch/bundle, and a 94 MB home tree always trips
KANBAN_ATTACHMENT_MAX_BYTES — blocking `complete` on work already committed and
running (t_e69d693a, blocked two sessions).
git patch-id is the content identity that survives the rewrite. Two escapes,
both fail-closed:
- _content_survivor(): when sha equality fails, shallow-fetch each durable head
into a throwaway bare repo borrowing the workspace's objects and look for a
commit with an identical patch-id. A hit records kind="ref-by-content" with
BOTH shas plus an implementation.json sidecar, instead of attempting a bundle.
The deepest fetched commit is a shallow boundary — git diff-tree --root there
yields the whole tree as an addition, which collides with any unpublished root
commit of the same content — so it is fetched but never scanned.
- metadata `landed` = [{repo_path, sha}]: an explicit claim that the work lives
in a repo the workspace only mirrors. Verified, never trusted: the sha must
resolve, be reachable from that repo's HEAD, AND be published on a durable
remote by sha or by patch-id. Every other outcome raises and holds the
workspace — accepting the claim authorises deleting the only copy.
Persistence is factored into one _record() choke point so the landed path
cannot drift from the normal path. complete_task's survivor note is now a
branch per kind (it indexed survivor['path'] and would KeyError on both new
kinds).
Verified:
- scripts/run_tests.sh over the full kanban surface (71 files): 841 passed,
0 failed, incl. all 37 pre-existing survivor tests.
- 12 new tests, mutation-proved — each guard REDs when removed: patch-id
comparison (3 RED), HEAD-reachability (1 RED), publication requirement
(1 RED), shallow-boundary exclusion (1 RED, real collision: an unpublished
commit blessed as published).
- Live against the reported environment (~/.hermes, 310 published heads):
sha path None, content path HIT 9f25d7cce -> e747d18db via patch-id in 2s.
Follow round-2 canonical-live-tree ruling. Fix inherited fixtures and superseded mirror-publication assertions. Landed escape precedes nested-repo scanning; cleanup revalidates canonical reachability. Verified live-tree suite 12 passed and landed suite 12 passed via scripts/run_tests.sh.
Kyzcreig
changed the title
Collaborator
Author
FleetReviewConfidence: 1/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-5, F=gpt-5.6-sol, G=grok-4.6 · cost: $12.66 · duration: 29m 12s · rounds: 1 · files examined: 5 |
Kyzcreig
added
the
do-not-merge
Collaborator
Author
|
🛑 Do not merge as-is. FleetReview on 0dc715c closed COMPLETED with 9 unresolved P0/P1 (posted above) after kanban approval — the |
Reject dirty, unrelated, nested-unbound, promisor, partial, and incomplete landed repositories before cleanup. Keep credential-bearing remote URLs out of git fetch argv and re-pin recovery tests to verify restored bytes across patch and bundle artifacts.\n\nVerified: 98/98 survivor tests; 28/28 external-survivor authority tests; ruff clean.
This was referenced Sep 21, 2026
Merged
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
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.
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
…orward `_patch_pointers` treated ANY `sidecar` as a recovery-patch pointer. This PR added `sidecar` to two kinds that store no implementation bytes -- `landed` and the canonical `ref` arm -- so when their manifest changed between completion's capture and cleanup's revalidation, `_unshrunk` carried the first manifest forward as a patch: the durable recovery row was relabelled `kind: "patch"` with `notice: "NOT PUSHED"` and a `patches` list of JSON manifests holding no patch bytes, after the workspace had already been deleted. The index then promised a patch that does not exist. Gate the top-level slot on `_carries_recovery_artifact()` -- does the row point at real bytes (`path`, or `bundles`)? Discriminating on the ARTIFACT rather than on the `kind` label sweeps both new sidecar-bearing kinds at once, covers any later one, and cannot be fooled by a row `_unshrunk` already relabelled. Entries already displaced into `patches` are not re-gated: a bundle row's pointer legitimately carries only a `sidecar` once its `bundles` were merged into the fresh row. Genuine patch/bundle non-shrink is unchanged. Verified by execution: - Argus's end-to-end repro (real `complete_task` + cleanup, temp board, branch renamed between phases): RECOVERY_KIND landed / NOTICE None / PATCH_LIST None. With the guard removed: kind=patch, NOT PUSHED, phantom patches entry -- Argus's reported output exactly. - 3 new regressions in test_kanban_survivor_live_tree.py (landed two-phase, canonical-ref two-phase, real-patch-still-carried positive control); all 3 mutation-proven to FAIL when the guard is removed. - 11-file survivor suite: 258 passed, 0 failed (255 baseline + 3 new). - Targeted safety reproducers (unrelated/dirty/unbound_nested): 4 passed. - ruff clean; git diff --check clean.
Reproduced a patch-id whitespace collision deleting the only executable implementation. Verified 259 survivor tests via scripts/run_tests.sh across 11 files; ruff and diff checks pass.
Kyzcreig
marked this pull request as draft
September 23, 2026 12:09
Collaborator
Author
Verified reverted ancestor and rewritten-history completion refusals, positive unrelated live addition, and survivor suite (262 passed across 11 files).
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.
Round-2 contract (supersedes original patch-id escape)
Patch-id identifies a normalized diff, not an entire tree. The hermes-home mirror rewrites trees as well as SHAs, so matching it is never authority to delete a workspace.
Verification
scripts/run_tests.sh tests/hermes_cli/test_kanban*.py -q: 71 files, 804 passed, 0 failed, 3 skipped at 3103900.
Final test-only increment: scripts/run_tests.sh tests/hermes_cli/test_kanban_survivor_live_tree.py -q: 13 passed, 0 failed.
Mutation adding the advisory result back into ref selection: divergent-history regression RED (ref instead of patch/bundle); restored source and suite GREEN.
Review / upstream
Requires independent review before queueing; not self-merged. Prior implementation claimed patch-id was content identity; this PR now explicitly rejects that premise.
Upstream surface rechecked at dec236b: hermes_cli/kanban_survivor.py is absent. No upstream fix PR opened for this fork-only subsystem.