From a2795e14c64c5b71226a62c42ab54301805c2438 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 3 May 2026 14:57:41 +0000 Subject: [PATCH 1/2] test(training-agent): add handler-dispatch smoke test per tool per tenant (#3978) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `server/tests/integration/training-agent-tool-dispatch-smoke.test.ts` which, for every (tenant, tool) pair in the tool catalog, calls the tool via MCP with minimal arguments and asserts the response is not NOT_IMPLEMENTED or UNSUPPORTED_FEATURE. Domain errors (INVALID_REQUEST, MEDIA_BUY_NOT_FOUND, …) count as passing — those confirm the handler ran. Extends CI façade detection beyond the name-match check in training-agent-tool-catalog-drift.test.ts. The gap surfaced by #3962/#3976: list_creative_formats was advertised on /creative and /creative-builder but the v6 platform method was missing. Catalog drift test passed; this smoke test would have caught it at CI time. https://claude.ai/code/session_012j7VQUJUxwvpbNQ1rbo2qM --- .changeset/tool-dispatch-smoke-test.md | 29 ++++ ...training-agent-tool-dispatch-smoke.test.ts | 142 ++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 .changeset/tool-dispatch-smoke-test.md create mode 100644 server/tests/integration/training-agent-tool-dispatch-smoke.test.ts diff --git a/.changeset/tool-dispatch-smoke-test.md b/.changeset/tool-dispatch-smoke-test.md new file mode 100644 index 0000000000..d17b6f7006 --- /dev/null +++ b/.changeset/tool-dispatch-smoke-test.md @@ -0,0 +1,29 @@ +--- +--- + +test(training-agent): add handler-dispatch smoke test per tool per tenant — closes #3978 + +Extends CI façade detection beyond name-match to dispatch coverage. +`tool-catalog-drift.test.ts` proves tool names match the catalog; this new test +proves each handler is wired: for every (tenant, tool) pair a `tools/call` with +minimal arguments must not return `NOT_IMPLEMENTED` or `UNSUPPORTED_FEATURE`. +Domain errors (`INVALID_REQUEST`, `MEDIA_BUY_NOT_FOUND`, …) are expected passes — +those confirm the handler ran. + +The gap surfaced by #3962/#3976: `list_creative_formats` was advertised on +`/creative` and `/creative-builder` but the v6 platform method was missing. +Catalog drift test passed (names matched); the storyboard caught it only after a +buyer-side request was made. This test would have caught it at CI time. + +**Files:** +- `server/tests/integration/training-agent-tool-dispatch-smoke.test.ts` (new) + +**Implementation notes:** +- Mirrors the drift test's server-boot and MCP-over-HTTP call pattern. +- Uses `{}` as arguments for read-only tools; adds `idempotency_key` for mutating + tools so the framework routes to the handler body. +- Does not attempt response-schema validation — `responseSchema` is not exposed on + the advertised tool object (`tools/list` returns `inputSchema` only). +- Two assertion forms: `structuredContent.adcp_error.code` for AdcpError-wrapped + NOT_IMPLEMENTED, plus a text-content regex check for SDK-level UNSUPPORTED_FEATURE + strings that don't follow the adcp_error envelope. diff --git a/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts b/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts new file mode 100644 index 0000000000..33e1094ed8 --- /dev/null +++ b/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts @@ -0,0 +1,142 @@ +/** + * Dispatch smoke test for the training agent. + * + * `tool-catalog-drift.test.ts` proves the names match. This test proves + * the dispatch path is wired: for every (tenant, tool) pair in the catalog, + * a `tools/call` with minimal arguments must NOT return NOT_IMPLEMENTED or + * UNSUPPORTED_FEATURE. Domain errors (MEDIA_BUY_NOT_FOUND, INVALID_REQUEST, + * etc.) count as passing — the handler ran. + * + * Façade class caught: a platform that advertises every tool in its catalog + * (passes drift test) but has one or more handlers missing or stubbed-out. + * This class escaped CI in #3962 (list_creative_formats on /creative + + * /creative-builder was advertised but unimplemented until #3976). + */ + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import express from 'express'; +import http from 'node:http'; +import { AddressInfo } from 'node:net'; +import crypto from 'node:crypto'; + +vi.hoisted(() => { + process.env.PUBLIC_TEST_AGENT_TOKEN = 'tool-dispatch-smoke-token'; + process.env.NODE_ENV = 'test'; +}); + +vi.mock('../../src/logger.js', () => ({ + createLogger: () => ({ + info: vi.fn(), debug: vi.fn(), warn: vi.fn(), error: vi.fn(), + }), +})); + +const { createTrainingAgentRouter } = await import('../../src/training-agent/index.js'); +const { stopSessionCleanup } = await import('../../src/training-agent/state.js'); +const { toolsForTenant } = await import('../../src/training-agent/tenants/tool-catalog.js'); +const { MUTATING_TOOLS } = await import('../../src/training-agent/idempotency.js'); + +const TENANT_IDS = ['signals', 'sales', 'governance', 'creative', 'creative-builder', 'brand'] as const; +type TenantId = typeof TENANT_IDS[number]; +const AUTH = 'Bearer tool-dispatch-smoke-token'; + +interface MCPResponse { + error?: unknown; + result?: { + content?: Array<{ type: string; text?: string }>; + structuredContent?: Record; + isError?: boolean; + }; +} + +async function callTool(baseUrl: string, tenantId: string, toolName: string): Promise { + const url = `${baseUrl}/api/training-agent/${tenantId}/mcp`; + const headers = { + 'content-type': 'application/json', + accept: 'application/json, text/event-stream', + authorization: AUTH, + }; + // Initialize handshake — required before tools/call. Verify it succeeds so + // a server-boot or auth failure surfaces here rather than as a phantom pass. + const initRes = await fetch(url, { + method: 'POST', + headers, + body: JSON.stringify({ + jsonrpc: '2.0', id: 1, method: 'initialize', + params: { protocolVersion: '2025-03-26', clientInfo: { name: 'smoke', version: '1' }, capabilities: {} }, + }), + }); + const initBody = await initRes.json() as MCPResponse; + if (initBody.error) { + throw new Error(`initialize failed for ${tenantId}: ${JSON.stringify(initBody.error)}`); + } + // Minimal arguments: mutating tools need an idempotency_key so the + // framework routes to the handler (missing key → framework validation + // error before dispatch, which would also not be NOT_IMPLEMENTED, but + // an idempotency_key lets us reach the handler body cleanly). + const args: Record = MUTATING_TOOLS.has(toolName) + ? { idempotency_key: `smoke-${crypto.randomUUID()}` } + : {}; + const res = await fetch(url, { + method: 'POST', + headers, + body: JSON.stringify({ + jsonrpc: '2.0', id: 2, method: 'tools/call', + params: { name: toolName, arguments: args }, + }), + }); + return res.json() as Promise; +} + +// Build the full test matrix once from the catalog. +const TEST_CASES: Array<[TenantId, string]> = TENANT_IDS.flatMap( + tenantId => toolsForTenant(tenantId).map(tool => [tenantId, tool] as [TenantId, string]), +); + +describe('tool dispatch smoke', () => { + let server: http.Server; + let baseUrl: string; + + beforeAll(async () => { + const app = express(); + app.use(express.json()); + app.use('/api/training-agent', createTrainingAgentRouter()); + server = http.createServer(app); + await new Promise(resolve => server.listen(0, '127.0.0.1', () => resolve())); + const port = (server.address() as AddressInfo).port; + baseUrl = `http://127.0.0.1:${port}`; + }); + + afterAll(async () => { + stopSessionCleanup(); + await new Promise(resolve => server.close(() => resolve())); + }); + + it.each(TEST_CASES)('%s / %s handler is wired (no NOT_IMPLEMENTED)', async (tenantId, toolName) => { + const body = await callTool(baseUrl, tenantId, toolName); + + const structured = body.result?.structuredContent; + const errorCode = (structured?.adcp_error as Record | undefined)?.code; + + // Domain errors (INVALID_REQUEST, MEDIA_BUY_NOT_FOUND, …) are expected + // and count as passing — those mean the handler ran. Only NOT_IMPLEMENTED + // and UNSUPPORTED_FEATURE indicate the dispatch path is unwired. + expect( + errorCode, + `${tenantId}/${toolName}: handler returned NOT_IMPLEMENTED — add the method to the v6 platform class`, + ).not.toBe('NOT_IMPLEMENTED'); + + expect( + errorCode, + `${tenantId}/${toolName}: handler returned UNSUPPORTED_FEATURE — wire the method in the platform class`, + ).not.toBe('UNSUPPORTED_FEATURE'); + + // SDK-level UNSUPPORTED_FEATURE surfaces as a text string, not an + // adcp_error envelope. Check the raw content text as a belt-and-suspenders + // guard so the test catches both the AdcpError and SDK-wrapper forms. + const textContent = body.result?.content?.[0]?.text ?? ''; + expect( + textContent, + `${tenantId}/${toolName}: SDK returned UNSUPPORTED_FEATURE — method missing from platform class`, + ).not.toMatch(/UNSUPPORTED_FEATURE/); + }, 15000); +}); From ce382353acecdc1248d50d2540218f9e67b3f0bb Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 11 May 2026 10:25:47 -0400 Subject: [PATCH 2/2] test(smoke): allowlist creative-builder list_creatives/get_creative_delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK auto-advertises these on /creative-builder via specialism membership, but CreativeBuilderPlatform has no method slots for them — they belong on CreativeAdServerPlatform. Calls return UNSUPPORTED_FEATURE today. This is an SDK bug (adcp-client#1688). Once the SDK narrows tool advertisement to interface method presence, remove this allowlist entry — the test will catch any future regression naturally. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...training-agent-tool-dispatch-smoke.test.ts | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts b/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts index 33e1094ed8..23e20e6f59 100644 --- a/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts +++ b/server/tests/integration/training-agent-tool-dispatch-smoke.test.ts @@ -111,11 +111,24 @@ describe('tool dispatch smoke', () => { await new Promise(resolve => server.close(() => resolve())); }); + // SDK bug allowlist — adcontextprotocol/adcp-client#1688. + // The SDK auto-advertises these tools on /creative-builder via specialism + // membership, but CreativeBuilderPlatform (stateless build/transform) has + // no method slots for them — they live on CreativeAdServerPlatform. + // Calls correctly return UNSUPPORTED_FEATURE today. Remove this allowlist + // entry once the SDK narrows tool advertisement to interface method + // presence; the test will then catch any future regression naturally. + const SDK_AUTO_ADVERTISED_UNIMPLEMENTED = new Set([ + 'creative-builder/list_creatives', + 'creative-builder/get_creative_delivery', + ]); + it.each(TEST_CASES)('%s / %s handler is wired (no NOT_IMPLEMENTED)', async (tenantId, toolName) => { const body = await callTool(baseUrl, tenantId, toolName); const structured = body.result?.structuredContent; const errorCode = (structured?.adcp_error as Record | undefined)?.code; + const knownSdkBug = SDK_AUTO_ADVERTISED_UNIMPLEMENTED.has(`${tenantId}/${toolName}`); // Domain errors (INVALID_REQUEST, MEDIA_BUY_NOT_FOUND, …) are expected // and count as passing — those mean the handler ran. Only NOT_IMPLEMENTED @@ -125,18 +138,20 @@ describe('tool dispatch smoke', () => { `${tenantId}/${toolName}: handler returned NOT_IMPLEMENTED — add the method to the v6 platform class`, ).not.toBe('NOT_IMPLEMENTED'); - expect( - errorCode, - `${tenantId}/${toolName}: handler returned UNSUPPORTED_FEATURE — wire the method in the platform class`, - ).not.toBe('UNSUPPORTED_FEATURE'); - - // SDK-level UNSUPPORTED_FEATURE surfaces as a text string, not an - // adcp_error envelope. Check the raw content text as a belt-and-suspenders - // guard so the test catches both the AdcpError and SDK-wrapper forms. - const textContent = body.result?.content?.[0]?.text ?? ''; - expect( - textContent, - `${tenantId}/${toolName}: SDK returned UNSUPPORTED_FEATURE — method missing from platform class`, - ).not.toMatch(/UNSUPPORTED_FEATURE/); + if (!knownSdkBug) { + expect( + errorCode, + `${tenantId}/${toolName}: handler returned UNSUPPORTED_FEATURE — wire the method in the platform class`, + ).not.toBe('UNSUPPORTED_FEATURE'); + + // SDK-level UNSUPPORTED_FEATURE surfaces as a text string, not an + // adcp_error envelope. Check the raw content text as a belt-and-suspenders + // guard so the test catches both the AdcpError and SDK-wrapper forms. + const textContent = body.result?.content?.[0]?.text ?? ''; + expect( + textContent, + `${tenantId}/${toolName}: SDK returned UNSUPPORTED_FEATURE — method missing from platform class`, + ).not.toMatch(/UNSUPPORTED_FEATURE/); + } }, 15000); });