Tell a coding attempt about work an earlier attempt left in the workspace (restart note) #377

Tell a coding attempt about work an earlier attempt left in the workspace

Component: every file belongs to the lab Go module (manifest lab/go.mod).

Why: this adopts DeepSeek's super-agent restart note. When an assignment restarts, they tell the worker to check its directory and history before repeating anything. Forgecell keeps the workspace on every later coding attempt: after forgecell recover, after amend --run, and after a plan revision.

resumeApprovedIssue requires the tree to equal p.PausedTree for revision 2 and later (lab/internal/molecule/readiness_run.go around 1121). But the coding request never says that work already exists. The only trace is approvedPlan.pausedTree, a bare tree hash.

The one existing note, resumeInstruction (lab/internal/molecule/run.go:153), covers only the automatic in-stage resume after a provider error. So a coder that starts on a kept tree can redo or revert work it never saw.

Build:

1. When the note applies: a coding attempt for a plan with Revision > 1 whose kept tree differs from the plan's capture base, that is, the captured tree before coding has changes. codingRequest (readiness_run.go around 1295) then appends a fixed retainedWorkInstruction to instruction.

2. What the note says: an earlier coding attempt already changed these files in this workspace, and it may have been interrupted, recovered or stopped for this amendment. Read them, and the plan's acceptance, before editing. Build on the earlier changes, and don't redo or revert them unless the plan requires it. If an earlier change contradicts the plan, report that instead of silently discarding it.

3. The file list:

   • It comes from the pre-coding capture's changed paths (the capture the existing PausedTree check already computes), in capture order.
   • At most 20 paths; beyond that the note adds "and N more".
   • Only paths that pass readiness.ValidatePath and are in the approved scope.
   • Each path is quoted as a JSON string inside the instruction text, so a path can never inject prose.

4. No contract change:

   • No new request key, so the adapter protocol is unchanged.
   • The plan digest is unchanged, because the note is derived at run time, never stored in the plan.
   • It joins the recorded invocation like the other instruction appends, so the ledger shows exactly what the coder was told.

5. Ledger note: a bounded note says the retained-work instruction was given, with the file count.

6. Unchanged:

   • revision-1 attempts;
   • attempts on a kept tree with no changes;
   • checks-only continuations, which invoke no coder;
   • resumeInstruction.

   If the automatic resume and this note both apply, the request carries both.

Scope (files):

• Edited: lab/internal/molecule/readiness_run.go, lab/internal/molecule/run.go (the new constant next to resumeInstruction)
• New: lab/internal/molecule/retained_work_test.go
• Existing _test.go files stay unchanged. If a base test pins the exact instruction of a revision-2 or later request on a changed tree, name it in the plan as an intended protected delta.

Tests (new file, a fake harness that records the request; no network and no sleeps):

• amend --run on a kept tree with 2 changed files: the instruction carries the note, naming exactly those 2 paths as JSON strings.
• A revision-2 attempt on an unchanged tree has no note.
• A revision-1 attempt has no note.
• 25 changed paths are listed as 20 plus "and 5 more".
• A path containing a quote or a newline is JSON-escaped and can't break the instruction.
• The plan digest is identical with and without the note.
• The recorded invocation and the ledger note both show the note.
• Combined with an operator note, both appear, in a fixed order.

Done when

• go test -timeout 20m ./internal/molecule/... passes from lab/.

Plan checks for this slice

Use one combined targeted go test -timeout 20m ./internal/molecule/... check. Every plan check leaves reviewedAcceptance and independentProvenance empty.

Review checklist (read before coding)

The adversarial review checks these every time. Recent slices failed review on all of them, so build them in from the start:

• Bounds: sizes, counts, time and total work are bounded, and hitting a bound fails safe (incomplete or truncated, never silently accepted).
• Filesystem safety: paths cannot traverse. Use Lstat and O_NOFOLLOW, and check symlinks and ownership. Watch for TOCTOU races on shared dirs (/tmp). Only ever delete Forgecell-owned, marked paths.
• Untrusted input: issue text, repository files, model and tool output, and ledgers can be malformed, huge or hostile. Validate them, never interpolate them into HTML or a shell, and never let them trigger retries or approvals.
• No local leaks: no absolute local paths, environment variables or credentials in output, logs, the UI or ledgers.
• Contract changes: a changed adapter invocation bumps the protocol, and approved digests still bind exactly what was shown.
• Tests: deterministic. Use no wall-clock budgets (use readiness markers or seams), write nothing into the user's real temp or home, and cover the failure path as well as the happy path.

Plan checks (for intake)

Check category must be one of regression, candidate or setup. Use targeted package tests only as plain go test -timeout 20m commands (for example go test -timeout 20m ./internal/<pkg>/...; without -timeout, go test stops a package after 10 minutes, and cli and molecule exceed that on a loaded machine); the protected view adds -json itself when protected deltas are declared. The Formula baseline already runs the full suite once, so don't add another full go test ./... check, and don't repeat its go vet or gofmt checks. A plan may hold at most 16 checks in total, counting the 5 Formula baseline checks, so prefer one combined targeted command over one check per package.

• Keep base tests compiling: the protected view runs the base _test.go files against your code. Never change the signature of an existing function, method or type that a base test file calls. Add a new function or field instead. A protected build failure can't be declared as an expected delta.
• Keep base tests runnable: changed defaults or behaviour must not make a base test hang or panic. A package that aborts in the protected view hides every test after it, and nothing can be declared. Keep the old default when unconfigured and opt in to the new one.
• Plan checks: the operator's visual or browser check is a manual step after delivery, never a plan check. Every check leaves reviewedAcceptance and independentProvenance empty.
• Plan structure: new files need a parent component (in this repo, under lab/; a new root-level file blocks intake). A check's definitions must never include a file the change edits. A file the check only reads as data under test is not a definition.
• Scope lists files, not directories: list every new file path, including each golden fixture under testdata/. A directory entry doesn't cover the files inside it, and the scope audit blocks them.