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

Activity

1. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
2. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
3. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
4. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
5. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
6. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
7. mchwang commented on Sep 30, 2026

   

   mchwang

   on Sep 30, 2026

   ContributorAuthor

   More actions

   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.
8. mchwang commented on Oct 1, 2026

   

   mchwang

   on Oct 1, 2026

   ContributorAuthor

   More actions

   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.
9. mchwang commented on Oct 1, 2026

   

   mchwang

   on Oct 1, 2026

   ContributorAuthor

   More actions

   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
10. mchwang commented on Oct 1, 2026

    

    mchwang

    on Oct 1, 2026

    ContributorAuthor

    More actions

    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
11. mchwang

    added 2 commits that reference this issue on Oct 1, 2026

    F2b: connect the real lane D workspace (#87, part 1) (#92)

    ...

    f21b8ac

    F2b: durable safety findings, failed-run audit and partial output (#87,…

    ...

    dfda871
12. mchwang commented on Oct 1, 2026

    

    mchwang

    on Oct 1, 2026

    ContributorAuthor

    More actions

    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
13. mchwang

    closed this as completedon Oct 1, 2026