fix(review): flatten gentle_review root schema for Claude Agent SDK (#1698)

[carlosmoradev]

Fixes #1698

Problem

Under Claude Agent SDK providers (such as pi-claude-bridge and Claude Code), the gentle_review tool is silently dropped upon session initialization. The sibling review tools (gentle_review_capture, gentle_review_capture_group, gentle_review_scope) remain present. Without gentle_review, the review lifecycle (inspect, START, consent, etc.) becomes completely unreachable.

Root cause: REVIEW_CONTROLLER_PARAMETERS in extensions/gentle-ai.ts was declared with a top-level anyOf to differentiate start/assess operations from string-only operations (#1678). The Anthropic Tool Definition specification strictly forbids root-level anyOf, oneOf, and allOf combinators in tool input_schema (only nested combinators inside properties.<prop> are supported). Claude Agent SDK / Claude Code strictly rejects the tool definition and drops the tool from the available catalog.

Change

• In extensions/gentle-ai.ts, flatten REVIEW_CONTROLLER_PARAMETERS by removing the root-level anyOf combinator. The root schema is now a clean type: "object".
• Preserve properties.input with nested anyOf: [REVIEW_JSON_STRING, { type: "object" }, { type: "null" }], which is valid across Anthropic and other providers and retains null for downstream fail-closed validation.
• Preserve fail-closed runtime safety: parseReviewControllerParameters already enforces that non-START/ASSESS operations reject objects with Review controller input must be a string and zero native calls.
• Add strict TDD test in tests/review-json-arguments.test.ts asserting that all registered review tools declare clean object schemas with no root-level anyOf/oneOf/allOf.
• Update controller operation assertions in tests/review-json-arguments.test.ts to verify that object inputs for non-START/ASSESS operations are rejected fail-closed at the controller facade boundary without dispatching to native CLI.

Verification

• Strict TDD RED: registered review schemas have plain object parameters with no root-level union combinators (#1698) failed against base main with gentle_review parameters must not have root-level anyOf (actual: 2 branches).
• Strict TDD GREEN: 8/8 tests passed in tests/review-json-arguments.test.ts.
• Routing regression suite: node --experimental-strip-types --test tests/review-controller-native-routing.test.ts (96/96 passed).
• Typecheck: pnpm run typecheck clean (186 recorded baseline diagnostics, 0 regressions, 12 improved).
• Package integrity: node scripts/verify-package-files.mjs clean (155 files, 69 exact byte-pinned artifacts verified).

Summary by CodeRabbit

• Bug Fixes
  • Review tools now expose a standard object-based parameter schema, improving compatibility with integrations that inspect tool definitions.
  • Invalid review-controller inputs are rejected during execution without triggering native calls. Capture tools continue to reject invalid inputs during validation. Null and false controller values can pass schema validation but are still rejected by the review facade.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: cad9fcb0-5899-4669-90bf-15d6cc39df38

📥 Commits

Reviewing files that changed from the base of the PR and between cf3012f and 2985fdf.

📒 Files selected for processing (3)

• extensions/gentle-ai.ts
• odd/tasks/fix-1698-gentle-review-schema-top-level-anyof.md
• tests/review-json-arguments.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The gentle_review controller schema now has a plain object root instead of root-level anyOf branches. Tests cover the updated schema shape and distinguish input validation from facade rejection.

Changes

gentle_review schema compatibility

Layer / File(s)	Summary
Flatten the controller schema extensions/gentle-ai.ts, tests/review-json-arguments.test.ts	The controller schema no longer splits operations into root-level anyOf branches. A test checks that registered review tools use object parameters without root-level union combinators.
Verify facade input handling tests/review-json-arguments.test.ts, odd/tasks/fix-1698-gentle-review-schema-top-level-anyof.md	Tests cover nullable controller inputs, rejected primitive inputs, and facade rejection of object inputs for other operations. The task document records the schema-flattening work and verification results.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: dnlrsls

Merge Risk: ⚪ Minimal · up to 2985f

The schema change addresses provider compatibility while retaining the checked input protections. No identified issue blocks merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2985f

The broader input schema still routes through operation-specific validation and existing consent and authorization controls. No security bypass was identified. Actual provider registration and native persistence behavior were not verified end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The exposure change is availability of existing review operations to tool callers on previously incompatible providers. Those operations can affect review authority and recovery state, but the inspected change does not add operation names or alter their existing authorization implementation.

Trust Boundaries and Controls

• observed — Tool arguments do not themselves create consent authority. The consent continuation checks a pending binding’s expiry, repository identity, digest, and candidate validity, and consumes it before awaiting native execution. Invalid object or null controller inputs are rejected separately by the parser.

Resilience and Maintainability Implications

• observed — The inspected recovery path rechecks target authority after interactive approval. Ambiguous mutations use status reconciliation, and acknowledgement distinguishes committed mutation from deferred cleanup rather than inviting replay. These are existing facade safeguards; native durable-store and locking behavior were not independently established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the change: flattening the gentle_review root schema for Claude Agent SDK compatibility.
Linked Issues check	✅ Passed	Issue [#1698] requires gentle_review to use a plain object root schema so Claude Agent SDK providers do not drop the tool. The PR summary reports that REVIEW_CONTROLLER_PARAMETERS now has a root `…
Out of Scope Changes check	✅ Passed	The schema change and regression tests directly address issue [#1698]. The task document records the same fix and its verification. The reported changes have a clear connection to the linked issue; no…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[danielgap]

Reviewed from the facade/schema side — the diagnosis matches what I can verify, and the trade is the right one.

Root cause check: REVIEW_CONTROLLER_PARAMETERS carried a root-level anyOf with per-operation branches (#1678). Anthropic's tool input_schema only allows combinators nested inside properties, and a root union makes Claude Code / Agent SDK silently drop the whole tool — worse than a validation error, since the review lifecycle becomes unreachable with no diagnostic. Removing the root union while keeping constraint enforcement in parseReviewControllerParameters and the facade branches is correct: the schema loosens (objects now pass schema validation for non-START/ASSESS operations), but the facade still rejects them fail-closed with zero native dispatch — the updated tests pin exactly that.

Verified locally (PR head merged with current origin/main — clean textual merge):

• tests/review-json-arguments.test.ts 8/8, tests/review-controller-native-routing.test.ts 96/96, typecheck 186 recorded / no regressions, full suite PASS.
• RED confirmed: restoring main's declaration makes the new "no root-level union combinators" test fail exactly as the PR describes (root anyOf, 2 branches).
• The select-intended-untracked exclusion in the updated test is legitimate: that operation goes through its own selection parser, which rejects an unexpected input key with its own fail-closed error — same closure, different message.
• The retained input union ([...REVIEW_JSON_ARGUMENT.anyOf, { type: "null" }]) sits nested inside the property — the spec-valid spot.

One suggestion (non-blocking): schema-level rejection is gone, so runtime branches are now the only guard; if a future operation is added without one, only the test catches it. A one-line comment at the input property pointing to review-json-arguments.test.ts as the guard would help the next editor — the test is the real net either way.

Ready from my side once #1698 gets its triage.