Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/deterministic-runtime-deadlines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@agent-bundle/runtime": patch
"agent-bundle": patch
---

Accept an optional `now` time source on `createAgentRenderEventSequence` and run the render dispatcher's `maxElapsedMs` deadline — the event sequence's elapsed check and the pending-boundary deadline sleep — against one injectable clock, so a Flight render's deadline can be driven by a test clock instead of wall-clock time. Add a `timers` option (`McpProbeTimers`) to the Workbench MCP probe service so its total-budget timeout, bounded teardown wait, and detached plugin-data cap can be scheduled without real timers; production behavior is unchanged. (#434)
61 changes: 46 additions & 15 deletions packages/agent-bundle/src/dev/playground/mcp-probe-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const mcpProbeFailureTextLimit = 2_048;
const mcpProbeCapabilityLimit = 32;
const mcpProbeNameTextLimit = 256;
/** How long a probe response waits for transport teardown before detaching it. */
const mcpProbeTeardownWaitMs = 50;
export const mcpProbeTeardownWaitMs = 50;
/**
* Upper bound a detached teardown may hold the plugin-data directory. The
* stdio transport's close runs its own TERM/KILL sequence, so this only guards
Expand All @@ -72,6 +72,33 @@ const connectionErrorCodes = new Set([

export type McpProbeTransport = Transport;

/**
* Timer seam behind every probe delay — the total-budget timeout, the bounded
* teardown wait, the detached plugin-data cap, and the removal retry. Production
* uses Node timers; tests inject a manual scheduler and fire timers in event
* order so no wall-clock time is involved. `schedule` returns the cancel.
*/
export interface McpProbeTimers {
readonly schedule: (
callback: () => void,
delayMs: number,
options: Readonly<{
/** A timer that must not keep the process alive on its own (`timer.unref()`). */
readonly unref: boolean;
}>,
) => () => void;
}

const nodeTimers: McpProbeTimers = {
schedule: (callback, delayMs, options) => {
const timer = setTimeout(callback, delayMs);
if (options.unref) timer.unref();
return () => {
clearTimeout(timer);
};
},
};

export interface McpProbeClient {
close(): Promise<void>;
connect(transport: Transport): Promise<void>;
Expand Down Expand Up @@ -100,6 +127,8 @@ export interface McpProbeServiceOptions {
readonly projectRoot: string;
readonly registry?: TargetRegistry;
readonly timeoutMs?: number;
/** Testing seam for every probe delay; production keeps Node timers. */
readonly timers?: McpProbeTimers;
}

export class McpProbeTargetNotFoundError extends Error {
Expand Down Expand Up @@ -314,6 +343,7 @@ export class McpProbeService {
readonly #registry: TargetRegistry;
readonly #removePluginData: (pluginData: string) => Promise<void>;
readonly #timeoutMs: number;
readonly #timers: McpProbeTimers;

constructor(options: McpProbeServiceOptions) {
this.#clock = options.clock ?? (() => performance.now());
Expand Down Expand Up @@ -341,6 +371,7 @@ export class McpProbeService {
this.#registry = options.registry ?? createDefaultRegistry();
this.#removePluginData = options.removePluginData ?? removePluginData;
this.#timeoutMs = positiveTimeout(options.timeoutMs ?? mcpProbeTimeoutMs);
this.#timers = options.timers ?? nodeTimers;
}

probe(options: {
Expand Down Expand Up @@ -448,20 +479,20 @@ export class McpProbeService {
* very case the cap bounds.
*/
#removePluginDataAfter(teardown: Promise<unknown>, pluginData: string): Promise<void> {
let cap: NodeJS.Timeout | undefined;
let cancelCap: (() => void) | undefined;
let capWon = false;
// The cap stays referenced on purpose: it is the only handle guaranteeing
// the removal runs when a stalled teardown outlives Workbench shutdown,
// and it is cleared the moment the teardown settles.
const capped = new Promise<void>((resolvePromise) => {
cap = setTimeout(() => {
cancelCap = this.#timers.schedule(() => {
capWon = true;
resolvePromise();
}, this.#pluginDataTeardownCapMs);
}, this.#pluginDataTeardownCapMs, { unref: false });
});
const pending = Promise.race([teardown, capped])
.then(() => {
if (cap !== undefined) clearTimeout(cap);
cancelCap?.();
return this.#removePluginData(pluginData);
})
.then(() => undefined, () => {
Expand All @@ -472,7 +503,9 @@ export class McpProbeService {
// The teardown settled (a close may have failed fast) but the child
// still held the directory for a moment: one bounded, fenced retry.
this.#track(
new Promise<void>((resolvePromise) => { setTimeout(resolvePromise, mcpProbePluginDataRetryDelayMs); })
new Promise<void>((resolvePromise) => {
this.#timers.schedule(resolvePromise, mcpProbePluginDataRetryDelayMs, { unref: false });
})
.then(() => this.#removePluginData(pluginData))
.then(() => undefined, () => undefined),
);
Expand Down Expand Up @@ -636,7 +669,7 @@ export class McpProbeService {
});
return report;
} finally {
let timer: NodeJS.Timeout | undefined;
let cancelWait: (() => void) | undefined;
// Keep transport teardown running through its TERM/KILL path without
// allowing a stalled close to extend the probe's total time budget. The
// plugin-data removal is chained behind that teardown, so a close that
Expand All @@ -651,13 +684,12 @@ export class McpProbeService {
]);
const cleanup = this.#removePluginDataAfter(teardown, options.pluginData);
const teardownWait = new Promise<void>((resolvePromise) => {
timer = setTimeout(resolvePromise, mcpProbeTeardownWaitMs);
timer.unref();
cancelWait = this.#timers.schedule(resolvePromise, mcpProbeTeardownWaitMs, { unref: true });
});
try {
await Promise.race([cleanup, teardownWait]);
} finally {
if (timer !== undefined) clearTimeout(timer);
cancelWait?.();
}
}
}
Expand Down Expand Up @@ -720,18 +752,17 @@ export class McpProbeService {
void settledClose(onTimeout);
throw new McpProbeTimeoutError(kind);
}
let timer: NodeJS.Timeout | undefined;
let cancelTimeout: (() => void) | undefined;
const timedOut = new Promise<never>((_resolve, reject) => {
timer = setTimeout(() => {
cancelTimeout = this.#timers.schedule(() => {
void settledClose(onTimeout);
reject(new McpProbeTimeoutError(kind));
}, remaining);
timer.unref();
}, remaining, { unref: true });
});
try {
return await Promise.race([operation, timedOut]);
} finally {
if (timer !== undefined) clearTimeout(timer);
cancelTimeout?.();
}
}
}
136 changes: 103 additions & 33 deletions packages/agent-bundle/tests/mcp-probe-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,74 @@ import {
McpProbeService,
McpProbeTargetNotFoundError,
mcpProbeInstructionTextLimit,
mcpProbePluginDataTeardownCapMs,
mcpProbeTeardownWaitMs,
mcpProbeToolLimit,
type McpProbeClient,
type McpProbeServiceOptions,
type McpProbeTimers,
type McpProbeTransport,
} from '../src/dev/playground/mcp-probe-service.ts';

interface ManualTimer {
readonly callback: () => void;
readonly delayMs: number;
}

/**
* Manual scheduler for the probe's timer seam: nothing fires on its own. A test
* waits for the service to arm a timer with a given delay (event-ordered — the
* promise settles when the code under test reaches that point), then fires it.
*/
const manualTimers = (): Readonly<{
readonly fire: (delayMs: number) => Promise<void>;
readonly pending: () => readonly number[];
readonly timers: McpProbeTimers;
}> => {
const armed = new Set<ManualTimer>();
const waiters = new Set<() => void>();
const find = (delayMs: number): ManualTimer | undefined =>
[...armed].find((timer) => timer.delayMs === delayMs);
return Object.freeze({
fire: async (delayMs) => {
let timer = find(delayMs);
while (timer === undefined) {
await new Promise<void>((resolvePromise) => { waiters.add(resolvePromise); });
timer = find(delayMs);
}
armed.delete(timer);
timer.callback();
},
pending: () => [...armed].map((timer) => timer.delayMs),
timers: {
schedule: (callback, delayMs) => {
const timer: ManualTimer = { callback, delayMs };
armed.add(timer);
for (const wake of [...waiters]) {
waiters.delete(wake);
wake();
}
return () => {
armed.delete(timer);
};
},
},
});
};

/** Never settles: a teardown that hangs for the rest of the test. */
const stalled = (): Promise<never> => new Promise<never>(() => undefined);

/**
* Whether `promise` has already settled, decided at the next macrotask so every
* microtask chained off the current turn has run first — no wall-clock wait.
*/
const settledBeforeNextTurn = (promise: Promise<unknown>): Promise<boolean> =>
Promise.race([
promise.then(() => true, () => true),
new Promise<boolean>((resolvePromise) => { setImmediate(() => resolvePromise(false)); }),
]);

const createBundle = async (
servers: Readonly<Record<string, unknown>> = {
timeline: {
Expand Down Expand Up @@ -393,80 +455,88 @@ it('returns a timed-out report without awaiting stalled teardown', async () => {
const root = await createBundle();
let clientCloses = 0;
let transportCloses = 0;
let guard: NodeJS.Timeout | undefined;
const timers = manualTimers();
try {
const stalledClose = async (): Promise<void> =>
new Promise((resolvePromise) => setTimeout(resolvePromise, 250));
const service = serviceFor(root, {
// A frozen clock keeps the whole budget for the connect step, so the
// budget timer is armed with exactly `timeoutMs`.
clock: () => 0,
createClient: () => client({
close: async () => {
close: () => {
clientCloses += 1;
await stalledClose();
return stalled();
},
connect: () => new Promise(() => undefined),
connect: () => stalled(),
}),
createStdioTransport: () => transport(async () => {
createStdioTransport: () => transport(() => {
transportCloses += 1;
await stalledClose();
return stalled();
}),
timeoutMs: 10,
timers: timers.timers,
});

const report = await Promise.race([
service.probe({ host: 'claude', serverName: 'timeline' }),
new Promise<never>((_resolve, reject) => {
guard = setTimeout(
() => reject(new Error('The timed-out probe remained blocked on teardown.')),
150,
);
}),
]);
const probe = service.probe({ host: 'claude', serverName: 'timeline' });
// The budget expires while connect is still pending: the timeout starts
// the transport close, and the report path arms the bounded teardown wait.
await timers.fire(10);
// Both closes hang forever; only the teardown wait may release the report.
await timers.fire(mcpProbeTeardownWaitMs);
expect(await settledBeforeNextTurn(probe)).toBe(true);

const report = await probe;
expect(report.status).toBe('timed-out');
expect(clientCloses).toBe(1);
expect(transportCloses).toBeGreaterThan(0);
// The detached plugin-data cap is the only timer still armed: teardown is
// running in the background, not on the response path. Firing it releases
// the plugin-data removal, which `settle()` then fences.
expect(timers.pending()).toEqual([mcpProbePluginDataTeardownCapMs]);
await timers.fire(mcpProbePluginDataTeardownCapMs);
await service.settle();
expect(timers.pending()).toEqual([]);
} finally {
if (guard !== undefined) clearTimeout(guard);
await rm(root, { force: true, recursive: true });
}
});

it('returns a timed-out report without awaiting stalled teardown when the budget is spent before connecting', async () => {
const root = await createBundle();
let transportCloses = 0;
let guard: NodeJS.Timeout | undefined;
let ticks = 0;
const timers = manualTimers();
try {
const service = serviceFor(root, {
// The clock reads 0 at probe start and the whole budget later at every
// subsequent read, so the connect step finds no time remaining.
clock: () => (ticks++ === 0 ? 0 : 10_000),
createClient: () => client({
close: async () => new Promise((resolvePromise) => setTimeout(resolvePromise, 250)),
connect: () => new Promise(() => undefined),
close: () => stalled(),
connect: () => stalled(),
}),
createStdioTransport: () => transport(async () => {
createStdioTransport: () => transport(() => {
transportCloses += 1;
await new Promise((resolvePromise) => setTimeout(resolvePromise, 250));
return stalled();
}),
timeoutMs: 10,
timers: timers.timers,
});

const report = await Promise.race([
service.probe({ host: 'claude', serverName: 'timeline' }),
new Promise<never>((_resolve, reject) => {
guard = setTimeout(
() => reject(new Error('The budget-exhausted probe remained blocked on teardown.')),
150,
);
}),
]);
const probe = service.probe({ host: 'claude', serverName: 'timeline' });
// No budget timer is armed on this path; the report waits only for the
// bounded teardown wait, never for the stalled closes.
await timers.fire(mcpProbeTeardownWaitMs);
expect(await settledBeforeNextTurn(probe)).toBe(true);

const report = await probe;
expect(report.status).toBe('timed-out');
expect(report.failure?.kind).toBe('connect');
expect(transportCloses).toBeGreaterThan(0);
expect(timers.pending()).toEqual([mcpProbePluginDataTeardownCapMs]);
await timers.fire(mcpProbePluginDataTeardownCapMs);
await service.settle();
expect(timers.pending()).toEqual([]);
} finally {
if (guard !== undefined) clearTimeout(guard);
await rm(root, { force: true, recursive: true });
}
});
Expand Down
Loading
Loading