From 32b93501d7a35a3b405b5022b4caaad492eed11c Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 7 Jul 2026 17:52:37 +0200 Subject: [PATCH] fix: Use SDK guards for elicitation validation (#850) --- package-lock.json | 8 ++++---- package.json | 2 +- src/elicitation.ts | 38 ++++++------------------------------- src/tests/acp-agent.test.ts | 11 ++++------- 4 files changed, 15 insertions(+), 44 deletions(-) diff --git a/package-lock.json b/package-lock.json index 514f47db..e881db9a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,7 @@ "version": "0.57.0", "license": "Apache-2.0", "dependencies": { - "@agentclientprotocol/sdk": "1.2.0", + "@agentclientprotocol/sdk": "1.2.1", "@anthropic-ai/claude-agent-sdk": "0.3.202", "zod": "^3.25.0 || ^4.0.0" }, @@ -36,9 +36,9 @@ } }, "node_modules/@agentclientprotocol/sdk": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-1.2.0.tgz", - "integrity": "sha512-rJ3766M2eouZsNCaK21JbPD++JLBZ0PWq38dzj1sA4/i0njaQEkiC2F5v7MlizFW6akX+qu5Ks8R00Xw+YMHKA==", + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-1.2.1.tgz", + "integrity": "sha512-jwYUdOQR7tc+Zfch53VL4JJyUNK/46q03uUTYb+PjECsmnNl94XFXOfYLJ8RBpMNidXd1rpOAVgb0vqD98xImA==", "license": "Apache-2.0", "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" diff --git a/package.json b/package.json index 4209c866..0e817d1c 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,7 @@ "node": ">=22" }, "dependencies": { - "@agentclientprotocol/sdk": "1.2.0", + "@agentclientprotocol/sdk": "1.2.1", "@anthropic-ai/claude-agent-sdk": "0.3.202", "zod": "^3.25.0 || ^4.0.0" }, diff --git a/src/elicitation.ts b/src/elicitation.ts index 4174c2cc..abda4672 100644 --- a/src/elicitation.ts +++ b/src/elicitation.ts @@ -1,9 +1,8 @@ import { randomUUID } from "node:crypto"; +import { CreateElicitationResponse } from "@agentclientprotocol/sdk"; import type { CreateElicitationRequest, - CreateElicitationResponse, ElicitationAcceptAction, - ElicitationContentValue, ElicitationPropertySchema, ElicitationSchema, EnumOption, @@ -70,43 +69,18 @@ export function mcpElicitationToCreateRequest( }; } -/** A wire value allowed in elicitation form content. */ -function isElicitationContentValue(value: unknown): value is ElicitationContentValue { - return ( - typeof value === "string" || - typeof value === "number" || - typeof value === "boolean" || - (Array.isArray(value) && value.every((item) => typeof item === "string")) - ); -} - /** * Content of an accepted elicitation response. * - * The response union includes a custom/future-action variant whose `action` is - * plain `string`, so a positive `=== "accept"` check can never narrow it away - * and `content` reads as `unknown`. Rather than asserting which union member - * this is, validate the values: the SDK's own wire validation has the same - * blind spot (an accept response with malformed content parses via the - * catch-all variant), so ill-typed values are dropped here either way. + * Uses the SDK's validating guard rather than an `action === "accept"` check: + * the guard both narrows past the union's custom/future variant and validates + * the payload, so a malformed accept (right tag, ill-typed content) yields + * empty content — the same classification the SDK's wire validators apply. */ function acceptedElicitationContent( response: CreateElicitationResponse, ): NonNullable { - if (response.action !== "accept") { - return {}; - } - const { content } = response; - if (!content || typeof content !== "object") { - return {}; - } - const validated: NonNullable = {}; - for (const [key, value] of Object.entries(content)) { - if (isElicitationContentValue(value)) { - validated[key] = value; - } - } - return validated; + return CreateElicitationResponse.isAccept(response) ? (response.content ?? {}) : {}; } /** diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 04fc47cc..fee005ad 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -215,9 +215,7 @@ describe.skipIf(!process.env.RUN_INTEGRATION_TESTS)("ACP subprocess integration" params: CreateElicitationRequest, ): Promise { this.elicitations.push(params); - // The `in` check also excludes the custom/future-mode variant, whose - // `mode: string` would otherwise survive the literal comparison. - if (params.mode !== "form" || !("requestedSchema" in params)) { + if (!CreateElicitationRequest.isForm(params)) { return { action: "decline" }; } // Accept the first option of every choice field (skip the free-text one). @@ -448,10 +446,9 @@ describe.skipIf(!process.env.RUN_INTEGRATION_TESTS)("ACP subprocess integration" // ...built by our converter (indexed field key + free-text "Other" field), // which confirms our interception path produced it rather than some other // mechanism. - const properties = - elicitation.mode === "form" && "requestedSchema" in elicitation - ? Object.keys(elicitation.requestedSchema.properties ?? {}) - : []; + const properties = CreateElicitationRequest.isForm(elicitation) + ? Object.keys(elicitation.requestedSchema.properties ?? {}) + : []; expect(properties).toContain("question_0"); expect(properties).toContain("question_0_custom");