F2b: connect the real workspace (partial export, failed-run audit, durable safety findings, commit export)

[mchwang]

Follow-up to #68 (F2b: per-item execution and the runner's commit step). Independent review of #68 found five gaps. They open only once the real lane D workspace from #66 part 2 replaces the test fake: until then, no production code runs ItemExecutor. They were deferred from #68 on purpose. Related: #66, #22, #51.

1. Partial-output export before task storage is removed

docs/implementation/runner-lifecycle.md (task storage row): "settled → the partial-output export if the attempt did not complete → the terminal write, which stores diagnostic_ref → removeTaskFilesystems". plan-format.md, "After each run": for a safety violation, "preserve the rejected output separately for diagnosis".

• Today: TaskWorkspace has no export operation. A writable attempt that does not complete, including a safety violation, has its storage removed with no export and no diagnostic_ref.

2. Audit the changes of an attempt whose agent exited non-zero

• Today: finish() runs only on a clean exit. An agent that makes unsafe changes (for example under .git) and then exits non-zero never reaches needs human: its work is discarded, and the task can be retried.
• Contract: "Any safety violation stops tests and commits and moves the task to needs human."
• Needs: the same inspection as item 1, run on failed attempts too.

3. Durable safety findings

• Today: SafetyFindings is in memory. A finding is lost when:

  • the terminal write fails (the attempt stays active, so the move to needs human is refused);
  • the process crashes between the terminal write and that move.

  After a restart, startup recovery finalizes the attempt as "Interrupted", and the task can be retried.

• Proposed: a durable mark on the attempt, written like the first reason, before the terminal write. The terminal write, startup recovery and admission (retry) would all respect it.

4. Act on a finding outside ItemExecutor

• Today: an execute attempt started through runner.start or runner.retry (for example the /api/runner retry) records a finding that nothing takes. The task stays running, and the finding stays in memory.
• Fix: item 3 covers this. The move to needs human must not depend on which caller started the attempt.

5. Take the runner's commit out of task storage before release

• Today: the runner commit exists only inside task storage, and release removes it:
  • the next item's materialize(head) has no way to get that object;
  • the review screen's HEAD observation (ReviewService.load) compares config.repository HEAD with the snapshot head. It would then record the old HEAD as a new snapshot, which rolls back the base and makes the next attempt stale.
• Plan (D: inspect task changes and make runner commits (needed by F2) #66 decision 1): exportTaskCommit(storage, {base, head}) bundles the commit into a runner-owned host repository, and F verifies the SHA. TaskWorkspace needs that step, run after the terminal write and before release.
• Also needed: the review screen's HEAD observation must account for runner commits.

🤖 Generated with Claude Code

[mchwang]

Also from the round-3 review of #68: once the review screen observes a HEAD that differs from the runner commit (item 5 above), the current snapshot moves away from a scope checkpoint's snapshot, and approveContinuation (which requires checkpoint.snapshotId to equal the current snapshot) then refuses to approve continuing. Fixing the HEAD observation should cover this; add a test that a paused task can still be approved after a review load.

[mchwang]

Round 4 of the #68 review found that F2a's audit types (core/run-audit.ts, merged in #67) do not match what lane D's inspection actually returns (agents/container/changes.ts, merged in #85). Nothing hits this until the real workspace replaces the test fake, but connecting it needs the following:

1. through-link declared links are never refused before launch.
   • TaskWorkspace.snapshotDeclaredLinks returns an opaque value, and F never reads it.
   • D does not throw on through-link: it reports the status in the DeclaredLinkSnapshot and leaves the refusal to F.
   • Contract (Publishing step 2): "A declared link whose snapshot status is through-link is refused before launch."
   • Needed: F must read the snapshot's links[].status before launch.
2. Every legitimate edit of a declared symlink would be rejected.
   • D's linkTargetChanges are LinkTargetChange objects, not strings, so the audit's validation now fails the whole report.
   • The retargeted entry for a link the item itself edits must be judged by the plan's link rules, not counted as a target change. That exemption is not implemented.
   • D never sets linkTargetTraversesLink: traversal is carried by the snapshot and by status changes. So auditRun rejects every symlink edit as "not checked".
   • Needed: build auditRun on D's TaskChangeManifest type, not a hand-written copy.
3. Git-ignored entries (policy decision needed).
   • D marks new ignored paths with ignored: true, and reports an ignored new directory as a single add of type directory.
   • F's type has no ignored field, so:
     • a test run that creates __pycache__/, node_modules/ or target/ becomes an "Unexpected directory entry" safety violation;
     • an ignored new file (for example coverage.xml) is counted out of scope and staged in the runner commit.
   • Decide: how ignored build output is treated (excluded from scope and commit, or something else), then implement it.

[mchwang]

Two more from the round-8 review of #68:

• Durable safety findings (item 3) should keep the violation text. When a stop, a stale context or the time limit wins the settlement, the attempt row records only that outcome (for example "Cancelled."). The executor still moves the task to needs human, but the reason is kept only in memory and in the returned outcome.
• One coordinator per process and Store. executionDeps.prepare refuses every kind but execute, and the coordinator uses finish for every kind whenever it is defined. Connecting the real runner needs deps that dispatch by attempt kind (execute and fix versus review and check), or one coordinator per kind.

[mchwang]

From the round-9 review of #68: merge admission does not know about owed findings. A safety finding owed at a human gate, or a scope pause refused because a merge was already in progress, is acted on only by the task's next ItemExecutor run. If a person moves the task from the gate straight to review and merges, the finding never reaches needs human. Once findings and owed pauses are durable (item 3), beginMergeAttempt should refuse while the task has one.

[mchwang]

From the round-11 review of #68: finish treats any inspection error as a safety finding unless it is the stop's own abort (signal.reason or an AbortError). This fails closed on purpose. So when D's inspectTaskChanges is aborted (a user stop or shutdown), it must reject with signal.reason or an AbortError, never with a generic Error from the killed docker exec. Otherwise a stop during inspection sends the task to needs human, and in shutdown it skips the requeue.

[mchwang]

From the round-17 review of #68: shutdown does not await an ItemExecutor run. After the last item's attempt settles, the executor writes its pause or escalation through the shutdown capability, but runner.close() awaits only the coordinator's own jobs, and step 6 awaits only the question and suggestion coordinators. When the executor is wired to the server, shutdown must await in-flight runTask calls before it closes the Store (step 7). Otherwise, whether that write lands depends on timing. A lost pause is re-derived on the next run; a lost safety finding is the in-memory gap of item 3.

[mchwang]

From the round-18 review of #68: retry admission ignores an owed safety finding. When a finding is owed at a human gate and a person moves the task to queued, /api/runner shows the failed attempt as retryable. runner.retry then admits the violating item again in a fresh workspace, before any ItemExecutor run escalates the finding. Once findings are durable (item 3), retry admission should refuse while one is owed, like merge admission.

[mchwang]

From the round-21 review of #68: if shutdown starts between items, runTask returns stopped with the next item not started. The task stays running and its latest attempt is completed. Startup recovery requeues only tasks whose latest attempt was interrupted or stopped by shutdown, so this task is never resumed. When the executor is wired in, recovery (or the I3 requeue) must also resume a running task whose plan has items left and no active attempt.

[mchwang]

Item 5 changes with #90 (#66 part 2). There is no exportTaskCommit. Instead:

• commitTaskChanges(storage, { base, linkSnapshot, digest, message, trailers, author, committer, imageId }) returns { head, unchanged, bundle }.
• F fetches bundle into its runner-owned repository and checks that TASK_COMMIT_REF (refs/heads/codeboost) is head.
• Do this inside finish(), before the terminal write. After that, release loses nothing.
• If the publish is refused, delete the fetched ref.

Adapter notes for TaskWorkspace.commit:

• It needs author and committer (name, email, and date as "<seconds> <offset>", never -0000). D takes nothing from the environment.
• paths is not needed. D commits exactly the approved manifest.
• TaskCommitRefused means needs human.

Found in the PR's review; these block real runs:

• Symlink changes: core/run-audit.ts requires linkTargetTraversesLink === false for any symlink change. D never reports that field, so every symlink change is a violation today.
• Directories: both the audit and the commit refuse directory entries. So a file replaced by a package directory, and new ignored directories (__pycache__ after tests), go to needs human.
• Ignored files: files base's own ignore rules ignore, written into existing directories, are committed (the part 1 contract). The audit marks them out of scope.

🤖 Generated with Claude Code

[mchwang]

Decisions before implementation (2026-10-01)

#	Question	Decision
1	Where do runner commits live on the host?	A runner-owned bare repository under the runner root, one per configured repository, seeded from config.repository. Bundles from commitTaskChanges are fetched into it under a per-task ref. materialize clones from it, the push reads from it, and the review screen's HEAD observation reads that task ref. codeboost never writes the user's repository.
2	Base-ignored new files (an agent-written .env, *.pyc)	Safety violation: any manifest change with ignored: true sends the task to needs human, and nothing is committed.
3	linkTargetTraversesLink	D reports it. The inspection resolves each changed symlink's new target, one part at a time, as the snapshot does, and reports whether the way passes through another link. The audit keeps its === false check. (A D change, in part 1 of this issue.)
4	Scope	Split. Part 1: the real TaskWorkspace over D, the runner-owned repository and bundle fetch, the audit on D's manifest shape (with decisions 2 and 3), and HEAD observation (item 5). Part 2: partial export and diagnostics, auditing failed runs, durable safety findings, findings outside ItemExecutor (items 1–4). Wiring the runner into web/cli.ts and calling startup recovery at server start becomes its own issue.

Facts behind these, from research on main plus #90:

• No production code builds RunnerDeps: web/cli.ts passes none, so the coordinator is null.
• recoverStartup has no production caller. Its exportTaskDiff dependency does not match D's signature: D takes base, imageId and metadataBaseline, and returns { diff, truncated }.
• Nothing persists metadataBaseline, which a recovery handle needs.
• ReviewService.load compares config.repository HEAD with the snapshot head. After a runner commit it would record the old head as a new snapshot.

🤖 Generated with Claude Code

[mchwang]

Done in two parts:

• Part 1: F2b: connect the real lane D workspace (#87, part 1) #92
  • the real TaskWorkspace over lane D;
  • the runner-owned repository and bundle fetch;
  • the audit on D's manifest shape, with ignored files as violations and D reporting linkTargetTraversesLink;
  • the review and Ask reading runner commits from the runner-owned repository (item 5).
• Part 2: F2b: durable safety findings, failed-run audit and partial output (#87, part 2) #94
  • durable safety findings, acted on by the terminal write and by startup recovery, however the attempt was started (items 3 and 4);
  • the failed-run audit (item 2);
  • partial output with retention (item 1);
  • through-link declared links refused before launch.

Three follow-ups from this thread are now on #91, where the runner gets wired into the server:

• one coordinator per attempt kind;
• shutdown awaiting ItemExecutor runs;
• resuming a task that shutdown stopped between items.

🤖 Generated with Claude Code