Skip to content

Commit aad53cf

Browse files
committed
fix(reactor): persist resolved approval removal with its result turn
Tool cycles write turns only and defer metadata to the next terminal commit. An approved or rejected call drops its pending operation from memory, but a teardown before that terminal left it in metadata.json; a reactor rebuilt from the store re-armed the answered call's gate and, at the original deadline, appended a second tool_result for the same call id, which the prompt tripwire rejects. The turns-only write now also writes metadata when a resolved approval changed the pending operations, so the removal lands with the turn that answers the call. Writing metadata in tryCorrelate instead would drop the op before the re-dispatched result is on disk, leaving a dangling tool_use with no gate to answer it. Cycles that leave pending operations untouched still skip the metadata write.
1 parent e2b235e commit aad53cf

3 files changed

Lines changed: 234 additions & 12 deletions

File tree

‎vendor/intx-inference/PATCHES.md‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,23 @@ segments in the working tree, and `load()` (the path an interrupt-rebuild
469469
takes) reads them, so the tool exchange survives a rebuild exactly as the
470470
old per-batch commit did.
471471

472+
Pending-operation durability contract: a resolved approval's pending
473+
operation leaves `metadata.json` together with the turn that answers the
474+
parked call, never later. `tryCorrelate`'s redispatch and `error_result`
475+
paths and `onGateCleared`'s timeout path set `pendingOperationsChanged`
476+
when they drop the op from memory; the shared turns-only write
477+
(`writeCycleTurns`, called from the `executeTools` tail and from the loop's
478+
`resume.tool_result` branch right after the synthetic error result is
479+
appended) writes metadata after the turns whenever that flag is set, and
480+
`writeMetadata` clears it. Without this, a teardown between the redispatch
481+
and the next terminal left the op on disk; a reactor rebuilt from that store
482+
re-armed the answered call's gate at its original deadline and injected a
483+
second `tool_result` for the same call id, failing the well-formedness
484+
tripwire. Tool cycles that leave pending operations untouched still write
485+
turns only, so the coalescing savings hold. A crash between the turns write
486+
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.
488+
472489
**Disposition:** Promotion candidate. Performance coalescing — collapses K
473490
git commits per agentic turn to one deferred commit while preserving
474491
rebuild-from-disk durability.
@@ -710,7 +727,7 @@ revisit point is the next vendored sync (see `docs/VENDORING.md`).
710727
| errors-ts-classify-abort-reason | Add optional `reason` param to `classifyAbortError`, carried as `raw: { origin }` | Alexander Guy <alexander.guy@pm.me> | This ledger (#errors-ts-classify-abort-reason) | Next vendored sync |
711728
| 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 |
712729
| 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 |
713-
| 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 | Alexander Guy <alexander.guy@pm.me> | This ledger (#reactor-ts-tool-cycle-turns-only) | Next vendored sync |
730+
| 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 |
714731
| 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 |
715732
| 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 |
716733
| 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: 179 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2443,6 +2443,7 @@ describe("createReactor — tool runner failures", () => {
24432443
// the behavior a regressed per-batch commit would break.
24442444
let commits = 0;
24452445
let turnWrites = 0;
2446+
let metadataWrites = 0;
24462447
const countingStore: ContextStore = {
24472448
async load() {
24482449
return {
@@ -2487,7 +2488,7 @@ describe("createReactor — tool runner failures", () => {
24872488
turnWrites += 1;
24882489
},
24892490
async writeMetadata() {
2490-
/* noop */
2491+
metadataWrites += 1;
24912492
},
24922493
async readManifestHistory() {
24932494
throw new Error("not implemented");
@@ -2518,8 +2519,10 @@ describe("createReactor — tool runner failures", () => {
25182519
await waitFor("reactor.done");
25192520

25202521
// One segment write per tool batch; exactly one commit, at the terminal.
2522+
// No pending operation changed, so metadata waits for that commit too.
25212523
expect(turnWrites).toBe(2);
25222524
expect(commits).toBe(1);
2525+
expect(metadataWrites).toBe(1);
25232526
});
25242527
});
25252528

@@ -5376,6 +5379,181 @@ describe("createReactor — approval resume error result", () => {
53765379
});
53775380
});
53785381

5382+
// ---------------------------------------------------------------------------
5383+
// A resolved approval's pending-operation removal reaches disk with the turn
5384+
// that answers the parked call, not at the next terminal commit
5385+
// ---------------------------------------------------------------------------
5386+
5387+
describe("createReactor — resolved approval persists pending-operation removal", () => {
5388+
// The first inference parks on the ask gate. The continuation after the
5389+
// parked call is answered never finishes on its own: it holds until the
5390+
// reactor aborts it, standing in for a teardown that lands mid re-inference
5391+
// (before the next terminal commit).
5392+
function parkedContinuationRunner(continuationStarted: () => void) {
5393+
let call = 0;
5394+
return async function* (
5395+
opts: InferenceHarnessOptions,
5396+
): AsyncGenerator<InferenceEvent> {
5397+
call += 1;
5398+
if (call === 1) {
5399+
yield {
5400+
type: "inference.done",
5401+
seq: opts.nextSeq(),
5402+
data: {
5403+
turn: suspendToolCallTurn,
5404+
usage: emptyUsage(),
5405+
source: TEST_SOURCE,
5406+
},
5407+
};
5408+
return;
5409+
}
5410+
continuationStarted();
5411+
await new Promise<void>((resolve) => {
5412+
const signal = opts.signal;
5413+
if (signal === undefined || signal.aborted) {
5414+
resolve();
5415+
return;
5416+
}
5417+
signal.addEventListener("abort", () => resolve(), { once: true });
5418+
});
5419+
};
5420+
}
5421+
5422+
function askExtension(approvalTimeoutMs: number) {
5423+
return createAuthzExtension({
5424+
authorize: async () => ({
5425+
effect: "ask" as const,
5426+
matchingGrants: [],
5427+
resolvedBy: null,
5428+
}),
5429+
approvalTimeoutMs,
5430+
});
5431+
}
5432+
5433+
// Suspend on the ask gate, deliver `decision`, and abort once the
5434+
// continuation inference starts. Returns the parked op's original deadline.
5435+
async function resolveThenAbortMidContinuation(
5436+
cell: PersistedCell,
5437+
decision: (correlationId: string) => InboundMessage,
5438+
approvalTimeoutMs: number,
5439+
): Promise<number> {
5440+
const continuation = Promise.withResolvers<void>();
5441+
const first = createTestReactor({
5442+
contextStore: makePersistingContextStore(cell),
5443+
director: createDefaultDirector("test agent", []),
5444+
inferenceRunner: parkedContinuationRunner(() => continuation.resolve()),
5445+
toolRunner: makeToolRunner(async (call) => ({
5446+
callId: call.id,
5447+
content: "charged",
5448+
})),
5449+
beforeToolExtensions: [askExtension(approvalTimeoutMs)],
5450+
});
5451+
5452+
first.reactor.start();
5453+
first.reactor.deliver(makeInboundMessage());
5454+
5455+
const blocked = await first.waitFor("reactor.gate.blocked");
5456+
if (blocked.type !== "reactor.gate.blocked") throw new Error("unreachable");
5457+
const correlationId = blocked.data.correlationId;
5458+
if (correlationId === undefined) throw new Error("expected correlationId");
5459+
const deadline = cell.pendingOperations[0]?.timeoutAt;
5460+
if (deadline === undefined) throw new Error("expected a persisted deadline");
5461+
5462+
first.reactor.deliver(decision(correlationId));
5463+
await continuation.promise;
5464+
5465+
first.reactor.abort("admin_kill");
5466+
await first.waitFor("reactor.done");
5467+
return deadline;
5468+
}
5469+
5470+
test("an approved re-dispatch leaves no stale pending operation, and a restart survives the original deadline", async () => {
5471+
const cell: PersistedCell = {
5472+
turns: [],
5473+
pendingOperations: [],
5474+
tokenUsage: emptyUsage(),
5475+
};
5476+
5477+
const deadline = await resolveThenAbortMidContinuation(
5478+
cell,
5479+
(correlationId) => makeApprovalMessage(correlationId),
5480+
500,
5481+
);
5482+
5483+
// The re-dispatched call's result and the parked op's removal are both on
5484+
// disk even though the run never reached a terminal commit.
5485+
expect(cell.pendingOperations).toEqual([]);
5486+
expect(
5487+
cell.turns.some((t) =>
5488+
t.content.some(
5489+
(b) => b.type === "tool_result" && b.callId === "call-ask",
5490+
),
5491+
),
5492+
).toBe(true);
5493+
expect(() => assertWellFormedToolSequence(cell.turns)).not.toThrow();
5494+
5495+
// A reactor rebuilt from that store must not re-arm the answered call's
5496+
// gate: at the original deadline a stale op would inject a second
5497+
// "approval timed out" result for the same call id and the next inference
5498+
// would fail the well-formedness tripwire.
5499+
const second = createTestReactor({
5500+
contextStore: makePersistingContextStore(cell),
5501+
director: createDefaultDirector("test agent", []),
5502+
inferenceRunner: makeInferenceRunner({
5503+
type: "done",
5504+
turn: makeAssistantTurn("resumed"),
5505+
usage: emptyUsage(),
5506+
}),
5507+
beforeToolExtensions: [askExtension(500)],
5508+
});
5509+
second.reactor.start();
5510+
await second.waitFor("reactor.start");
5511+
5512+
// The margin lets a stale gate that fired at the deadline run its
5513+
// continuation to completion before the probe message below.
5514+
await waitUntil(() => Date.now() > deadline + 50);
5515+
second.reactor.deliver(makeInboundMessage());
5516+
const settled = await waitForEvent(
5517+
second.events,
5518+
(e) => e.type === "connector.reply" || e.type === "reactor.done",
5519+
);
5520+
5521+
expect(second.events.filter((e) => e.type === "reactor.error")).toEqual([]);
5522+
expect(settled.type).toBe("connector.reply");
5523+
expect(() => assertWellFormedToolSequence(cell.turns)).not.toThrow();
5524+
5525+
second.reactor.abort("admin_kill");
5526+
await second.waitFor("reactor.done");
5527+
});
5528+
5529+
test("a rejected call's error result and pending-operation removal reach disk before the next terminal", async () => {
5530+
const cell: PersistedCell = {
5531+
turns: [],
5532+
pendingOperations: [],
5533+
tokenUsage: emptyUsage(),
5534+
};
5535+
5536+
await resolveThenAbortMidContinuation(
5537+
cell,
5538+
(correlationId) => makeApprovalMessage(correlationId, "rejected"),
5539+
60_000,
5540+
);
5541+
5542+
expect(cell.pendingOperations).toEqual([]);
5543+
expect(
5544+
cell.turns.some((t) =>
5545+
t.content.some(
5546+
(b) =>
5547+
b.type === "tool_result" &&
5548+
b.callId === "call-ask" &&
5549+
b.isError === true,
5550+
),
5551+
),
5552+
).toBe(true);
5553+
expect(() => assertWellFormedToolSequence(cell.turns)).not.toThrow();
5554+
});
5555+
});
5556+
53795557
// ---------------------------------------------------------------------------
53805558
// Corrupt persisted state surfaces loud at startup
53815559
// ---------------------------------------------------------------------------

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

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,8 @@ export function createReactor(config: ReactorConfig): Reactor {
785785
correlations.remove(correlationId);
786786
if (stateManager !== null) {
787787
stateManager.removePendingOperation(correlationId);
788+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
789+
pendingOperationsChanged = true;
788790
}
789791
// The grant is already recorded (synchronously, in
790792
// resumePendingOperation) with no await since; enqueue the re-dispatch
@@ -808,6 +810,8 @@ export function createReactor(config: ReactorConfig): Reactor {
808810
correlations.remove(correlationId);
809811
if (stateManager !== null) {
810812
stateManager.removePendingOperation(correlationId);
813+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
814+
pendingOperationsChanged = true;
811815
}
812816
enqueue({ type: "resume.tool_result", result: dispatch.result });
813817
break;
@@ -1224,16 +1228,26 @@ export function createReactor(config: ReactorConfig): Reactor {
12241228
// agentic turn to one deferred commit.
12251229
//
12261230
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1227-
if (addToHistory && pendingCompactOutput === null) {
1228-
const revision = stateManager?.getTurnsRevision();
1229-
if (
1230-
stateManager !== null &&
1231-
revision !== undefined &&
1232-
revision !== lastWrittenTurnsRevision
1233-
) {
1234-
await contextStore.writeTurns(stateManager.getTurns());
1235-
lastWrittenTurnsRevision = revision;
1236-
}
1231+
if (addToHistory) {
1232+
await writeCycleTurns();
1233+
}
1234+
}
1235+
1236+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1237+
// The turns-only write between terminals. When the turn just appended
1238+
// answers a parked call, the pending operation it resolved is gone from
1239+
// memory too; metadata goes out with the turns so a rebuild from disk never
1240+
// re-arms that call's gate and answers it a second time at the deadline.
1241+
// Cycles that leave pending operations untouched still skip metadata.
1242+
async function writeCycleTurns(): Promise<void> {
1243+
if (stateManager === null || pendingCompactOutput !== null) return;
1244+
const revision = stateManager.getTurnsRevision();
1245+
if (revision !== lastWrittenTurnsRevision) {
1246+
await contextStore.writeTurns(stateManager.getTurns());
1247+
lastWrittenTurnsRevision = revision;
1248+
}
1249+
if (pendingOperationsChanged) {
1250+
await writeMetadata();
12371251
}
12381252
}
12391253

@@ -1379,6 +1393,8 @@ export function createReactor(config: ReactorConfig): Reactor {
13791393
pendingOperations: stateManager.getPendingOperations(),
13801394
tokenUsage: stateManager.getTokenUsage(),
13811395
});
1396+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1397+
pendingOperationsChanged = false;
13821398
}
13831399

13841400
// -------------------------------------------------------------------------
@@ -1443,6 +1459,8 @@ export function createReactor(config: ReactorConfig): Reactor {
14431459
correlations.remove(op.correlationId);
14441460
if (stateManager !== null) {
14451461
stateManager.removePendingOperation(op.correlationId);
1462+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1463+
pendingOperationsChanged = true;
14461464
}
14471465
enqueue({
14481466
type: "resume.tool_result",
@@ -1650,6 +1668,8 @@ export function createReactor(config: ReactorConfig): Reactor {
16501668
if (stateManager !== null) {
16511669
stateManager.appendTurn(createToolResultTurn([event.result]));
16521670
}
1671+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1672+
await writeCycleTurns();
16531673
}
16541674

16551675
let actions;
@@ -1869,6 +1889,13 @@ export function createReactor(config: ReactorConfig): Reactor {
18691889
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-last-written-turns-revision
18701890
let lastWrittenTurnsRevision = 0;
18711891

1892+
// Set when a resolved approval drops its pending operation from memory;
1893+
// cleared by the next metadata write. Lets the turns-only write persist
1894+
// metadata only for the cycles that need it.
1895+
//
1896+
// Locally patched — see vendor/intx-inference/PATCHES.md#reactor-ts-tool-cycle-turns-only
1897+
let pendingOperationsChanged = false;
1898+
18721899
async function initiateShutdown(): Promise<void> {
18731900
if (shutdownStarted) return;
18741901
shutdownStarted = true;

0 commit comments

Comments
 (0)