diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 39edf9a2e..faa1a6af4 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -5293,7 +5293,7 @@ const REVIEW_CONTROLLER_PARAMETER_FIELDS = { }, input: { type: "string", - description: "A JSON object or serialized object string for START/ASSESS only; every other operation requires a serialized object string. New native ordinary START uses {\"mode\":\"ordinary\"}, plus the agent lens selection {\"lenses\":[\"risk\",...],\"lensesReason\":\"\"} (both or neither, 1-4 of risk, resilience, readability, reliability; never with focus; providers that predate it keep the tier default); answer-consent uses exactly {\"consentBinding\":\"\",\"answer\":\"granted|declined\"}. Ordinary provider capture belongs only to gentle_review_capture. An explicit baseRef requires committedOnly: true and requests a committed range, while repository-local policyPath remains optional. baseRef must be HEAD, a full 40- or 64-character commit id, or a ref name; abbreviated commit ids are rejected as base-ref-unresolvable. ASSESS accepts an optional object with baseRef, committedOnly, writerModelId, writerEffort, and nativeReviewOutcome (gentle-pi#662/#668/#1175). The writer profile comes from the runtime-recorded model and effort of the pending mutations for this root (writerProfileSource runtime); writerModelId and writerEffort are only a fallback when no runtime evidence exists (caller), and with neither the profile fails closed to small (fallback). The writer profile is reported but never adds a verifier: the plan adds the independent verifier only when escalate names a Task Size high-risk item. ASSESS derives `closed` only from the native candidate.consumed fact for this exact candidate, which native records only when the approved review for that candidate is acknowledged; a caller-supplied closed is not authority and, without that fact, resolves to unknown. Every outcome yields the exact risk-gated plan ASSESS returns when RDD is off: a closed review never removes the separate verifier the tier requires (verify-always-rdd-high S1), unknown is never treated as closed, and a decline is candidate-scoped and never lowers the bar below the RDD-off path. A declined or unavailable outcome, stated explicitly or recorded by this process for this exact candidate (never a different one, and never from repository state alone), wins over closure. The returned outcome_source (explicit|derived|unknown) says which of these produced the value. Legacy controller input remains separate.", + description: "A JSON object or serialized object string for START/ASSESS/answer-consent; every other operation requires a serialized object string. New native ordinary START uses {\"mode\":\"ordinary\"}, plus the agent lens selection {\"lenses\":[\"risk\",...],\"lensesReason\":\"\"} (both or neither, 1-4 of risk, resilience, readability, reliability; never with focus; providers that predate it keep the tier default); answer-consent uses exactly {\"consentBinding\":\"\",\"answer\":\"granted|declined\"}. Ordinary provider capture belongs only to gentle_review_capture. An explicit baseRef requires committedOnly: true and requests a committed range, while repository-local policyPath remains optional. baseRef must be HEAD, a full 40- or 64-character commit id, or a ref name; abbreviated commit ids are rejected as base-ref-unresolvable. ASSESS accepts an optional object with baseRef, committedOnly, writerModelId, writerEffort, and nativeReviewOutcome (gentle-pi#662/#668/#1175). The writer profile comes from the runtime-recorded model and effort of the pending mutations for this root (writerProfileSource runtime); writerModelId and writerEffort are only a fallback when no runtime evidence exists (caller), and with neither the profile fails closed to small (fallback). The writer profile is reported but never adds a verifier: the plan adds the independent verifier only when escalate names a Task Size high-risk item. ASSESS derives `closed` only from the native candidate.consumed fact for this exact candidate, which native records only when the approved review for that candidate is acknowledged; a caller-supplied closed is not authority and, without that fact, resolves to unknown. Every outcome yields the exact risk-gated plan ASSESS returns when RDD is off: a closed review never removes the separate verifier the tier requires (verify-always-rdd-high S1), unknown is never treated as closed, and a decline is candidate-scoped and never lowers the bar below the RDD-off path. A declined or unavailable outcome, stated explicitly or recorded by this process for this exact candidate (never a different one, and never from repository state alone), wins over closure. The returned outcome_source (explicit|derived|unknown) says which of these produced the value. Legacy controller input remains separate.", }, outputPath: { type: "string", description: "Retired with legacy bundle export; ignored. Export returns legacy-operation-retired." }, inputPath: { type: "string", description: "Repository-local JSON input file for the separate legacy controller flow (alternative to input). Legacy bundle import is retired." }, @@ -5564,7 +5564,7 @@ function parseReviewControllerParameters(value: unknown): ReviewControllerParame }; for (const key of ["changeName", "idempotencyKey", "transition", "input", "outputPath", "inputPath", "operationId", "lineageIds", "acknowledgeUntrustedBundleSource", "workspaceRoot"] as const) { const optional = value[key]; - if (key === "input" && key in value && (value.operation === REVIEW_CONTROLLER_OPERATION.START || value.operation === REVIEW_CONTROLLER_OPERATION.ASSESS)) { + if (key === "input" && key in value && (value.operation === REVIEW_CONTROLLER_OPERATION.START || value.operation === REVIEW_CONTROLLER_OPERATION.ASSESS || value.operation === REVIEW_CONTROLLER_OPERATION.ANSWER_CONSENT)) { parameters.input = serializeReviewJsonArgument(optional); continue; } diff --git a/odd/tasks/fix-2375-gentle-review-answer-consent-input-coercion.md b/odd/tasks/fix-2375-gentle-review-answer-consent-input-coercion.md new file mode 100644 index 000000000..a50b87c5c --- /dev/null +++ b/odd/tasks/fix-2375-gentle-review-answer-consent-input-coercion.md @@ -0,0 +1,30 @@ +# Task Breakdown: Fix #2375 Gentle Review Answer Consent Input Coercion + +**Feature:** `fix-2375-gentle-review-answer-consent-input-coercion` +**Issue:** #2375 (`bug(review): gentle_review answer-consent input is unreachable from direct tool calls (object never coerced to string) (4.3.0)`) +**Strategy:** Strict TDD, ODD workflow, Conventional Commits + +--- + +## Tasks + +### 1. Test (RED): Unit test for answer-consent object input coercion +- [x] In `tests/review-json-arguments.test.ts`: + - Add test asserting that `validate("gentle_review", { operation: "answer-consent", input })` with object input `{ consentBinding: "bound-1", answer: "granted" }` is admitted and coerced into a serialized string by `parseReviewControllerParameters`. + - Assert that execution reaches the consent registry without throwing `"Review controller input must be a string"`. + - Evidence: Red test failure `Review controller input must be a string` observed before fix. + +### 2. Implementation (GREEN): Coerce answer-consent object input +- [x] In `extensions/gentle-ai.ts`: + - Include `value.operation === REVIEW_CONTROLLER_OPERATION.ANSWER_CONSENT` alongside `START` and `ASSESS` in `parseReviewControllerParameters`. + - Update schema description in `REVIEW_CONTROLLER_PARAMETER_FIELDS.input` to `START/ASSESS/answer-consent`. + - In `tests/review-json-arguments.test.ts`, exempt `answer-consent` from fail-closed string-only operation test. + - Evidence: All 9 tests in `tests/review-json-arguments.test.ts` pass green. + +### 3. Verification & Regressions +- [x] Run test suite: `node --experimental-strip-types --test tests/review-json-arguments.test.ts tests/review-controller.test.ts`. +- [x] Run typecheck: `npm run typecheck` (0 diagnostics, 0 regressions). +- [x] Evidence: 0 regressions, all tests passing. + +### 4. Work-Unit Commit +- [x] Create conventional commit: `fix(review): coerce answer-consent object input to serialized string (#2375)`. diff --git a/tests/review-json-arguments.test.ts b/tests/review-json-arguments.test.ts index a19e31d0f..1fe6a53e3 100644 --- a/tests/review-json-arguments.test.ts +++ b/tests/review-json-arguments.test.ts @@ -53,7 +53,7 @@ test("public Anthropic non-strict adapter preserves the complete review tool dec assert.deepEqual(controller.required, ["operation"]); assert.deepEqual(Object.keys(controller.properties).sort(), ["operation", "lineageId", "selectionBinding", "intendedUntracked", "untrackedScope", "changeName", "idempotencyKey", "transition", "input", "outputPath", "inputPath", "operationId", "lineageIds", "workspaceRoot"].sort()); assert.ok(controller.properties.input.anyOf.some((schema: any) => schema.type === "object")); - assert.match(controller.properties.input.description, /START\/ASSESS only/); + assert.match(controller.properties.input.description, /START\/ASSESS\/answer-consent/); assert.deepEqual(controller.properties.operation.enum, tools.get("gentle_review").parameters.properties.operation.enum); for (const name of ["gentle_review_capture", "gentle_review_capture_group"]) { const schema = emitted.get(name); @@ -66,7 +66,7 @@ test("public Anthropic non-strict adapter preserves the complete review tool dec let nativeCalls = 0; const native = new Proxy({}, { get() { return async () => { nativeCalls++; }; } }) as NativeReviewCli; for (const operation of ["start", "assess", "answer-consent", "inspect", "reset", "recover"]) { - const inputs = operation === "start" || operation === "assess" ? [null] : [null, { consentBinding: "opaque", answer: "granted" }]; + const inputs = operation === "start" || operation === "assess" || operation === "answer-consent" ? [null] : [null, { consentBinding: "opaque", answer: "granted" }]; for (const input of inputs) { const args = validateToolArguments({ ...tools.get("gentle_review"), parameters: controller }, { type: "toolCall", id: "provider-shell", name: "gentle_review", arguments: { operation, input } }); assert.deepEqual(args.input, input, "provider-shell validation accepts and retains this input"); @@ -89,6 +89,23 @@ test("registered review schemas admit START/ASSESS objects and ordered capture o assert.deepEqual(__testing.parseReviewCaptureGroupParameters(args).collectBindings, [JSON.stringify(binding), ordered[1]]); }); +test("answer-consent admits JSON object input and coerces it cleanly (#2375)", async () => { + const input = { consentBinding: "bound-1", answer: "granted" }; + assert.deepEqual(validate("gentle_review", { operation: "answer-consent", input }).input, input); + assert.equal( + __testing.parseReviewControllerParameters(validate("gentle_review", { operation: "answer-consent", input })).input, + JSON.stringify(input), + ); + const native = { targetStatus: async () => ({ targetIdentity: "t1" }) } as unknown as NativeReviewCli; + const outcome = await __testing.executeReviewControllerOperation( + validate("gentle_review", { operation: "answer-consent", input }), + process.cwd(), + native, + ); + assert.equal(outcome.operation, "answer-consent"); + assert.match(String(outcome.outcome), /consent-binding-stale/); +}); + test("validated ASSESS objects reach the mocked native facade with parsed fields", async () => { const requests: unknown[] = []; const native = { assess: async (request: unknown) => { requests.push(request); throw new Error("mock assessment unavailable"); } } as unknown as NativeReviewCli; @@ -132,7 +149,7 @@ test("actual validation rejects arrays, null and coerced primitives rather than }); 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"); + const operations = (tools.get("gentle_review").parameters.properties.operation.enum as string[]).filter((operation) => operation !== "start" && operation !== "assess" && operation !== "answer-consent" && 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) {