implement-loop: iteration DONE has no positive-evidence precondition (the #11 no-op-reads-as-success pattern in the main path) #12

New Issue

2026-06-27T13:21:58+02:00

Brummel commented

2026-06-27 13:21:58 +02:00

Found by the sibling scan while fixing #11. The vacuous-DONE pattern #11
fixed in compiler-driven-edit.js is also present — unguarded — in the
main implementation path, implement/workflows/implement-loop.js.
Unlike compiler-driven-edit (now gated on verify.working_tree_dirty),
implement-loop has no positive-evidence precondition on DONE: nothing
asserts that an edit actually landed, and nothing independently re-runs
build/suite. Four facets, all observed in the current script:

1 — Per-task no-op can aggregate to iteration DONE (`:357-362`)

outcome becomes DONE purely from the absence of a BLOCKED task
(if (!blocked) outcome = 'DONE', ~~:360). A per-task DONE (~~:347) is
reached when the implementer self-reports DONE/DONE_WITH_CONCERNS and
the two read-only reviewers pass — none of which is a deterministic
"the working tree differs from HEAD" check. So a task whose implementer
no-ops (does nothing, reports DONE) can roll up to a full-iteration DONE.

The whole-tree-empty case is partly caught at the agent level
(spec-reviewer returns NEEDS_CONTEXT when its diff_command produces no
output), but a per-task no-op mid-iteration is not: git diff HEAD is
the accumulated cross-task diff, already non-empty from earlier tasks, so
the empty-diff guard never fires and a coincidentally-already-satisfied
task reads compliant over a zero-footprint contribution.

2 — `files_touched` present-but-unused — the exact #11 lever (`:402,:420`)

The finalize agent already computes files_touched from
git diff --name-only HEAD | wc -l (:402) and the end-report surfaces
it (:420) — but it is computed after outcome is decided and is
never asserted > 0. status: DONE with files_touched: 0 is
reachable. This is #11's wording verbatim: "the workflow has access to
whether the working tree differs from HEAD … but neither is used as a
precondition for DONE." The ground-truth signal is gathered and shown,
just never gated on.

3 — E2E coverage phase can no-op invisibly (`:366-375,:417`)

The e2e agent is dispatched and its schema carries a status enum
(DONE/DONE_WITH_CONCERNS/NEEDS_CONTEXT/BLOCKED), but the script never
reads e2e.status — :417 only pulls e2e.fixtures || []. A tester
that writes zero fixtures, or returns BLOCKED, or DONE_WITH_CONCERNS, does
not change the iteration outcome (stays DONE) and is not surfaced as a
concern. The phase reads as a successful DONE iteration with
e2e_fixtures: [] even when it added zero coverage or actively failed.

4 — Mini-mode: GREEN asserted, never observed (`:347`, task `:212-222`) — most serious

The implementer's prompt says "build and test must be green before you
report DONE," but nothing in the loop re-runs build or tests; the
spec/quality reviewers only read git diff HEAD. In mini-mode (the
debug/tdd RED→GREEN handoff) the whole point is to drive a specific RED
test to GREEN, yet that RED test is never independently re-run — a
no-op leaves it RED while the implementer can still self-report DONE, and
the only thing between that and an aggregated DONE is a reviewer's
natural-language judgement. The GREEN is asserted, never observed.
Contrast compiler-driven-edit.js, whose Phase-2 verify agent
independently re-runs build+suite and checks working_tree_dirty.

Direction (not prescribing the implementation)

A positive-evidence precondition on DONE, analogous to the #11 fix:
at minimum gate the iteration/task DONE on files_touched > 0 (the lever
is already computed); ideally an independent observation of the
success condition — re-run the build/suite, and in mini-mode re-run the
handed-off RED test to confirm it is actually GREEN — rather than trusting
the implementer's self-report. Also read e2e.status and let a non-DONE
E2E status surface as a concern. Facet 4 is arguably its own correctness
bug and may warrant splitting out.

Refs #11.

Found by the sibling scan while fixing #11. The vacuous-DONE pattern #11 fixed in `compiler-driven-edit.js` is also present — unguarded — in the **main implementation path**, `implement/workflows/implement-loop.js`. Unlike compiler-driven-edit (now gated on `verify.working_tree_dirty`), implement-loop has **no positive-evidence precondition on DONE**: nothing asserts that an edit actually landed, and nothing independently re-runs build/suite. Four facets, all observed in the current script: ## 1 — Per-task no-op can aggregate to iteration DONE (`:357-362`) `outcome` becomes DONE purely from the *absence* of a BLOCKED task (`if (!blocked) outcome = 'DONE'`, ~`:360`). A per-task DONE (~`:347`) is reached when the implementer **self-reports** DONE/DONE_WITH_CONCERNS and the two read-only reviewers pass — none of which is a deterministic "the working tree differs from HEAD" check. So a task whose implementer no-ops (does nothing, reports DONE) can roll up to a full-iteration DONE. The whole-tree-empty case is *partly* caught at the agent level (spec-reviewer returns NEEDS_CONTEXT when its `diff_command` produces no output), but a per-task no-op **mid-iteration** is not: `git diff HEAD` is the accumulated cross-task diff, already non-empty from earlier tasks, so the empty-diff guard never fires and a coincidentally-already-satisfied task reads `compliant` over a zero-footprint contribution. ## 2 — `files_touched` present-but-unused — the exact #11 lever (`:402,:420`) The finalize agent already computes `files_touched` from `git diff --name-only HEAD | wc -l` (~`:402`) and the end-report surfaces it (~`:420`) — but it is computed **after** `outcome` is decided and is **never asserted `> 0`**. `status: DONE` with `files_touched: 0` is reachable. This is #11's wording verbatim: "the workflow has access to whether the working tree differs from HEAD … but neither is used as a precondition for DONE." The ground-truth signal is gathered and shown, just never gated on. ## 3 — E2E coverage phase can no-op invisibly (`:366-375,:417`) The `e2e` agent is dispatched and its schema carries a status enum (DONE/DONE_WITH_CONCERNS/NEEDS_CONTEXT/BLOCKED), but the script **never reads `e2e.status`** — `:417` only pulls `e2e.fixtures || []`. A tester that writes zero fixtures, or returns BLOCKED, or DONE_WITH_CONCERNS, does not change the iteration outcome (stays DONE) and is not surfaced as a concern. The phase reads as a successful DONE iteration with `e2e_fixtures: []` even when it added zero coverage or actively failed. ## 4 — Mini-mode: GREEN asserted, never observed (`:347`, task `:212-222`) — most serious The implementer's prompt says "build and test must be green before you report DONE," but nothing in the loop re-runs build or tests; the spec/quality reviewers only **read** `git diff HEAD`. In mini-mode (the debug/tdd RED→GREEN handoff) the whole point is to drive a specific RED test to GREEN, yet **that RED test is never independently re-run** — a no-op leaves it RED while the implementer can still self-report DONE, and the only thing between that and an aggregated DONE is a reviewer's natural-language judgement. The GREEN is asserted, never observed. Contrast `compiler-driven-edit.js`, whose Phase-2 `verify` agent independently re-runs build+suite **and** checks `working_tree_dirty`. ## Direction (not prescribing the implementation) A positive-evidence precondition on DONE, analogous to the #11 fix: at minimum gate the iteration/task DONE on `files_touched > 0` (the lever is already computed); ideally an **independent** observation of the success condition — re-run the build/suite, and in mini-mode re-run the handed-off RED test to confirm it is actually GREEN — rather than trusting the implementer's self-report. Also read `e2e.status` and let a non-DONE E2E status surface as a concern. Facet 4 is arguably its own correctness bug and may warrant splitting out. Refs #11.

Brummel added the bug label 2026-06-27 13:21:58 +02:00

Brummel commented

2026-06-27 14:28:17 +02:00

Fixed in `d6c4faa` (closes on push)

DONE now requires positive evidence that an edit landed, taken from git ground truth, never a self-report — across all four facets this issue named.

Design decisions worth recording

The verdict must never come from a self-report. The first cut keyed the iteration no-op gate on the per-task applied_changes self-report. An adversarial review found a real false-BLOCKED: the initial implementer can legitimately no-op (code already in shape) while a repair writes the files — and applied_changes was captured only from the initial dispatch. A self-report must never be able to fail a run that actually did the work. So applied_changes (now OR-ed across the initial dispatch and both repairs) feeds only a neutral concern, never the outcome. The outcome verdict is git ground truth.
git status --porcelain, not git diff HEAD. A second review caught that git diff HEAD --name-only omits brand-new untracked files — and the implementer leaves edits unstaged, so a new-file deliverable is untracked. The first git-based attempt would have false-BLOCKED any iteration whose deliverable is a new file — re-introducing the very bug. The gate counts git status --porcelain (untracked-aware).
A dedicated tree-check agent, run BEFORE E2E/finalize. The count cannot come from the finalize agent: (a) a null finalize return would silently disable the gate (defaulting to vacuous DONE); (b) E2E fixtures and the stats/BLOCKED.md artefacts would inflate the count; (c) taking the verdict after finalize left stats.json recording a stale DONE while the end-report returned BLOCKED. Running a dedicated check pre-E2E/finalize fixes all three: the gate is independent of finalize (a non-returning agent → BLOCKED, infra), pre-artefact, and sets the outcome before the aggregate so stats.json is never stale.
Mini mode observes, never asserts. The whole telos of a mini handoff is RED→GREEN, yet nothing re-ran the test. An independent mini-verify re-runs the handed-off RED test by name + the suite + checks working_tree_dirty — the implement-loop analogue of compiler-driven-edit's verify agent. A still-red test / regression / clean tree → BLOCKED (route back to debug).
Clean-tree no-op → BLOCKED with no BLOCKED.md. There is nothing to clean up; the status + reason ride the end-report. A noopClean flag suppresses BLOCKED.md only on a confirmed-clean tree (a null/infra return still writes one — the tree might be dirty). SKILL.md Step 4 carves out this one no-BLOCKED.md case.
e2e.status is read now (not just fixtures): a non-DONE status or a zero-fixture run surfaces as a concern.

Verification

Three adversarial passes. The first two each returned BLOCK and found a real defect (the self-report false-BLOCKED; then the untracked-blind git diff + finalize-null + stats mismatch + E2E pollution). The third returned SOUND: hole_closed, false_positive_eliminated (new-untracked-file deliverables reach DONE), all_paths_coherent across all six outcome paths, no new issues. Two residual doc drifts it flagged (Iron Law still said git diff HEAD; the Step-4 signature assumed standard-mode files_touched 0) are fixed in the same commit.

Facet 4 (mini-mode GREEN-unobserved) was noted in the issue as possibly its own bug; it is folded here since all four share the "observe, don't assert" root.

## Fixed in d6c4faa (closes on push) `DONE` now requires positive evidence that an edit landed, taken from **git ground truth, never a self-report** — across all four facets this issue named. ### Design decisions worth recording 1. **The verdict must never come from a self-report.** The first cut keyed the iteration no-op gate on the per-task `applied_changes` self-report. An adversarial review found a real false-BLOCKED: the *initial* implementer can legitimately no-op (code already in shape) while a **repair** writes the files — and `applied_changes` was captured only from the initial dispatch. A self-report must never be able to *fail* a run that actually did the work. So `applied_changes` (now OR-ed across the initial dispatch and both repairs) feeds **only a neutral concern**, never the outcome. The outcome verdict is git ground truth. 2. **`git status --porcelain`, not `git diff HEAD`.** A second review caught that `git diff HEAD --name-only` **omits brand-new untracked files** — and the implementer leaves edits unstaged, so a new-file deliverable is untracked. The first git-based attempt would have *false-BLOCKED any iteration whose deliverable is a new file* — re-introducing the very bug. The gate counts `git status --porcelain` (untracked-aware). 3. **A dedicated `tree-check` agent, run BEFORE E2E/finalize.** The count cannot come from the finalize agent: (a) a null finalize return would silently disable the gate (defaulting to vacuous DONE); (b) E2E fixtures and the stats/`BLOCKED.md` artefacts would inflate the count; (c) taking the verdict after finalize left `stats.json` recording a stale `DONE` while the end-report returned `BLOCKED`. Running a dedicated check pre-E2E/finalize fixes all three: the gate is independent of finalize (a non-returning agent → `BLOCKED`, infra), pre-artefact, and sets the outcome before the aggregate so `stats.json` is never stale. 4. **Mini mode observes, never asserts.** The whole telos of a mini handoff is RED→GREEN, yet nothing re-ran the test. An independent `mini-verify` re-runs the handed-off RED test by name + the suite + checks `working_tree_dirty` — the implement-loop analogue of compiler-driven-edit's verify agent. A still-red test / regression / clean tree → `BLOCKED` (route back to debug). 5. **Clean-tree no-op → BLOCKED with no `BLOCKED.md`.** There is nothing to clean up; the status + reason ride the end-report. A `noopClean` flag suppresses `BLOCKED.md` only on a *confirmed*-clean tree (a null/infra return still writes one — the tree might be dirty). SKILL.md Step 4 carves out this one no-`BLOCKED.md` case. 6. **`e2e.status` is read now** (not just `fixtures`): a non-`DONE` status or a zero-fixture run surfaces as a concern. ### Verification Three adversarial passes. The first two each returned **BLOCK** and found a real defect (the self-report false-BLOCKED; then the untracked-blind `git diff` + finalize-null + stats mismatch + E2E pollution). The third returned **SOUND**: `hole_closed`, `false_positive_eliminated` (new-untracked-file deliverables reach DONE), `all_paths_coherent` across all six outcome paths, no new issues. Two residual doc drifts it flagged (Iron Law still said `git diff HEAD`; the Step-4 signature assumed standard-mode `files_touched 0`) are fixed in the same commit. Facet 4 (mini-mode GREEN-unobserved) was noted in the issue as possibly its own bug; it is folded here since all four share the "observe, don't assert" root.

Brummel referenced this issue from a commit

2026-06-29 11:08:57 +02:00

fix(implement-loop): require positive edit evidence before DONE

Brummel closed this issue

2026-06-29 11:08:57 +02:00

Brummel referenced this issue

2026-07-06 13:42:55 +02:00

implement-loop: guard the plan-extract against silent truncation on big plans #22

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

Sign in to join this conversation.