fix(a2a): bound the memory recall with its own deadline

collectMemoryHits() runs BEFORE the skill handler and had no deadline at all,
so a slow memory backend delayed the start of every A2A task — the HTTP
genericBackend alone defaults to a 30s timeout.

The search now races a MEMORY_RECALL_TIMEOUT_MS (1500ms) deadline. Overshooting
degrades exactly like any other recall failure: empty hits, a warn log, and the
task proceeds normally (best-effort contract unchanged, nothing propagates).
The deadline timer is cleared in a finally on BOTH paths so no handle is left
holding the event loop open, and MemoryHitsDeps.timeoutMs makes it injectable
so the tests cost milliseconds instead of 1.5s of wall clock.

Refs #12639
This commit is contained in:
diegosouzapw
2026-09-07 16:14:24 -03:00
parent da8014442d
commit 3ef0aef15b
2 changed files with 120 additions and 2 deletions

View File

@@ -1,6 +1,9 @@
import type { A2ATask, TaskArtifact } from "./taskManager";
import { appendA2ATaskEvent } from "@/lib/db/a2aTasks";
import { memoryManager } from "@/lib/memory/manager";
import { logger } from "@omniroute/open-sse/utils/logger";
const log = logger("A2A_TASKS");
type TaskManagerLike = {
updateTask: (
@@ -41,8 +44,22 @@ export interface MemoryHitsDeps {
limit?: number;
}) => Promise<Array<{ id: string; key: string; type: string; content: string }>>;
appendEvent?: (taskId: string, eventType: string, dataJson?: string) => void;
/** Recall deadline override — tests inject a few ms instead of waiting {@link MEMORY_RECALL_TIMEOUT_MS}. */
timeoutMs?: number;
}
/**
* Task C2 (Orchestration Canvas Fase 3, PR-C): deadline for the observability-only memory
* recall. The recall runs BEFORE the skill handler, so an unbounded one delays the task
* itself — the HTTP memory backend (`genericBackend`) alone defaults to a 30s timeout.
* Overshooting the deadline degrades exactly like any other recall failure: empty hits,
* task proceeds.
*/
export const MEMORY_RECALL_TIMEOUT_MS = 1500;
/** Internal marker so the catch below can tell a deadline apart from a backend error. */
class MemoryRecallTimeoutError extends Error {}
/**
* Collect the memories consulted for a task's last user message, as pure observability.
*
@@ -82,21 +99,37 @@ export async function collectMemoryHits(
}
if (!query || query.trim() === "") return [];
const timeoutMs = deps?.timeoutMs ?? MEMORY_RECALL_TIMEOUT_MS;
let timer: ReturnType<typeof setTimeout> | undefined;
try {
const search =
deps?.search ??
(async (cfg: { query: string; apiKeyId: string; limit?: number }) =>
memoryManager.getPrimaryBackend().search(cfg));
const apiKeyId = task.owner ?? "mcp";
const results = await search({ query, apiKeyId, limit: 5 });
const deadline = new Promise<never>((_resolve, reject) => {
timer = setTimeout(
() => reject(new MemoryRecallTimeoutError("memory recall deadline exceeded")),
timeoutMs
);
});
const results = await Promise.race([search({ query, apiKeyId, limit: 5 }), deadline]);
return results.map((m) => ({
id: m.id,
key: m.key,
type: m.type,
snippet: m.content.slice(0, 200),
}));
} catch {
} catch (err) {
if (err instanceof MemoryRecallTimeoutError) {
log.warn(
`Memory recall for task ${task.id} exceeded ${timeoutMs}ms — continuing without hits`
);
}
return [];
} finally {
// Cleared on BOTH paths: a surviving timer holds the event loop open.
if (timer) clearTimeout(timer);
}
}

View File

@@ -13,6 +13,7 @@ import assert from "node:assert/strict";
import {
collectMemoryHits,
executeA2ATaskWithState,
MEMORY_RECALL_TIMEOUT_MS,
type MemoryHit,
type MemoryHitsDeps,
} from "../../src/lib/a2a/taskExecution.ts";
@@ -329,3 +330,87 @@ test("executeA2ATaskWithState never leaks memoryHits into task.input.metadata or
tm.destroy();
}
});
/**
* Task C2 (Orchestration Canvas Fase 3, PR-C): the recall is best-effort, so it must also be
* BOUNDED. Without a deadline a slow memory backend (the HTTP `genericBackend` defaults to a
* 30s timeout) delays the start of every A2A task. The deadline is injectable through
* `MemoryHitsDeps.timeoutMs` so these tests cost milliseconds, not 1.5s of wall clock.
*/
test("MEMORY_RECALL_TIMEOUT_MS is the 1.5s default deadline", () => {
assert.equal(MEMORY_RECALL_TIMEOUT_MS, 1500);
});
test("collectMemoryHits returns [] when search never resolves (deadline hit)", async () => {
const deps: MemoryHitsDeps = {
search: () => new Promise(() => {}), // never settles
timeoutMs: 5,
};
const task = makeTask();
const started = Date.now();
const hits = await collectMemoryHits(task, deps);
assert.deepEqual(hits, []);
assert.ok(Date.now() - started < 1000, "gave up on the injected deadline, not the default");
});
test("collectMemoryHits returns the hits when search resolves inside the deadline", async () => {
const deps: MemoryHitsDeps = {
search: async () => {
await new Promise((resolve) => setTimeout(resolve, 1));
return [{ id: "m1", key: "k1", type: "factual", content: "hello" }];
},
timeoutMs: 1000,
};
const hits = await collectMemoryHits(makeTask(), deps);
assert.deepEqual(hits, [{ id: "m1", key: "k1", type: "factual", snippet: "hello" }]);
});
test("collectMemoryHits leaves no pending timer behind on either path", async () => {
const countTimers = () => process.getActiveResourcesInfo().filter((r) => r === "Timeout").length;
const before = countTimers();
// Success path with a long deadline: the timer must be cleared, not left ticking.
await collectMemoryHits(makeTask(), {
search: async () => [{ id: "m1", key: "k1", type: "factual", content: "hello" }],
timeoutMs: 60_000,
});
assert.equal(countTimers(), before, "success path cleared its deadline timer");
// Timeout path: the timer has fired, so nothing may stay registered either.
await collectMemoryHits(makeTask(), {
search: () => new Promise(() => {}),
timeoutMs: 5,
});
assert.equal(countTimers(), before, "timeout path left no timer registered");
});
test("executeA2ATaskWithState completes the task normally when memory recall times out", async () => {
const deps: MemoryHitsDeps = {
search: () => new Promise(() => {}),
timeoutMs: 5,
};
let completedState: string | undefined;
const tm = {
updateTask: (_taskId: string, state: string) => {
completedState = state;
},
};
const task = makeTask();
const result = await executeA2ATaskWithState(
tm,
task,
async () => ({ artifacts: [{ type: "text", content: "ok" }], metadata: {} }),
deps
);
assert.equal(completedState, "completed");
assert.deepEqual(result.artifacts, [{ type: "text", content: "ok" }]);
assert.equal("memoryHits" in task.metadata, false);
});