reap: guard against a stale ward-version pin false-salvaging landed runs (`9013c9d` recurs when the container reaps with pre-v0.298 ward) #529

New issue

Closed

opened 2026-07-02 09:21:29 +00:00 by coilysiren · 2 comments

coilysiren commented

2026-07-02 09:21:29 +00:00

Owner

Symptom

A ward container reaper can false-salvage a run that already landed and
re-grow the #504 salvage mega-issue - even though 9013c9d (v0.298.0) fixed
exactly that. Observed live: issue #525's run merged to main at 09:07 (merge
commit 85518c7, release run 751 fired off it), then its teardown reaper
salvaged the clean, already-landed tree to ward-salvage/ward-ea478847 and
printed appending to open salvage issue #504 - a code path 9013c9d deleted.
Current main has no such path.

Root cause: the reaper's ward version is not guarded against a stale pin

The reaper runs in-process inside the container's ward (container_bootstrap.go:963),
and that ward is whatever entrypoint.sh:236 install_ward() installs, resolved
from WARD_VERSION (entrypoint.sh:216). WARD_VERSION is pinned per-dispatch
from --ward-version, defaulting to the dispatching host ward's own Version
(container.go:97, agent_director.go:121). So when the host ward (or an
explicit --ward-version) is older than a reaper-correctness fix, every container
it dispatches installs that older ward and its reaper reproduces the old,
already-fixed bug. The #504 false-salvages cluster 08:13-09:08 on 2026-07-02,
right after 9013c9d landed at 07:46 but before the dispatching host picked it up.

Nothing warns or refuses when a dispatch pins a ward older than the host's
own, so a known-buggy reaper silently ships into the container.

Want

Guard the downgrade. A container should not silently reap with a ward older than
the dispatcher's own, since the reaper is the last line against lost/false-salvaged
work. Options for the implementer to weigh:

Refuse or loud-warn on a downgrade pin (preferred). At dispatch/gate time
(agent_gate.go already renders ward: vs current: host version), compare
the resolved WARD_VERSION against the host ward's Version. If the pin is
strictly older, refuse by default with a clear message, or require an explicit
--allow-ward-downgrade to proceed. An equal-or-newer pin is always fine.
Pin-forward default. When --ward-version is unset, prefer the host's own
Version over releases/latest only when the host is itself a release build,
so a current host never dispatches an older reaper by resolving latest to a
stale mirror.

Acceptance

A dispatch whose resolved WARD_VERSION is older than the host ward is refused
(or loudly warns + requires an opt-in flag), with a message naming both versions.
An equal-or-newer pin dispatches unchanged.
A unit test in agent_gate_test.go covers the downgrade-refused and
equal-or-newer-allowed cases.

Notes

Found from the director read-only surface while diagnosing why the #504
false-salvage recurrence "still looks live" after 9013c9d. The fix is real
and in main; the recurrence is stale reaper binaries from downgrade pins, not
a code regression.
Related: #504 (the salvage mega-issue this keeps feeding), #497 (single
dispatch's transient resolve error). Distinct from both.

## Symptom A `ward container` reaper can **false-salvage a run that already landed** and re-grow the #504 salvage mega-issue - even though `9013c9d` (v0.298.0) fixed exactly that. Observed live: issue #525's run merged to `main` at 09:07 (merge commit `85518c7`, release run 751 fired off it), then its teardown reaper salvaged the clean, already-landed tree to `ward-salvage/ward-ea478847` and printed `appending to open salvage issue #504` - a code path `9013c9d` deleted. Current `main` has no such path. ## Root cause: the reaper's ward version is not guarded against a stale pin The reaper runs in-process inside the container's ward (`container_bootstrap.go:963`), and that ward is whatever `entrypoint.sh:236` `install_ward()` installs, resolved from `WARD_VERSION` (`entrypoint.sh:216`). `WARD_VERSION` is pinned per-dispatch from `--ward-version`, defaulting to the **dispatching host ward's own `Version`** (`container.go:97`, `agent_director.go:121`). So when the host ward (or an explicit `--ward-version`) is older than a reaper-correctness fix, every container it dispatches installs that older ward and its reaper reproduces the old, already-fixed bug. The #504 false-salvages cluster 08:13-09:08 on 2026-07-02, right after `9013c9d` landed at 07:46 but before the dispatching host picked it up. Nothing warns or refuses when a dispatch pins a ward **older** than the host's own, so a known-buggy reaper silently ships into the container. ## Want Guard the downgrade. A container should not silently reap with a ward older than the dispatcher's own, since the reaper is the last line against lost/false-salvaged work. Options for the implementer to weigh: 1. **Refuse or loud-warn on a downgrade pin (preferred).** At dispatch/gate time (`agent_gate.go` already renders `ward:` vs `current:` host version), compare the resolved `WARD_VERSION` against the host ward's `Version`. If the pin is strictly older, refuse by default with a clear message, or require an explicit `--allow-ward-downgrade` to proceed. An equal-or-newer pin is always fine. 2. **Pin-forward default.** When `--ward-version` is unset, prefer the host's own `Version` over `releases/latest` only when the host is itself a release build, so a current host never dispatches an older reaper by resolving latest to a stale mirror. ## Acceptance - A dispatch whose resolved `WARD_VERSION` is older than the host ward is refused (or loudly warns + requires an opt-in flag), with a message naming both versions. - An equal-or-newer pin dispatches unchanged. - A unit test in `agent_gate_test.go` covers the downgrade-refused and equal-or-newer-allowed cases. ## Notes - Found from the director read-only surface while diagnosing why the #504 false-salvage recurrence "still looks live" after `9013c9d`. The fix is real and in `main`; the recurrence is stale reaper binaries from downgrade pins, not a code regression. - Related: #504 (the salvage mega-issue this keeps feeding), #497 (single dispatch's transient resolve error). Distinct from both.

coilyco-ops commented

2026-07-02 09:21:45 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-529 on host kais-macbook-pro-2.local is carrying this issue (reserved 2026-07-02T09:21:43Z). 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-529` on host `kais-macbook-pro-2.local` is carrying this issue (reserved 2026-07-02T09:21:43Z). 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

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

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

coilysiren referenced this issue from a commit

2026-07-02 09:29:39 +00:00

fix(agent): refuse a ward-version downgrade pin that ships a stale reaper

coilysiren closed this issue

2026-07-02 09:29:39 +00:00

coilysiren commented

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

Author

Owner

WARD-OUTCOME: done - buildUpPlan now refuses a --ward-version pin older than the dispatching host (hidden --allow-ward-downgrade opts in), so a stale reaper never ships.

Retrospective: this one felt clean once I found the real chokepoint. The issue pointed at agent_gate.go, but the honest single place to guard is buildUpPlan in container.go, where every dispatch path (headless engineer, director-forwarded engineer, interactive surface) resolves WARD_VERSION. I kept the pure comparison helper (wardDowngradeGuard) in agent_gate.go so the acceptance test lives where asked, and wired the one-line call at the plan-build chokepoint. version.Behind from cli-guard already had exactly the semantics I needed, including the "dev host never fires" leniency, so I leaned on it rather than reinventing a compare.

What fought back was entirely the pre-commit suite, not the logic: the 2-line comment cap, the 90-char line cap, the 80-line/4000-char doc caps (container-reap.md was already at 3986/4000, zero headroom, so I pulled the rationale into a new docs/agent-ward-downgrade.md instead), and the terse-issue-ref linter on my doc title. Three commit attempts to satisfy them all. Good discipline, just iterative.

Confidence is high: pure helper unit-tested across refused/equal/newer/opted-in/dev cases, plus an end-to-end test that overrides Version and drives the real --allow-ward-downgrade flag through buildUpPlan. Zero new lint issues over the 28-issue baseline.

One rough edge worth a note, not a blocker: I chose refuse-by-default with no loud "you are overriding" warning on the opt-in path, since buildUpPlan is a pure plan builder with no writer handy. If operators want a stderr heads-up when --allow-ward-downgrade is actually used, that is a small follow-up. Also did not forward --allow-ward-downgrade director->engineer, because the forwarded pin always equals the container's own resolved Version so a child dispatch sees equal-or-newer - worth a second look if that invariant ever changes.

WARD-OUTCOME: done - buildUpPlan now refuses a --ward-version pin older than the dispatching host (hidden --allow-ward-downgrade opts in), so a stale reaper never ships. Retrospective: this one felt clean once I found the real chokepoint. The issue pointed at agent_gate.go, but the honest single place to guard is buildUpPlan in container.go, where every dispatch path (headless engineer, director-forwarded engineer, interactive surface) resolves WARD_VERSION. I kept the pure comparison helper (wardDowngradeGuard) in agent_gate.go so the acceptance test lives where asked, and wired the one-line call at the plan-build chokepoint. version.Behind from cli-guard already had exactly the semantics I needed, including the "dev host never fires" leniency, so I leaned on it rather than reinventing a compare. What fought back was entirely the pre-commit suite, not the logic: the 2-line comment cap, the 90-char line cap, the 80-line/4000-char doc caps (container-reap.md was already at 3986/4000, zero headroom, so I pulled the rationale into a new docs/agent-ward-downgrade.md instead), and the terse-issue-ref linter on my doc title. Three commit attempts to satisfy them all. Good discipline, just iterative. Confidence is high: pure helper unit-tested across refused/equal/newer/opted-in/dev cases, plus an end-to-end test that overrides Version and drives the real --allow-ward-downgrade flag through buildUpPlan. Zero new lint issues over the 28-issue baseline. One rough edge worth a note, not a blocker: I chose refuse-by-default with no loud "you are overriding" warning on the opt-in path, since buildUpPlan is a pure plan builder with no writer handy. If operators want a stderr heads-up when --allow-ward-downgrade is actually used, that is a small follow-up. Also did not forward --allow-ward-downgrade director->engineer, because the forwarded pin always equals the container's own resolved Version so a child dispatch sees equal-or-newer - worth a second look if that invariant ever changes.

coilysiren referenced this issue

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

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

coilyco-ops referenced this issue

2026-07-07 22:57:09 +00:00

Fix residual reap landing path that can push work without closing the carried issue #515

coilyco-ops referenced this issue

2026-07-08 22:19:00 +00:00

Make agent workflow defaults configurable and default engineers to PRs #707