diff --git a/src/lib/a2a/taskExecution.ts b/src/lib/a2a/taskExecution.ts index 68c8ff0937..ff154f7efa 100644 --- a/src/lib/a2a/taskExecution.ts +++ b/src/lib/a2a/taskExecution.ts @@ -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>; 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 | 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((_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); } } diff --git a/tests/unit/a2a-memory-hits.test.ts b/tests/unit/a2a-memory-hits.test.ts index 9d6a42486f..3e69b76446 100644 --- a/tests/unit/a2a-memory-hits.test.ts +++ b/tests/unit/a2a-memory-hits.test.ts @@ -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); +});