Merge remote-tracking branch 'origin/worktree-cancel-primitive' into worktree-agent-handle

docs/rfc/proposed/2026-06-14-acp-agent-client-protocol.md

@@ -3,7 +3,7 @@
 Status: proposed
 
 <!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
-> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap<Agent, sessionId>` ownership seam the gate will build on. Status stays `proposed` until the gate lands. One further best-effort limitation is tracked as `TODO(rfc010-cancel-prestep)`: `session/cancel` aborts a running step and settles the RPC as `cancelled`, but a turn still queued (not yet started) when the cancel arrives may execute before the abort takes effect, pending a loop-level pre-step cancel. **Per-session `cwd` is now honored** (lifting the original "launch the server in the workspace root" restriction — see § Deferred): `session/new` accepts any absolute `cwd`, and `session/load` requires the request `cwd` to match the persisted session `cwd` so the editor and bash executor agree on the workspace.
+> **Implementation status (MVP landed):** steps 1, 2, 3, 4, 6, 7, 8 are implemented in `packages/acp` + `examples/acp-agent`. **Step 5 (the `session/request_permission` permission gate) is deferred** — the bridge ships a pass-through (tools run with the executor's full authority) marked `TODO(rfc010-permission-gate)`, and lays down only the `WeakMap<Agent, sessionId>` ownership seam the gate will build on. Status stays `proposed` until the gate lands. `session/cancel` is the queue-aware `agent.cancel()`: it aborts a running step, clears queued + steering work, and drops a turn that is about to start, so a queued-but-not-yet-started prompt never runs and a later prompt cannot be batched into the cancelled turn. **Per-session `cwd` is now honored** (lifting the original "launch the server in the workspace root" restriction — see § Deferred): `session/new` accepts any absolute `cwd`, and `session/load` requires the request `cwd` to match the persisted session `cwd` so the editor and bash executor agree on the workspace.
 
 ## Problem
 
@@ -33,7 +33,7 @@ The mapping between ACP and existing harness seams — each row names the seam a
 | `session/update: tool_call` (pending→in_progress) | `session/event` `tool/call` | demux via a Session→sessionId map; `kind` inferred from the tool name |
 | `session/update: tool_call_update` (completed/failed) | `session/event` `tool/result` | a throwing `tools/execute` yields NO `tool/result` → fail the pending tool UI from `agent/error`/turn-end |
 | `session/request_permission {sessionId, toolCall, options}` | prepended `tools/execute` listener | no-op unless `exec.agent` is ACP-owned; await the outcome; `selected/allow_*` → `next()`; `reject_*`/`cancelled` → veto `ToolExecutionResult{isError}` |
-| `session/cancel` (notification) | `agent.abort(reason)` | settle the in-flight prompt as `cancelled`; resolve any pending permission as `cancelled` exactly once |
+| `session/cancel` (notification) | `agent.cancel(reason)` | the queue-aware cancel (abort running step, clear queued + steering, drop an about-to-start turn); settle the in-flight prompt as `cancelled`; resolve any pending permission as `cancelled` exactly once |
 
 The permission gate is the first real consumer of the `tools/execute` veto seam (the documented "single veto/sandbox/permission seam" plus the deferred "Permission system" TODO in [docs/architecture.md](../../architecture.md)). It is a single global listener registered with `prepend: true` so it runs before any other tool wrapper. `ToolExecution.agent` is optional and the `Agent` interface carries no origin marker, so the bridge tracks ownership itself: it records each agent it creates in a `WeakMap<Agent, sessionId>` and the gate no-ops (calls `next()` immediately) for any `exec.agent` it does not own — non-ACP agents and the no-agent case pass straight through. For an owned agent it resolves the session, issues `session/request_permission`, and stores the pending resolver on that session's record so the outcome — or a `session/cancel`/connection-close — settles it exactly once.
 

docs/rfc/proposed/2026-06-14-acp-multi-session.md

@@ -18,7 +18,7 @@ The harness core already supports many agents (`AgentRegistry.list()` and `Agent
 - Lift the single-session guard in `session/new`; allow N live sessions, each mapped to its own `ReactLoopAgent`.
 - The bridge's `sessionId→agent` and `Session→sessionId` maps (introduced single-entry by [the ACP support RFC](2026-06-14-acp-agent-client-protocol.md)) become true multi-entry, plus a third `agent→sessionId` reverse map: the `tools/execute` permission gate receives only `exec.agent` (no sessionId), so it needs an O(1) reverse lookup to find the owning session. Every `agent/*` event and every `session/event` is demuxed strictly by id, so two sessions streaming at once never interleave their `session/update` notifications.
 - Per-session prompt queues: [the ACP support RFC](2026-06-14-acp-agent-client-protocol.md)'s single-entry in-flight-prompt state becomes multi-entry — one in-flight prompt *per session*, tracked per `sessionId`.
-- Per-session cancel routing: `session/cancel` aborts only its own session's agent and settles only that session's in-flight prompt. `agent.abort()` drives a per-agent `AbortController`, so the per-session `exec.signal` is the natural isolation fence.
+- Per-session cancel routing: `session/cancel` cancels only its own session's agent (via the queue-aware `agent.cancel()`) and settles only that session's in-flight prompt. The cancel is scoped to that one agent — a per-agent `AbortController` for the running step plus the agent's own queued/steering FIFOs — so it never touches another session's stream or pending prompt.
 - Per-session permission ownership: a `session/request_permission` and its outcome are bound to the originating session via the reverse map, so a permission prompt or a cancel in one session can never resolve another session's pending permission.
 
 ## Plan

packages/agent-loop/src/loop.ts

@@ -201,14 +201,22 @@ export async function runLoop(ctx: Context, agent: ReactLoopAgent, handle: LoopH
 
     // Pre-step cancel (window 2): `setStatus('running')` emits `agent/status`
     // SYNCHRONOUSLY, so a `running` listener can `cancel()` in the gap between the
-    // check above and `runTurn`. `cancel()` already cleared the queued FIFO, so
-    // drop the turn before it starts (runTurn would otherwise throw on an empty
-    // queue) and transition back to idle — `running` was already emitted, so a
-    // real `idle` transition (which also settles waiters) balances the status.
+    // check above and `runTurn`. Mirror window 1: clear the marker, then
+    //   - if NOTHING new is queued, drop the about-to-run turn and transition
+    //     back to `idle` (`running` was already emitted, so a real idle
+    //     transition balances the status AND settles `whenIdle()` waiters);
+    //   - if a NEW prompt was queued AFTER the cancel (a `running` listener that
+    //     cancels then sends), the marker was for the cancelled work only — fall
+    //     through and run the new prompt's turn (status is already `running`), so
+    //     a `whenIdle()` waiter resolves on THAT turn's running→idle, not before
+    //     it runs. Settling here would resolve quiescence while the replacement
+    //     is still queued and unrun (the same early-resolve race window 1 fixes).
     if (handle.isCancelled()) {
       handle.clearCancel()
-      handle.setStatus('idle')
-      continue
+      if (!agent.inbox.hasQueued) {
+        handle.setStatus('idle')
+        continue
+      }
     }
 
     // Re-derive the turn number from the log each iteration (do NOT keep a local

packages/agent-loop/tests/cancel.spec.ts

@@ -252,6 +252,36 @@ describe('Agent.cancel()', () => {
     expect(agent.session.events.some(e => e.type === 'turn/start')).toBe(false)
   })
 
+  it('window 2: whenIdle() does NOT resolve early when a running listener cancels then queues replacement work', async () => {
+    // The window-1 early-resolve race has a window-2 twin: a synchronous
+    // agent/status('running') listener cancels the about-to-run turn AND queues a
+    // replacement. window 2 must NOT settle waiters (via setStatus('idle')) while
+    // the replacement is still queued-and-unrun — it must fall through and run it,
+    // so whenIdle() resolves on the replacement turn's running→idle, not before.
+    const adapter = new MockAdapter([textResponse('A reply'), textResponse('B reply')])
+    const ctx = await harness(adapter)
+    const agent = ctx.agentLoop.create('a1', { model: 'mock' })
+
+    let replaced = false
+    const dispose = ctx.on('agent/status', (subject, status) => {
+      if (subject !== agent || status !== 'running' || replaced) return
+      replaced = true
+      agent.cancel('drop A')
+      send(agent, 'B')
+    })
+
+    send(agent, 'A')
+    const idle = agent.whenIdle()
+    await idle
+    dispose()
+
+    // whenIdle() resolved only AFTER B's turn ran: B's user message + a turn/end
+    // are in the log, and A was dropped.
+    expect(userTexts(agent)).toContain('B')
+    expect(userTexts(agent)).not.toContain('A')
+    expect(agent.session.events.some(e => e.type === 'turn/end')).toBe(true)
+  })
+
   it('whenIdle() does NOT resolve early when a new prompt is queued during a pre-step cancel', async () => {
     // The subtle race: a whenIdle() waiter is registered for prompt A; cancel()
     // clears A; prompt B is queued BEFORE the loop resumes from the idle wait.

packages/session-persistence/src/coordinator.ts

@@ -2,21 +2,20 @@
  * The backend-agnostic write-path orchestration shared by every first-party
  * {@link SessionPersistence} backend.
  *
- * The two durable backends (`dsh-session-persistence-jsonl` over file bytes,
- * `dsh-session-persistence-sqlite` over `node:sqlite` rows) were byte-identical
- * — or same-algorithm — for ALL of their orchestration: the in-memory
- * bookkeeping (the per-id state, the write-behind buffers, the per-id
- * serialization chains, the per-session init promises), the `session/event` →
- * buffer → `session/flush` drain, lazy materialization, crash-tail repair on
- * load, the four `session/created` adoption cases (new / HMR-adopt / collision /
- * ownerless-claim), and dispose-time quiescence. Only the STORAGE primitives
- * differed (write bytes vs. INSERT rows). {@link PersistenceCoordinator} owns
- * the orchestration once; a backend supplies the storage primitives as a small
+ * Every durable backend needs the same orchestration: the in-memory bookkeeping
+ * (the per-id state, the write-behind buffers, the per-id serialization chains,
+ * the per-session init promises), the `session/event` → buffer → `session/flush`
+ * drain, lazy materialization, crash-tail repair on load, the four
+ * `session/created` adoption cases (new / HMR-adopt / collision /
+ * ownerless-claim), and dispose-time quiescence. Only the STORAGE primitives are
+ * backend-specific (file bytes for `dsh-session-persistence-jsonl`, `node:sqlite`
+ * rows for `dsh-session-persistence-sqlite`). {@link PersistenceCoordinator} owns
+ * the orchestration; a backend supplies the storage primitives as a small
  * {@link PersistenceBackend} hook object.
  *
- * The abstract {@link SessionPersistence} service's public API is unchanged: a
- * backend still IS a `SessionPersistence` (its six public methods delegate to a
- * coordinator it composes), so a third-party backend MAY implement the service
+ * The abstract {@link SessionPersistence} service's public API is independent of
+ * this: a backend IS a `SessionPersistence` (its six public methods delegate to
+ * a coordinator it composes), so a third-party backend MAY implement the service
  * directly without using the coordinator at all.
  *
  * See the write-coordinator RFC (docs/rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md)
@@ -115,7 +114,19 @@ interface SessionState {
   meta: SessionHeader
   /** The next seq the backend expects to append (the stored log length). */
   cursor: number
-  /** Whether the session has been physically materialized. */
+  /**
+   * Whether the backend has physically written this session (a JSONL file /
+   * SQLite row exists). `create()` registers state LAZILY — cursor 0,
+   * materialized false, nothing on disk — so an empty session leaves no
+   * artifact and the FIRST `appendBatch` writes the header + its events in ONE
+   * transaction (the "a row exists ⇔ it has events" invariant `has`/`list`
+   * rely on; a separate up-front materialize could crash leaving a row with
+   * zero events). The flag is the only signal that distinguishes a session
+   * registered-but-never-written from one durably present, which two callers
+   * need: `has()` (lazy-but-unwritten is not yet durable) and the reclaim path
+   * (an abandoned id with no artifact AND no buffered events is free to reuse;
+   * a materialized one is a real collision).
+   */
   materialized: boolean
   /**
    * The live Session this state was bound to via `onCreated`, if any. State
@@ -450,9 +461,18 @@ export class PersistenceCoordinator<TornMarker = unknown> {
       if (tracked.owner === session) return
       if (tracked.owner === undefined) {
         // Ownerless state from the public create()/load() API. The FIRST live
-        // session claims it — but ONLY if its seed reproduces the persisted
-        // prefix (else a fresh, unrelated session reusing the id would have its
-        // seq 0..cursor-1 events filtered as already-written and grafted on).
+        // session claims it — but ONLY if BOTH the cwd scope and the seed match.
+        // The cwd guard mirrors case-2's cwd-scoped loadLive(): a same-id
+        // ownerless artifact at a DIFFERENT cwd is a collision, not a claim
+        // (claiming it would append the live cwd's events under the stored
+        // header's cwd, the exact cross-cwd corruption the loadLive scope
+        // prevents). The seed guard then ensures the live events reproduce the
+        // persisted prefix (else a fresh, unrelated session reusing the id would
+        // have its seq 0..cursor-1 events filtered as already-written and
+        // grafted on).
+        if (tracked.meta.cwd !== session.header.cwd) {
+          throw new Error(`session "${id}" is already persisted at a different cwd (persisted: ${String(tracked.meta.cwd)}, live: ${String(session.header.cwd)}) (id collision)`)
+        }
         if (!await this.seedMatchesPersisted(id, seed, tracked.cursor)) {
           throw new Error(`session "${id}" is already persisted with ${tracked.cursor} event(s) that do not match this live session (id collision)`)
         }

packages/session-persistence/tests/coordinator-contract.ts

@@ -68,6 +68,7 @@ export interface CoordinatorFixture {
 
 /** A constant absolute cwd; jsonl keys directories off it, memory/sqlite ignore it. */
 const WORK = '/w'
+const OTHER = '/other'
 
 /** The per-session init map a backend exposes for white-box init awaits. */
 function inits(persistence: SessionPersistence): Map<Session, Promise<void>> {
@@ -535,6 +536,58 @@ export function runCoordinatorContract(name: string, makeFixture: () => Promise<
       }
     })
 
+    it('a live session at a DIFFERENT cwd cannot claim cursor-0 ownerless state (cwd scope)', async () => {
+      const fix = await makeFixture()
+      const { ctx, fiber } = await freshCtx(fix)
+      try {
+        // create() registers ownerless state at cwd /a (cursor 0 — claims would
+        // otherwise match trivially on the seed).
+        await ctx.sessionPersistence.create(meta('wrong-cwd-claim', OTHER))
+        // A live session reusing the id but at cwd WORK must NOT claim it — the
+        // cwd scope is the fence (without it, WORK events would append under the
+        // OTHER header). Rejected as a collision.
+        const live = ctx.sessions.create('wrong-cwd-claim', { seed: oneTurnLog(), meta: { cwd: WORK } })
+        await expect(inits(ctx.sessionPersistence).get(live)).rejects.toThrow(/different cwd|id collision/)
+      } finally {
+        await fiber.dispose()
+        await fix.cleanup()
+      }
+    })
+
+    it('a live session at a DIFFERENT cwd cannot claim loaded-prefix ownerless state (cwd scope)', async () => {
+      const fix = await makeFixture()
+      const { ctx, fiber } = await freshCtx(fix)
+      try {
+        // Materialize + load at cwd OTHER (ownerless, cursor = 6).
+        await ctx.sessionPersistence.create(meta('wrong-cwd-load', OTHER))
+        await ctx.sessionPersistence.append(SessionId('wrong-cwd-load'), oneTurnLog())
+        const { events } = await ctx.sessionPersistence.load(SessionId('wrong-cwd-load'))
+        // A live session whose SEED matches the loaded prefix but whose cwd is
+        // WORK must still be rejected — the cwd guard runs before the seed check.
+        const live = ctx.sessions.create('wrong-cwd-load', { seed: events, meta: { cwd: WORK } })
+        await expect(inits(ctx.sessionPersistence).get(live)).rejects.toThrow(/different cwd|id collision/)
+      } finally {
+        await fiber.dispose()
+        await fix.cleanup()
+      }
+    })
+
+    it('a no-cwd ownerless state cannot be claimed by a live session WITH a cwd (cwd scope, undefined side)', async () => {
+      const fix = await makeFixture()
+      const { ctx, fiber } = await freshCtx(fix)
+      try {
+        // Ownerless state created WITHOUT a cwd (the no-cwd bucket).
+        await ctx.sessionPersistence.create(meta('no-cwd-state'))
+        // A live session reusing the id but WITH cwd WORK is a cwd mismatch
+        // (undefined vs WORK) and must be rejected.
+        const live = ctx.sessions.create('no-cwd-state', { seed: oneTurnLog(), meta: { cwd: WORK } })
+        await expect(inits(ctx.sessionPersistence).get(live)).rejects.toThrow(/different cwd|id collision/)
+      } finally {
+        await fiber.dispose()
+        await fix.cleanup()
+      }
+    })
+
     // --- append adopts a storage-only session (fresh instance, no prior create/load) ---
 
     it('append adopts a storage-only session (fresh instance) and continues the seq', async () => {