fix(x): make stopTurn reliable under concurrent advances on one turn

The sessions layer tracked live advances in a map keyed by bare turnId
with overwrite-on-set and unconditional delete-on-settle. A turn can
legally have several live advances at once — a running invocation plus
external-input invocations queued on the turn lock (e.g. a permission
response while an async result is pending). The overwrite made stopTurn
abort the wrong controller and await an outcome that couldn't settle;
the unconditional delete untracked a sibling advance, so a later
stopTurn missed the abort path, fell back to a cancel input, and could
reject with TurnInputError if the turn had completed meanwhile — a
user-visible stop failure. Durable state was never affected.

Track a set of advances per turn: each advance removes only its own
entry on settle, and stopTurn aborts every live controller and awaits
all outcomes. A cancel input that loses the race with a concurrent
settle now counts as a successful stop (the turn is already terminal)
instead of rejecting.

Adds four tests: concurrent invocations both aborted, an earlier settle
not untracking a later advance, the settle-race cancel treated as
success, and non-terminal rejections still surfacing. Updates
session-design.md §9.3 and §13.5 accordingly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Ramnique Singh 2026-07-10 22:46:06 +05:30
parent 284dc77f5e
commit e22d3b4796
3 changed files with 172 additions and 23 deletions

View file

@ -338,9 +338,14 @@ runtime's job; the session layer passes its errors through.
### 9.3 stopTurn and resumeTurn
- `stopTurn` cancels via the turn runtime: aborting the active invocation's
signal if one is running, else advancing a suspended turn with a `cancel`
input.
- `stopTurn` cancels via the turn runtime: aborting the signal of every
live advance the layer has started for the turn, else advancing the turn
with a `cancel` input. A turn can legally have several live advances at
once — one running invocation plus external-input invocations queued on
the turn lock — so the layer tracks them per turn and stop aborts them
all; the queued ones observe their aborted signal when the lock frees. A
cancel input that loses the race with a concurrent settle (the turn is
already terminal by the time it applies) counts as a successful stop.
- `resumeTurn` re-enters the latest turn with no input — the turn spec's
recovery entry point — for turns left `idle` by a crash. There is no
automatic resume sweep at startup: recovery re-issues interrupted model
@ -499,6 +504,11 @@ All tests use the in-memory/mocked turn runtime and repo fakes.
correct turn with the correct input type.
- Turn-runtime rejections (unknown call, terminal turn) pass through.
- `sendMessage` never routes to ask-human.
- `stopTurn` aborts every live advance when a turn has concurrent
invocations, and an earlier advance settling does not untrack a later
one.
- A `stopTurn` cancel input that lost the race with a concurrent settle
resolves as a successful stop; a non-terminal rejection still surfaces.
### 13.6 Index

View file

@ -178,7 +178,8 @@ type AdvanceScript = (call: {
}) =>
| { events?: TurnStreamEvent[]; outcome: TurnOutcome }
| { error: unknown }
| { untilAbort: true };
| { untilAbort: true }
| { pending: Promise<TurnOutcome> };
class FakeTurnRuntime implements ITurnRuntime {
createTurnInputs: CreateTurnInput[] = [];
@ -229,6 +230,8 @@ class FakeTurnRuntime implements ITurnRuntime {
};
if ("error" in result) {
stream.fail(result.error);
} else if ("pending" in result) {
void result.pending.then((outcome) => stream.end(outcome));
} else if ("untilAbort" in result) {
const signal = options?.signal;
if (!signal) {
@ -641,6 +644,101 @@ describe("stopTurn and resumeTurn", () => {
/no turns to resume/,
);
});
it("aborts every live advance when a turn has concurrent invocations", async () => {
// A suspended turn with two pending externals gets a permission
// response (advance 1, running) and an async result (advance 2,
// queued on the turn lock in the real runtime). Stop must abort
// both — aborting only the latest leaves the first streaming.
const { sessions, fake } = makeSessions();
const signals: AbortSignal[] = [];
fake.script = ({ signal }) => {
signals.push(signal as AbortSignal);
return { untilAbort: true };
};
const sessionId = await sessions.createSession();
const { turnId } = await sessions.sendMessage(sessionId, user("go"), {
agent: { agentId: "copilot" },
});
const delivery = sessions
.deliverAsyncToolResult(turnId, "B", { output: "done", isError: false })
.catch(() => undefined);
await flush();
expect(fake.advanceCalls).toHaveLength(2);
await sessions.stopTurn(turnId);
await delivery;
expect(signals.map((s) => s.aborted)).toEqual([true, true]);
// Abort path only — no third advance carrying a cancel input.
expect(fake.advanceCalls).toHaveLength(2);
});
it("an earlier advance settling does not untrack a later one", async () => {
// Advance 1 settles while advance 2 is still live. Its cleanup must
// remove only its own entry: stop must still find and abort
// advance 2 instead of falling back to a cancel input.
const { sessions, fake } = makeSessions();
let releaseFirst!: (outcome: TurnOutcome) => void;
const firstOutcome = new Promise<TurnOutcome>((resolve) => {
releaseFirst = resolve;
});
const signals: AbortSignal[] = [];
fake.script = ({ signal }) => {
signals.push(signal as AbortSignal);
if (signals.length === 1) {
return { pending: firstOutcome };
}
if (signals.length === 2) {
return { untilAbort: true };
}
// A cancel-input advance would land here; it must never happen.
return { outcome: completedOutcome() };
};
const sessionId = await sessions.createSession();
const { turnId } = await sessions.sendMessage(sessionId, user("go"), {
agent: { agentId: "copilot" },
});
const delivery = sessions
.deliverAsyncToolResult(turnId, "B", { output: "done", isError: false })
.catch(() => undefined);
await flush();
releaseFirst(completedOutcome());
await flush();
await sessions.stopTurn(turnId);
await delivery;
expect(signals).toHaveLength(2);
expect(signals[1].aborted).toBe(true);
expect(fake.advanceCalls).toHaveLength(2);
});
it("treats a cancel input that lost the race with a settle as a successful stop", async () => {
const { sessions, fake } = makeSessions();
const sessionId = await sessions.createSession();
const { turnId } = await sessions.sendMessage(sessionId, user("go"), {
agent: { agentId: "copilot" },
});
await flush();
fake.setLog(turnId, turnLog(turnId, sessionId, "completed"));
fake.script = () => ({
error: new TurnInputError(`turn ${turnId} is terminal; input rejected`),
});
await expect(sessions.stopTurn(turnId)).resolves.toBeUndefined();
});
it("rethrows a cancel-input rejection when the turn is not terminal", async () => {
const { sessions, fake } = makeSessions();
const sessionId = await sessions.createSession();
const { turnId } = await sessions.sendMessage(sessionId, user("go"), {
agent: { agentId: "copilot" },
});
await flush();
fake.setLog(turnId, turnLog(turnId, sessionId, "suspended"));
fake.script = () => ({
error: new TurnInputError("no pending async tool call B"),
});
await expect(sessions.stopTurn(turnId)).rejects.toThrowError(
/no pending async tool call/,
);
});
});
describe("event forwarding and index maintenance (13.6, 13.7)", () => {

View file

@ -21,12 +21,13 @@ import {
} from "@x/shared/dist/turns.js";
import type { IMonotonicallyIncreasingIdGenerator } from "../../application/lib/id-gen.js";
import { chatActivity } from "../../application/lib/chat-activity.js";
import type {
ITurnRuntime,
Turn,
TurnExecution,
TurnExternalInput,
TurnOutcome,
import {
type ITurnRuntime,
type Turn,
type TurnExecution,
type TurnExternalInput,
TurnInputError,
type TurnOutcome,
} from "../turns/api.js";
// traits.js, not registry.js: the registry's builders transitively reach
// di/container, which imports this module — a cycle. The traits leaf exists
@ -50,6 +51,12 @@ export interface SessionsDependencies {
sessionBus: ISessionBus;
}
interface ActiveAdvance {
sessionId: string | null;
controller: AbortController;
execution: TurnExecution;
}
// The session layer per session-design.md: owns conversations as ordered
// chains of turn references, enforces one active turn per session, assembles
// context as a reference to the previous turn, and maintains the in-memory
@ -64,10 +71,10 @@ export class SessionsImpl implements ISessions {
private readonly index = new SessionIndex();
// Ephemeral: executions this process started, for stopTurn's abort path.
private readonly active = new Map<
string,
{ sessionId: string | null; controller: AbortController; execution: TurnExecution }
>();
// A turn can legally have more than one live advance at once — a running
// invocation plus external-input invocations queued on the turn lock —
// so entries accumulate per turn and each advance removes only its own.
private readonly active = new Map<string, Set<ActiveAdvance>>();
constructor({
sessionRepo,
@ -270,15 +277,35 @@ export class SessionsImpl implements ISessions {
async stopTurn(turnId: string, reason?: string): Promise<void> {
const running = this.active.get(turnId);
if (running) {
running.controller.abort();
await running.execution.outcome.catch(() => undefined);
if (running && running.size > 0) {
// Abort every live advance for this turn: the running invocation
// cancels, and queued ones observe their aborted signal once the
// turn lock frees. Await them all so stop returns settled.
const advances = [...running];
for (const advance of advances) {
advance.controller.abort();
}
await Promise.all(
advances.map((a) => a.execution.outcome.catch(() => undefined)),
);
return;
}
await this.advanceWithInput(turnId, {
type: "cancel",
...(reason === undefined ? {} : { reason }),
});
try {
await this.advanceWithInput(turnId, {
type: "cancel",
...(reason === undefined ? {} : { reason }),
});
} catch (error) {
// A cancel input that loses the race with a concurrent settle is
// a successful stop: the turn is already terminal.
if (error instanceof TurnInputError) {
const turn = await this.turnRuntime.getTurn(turnId);
if (reduceTurn(turn.events).terminal) {
return;
}
}
throw error;
}
}
// Recovery entry for idle (crash-interrupted) turns. Deliberately not run
@ -372,7 +399,10 @@ export class SessionsImpl implements ISessions {
if (sessionId !== null) {
void execution.outcome.catch(() => undefined).finally(() => chatActivity.exit());
}
this.active.set(turnId, { sessionId, controller, execution });
const advance: ActiveAdvance = { sessionId, controller, execution };
const live = this.active.get(turnId) ?? new Set<ActiveAdvance>();
live.add(advance);
this.active.set(turnId, live);
void (async () => {
try {
@ -387,7 +417,18 @@ export class SessionsImpl implements ISessions {
void execution.outcome
.then((outcome) => this.onSettled(sessionId, turnId, outcome))
.catch(() => undefined)
.finally(() => this.active.delete(turnId));
.finally(() => {
// Remove only this advance's entry; a sibling advance for the
// same turn may still be live and must stay stoppable.
const set = this.active.get(turnId);
if (!set) {
return;
}
set.delete(advance);
if (set.size === 0) {
this.active.delete(turnId);
}
});
return execution;
}