diff --git a/implement/SKILL.md b/implement/SKILL.md index 01677bb..96139e9 100644 --- a/implement/SKILL.md +++ b/implement/SKILL.md @@ -65,10 +65,16 @@ Its executor is **observe-then-bounce**, shipped as 2. **Run the project's real build + full suite.** This run — not an ex-ante guess that the change is "behaviour-preserving" — is the oracle. -3. **Done-signal: clean build AND suite green unchanged → the edits sit - unstaged for the orchestrator to commit.** That conjunction is the - whole gate; it keeps the light path light without letting a - green-but-wrong change through. +3. **Done-signal: an edit actually landed AND clean build AND suite + green unchanged → the edits sit unstaged for the orchestrator to + commit.** That conjunction is the whole gate; it keeps the light path + light without letting a green-but-wrong change through. The + "edit actually landed" conjunct is load-bearing: `clean build AND + suite green` is *vacuously* true on an empty tree, so a no-op edit + would otherwise read as DONE. A clean working tree after the edit + phase is a no-op, not a success → distinct `BLOCKED` (no-op-edit), + never DONE. Positive evidence (tree differs from HEAD) is required, + checked both at the edit phase and independently at verify. 4. **Bounce** otherwise — the change was not purely behaviour-preserving after all, so route up per the straddle rule: a site that forces a design decision → `specify`; a site that turns out to **encode new diff --git a/implement/workflows/compiler-driven-edit.js b/implement/workflows/compiler-driven-edit.js index 2d415f2..627dfa5 100644 --- a/implement/workflows/compiler-driven-edit.js +++ b/implement/workflows/compiler-driven-edit.js @@ -8,12 +8,20 @@ // enumerates" — NOT on a prediction that the change is behaviour-preserving. // A real build + suite run then settles the verdict: // -// clean build AND suite green unchanged -> DONE (edits unstaged; orchestrator commits) +// an edit landed AND clean build AND suite green unchanged -> DONE (edits unstaged; orchestrator commits) +// the edit phase produced NO change (clean tree) -> BLOCKED (no-op-edit) — never DONE // a hole the edit cannot resolve -> BOUNCE up, per the straddle rule: // forces a design choice -> specify // is test-specifiable new behaviour -> tdd (RED-first) // the suite is not green / not unchanged -> BOUNCE to debug (RED-first) // +// "An edit landed" is load-bearing: the done-signal `build_clean && +// suite_green_unchanged` is vacuously true on an EMPTY tree, so doing nothing +// would otherwise read as DONE. DONE requires positive evidence the tree +// differs from HEAD — checked twice: the edit agent's applied_changes self- +// report (cheap, pre-suite) and the verify agent's independent working_tree_ +// dirty observation (authoritative backstop). +// // The fallible ex-ante guess ("this is behaviour-preserving") is replaced by // a cheap ex-post detection: a mis-routed behaviour change fails the // post-condition and bounces instead of shipping green-but-wrong. This @@ -52,9 +60,16 @@ const STANDING = const EDIT_SCHEMA = { type: 'object', additionalProperties: false, - required: ['build_clean', 'hole_needs_decision'], + required: ['build_clean', 'hole_needs_decision', 'applied_changes'], properties: { build_clean: { type: 'boolean', description: 'the project build is clean after the edit + mechanical propagation' }, + applied_changes: { + type: 'boolean', + description: + 'GROUND TRUTH, not intent: the working tree actually differs from HEAD after the edit. Determine it by running ' + + '`git status --porcelain` (any line) or `git diff HEAD --stat` (non-empty) — true only if at least one file changed. ' + + 'false means a no-op edit (you touched nothing); report it as false rather than guessing true.', + }, hole_needs_decision: { type: 'boolean', description: @@ -73,13 +88,20 @@ const EDIT_SCHEMA = { const VERIFY_SCHEMA = { type: 'object', additionalProperties: false, - required: ['build_clean', 'suite_green_unchanged'], + required: ['build_clean', 'suite_green_unchanged', 'working_tree_dirty'], properties: { build_clean: { type: 'boolean' }, suite_green_unchanged: { type: 'boolean', description: 'the full suite passes with NO test added, removed, or weakened to make it pass', }, + working_tree_dirty: { + type: 'boolean', + description: + 'GROUND TRUTH: the working tree differs from HEAD — i.e. an edit actually landed. Run `git status --porcelain` ' + + '(any line) or `git diff HEAD --stat` (non-empty). false means a clean tree / no-op edit, which is NOT a ' + + 'behaviour-preserving success no matter how green the build and suite are.', + }, detail: { type: 'string', description: 'on failure: which tests went red, or which test was changed' }, }, } @@ -95,7 +117,9 @@ const edit = await agent( 'choice, or "tdd" if the discovered work is new behaviour pinnable as one falsifiable assertion. When unsure, ' + '"specify". Leave all edits UNSTAGED; never commit.\n\n' + `EDIT: ${edit_description}\nDEFINITION SITE: ${def_site}\n\n` + - 'Then run the project build and report: is the build clean, does any hole need a decision (with detail + bounce_to), and how many sites were touched.', + 'Then run the project build and report: is the build clean, does any hole need a decision (with detail + bounce_to), ' + + 'how many sites were touched, and — checked against `git status --porcelain` / `git diff HEAD`, NOT from memory — ' + + 'whether the working tree actually changed (applied_changes).', { agentType: 'implementer', label: 'edit+propagate', phase: 'Edit + propagate', schema: EDIT_SCHEMA }, ) if (!edit) return { status: 'BLOCKED', reason: 'edit agent did not return' } @@ -112,6 +136,23 @@ if (edit.hole_needs_decision) { detail: edit.hole_detail || '', } } +// No-op guard (cheap, first line of defence): the done-signal must not be +// satisfiable by doing nothing. If the edit phase reports no hole yet left the +// working tree clean, no edit landed — a no-op is not a behaviour-preserving +// success, and running the suite against a clean tree would only manufacture a +// vacuous green. Stop here with a distinct infra-style BLOCKED rather than +// proceeding toward DONE. (`sites` is the wrong signal: a legitimate edit can +// touch only the definition site, sites 0, and still dirty the tree.) +if (!edit.applied_changes) { + return { + status: 'BLOCKED', + kind: 'no-op-edit', + reason: + 'the edit phase produced no changes — the working tree is clean against HEAD, so no edit landed. ' + + 'A no-op edit is not a behaviour-preserving success. Likely the task fell outside the shape this arm ' + + 'handles (a type/signature edit the compiler enumerates), e.g. a crate scaffold/extraction; re-route it.', + } +} // ---- Phase 2 — verify (the independently-invokable phase) --------------- phase('Verify') @@ -119,19 +160,34 @@ const verify = await agent( `${STANDING}\n\nVerify the edit is behaviour-preserving WITHOUT changing any code or test. Run the project build ` + 'and the full test suite. Report whether the build is clean and whether the suite is green AND UNCHANGED — i.e. ' + 'every test passes and NO test was added, removed, or weakened versus the pre-edit baseline (compare `git diff ' + - 'HEAD` for test-file changes). Do NOT edit anything; do NOT commit.', + 'HEAD` for test-file changes). Also report working_tree_dirty — whether `git status --porcelain` / `git diff HEAD` ' + + 'shows ANY change at all (a clean tree means no edit landed). Do NOT edit anything; do NOT commit.', { label: 'verify(build+suite)', phase: 'Verify', schema: VERIFY_SCHEMA }, ) // ---- Phase 3 — verdict -------------------------------------------------- phase('Verdict') -if (verify && verify.build_clean && verify.suite_green_unchanged) { +if (verify && verify.build_clean && verify.suite_green_unchanged && verify.working_tree_dirty) { return { status: 'DONE', - sites: edit.sites || null, + sites: edit.sites ?? null, // ?? not ||: a legitimate sites:0 (def-site-only edit) must report 0, not null note: 'behaviour-preserving edit verified; edits unstaged in the working tree for the orchestrator to commit', } } +// Vacuous-green backstop (authoritative, second line of defence): build + suite +// are green but verify INDEPENDENTLY observed a clean tree. A clean tree is not +// a regression, so debug (RED-first) would be the wrong route — there is nothing +// to reproduce. It is a no-op edit the Phase-1 self-report missed; surface it as +// the same distinct BLOCKED, never as DONE. +if (verify && verify.build_clean && verify.suite_green_unchanged && !verify.working_tree_dirty) { + return { + status: 'BLOCKED', + kind: 'no-op-edit', + reason: + 'verify observed a clean working tree (no diff against HEAD) — no edit landed, so the green build + suite are ' + + 'vacuous. DONE requires positive evidence that an edit actually landed; a no-op edit is not a success.', + } +} // observe-then-bounce: the post-condition failed, so the change was NOT // behaviour-preserving after all. Route RED-first via debug rather than // shipping a green-but-wrong commit.