fix(workers): isolate each worker in its own git worktree

.agents/skills/pr-gates/SKILL.md

@@ -29,6 +29,31 @@ When touching worker lifecycle, cancellation, retries, state transitions, or cac
 - Run targeted tests in addition to broad gate runs.
 - Capture the exact command proving the behavior.
 
+## Commit Hygiene — Never Sweep Sibling Edits (issue #224)
+
+Workers now run in an isolated per-worker `git worktree` (see
+`src/agent/worker_workspace.rs`), but keep these rules as a second layer so a
+commit can never contain files this worker did not touch:
+
+- Stage **explicit paths** you changed: `git add <path> [<path> ...]`. Never
+  `git add -A` or `git add .` — a blind sweep can pick up unrelated content.
+- Verify a **clean, intentional** stage before committing: run
+  `git status --porcelain` and confirm every staged path is one you edited.
+- If the working tree is unexpectedly dirty at handout (files you did not
+  create), stop and investigate rather than committing over them.
+
+Anti-pattern (what caused #211/#218/#219):
+
+```sh
+# BAD — sweeps up whatever else is dirty in the tree, including sibling edits
+git add -A && git commit -m "..."
+
+# GOOD — stage only what you changed, then verify before committing
+git add src/foo.rs src/bar.rs
+git status --porcelain   # confirm every staged path is yours
+git commit -m "..."
+```
+
 ## Migration Safety
 
 - Never edit an existing file in `migrations/`.

src/agent.rs

@@ -18,6 +18,7 @@ pub mod prompt_snapshot;
 pub mod status;
 pub mod wake;
 pub mod worker;
+pub mod worker_workspace;
 
 pub(crate) fn panic_payload_to_string(panic_payload: &(dyn std::any::Any + Send)) -> String {
     panic_payload

src/agent/channel_dispatch.rs

@@ -636,6 +636,43 @@ pub async fn spawn_worker_from_state(
     result
 }
 
+/// Provision and attach a per-worker isolated workspace (issue #224).
+///
+/// Gives the worker its own `git worktree` per repo in `shared_workspace`, so
+/// concurrent workers can't see or clobber each other's edits. Provisioning is
+/// best-effort: on failure the worker is returned unchanged and falls back to
+/// the shared workspace, so a provisioning hiccup never blocks dispatch.
+///
+/// # Completeness contract (issue #224, finding [3])
+///
+/// A successful `provision(..)` does **not** guarantee every repo was isolated —
+/// per-repo failures are logged and skipped rather than made fatal. The returned
+/// [`WorkerWorkspace`](crate::agent::worker_workspace::WorkerWorkspace) carries
+/// an explicit [`Isolation`](crate::agent::worker_workspace::Isolation) signal
+/// (read via `.isolation()`) recording whether isolation was *complete*,
+/// *partial*, or *absent*. We attach the workspace regardless so the worker
+/// still benefits from whatever isolation was achieved; the worker's
+/// `tool_workspace` selection then consults that signal and refuses to
+/// *silently* fall back to the shared checkout when isolation was incomplete
+/// (finding [1]).
+async fn attach_isolated_workspace(worker: Worker, shared_workspace: &std::path::Path) -> Worker {
+    match crate::agent::worker_workspace::WorkerWorkspace::provision(shared_workspace, worker.id)
+        .await
+    {
+        Ok(workspace) => worker.with_isolated_workspace(workspace),
+        Err(error) => {
+            tracing::warn!(
+                error = %error,
+                error_debug = ?error,
+                worker_id = %worker.id,
+                shared_workspace = %shared_workspace.display(),
+                "failed to provision isolated workspace — worker will use shared workspace"
+            );
+            worker
+        }
+    }
+}
+
 /// Inner implementation of worker spawning, separated so the caller can
 /// handle task reservation cleanup in a single place.
 async fn spawn_worker_inner(
@@ -763,6 +800,13 @@ async fn spawn_worker_inner(
         .resolve_model("worker")
         .map(String::from);
 
+    // Provision a per-worker isolated workspace (issue #224): each worker
+    // gets its own `git worktree` per repo so concurrent workers can't see or
+    // clobber each other's edits. Best-effort — if provisioning fails, the
+    // worker falls back to the shared workspace with a warning rather than
+    // failing dispatch.
+    let shared_workspace = state.deps.runtime_config.workspace_dir.clone();
+
     let worker = if interactive {
         let (worker, input_tx, inject_tx) = Worker::new_interactive(
             Some(state.channel_id.clone()),
@@ -778,6 +822,7 @@ async fn spawn_worker_inner(
             worker_context.wiki_write,
             worker_model_override,
         );
+        let worker = attach_isolated_workspace(worker, &shared_workspace).await;
         let worker_id = worker.id;
         state
             .worker_inputs
@@ -805,6 +850,7 @@ async fn spawn_worker_inner(
             worker_context.wiki_write,
             worker_model_override,
         );
+        let worker = attach_isolated_workspace(worker, &shared_workspace).await;
         state
             .worker_injections
             .write()

src/agent/cortex.rs

@@ -1778,6 +1778,18 @@ pub fn spawn_cortex_loop(deps: AgentDeps, logger: CortexLogger) -> tokio::task::
         drop(prompt_engine);
 
         let cortex = Cortex::new(deps.clone(), system_prompt);
+
+        // Reap orphaned per-worker workspaces left by crashes or retained
+        // failures (issue #224), keeping recent ones for forensics. Best-effort.
+        {
+            let shared_workspace = deps.runtime_config.workspace_dir.clone();
+            if let Err(error) =
+                crate::agent::worker_workspace::reap_orphaned(&shared_workspace).await
+            {
+                tracing::warn!(%error, "failed to reap orphaned worker workspaces on startup");
+            }
+        }
+
         let mut event_rx = deps.event_tx.subscribe();
         let mut memory_event_rx = deps.memory_event_tx.subscribe();
         let mut tool_output_rx = deps.tool_output_tx.subscribe();

src/agent/worker.rs

@@ -274,6 +274,12 @@ pub struct Worker {
     /// loop reads it at each iteration and short-circuits to
     /// `WorkerOutcome::Blocked` when populated.
     pub blocked_signal: BlockSignal,
+    /// Per-worker isolated workspace (issue #224). When set, the worker's
+    /// shell and file tools operate in this directory — its own `git worktree`
+    /// per repo — instead of the shared `workspace_dir`, so concurrent workers
+    /// can't see or clobber each other's edits. Released on successful
+    /// completion; retained for forensics (and reaped later) on failure.
+    pub isolated_workspace: Option<crate::agent::worker_workspace::WorkerWorkspace>,
 }
 
 impl Worker {
@@ -339,6 +345,7 @@ impl Worker {
                 worker_wall_clock_timeout_secs,
                 segments_run: std::sync::Arc::new(std::sync::atomic::AtomicUsize::new(0)),
                 blocked_signal: new_block_signal(),
+                isolated_workspace: None,
             },
             inject_tx,
         )
@@ -421,6 +428,77 @@ impl Worker {
         (worker, input_tx, inject_tx)
     }
 
+    /// Attach a per-worker isolated workspace (issue #224).
+    ///
+    /// When set, the worker's shell and file tools operate in this directory
+    /// instead of the shared `workspace_dir`, giving concurrent workers their
+    /// own `git worktree` per repo. Consumed builder-style so it reads
+    /// naturally at the call site.
+    pub fn with_isolated_workspace(
+        mut self,
+        workspace: crate::agent::worker_workspace::WorkerWorkspace,
+    ) -> Self {
+        self.isolated_workspace = Some(workspace);
+        self
+    }
+
+    /// The base directory the worker's tools should use: the isolated
+    /// workspace when one was provisioned, otherwise the shared workspace.
+    ///
+    /// # Non-recoverable degradation guard (issue #224, finding [1])
+    ///
+    /// When an isolated workspace was provisioned we must not *silently* fall
+    /// back to the shared checkout — that would re-introduce the exact
+    /// cross-worker contamination race #224 exists to prevent. We consult the
+    /// workspace's [`Isolation`](crate::agent::worker_workspace::Isolation)
+    /// completeness signal (finding [3]) to tell apart:
+    ///   - **complete isolation** (every repo isolated, or the degenerate
+    ///     no-repo case): safe to use the isolated root, or safe to fall back to
+    ///     the shared workspace when the isolated root is empty because there was
+    ///     genuinely nothing to isolate.
+    ///   - **incomplete isolation** (repos existed but some/all failed): a
+    ///     `debug_assert!` trips in debug builds because this is a programmer- or
+    ///     environment-level fault the provisioning path should have surfaced;
+    ///     in release builds we log at `error` (not `warn`) so the loss of
+    ///     isolation is loud and greppable rather than silent, then fall back.
+    fn tool_workspace(&self) -> PathBuf {
+        match &self.isolated_workspace {
+            Some(ws) if ws.has_worktrees() => ws.root().to_path_buf(),
+            Some(ws) => {
+                // A workspace was provisioned but isolated no repos. This is
+                // only safe when there was genuinely nothing to isolate
+                // (`Isolation::Fully` over zero repos). If repos existed and
+                // none isolated, falling back to the shared workspace silently
+                // degrades — treat that as non-recoverable in debug and loud in
+                // release. See the guard doc above.
+                let isolation = ws.isolation();
+                debug_assert!(
+                    isolation.is_complete(),
+                    "isolated workspace provisioned with no worktrees despite \
+                     repos being present ({isolation:?}) — silent fallback to the \
+                     shared checkout would re-open issue #224"
+                );
+                if !isolation.is_complete() {
+                    tracing::error!(
+                        worker_id = %self.id,
+                        ?isolation,
+                        "isolated workspace has no worktrees but repos were present — \
+                         isolation FAILED; falling back to shared workspace (this can \
+                         re-open the #224 contamination race)"
+                    );
+                } else {
+                    tracing::warn!(
+                        worker_id = %self.id,
+                        "isolated workspace has no worktrees (nothing to isolate) — \
+                         falling back to shared workspace"
+                    );
+                }
+                self.deps.runtime_config.workspace_dir.clone()
+            }
+            None => self.deps.runtime_config.workspace_dir.clone(),
+        }
+    }
+
     /// Resume an interactive worker that was idle at shutdown.
     ///
     /// Instead of running the initial task, skips directly to the follow-up
@@ -561,6 +639,11 @@ impl Worker {
             .clone()
             .with_tool_call_registry(tool_call_registry.clone());
 
+        // Per-worker isolated workspace (issue #224): when a worktree was
+        // provisioned, the shell/file tools operate there so concurrent
+        // workers can't clobber each other's edits.
+        let tool_workspace = self.tool_workspace();
+
         // Create per-worker ToolServer with task tools
         let worker_tool_server = crate::tools::create_worker_tool_server(
             self.deps.agent_id.clone(),
@@ -573,7 +656,7 @@ impl Worker {
             self.browser_config.clone(),
             self.screenshot_dir.clone(),
             self.brave_search_key.clone(),
-            self.deps.runtime_config.workspace_dir.clone(),
+            tool_workspace,
             self.deps.sandbox.clone(),
             mcp_tools,
             self.deps.runtime_config.clone(),
@@ -1066,6 +1149,16 @@ impl Worker {
         }
 
         tracing::info!(worker_id = %self.id, "worker completed");
+        // Successful completion: release the isolated workspace (remove its
+        // worktrees). On error/timeout paths we deliberately skip this so the
+        // workspace is retained for forensics; the startup reaper bounds it to
+        // MAX_RETAINED_WORKSPACES (by mtime), so retained failures can't
+        // accumulate unbounded across restarts.
+        if let Some(workspace) = self.isolated_workspace.take()
+            && let Err(error) = workspace.release().await
+        {
+            tracing::warn!(%error, worker_id = %self.id, "failed to release isolated workspace");
+        }
         if hit_max_segments {
             Ok(WorkerOutcome::Partial {
                 result,