Reaper granted-repo landing check false-positives on a merge-commit push (spurious reopen + salvage of a landed run) #583

New issue

Closed

opened 2026-07-03 19:28:02 +00:00 by coilysiren · 2 comments

coilysiren commented

2026-07-03 19:28:02 +00:00

Owner

Symptom

ward#578 was a cross-repo run (warded engineer ward#578 --repo cli-guard). It landed correctly on both mains, but the reaper reopened #578 and cut ward-salvage/cli-guard-4c59766a, claiming "1 local commit(s) never reached origin/main" for cli-guard.

It actually landed

cli-guard main HEAD = 05d604201e60 ("Merge issue-578: per-role capability roster in fleetconfig"), which carries the roles block.
ward main go.mod pins cli-guard v0.67.1-0.20260703191021-05d604201e60 - exactly cli-guard main HEAD.
The salvage branch HEAD is also 05d604201e60.

So the dependency is consistent and the work is on cli-guard main. The reopen is spurious.

Root cause

The reaper's granted-repo landing check (verifyExtraReposLanded / checkExtraRepoLanded, cmd/ward/container_reap.go) compares the engineer's local HEAD SHA against the freshly-fetched origin/main for equality. When the granted-repo push lands via a merge commit (server-side merge, or a merge created on push), or origin/main advances between push and reap, or there's propagation lag, the local HEAD SHA differs from origin/main even though the local commit is reachable from origin/main. A genuinely-landed push then reads as unlanded.

Impact

Spurious reopens + salvage branches + reopened (previously-closed) issues on cross-repo runs that actually succeeded. It undoes a correct "done" state, creates cleanup noise, and erodes trust in the ward#291 cross-repo landing verification - the exact backstop meant to prevent half-landed cross-repo runs now cries wolf on fully-landed ones.

Fix direction

Test reachability/ancestry, not SHA equality: git merge-base --is-ancestor <local-work-commit> origin/main (landed iff the local commit is an ancestor of origin/main). Tolerate merge commits and a short propagation window before declaring a miss. Consider checking each local commit for reachability rather than only HEAD==origin/main.

ward#291 (granted-repo landing verification - this is its false-positive mode), ward#578 (the run it falsely reopened), ward#570 (reaper reservation rollback, adjacent).

## Symptom ward#578 was a cross-repo run (`warded engineer ward#578 --repo cli-guard`). It landed correctly on **both** mains, but the reaper **reopened** #578 and cut `ward-salvage/cli-guard-4c59766a`, claiming "1 local commit(s) never reached origin/main" for cli-guard. ## It actually landed - cli-guard main HEAD = `05d604201e60` ("Merge issue-578: per-role capability roster in fleetconfig"), which carries the `roles` block. - ward main `go.mod` pins cli-guard `v0.67.1-0.20260703191021-05d604201e60` - **exactly** cli-guard main HEAD. - The salvage branch HEAD is also `05d604201e60`. So the dependency is consistent and the work is on cli-guard main. The reopen is spurious. ## Root cause The reaper's granted-repo landing check (`verifyExtraReposLanded` / `checkExtraRepoLanded`, `cmd/ward/container_reap.go`) compares the engineer's **local HEAD SHA** against the freshly-fetched `origin/main` for **equality**. When the granted-repo push lands via a **merge commit** (server-side merge, or a merge created on push), or `origin/main` advances between push and reap, or there's propagation lag, the local HEAD SHA differs from `origin/main` **even though the local commit is reachable from origin/main**. A genuinely-landed push then reads as unlanded. ## Impact Spurious reopens + salvage branches + reopened (previously-closed) issues on cross-repo runs that actually succeeded. It undoes a correct "done" state, creates cleanup noise, and erodes trust in the ward#291 cross-repo landing verification - the exact backstop meant to *prevent* half-landed cross-repo runs now cries wolf on fully-landed ones. ## Fix direction Test **reachability/ancestry**, not SHA equality: `git merge-base --is-ancestor <local-work-commit> origin/main` (landed iff the local commit is an ancestor of origin/main). Tolerate merge commits and a short propagation window before declaring a miss. Consider checking each local commit for reachability rather than only HEAD==origin/main. ## Related ward#291 (granted-repo landing verification - this is its false-positive mode), ward#578 (the run it falsely reopened), ward#570 (reaper reservation rollback, adjacent).

coilyco-ops commented

2026-07-03 19:28:49 +00:00

Member

🔒 Reserved by ward agent --driver claude — container engineer-claude-ward-583 on host KAI-DESKTOP-TOWER is carrying this issue (reserved 2026-07-03T19:28:44Z). Concurrent ward agent runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); --force overrides.

Do not comment on or edit this issue to steer the run while it is reserved. The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a new issue, dispatched fresh — that is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494).

— Claude (she/her), via ward agent

🔒 Reserved by `ward agent --driver claude` — container `engineer-claude-ward-583` on host `KAI-DESKTOP-TOWER` is carrying this issue (reserved 2026-07-03T19:28:44Z). Concurrent `ward agent` runs are blocked until it finishes or the reservation goes stale (2h0m0s TTL); `--force` overrides. **Do not comment on or edit this issue to steer the run while it is reserved.** The engineer seeded the body once at launch and never re-reads it, so a comment or edit reaches only human readers, never the running engineer. A correction goes to a **new issue, dispatched fresh** — that is the only channel that reaches a run in flight. Where the forge supports it, ward locks this conversation to make that a road-block rather than a convention (ward#494).  — Claude (she/her), via `ward agent`

coilysiren referenced this issue from a commit

2026-07-03 19:41:25 +00:00

fix(reap): test grant landing by reachability, not HEAD equality (closes #583)

coilysiren closed this issue

2026-07-03 19:41:25 +00:00

coilysiren commented

2026-07-03 19:41:53 +00:00

Author

Owner

WARD-OUTCOME: done - reaper grant-landing check now tests HEAD reachability from origin/main (not SHA equality), with a re-fetch propagation window.

The interesting part was that the root cause wasn't quite where the issue pointed. The blamed origin/main..HEAD == 0 ahead-count is actually a reachability test already, so swapping it for merge-base --is-ancestor alone would have changed nothing. The real trigger was the closing-reference gate added later (fd60ec7): on a fully-landed grant the origin/main..HEAD range is empty, so it returned false, hardcoded Ahead=1, and salvaged - which matches the reported symptom (Ahead=1, salvage HEAD == origin/main HEAD) exactly. Requiring the target's closes #578 in the granted cli-guard repo was wrong anyway (that keyword would close cli-guard#578).

So the fix has two coupled halves: drop the closing-ref gate from the granted path (the target repo still enforces its own), and make landing a pure HEAD-ancestry check with a 3-attempt / 2s re-fetch window for propagation lag / merge-commit / concurrent-advance cases.

Fairly confident - three real-git tests cover the regression, merge-commit ancestry (HEAD != origin/main but reachable), and a genuine miss that exhausts the window. Rough edge worth noting: a squash-merge landing still can't be detected by ancestry (the squashed commit is a new SHA), so a squash-landed grant would false-salvage. Ward pushes straight to main / merge rather than squashing, so it's out of scope here, but worth a follow-up if squash ever enters the landing path.

WARD-OUTCOME: done - reaper grant-landing check now tests HEAD reachability from origin/main (not SHA equality), with a re-fetch propagation window. The interesting part was that the root cause wasn't quite where the issue pointed. The blamed `origin/main..HEAD == 0` ahead-count is actually a reachability test already, so swapping it for `merge-base --is-ancestor` alone would have changed nothing. The real trigger was the *closing-reference* gate added later (fd60ec7): on a fully-landed grant the `origin/main..HEAD` range is empty, so it returned false, hardcoded `Ahead=1`, and salvaged - which matches the reported symptom (Ahead=1, salvage HEAD == origin/main HEAD) exactly. Requiring the target's `closes #578` in the *granted* cli-guard repo was wrong anyway (that keyword would close cli-guard#578). So the fix has two coupled halves: drop the closing-ref gate from the granted path (the target repo still enforces its own), and make landing a pure HEAD-ancestry check with a 3-attempt / 2s re-fetch window for propagation lag / merge-commit / concurrent-advance cases. Fairly confident - three real-git tests cover the regression, merge-commit ancestry (HEAD != origin/main but reachable), and a genuine miss that exhausts the window. Rough edge worth noting: a squash-merge landing still can't be detected by ancestry (the squashed commit is a new SHA), so a squash-landed grant would false-salvage. Ward pushes straight to main / merge rather than squashing, so it's out of scope here, but worth a follow-up if squash ever enters the landing path.

coilyco-ops referenced this issue

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

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