fix(repo): hold a checkout across an approval park — supervised write-tier deadlock (#796)

docs/spec/runtime/repos.md

@@ -227,12 +227,25 @@ limit"](#the-honest-limit), and are not claimed here.
 
 ### The lifecycle
 
-A checkout's life is exactly one turn's. Every path the tools create is recorded
-on a per-turn ledger, and an RAII janitor claimed at each turn's entry point
-deletes them on the way out — success, error, steer cancel, redirect exhaustion
-and panic-unwind alike. A mid-loop redirect deletes the abandoned turn's
-checkout too, so a re-run starts from a fresh tree rather than one a discarded
-turn half-patched.
+A checkout's life is one turn's — with one deliberate exception the write tier
+needs (issue #796). Every path the tools create is recorded on a per-turn
+ledger, and an RAII janitor claimed at each turn's entry point deletes them on
+the way out — success, error, steer cancel, redirect exhaustion and panic-unwind
+alike. A mid-loop redirect deletes the abandoned turn's checkout too, so a re-run
+starts from a fresh tree rather than one a discarded turn half-patched.
+
+**The exception: surviving an approval park.** A write is not one turn's work —
+`repo_checkout` → edit → `git_operations` commit → `repo_publish` are each a
+`Reach::Consequence` step that parks under `supervised`, and a park ends the
+turn. So a checkout a *task* turn parked with is held on a task-keyed retained
+set the janitor does not touch, keyed by the task the parked approval carries
+(`GrantedCall::origin_task`); the approval's re-issue reclaims it, so the resumed
+step commits and publishes on the same tree — and the same commit — the parked
+step left. It is deleted when the task's resumed step finishes without parking
+again, or — if the approval is denied or expired — swept the next time any turn
+claims the janitor and no live grant still names the task. This is the "deleted
+at task end" this tier always promised; a per-turn delete made it a deadlock
+under supervision.
 
 A host killed mid-turn ends no turn, so boot sweeps
 `<harness>/<company>/*/workspace/repos` before the company starts. It is

src/company/runtime.rs

@@ -1499,6 +1499,10 @@ impl CompanyRuntime {
         let expired = self.approval_gate.sweep_expired(now);
         for id in &expired {
             self.journal.record_expired(id, now).await?;
+            // Issue #796: the parked approval is gone, so its work unit is no
+            // longer awaiting a resume — drop the pending mark so the checkout it
+            // was holding across the park becomes sweepable.
+            self.grants.clear_pending(id);
             // Issue #469: releasing the turn this approval was blocking, and
             // running its continuation when this expiry was the last thing it
             // waited on. Spawned rather than awaited: the continuation is a full

src/harness/brain.rs

@@ -302,6 +302,10 @@ impl HarnessBrain {
             tool: String,
             instruction: String,
             origin_thread: Option<String>,
+            /// The task this approval was parked from (issue #796), so the
+            /// re-issue turn can reclaim its held-across-park checkout and stamp
+            /// the ledger so `repo_publish` can name the task branch.
+            origin_task: Option<String>,
         }
 
         let grants = self.deps.approval_requests.grants();
@@ -320,6 +324,7 @@ impl HarnessBrain {
                 tool: grant.tool,
                 agent: grant.agent,
                 origin_thread: grant.origin_thread,
+                origin_task: grant.origin_task,
             }
         } else if let Some(standing) = grants.peek_standing_by_approval(approval_id) {
             // No exact-arguments pin, and deliberately so: a standing grant
@@ -336,11 +341,15 @@ impl HarnessBrain {
                 tool: standing.tool,
                 agent: standing.agent,
                 origin_thread: standing.origin_thread,
+                origin_task: standing.origin_task,
             }
         } else {
             return Ok(None);
         };
         let instruction = grant.instruction.clone();
+        // Issue #796: the task (if any) this approval resumes. Bound before the
+        // struct's fields are moved into the run below.
+        let origin_task = grant.origin_task.clone();
 
         let guard = self.deps.steer.register(
             &self.record().id,
@@ -387,14 +396,27 @@ impl HarnessBrain {
                     .claim(publish::PublishDestination::Conversation)
             });
         // Issue #245: a re-dispatched approval is a full agent turn with the
-        // whole toolbelt, so it can check a repository out — and a checkout must
-        // not outlive the turn that asked for it. Unconditional: the janitor
-        // over an empty ledger is a no-op, which is what every turn that touches
-        // no repository does.
+        // whole toolbelt, so it can check a repository out, and the janitor
+        // claimed here deletes what this turn creates. Issue #796 refines what
+        // "this turn's checkout" is: a turn resuming a task first reclaims that
+        // task's held-across-park tree, so the resumed step operates on the same
+        // working tree — and the commit it needs — the parked step left behind.
         let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
-        // Issue #735: a re-dispatched grant is not a task card, so clear any task
-        // a prior turn stamped — `repo_publish` requires a task and refuses here.
-        self.deps.checkouts.set_task(None);
+        // Issue #796: at the claim, drop any task's retained checkout whose
+        // approval was denied or expired — no live grant names it, so nothing
+        // will ever resume it. `grants` is the same live set peeked above.
+        self.deps
+            .checkouts
+            .sweep_orphans(|task| grants.any_for_task(task));
+        // Issue #735/#796: stamp the task this grant resumes so `repo_publish`
+        // can name its branch, and reclaim the checkout the parked step left so
+        // the resumed step — a commit, a publish — finds its own work. A
+        // re-dispatch with no task (a plain operator-chat approval) clears the
+        // cell and reclaims nothing, exactly as #735 did.
+        self.deps.checkouts.set_task(origin_task.clone());
+        if let Some(task) = &origin_task {
+            self.deps.checkouts.reclaim(task);
+        }
         // Un-streamed, like a dispatched card: this turn is answered by the
         // bubble returned below, and its transient frames would otherwise
         // misattribute onto whichever chat thread the console is watching.
@@ -408,6 +430,22 @@ impl HarnessBrain {
             )
             .await;
         drop(guard);
+        // Issue #796: hold the task's checkout across the turn boundary on EVERY
+        // re-issue, not only one that parks a new approval.
+        //
+        // A write is a chain of separately-approved steps — checkout, edit,
+        // commit, publish — and an operator commonly approves them in a batch, so
+        // the grants exist up front. Re-issuing one grant then need NOT queue a
+        // new approval, yet the checkout it just materialized (or the commit it
+        // just made) must still be there when the next grant is re-issued in its
+        // own turn. Retaining only on a fresh park dropped exactly that tree the
+        // turn it was created. So retain unconditionally here; the checkout is
+        // reclaimed on the next re-issue, and `sweep_orphans` at the next claim
+        // deletes it once no live grant names the task — the flow finished, was
+        // denied, or expired.
+        if let Some(task) = &origin_task {
+            self.deps.checkouts.retain_for_task(task);
+        }
 
         let published = self.deps.pending_publishes.drain();
         if !published.is_empty()
@@ -636,9 +674,20 @@ impl HarnessBrain {
         // guard's `Drop` is what deletes the tree on every exit — success,
         // error, cancel, redirect exhaustion and panic-unwind alike.
         let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
+        // Issue #796: at the claim, drop any task's retained checkout whose
+        // approval was denied or expired — no live grant names it, so nothing
+        // will resume it.
+        {
+            let grants = self.deps.approval_requests.grants();
+            self.deps
+                .checkouts
+                .sweep_orphans(|task| grants.any_for_task(task));
+        }
         // Issue #735: this is a dispatched card, so `repo_publish` names its
         // branch `oc/<company>/<card>`. Stamped on the same per-turn cell the
-        // janitor above claims.
+        // janitor above claims. A parked step of this card resumes through the
+        // approval re-issue path (which reclaims the tree there, issue #796), not
+        // by re-running the card, so nothing is reclaimed here.
         self.deps.checkouts.set_task(Some(card.id.clone()));
         // Issue #339, same argument for staged workflow references: an operator
         // chat turn earlier in this cycle may have run a workflow through the
@@ -896,6 +945,17 @@ impl HarnessBrain {
             }
         };
 
+        // Issue #796: if this dispatch parked (a step it ran needs approval),
+        // hold whatever checkout it built across that park so the approved step
+        // resumes on the same tree. A dispatch that ended without parking keeps
+        // the pre-#796 behaviour — the janitor's `Drop` deletes its checkout.
+        // Orphan cleanup for the denied/expired case is the `sweep_orphans` at
+        // the next claim, which is safe against a still-pending approval in a way
+        // an unconditional purge here would not be.
+        if self.deps.approval_requests.queued() > approvals_before {
+            self.deps.checkouts.retain_for_task(&card.id);
+        }
+
         // ── Issue #244: the deliverable gate, and the one nudge ─────────────
         //
         // This sits between the primary turn and the completion bookkeeping —
@@ -2466,6 +2526,15 @@ impl HarnessBrain {
                     // so it can clone a repository, and the guard's `Drop`
                     // removes it when this turn ends.
                     let _checkout_janitor = CheckoutJanitor::claim(&self.deps.checkouts);
+                    // Issue #796: sweep any task checkout orphaned by a
+                    // denied/expired approval, on this turn's claim like every
+                    // other.
+                    {
+                        let grants = self.deps.approval_requests.grants();
+                        self.deps
+                            .checkouts
+                            .sweep_orphans(|task| grants.any_for_task(task));
+                    }
                     // Issue #735: a conversation is not a task card, so clear any
                     // task a prior turn stamped — `repo_publish` requires a task
                     // and refuses on a chat turn (task turns only, this tier).
@@ -6467,6 +6536,7 @@ members = ["eng1", "eng2"]
                 at_millis: now_millis(),
                 origin_thread: None,
                 origin_parent: None,
+                origin_task: None,
             });
         let brain = brain_with_queue_and_events(dir.path(), requests, log.clone());
 
@@ -6523,6 +6593,133 @@ members = ["eng1", "eng2"]
             .all(|e| !matches!(e.event, CompanyEvent::AgentReply { .. }))
     }
 
+    /// Issue #796: a task's checkout survives a whole BATCH of re-issues, the way
+    /// a supervised write actually runs — the operator approves `repo_checkout`,
+    /// the edit, the commit and the publish up front, and each grant is re-issued
+    /// in its own turn. The checkout the first re-issue materializes must still be
+    /// there when the next grant is re-issued, and the one after that.
+    ///
+    /// This is the exact loop the first cut of the fix still had: retaining the
+    /// tree only on a turn that parked a NEW approval dropped it the moment a
+    /// batched re-issue parked nothing, so the next approved step found the tree
+    /// gone. `MockProvider` parks and consumes nothing, so both grants stay live —
+    /// the task is in flight — and the tree must be held across every re-issue.
+    #[tokio::test]
+    async fn a_task_checkout_survives_a_batch_of_re_issues() {
+        let dir = tempfile::tempdir().unwrap();
+        let log: Arc<dyn crate::ports::EventLog> =
+            Arc::new(crate::store::FsEventLog::new(dir.path().to_path_buf()));
+        let requests = crate::harness::policy::ApprovalRequestQueue::default();
+        // Two of the task's steps approved up front (a batch), both under t-1.
+        for (id, tool) in [("appr-1", "repo_checkout"), ("appr-2", "git_operations")] {
+            requests
+                .grants()
+                .grant(crate::runtime::grants::GrantedCall {
+                    approval_id: ApprovalId::new(id),
+                    agent: "ceo".into(),
+                    tool: tool.into(),
+                    args: serde_json::json!({}),
+                    at_millis: now_millis(),
+                    origin_thread: None,
+                    origin_parent: None,
+                    // The link that makes each re-issue reclaim the same tree.
+                    origin_task: Some("t-1".into()),
+                });
+        }
+        let brain = brain_with_queue_and_events(dir.path(), requests, log.clone());
+
+        // The checkout the task's first step materialized, held under its task.
+        let tree = dir.path().join("held-checkout");
+        std::fs::create_dir_all(&tree).unwrap();
+        brain.deps.checkouts.record(tree.clone());
+        brain.deps.checkouts.retain_for_task("t-1");
+
+        // Re-issue the first approved step. The tree must survive the turn — the
+        // task is not done, its other step is still granted.
+        brain
+            .run_cycle(
+                cycle_over(vec![approval_resolved("appr-1", Verdict::Approve)]),
+                &NoopHost,
+            )
+            .await
+            .expect("cycle runs");
+        assert!(
+            tree.is_dir(),
+            "the checkout was wiped between two batched re-issues — the loop is back"
+        );
+        assert_eq!(
+            brain.deps.checkouts.retained_tasks(),
+            vec!["t-1".to_string()],
+            "the task's checkout must stay held while the task is in flight"
+        );
+
+        // Re-issue the second approved step. Still held.
+        brain
+            .run_cycle(
+                cycle_over(vec![approval_resolved("appr-2", Verdict::Approve)]),
+                &NoopHost,
+            )
+            .await
+            .expect("cycle runs");
+        assert!(
+            tree.is_dir(),
+            "the checkout did not survive the second re-issue"
+        );
+        assert_eq!(
+            brain.deps.checkouts.retained_tasks(),
+            vec!["t-1".to_string()]
+        );
+    }
+
+    /// Issue #796: a checkout held for a task no live grant names — the
+    /// approval was denied or expired, so nothing will ever resume it — is swept
+    /// the next time any turn claims the janitor, rather than leaking to the boot
+    /// sweep. Here an unrelated approval drives that turn.
+    #[tokio::test]
+    async fn an_orphaned_task_checkout_is_swept_at_the_next_janitor_claim() {
+        let dir = tempfile::tempdir().unwrap();
+        let log: Arc<dyn crate::ports::EventLog> =
+            Arc::new(crate::store::FsEventLog::new(dir.path().to_path_buf()));
+        let requests = crate::harness::policy::ApprovalRequestQueue::default();
+        // A live grant with no task of its own — an ordinary chat approval — to
+        // drive one redispatch turn. It names no task, so it cannot keep the
+        // orphan alive.
+        requests
+            .grants()
+            .grant(crate::runtime::grants::GrantedCall {
+                approval_id: ApprovalId::new("appr-1"),
+                agent: "ceo".into(),
+                tool: "composio_execute".into(),
+                args: serde_json::json!({}),
+                at_millis: now_millis(),
+                origin_thread: None,
+                origin_parent: None,
+                origin_task: None,
+            });
+        let brain = brain_with_queue_and_events(dir.path(), requests, log.clone());
+
+        // A tree held for a task whose approval is gone: no grant names it.
+        let tree = dir.path().join("orphaned-checkout");
+        std::fs::create_dir_all(&tree).unwrap();
+        brain.deps.checkouts.record(tree.clone());
+        brain.deps.checkouts.retain_for_task("t-gone");
+        assert!(tree.is_dir());
+
+        brain
+            .run_cycle(
+                cycle_over(vec![approval_resolved("appr-1", Verdict::Approve)]),
+                &NoopHost,
+            )
+            .await
+            .expect("cycle runs");
+
+        assert!(
+            !tree.exists(),
+            "an orphaned task checkout was not swept at the janitor claim"
+        );
+        assert!(brain.deps.checkouts.retained_tasks().is_empty());
+    }
+
     // Issue #379's reply routing — a channel's continuation must resume in that
     // channel and not in its lead's private DM, and the mirror — used to be
     // pinned here, against a hand-built grant. It moved with the journaling
@@ -6563,6 +6760,7 @@ members = ["eng1", "eng2"]
                 expires_at_millis: now_millis() + 60 * 60 * 1000,
                 origin_thread: None,
                 origin_parent: None,
+                origin_task: None,
                 scope: None,
             });
         let brain = brain_with_queue_and_events(dir.path(), requests, log.clone());
@@ -6619,6 +6817,7 @@ members = ["eng1", "eng2"]
                 at_millis: now_millis(),
                 origin_thread: None,
                 origin_parent: None,
+                origin_task: None,
             });
         let brain = brain_with_queue_and_events(dir.path(), requests, log.clone());
 
@@ -6805,6 +7004,7 @@ members = ["eng1", "eng2"]
             at_millis: now_millis(),
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
         }
     }
 

src/harness/policy.rs

@@ -2806,6 +2806,7 @@ mod tests {
             at_millis: 1_000,
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
         }
     }
 
@@ -3012,6 +3013,7 @@ mod tests {
             at_millis: 1_000,
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
         });
 
         queue.clear();
@@ -3284,6 +3286,7 @@ mod tests {
             at_millis: 1_000,
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
         });
 
         {
@@ -3320,6 +3323,7 @@ mod tests {
             at_millis: 1_000,
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
         };
         shared.grant(call.clone());
 
@@ -3834,6 +3838,7 @@ mod tests {
             expires_at_millis,
             origin_thread: None,
             origin_parent: None,
+            origin_task: None,
             scope: None,
         }
     }