implement-loop: anchor the quality reviewer to the loop's repo root (worktree sessions review the wrong tree) #25

New Issue

2026-07-10T20:57:36+02:00

Brummel commented

2026-07-10 20:57:36 +02:00

Filed autonomously by the /boss orchestrator while running on
project "aura". No human was in the loop; this records a
skill-system deficiency the orchestrator hit mid-run.

Motivation

In a worktree-based session, the implement-loop quality gate reviews the wrong tree and produces false BLOCKEDs. In one /boss run over the aura project (2026-07-10), 6 of 9 quality dispatches across two mini-mode iterations returned infra_blocked with git diff HEAD produces no output and the sha256-of-empty-input fingerprint (e3b0c442...), while the working tree in the session's git worktree held a real, verified diff the whole time — one reviewer even reported the primary checkout's HEAD (3016bd6) instead of the worktree's. Both iterations exhausted the quality re-loop and ended BLOCKED: review-loop-exhausted (quality) although implementer, spec-reviewer, and mini-verify had all seen the correct tree; the orchestrator had to re-dispatch a fresh quality review by hand with an explicit worktree path to get a substantive verdict.

Problem

The quality prompt in workflows/implement-loop.js (the 2.3 — quality check block, ~lines 603-623) instructs the agent to read git diff HEAD but carries no working-directory anchor, and it is deliberately blind to the task text (comment at ~line 605) — so unlike the implementer/spec/verify prompts, which usually inherit a repo path via the task text or the mini-mode carrier fields, the quality agent has no textual cue for WHERE git diff HEAD should run. When the invoking session works in a git worktree (e.g. background jobs, which are worktree-isolated by harness policy), the spawned agent's default cwd can resolve to the primary checkout, whose tree is clean — the review then sees an empty diff. The shared STANDING preamble (~line 167) names project facts and git log, but no repo root either.

Observed distribution: the failure is per-dispatch, not per-run (in a third iteration of the same run the second quality dispatch DID land in the worktree and returned a substantive verdict), which fits a cwd-inheritance race rather than a deterministic wrong path.

Two candidate remedies (not prescriptive): stamp the loop's own repo root (the workflow can read it once via a cheap git rev-parse --show-toplevel agent, or take it from the carrier) into every reviewer prompt as an explicit git -C <root> instruction; or extend the existing empty-diff infra_blocked handling to re-dispatch once with an explicit root instead of burning the re-loop budget on identical empty-diff reviews (the current loop retries the SAME prompt and exhausts).

The empty-diff shape is already half-recognized: the quality schema's diff_fingerprint requirement (~line 287) makes the empty review detectable (e3b0c442... = sha256 of empty input) — it is just not acted on as an infra retry-with-anchor.

> Filed autonomously by the `/boss` orchestrator while running on > project "aura". No human was in the loop; this records a > skill-system deficiency the orchestrator hit mid-run. ## Motivation In a worktree-based session, the implement-loop quality gate reviews the wrong tree and produces false BLOCKEDs. In one `/boss` run over the aura project (2026-07-10), 6 of 9 quality dispatches across two mini-mode iterations returned `infra_blocked` with `git diff HEAD produces no output` and the sha256-of-empty-input fingerprint (`e3b0c442...`), while the working tree in the session's git worktree held a real, verified diff the whole time — one reviewer even reported the primary checkout's HEAD (`3016bd6`) instead of the worktree's. Both iterations exhausted the quality re-loop and ended `BLOCKED: review-loop-exhausted (quality)` although implementer, spec-reviewer, and mini-verify had all seen the correct tree; the orchestrator had to re-dispatch a fresh quality review by hand with an explicit worktree path to get a substantive verdict. ## Problem The quality prompt in `workflows/implement-loop.js` (the `2.3 — quality check` block, ~lines 603-623) instructs the agent to read `git diff HEAD` but carries no working-directory anchor, and it is *deliberately blind to the task text* (comment at ~line 605) — so unlike the implementer/spec/verify prompts, which usually inherit a repo path via the task text or the mini-mode carrier fields, the quality agent has no textual cue for WHERE `git diff HEAD` should run. When the invoking session works in a git worktree (e.g. background jobs, which are worktree-isolated by harness policy), the spawned agent's default cwd can resolve to the primary checkout, whose tree is clean — the review then sees an empty diff. The shared `STANDING` preamble (~line 167) names project facts and git log, but no repo root either. Observed distribution: the failure is per-dispatch, not per-run (in a third iteration of the same run the second quality dispatch DID land in the worktree and returned a substantive verdict), which fits a cwd-inheritance race rather than a deterministic wrong path. Two candidate remedies (not prescriptive): stamp the loop's own repo root (the workflow can read it once via a cheap `git rev-parse --show-toplevel` agent, or take it from the carrier) into every reviewer prompt as an explicit `git -C <root>` instruction; or extend the existing empty-diff `infra_blocked` handling to re-dispatch once with an explicit root instead of burning the re-loop budget on identical empty-diff reviews (the current loop retries the SAME prompt and exhausts). The empty-diff shape is already half-recognized: the quality schema's `diff_fingerprint` requirement (~line 287) makes the empty review detectable (`e3b0c442...` = sha256 of empty input) — it is just not acted on as an infra retry-with-anchor.

Brummel added the bug label 2026-07-10 20:57:36 +02:00

Brummel commented

2026-07-11 07:25:25 +02:00

Two follow-up observations from the same run, sharpening the scope and validating a mitigation:

The tree-check gate has the same defect. In a standard-mode iteration whose implementer, spec review, and quality review all completed against the real worktree diff (quality returned a REAL fingerprint and approved), the tree-check agent (workflows/implement-loop.js ~line 827, "Report the working-tree footprint ... run git status --porcelain") counted a clean tree — the primary checkout again — and returned files_touched: 0, which the no-op guard rightly converted to BLOCKED ("every task reported DONE but nothing landed in the working tree"). So the cwd instability can produce a FALSE no-op verdict on a genuinely-DONE iteration, not just a wasted review loop: the issue-#12 positive-evidence gate then punishes the run for the harness's own directory bug.
Prompt-anchoring works as a mitigation. Patching the dispatched prompts (quality, spec, tree-check) with one leading sentence — "CRITICAL: the tree lives in the git worktree — run git -C <abs path> diff HEAD; an empty diff or HEAD means you are in the WRONG directory" — produced the first quality review of the run that saw the real diff on its first attempt (real sha256 fingerprint, substantive verdict). This supports the "stamp the loop's repo root into every reviewer prompt" remedy sketched in the issue body: the anchor belongs in ALL agents whose verdicts read git state (implementer prompts get the path implicitly via task text; quality/tree-check/mini-verify do not).

Two follow-up observations from the same run, sharpening the scope and validating a mitigation: 1. The `tree-check` gate has the same defect. In a standard-mode iteration whose implementer, spec review, and quality review all completed against the real worktree diff (quality returned a REAL fingerprint and `approved`), the tree-check agent (`workflows/implement-loop.js` ~line 827, "Report the working-tree footprint ... run `git status --porcelain`") counted a clean tree — the primary checkout again — and returned `files_touched: 0`, which the no-op guard rightly converted to BLOCKED ("every task reported DONE but nothing landed in the working tree"). So the cwd instability can produce a FALSE no-op verdict on a genuinely-DONE iteration, not just a wasted review loop: the issue-#12 positive-evidence gate then punishes the run for the harness's own directory bug. 2. Prompt-anchoring works as a mitigation. Patching the dispatched prompts (quality, spec, tree-check) with one leading sentence — "CRITICAL: the tree lives in the git worktree <abs path> — run `git -C <abs path> diff HEAD`; an empty diff or HEAD <primary-sha> means you are in the WRONG directory" — produced the first quality review of the run that saw the real diff on its first attempt (real sha256 fingerprint, substantive verdict). This supports the "stamp the loop's repo root into every reviewer prompt" remedy sketched in the issue body: the anchor belongs in ALL agents whose verdicts read git state (implementer prompts get the path implicitly via task text; quality/tree-check/mini-verify do not).

claude referenced this issue

2026-07-17 15:20:05 +02:00

Cut fixed per-iteration dispatch overhead in implement-loop; add the missing standard-mode suite gate #29

Brummel referenced this issue from a commit

2026-07-17 16:01:20 +02:00

fix(implement-loop): stamp a repo-root anchor into every stage prompt

Brummel closed this issue

2026-07-17 16:01:20 +02:00

Sign in to join this conversation.