Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions extensions/gentle-ai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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\":\"<what you touched and how>\"} (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\":\"<opaque id>\",\"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\":\"<what you touched and how>\"} (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\":\"<opaque id>\",\"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." },
Expand Down Expand Up @@ -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;
}
Expand Down
30 changes: 30 additions & 0 deletions odd/tasks/fix-2375-gentle-review-answer-consent-input-coercion.md
Original file line number Diff line number Diff line change
@@ -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)`.
23 changes: 20 additions & 3 deletions tests/review-json-arguments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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");
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
Loading