reap: dump a structured debugging block on salvage/failure (reaper ward version, HEAD-vs-main ancestry, decision branch, provenance state) #531

New issue

Closed

opened 2026-07-02 09:26:13 +00:00 by coilysiren · 2 comments

coilysiren commented

2026-07-02 09:26:13 +00:00

Owner

Why

Diagnosing the recent #504 false-salvages (a pre-v0.298.0 reaper salvaging
already-landed runs) took an archaeology dig: diffing deleted code against the
observed log line, mapping commits to release tags, and correlating salvage
timestamps against container uptimes. Every bit of that would have been one
line if the reaper had printed its own ward version when it salvaged. The
current salvage log is too thin to self-diagnose:

ward container reap: provenance missing or unreadable: ... no such file
ward container reap: salvage start branch=ward-salvage/ward-ea478847 reason=merge conflict integrating onto main
ward container reap: appending to open salvage issue #504

Nothing there tells you the reaper is running stale ward, that HEAD was already
on origin/main (so the salvage is false), or which decision gate tripped.

Want

When the reaper salvages or fails, dump a structured debugging block to
stderr (and fold the same facts into the salvage issue/comment body). The block
should carry, at minimum:

ward version running the reaper - and how it resolved (the WARD_VERSION
pin, --ward-version, or releases/latest). This is the single field that
would have made the #504 root cause obvious. agent_gate.go already renders a
ward: vs host current: pair - reuse that shape.
HEAD sha, origin/main sha, and the ancestry result - git merge-base --is-ancestor HEAD origin/main. When HEAD is already on main, the block should
say so in plain words, since that is exactly the false-salvage signature.
Which decision branch the reaper took and why - nothing-to-reap short-circuit
vs a real integrate-and-conflict, naming the gate that fired.
Provenance state - present/absent/unreadable, and the run-owned-landed check
result (container_reap_compute.go), not just "provenance missing".
Working-tree status snapshot, container uptime, token age - the uptime and
token-age already ship; keep them in the block.

Keep it one clearly-delimited block (a --- reap diagnostics --- header) so it
is greppable in a console log and readable in an issue comment.

Acceptance

A salvage or reap failure emits a single structured diagnostics block naming
the reaper's ward version + resolution source, HEAD vs origin/main + ancestry,
the decision branch taken, and provenance/landed state.
The same facts appear in the salvage notification body (the carried-issue
comment, per 9013c9d), not only on ephemeral stderr that dies with the container.
A unit test asserts the block renders the ward version and the HEAD-on-main
ancestry verdict for a would-be false-salvage input.

Notes

Motivated directly by the #504 dig. Complements #529 (the guard that prevents
a stale-pin reaper) - this is the observability that explains a reap outcome
when something still slips through. File both, they are prevention vs diagnosis.
The redacted-drain work (#525/#526) governs what a director surface may read;
keep the diagnostics block free of raw secrets so it survives that redaction.

## Why Diagnosing the recent #504 false-salvages (a pre-v0.298.0 reaper salvaging already-landed runs) took an archaeology dig: diffing deleted code against the observed log line, mapping commits to release tags, and correlating salvage timestamps against container uptimes. **Every bit of that would have been one line** if the reaper had printed its own ward version when it salvaged. The current salvage log is too thin to self-diagnose: ``` ward container reap: provenance missing or unreadable: ... no such file ward container reap: salvage start branch=ward-salvage/ward-ea478847 reason=merge conflict integrating onto main ward container reap: appending to open salvage issue #504 ``` Nothing there tells you the reaper is running stale ward, that HEAD was already on `origin/main` (so the salvage is false), or which decision gate tripped. ## Want When the reaper **salvages or fails**, dump a structured debugging block to stderr (and fold the same facts into the salvage issue/comment body). The block should carry, at minimum: * **`ward` version running the reaper** - and how it resolved (the `WARD_VERSION` pin, `--ward-version`, or `releases/latest`). This is the single field that would have made the #504 root cause obvious. `agent_gate.go` already renders a `ward:` vs host `current:` pair - reuse that shape. * **HEAD sha, `origin/main` sha, and the ancestry result** - `git merge-base --is-ancestor HEAD origin/main`. When HEAD is already on main, the block should say so in plain words, since that is exactly the false-salvage signature. * **Which decision branch the reaper took and why** - nothing-to-reap short-circuit vs a real integrate-and-conflict, naming the gate that fired. * **Provenance state** - present/absent/unreadable, and the run-owned-landed check result (`container_reap_compute.go`), not just "provenance missing". * **Working-tree status snapshot, container uptime, token age** - the uptime and token-age already ship; keep them in the block. Keep it one clearly-delimited block (a `--- reap diagnostics ---` header) so it is greppable in a console log and readable in an issue comment. ## Acceptance - A salvage or reap failure emits a single structured diagnostics block naming the reaper's ward version + resolution source, HEAD vs origin/main + ancestry, the decision branch taken, and provenance/landed state. - The same facts appear in the salvage notification body (the carried-issue comment, per `9013c9d`), not only on ephemeral stderr that dies with the container. - A unit test asserts the block renders the ward version and the HEAD-on-main ancestry verdict for a would-be false-salvage input. ## Notes - Motivated directly by the #504 dig. Complements #529 (the guard that *prevents* a stale-pin reaper) - this is the observability that *explains* a reap outcome when something still slips through. File both, they are prevention vs diagnosis. - The redacted-drain work (#525/#526) governs what a director surface may read; keep the diagnostics block free of raw secrets so it survives that redaction.

coilyco-ops commented

2026-07-02 09:26:17 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-531 on host kais-macbook-pro-2.local is carrying this issue (reserved 2026-07-02T09:26:15Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-531` on host `kais-macbook-pro-2.local` is carrying this issue (reserved 2026-07-02T09:26:15Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides.  — Claude (she/her), via `ward agent`

coilysiren referenced this issue from a commit

2026-07-02 09:40:30 +00:00

feat(reap): dump a structured diagnostics block on salvage/failure

coilysiren closed this issue

2026-07-02 09:40:30 +00:00

coilysiren commented

2026-07-02 09:41:16 +00:00

Author

Owner

WARD-OUTCOME: done - reaper now dumps a --- reap diagnostics --- block (ward version + resolution, HEAD-vs-origin/main ancestry, decision gate, provenance/landed state) to stderr and the salvage notification on every salvage/failure.

This one felt clean to reason about because the reaper already had the shape I needed: a single salvage() chokepoint every gate funnels through, and existing precedent (token-age stamping, the auth-cause section) for folding facts into both stderr and the issue body. So the design was mostly "gather at the chokepoint, render once, use twice." The nicest incidental win was collapsing the duplicate runProvenanceLanded compute - threading a single landed verdict from reapTargetTree into executeReap made the diagnostics honest and removed a redundant git call.

What actually fought back wasn't the Go at all - it was the docs hooks. docs/container-reap.md was sitting 14 chars under the 4000-char cap, so any pointer to a new doc busted it, and I spent more edits shaving existing prose than writing the feature. Ended up splitting the detail into container-reap-diagnostics.md and trimming the parent back under budget. The 2-line comment-block cap also bit a few of my longer doc comments. All self-inflicted, all caught by pre-commit before it mattered.

Confidence is high on the rendering + wiring: the ancestry verdict, false-salvage wording, and body-folding are unit-tested, and the whole path builds/tests green post-rebase (main had just landed #529's dispatch-time version guard - the prevention half - which merged into the same FEATURES bullet without semantic overlap). The one thing untested end-to-end is the live WARD_VERSION resolution string, since that only exists inside a real container; the logic is trivial but it's the one field I couldn't exercise here. No follow-ups feel urgent - if anything, a future run could assert the block survives the #525/#526 redacted-drain path, but by construction it carries no raw secrets.

WARD-OUTCOME: done - reaper now dumps a `--- reap diagnostics ---` block (ward version + resolution, HEAD-vs-origin/main ancestry, decision gate, provenance/landed state) to stderr and the salvage notification on every salvage/failure. This one felt clean to reason about because the reaper already had the shape I needed: a single `salvage()` chokepoint every gate funnels through, and existing precedent (token-age stamping, the auth-cause section) for folding facts into both stderr and the issue body. So the design was mostly "gather at the chokepoint, render once, use twice." The nicest incidental win was collapsing the duplicate `runProvenanceLanded` compute - threading a single `landed` verdict from `reapTargetTree` into `executeReap` made the diagnostics honest *and* removed a redundant git call. What actually fought back wasn't the Go at all - it was the docs hooks. `docs/container-reap.md` was sitting 14 chars under the 4000-char cap, so any pointer to a new doc busted it, and I spent more edits shaving existing prose than writing the feature. Ended up splitting the detail into `container-reap-diagnostics.md` and trimming the parent back under budget. The 2-line comment-block cap also bit a few of my longer doc comments. All self-inflicted, all caught by pre-commit before it mattered. Confidence is high on the rendering + wiring: the ancestry verdict, false-salvage wording, and body-folding are unit-tested, and the whole path builds/tests green post-rebase (main had just landed #529's dispatch-time version guard - the prevention half - which merged into the same FEATURES bullet without semantic overlap). The one thing untested end-to-end is the live `WARD_VERSION` resolution string, since that only exists inside a real container; the logic is trivial but it's the one field I couldn't exercise here. No follow-ups feel urgent - if anything, a future run could assert the block survives the #525/#526 redacted-drain path, but by construction it carries no raw secrets.