diff --git a/.changeset/cross-app-agent-target-resolution.md b/.changeset/cross-app-agent-target-resolution.md new file mode 100644 index 00000000000..776e10e8705 --- /dev/null +++ b/.changeset/cross-app-agent-target-resolution.md @@ -0,0 +1,15 @@ +--- +"@agent-native/core": patch +--- + +Fix cross-app delegation reporting an unresolvable target as remote downtime. +`findAgent` matched a handle exactly, so `agent="plans"` missed the `plan` app +even though the Plan app labels itself "Plans" in its own sidebar, nav state, +and skills — twelve of thirteen first-party apps had the same latent miss in one +grammatical number or the other. `findAgent` now also resolves the singular or +plural variant, and refuses to guess when two agents differ only by a trailing +"s". When a target still cannot be resolved, `call-agent` now throws a typed +`agent_not_found` failure, logs it, and emits `$a2a_invocation` telemetry +instead of returning an `Error: ...` string that the agent loop scored as a +successful tool call and the model retold as "The Plans app is temporarily +unavailable." diff --git a/packages/core/src/scripts/call-agent.spec.ts b/packages/core/src/scripts/call-agent.spec.ts index fb0c232c796..10952e2c7f0 100644 --- a/packages/core/src/scripts/call-agent.spec.ts +++ b/packages/core/src/scripts/call-agent.spec.ts @@ -800,6 +800,114 @@ describe("call-agent action", () => { } }); + it("fails loudly when the delegation target cannot be resolved", async () => { + const discovery = await import("../server/agent-discovery.js"); + vi.mocked(discovery.findAgent).mockResolvedValueOnce(undefined); + vi.mocked(discovery.discoverAgents).mockResolvedValueOnce([ + { id: "plan", name: "Plan", description: "", url: "", color: "" }, + ]); + const logged: string[] = []; + const consoleError = vi + .spyOn(console, "error") + .mockImplementation((...args: unknown[]) => { + logged.push(args.join(" ")); + }); + const tracked: TrackingEvent[] = []; + registerTrackingProvider({ + name: "qa-a2a-not-found", + track(event) { + tracked.push(event); + }, + }); + + try { + const { run } = await import("./call-agent.js"); + // A returned "Error: ..." string is scored as a SUCCESSFUL tool call, + // which is what let the model retell an unresolved target as downtime. + const outcome = await run( + { agent: "nosuchapp", message: "Create the rollout plan" }, + { send: vi.fn(), threadId: "t", runId: "r", turnId: "u" } as any, + "brain", + ).then( + (resolved) => ({ resolved }) as const, + (error) => ({ error }) as const, + ); + + expect("error" in outcome).toBe(true); + const error = (outcome as { error: any }).error; + expect(error.name).toBe("A2AInvocationError"); + expect(error.errorCode).toBe("agent_not_found"); + expect(error.message).toContain("nosuchapp"); + expect(error.message).toContain("plan"); + // The reported production failure: the model narrated a resolution + // failure as "The Plans app is temporarily unavailable." + expect(error.message).toMatch(/not an outage/i); + + expect( + logged.some((line) => line.includes("Unresolvable delegation target")), + ).toBe(true); + expect( + tracked.find((event) => event.name === "$a2a_invocation")?.properties, + ).toMatchObject({ + caller_app: "brain", + target_app: "nosuchapp", + status: "error", + terminal_code: "agent_not_found", + mode: "message", + }); + } finally { + unregisterTrackingProvider("qa-a2a-not-found"); + consoleError.mockRestore(); + } + }); + + it.each([ + { + label: "direct action", + args: { action: "gong-calls" }, + mode: "direct_action", + }, + { label: "task poll", args: { taskId: "task-1" }, mode: "task_poll" }, + ])( + "reports the caller's own mode when a $label target cannot be resolved", + async ({ args, mode }) => { + // Target resolution runs before the action/taskId dispatch, so this + // branch is reachable in every mode and must not label them all + // "message" — that would misattribute the failure in $a2a_invocation. + const discovery = await import("../server/agent-discovery.js"); + vi.mocked(discovery.findAgent).mockResolvedValueOnce(undefined); + vi.mocked(discovery.discoverAgents).mockResolvedValueOnce([]); + const consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + const tracked: TrackingEvent[] = []; + registerTrackingProvider({ + name: "qa-a2a-not-found-mode", + track(event) { + tracked.push(event); + }, + }); + + try { + const { run } = await import("./call-agent.js"); + await expect( + run( + { agent: "nosuchapp", ...args }, + { send: vi.fn() } as any, + "brain", + ), + ).rejects.toMatchObject({ errorCode: "agent_not_found" }); + + expect( + tracked.find((event) => event.name === "$a2a_invocation")?.properties, + ).toMatchObject({ mode, terminal_code: "agent_not_found" }); + } finally { + unregisterTrackingProvider("qa-a2a-not-found-mode"); + consoleError.mockRestore(); + } + }, + ); + it("does not report an empty delegated response as success", async () => { callAgentMock.mockResolvedValueOnce(""); const { run } = await import("./call-agent.js"); diff --git a/packages/core/src/scripts/call-agent.ts b/packages/core/src/scripts/call-agent.ts index f9b1abb869a..33b1507c0fd 100644 --- a/packages/core/src/scripts/call-agent.ts +++ b/packages/core/src/scripts/call-agent.ts @@ -59,12 +59,13 @@ const INTEGRATION_A2A_TOKEN_TTL = "30m"; const A2A_INVOCATION_EVENT = "$a2a_invocation"; type A2AInvocationStatus = "success" | "pending" | "error"; +type A2AInvocationMode = "message" | "task_poll" | "direct_action"; function trackA2AInvocation(args: { invocationId: string; callerApp?: string; targetApp: string; - mode: "message" | "task_poll" | "direct_action"; + mode: A2AInvocationMode; status: A2AInvocationStatus; startedAt: number; taskId?: string; @@ -183,6 +184,51 @@ class A2AInvocationError extends Error { } } +/** + * An unresolvable delegation target used to leave this tool as a plain + * `Error: ...` string. The agent loop scores a returned string as a SUCCESSFUL + * tool call — no `isError`, no loop-breaker entry — and because the return + * happened before the tracked call began, no `$a2a_invocation` row was written + * either, so the single most common cross-app failure was invisible in logs and + * telemetry at once. Handed a "successful" result that merely contains prose, + * the model is free to retell it: in production it retold a target it could not + * resolve as "The Plans app is temporarily unavailable", inventing an outage + * that never happened. Resolution failure is a caller-side naming or + * registration fault and never remote downtime, so say so in the message the + * model receives. + */ +function unresolvableAgentTargetError( + requestedAgent: string, + available: Array<{ id: string; name: string }>, + callerApp: string | undefined, + correlation: A2ACorrelationMetadata, + mode: A2AInvocationMode, +): A2AInvocationError { + const connected = available.map((a) => a.id).join(", "); + console.error( + `[call-agent] Unresolvable delegation target "${requestedAgent}" from ${ + callerApp || "unknown" + }. Connected agents: ${connected || "(none)"}`, + ); + trackA2AInvocation({ + invocationId: randomUUID(), + callerApp, + targetApp: normalizeAppHandle(requestedAgent) || "unknown", + mode, + status: "error", + startedAt: Date.now(), + terminalCode: "agent_not_found", + correlation, + }); + return new A2AInvocationError( + `No connected agent matches "${requestedAgent}". This is a target-resolution failure, ` + + "not an outage: do not describe the app as unavailable, down, or temporarily broken. " + + `Connected agents: ${connected || "(none)"}. ` + + `Retry with one of those exact ids, or tell the user that "${requestedAgent}" is not connected to this workspace.`, + { errorCode: "agent_not_found" }, + ); +} + function buildMessageIdempotencyKey( originatingTurnId: string | undefined, target: string, @@ -691,10 +737,15 @@ export async function run( const agent = await findAgent(agentIdOrName, selfAppId); if (!agent) { - const available = (await discoverAgents(selfAppId)) - .map((a) => a.name) - .join(", "); - return `Error: Agent "${agentIdOrName}" not found. Available agents: ${available || "(none)"}`; + // Target resolution runs ahead of the action/taskId dispatch below, so all + // three modes reach this branch and must report their own. + throw unresolvableAgentTargetError( + agentIdOrName, + await discoverAgents(selfAppId), + selfAppId, + buildDelegationCorrelation(context, selfAppId), + action ? "direct_action" : taskId ? "task_poll" : "message", + ); } if (!taskId) { diff --git a/packages/core/src/server/agent-discovery.spec.ts b/packages/core/src/server/agent-discovery.spec.ts index 814560a5a14..22ef8125eda 100644 --- a/packages/core/src/server/agent-discovery.spec.ts +++ b/packages/core/src/server/agent-discovery.spec.ts @@ -2,9 +2,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { TEMPLATES } from "../cli/templates-meta.js"; import { + agentHandleNumberVariant, BUILTIN_AGENTS_FOR_SEEDING, discoverAgents, discoverOrgDirectoryAgents, + findAgent, findWorkspaceDispatchAgent, getBuiltinAgents, normalizeAgentId, @@ -1037,6 +1039,70 @@ describe("agent discovery", () => { await expect(pending).resolves.toMatchObject({ status: "available" }); }); + + describe("singular/plural handle resolution", () => { + it("resolves the Plan app when a caller asks for 'plans'", async () => { + // The Plan app labels itself "Plans" in its own sidebar, nav state, and + // skills, so the model naturally delegates to agent="plans". + await expect(findAgent("plans", "brain")).resolves.toMatchObject({ + id: "plan", + }); + }); + + it("resolves every built-in agent from its other grammatical number", async () => { + const unresolved: string[] = []; + for (const agent of getBuiltinAgents()) { + const variant = agentHandleNumberVariant(agent.id); + if (!variant) continue; + if ((await findAgent(variant))?.id !== agent.id) { + unresolved.push(`${variant} -> ${agent.id}`); + } + } + expect(unresolved).toEqual([]); + }); + + it("leaves a genuinely unknown handle unresolved", async () => { + await expect(findAgent("nosuchapp", "brain")).resolves.toBeUndefined(); + }); + + it("refuses to guess when two agents differ only by a trailing s", async () => { + resourceListMock.mockResolvedValue([ + { id: "r-report", path: "remote-agents/report.json" }, + { id: "r-reports", path: "remote-agents/reports.json" }, + ]); + resourceGetMock.mockImplementation(async (id: string) => + id === "r-report" + ? { + content: JSON.stringify({ + id: "report", + name: "Report", + url: "https://report.example.com", + }), + } + : { + content: JSON.stringify({ + id: "reports", + name: "Reports", + url: "https://reports.example.com", + }), + }, + ); + + // "report" matches exactly; only the ambiguous variant lookup is refused. + await expect(findAgent("report")).resolves.toMatchObject({ + id: "report", + }); + await expect(findAgent("reportss")).resolves.toBeUndefined(); + }); + + it("produces no variant for handles where the swap is meaningless", () => { + expect(agentHandleNumberVariant("")).toBeNull(); + expect(agentHandleNumberVariant(" ")).toBeNull(); + expect(agentHandleNumberVariant("access")).toBeNull(); + expect(agentHandleNumberVariant("plan")).toBe("plans"); + expect(agentHandleNumberVariant("Forms")).toBe("form"); + }); + }); }); function restoreEnv(name: string, value: string | undefined): void { diff --git a/packages/core/src/server/agent-discovery.ts b/packages/core/src/server/agent-discovery.ts index f67391b6310..a5300bd6c48 100644 --- a/packages/core/src/server/agent-discovery.ts +++ b/packages/core/src/server/agent-discovery.ts @@ -648,6 +648,20 @@ function isAbsoluteHttpUrl(value: string): boolean { } } +/** + * First-party app handles are singular or plural by historical accident + * ("plan", but "forms"), and an app's own UI rarely agrees with its handle — + * the Plan app labels its sidebar, nav state, and skills "Plans". A caller that + * writes the other number is naming a real, reachable app, so resolve the + * variant instead of reporting the app missing. Returns null where the swap is + * meaningless so an empty or `-ss` handle cannot manufacture a candidate. + */ +export function agentHandleNumberVariant(handle: string): string | null { + const value = handle.trim().toLowerCase(); + if (!value || value.endsWith("ss")) return null; + return value.endsWith("s") ? value.slice(0, -1) : `${value}s`; +} + /** * Look up a single agent by ID or name (case-insensitive). */ @@ -657,7 +671,20 @@ export async function findAgent( ): Promise { const lower = normalizeAgentId(idOrName); const agents = await discoverAgents(selfAppId); - return agents.find((a) => a.id === lower || a.name.toLowerCase() === lower); + const exact = agents.find( + (a) => a.id === lower || a.name.toLowerCase() === lower, + ); + if (exact) return exact; + + const variant = agentHandleNumberVariant(lower); + if (!variant) return undefined; + const handles = new Set([variant, normalizeAgentId(variant)]); + const near = agents.filter( + (a) => handles.has(a.id) || handles.has(a.name.toLowerCase()), + ); + // Two apps whose handles differ only by a trailing "s" must fail loudly + // rather than have one of them silently chosen for the caller. + return near.length === 1 ? near[0] : undefined; } function hostnameFromUrlLike(value: string | undefined): string | null {