Proposal: Per-Step Git Worktree Isolation for Mutation Steps

[ShinonKagura]

Proposal: Per-Step Git Worktree Isolation for Mutation Steps

Feature proposal — looking for maintainer feedback on shape before writing any code.

Note: this proposal explicitly addresses 2 implementation concerns you raised in PR #2 (worktree uncommitted-loss + worktree-isolated steps ignoring step.cwd). I want to bake the fixes into the design from the start, not punt them.

Motivation

Currently, mutation-capable steps in a graph run with edit/write/bash directly in the invocation cwd. If a parent runs two mutation steps in parallel (or even one mutation step alongside an audit step that reads), they all share the same working tree:

• Edit-collisions when two steps touch the same file
• Tests can't safely run mutation evidence in parallel
• No automatic per-step diff/patch evidence — operator has to inspect the working tree manually after the fact
• One step's incomplete work is visible to dependent steps before the producing step finalizes
• Failed mutation steps leave the working tree dirty, blocking subsequent runs

Use cases this would solve:

1. Map-reduce mutations: shard N edits across worktrees, fan-in via review
2. Tournament patterns: 3 worker agents propose alternative fixes, reviewer picks one
3. Safe experiment loops: cap each iteration's blast radius to a per-step worktree
4. Mutation evidence: produce a .patch per step automatically as terminal artifact

Sketch (open to revision)

Opt-in per-step

{
  action: "start",
  graph: {
    authority: {
      // ...
      allowMutationWorktree: true,  // gate at graph-authority level
    },
    steps: [
      {
        id: "patch-file-x",
        agent: { ref: "package:worker", tools: ["read", "grep", "find", "ls", "edit"] },
        task: "Edit file x to ...",
        isolation: "worktree",                              // ← opt-in
        worktreeSetup: {                                    // ← optional setup
          propagateNodeModules: true,                       // symlink node_modules/ into worktree
          symlinkPaths: ["data/", ".env.local"]             // explicit extra symlinks
        }
      }
    ]
  }
}

Lifecycle

1. Step start with isolation:"worktree":
   - Verify invocation-cwd is a git repo (fail-closed if not)
   - Verify working tree clean (fail-closed if dirty — see I3 below)
   - Create worktree at $TMPDIR/pi-multiagent-wt-<runId>-<stepId>/
   - branchName = pi-multiagent/<runId>/<stepId>
   - baseCommit = HEAD at start time
   - Run child with cwd = worktree path

2. Step terminal (succeeded, failed, timed_out, canceled):
   - Capture diff stat and patch as terminal artifact (see I1 below)
   - Remove the worktree (git worktree remove + delete dir)
   - Optionally prune the branch (preserve on failure for postmortem?)

Known concerns from PR #2 review — addressed here

I1: "worktree teardown appears to capture committed changes only, so normal uncommitted edit/write changes can be lost"

You're right. The naive teardown does:

git diff --stat <baseCommit>..HEAD  // captures only commits
git diff       <baseCommit>..HEAD  // captures only commits

If a child edits files via edit/write and never calls git add && git commit, those edits are not in HEAD and disappear when the worktree is removed.

Proposed fix (3 options, looking for your preference):

• (a) Auto-stage + auto-commit before teardown: run git add -A && git commit -m "step <id> output" --allow-empty in the worktree as the last step before diff capture. Mutation diff is then always baseCommit..HEAD. Side-effect: creates a commit on the worktree branch, which we prune anyway. Simple, but feels heavy-handed.
• (b) Capture uncommitted via working-tree diff: change to git diff <baseCommit> (no ..HEAD), which compares working tree (incl. unstaged) against base. Requires explicit git add -N for untracked files to appear in diff, or a separate git status --porcelain + per-file copy.
• (c) Both diff types: capture committed.patch (baseCommit..HEAD) AND working.patch (baseCommit). Operator sees both. Most evidence, most artifacts.

I'd lean (a) — simplest mental model, single artifact, child doesn't have to know about staging. But (b) is more "minimal change" since it doesn't synthesize commits.

Your preference?

I3: "worktree-isolated steps appear to ignore step.cwd"

You're right. Current sketch resolves the child's cwd to the worktree root, but ignores step.cwd (which the user might have set relative to invocation-cwd).

Proposed fix: when isolation: "worktree" and step.cwd is set, resolve step.cwd relative to the worktree root, not the invocation cwd. Verify the resolved path exists in the worktree (after symlink-setup) before launching the child.

Edge case: if step.cwd points to a path that exists in invocation-cwd but not in worktree (e.g. a build output dir that wasn't symlinked), fail-closed at launch time with a clear error. Operator's responsibility to declare it in worktreeSetup.symlinkPaths if needed.

Open questions for maintainer

1. Authorization shape

• Single gate authority.allowMutationWorktree (above) — yes/no?
• Or per-step opt-in only valid when step has mutation tools, otherwise diagnostic at planning?
• Both? Proposed: authority.allowMutationWorktree is the master switch, individual steps opt-in via isolation: "worktree". Non-mutating steps requesting worktree → diagnostic at planning ("worktree isolation is for mutation steps").

2. Concurrent worktree limits

• Cap on concurrent worktrees per run? (each consumes inode + disk)
• Proposed: bounded by MAX_CONCURRENCY (which already exists), no separate worktree-cap.

3. Dirty tree precondition

• If invocation-cwd has uncommitted changes at the start of a worktree-isolated step → fail-closed, or stash+restore, or just warn?
• Proposed: fail-closed. Operator should commit or stash explicitly. Surface dirty entries in the error (which is what FIX-7 from PR feat(v0.10.0): F5 worktree isolation + B1a/B1b persistent runs + G1/3/4/6/7 + NEU-B + NEU-C + 4 FIXES #2 added — that's worth keeping).

4. Branch retention on failure

• On step failure, do we prune the worktree branch or keep it for postmortem?
• Proposed: keep on failed / timed_out, prune on succeeded / canceled. Operator can manually inspect failed-step worktree branches.

5. node_modules / .env propagation

• worktreeSetup.propagateNodeModules: true → symlink (not copy)
• Symlink = changes inside node_modules visible across parent + worktree. That's usually fine but operator should know.
• Proposed: doc-string warning, no warning at runtime.

6. step.cwd outside invocation-cwd

• Some users want step.cwd: '../other-repo'. Currently this is denied (lexical escape probe).
• Worktree isolation on a step.cwd outside invocation-cwd: what does that mean?
• Proposed: deny worktree-isolated steps with step.cwd outside invocation-cwd. The invocation-cwd git repo is the only one we own; other repos are user-managed.

7. Strict-no-fallback

• If worktree creation fails (not a git repo, dirty tree, disk full, etc.), should we fail-closed or silently fall back to invocation-cwd?
• Proposed: strict fail-closed. Silent fallback would defeat the whole point — operator asked for isolation, didn't get it, child runs unsandboxed without knowing. Fail-closed at step start with explicit error.

What I'd write if you say go

• New file extensions/multiagent/src/mutation-worktree.ts (~300 lines)
  • prepareWorktreeForStep(runId, stepId, repoRoot, agent, diagnostics)
  • teardownWorktreeForStep(state, artifactStore) — includes fix for I1
  • pruneOrphanWorktreeBranches(repoRoot) for cleanup
• Add isolation, worktreeSetup to StepSchema and TeamStepSpec
• Add allowMutationWorktree to AuthoritySchema
• Wire into detached-run: per-step worktree lifecycle around child launch
• Plumb step.cwd resolution through worktree-aware resolver (fix for I3)
• Tests covering: success path, dirty-tree fail-closed, uncommitted capture (I1), step.cwd in worktree (I3), branch cleanup, failure preservation, symlinkPaths setup

Roughly ~600-800 lines + tests, plus the I1/I3 fixes are bundled by design.

Status

This is the largest of the 3 proposal-issues. I have a working implementation from PR #2 — but it has both I1 and I3 bugs you flagged. I'd rather fix them in the proposal stage than re-submit the same broken code.

The I1 fix in particular changes one of the central design decisions (auto-commit vs working-tree-diff). I want your call on (a)/(b)/(c) before I touch code.

---

Decision needed from you

• Direction OK in principle?
• Critical: which I1 fix variant (a/b/c)?
• Critical: I3 fix approach OK (resolve step.cwd inside worktree, fail-closed if missing)?
• Preferred answers to the 7 open questions — especially Q1 (authorization shape) and Q7 (strict-no-fallback)?
• Any showstopper concerns I should know before writing code?

If green with answers, I'll open a focused PR against current main with I1 + I3 fixes built in from the start.

[tiziano-contorno]

This remains a valid isolation problem, but it is intentionally not part of the current maintainer integration PR (#13).

Per-step worktree isolation is high-risk and needs a separate design pass around execution-context ownership, dirty-parent fail-closed behavior, symlink/secrets handling, no silent fallback to shared cwd, no auto-stash/restore, package-owned cleanup/retention, and artifact/patch evidence semantics.

Keeping this open as design backlog rather than reviving the broad #2 shape or bundling it into this release batch.

[tiziano-contorno]

Closing this backlog proposal after the 0.9.8 release. Per-step mutation worktrees are not part of the shipped contract and remain too broad/high-risk without a separate design for dirty-parent denial, cwd ownership, symlink/secrets handling, no fallback, cleanup/retention, and patch evidence. If revived, it should come back as a fresh focused proposal rather than an open release tail.

[tiziano-contorno]

Reopening as backlog / potential future PR. This was not a 0.9.8 release blocker and is not part of the shipped 0.9.8 contract; it remains a separately scoped future design candidate.