From 8cf4c1473236b23a4a43d2cf239eda65b1d0432d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sun, 30 Aug 2026 12:47:22 -0500 Subject: [PATCH] feat(runner): normalize ACP form questions (#12388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The runner uses one provider-neutral question contract for user input. > - ACP providers describe form input with provider-specific JSON Schema values. > - Passing those values through would couple the task page to ACP and could bypass the existing response validator. > - This pull request converts bounded ACP forms to the existing Paperclip question contract and converts validated answers back to ACP content. > - The benefit is one question path that does not change any legacy adapter behavior. ## Linked Issues or Issue Description **Agent or provider** ACP-compatible providers that use form elicitation. **Why this adapter is useful** ACP providers need structured user answers during a turn. Paperclip must present those questions through its provider-neutral contract so the existing task experience and validation rules remain consistent. **How the agent is invoked** A later pull request will connect this internal adapter to the ACPX sidecar. This pull request only implements the conversion boundary. It does not launch ACPX, add a dependency, or enable an adapter. **Additional context** This pull request is stacked on #12387. URL elicitation remains unsupported and returns no form projection. ## What Changed - Convert bounded ACP string, enum, multi-select, Boolean, number, and integer fields to `paperclip.question_set.v1`. - Validate every answer with the existing provider-neutral response parser before conversion. - Convert validated answers back to typed ACP form content. - Bound provider-controlled field and option inventories. - Use stable question identities and define arbitrary property names without prototype mutation. - Keep ACP runtime types and dependencies outside this package-local conversion boundary. ## Verification - Runner TypeScript typecheck — passed. - Runner TypeScript tests — 41 files and 367 Vitest tests passed; 12 Node contract tests passed. - `pnpm -r typecheck` — passed for all applicable workspaces. - `pnpm build` — passed, including runner binary, server, UI, and workspace packages. - Prettier and `git diff --check` — passed. - The diff contains 2 files and does not change `pnpm-lock.yaml`, a workflow, a package dependency, a public export, server selection, or UI behavior. ## Risks The main risk is accepting an ACP form that cannot be represented safely by the Paperclip question contract. Unsupported field types fail closed. Field and option inventories are bounded. The existing question parser validates all text, selection, numeric, and required-field constraints before any response returns to ACP. ## Model Used OpenAI Codex with GPT-5 and repository tool use. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either linked an existing public item or described the issue in this PR - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal task identifier - [x] I have run the affected tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have documented the compatibility and security boundary - [ ] All applicable GitHub Actions are green - [ ] Greptile is 5/5 with every actionable comment resolved - [x] I will address all review findings before requesting merge --- .../drivers/acpx/acp-question-adapter.test.ts | 203 ++++++++++ .../src/drivers/acpx/acp-question-adapter.ts | 378 ++++++++++++++++++ 2 files changed, 581 insertions(+) create mode 100644 packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.test.ts create mode 100644 packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.ts diff --git a/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.test.ts b/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.test.ts new file mode 100644 index 0000000000..e2e6b87ce3 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.test.ts @@ -0,0 +1,203 @@ +import { describe, expect, it } from "vitest"; + +import { normalizeAcpFormElicitation } from "./acp-question-adapter.js"; + +describe("ACP form question adapter", () => { + it("normalizes supported fields and restores typed ACP content", () => { + const normalized = normalizeAcpFormElicitation({ + mode: "form", + message: "Choose deployment settings.", + requestedSchema: { + type: "object", + title: "Deployment", + required: ["name", "region", "features", "confirmed", "replicas"], + properties: { + name: { + type: "string", + title: "Name", + minLength: 2, + maxLength: 20, + }, + region: { + type: "string", + title: "Region", + oneOf: [ + { + const: "us-east-1", + title: "Virginia", + description: "Lowest latency for US East.", + }, + { const: "eu-west-1", title: "Ireland" }, + ], + }, + features: { + type: "array", + title: "Features", + items: { + anyOf: [ + { const: "tracing", title: "Tracing" }, + { const: "backups", title: "Backups" }, + ], + }, + }, + confirmed: { type: "boolean", title: "Confirm" }, + replicas: { + type: "integer", + title: "Replicas", + minimum: 1, + maximum: 10, + }, + }, + }, + }); + + expect(normalized).not.toBeNull(); + const questionSet = normalized!.questionSet; + expect(questionSet.title).toBe("Deployment"); + expect(questionSet.description).toContain("Choose deployment settings."); + expect( + questionSet.questions.map((question) => question.answerMode), + ).toEqual([ + "text", + "single_select", + "multi_select", + "single_select", + "text", + ]); + expect(questionSet.questions[1]?.options?.[0]).toMatchObject({ + label: "Virginia", + description: "Lowest latency for US East.", + }); + expect(questionSet.questions[4]?.textValidation).toMatchObject({ + inputType: "integer", + minimum: 1, + maximum: 10, + }); + + const [name, region, features, confirmed, replicas] = questionSet.questions; + const response = normalized!.accept({ + schema: "paperclip.question_response.v1", + answers: { + [name!.id]: { text: "paperclip" }, + [region!.id]: { + selectedOptionIds: [region!.options![1]!.id], + }, + [features!.id]: { + selectedOptionIds: features!.options!.map((option) => option.id), + }, + [confirmed!.id]: { + selectedOptionIds: [confirmed!.options![0]!.id], + }, + [replicas!.id]: { text: "3" }, + }, + }); + expect(response).toEqual({ + action: "accept", + content: { + name: "paperclip", + region: "eu-west-1", + features: ["tracing", "backups"], + confirmed: true, + replicas: 3, + }, + }); + }); + + it("rejects invalid numeric answers before they reach ACP", () => { + const normalized = normalizeAcpFormElicitation({ + mode: "form", + message: "How many?", + requestedSchema: { + type: "object", + required: ["count"], + properties: { count: { type: "integer", minimum: 1 } }, + }, + })!; + expect(() => + normalized.accept({ + schema: "paperclip.question_response.v1", + answers: { + [normalized.questionSet.questions[0]!.id]: { text: "1.5" }, + }, + }), + ).toThrow(/must be a valid integer/); + }); + + it("does not advertise a question shape for URL elicitation", () => { + expect( + normalizeAcpFormElicitation({ + mode: "url", + message: "Authenticate", + }), + ).toBeNull(); + }); + + it("bounds provider-controlled form and option inventories", () => { + const tooManyProperties = Object.fromEntries( + Array.from({ length: 65 }, (_, index) => [ + `field-${index}`, + { type: "string" }, + ]), + ); + expect(() => + normalizeAcpFormElicitation({ + mode: "form", + requestedSchema: { properties: tooManyProperties }, + }), + ).toThrow(/between 1 and 64/); + + expect(() => + normalizeAcpFormElicitation({ + mode: "form", + requestedSchema: { + properties: { + choice: { + type: "string", + enum: Array.from({ length: 129 }, (_, index) => `v-${index}`), + }, + }, + }, + }), + ).toThrow(/more than 128 options/); + }); + + it("ignores unknown required names before applying field semantics", () => { + const normalized = normalizeAcpFormElicitation({ + mode: "form", + requestedSchema: { + type: "object", + required: [ + ...Array.from({ length: 80 }, (_, index) => `unknown-${index}`), + "known", + ], + properties: { known: { type: "string" } }, + }, + }); + + expect(normalized?.questionSet.questions).toEqual([ + expect.objectContaining({ required: true }), + ]); + }); + + it("preserves property names without allowing prototype mutation", () => { + const properties = JSON.parse( + '{"__proto__":{"type":"string","title":"Value"}}', + ); + const normalized = normalizeAcpFormElicitation({ + mode: "form", + requestedSchema: { + required: ["__proto__"], + properties, + }, + })!; + const question = normalized.questionSet.questions[0]!; + const response = normalized.accept({ + schema: "paperclip.question_response.v1", + answers: { [question.id]: { text: "safe" } }, + }); + + expect(Object.getPrototypeOf(response.content)).toBe(Object.prototype); + expect(Object.hasOwn(response.content, "__proto__")).toBe(true); + expect(JSON.stringify(response.content)).toBe('{"__proto__":"safe"}'); + }); +}); diff --git a/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.ts b/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.ts new file mode 100644 index 0000000000..7250cc1a43 --- /dev/null +++ b/packages/paperclip-runner/src/drivers/acpx/acp-question-adapter.ts @@ -0,0 +1,378 @@ +import { createHash } from "node:crypto"; + +import { + PAPERCLIP_QUESTION_SET_SCHEMA, + parsePaperclipQuestionResponse, + parsePaperclipQuestionSet, + type PaperclipQuestion, + type PaperclipQuestionOption, + type PaperclipQuestionResponse, + type PaperclipQuestionSet, +} from "../../contracts/question-set.js"; + +const MAX_ACP_FORM_FIELDS = 64; +const MAX_ACP_FIELD_OPTIONS = 128; + +export interface AcpFormElicitationRequest { + mode: string; + message?: unknown; + requestedSchema?: unknown; +} + +export type AcpFormContent = Record< + string, + string | number | boolean | string[] +>; + +export interface AcpAcceptElicitationResponse { + action: "accept"; + content: AcpFormContent; +} + +interface AcpFieldBinding { + propertyName: string; + property: Record; + question: PaperclipQuestion; + optionValues: Map; +} + +export interface NormalizedAcpForm { + questionSet: PaperclipQuestionSet; + /** Convert a validated Paperclip response back into typed ACP content. */ + accept(response: unknown): AcpAcceptElicitationResponse; +} + +/** + * ACP remains private to this adapter. Only the normalized question set is + * allowed to cross the Paperclip runtime-request boundary. + */ +export function normalizeAcpFormElicitation( + request: AcpFormElicitationRequest, +): NormalizedAcpForm | null { + const rawRequest = record(request); + if (rawRequest.mode !== "form") return null; + const schema = record(rawRequest.requestedSchema); + const properties = record(schema.properties); + const propertyEntries = Object.entries(properties); + if ( + propertyEntries.length === 0 || + propertyEntries.length > MAX_ACP_FORM_FIELDS + ) { + throw new Error( + `ACP form elicitation must define between 1 and ${MAX_ACP_FORM_FIELDS} supported properties`, + ); + } + const propertyNames = new Set(propertyEntries.map(([name]) => name)); + const required = new Set( + Array.isArray(schema.required) + ? schema.required + .filter( + (value): value is string => + typeof value === "string" && propertyNames.has(value), + ) + : [], + ); + const bindings = propertyEntries.map(([propertyName, value], index) => + normalizeField(propertyName, value, index, required.has(propertyName)), + ); + const title = optionalText(schema.title) ?? "Additional information needed"; + const descriptions = [ + optionalText(rawRequest.message), + optionalText(schema.description), + ] + .filter((value) => value !== title) + .filter( + (value, position, all): value is string => + Boolean(value) && all.indexOf(value) === position, + ); + const questionSet = parsePaperclipQuestionSet({ + schema: PAPERCLIP_QUESTION_SET_SCHEMA, + title, + ...(descriptions.length > 0 + ? { description: descriptions.join("\n\n") } + : {}), + submitLabel: "Submit answers", + questions: bindings.map((binding) => binding.question), + }); + + return { + questionSet, + accept(response: unknown): AcpAcceptElicitationResponse { + const parsed = parsePaperclipQuestionResponse(questionSet, response); + return { + action: "accept", + content: acpContent(bindings, parsed), + }; + }, + }; +} + +function normalizeField( + propertyName: string, + value: unknown, + index: number, + required: boolean, +): AcpFieldBinding { + const property = record(value); + const type = text(property.type); + const id = stableFieldId(propertyName, index); + const header = optionalText(property.title) ?? propertyName; + const prompt = optionalText(property.description) ?? header; + const base = { id, header, prompt, required }; + + if (type === "string") { + const nativeOptions = enumOptions( + property.oneOf ?? property.anyOf, + property.enum, + ); + if (nativeOptions.length > 0) { + const normalized = normalizeOptions(nativeOptions); + return { + propertyName, + property, + optionValues: normalized.values, + question: { + ...base, + answerMode: "single_select", + options: normalized.options, + }, + }; + } + const minLength = finiteNonNegativeInteger(property.minLength); + const maxLength = finiteNonNegativeInteger(property.maxLength); + const pattern = optionalText(property.pattern); + return { + propertyName, + property, + optionValues: new Map(), + question: { + ...base, + answerMode: "text", + textValidation: { + inputType: "text", + ...(minLength !== undefined ? { minLength } : {}), + ...(maxLength !== undefined ? { maxLength } : {}), + ...(pattern !== undefined ? { pattern } : {}), + }, + }, + }; + } + + if (type === "number" || type === "integer") { + const minimum = finiteNumber(property.minimum); + const maximum = finiteNumber(property.maximum); + return { + propertyName, + property, + optionValues: new Map(), + question: { + ...base, + answerMode: "text", + textValidation: { + inputType: type, + ...(minimum !== undefined ? { minimum } : {}), + ...(maximum !== undefined ? { maximum } : {}), + }, + }, + }; + } + + if (type === "boolean") { + const normalized = normalizeOptions([ + { value: "true", label: "Yes" }, + { value: "false", label: "No" }, + ]); + return { + propertyName, + property, + optionValues: normalized.values, + question: { + ...base, + answerMode: "single_select", + options: normalized.options, + }, + }; + } + + if (type === "array") { + const items = record(property.items); + const nativeOptions = enumOptions(items.anyOf ?? items.oneOf, items.enum); + if (nativeOptions.length === 0) { + throw new Error( + `ACP multi-select property ${propertyName} must define enum, anyOf, or oneOf options`, + ); + } + const normalized = normalizeOptions(nativeOptions); + return { + propertyName, + property, + optionValues: normalized.values, + question: { + ...base, + answerMode: "multi_select", + options: normalized.options, + }, + }; + } + + throw new Error( + `Unsupported ACP elicitation property type ${JSON.stringify(type)} for ${propertyName}`, + ); +} + +function acpContent( + bindings: AcpFieldBinding[], + response: PaperclipQuestionResponse, +): AcpFormContent { + const content: AcpFormContent = {}; + for (const binding of bindings) { + const answer = response.answers[binding.question.id]; + if (!answer) continue; + const type = text(binding.property.type); + if (type === "string" && binding.question.answerMode === "text") { + if (answer.text !== undefined) + setContent(content, binding.propertyName, answer.text); + continue; + } + if (type === "number" || type === "integer") { + if (answer.text !== undefined) + setContent(content, binding.propertyName, Number(answer.text)); + continue; + } + if (type === "boolean") { + const selected = answer.selectedOptionIds?.[0]; + if (selected !== undefined) + setContent( + content, + binding.propertyName, + binding.optionValues.get(selected) === "true", + ); + continue; + } + const selectedValues = (answer.selectedOptionIds ?? []).map((id) => { + const selected = binding.optionValues.get(id); + if (selected === undefined) + throw new Error(`ACP option ${id} is no longer available`); + return selected; + }); + if (type === "array") + setContent(content, binding.propertyName, selectedValues); + else if (selectedValues[0] !== undefined) + setContent(content, binding.propertyName, selectedValues[0]); + } + return content; +} + +function setContent( + content: AcpFormContent, + propertyName: string, + value: AcpFormContent[string], +): void { + Object.defineProperty(content, propertyName, { + configurable: true, + enumerable: true, + value, + writable: true, + }); +} + +function enumOptions( + titled: unknown, + values: unknown, +): Array<{ value: string; label: string; description?: string }> { + const source = Array.isArray(titled) + ? titled + : Array.isArray(values) + ? values + : []; + if (source.length > MAX_ACP_FIELD_OPTIONS) { + throw new Error( + `ACP elicitation fields cannot define more than ${MAX_ACP_FIELD_OPTIONS} options`, + ); + } + if (Array.isArray(titled)) { + return titled.map((entry, index) => { + const option = record(entry); + const optionValue = requiredText( + option.const, + `ACP enum option ${index} const`, + ); + const description = optionalText(option.description); + return { + value: optionValue, + label: optionalText(option.title) ?? optionValue, + ...(description !== undefined ? { description } : {}), + }; + }); + } + return source.map((value, index) => { + const native = requiredText(value, `ACP enum option ${index}`); + return { value: native, label: native }; + }); +} + +function normalizeOptions( + nativeOptions: Array<{ + value: string; + label: string; + description?: string; + }>, +): { options: PaperclipQuestionOption[]; values: Map } { + const values = new Map(); + const options = nativeOptions.map( + (option, index): PaperclipQuestionOption => { + const id = `option-${index + 1}`; + values.set(id, option.value); + return { + id, + label: option.label, + ...(option.description !== undefined + ? { description: option.description } + : {}), + }; + }, + ); + return { options, values }; +} + +function stableFieldId(value: string, index: number): string { + const readable = value + .trim() + .replace(/[^A-Za-z0-9._:-]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 96); + const digest = createHash("sha256").update(value).digest("hex").slice(0, 12); + return `field-${index + 1}-${readable || "value"}-${digest}`; +} + +function record(value: unknown): Record { + return typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as Record) + : {}; +} + +function text(value: unknown): string { + return typeof value === "string" ? value : ""; +} + +function optionalText(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function requiredText(value: unknown, field: string): string { + const result = optionalText(value); + if (!result) throw new Error(`${field} is required`); + return result; +} + +function finiteNumber(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) + ? value + : undefined; +} + +function finiteNonNegativeInteger(value: unknown): number | undefined { + return Number.isSafeInteger(value) && Number(value) >= 0 + ? Number(value) + : undefined; +}