Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/subagent/agent-fleet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,12 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
deps.sessions.markRunInFlight(session.id);
void (async () => {
if (!stillAdmissible()) {
// CL-7787: the run was marked in flight above but will never
// start — settle through the normal terminal path instead of just
// releasing the admission slot, or the wait projection strands on
// "running" with nothing left to settle it.
deps.sessions.settleRun(session.id);
finalizeEnd();
admission.release(session.id);
return;
}
Expand Down Expand Up @@ -1292,6 +1298,10 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
}

if (!stillAdmissible()) {
// CL-7787: same stranded-run settle as above — the interrupt (or
// cancel) landed while worktree setup was in flight.
deps.sessions.settleRun(session.id);
finalizeEnd();
await reclaimWorktree();
admission.release(session.id);
return;
Expand Down
10 changes: 10 additions & 0 deletions src/subagent/session-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
createSubAgentSessionStore,
DEFAULT_MAX_ENTRY_CHARS,
} from "./session-store.js";
import { projectWaitStatus } from "./lifecycle.js";
import { createAdmissionQueue } from "./admission.js";
import { forcedStopReport } from "./stop-policy.js";
import { agentLaneIsLive, fleetProgress } from "../tui/agent-progress.js";
Expand Down Expand Up @@ -1321,6 +1322,15 @@ describe("CL-7269 one stored worker lifecycle", () => {
expect(store.interruptOne(session.id).ok).toBe(true);
expect(aborted).toBe(1);
expect(store.get(session.id)?.lifecycleStatus).toBe("interrupted");
// CL-7787: the pending_init interrupt must not strand a run-in-flight
// marker — projectWaitStatus would otherwise report "running" forever and
// the fleet would never go dry.
expect(store.isRunInFlight(session.id)).toBe(false);
expect(store.get(session.id)?.runInFlight).toBe(false);
const snap = defined(store.get(session.id));
expect(
projectWaitStatus(snap.lifecycle, store.isRunInFlight(session.id)),
).toBe("interrupted");
});

test("closeOne of a queued pending_init session does not wait for a close handle", async () => {
Expand Down
37 changes: 37 additions & 0 deletions src/subagent/session-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,31 @@ export function createSubAgentSessionStore(
cancelDescendantAsks(id, reason);
};

// CL-7787: store-level invariant — a non-live lifecycle must never coexist
// with a run-in-flight marker once no settle-capable handle remains. A
// soft-interrupted run still holds its interrupt/close/followup/deliver
// handle and settles through it; anything else reaching a terminal state
// through mutate has nothing left to settle it, so the store drops the
// marker itself instead of trusting every call site to remember.
// (Cancel-abort hooks don't settle runs, so cancelHandles is deliberately
// not in the set below. And cancel itself is excluded entirely — see
// markCancelled: after cancel the marker is the live run's outstanding
// settlement promise, even when no handle was ever registered.)
const enforceSettledRunInvariant = (id: string): void => {
const session = sessions.get(id);
if (session === undefined || !runInFlight.has(id)) return;
const state = session.lifecycle.state;
if (state === "pending_init" || state === "running") return;
if (
interruptHandles.has(id) ||
closeHandles.has(id) ||
followupHandles.has(id) ||
deliverHandles.has(id)
)
return;
runInFlight.delete(id);
};

const markCancelled = (session: StoredSession, reason: string): void => {
session.lifecycle = { state: "cancelled", error: reason };
session.retained = false;
Expand All @@ -649,6 +674,12 @@ export function createSubAgentSessionStore(
cancelHandles.delete(session.id);
// closeHandles are owned by releaseHandles / closeOne — dropping them
// here would skip teardown for a retained session that is mid-turn.
// NOTE: no enforceSettledRunInvariant here — after cancel the in-flight
// marker is the live run's outstanding settlement promise (its salvage
// still lands via attachReport); clearing it would resolve wait_agents
// before the salvage arrives (CL-6915). The stranded shape this guards
// (no run will ever settle) is closed at the pending_init interrupt
// branch and the fleet's not-admissible early return instead.
bumpRevision(session.id);
pruneCompleted();
};
Expand Down Expand Up @@ -821,6 +852,7 @@ export function createSubAgentSessionStore(
const session = sessions.get(id);
if (session === undefined) return;
fn(session);
enforceSettledRunInvariant(id);
session.lastActivityAt = now();
bumpRevision(id);
notify();
Expand Down Expand Up @@ -1689,6 +1721,11 @@ export function createSubAgentSessionStore(
} catch {
// Abort hooks must not throw into the interrupt path.
}
// CL-7787: this is a terminal transition — drop the run-in-flight
// marker alongside the lifecycle flip like every other terminal
// transition, otherwise the wait projection reports "running"
// forever with no run left to settle it.
runInFlight.delete(id);
mutate(id, (s) => {
s.lifecycle = {
state: "interrupted",
Expand Down
77 changes: 77 additions & 0 deletions src/subagent/spawn-agent-worktree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { join } from "node:path";
import { promisify } from "node:util";

import { createFleetMailbox, createSpawnAgentTool } from "./agent-fleet.js";
import { isLiveWaitStatus, projectWaitStatus } from "./lifecycle.js";
import { unlimitedAdmissionQueue } from "./admission.js";
import { createSubAgentSessionStore } from "./session-store.js";
import { createPermissionGate } from "../permission/gate.js";
Expand Down Expand Up @@ -454,4 +455,80 @@ describe("spawn_agent worktree isolation", () => {

expect(await pathExists(completedWorkerCwd)).toBe(false);
});

test("interrupt during worktree setup settles the run instead of stranding it", async () => {
const repo = await makeRepo();
tempDirs.push(repo);
const workdirBase = await mkdtemp(join(tmpdir(), "corbits-workdir-"));
tempDirs.push(workdirBase);

let started = 0;
const { telemetry, events } = telemetryCapture();
const sessions = createSubAgentSessionStore();
const mailbox = createFleetMailbox(sessions);
const tool = createSpawnAgentTool({
permissionGate: testPermissionGate,
cwd: repo,
getWorkdirBase: () => workdirBase,
provider,
useWorktree: true,
telemetry,
run: async () => {
started += 1;
return { report: "ok" };
},
sessions,
fleetRecords: mailbox,
admission: unlimitedAdmissionQueue(),
});
if (tool.kind !== "full") throw new Error("expected full tool");
const spawned = await tool.handler(
{
id: "wt-interrupt",
name: "spawn_agent",
arguments: {
description: "interrupted setup",
prompt: "Do the work",
intent: "explore",
},
},
new AbortController().signal,
);
const content = typeof spawned.content === "string" ? spawned.content : "";
const agentId = (JSON.parse(content) as { agent_id: string }).agent_id;

// CL-7787: the fleet admitted the spawn and marked a run in flight, then
// suspended on worktree creation — the interrupt lands in exactly that
// window, before any run handle exists.
expect(sessions.isRunInFlight(agentId)).toBe(true);
expect(sessions.interruptOne(agentId).ok).toBe(true);

// Once the worktree resolves, the stranded run must settle through the
// normal terminal path: wait status leaves "running", the fleet goes dry
// so mail drives fire, and run() never starts leftover work.
await waitFor(
() =>
mailbox.peek(agentId) !== undefined &&
!isLiveWaitStatus(defined(mailbox.peek(agentId)).status),
);
expect(started).toBe(0);
expect(sessions.isRunInFlight(agentId)).toBe(false);
const snap = defined(sessions.get(agentId));
expect(snap.lifecycleStatus).toBe("interrupted");
expect(
projectWaitStatus(snap.lifecycle, sessions.isRunInFlight(agentId)),
).toBe("interrupted");
expect(mailbox.peek(agentId)?.status).toBe("interrupted");
// The fleet is dry: no session projects a live wait status, so mail
// drives fire.
expect(
sessions
.list()
.every((s) => !isLiveWaitStatus(projectWaitStatus(s.lifecycle, s.runInFlight === true))),
).toBe(true);
await waitFor(() => events.some((event) => event.event === "subagent_end"));
const ends = events.filter((event) => event.event === "subagent_end");
expect(ends).toHaveLength(1);
expect(ends[0]?.properties).toMatchObject({ status: "interrupted" });
});
});
Loading