Skip to content

Commit 6b219fa

Browse files
Settle stranded runs interrupted during the admission window (#939)
1 parent 0e2d903 commit 6b219fa

4 files changed

Lines changed: 134 additions & 0 deletions

File tree

src/subagent/agent-fleet.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1258,6 +1258,12 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
12581258
deps.sessions.markRunInFlight(session.id);
12591259
void (async () => {
12601260
if (!stillAdmissible()) {
1261+
// CL-7787: the run was marked in flight above but will never
1262+
// start — settle through the normal terminal path instead of just
1263+
// releasing the admission slot, or the wait projection strands on
1264+
// "running" with nothing left to settle it.
1265+
deps.sessions.settleRun(session.id);
1266+
finalizeEnd();
12611267
admission.release(session.id);
12621268
return;
12631269
}
@@ -1292,6 +1298,10 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
12921298
}
12931299

12941300
if (!stillAdmissible()) {
1301+
// CL-7787: same stranded-run settle as above — the interrupt (or
1302+
// cancel) landed while worktree setup was in flight.
1303+
deps.sessions.settleRun(session.id);
1304+
finalizeEnd();
12951305
await reclaimWorktree();
12961306
admission.release(session.id);
12971307
return;

src/subagent/session-store.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
createSubAgentSessionStore,
55
DEFAULT_MAX_ENTRY_CHARS,
66
} from "./session-store.js";
7+
import { projectWaitStatus } from "./lifecycle.js";
78
import { createAdmissionQueue } from "./admission.js";
89
import { forcedStopReport } from "./stop-policy.js";
910
import { agentLaneIsLive, fleetProgress } from "../tui/agent-progress.js";
@@ -1321,6 +1322,15 @@ describe("CL-7269 one stored worker lifecycle", () => {
13211322
expect(store.interruptOne(session.id).ok).toBe(true);
13221323
expect(aborted).toBe(1);
13231324
expect(store.get(session.id)?.lifecycleStatus).toBe("interrupted");
1325+
// CL-7787: the pending_init interrupt must not strand a run-in-flight
1326+
// marker — projectWaitStatus would otherwise report "running" forever and
1327+
// the fleet would never go dry.
1328+
expect(store.isRunInFlight(session.id)).toBe(false);
1329+
expect(store.get(session.id)?.runInFlight).toBe(false);
1330+
const snap = defined(store.get(session.id));
1331+
expect(
1332+
projectWaitStatus(snap.lifecycle, store.isRunInFlight(session.id)),
1333+
).toBe("interrupted");
13241334
});
13251335

13261336
test("closeOne of a queued pending_init session does not wait for a close handle", async () => {

src/subagent/session-store.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,31 @@ export function createSubAgentSessionStore(
633633
cancelDescendantAsks(id, reason);
634634
};
635635

636+
// CL-7787: store-level invariant — a non-live lifecycle must never coexist
637+
// with a run-in-flight marker once no settle-capable handle remains. A
638+
// soft-interrupted run still holds its interrupt/close/followup/deliver
639+
// handle and settles through it; anything else reaching a terminal state
640+
// through mutate has nothing left to settle it, so the store drops the
641+
// marker itself instead of trusting every call site to remember.
642+
// (Cancel-abort hooks don't settle runs, so cancelHandles is deliberately
643+
// not in the set below. And cancel itself is excluded entirely — see
644+
// markCancelled: after cancel the marker is the live run's outstanding
645+
// settlement promise, even when no handle was ever registered.)
646+
const enforceSettledRunInvariant = (id: string): void => {
647+
const session = sessions.get(id);
648+
if (session === undefined || !runInFlight.has(id)) return;
649+
const state = session.lifecycle.state;
650+
if (state === "pending_init" || state === "running") return;
651+
if (
652+
interruptHandles.has(id) ||
653+
closeHandles.has(id) ||
654+
followupHandles.has(id) ||
655+
deliverHandles.has(id)
656+
)
657+
return;
658+
runInFlight.delete(id);
659+
};
660+
636661
const markCancelled = (session: StoredSession, reason: string): void => {
637662
session.lifecycle = { state: "cancelled", error: reason };
638663
session.retained = false;
@@ -649,6 +674,12 @@ export function createSubAgentSessionStore(
649674
cancelHandles.delete(session.id);
650675
// closeHandles are owned by releaseHandles / closeOne — dropping them
651676
// here would skip teardown for a retained session that is mid-turn.
677+
// NOTE: no enforceSettledRunInvariant here — after cancel the in-flight
678+
// marker is the live run's outstanding settlement promise (its salvage
679+
// still lands via attachReport); clearing it would resolve wait_agents
680+
// before the salvage arrives (CL-6915). The stranded shape this guards
681+
// (no run will ever settle) is closed at the pending_init interrupt
682+
// branch and the fleet's not-admissible early return instead.
652683
bumpRevision(session.id);
653684
pruneCompleted();
654685
};
@@ -821,6 +852,7 @@ export function createSubAgentSessionStore(
821852
const session = sessions.get(id);
822853
if (session === undefined) return;
823854
fn(session);
855+
enforceSettledRunInvariant(id);
824856
session.lastActivityAt = now();
825857
bumpRevision(id);
826858
notify();
@@ -1689,6 +1721,11 @@ export function createSubAgentSessionStore(
16891721
} catch {
16901722
// Abort hooks must not throw into the interrupt path.
16911723
}
1724+
// CL-7787: this is a terminal transition — drop the run-in-flight
1725+
// marker alongside the lifecycle flip like every other terminal
1726+
// transition, otherwise the wait projection reports "running"
1727+
// forever with no run left to settle it.
1728+
runInFlight.delete(id);
16921729
mutate(id, (s) => {
16931730
s.lifecycle = {
16941731
state: "interrupted",

src/subagent/spawn-agent-worktree.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { join } from "node:path";
66
import { promisify } from "node:util";
77

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

455456
expect(await pathExists(completedWorkerCwd)).toBe(false);
456457
});
458+
459+
test("interrupt during worktree setup settles the run instead of stranding it", async () => {
460+
const repo = await makeRepo();
461+
tempDirs.push(repo);
462+
const workdirBase = await mkdtemp(join(tmpdir(), "corbits-workdir-"));
463+
tempDirs.push(workdirBase);
464+
465+
let started = 0;
466+
const { telemetry, events } = telemetryCapture();
467+
const sessions = createSubAgentSessionStore();
468+
const mailbox = createFleetMailbox(sessions);
469+
const tool = createSpawnAgentTool({
470+
permissionGate: testPermissionGate,
471+
cwd: repo,
472+
getWorkdirBase: () => workdirBase,
473+
provider,
474+
useWorktree: true,
475+
telemetry,
476+
run: async () => {
477+
started += 1;
478+
return { report: "ok" };
479+
},
480+
sessions,
481+
fleetRecords: mailbox,
482+
admission: unlimitedAdmissionQueue(),
483+
});
484+
if (tool.kind !== "full") throw new Error("expected full tool");
485+
const spawned = await tool.handler(
486+
{
487+
id: "wt-interrupt",
488+
name: "spawn_agent",
489+
arguments: {
490+
description: "interrupted setup",
491+
prompt: "Do the work",
492+
intent: "explore",
493+
},
494+
},
495+
new AbortController().signal,
496+
);
497+
const content = typeof spawned.content === "string" ? spawned.content : "";
498+
const agentId = (JSON.parse(content) as { agent_id: string }).agent_id;
499+
500+
// CL-7787: the fleet admitted the spawn and marked a run in flight, then
501+
// suspended on worktree creation — the interrupt lands in exactly that
502+
// window, before any run handle exists.
503+
expect(sessions.isRunInFlight(agentId)).toBe(true);
504+
expect(sessions.interruptOne(agentId).ok).toBe(true);
505+
506+
// Once the worktree resolves, the stranded run must settle through the
507+
// normal terminal path: wait status leaves "running", the fleet goes dry
508+
// so mail drives fire, and run() never starts leftover work.
509+
await waitFor(
510+
() =>
511+
mailbox.peek(agentId) !== undefined &&
512+
!isLiveWaitStatus(defined(mailbox.peek(agentId)).status),
513+
);
514+
expect(started).toBe(0);
515+
expect(sessions.isRunInFlight(agentId)).toBe(false);
516+
const snap = defined(sessions.get(agentId));
517+
expect(snap.lifecycleStatus).toBe("interrupted");
518+
expect(
519+
projectWaitStatus(snap.lifecycle, sessions.isRunInFlight(agentId)),
520+
).toBe("interrupted");
521+
expect(mailbox.peek(agentId)?.status).toBe("interrupted");
522+
// The fleet is dry: no session projects a live wait status, so mail
523+
// drives fire.
524+
expect(
525+
sessions
526+
.list()
527+
.every((s) => !isLiveWaitStatus(projectWaitStatus(s.lifecycle, s.runInFlight === true))),
528+
).toBe(true);
529+
await waitFor(() => events.some((event) => event.event === "subagent_end"));
530+
const ends = events.filter((event) => event.event === "subagent_end");
531+
expect(ends).toHaveLength(1);
532+
expect(ends[0]?.properties).toMatchObject({ status: "interrupted" });
533+
});
457534
});

0 commit comments

Comments
 (0)