diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 028ce2242..2649eaca2 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -4862,10 +4862,11 @@ const REVIEW_CONTROLLER_PARAMETER_FIELDS = { }, } as const; -// Providers such as non-strict Anthropic emit only root properties/required. -// Keep the full declaration here and operation constraints in runtime branches. -// The nullable root shell prevents Pi from deleting an optional supplied null; -// both the branches below and the facade reject it, so it never becomes omitted. +// Root-level union combinators (anyOf/oneOf/allOf) are forbidden by the Anthropic +// tool definition spec and cause Claude Code/Agent SDK to drop the tool (#1698). +// The root schema is a plain object; the nullable root shell prevents Pi from +// deleting an optional supplied null, while runtime branches in the facade +// enforce that null and non-START/ASSESS objects are rejected fail-closed. const REVIEW_CONTROLLER_PARAMETERS = { ...REVIEW_CONTROLLER_PARAMETER_FIELDS, properties: { @@ -4875,24 +4876,6 @@ const REVIEW_CONTROLLER_PARAMETERS = { description: `${REVIEW_CONTROLLER_PARAMETER_FIELDS.properties.input.description} Null is invalid; omit input when optional.`, }, }, - anyOf: [ - { - ...REVIEW_CONTROLLER_PARAMETER_FIELDS, - properties: { - ...REVIEW_CONTROLLER_PARAMETER_FIELDS.properties, - operation: { ...REVIEW_CONTROLLER_PARAMETER_FIELDS.properties.operation, enum: ["start", "assess"] }, - input: { ...REVIEW_JSON_ARGUMENT, description: REVIEW_CONTROLLER_PARAMETER_FIELDS.properties.input.description }, - }, - }, - { - ...REVIEW_CONTROLLER_PARAMETER_FIELDS, - properties: { - ...REVIEW_CONTROLLER_PARAMETER_FIELDS.properties, - operation: { ...REVIEW_CONTROLLER_PARAMETER_FIELDS.properties.operation, enum: Object.values(REVIEW_CONTROLLER_OPERATION).filter((operation) => operation !== "start" && operation !== "assess") }, - input: { ...REVIEW_JSON_STRING, description: "Serialized JSON object string only; objects are accepted only by START/ASSESS." }, - }, - }, - ], } as const; const REVIEW_CAPTURE_PARAMETERS = { diff --git a/odd/tasks/fix-1698-gentle-review-schema-top-level-anyof.md b/odd/tasks/fix-1698-gentle-review-schema-top-level-anyof.md new file mode 100644 index 000000000..2a9d13155 --- /dev/null +++ b/odd/tasks/fix-1698-gentle-review-schema-top-level-anyof.md @@ -0,0 +1,18 @@ +# Flatten gentle_review root schema for Claude Agent SDK compatibility (#1698) + +## Objective and scope +Prevent `gentle_review` from being dropped under Claude Agent SDK providers (such as `pi-claude-bridge` and Claude Code). Anthropic API rejects root-level `anyOf`/`oneOf`/`allOf` combinators in tool `input_schema`. Flatten `REVIEW_CONTROLLER_PARAMETERS` to a clean `type: "object"` root schema, keeping operation-specific `input` type constraints fail-closed at the facade/runtime level. + +## Completed tasks +- [x] T1: Strict TDD test reproducing rejection of root-level `anyOf`/`oneOf`/`allOf` for `gentle_review`. +- [x] T2: Flatten `REVIEW_CONTROLLER_PARAMETERS` in `extensions/gentle-ai.ts` by removing root-level `anyOf`. +- [x] T3: Align `tests/review-json-arguments.test.ts` to assert that non-START/ASSESS operations reject objects at the controller execution layer. +- [x] T4: Verify full test suite, typecheck, and package files. + +## Evidence +Base: upstream/main at cf3012f7. Branch: fix/1698-gentle-review-schema-top-level-anyof. +- TDD RED: `gentle_review parameters must not have root-level anyOf` (actual: 2 branches, expected: undefined). +- TDD GREEN: 8/8 tests passed in `tests/review-json-arguments.test.ts`. +- Routing tests: 96/96 passed in `tests/review-controller-native-routing.test.ts`. +- Typecheck: 186 recorded diagnostics, 0 regressions, 12 improved. +- Package files: 155 files, 69 exact byte-pinned artifacts checked. diff --git a/tests/review-json-arguments.test.ts b/tests/review-json-arguments.test.ts index 5dffd6e03..a19e31d0f 100644 --- a/tests/review-json-arguments.test.ts +++ b/tests/review-json-arguments.test.ts @@ -19,6 +19,15 @@ function validate(name: string, args: Record): any { return validateToolArguments(tools.get(name), { type: "toolCall", id: "json-arguments", name, arguments: args as ToolCall["arguments"] }); } +test("registered review schemas have plain object parameters with no root-level union combinators (#1698)", () => { + for (const [name, tool] of tools) { + assert.equal(tool.parameters.type, "object", `${name} parameters must be an object`); + assert.equal(tool.parameters.anyOf, undefined, `${name} parameters must not have root-level anyOf`); + assert.equal(tool.parameters.oneOf, undefined, `${name} parameters must not have root-level oneOf`); + assert.equal(tool.parameters.allOf, undefined, `${name} parameters must not have root-level allOf`); + } +}); + test("public Anthropic non-strict adapter preserves the complete review tool declarations", async () => { const model: Model<"anthropic-messages"> = { id: "claude-sonnet-4-20250514", name: "Payload-only model", api: "anthropic-messages", provider: "anthropic", @@ -100,24 +109,38 @@ test("serialized inputs remain byte-for-byte unchanged at validation and facade assert.equal(__testing.parseReviewCaptureParameters(validate("gentle_review_capture", { lineageId: "l", collectBinding })).collectBinding, collectBinding); }); -test("actual validation rejects arrays, null and coerced primitives rather than admitting native calls", () => { - for (const value of [[], null, 123, true, false, "123", "null", "[]"]) { +test("actual validation rejects arrays, null and coerced primitives rather than admitting native calls", async () => { + for (const value of [[], 123, true, "123", "null", "[]"]) { for (const operation of ["start", "assess", "answer-consent", "inspect", "reset", "recover"]) { assert.throws(() => validate("gentle_review", { operation, input: value }), /Validation failed/); } assert.throws(() => validate("gentle_review_capture", { lineageId: "l", collectBinding: value }), /Validation failed/); assert.throws(() => validate("gentle_review_capture_group", { lineageId: "l", collectBindings: [binding, value] }), /Validation failed/); } + let calls = 0; + const native = new Proxy({}, { get() { return async () => { calls++; throw new Error("unexpected native execution"); }; } }) as NativeReviewCli; + for (const value of [null, false]) { + for (const operation of ["start", "assess", "answer-consent", "inspect", "reset", "recover"]) { + const validated = validate("gentle_review", { operation, input: value }); + assert.equal(validated.input, null, "nullable shell preserves null instead of stripping it to omitted"); + await assert.rejects(__testing.executeReviewControllerOperation(validated, process.cwd(), native)); + } + assert.throws(() => validate("gentle_review_capture", { lineageId: "l", collectBinding: value }), /Validation failed/); + assert.throws(() => validate("gentle_review_capture_group", { lineageId: "l", collectBindings: [binding, value] }), /Validation failed/); + } + assert.equal(calls, 0); }); -test("all other controller operations keep object input fail-closed in schema and facade", async () => { - const operations = tools.get("gentle_review").parameters.anyOf[1].properties.operation.enum; +test("all other controller operations keep object input fail-closed at the facade boundary (#1698)", async () => { + const operations = (tools.get("gentle_review").parameters.properties.operation.enum as string[]).filter((operation) => operation !== "start" && operation !== "assess" && operation !== "select-intended-untracked"); let calls = 0; const native = new Proxy({}, { get() { return async () => { calls++; throw new Error("unexpected native execution"); }; } }) as NativeReviewCli; for (const operation of operations) { const args = { operation, lineageId: "l", input: { consentBinding: "opaque", answer: "granted" } }; - assert.throws(() => validate("gentle_review", args), /Validation failed/); - await assert.rejects(__testing.executeReviewControllerOperation(args, process.cwd(), native)); + await assert.rejects( + __testing.executeReviewControllerOperation(args, process.cwd(), native), + /Review controller input must be a string/, + ); } assert.equal(calls, 0); });