Skip to content

Commit edbfce1

Browse files
t3dotggclaude
andcommitted
fix(server): restart recovery skips settled delegated deliveries
Recovery finalized every terminal run whose input was ever a delegated delivery. Each call reloaded the parent thread and returned early. A parent with 190 past deliveries held startup for about 66 s. Recovery now finalizes only runs that consumed a cohort's current delivery message. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 348f217 commit edbfce1

2 files changed

Lines changed: 149 additions & 14 deletions

File tree

‎apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import * as DateTime from "effect/DateTime";
2121
import * as Effect from "effect/Effect";
2222
import * as Layer from "effect/Layer";
2323
import * as Stream from "effect/Stream";
24+
import * as Tracer from "effect/Tracer";
2425

2526
import * as CheckpointStore from "../checkpointing/CheckpointStore.ts";
2627
import * as ServerConfig from "../config.ts";
@@ -749,6 +750,24 @@ it.layer(layerTest)("delegated completion delivery repairs", (it) => {
749750

750751
// Runtime reconciliation writes restart cancellations under this command
751752
// prefix, which the live terminal-run listener skips.
753+
// Collects the SQL text of every statement run under the returned tracer.
754+
const traceSqlStatements = () => {
755+
const statements: Array<string> = [];
756+
const tracer = Tracer.make({
757+
span(options) {
758+
const span = new Tracer.NativeSpan(options);
759+
const end = span.end.bind(span);
760+
span.end = (endTime, exit) => {
761+
end(endTime, exit);
762+
const query = span.attributes.get("db.query.text");
763+
if (typeof query === "string") statements.push(query);
764+
};
765+
return span;
766+
},
767+
});
768+
return { statements, tracer };
769+
};
770+
752771
const reconcileCommandId = (name: string) => CommandId.make(`command:runtime-reconcile:${name}`);
753772

754773
const parentProviderThreadId = (threadId: ThreadId) =>
@@ -1578,6 +1597,111 @@ it.layer(layerTest)("delegated tasks across a server restart", (it) => {
15781597
}),
15791598
);
15801599

1600+
it.effect("settles only the current delivery of a parent with long delivery history", () =>
1601+
Effect.gen(function* () {
1602+
const orchestrator = yield* Orchestrator.OrchestratorV2;
1603+
const eventSink = yield* EventSink.EventSinkV2;
1604+
const now = yield* DateTime.now;
1605+
const threadId = ThreadId.make("thread:delivery-history");
1606+
const runId = RunId.make("run:delivery-history");
1607+
const taskId = NodeId.make("node:delivery-history-task");
1608+
const currentMessageId = MessageId.make(`message:delegated-delivery:${threadId}`);
1609+
const currentRunId = RunId.make("run:delivery-history:current");
1610+
const HISTORY = 40;
1611+
yield* seedParentWithTerminalTask({
1612+
threadId,
1613+
projectId: ProjectId.make("project:delivery-history"),
1614+
runId,
1615+
rootNodeId: NodeId.make("node:delivery-history-root"),
1616+
taskId,
1617+
deliveryState: "claimed",
1618+
completionWake: "always",
1619+
deliveryTaskIds: [taskId],
1620+
now,
1621+
});
1622+
const deliveryMessage = (id: MessageId, deliveryRunId: RunId) => ({
1623+
id: EventId.make(`event:${deliveryRunId}:message`),
1624+
type: "message.updated" as const,
1625+
threadId,
1626+
runId: deliveryRunId,
1627+
occurredAt: now,
1628+
payload: {
1629+
id,
1630+
threadId,
1631+
runId: deliveryRunId,
1632+
nodeId: null,
1633+
role: "user" as const,
1634+
text: `Delegated task ${taskId} reached a terminal state.`,
1635+
attachments: [],
1636+
streaming: false,
1637+
createdBy: "agent" as const,
1638+
creationSource: "server" as const,
1639+
createdAt: now,
1640+
updatedAt: now,
1641+
delegatedCompletion: { parentRunId: runId, generation: 1, taskIds: [taskId] },
1642+
},
1643+
});
1644+
// Earlier deliveries that already settled. Each one still owns a
1645+
// delegated message and a terminal run.
1646+
const history = Array.from({ length: HISTORY }, (_, index) => {
1647+
const historyRunId = RunId.make(`run:delivery-history:${index}`);
1648+
const messageId = MessageId.make(`message:delivery-history:${index}`);
1649+
return [
1650+
deliveryMessage(messageId, historyRunId),
1651+
runEvent({
1652+
threadId,
1653+
runId: historyRunId,
1654+
ordinal: index + 2,
1655+
status: "completed",
1656+
now,
1657+
providerThreadId: parentProviderThreadId(threadId),
1658+
userMessageId: messageId,
1659+
}),
1660+
];
1661+
}).flat();
1662+
yield* eventSink.write({
1663+
commandId: reconcileCommandId("delivery-history"),
1664+
events: [
1665+
...history,
1666+
deliveryMessage(currentMessageId, currentRunId),
1667+
runEvent({
1668+
threadId,
1669+
runId: currentRunId,
1670+
ordinal: HISTORY + 2,
1671+
status: "completed",
1672+
now,
1673+
providerThreadId: parentProviderThreadId(threadId),
1674+
userMessageId: currentMessageId,
1675+
}),
1676+
],
1677+
});
1678+
1679+
const { statements, tracer } = traceSqlStatements();
1680+
yield* orchestrator.recoverDelegatedTasks.pipe(Effect.withTracer(tracer));
1681+
1682+
const projection = yield* orchestrator.getThreadProjection(threadId);
1683+
assert.equal(
1684+
projection.subagents.find((row) => row.id === taskId)?.completionDelivery?.state,
1685+
"delivered",
1686+
);
1687+
assert.isNull(projection.runs.find((row) => row.id === runId)?.delegatedCompletion?.delivery);
1688+
// Recovery reads the delivery message once, for the current delivery
1689+
// run only. History must not cost one thread reload per past delivery.
1690+
assert.lengthOf(
1691+
statements.filter((statement) =>
1692+
statement.includes("FROM orchestration_v2_projection_messages AS message"),
1693+
),
1694+
1,
1695+
);
1696+
1697+
// A second recovery finds nothing open on this thread and changes nothing.
1698+
yield* orchestrator.recoverDelegatedTasks;
1699+
const again = yield* orchestrator.getThreadProjection(threadId);
1700+
assert.deepEqual(again.runs, projection.runs);
1701+
assert.deepEqual(again.subagents, projection.subagents);
1702+
}),
1703+
);
1704+
15811705
it.effect("settles a cancelled child whose held wakes wait behind it", () =>
15821706
Effect.gen(function* () {
15831707
const orchestrator = yield* Orchestrator.OrchestratorV2;

‎apps/server/src/orchestration-v2/Orchestrator.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11333,6 +11333,11 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
1133311333
// queue.resume command arrives.
1133411334
const recoverDelegatedTasks = Effect.gen(function* () {
1133511335
yield* projectionStore.getRecoveryThreadIds("subagent-results").pipe(
11336+
// Candidate counts let a startup trace show whether recovery scales
11337+
// with pending work or with history.
11338+
Effect.tap((threadIds) =>
11339+
Effect.annotateCurrentSpan("recovery.subagentResultThreads", threadIds.length),
11340+
),
1133611341
Effect.flatMap((threadIds) =>
1133711342
Effect.forEach(
1133811343
threadIds,
@@ -11360,6 +11365,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
1136011365
),
1136111366
);
1136211367
yield* projectionStore.getRecoveryThreadIds("delegated-completions").pipe(
11368+
Effect.tap((threadIds) =>
11369+
Effect.annotateCurrentSpan("recovery.delegatedCompletionThreads", threadIds.length),
11370+
),
1136311371
Effect.flatMap((threadIds) =>
1136411372
Effect.forEach(
1136511373
threadIds,
@@ -11368,18 +11376,23 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
1136811376
.withLock(
1136911377
threadId,
1137011378
Effect.gen(function* () {
11371-
const projection = yield* projectionStore.getThreadRecords(threadId, [
11372-
"runs",
11373-
"messages",
11374-
]);
11379+
const projection = yield* projectionStore.getThreadRecords(threadId, ["runs"]);
11380+
// Only the run that consumed a cohort's current delivery
11381+
// message can settle it. A long-lived parent keeps every
11382+
// past delivery run, and finalizing each of them would
11383+
// reload the thread once per historical delivery.
11384+
const openDeliveryMessageIds = new Set(
11385+
projection.runs.flatMap((run) =>
11386+
run.delegatedCompletion?.delivery == null
11387+
? []
11388+
: [run.delegatedCompletion.delivery.messageId],
11389+
),
11390+
);
1137511391
const terminalDeliveryRunIds = projection.runs
11376-
.filter((run) => delegatedTaskTerminalStatus(run.status) !== null)
11377-
.filter((run) =>
11378-
projection.messages.some(
11379-
(message) =>
11380-
message.id === run.userMessageId &&
11381-
message.delegatedCompletion !== undefined,
11382-
),
11392+
.filter(
11393+
(run) =>
11394+
delegatedTaskTerminalStatus(run.status) !== null &&
11395+
openDeliveryMessageIds.has(run.userMessageId),
1138311396
)
1138411397
.map((run) => run.id);
1138511398
for (const runId of terminalDeliveryRunIds) {
@@ -11388,9 +11401,7 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
1138811401
const refreshed =
1138911402
terminalDeliveryRunIds.length === 0
1139011403
? projection
11391-
: yield* projectionStore.getThreadRecords(threadId, ["runs", "messages"], {
11392-
messageRoles: ["user"],
11393-
});
11404+
: yield* projectionStore.getThreadRecords(threadId, ["runs"]);
1139411405
for (const run of refreshed.runs) {
1139511406
if (
1139611407
run.delegatedCompletion?.delivery !== null &&

0 commit comments

Comments
 (0)