fix(implement-loop): make resume survive plan edits and interim subset commits
Two interacting defects broke the documented PARTIAL repair flow (fix the plan, commit the known-good subset, rm BLOCKED.md, resume) — observed on run wf_81a13333-be3 (aura): the resume replayed all 78 agents from cache in 33 ms and returned the byte-identical stale end-report. 1. Plan content was invisible to the resume cache key. The substrate caches per (prompt, opts) and every plan-reading prompt carried only the plan PATH, so a plan edit changed no prompt and the extraction replayed stale text. New required standard-mode carrier field plan_sha256 (recomputed by the orchestrator at every invocation) is stamped into all three plan-reading prompts (plan-index, plan-extract-all, plan-extract:<id>): a plan edit re-runs the extraction, and a downstream task replays from cache iff its re-extracted text is byte-identical — worst case a redundant live re-run, never stale text. Alternative considered: documenting "plan edits need a fresh run with task_range" — rejected, it keeps the trap armed and forfeits the cache replay a resume exists for. 2. The cross-task discard guard compared boundary path sets across a moved HEAD. Boundary sets are diffs AGAINST HEAD; after the sanctioned interim subset commit, the first live boundary measures against the new HEAD while the replayed boundary carries the old one — the entire committed subset read as "lost paths" and hard-BLOCKed. The snapshot agent now also reports `git rev-parse HEAD`; a boundary pair straddling a HEAD move resets the guard with a concern (which also flags a rogue mid-run commit) instead of comparing. Same-HEAD discards still hard-BLOCK; a blank sha falls through to the comparison so a flaky report cannot disarm the guard. Verified with a zero-token mock harness (AsyncFunction + stubbed agent()): fail-fast on missing plan_sha256, hash present in all three extraction prompts (batched and fan-out paths), same-HEAD discard still BLOCKs, HEAD-move resets with a concern, mini mode unaffected. Adversarially reviewed (4-verifier workflow): SOUND. Known residuals: the script cannot verify the hash matches the plan bytes (operator obligation, documented in the carrier contract and SKILL.md), and a coincident HEAD-move + discard surfaces as a concern rather than a BLOCK. closes #32
This commit is contained in:
+27
-2
@@ -200,7 +200,13 @@ verdict means *adjudicate against the snapshot*, not proven malice.
|
||||
Renames do not trip it (the boundary diff runs `--no-renames`, so a
|
||||
renamed file's old path stays listed as a deletion). Untracked files
|
||||
sit outside the threat model (`git checkout -- <path>` cannot discard
|
||||
them). Cost: one extra sonnet/medium snapshot call per task of a
|
||||
them). Boundaries are HEAD-aware (issue #32): each snapshot records
|
||||
the HEAD its diff ran against, and a boundary pair straddling a HEAD
|
||||
move — the sanctioned commit-the-subset-then-resume repair — resets
|
||||
the guard with a concern instead of comparing, so a replayed
|
||||
pre-commit boundary never counts the committed subset as lost paths;
|
||||
in a single live run the same concern flags an agent that moved HEAD
|
||||
mid-run. Cost: one extra sonnet/medium snapshot call per task of a
|
||||
multi-task run; single-task and mini runs skip the guard. The
|
||||
detection details live in `workflows/implement-loop.js`, the single
|
||||
source.
|
||||
@@ -220,6 +226,7 @@ Workflow({ name: "implement-loop", args: {
|
||||
mode: "standard",
|
||||
iter_id: "<iter_id>",
|
||||
plan_path: "<path under docs/plans>",
|
||||
plan_sha256: "<sha256sum of the plan file>", // recompute at EVERY (re-)invocation (issue #32)
|
||||
task_range: [3, 8], // optional
|
||||
repo_root: "<abs path of the working tree>" // strongly recommended; effectively required in worktree sessions (issue #25)
|
||||
}})
|
||||
@@ -238,6 +245,14 @@ Workflow({ name: "implement-loop", args: {
|
||||
}})
|
||||
```
|
||||
|
||||
`plan_sha256` content-addresses the plan (issue #32): a resume replays
|
||||
agents cached per (prompt, opts), and with only the plan *path* in the
|
||||
extraction prompts an edit-and-resume would replay the stale pre-edit
|
||||
task text — the repair silently no-ops. Compute the hash fresh
|
||||
(`sha256sum <plan_path>`) at every invocation, including a
|
||||
`resumeFromRunId` re-invocation after a plan repair; never reuse the
|
||||
old value.
|
||||
|
||||
`repo_root` is stamped into every stage prompt as a `git -C` anchor:
|
||||
in worktree sessions a dispatched agent's cwd can resolve to the
|
||||
primary checkout and review the wrong tree (false `infra_blocked`
|
||||
@@ -332,7 +347,17 @@ orchestrator decides what to do with the dirty working tree:
|
||||
before re-running the workflow — its preflight clean-tree check
|
||||
counts the file as dirt. Either stash everything and re-run on a
|
||||
clean tree, or commit the known-good subset, then `rm BLOCKED.md`,
|
||||
then re-run.
|
||||
then re-run. The re-run is either a fresh invocation with
|
||||
`task_range` covering the remaining tasks, or a resume
|
||||
(`resumeFromRunId` on the same script) — for a resume, recompute
|
||||
`plan_sha256` from the edited plan so the extraction prompts
|
||||
change: the repaired task runs live, and a completed task replays
|
||||
from cache when its re-extracted text is byte-identical (the
|
||||
usual case; worst case it re-runs redundantly, never stale —
|
||||
issue #32). The discard guard
|
||||
tolerates the interim subset commit: boundaries are HEAD-aware,
|
||||
so the moved baseline resets the guard with a concern instead of
|
||||
a false lost-paths `BLOCKED`.
|
||||
- **Discard:** `git checkout -- .` to drop unstaged file changes;
|
||||
`git clean -fd` / `rm BLOCKED.md` plus anything else new. main HEAD
|
||||
does NOT move.
|
||||
|
||||
@@ -73,7 +73,18 @@
|
||||
// `stash create` (a bare `git stash` would move the tree; the following
|
||||
// boundary would catch that wipe, but only after the fact). Untracked
|
||||
// files sit outside the threat model (`git checkout -- <path>` cannot
|
||||
// discard them).
|
||||
// discard them). Boundaries are HEAD-aware (issue #32): each snapshot
|
||||
// records the HEAD its diff ran against, and a boundary pair straddling
|
||||
// a HEAD move — the sanctioned commit-the-subset-then-resume repair — is
|
||||
// reset-and-surfaced as a concern instead of compared, so a replayed
|
||||
// pre-commit boundary can never count the committed subset as lost paths.
|
||||
// • Resume content-addressing (issue #32): the carrier's plan_sha256 rides
|
||||
// every plan-reading prompt, so the documented PARTIAL repair (edit the
|
||||
// plan, commit the good subset, resume) re-runs the extraction instead
|
||||
// of replaying the stale pre-edit plan from cache. Downstream, a task
|
||||
// replays iff its re-extracted text is byte-identical — typically every
|
||||
// unchanged task; the failure mode is a redundant live re-run, never
|
||||
// stale text.
|
||||
// • The script has no filesystem or shell access of its own — every
|
||||
// working-tree mutation (code, tests) happens inside an agent() call;
|
||||
// the stats.json / BLOCKED.md contents are built in-script and ride
|
||||
@@ -99,6 +110,20 @@
|
||||
// mode: 'standard' | 'mini'
|
||||
// iter_id: e.g. 'ct.2.3' (standard) or 'bugfix-<symptom>' / 'feat-<behaviour>' (mini)
|
||||
// plan_path: (standard) path under docs/plans
|
||||
// plan_sha256: (standard) sha256 hex of the plan file's CURRENT bytes
|
||||
// (`sha256sum <plan_path>`). Stamped into every plan-reading
|
||||
// prompt as a content-address (issue #32): a resume replays
|
||||
// agents cached per (prompt, opts), and a prompt that
|
||||
// carries only the plan PATH replays a stale extraction
|
||||
// after a plan edit — the documented edit-and-resume repair
|
||||
// silently no-ops. With the bytes hashed into the prompt, a
|
||||
// plan edit re-runs the extraction; downstream, a task
|
||||
// replays from cache iff its re-extracted text is
|
||||
// byte-identical — typically every unchanged task, worst
|
||||
// case a redundant live re-run, never stale text. An
|
||||
// unchanged plan replays fully from cache. Recompute it at
|
||||
// EVERY (re-)invocation — a stale or constant value
|
||||
// re-opens the no-op resume.
|
||||
// task_range: (standard, optional) [from, to] inclusive, 1-based
|
||||
// red_test_path: (mini) absolute path to the RED test from debug / tdd
|
||||
// cause_summary: (mini) 1–2 sentences (debugger cause, or tdd spec_summary)
|
||||
@@ -160,6 +185,7 @@ const {
|
||||
mode = 'standard',
|
||||
iter_id,
|
||||
plan_path,
|
||||
plan_sha256,
|
||||
task_range = null,
|
||||
red_test_path,
|
||||
cause_summary,
|
||||
@@ -180,7 +206,7 @@ if (mode !== 'standard' && mode !== 'mini') {
|
||||
// The required carrier fields per mode (the header comment above is the
|
||||
// contract). Missing or blank fields fail fast by name — the loop must never
|
||||
// glob for a plan or brief an implementer with the literal string "undefined".
|
||||
const requiredByMode = mode === 'mini' ? { iter_id, red_test_path, cause_summary } : { iter_id, plan_path }
|
||||
const requiredByMode = mode === 'mini' ? { iter_id, red_test_path, cause_summary } : { iter_id, plan_path, plan_sha256 }
|
||||
const missingFields = Object.entries(requiredByMode)
|
||||
.filter(([, v]) => typeof v !== 'string' || !v.trim())
|
||||
.map(([k]) => k)
|
||||
@@ -218,6 +244,21 @@ const STANDING_NONE =
|
||||
'Schema-bound mechanical stage: NO standing reading (no CLAUDE.md, no git log) — do exactly what is instructed ' +
|
||||
'below, nothing else.'
|
||||
|
||||
// Plan content-address (issue #32): appended to EVERY prompt that reads the
|
||||
// plan file (plan-index, plan-extract-all, plan-extract:<id>). The resume
|
||||
// cache keys on (prompt, opts); with only the plan PATH in the prompt, an
|
||||
// edit-and-resume replays the pre-edit task text from cache and the repair
|
||||
// silently no-ops. Hashing the plan's bytes into each reading prompt makes a
|
||||
// plan edit re-run the extraction — and, because the extracted text flows
|
||||
// verbatim into the downstream per-task prompts, a task replays from cache
|
||||
// iff its re-extracted text is byte-identical (typically every unchanged
|
||||
// task; the failure mode is a redundant live re-run, never stale text).
|
||||
const PLAN_ADDRESS =
|
||||
mode === 'standard'
|
||||
? `\n\nPlan content-address: sha256 ${plan_sha256} — binds this extraction to the plan's current bytes for ` +
|
||||
'resume caching; informational only, nothing to verify or act on.'
|
||||
: ''
|
||||
|
||||
// Working-directory anchor (issue #25): in worktree sessions a dispatched
|
||||
// agent's cwd can resolve to the PRIMARY checkout instead of the worktree the
|
||||
// loop operates on — observed as a per-dispatch race producing false
|
||||
@@ -508,7 +549,7 @@ const STD_VERIFY_SCHEMA = {
|
||||
const SNAP_SCHEMA = {
|
||||
type: 'object',
|
||||
additionalProperties: false,
|
||||
required: ['snapshot_sha', 'modified_paths'],
|
||||
required: ['snapshot_sha', 'modified_paths', 'head_sha'],
|
||||
properties: {
|
||||
snapshot_sha: {
|
||||
type: 'string',
|
||||
@@ -524,6 +565,12 @@ const SNAP_SCHEMA = {
|
||||
'`--no-renames` keeps a renamed file\'s old path listed as a deletion; untracked files are invisible ' +
|
||||
'here by design)',
|
||||
},
|
||||
head_sha: {
|
||||
type: 'string',
|
||||
description:
|
||||
'output of `git rev-parse HEAD` — the baseline modified_paths was diffed against; the guard compares two ' +
|
||||
'boundaries only when their baselines match (issue #32)',
|
||||
},
|
||||
},
|
||||
}
|
||||
// ---- Phase 0 — clean-tree preflight (mini mode only) --------------------
|
||||
@@ -614,7 +661,7 @@ if (mode === 'mini') {
|
||||
: 'ids of EVERY task') +
|
||||
' that the plan actually defines, each with its one-line title. Return ONLY ids and titles — do NOT include ' +
|
||||
'any task body or code block. Do not summarise, invent, reorder, or pad the list. Separately report ' +
|
||||
'`total_tasks`: the count of ALL tasks the entire plan defines, ignoring any range filter. Do not edit anything.',
|
||||
`\`total_tasks\`: the count of ALL tasks the entire plan defines, ignoring any range filter. Do not edit anything.${PLAN_ADDRESS}`,
|
||||
{ model: 'sonnet', effort: 'medium', label: 'plan-index', phase: 'Per-task loop', schema: TASK_INDEX_SCHEMA },
|
||||
)
|
||||
if (!index) {
|
||||
@@ -728,7 +775,7 @@ if (mode === 'mini') {
|
||||
const all = await agent(
|
||||
`${STANDING_NONE}${ANCHOR}\n\nRead the plan at ${plan_path}. Extract EXACTLY these tasks (1-based ids): ${expectedIds.join(', ')} — ` +
|
||||
'EACH as its VERBATIM block: the full task text as written, including any code blocks. Do not summarise, ' +
|
||||
'truncate, reorder, or merge tasks into each other. Return exactly those tasks, nothing else. Do not edit anything.',
|
||||
`truncate, reorder, or merge tasks into each other. Return exactly those tasks, nothing else. Do not edit anything.${PLAN_ADDRESS}`,
|
||||
{ model: 'sonnet', effort: 'medium', label: 'plan-extract-all', phase: 'Per-task loop', schema: TASK_TEXTS_SCHEMA },
|
||||
)
|
||||
const returned = all && Array.isArray(all.tasks) ? all.tasks : []
|
||||
@@ -746,7 +793,7 @@ if (mode === 'mini') {
|
||||
agent(
|
||||
`${STANDING_NONE}${ANCHOR}\n\nRead the plan at ${plan_path}. Extract EXACTLY task ${id} (1-based) as its VERBATIM block — ` +
|
||||
'the full task text as written, including any code blocks. Do not summarise, truncate, reorder, or merge in ' +
|
||||
'any other task. Return that one task only. Do not edit anything.',
|
||||
`any other task. Return that one task only. Do not edit anything.${PLAN_ADDRESS}`,
|
||||
{ model: 'sonnet', effort: 'medium', label: `plan-extract:${id}`, phase: 'Per-task loop', schema: TASK_TEXT_SCHEMA },
|
||||
).then((t) => ({ reqId: id, t })),
|
||||
),
|
||||
@@ -1011,17 +1058,40 @@ for (const task of tasks) {
|
||||
results.push(r)
|
||||
if (tasks.length > 1) {
|
||||
const snap = await agent(
|
||||
`${STANDING_NONE}${ANCHOR}\n\nRecord a working-tree snapshot boundary WITHOUT changing anything. Run exactly these two ` +
|
||||
`${STANDING_NONE}${ANCHOR}\n\nRecord a working-tree snapshot boundary WITHOUT changing anything. Run exactly these three ` +
|
||||
'commands and report their output:\n' +
|
||||
`1. \`git stash create "iter ${iter_id} post-task ${task.id}"\` — prints a sha (a dangling commit; it ` +
|
||||
'touches neither HEAD, nor the index, nor the working tree). Report it as snapshot_sha; empty string if ' +
|
||||
'it printed nothing.\n' +
|
||||
'2. `git diff HEAD --name-only --no-renames` — report every printed path as modified_paths.\n' +
|
||||
'3. `git rev-parse HEAD` — report it as head_sha.\n' +
|
||||
'NEVER run bare `git stash` or `git stash push/pop/apply/drop` — those MOVE the changes. Do NOT edit, ' +
|
||||
'stage, or commit anything.',
|
||||
{ model: 'sonnet', effort: 'medium', label: `snapshot:${task.id}`, phase: 'Per-task loop', schema: SNAP_SCHEMA },
|
||||
)
|
||||
if (snap && prevBoundary) {
|
||||
// HEAD-aware baseline (issue #32): each boundary's path set is a diff
|
||||
// AGAINST HEAD, so two boundaries are comparable only when they were
|
||||
// measured against the SAME HEAD. The sanctioned PARTIAL repair
|
||||
// (commit the known-good subset, then resume) moves HEAD between the
|
||||
// replayed boundaries and the fresh ones — comparing across that move
|
||||
// would report the entire committed subset as "lost" and hard-BLOCK
|
||||
// the first live task. On a baseline move the guard resets instead of
|
||||
// comparing, and surfaces the move: the loop itself never commits, so
|
||||
// a HEAD move inside a single live run means some agent committed
|
||||
// mid-run — worth eyes either way. A blank/missing sha falls through
|
||||
// to the comparison — a flaky report must not quietly disarm the guard.
|
||||
const prevHead = typeof prevBoundary.head === 'string' ? prevBoundary.head.trim() : ''
|
||||
const nowHead = typeof snap.head_sha === 'string' ? snap.head_sha.trim() : ''
|
||||
if (prevHead && nowHead && prevHead !== nowHead) {
|
||||
guardConcerns.push(
|
||||
`discard-guard baseline moved between task #${prevBoundary.taskId} and task #${task.id} ` +
|
||||
`(HEAD ${prevHead} -> ${nowHead}) — expected only on a resume after the orchestrator committed a ` +
|
||||
'known-good subset (the sanctioned PARTIAL repair); in a single live run with no interim commit it ' +
|
||||
'means an agent moved HEAD mid-run — investigate. The guard reset its baseline; this boundary pair ' +
|
||||
'went uncompared.',
|
||||
)
|
||||
} else {
|
||||
const now = new Set(snap.modified_paths || [])
|
||||
const lost = [...prevBoundary.paths].filter((p) => !now.has(p))
|
||||
if (lost.length) {
|
||||
@@ -1038,10 +1108,13 @@ for (const task of tasks) {
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!snap) guardConcerns.push(`discard-guard snapshot after task #${task.id} did not return — that boundary went unchecked`)
|
||||
// A non-returning snapshot agent skips the NEXT comparison (prevBoundary
|
||||
// null) rather than false-positiving against a stale boundary.
|
||||
prevBoundary = snap ? { taskId: task.id, sha: snap.snapshot_sha || null, paths: new Set(snap.modified_paths || []) } : null
|
||||
prevBoundary = snap
|
||||
? { taskId: task.id, sha: snap.snapshot_sha || null, paths: new Set(snap.modified_paths || []), head: snap.head_sha || null }
|
||||
: null
|
||||
}
|
||||
if (r.outcome === 'BLOCKED') break // do not skip ahead — task ordering dependencies are unknown
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user