Skip to content

Commit 0278479

Browse files
committed
fix(reactor): never re-answer a parked call that already has a result
The pending-operation removal reaches metadata in a second await after the result turn, so a crash between the two leaves a stale op for a call history already answers. A reactor rebuilt from that store re-armed its gate and, at the original deadline, appended a second tool_result for the same call id, which the prompt tripwire rejects. Startup now drops and persists the removal of any loaded op whose parked call already has a result, and the approval timeout injects no synthetic result for such a call.
1 parent 19ce192 commit 0278479

3 files changed

Lines changed: 199 additions & 4 deletions

File tree

‎vendor/intx-inference/PATCHES.md‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,8 @@ second `tool_result` for the same call id, failing the well-formedness
484484
tripwire. Tool cycles that leave pending operations untouched still write
485485
turns only, so the coalescing savings hold. A crash between the turns write
486486
and the metadata write can still leave a stale op for an answered call;
487-
that narrower window is the rehydration guard's concern, not this patch's.
487+
that narrower window is #reactor-ts-skip-answered-parked-call's concern, not
488+
this patch's.
488489

489490
**Disposition:** Promotion candidate. Performance coalescing — collapses K
490491
git commits per agentic turn to one deferred commit while preserving
@@ -498,6 +499,38 @@ batch committed); the turns still reach disk segments, so only the cycle's
498499
manifest records and metadata staging stay uncommitted. Intentionally not
499500
committed; re-verify after upstream doom-loop accounting changes.
500501

502+
## reactor-ts-skip-answered-parked-call
503+
504+
`reactor.ts` — A parked call that history already answers is never answered
505+
again. `start()` drops every loaded pending operation whose
506+
`suspendedCall.id` already has a `tool_result` in the loaded turns: the
507+
state manager never holds it, `rehydrateGates` never re-arms it, and the
508+
removal is written to metadata before `reactor.start`. `onGateCleared`'s
509+
timeout path still removes the op but, when the parked call already has a
510+
result, enqueues no synthetic "approval timed out" result (and no
511+
`reactor.gate.cleared`) and logs a warning instead. Both checks scan the
512+
turns through the module-level `answeredCallIds` helper.
513+
514+
Defense in depth behind the pending-operation durability contract in
515+
#reactor-ts-tool-cycle-turns-only: that patch writes the result turn and then
516+
metadata in two awaits, so a crash between them leaves a stale op for an
517+
answered call. Re-armed, it fired at the original deadline, appended a second
518+
`tool_result` for the same call id, and the next inference failed the
519+
well-formedness tripwire (`Malformed tool sequence: duplicate tool_result`),
520+
killing the session. The timeout guard leaves `pendingOperationsChanged` set
521+
rather than writing metadata from the timer callback; the next turns-only
522+
write or terminal persists the removal, and a crash before then is caught by
523+
the startup check.
524+
525+
**Disposition:** Promotion candidate. Upstream's rehydration and timeout
526+
paths trust persisted pending operations without consulting history.
527+
**Removal path:** Upstream PR making rehydration and the approval timeout
528+
skip calls that already have a result.
529+
**Re-carry:** new at this patch; sits in `start()` around
530+
`createStateManager`/`rehydrateGates` and inside the `onGateCleared` timeout
531+
branch. Re-verify after upstream changes to rehydration or to the ask-rail
532+
timeout result.
533+
501534
## reactor-ts-skip-unchanged-history
502535

503536
`reactor.ts` — Skip re-serializing unchanged history on checkpoint.
@@ -728,6 +761,7 @@ revisit point is the next vendored sync (see `docs/VENDORING.md`).
728761
| harness-ts-inactivity-on-semantic-progress | Gate the inactivity watchdog's re-arm on parsed events, not raw SSE bytes | Alexander Guy <alexander.guy@pm.me> | This ledger (#harness-ts-inactivity-on-semantic-progress) | Next vendored sync |
729762
| reactor-ts-correlating-ids-leak | Wrap `tryCorrelate` in try/finally so `correlatingIds` clears on success dispatch paths | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-correlating-ids-leak) | Next vendored sync |
730763
| reactor-ts-tool-cycle-turns-only (supersedes reactor-ts-checkpoint-after-tool-cycle) | `executeTools` writes turns to store segments only and defers the cycle commit to terminals; metadata rides along only when a resolved approval dropped a pending operation | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-tool-cycle-turns-only) | Next vendored sync |
764+
| reactor-ts-skip-answered-parked-call | Drop pending operations whose parked call already has a result at rehydration, and inject no approval-timeout result for such a call | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-skip-answered-parked-call) | Next vendored sync |
731765
| reactor-ts-skip-unchanged-history (+ reactor-ts-last-written-turns-revision) | Skip `contextStore.writeTurns` when `getTurnsRevision()` is unchanged | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-skip-unchanged-history) | Next vendored sync |
732766
| reactor-ts-compact-publish-then-memory | Persist compact blobs, stage `writeTurns`, commit, then `replaceTurns` so memory stays on the old generation until publication | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-compact-publish-then-memory) | Next vendored sync |
733767
| reactor-ts-after-checkpoint-director-only | Gate `afterCheckpoint` on `hasOverride` so auto-commits do not emit it | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-after-checkpoint-director-only) | Next vendored sync |

‎vendor/intx-inference/src/reactor.test.ts‎

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createCorrelationRegistry } from "./correlation";
77
import { createReactor, mergeInferenceOptions } from "./reactor";
88
import { createDefaultDependencies } from "./providers";
99
import { createDefaultDirector } from "./default-director";
10-
import { assertWellFormedToolSequence } from "./turns";
10+
import { assertWellFormedToolSequence, createToolResultTurn } from "./turns";
1111
import { createInboundMessage } from "@intx/mime";
1212
import { waitUntil } from "@intx/types/testing";
1313
import { PerCallInferenceOptions } from "@intx/types/runtime";
@@ -5554,6 +5554,125 @@ describe("createReactor — resolved approval persists pending-operation removal
55545554
});
55555555
});
55565556

5557+
// ---------------------------------------------------------------------------
5558+
// A parked call that already has a result is never answered again
5559+
// ---------------------------------------------------------------------------
5560+
5561+
describe("createReactor — answered parked call is not re-answered", () => {
5562+
function parkedOp(correlationId: string, timeoutAt: number): PendingOperation {
5563+
return {
5564+
correlationId,
5565+
kind: "approval",
5566+
registeredAt: Date.now(),
5567+
gateId: `pending-${correlationId}`,
5568+
timeoutAt,
5569+
suspendedCall: { id: "call-ask", name: "charge_card", arguments: {} },
5570+
};
5571+
}
5572+
5573+
function resultsFor(turns: ConversationTurn[], callId: string): number {
5574+
return turns
5575+
.flatMap((t) => t.content)
5576+
.filter((b) => b.type === "tool_result" && b.callId === callId).length;
5577+
}
5578+
5579+
test("startup drops and persists the removal of a pending operation whose call already has a result", async () => {
5580+
// The state a crash between the result-turn write and the metadata write
5581+
// leaves behind: the parked call is answered on disk, but its pending
5582+
// operation was never removed.
5583+
const deadline = Date.now() + 200;
5584+
const cell: PersistedCell = {
5585+
turns: [
5586+
suspendToolCallTurn,
5587+
createToolResultTurn([{ callId: "call-ask", content: "charged" }]),
5588+
],
5589+
pendingOperations: [parkedOp("stale-corr", deadline)],
5590+
tokenUsage: emptyUsage(),
5591+
};
5592+
5593+
const { reactor, events, waitFor } = createTestReactor({
5594+
contextStore: makePersistingContextStore(cell),
5595+
director: createDefaultDirector("test agent", []),
5596+
inferenceRunner: makeInferenceRunner({
5597+
type: "done",
5598+
turn: makeAssistantTurn("resumed"),
5599+
usage: emptyUsage(),
5600+
}),
5601+
});
5602+
reactor.start();
5603+
await waitFor("reactor.start");
5604+
5605+
expect(cell.pendingOperations).toEqual([]);
5606+
5607+
// A re-armed gate would fire at the deadline and append "approval timed
5608+
// out" for the answered call; the next inference would then fail the
5609+
// well-formedness tripwire.
5610+
await waitUntil(() => Date.now() > deadline + 50);
5611+
reactor.deliver(makeInboundMessage());
5612+
const settled = await waitForEvent(
5613+
events,
5614+
(e) => e.type === "connector.reply" || e.type === "reactor.done",
5615+
);
5616+
5617+
expect(events.filter((e) => e.type === "reactor.error")).toEqual([]);
5618+
expect(settled.type).toBe("connector.reply");
5619+
expect(resultsFor(cell.turns, "call-ask")).toBe(1);
5620+
5621+
reactor.abort("admin_kill");
5622+
await waitFor("reactor.done");
5623+
});
5624+
5625+
test("a gate timeout for a parked call that already has a result injects no second result", async () => {
5626+
// The op is live at startup (its call is unanswered), then the call is
5627+
// answered by a path other than its approval, so only the timeout guard
5628+
// stands between the deadline and a duplicate tool_result.
5629+
const deadline = Date.now() + 200;
5630+
const cell: PersistedCell = {
5631+
turns: [suspendToolCallTurn],
5632+
pendingOperations: [parkedOp("answered-corr", deadline)],
5633+
tokenUsage: emptyUsage(),
5634+
};
5635+
5636+
let messages = 0;
5637+
const { reactor, events, waitFor } = createTestReactor({
5638+
contextStore: makePersistingContextStore(cell),
5639+
director: directorFromTable(
5640+
{
5641+
"message.received": (_e, _s, caps) => {
5642+
messages += 1;
5643+
return messages === 1
5644+
? caps.executeTools([
5645+
{ id: "call-ask", name: "charge_card", arguments: {} },
5646+
])
5647+
: caps.reply("ok");
5648+
},
5649+
},
5650+
"wait",
5651+
),
5652+
toolRunner: makeToolRunner(async (call) => ({
5653+
callId: call.id,
5654+
content: "charged",
5655+
})),
5656+
});
5657+
reactor.start();
5658+
await waitFor("reactor.start");
5659+
5660+
reactor.deliver(makeInboundMessage());
5661+
await waitFor("tool.done");
5662+
expect(resultsFor(cell.turns, "call-ask")).toBe(1);
5663+
5664+
await waitUntil(() => Date.now() > deadline + 50);
5665+
reactor.deliver(makeInboundMessage());
5666+
await waitFor("connector.reply");
5667+
5668+
expect(resultsFor(cell.turns, "call-ask")).toBe(1);
5669+
expect(() => assertWellFormedToolSequence(cell.turns)).not.toThrow();
5670+
5671+
reactor.abort("admin_kill");
5672+
await waitFor("reactor.done");
5673+
});
5674+
});
5675+
55575676
// ---------------------------------------------------------------------------
55585677
// Corrupt persisted state surfaces loud at startup
55595678
// ---------------------------------------------------------------------------

‎vendor/intx-inference/src/reactor.ts‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,17 @@ function isTrivialNoopShellCommand(command: unknown): boolean {
329329
return TRIVIAL_NOOP_SHELL_PROGRAMS.has(unquoted);
330330
}
331331

332+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-skip-answered-parked-call
333+
function answeredCallIds(turns: readonly ConversationTurn[]): Set<string> {
334+
const answered = new Set<string>();
335+
for (const turn of turns) {
336+
for (const block of turn.content) {
337+
if (block.type === "tool_result") answered.add(block.callId);
338+
}
339+
}
340+
return answered;
341+
}
342+
332343
/**
333344
* Creates a reactor instance bound to the given configuration.
334345
* Call `start()` to begin the event loop.
@@ -1462,6 +1473,17 @@ export function createReactor(config: ReactorConfig): Reactor {
14621473
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
14631474
pendingOperationsChanged = true;
14641475
}
1476+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-skip-answered-parked-call
1477+
// A call that already has a result was resumed some other way; a
1478+
// second result for the same id is a malformed tool sequence that
1479+
// fails the next inference, so the timeout drives no continuation.
1480+
if (
1481+
stateManager !== null &&
1482+
answeredCallIds(stateManager.getTurns()).has(op.suspendedCall.id)
1483+
) {
1484+
logger.warn`Skipped approval timeout result for ${op.suspendedCall.id}: the parked call already has a result`;
1485+
return;
1486+
}
14651487
enqueue({
14661488
type: "resume.tool_result",
14671489
result: {
@@ -1964,10 +1986,25 @@ export function createReactor(config: ReactorConfig): Reactor {
19641986
return;
19651987
}
19661988

1989+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-skip-answered-parked-call
1990+
// A crash between the result-turn write and the metadata write leaves a
1991+
// pending operation for a call history already answers. Re-arming it
1992+
// would answer the call again at its deadline, so it is dropped here
1993+
// and its removal persisted before the reactor starts.
1994+
const answered = answeredCallIds(initialTurns);
1995+
const answeredOps = initialOps.filter(
1996+
(op) =>
1997+
op.suspendedCall !== undefined && answered.has(op.suspendedCall.id),
1998+
);
1999+
for (const op of answeredOps) {
2000+
logger.warn`Dropped pending operation ${op.correlationId}: its parked call ${op.suspendedCall?.id ?? ""} already has a result`;
2001+
}
2002+
const liveOps = initialOps.filter((op) => !answeredOps.includes(op));
2003+
19672004
stateManager = createStateManager(
19682005
sessionId,
19692006
initialTurns,
1970-
initialOps,
2007+
liveOps,
19712008
initialUsage,
19722009
);
19732010

@@ -1986,10 +2023,15 @@ export function createReactor(config: ReactorConfig): Reactor {
19862023
// correlationId or gateId. A throw must surface as reactor.error plus
19872024
// reactor.done (matching the load-failure path), not brick the reactor
19882025
// as a silent unhandled rejection.
1989-
rehydrateGates(initialOps);
2026+
rehydrateGates(liveOps);
19902027

19912028
stateManager.setGatesSnapshot(gates.snapshot());
19922029

2030+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-skip-answered-parked-call
2031+
if (answeredOps.length > 0) {
2032+
await writeMetadata();
2033+
}
2034+
19932035
emit({ type: "reactor.start", seq: nextSeq(), data: {} });
19942036

19952037
const bufferedDeliveries = startupDeliveries;

0 commit comments

Comments
 (0)