Skip to content

Commit 5ba3ac7

Browse files
authored
Merge pull request #264 from pylon-code/fix/steer-requires-active-turn
fix(server): steer a turn only when one is actually running
2 parents b671cb6 + ef1b55b commit 5ba3ac7

3 files changed

Lines changed: 205 additions & 2 deletions

File tree

apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1663,6 +1663,87 @@ describe("ProviderCommandReactor", () => {
16631663
}),
16641664
);
16651665

1666+
effectIt.effect("steers a running admitted turn without reopening admission", () =>
1667+
Effect.gen(function* () {
1668+
// The everyday steer: a user message lands while an admitted turn is
1669+
// running. It must reach the provider tagged with the running turn's
1670+
// request id, and the session must stay as it was: no restart, no
1671+
// pending admission, same active turn.
1672+
const testClock = yield* TestClock.make();
1673+
yield* testClock.setTime(PROVIDER_TURN_ADMISSION_TIMEOUT_MS + 1);
1674+
const requestId = CommandId.make("cmd-steer-admitted-boot");
1675+
const messageId = asMessageId("message-steer-admitted-boot");
1676+
const sessionIncarnationId = RuntimeSessionId.make("session-steer-admitted");
1677+
const activeTurnId = asTurnId("turn-steer-admitted");
1678+
const runningSession = {
1679+
provider: ProviderDriverKind.make("codex"),
1680+
providerInstanceId: ProviderInstanceId.make("codex"),
1681+
status: "running" as const,
1682+
runtimeMode: "approval-required" as const,
1683+
threadId: ThreadId.make("thread-1"),
1684+
cwd: "/tmp/provider-project",
1685+
sessionIncarnationId,
1686+
activeTurnRequestId: requestId,
1687+
activeTurnId,
1688+
createdAt: isoAt(0),
1689+
updatedAt: isoAt(1),
1690+
};
1691+
const harness = yield* Effect.promise(() =>
1692+
createHarness({
1693+
clock: testClock,
1694+
overdueTurnStartBeforeReactor: {
1695+
commandId: requestId,
1696+
messageId,
1697+
createdAt: isoAt(0),
1698+
sessionIncarnationId,
1699+
},
1700+
inventoryEffect: () => Effect.succeed([runningSession]),
1701+
initialRuntimeSessions: [runningSession],
1702+
}),
1703+
);
1704+
const before = yield* Effect.promise(() => harness.readModel());
1705+
const runningThread = before.threads.find((entry) => entry.id === ThreadId.make("thread-1"));
1706+
expect(runningThread?.session?.status).toBe("running");
1707+
expect(runningThread?.session?.activeTurnRequestId).toBe(requestId);
1708+
1709+
const steerRequestId = CommandId.make("cmd-steer-admitted");
1710+
yield* harness.engine.dispatch({
1711+
type: "thread.turn.start",
1712+
commandId: steerRequestId,
1713+
threadId: ThreadId.make("thread-1"),
1714+
message: {
1715+
messageId: asMessageId("user-message-steer-admitted"),
1716+
role: "user",
1717+
text: "actually, also run the tests",
1718+
attachments: [],
1719+
},
1720+
interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE,
1721+
runtimeMode: "approval-required",
1722+
createdAt: isoAt(PROVIDER_TURN_ADMISSION_TIMEOUT_MS + 2),
1723+
});
1724+
yield* Effect.promise(() => waitFor(() => harness.sendTurn.mock.calls.length === 1));
1725+
1726+
const sent = harness.sendTurn.mock.calls[0]?.[0] as ProviderSendTurnInput;
1727+
expect(sent.input).toBe("actually, also run the tests");
1728+
expect(sent.admissionRequestId).toBe(requestId);
1729+
expect(harness.startSession.mock.calls.map((call) => JSON.stringify(call[1]))).toEqual([]);
1730+
expect(sent.sessionIncarnationId).toBe(sessionIncarnationId);
1731+
1732+
const after = yield* Effect.promise(() => harness.readModel());
1733+
const thread = after.threads.find((entry) => entry.id === ThreadId.make("thread-1"));
1734+
expect(thread?.session?.status).toBe("running");
1735+
expect(thread?.session?.activeTurnId).toBe(activeTurnId);
1736+
expect(thread?.session?.activeTurnRequestId).toBe(requestId);
1737+
expect(thread?.session?.pendingTurnRequestId).toBeUndefined();
1738+
expect(thread?.messages.map((message) => message.id)).toContain(
1739+
asMessageId("user-message-steer-admitted"),
1740+
);
1741+
expect(
1742+
thread?.activities.filter((activity) => activity.kind === "provider.turn.start.failed"),
1743+
).toHaveLength(0);
1744+
}),
1745+
);
1746+
16661747
for (const inventoryStatus of ["ready", "absent"] as const) {
16671748
effectIt.effect(`fails an overdue ${inventoryStatus} per-instance inventory as absence`, () =>
16681749
Effect.gen(function* () {

apps/server/src/orchestration/decider.sessionLifecycle.test.ts

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,114 @@ it.layer(NodeServices.layer)("session lifecycle CAS decider", (it) => {
417417
}),
418418
);
419419

420+
it.effect("starts a new turn instead of steering when a running session has no active turn", () =>
421+
Effect.gen(function* () {
422+
// Claude flips the session to "running" on its own system/status
423+
// notifications between turns, so status alone cannot prove a turn
424+
// exists. Steering nothing would hand the provider a turn the admission
425+
// gate can never correlate, and the user's message would vanish.
426+
const runningWithoutTurn = makeSession({
427+
status: "running",
428+
pendingTurnRequestId: undefined,
429+
pendingTurnMessageId: undefined,
430+
pendingTurnRequestedAt: undefined,
431+
pendingTurnDeadlineAt: undefined,
432+
pendingTurnSessionId: undefined,
433+
activeTurnRequestId: undefined,
434+
activeTurnId: null,
435+
});
436+
const commandId = CommandId.make("cmd-running-without-turn");
437+
const decided = yield* decideOrchestrationCommand({
438+
command: {
439+
type: "thread.turn.start",
440+
commandId,
441+
threadId: THREAD_ID,
442+
message: {
443+
messageId: MessageId.make("message-running-without-turn"),
444+
role: "user",
445+
text: "nothing is running, start a turn",
446+
attachments: [],
447+
},
448+
modelSelection: { instanceId: INSTANCE_ID, model: "gpt-5.4" },
449+
runtimeMode: "full-access",
450+
interactionMode: "default",
451+
createdAt: NOW,
452+
},
453+
readModel: makeReadModel(runningWithoutTurn),
454+
});
455+
const events = Array.isArray(decided) ? decided : [decided];
456+
expect(events.map((event) => event.type)).toEqual([
457+
"thread.message-sent",
458+
"thread.session-set",
459+
"thread.turn-start-requested",
460+
]);
461+
expect(events[1]).toMatchObject({
462+
type: "thread.session-set",
463+
payload: {
464+
session: {
465+
status: "starting",
466+
pendingTurnRequestId: commandId,
467+
activeTurnId: null,
468+
},
469+
},
470+
});
471+
expect(events[2]).toMatchObject({
472+
type: "thread.turn-start-requested",
473+
payload: { admissionIntent: { kind: "start", expectedActiveTurnRequestId: null } },
474+
});
475+
}),
476+
);
477+
478+
it.effect("starts a new turn instead of steering a provider-initiated turn", () =>
479+
Effect.gen(function* () {
480+
// A turn the provider opened on its own (Claude continuing after a
481+
// background task) is running but was never admitted, so it has no
482+
// active request id. Steering it would tag the provider's next turn with
483+
// a request id the admission gate cannot correlate. Start exactly.
484+
const providerInitiated = makeSession({
485+
status: "running",
486+
pendingTurnRequestId: undefined,
487+
pendingTurnMessageId: undefined,
488+
pendingTurnRequestedAt: undefined,
489+
pendingTurnDeadlineAt: undefined,
490+
pendingTurnSessionId: undefined,
491+
activeTurnRequestId: undefined,
492+
activeTurnId: TurnId.make("turn-provider-initiated"),
493+
});
494+
const commandId = CommandId.make("cmd-provider-initiated-turn");
495+
const decided = yield* decideOrchestrationCommand({
496+
command: {
497+
type: "thread.turn.start",
498+
commandId,
499+
threadId: THREAD_ID,
500+
message: {
501+
messageId: MessageId.make("message-provider-initiated-turn"),
502+
role: "user",
503+
text: "take over from the background continuation",
504+
attachments: [],
505+
},
506+
modelSelection: { instanceId: INSTANCE_ID, model: "gpt-5.4" },
507+
runtimeMode: "full-access",
508+
interactionMode: "default",
509+
createdAt: NOW,
510+
},
511+
readModel: makeReadModel(providerInitiated),
512+
});
513+
const events = Array.isArray(decided) ? decided : [decided];
514+
expect(events.map((event) => event.type)).toEqual([
515+
"thread.message-sent",
516+
"thread.session-set",
517+
"thread.turn-start-requested",
518+
]);
519+
expect(events[1]).toMatchObject({
520+
payload: { session: { status: "starting", pendingTurnRequestId: commandId } },
521+
});
522+
expect(events[2]).toMatchObject({
523+
payload: { admissionIntent: { kind: "start" } },
524+
});
525+
}),
526+
);
527+
420528
it.effect("captures the exact stop target and projects stopped atomically", () =>
421529
Effect.gen(function* () {
422530
const turnId = TurnId.make("turn-stop-target");

apps/server/src/orchestration/decider.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,12 +1033,26 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand"
10331033
updatedAt: command.createdAt,
10341034
},
10351035
};
1036+
// Steering needs an admitted turn to steer into. A session can sit in
1037+
// "running" with no active turn (Claude reports system/status between
1038+
// turns) or with a turn the provider opened on its own (Claude continuing
1039+
// after a background task). Steering there hands the provider a turn
1040+
// ingestion can never correlate to an admission, so the user's message
1041+
// silently disappears. Under an incarnation-tracked session every
1042+
// admitted turn carries its request id, so a running turn without one is
1043+
// provider-initiated and gets an exact start instead. Sessions without an
1044+
// incarnation predate admission tracking and keep plain steering.
1045+
const hasSteerableTurn =
1046+
targetThread.session?.status === "running" &&
1047+
targetThread.session.activeTurnId !== null &&
1048+
(targetThread.session.activeTurnRequestId !== undefined ||
1049+
targetThread.session.sessionIncarnationId === undefined);
10361050
const admissionIntent = {
10371051
kind:
10381052
targetThread.session?.providerInstanceId !== undefined &&
10391053
targetThread.session.providerInstanceId !== effectiveModelSelection.instanceId
10401054
? ("compatible-transition" as const)
1041-
: targetThread.session?.status === "running" && !providerSettingsChanged
1055+
: hasSteerableTurn && !providerSettingsChanged
10421056
? ("steer" as const)
10431057
: ("start" as const),
10441058
expectedProviderInstanceId: targetThread.session?.providerInstanceId ?? null,
@@ -1071,7 +1085,7 @@ export const decideOrchestrationCommand = Effect.fn("decideOrchestrationCommand"
10711085
},
10721086
};
10731087
const admissionPendingEvents: Array<Omit<OrchestrationEvent, "sequence">> = [];
1074-
if (targetThread.session?.status !== "running" || providerSettingsChanged) {
1088+
if (!hasSteerableTurn || providerSettingsChanged) {
10751089
admissionPendingEvents.push({
10761090
...(yield* withEventBase({
10771091
aggregateKind: "thread",

0 commit comments

Comments
 (0)