From 2aae189d5c697000fe4fc28052d162d5a63948f1 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Thu, 12 Nov 2026 14:19:27 +0100 Subject: [PATCH 1/4] feat(review): bound non-blocking findings in coordinator results --- lib/review-coordinator.ts | 150 +++++++++++++++++++++- tests/review-coordinator-findings.test.ts | 133 +++++++++++++++++++ 2 files changed, 280 insertions(+), 3 deletions(-) create mode 100644 tests/review-coordinator-findings.test.ts diff --git a/lib/review-coordinator.ts b/lib/review-coordinator.ts index a7dd8fa87..6f9b29c9d 100644 --- a/lib/review-coordinator.ts +++ b/lib/review-coordinator.ts @@ -4,6 +4,129 @@ type Body = Record; const record = (value: unknown): Body => value !== null && typeof value === "object" && !Array.isArray(value) ? value as Body : {}; const nonempty = (value: unknown): value is string => typeof value === "string" && value.trim().length > 0; +// odd/tasks/review-one-call.md S5: what each finding says, in the reviewer's +// own words. Each id is looked up in `findings` by exact id and its claim is +// copied unchanged, cut at the last whole character within +// REVIEW_FINDING_CLAIM_MAX_BYTES. Claims are added in `ids` order until the +// next one would pass REVIEW_FINDING_CLAIMS_MAX_BYTES in total; the rest get no +// entry. An id with no matching finding, or with more than one, is omitted. +// Beside them, `truncated` lists the ids whose kept claim is a cut prefix and +// `omitted` the ids whose one matching claim the total limit left out, so +// neither passes for a whole claim. Reviewer text is untrusted: nothing routes +// on it. Shared by the facade's approved closure (advisory findings) and the +// run's correction request (correction findings). +export const REVIEW_FINDING_CLAIM_MAX_BYTES = 1024; +export const REVIEW_FINDING_CLAIMS_MAX_BYTES = 8192; + +function withinUtf8Bytes(text: string, maximum: number): string { + if (Buffer.byteLength(text) <= maximum) return text; + let kept = "", bytes = 0; + for (const character of text) { + bytes += Buffer.byteLength(character); + if (bytes > maximum) break; + kept += character; + } + return kept; +} + +export function boundedFindingClaims(ids: readonly string[], findings: readonly { id: string; claim: string }[]): { claims: Record; truncated: string[]; omitted: string[] } | undefined { + const entries: [string, string][] = []; + const truncated: string[] = [], omitted: string[] = []; + let total = 0; + for (const id of ids) { + const matches = findings.filter((finding) => finding.id === id); + if (matches.length !== 1) continue; + const claim = withinUtf8Bytes(matches[0]!.claim, REVIEW_FINDING_CLAIM_MAX_BYTES); + if (omitted.length > 0 || total + Buffer.byteLength(claim) > REVIEW_FINDING_CLAIMS_MAX_BYTES) { omitted.push(id); continue; } + total += Buffer.byteLength(claim); + entries.push([id, claim]); + if (claim !== matches[0]!.claim) truncated.push(id); + } + // fromEntries defines own data properties, so no id can reach a prototype. + return entries.length === 0 ? undefined : { claims: Object.fromEntries(entries), truncated, omitted }; +} + +// The provider fields of a correction-request finding relayed in +// correction_findings, unchanged. The claim travels bounded in +// correction_finding_claims; evidence and proof references stay in the relayed +// STATUS envelope. +const CORRECTION_FINDING_REQUIRED = ["id", "lens", "location", "severity"] as const; +const CORRECTION_FINDING_OPTIONAL = ["evidence_class", "causal_disposition"] as const; + +/** Relay keys for a correction request's findings, or none when any entry is malformed. Never routed on. */ +function correctionFindingKeys(request: unknown): Body { + const findings = record(request).findings; + if (!Array.isArray(findings) || findings.length === 0) return {}; + const plain = (value: unknown): value is Body => value !== null && typeof value === "object" && !Array.isArray(value) && [Object.prototype, null].includes(Object.getPrototypeOf(value)); + if (!findings.every((finding) => plain(finding) && [...CORRECTION_FINDING_REQUIRED, "claim"].every((key) => Object.hasOwn(finding, key) && nonempty(finding[key])) + && CORRECTION_FINDING_OPTIONAL.every((key) => !Object.hasOwn(finding, key) || typeof finding[key] === "string"))) return {}; + const entries = findings as Body[]; + const listed = entries.map((finding) => Object.fromEntries([...CORRECTION_FINDING_REQUIRED, ...CORRECTION_FINDING_OPTIONAL] + .filter((key) => Object.hasOwn(finding, key)).map((key) => [key, finding[key]]))); + const bounded = boundedFindingClaims(entries.map((finding) => finding.id as string), entries as { id: string; claim: string }[]); + return { correction_findings: listed, + ...(bounded === undefined ? {} : { correction_finding_claims: bounded.claims, + ...(bounded.truncated.length === 0 ? {} : { correction_finding_claims_truncated: bounded.truncated }), + ...(bounded.omitted.length === 0 ? {} : { correction_finding_claims_omitted: bounded.omitted }) }) }; +} + +// The provider fields of a non-blocking finding relayed in +// non_blocking_findings, unchanged; the claim travels bounded in +// non_blocking_finding_claims. +const NON_BLOCKING_FINDING_REQUIRED = ["id", "severity", "disposition"] as const; +const NON_BLOCKING_FINDING_OPTIONAL = ["lens", "location"] as const; +const NON_BLOCKING_FINDING_FIELDS = ["id", "lens", "location", "severity", "disposition"] as const; +export const NON_BLOCKING_FINDINGS_MAX_BYTES = 16 * 1024; + +/** + * The run's nested provider record without the raw `non_blocking_findings` + * (L19): the bounded top-level keys are the only copy. Nothing else changes. + */ +function withoutRawNonBlockingFindings(result: Body): Body { + const nested = result.result; + if (nested === null || typeof nested !== "object" || Array.isArray(nested) || !Object.hasOwn(nested, "non_blocking_findings")) return result; + const { non_blocking_findings: _raw, ...rest } = nested as Body; + return { ...result, result: rest }; +} + +/** + * Relay keys for STATUS `non_blocking_findings` (gentle-ai#6088), or none when + * the provider sent no such key or any part of it is malformed. An empty + * provider list is the provider's answer ("none") and is relayed as such. + * Never routed on. + */ +function nonBlockingFindingKeys(status: unknown): Body { + const native = record(status); + if (!Object.hasOwn(native, "non_blocking_findings")) return {}; + const value = native.non_blocking_findings; + const plain = (entry: unknown): entry is Body => entry !== null && typeof entry === "object" && !Array.isArray(entry) && [Object.prototype, null].includes(Object.getPrototypeOf(entry)); + if (!plain(value) || !nonempty(value.statement) || !Array.isArray(value.findings)) return {}; + const findings = value.findings as unknown[]; + if (!findings.every((finding) => plain(finding) && NON_BLOCKING_FINDING_REQUIRED.every((key) => Object.hasOwn(finding, key) && nonempty(finding[key])) + && Object.hasOwn(finding, "claim") && typeof finding.claim === "string" + && NON_BLOCKING_FINDING_OPTIONAL.every((key) => !Object.hasOwn(finding, key) || nonempty(finding[key])))) return {}; + // The list is bounded too (L19): findings are listed in provider order until + // the next one would bring the list past NON_BLOCKING_FINDINGS_MAX_BYTES of + // JSON; the rest are only counted. Claims are bounded over the listed ones. + const listed: Body[] = []; + let listBytes = 2; + for (const finding of findings as Body[]) { + const entry = Object.fromEntries(NON_BLOCKING_FINDING_FIELDS.filter((key) => Object.hasOwn(finding, key)).map((key) => [key, finding[key]])); + const entryBytes = Buffer.byteLength(JSON.stringify(entry)) + (listed.length === 0 ? 0 : 1); + if (listBytes + entryBytes > NON_BLOCKING_FINDINGS_MAX_BYTES) break; + listBytes += entryBytes; + listed.push(entry); + } + const omittedCount = findings.length - listed.length; + const entries = (findings as Body[]).slice(0, listed.length); + const bounded = boundedFindingClaims(entries.map((finding) => finding.id as string), entries as { id: string; claim: string }[]); + return { non_blocking_findings: { statement: value.statement, findings: listed }, + ...(omittedCount === 0 ? {} : { non_blocking_findings_omitted_count: omittedCount }), + ...(bounded === undefined ? {} : { non_blocking_finding_claims: bounded.claims, + ...(bounded.truncated.length === 0 ? {} : { non_blocking_finding_claims_truncated: bounded.truncated }), + ...(bounded.omitted.length === 0 ? {} : { non_blocking_finding_claims_omitted: bounded.omitted }) }) }; +} + export interface ReviewCoordinatorInput { workspaceRoot?: string; baseRef?: string; @@ -74,6 +197,12 @@ export async function runReviewCoordinator(value: unknown, deps: ReviewCoordinat // The facade's marks on those claims: ids whose claim is a cut prefix, and ids // whose claim the per-closure limit left out. Relayed with the claims only. let closureClaimMarks: Body = {}; + // The correction request's findings and their bounded claims, once STATUS + // asked for a correction plan: carried by every result after it. Never routed on. + let correctionKeys: Body = {}; + // The provider's non-blocking findings of the STATUS that stopped the run on + // a correction request or a stop (escalated). Never routed on. + let nonBlockingKeys: Body = {}; // That closure's capture envelope, kept unread for cancelled only while the // retained closure carries advisory_findings. let pendingCapture: Body | undefined; @@ -112,9 +241,11 @@ export async function runReviewCoordinator(value: unknown, deps: ReviewCoordinat // caller resumes the run with these plus the chosen untracked selection, so // START keeps the requested lenses and request context. const { untrackedScope: _scope, intendedUntracked: _paths, ...resumeInput } = structuredClone(input); - const finish = (outcome: string, result: Body) => ({ schema: "gentle-pi.review-coordinator-experiment/v1", outcome, result, forecasts, reviewer_runs: reviewerRuns, + const finish = (outcome: string, result: Body) => ({ schema: "gentle-pi.review-coordinator-experiment/v1", outcome, result: withoutRawNonBlockingFindings(result), forecasts, reviewer_runs: reviewerRuns, ...(closureFindings !== undefined ? { advisory_findings: closureFindings.value } : {}), ...(closureFindings !== undefined && closureClaims !== undefined ? { advisory_finding_claims: closureClaims.value, ...closureClaimMarks } : {}), + ...structuredClone(correctionKeys), + ...structuredClone(nonBlockingKeys), ...(outcome === "selection-required" ? { resume_input: resumeInput } : {}), ...(lineage === undefined ? {} : { lineage_id: lineage }), metrics: { elapsed_ms: now() - began, facade_calls: steps.length, steps } }); const status = () => invoke("gentle_review", { operation: "status", lineageId: lineage, ...selectorInput }); @@ -145,7 +276,11 @@ export async function runReviewCoordinator(value: unknown, deps: ReviewCoordinat } if (details.failure !== undefined || details.native_failure !== undefined || details.reconciliation !== undefined || details.reconciliation_failure !== undefined) return finish("blocked", details); const transition = record(native.next_transition); - if (transition.kind === "stop") return finish("stopped", details); + if (transition.kind === "stop") { + // Read for relay only, after the routing decision above. + nonBlockingKeys = nonBlockingFindingKeys(native); + return finish("stopped", details); + } if (transition.kind === "execute") { const operation = record(transition.execute).operation; if (operation === "review.start") { @@ -172,7 +307,16 @@ export async function runReviewCoordinator(value: unknown, deps: ReviewCoordinat const entries = Array.isArray(details.collectBindings) ? details.collectBindings.map(record) : []; // Native explicitly supplies the authored correction request. Do not derive // host routing metadata from submission domains or inspect opaque bindings. - if (transition.correction_request !== undefined) return finish("input-required", details); + if (transition.correction_request !== undefined) { + // Read for relay only, after the routing decision above. The correction + // fields are returned only beside the provider's answer to the + // non-blocking question (L21): without it, a parent holding only the + // findings to correct would never learn the non-blocking ones, so the + // result stays as before T11 and the parent reads the review as it did. + nonBlockingKeys = nonBlockingFindingKeys(native); + if (Object.hasOwn(nonBlockingKeys, "non_blocking_findings")) correctionKeys = correctionFindingKeys(transition.correction_request); + return finish("input-required", details); + } // The facade's own pre-lineage intended-untracked selection (selectionBinding // and nextStep, no collectBindings): the parent chooses, never the run. The // facade's nextStep names the single selection operation, which would start diff --git a/tests/review-coordinator-findings.test.ts b/tests/review-coordinator-findings.test.ts new file mode 100644 index 000000000..a3a0391a5 --- /dev/null +++ b/tests/review-coordinator-findings.test.ts @@ -0,0 +1,133 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { runReviewCoordinator, type ReviewCoordinatorTool } from "../lib/review-coordinator.ts"; + +// odd/tasks/review-one-call.md T11 (S5), L18: a run that stops on a correction +// request or an escalation returns the provider's non-blocking findings +// (STATUS non_blocking_findings, gentle-ai#6088) with bounded reviewer claims. + +type Body = Record; +interface Metrics { steps: Body[] } +const target = "sha256:" + "a".repeat(64); +const lineage = "review-fixture"; +const collect = { result: { target_identity: target, authority: { lineage_id: lineage }, next_transition: { kind: "collect" } }, collectBindings: [{ collectBinding: "opaque-not-json" }] }; +const approved = { result: { target_identity: target, authority: { lineage_id: lineage }, next_transition: { kind: "execute", execute: { operation: "review.acknowledge-approved" } } } }; +const closed = { outcome: "native-approved-acknowledgement-completed", status: "closed", authority: "burned", target_identity: target, lineage_id: lineage }; +const captured = { outcome: "native-last-event-closure" }; +function backend(responses: Body[]) { + const calls: { tool: ReviewCoordinatorTool; parameters: Body }[] = []; + return { calls, async invoke(tool: ReviewCoordinatorTool, parameters: Body) { + calls.push({ tool, parameters }); + assert.ok(responses.length, "no unexpected follow-up or retry"); + return responses.shift()!; + } }; +} + +const blocking = { id: "B1", lens: "risk", location: "a.ts:1", severity: "CRITICAL", claim: "blocking claim", proof_refs: ["p"], evidence: "e", evidence_class: "deterministic", causal_disposition: "introduced" }; +const finding = (id: string, claim: unknown, extra: Body = {}) => ({ id, lens: "readability", location: "a.ts:2", severity: "WARNING", disposition: "informational", claim, ...extra }); +const set = (findings: unknown) => ({ statement: "Fixture statement; not real review output.", findings }); +const correction = (nonBlocking?: unknown) => ({ ...collect, result: { ...collect.result, + next_transition: { kind: "collect", correction_request: { request_hash: "opaque-request", findings: [blocking] } }, + ...(nonBlocking === undefined ? {} : { non_blocking_findings: nonBlocking }) }, collectBindings: [{ collectBinding: "plan" }] }); +const escalated = (nonBlocking?: unknown) => ({ ...collect, result: { ...collect.result, authority: { lineage_id: lineage, state: "escalated" }, + escalation: { cause: "unknown_causality", finding_ids: ["B1"] }, next_transition: { kind: "stop", reason_code: "native_stop_required" }, + ...(nonBlocking === undefined ? {} : { non_blocking_findings: nonBlocking }) } }); +const NON_BLOCKING_KEYS = ["non_blocking_findings", "non_blocking_finding_claims", "non_blocking_finding_claims_truncated", "non_blocking_finding_claims_omitted"]; +const listed = ({ claim: _claim, ...rest }: Body) => rest; + +test("a correction request returns the provider's non-blocking findings and their claims beside correction_findings", async () => { + const findings = [finding("W1", "first claim"), finding("F2", "second claim", { severity: "CRITICAL", disposition: "follow_up", lens: undefined }), finding("R3", "third claim", { disposition: "refuted", location: undefined })] + .map((entry) => Object.fromEntries(Object.entries(entry).filter(([, value]) => value !== undefined))); + const response = correction(set(structuredClone(findings))); + const host = backend([response]); + const result = await runReviewCoordinator({ lineageId: lineage }, host); + assert.equal(result.outcome, "input-required"); assert.equal(host.calls.length, 1); + const { non_blocking_findings: _raw, ...nested } = response.result; + assert.deepEqual(result.result, { ...response, result: nested }, "the STATUS envelope is relayed with only the raw set left out (L19)"); + assert.deepEqual(result.non_blocking_findings, set(findings.map(listed)), "statement and provider fields in provider order, without the claim"); + assert.deepEqual(result.non_blocking_finding_claims, { W1: "first claim", F2: "second claim", R3: "third claim" }); + for (const key of ["non_blocking_finding_claims_truncated", "non_blocking_finding_claims_omitted"]) assert.equal(Object.hasOwn(result, key), false, key); + assert.deepEqual(result.correction_findings, [listed({ ...blocking, evidence: undefined, proof_refs: undefined })].map((entry) => Object.fromEntries(Object.entries(entry).filter(([, value]) => value !== undefined)))); + assert.deepEqual(result.correction_finding_claims, { B1: "blocking claim" }, "correction keys unchanged"); + for (const key of ["advisory_findings", "advisory_finding_claims"]) assert.equal(Object.hasOwn(result, key), false, key); +}); + +test("non-blocking claims keep the 1,024-byte cut, the 8,192-byte limit and their marks; an empty claim is a claim", async () => { + const many = [finding("N0", "é".repeat(600)), ...Array.from({ length: 8 }, (_, i) => finding(`N${i + 1}`, "z".repeat(1024))), finding("N9", "")]; + const result = await runReviewCoordinator({ lineageId: lineage }, backend([correction(set(structuredClone(many)))])); + assert.equal((result.non_blocking_finding_claims as Body).N0, "é".repeat(512)); + assert.deepEqual(result.non_blocking_finding_claims_truncated, ["N0"]); + assert.deepEqual(Object.keys(result.non_blocking_finding_claims as Body), ["N0", "N1", "N2", "N3", "N4", "N5", "N6", "N7"]); + assert.deepEqual(result.non_blocking_finding_claims_omitted, ["N8", "N9"]); + assert.deepEqual((result.non_blocking_findings as Body).findings, many.map(listed), "every finding is listed, with or without a claim"); + const empty = await runReviewCoordinator({ lineageId: lineage }, backend([correction(set([finding("E1", "")]))])); + assert.deepEqual(empty.non_blocking_finding_claims, { E1: "" }); +}); + +test("an escalated stop returns the provider's non-blocking findings with claims", async () => { + const response = escalated(set([finding("W1", "escalated claim")])); + const result = await runReviewCoordinator({ lineageId: lineage }, backend([response])); + assert.equal(result.outcome, "stopped"); + const { non_blocking_findings: _raw, ...nested } = response.result; + assert.deepEqual(result.result, { ...response, result: nested }, "only the raw set is left out of the nested record (L19)"); + assert.deepEqual(result.non_blocking_findings, set([listed(finding("W1", ""))])); + assert.deepEqual(result.non_blocking_finding_claims, { W1: "escalated claim" }); + for (const key of ["correction_findings", "correction_finding_claims"]) assert.equal(Object.hasOwn(result, key), false, key); +}); + +test("an empty provider list is returned as the provider's answer; no key when the provider said nothing", async () => { + for (const response of [correction(set([])), escalated(set([]))]) { + const result = await runReviewCoordinator({ lineageId: lineage }, backend([response])); + assert.deepEqual(result.non_blocking_findings, set([])); + for (const key of NON_BLOCKING_KEYS.slice(1)) assert.equal(Object.hasOwn(result, key), false, key); + } + for (const response of [correction(), escalated()]) { + const result = await runReviewCoordinator({ lineageId: lineage }, backend([response])); + for (const key of NON_BLOCKING_KEYS) assert.equal(Object.hasOwn(result, key), false, key); + } +}); + +test("a malformed non-blocking set is dropped whole and never changes the outcome", async () => { + const proto = Object.create({ id: "N1" }); + for (const value of ["s", [], null, { statement: "", findings: [] }, { statement: "s" }, { statement: "s", findings: {} }, set([finding("N1", 3)]), set([{ ...finding("N1", "c"), id: "" }]), + set([(({ claim: _claim, ...rest }) => rest)(finding("N1", "c"))]), set([{ ...finding("N1", "c"), disposition: 1 }]), set([{ ...finding("N1", "c"), severity: "" }]), set([proto]), set([finding("N1", "c"), "N2"])]) { + for (const response of [correction(value), escalated(value)]) { + const result = await runReviewCoordinator({ lineageId: lineage }, backend([response])); + assert.equal(result.outcome, response.result.next_transition.kind === "stop" ? "stopped" : "input-required", JSON.stringify(value)); + for (const key of NON_BLOCKING_KEYS) assert.equal(Object.hasOwn(result, key), false, `${key}: ${JSON.stringify(value)}`); + } + } +}); + +test("non-blocking findings are read only from the STATUS that ends the run; the approved path is unchanged", async () => { + // Present on a STATUS that the run continues past: never relayed. + const continuing = { ...collect, result: { ...collect.result, non_blocking_findings: set([finding("W1", "c")]) } }; + const run = await runReviewCoordinator({ lineageId: lineage }, backend([continuing, { ...captured, closure: { advisory_findings: { statement: "s", findings: [{ id: "F1" }] }, advisory_finding_claims: { F1: "c" } } }, approved, closed])); + assert.equal(run.outcome, "closed"); + assert.deepEqual(run.advisory_findings, { statement: "s", findings: [{ id: "F1" }] }); assert.deepEqual(run.advisory_finding_claims, { F1: "c" }); + for (const key of [...NON_BLOCKING_KEYS, "correction_findings"]) assert.equal(Object.hasOwn(run, key), false, key); + // Nothing routes on it: the same STATUS without the key takes the same route. + const routed = await runReviewCoordinator({ lineageId: lineage }, backend([{ ...collect }, { ...captured }, approved, closed])); + assert.equal(routed.outcome, "closed"); assert.deepEqual((routed.metrics as Metrics).steps.map((step: Body) => step.tool), (run.metrics as Metrics).steps.map((step: Body) => step.tool)); +}); + +test("the listed findings fit the 16 KiB JSON boundary and count the unlisted remainder", async () => { + const base = [finding("N1", "first"), finding("N2", "second")]; + const padding = 16 * 1024 - Buffer.byteLength(JSON.stringify(base.map(listed))); + for (const delta of [-1, 0, 1]) { + const findings = [base[0]!, finding("N2", "second", { location: `${base[1]!.location}${"x".repeat(padding + delta)}` })]; + assert.equal(Buffer.byteLength(JSON.stringify(findings.map(listed))), 16 * 1024 + delta); + const response = escalated(set(findings)); + const host = backend([response]); + const result = await runReviewCoordinator({ lineageId: lineage }, host); + const expected = delta > 0 ? findings.slice(0, 1) : findings; + assert.equal(result.outcome, "stopped"); + assert.equal(host.calls.length, 1); + assert.deepEqual(result.non_blocking_findings, set(expected.map(listed))); + assert.deepEqual(result.non_blocking_finding_claims, delta > 0 ? { N1: "first" } : { N1: "first", N2: "second" }); + assert.equal(Object.hasOwn(result, "non_blocking_findings_omitted_count"), delta > 0); + if (delta > 0) assert.equal(result.non_blocking_findings_omitted_count, 1); + const { non_blocking_findings: _raw, ...nested } = response.result; + assert.deepEqual(result.result, { ...response, result: nested }); + } +}); From 55f3b664f1cbbe25a286edb78c486457f2ae0116 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Thu, 12 Nov 2026 14:57:03 +0100 Subject: [PATCH 2/4] feat(review): scope bounded findings requests to review runs --- extensions/gentle-ai.ts | 18 +- ...review-coordinator-correction-gate.test.ts | 105 ++++++++++++ ...view-coordinator-nonblocking-scope.test.ts | 162 ++++++++++++++++++ .../review-one-call-reviewer-requests.test.ts | 10 +- tests/support/review-coordinator-runtime.ts | 18 +- 5 files changed, 310 insertions(+), 3 deletions(-) create mode 100644 tests/review-coordinator-correction-gate.test.ts create mode 100644 tests/review-coordinator-nonblocking-scope.test.ts diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 4afa5e68a..bbc66823b 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -2,6 +2,7 @@ import { emptyFallbacksFile, fallbacksFilePath, followProfileRename, readFallbac import { getSupportedThinkingLevels } from "@earendil-works/pi-ai"; import { PROFILE_TIER_EVENT, supportsOpenAIFast } from "../lib/openai-fast-tier.ts"; import { isDeepStrictEqual } from "node:util"; +import { AsyncLocalStorage } from "node:async_hooks"; import { appendSystemPromptOnce } from "../lib/append-system-prompt.ts"; import { filterChildSessionContextFiles, type ContextFileOptions } from "../lib/child-context-files.ts"; import { dataArgumentRanges, recognizeDestructiveCommands } from "../lib/destructive-command-guard.ts"; @@ -9975,6 +9976,16 @@ interface NegotiatedHostTransportStatus { } const reviewTransportRefusalByProvider = new WeakMap(); +// odd/tasks/review-one-call.md L19: the STATUS opt-in for a stopped review's +// non-blocking findings (gentle-ai#6088) is asked only by facade calls that +// gentle_review_run dispatches. The scope is set by the run around its own +// facade dispatch, in process; it is not a tool parameter, so no model input +// can set it, and a direct gentle_review call never asks for the findings. +const reviewRunScope = new AsyncLocalStorage(); +function nonBlockingFindingsForRun(): { nonBlockingFindings?: true } { + return reviewRunScope.getStore() === true ? { nonBlockingFindings: true } : {}; +} + function clearReviewTransportProbeForTesting(nativeReviewCli: NativeReviewCli | null): void { if (nativeReviewCli !== null) reviewTransportRefusalByProvider.delete(nativeReviewCli as unknown as object); } @@ -10794,6 +10805,8 @@ async function executeReviewControllerOperationImpl( { cwd: defaultCwd, ...inspectSelector, + // gentle-ai#6088: only on behalf of gentle_review_run (L19). + ...nonBlockingFindingsForRun(), ...(signal === undefined ? {} : { signal }), }, retainedUntrackedSelections, @@ -11747,6 +11760,9 @@ async function executeReviewControllerOperationImpl( ...(parameters.lineageId === undefined ? {} : { lineageId: parameters.lineageId }), ...(effectiveBaseRef === undefined ? {} : { baseRef: effectiveBaseRef, committedOnly: true }), ...(effectiveUntrackedSelection.untrackedScope === undefined ? {} : effectiveUntrackedSelection), + // gentle-ai#6088: only on behalf of gentle_review_run (L19); the run + // bounds them and drops the raw set from its nested record. + ...nonBlockingFindingsForRun(), ...(signal === undefined ? {} : { signal }), }, retainedUntrackedSelections, defaultCwd); if (negotiated.transport !== undefined) { @@ -12529,7 +12545,7 @@ function createGentleAiExtensionForTesting( // but emits no nested tool_call/tool_result/execution events. SDK // accounting sees only the outer call; internal facade calls have // no additional budget/timing events. Returned metrics describe them. - const result = await tools[name].execute(toolCallId, args as never, signal, onUpdate, ctx); + const result = await reviewRunScope.run(true, () => tools[name].execute(toolCallId, args as never, signal, onUpdate, ctx)); return result.details as Record; }, presentForecast: async (forecast) => { diff --git a/tests/review-coordinator-correction-gate.test.ts b/tests/review-coordinator-correction-gate.test.ts new file mode 100644 index 000000000..f24c44909 --- /dev/null +++ b/tests/review-coordinator-correction-gate.test.ts @@ -0,0 +1,105 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { after } from "node:test"; +import { createGentleAiExtension, __testing } from "../extensions/gentle-ai.ts"; +import { NativeReviewCliV216, type ExecFileAdapter } from "../lib/native-review-cli.ts"; + +// odd/tasks/review-one-call.md T11 (S5), L21: the run returns the correction +// fields at the top level only when the provider answered the non-blocking +// question; otherwise its correction result is what it was before T11. + +type Body = Record; +// A real contracted v2 STATUS (correction_required, correction_plan_required) +// from a locally built gentle-ai at gentle-ai#6088 (e7720e6f4), on a scratch +// lineage with one synthetic CRITICAL to correct and one synthetic WARNING; +// captured without the opt-in. Fixture text only; not real review output. +const STATUS: Body = JSON.parse("{\"schema\":\"gentle-ai.review-integration.status/v9\",\"contract\":\"gentle-ai.review-integration/v2\",\"operation\":\"review.status\",\"applicability\":\"current_target\",\"authority\":{\"version\":\"compact-v2\",\"lineage_id\":\"review-0d8b087fff7b6eed\",\"state\":\"correction_required\",\"generation\":1,\"revision\":\"sha256:a7ddf6edace20fa270507183a2a60dbedd3ab6b41f03d0462e9228c80d9c7672\"},\"action\":\"collect\",\"replayability\":\"manual_action_required\",\"frozen\":{\"tier\":\"medium\",\"original_changed_lines\":1,\"correction_budget\":1},\"target_identity\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\",\"projection\":{\"schema\":\"gentle-ai.review-integration.projection/v1\",\"kind\":\"current-changes\",\"projection\":\"workspace\",\"base_tree\":\"a0b17eb7f8eba95c76ff14a4d2cce51b601e76f2\",\"initial_review_tree\":\"1c46f89a3ac76bcd75f1b1afaef64a6dc089c6df\",\"current_candidate_tree\":\"1c46f89a3ac76bcd75f1b1afaef64a6dc089c6df\",\"paths_digest\":\"sha256:85fde7c14a0bab6e322280540f42e1a3475f31abba2454770081982d81532c0b\",\"paths\":[\"a.ts\"],\"intended_untracked\":[],\"intended_untracked_proof\":\"sha256:b97229718d4c7fe50a8892eb474ae5aa6491697bccb400e3e31dbaca4894d2f3\",\"initial_snapshot_identity\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\",\"current_snapshot_identity\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\"},\"eligible_untracked_inventory\":\"sha256:0fc9c89f9fb57a5340cb06a0d61707dbf4d16b8d18d78a55fd8c28d114b47735\",\"repair\":{\"schema\":\"gentle-ai.review-authority-repair-assessment/v1\",\"status\":\"unsupported\",\"counts\":{\"lineages\":0,\"compact_lineages\":0,\"legacy_lineages\":0,\"events\":0,\"bytes\":0,\"eligible_candidates\":0,\"unsupported_lineages\":0,\"conflicts\":0},\"supported_operations\":[\"review/complete-fix\",\"review/validate-fix\"],\"authorization_schema\":\"gentle-ai.review-repair-authorization/v1\"},\"candidates\":[],\"forecast\":{\"horizon\":\"partial\",\"steps\":[{\"step\":1,\"kind\":\"collect\",\"reason_code\":\"correction_plan_required\",\"description\":\"Correction plan required to resolve review findings\"}]},\"next_transition\":{\"kind\":\"collect\",\"reason_code\":\"correction_plan_required\",\"collect\":{\"inputs\":[{\"name\":\"correction_lines\",\"schema\":\"gentle-ai.review-correction-plan/v1\",\"capture_operation\":\"review.capture-correction-plan\",\"arguments\":[{\"name\":\"lineage\",\"value\":\"review-0d8b087fff7b6eed\",\"token\":\"--lineage=review-0d8b087fff7b6eed\"},{\"name\":\"expected-revision\",\"value\":\"sha256:358845cd62d643d9a7b747d8a96891e154f22506e2f4a495ee46bd602e73f020\",\"token\":\"--expected-revision=sha256:358845cd62d643d9a7b747d8a96891e154f22506e2f4a495ee46bd602e73f020\"},{\"name\":\"target\",\"value\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\",\"token\":\"--target=sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\"},{\"name\":\"repository-context\",\"value\":\"rctx2_16252ab07879c5e12cc36ca2c2051415bdb3af5ce6f1d55457f5cdde6d18753a\",\"token\":\"--repository-context=rctx2_16252ab07879c5e12cc36ca2c2051415bdb3af5ce6f1d55457f5cdde6d18753a\"},{\"name\":\"request-hash\",\"value\":\"sha256:71d2f500490dd020938d25102ed69bf71abfa672dbad70f13ae4e6689a8ea732\",\"token\":\"--request-hash=sha256:71d2f500490dd020938d25102ed69bf71abfa672dbad70f13ae4e6689a8ea732\"}],\"submission\":{\"operation_token\":\"capture-correction-plan\",\"argument_tokens\":[\"--lineage=review-0d8b087fff7b6eed\",\"--expected-revision=sha256:358845cd62d643d9a7b747d8a96891e154f22506e2f4a495ee46bd602e73f020\",\"--target=sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\",\"--request-hash=sha256:71d2f500490dd020938d25102ed69bf71abfa672dbad70f13ae4e6689a8ea732\",\"--repository-context=rctx2_16252ab07879c5e12cc36ca2c2051415bdb3af5ce6f1d55457f5cdde6d18753a\",\"--correction-lines={{value}}\"],\"value\":{\"slot\":\"correction_lines\",\"domain\":\"positive_correction_lines\",\"minimum\":1,\"maximum\":2,\"substitution_location\":5}}}]},\"correction_request\":{\"schema\":\"gentle-ai.review-correction-plan-request/v1\",\"request_hash\":\"sha256:71d2f500490dd020938d25102ed69bf71abfa672dbad70f13ae4e6689a8ea732\",\"lineage_id\":\"review-0d8b087fff7b6eed\",\"expected_revision\":\"sha256:358845cd62d643d9a7b747d8a96891e154f22506e2f4a495ee46bd602e73f020\",\"target_identity\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\",\"correction_budget\":2,\"fix_finding_ids\":[\"R3-001\"],\"findings\":[{\"id\":\"R3-001\",\"lens\":\"reliability\",\"location\":\"a.ts:2\",\"severity\":\"CRITICAL\",\"claim\":\"Fixture: the added export breaks the reader; not real review output.\",\"proof_refs\":[\"the changed line deterministically causes the reproduced failure\"],\"evidence\":\"the changed line deterministically causes the reproduced failure\",\"evidence_class\":\"deterministic\",\"causal_disposition\":\"introduced\"}]}},\"repository_context\":{\"capability\":\"review.opaque_repository_context\",\"handle\":\"rctx2_16252ab07879c5e12cc36ca2c2051415bdb3af5ce6f1d55457f5cdde6d18753a\",\"revision\":\"sha256:358845cd62d643d9a7b747d8a96891e154f22506e2f4a495ee46bd602e73f020\",\"target_identity\":\"sha256:83e0f87755b27e526b27f89d986374c0b1415a7851718f2ce1aa800bb68e371f\"}}"); +// The same STATUS's non_blocking_findings with --non-blocking-findings (the +// rest of the body is identical); statement treated as opaque text. +const NON_BLOCKING: Body = JSON.parse("{\"statement\":\"Every finding listed here is non-blocking: it is not a finding to correct and not an unresolved finding that stopped this review, and it needs no correction in this review. A `follow_up` finding is separate later work; a `refuted` finding needs none.\",\"findings\":[{\"id\":\"R3-W01\",\"lens\":\"reliability\",\"location\":\"a.ts:2\",\"severity\":\"WARNING\",\"disposition\":\"informational\",\"claim\":\"Fixture warning claim; not real review output.\"}]}"); +const lineage: string = STATUS.authority.lineage_id; + +const scratch = realpathSync(mkdtempSync(join(realpathSync(tmpdir()), "rdd-correction-gate-"))); +after(() => rmSync(scratch, { recursive: true, force: true })); +execFileSync("git", ["init", "-q"], { cwd: scratch }); +writeFileSync(join(scratch, "a.ts"), "export const a = 1;\n"); +const executable = join(scratch, "gentle-ai-fixture"); +writeFileSync(executable, "fixture provider; never executed"); +const mode = JSON.stringify({ schema: "gentle-ai.review-mode/v1", operation: "status", scope: "both", status: { schema: "gentle-ai.rdd-mode-status/v1", global: "on", clone_local: "", effective: "on", source: "global", revision: `sha256:${"f".repeat(64)}` } }); +const refusal = { schema: "gentle-ai.review-integration.failure/v2", contract: "gentle-ai.review-integration/v2", operation: "review.status", phase: "preflight", + code: "invalid_request", message: "The negotiated review request is invalid.", mutation_outcome: "not_started", authority_applicability: "not_evaluated", + retry_safe: true, replayability: "not_replayable", required_inputs: [], next_action: "correct_request", cause: "flag provided but not defined: -non-blocking-findings" }; + +type Provider = "answers" | "answers-empty" | "refuses" | "silent"; +/** answers: the flag adds the key; answers-empty: the key with no findings; refuses: the pinned provider's refusal; silent: accepts the flag and sends no key. */ +function host(kind: Provider) { + const statusCalls: boolean[] = []; + const adapter: ExecFileAdapter = async (request) => { + const args = request.arguments; + const reply = (body: unknown, exitCode = 0) => ({ stdout: JSON.stringify(body), stderr: exitCode === 0 ? "" : "Error: The negotiated review request is invalid. [invalid_request]\n", exitCode, signal: null, timedOut: false, outputLimitExceeded: false }); + if (args[0] === "review" && args[1] === "mode") return { ...reply(undefined), stdout: mode }; + if (args[0] !== "review" || args[1] !== "status") throw new Error(`unexpected native invocation: ${args.join(" ")}`); + const flagged = args.includes("--non-blocking-findings"); + statusCalls.push(flagged); + if (flagged && kind === "refuses") return reply(refusal, 1); + const body = structuredClone(STATUS); + if (flagged && kind === "answers") body.non_blocking_findings = structuredClone(NON_BLOCKING); + if (flagged && kind === "answers-empty") body.non_blocking_findings = { ...structuredClone(NON_BLOCKING), findings: [] }; + return reply(body); + }; + // One executable per provider kind: a refusal is remembered per binary. + const binary = join(scratch, `gentle-ai-${kind}-${Math.random().toString(36).slice(2)}`); + writeFileSync(binary, `fixture provider ${kind} ${binary}`); + const tools = new Map(); + createGentleAiExtension({ nativeReviewCli: new NativeReviewCliV216(adapter, binary, 30_000, 8 * 1024 * 1024), candidateViews: null, processEnv: { GENTLE_AI_TELEMETRY: "0" } })( + { on() {}, registerCommand() {}, registerTool: (tool: any) => tools.set(tool.name, tool) } as never); + const context = { cwd: scratch, hasUI: false, sessionManager: { getSessionId: () => "fixture-gate", getBranch: () => [], getSessionFile: () => undefined } }; + return { statusCalls, run: async () => { + try { + const result = await tools.get("gentle_review_run").execute("fixture-call", { lineageId: lineage }, undefined, undefined, context); + return result.details as Body; + } finally { __testing.setReviewHostRelayRunnerForTesting(); __testing.setReviewHostRelayGroupRunnersForTesting(); } + } }; +} + +const CORRECTION_KEYS = ["correction_findings", "correction_finding_claims", "correction_finding_claims_truncated", "correction_finding_claims_omitted"]; +const NON_BLOCKING_KEYS = ["non_blocking_findings", "non_blocking_finding_claims", "non_blocking_finding_claims_truncated", "non_blocking_finding_claims_omitted", "non_blocking_findings_omitted_count"]; +const request = STATUS.next_transition.correction_request; + +test("the provider answered: the correction fields and the non-blocking fields come back together", async () => { + const provider = host("answers"); + const details = await provider.run(); + assert.equal(details.outcome, "input-required", JSON.stringify(details).slice(0, 300)); + assert.deepEqual(provider.statusCalls, [true]); + assert.deepEqual(details.correction_findings, request.findings.map(({ id, lens, location, severity, evidence_class, causal_disposition }: Body) => ({ id, lens, location, severity, evidence_class, causal_disposition }))); + assert.deepEqual(details.correction_finding_claims, Object.fromEntries(request.findings.map((finding: Body) => [finding.id, finding.claim]))); + assert.deepEqual(details.non_blocking_findings, { statement: NON_BLOCKING.statement, findings: NON_BLOCKING.findings.map(({ claim: _claim, ...finding }: Body) => finding) }); + assert.deepEqual(details.non_blocking_finding_claims, Object.fromEntries(NON_BLOCKING.findings.map((finding: Body) => [finding.id, finding.claim]))); + assert.equal(Object.hasOwn(details.result.result, "non_blocking_findings"), false); +}); + +test("the provider answered with an empty list: the correction fields come back with the empty answer", async () => { + const provider = host("answers-empty"); + const details = await provider.run(); + assert.equal(details.outcome, "input-required"); + assert.equal(details.correction_findings.length, request.findings.length); + assert.deepEqual(Object.keys(details.correction_finding_claims), request.findings.map((finding: Body) => finding.id)); + assert.deepEqual(details.non_blocking_findings, { statement: NON_BLOCKING.statement, findings: [] }); + assert.equal(Object.hasOwn(details, "non_blocking_finding_claims"), false); +}); + +for (const kind of ["refuses", "silent"] as const) { + test(`the provider did not answer (${kind}): no top-level correction or non-blocking field; the nested record is the provider's STATUS`, async () => { + const provider = host(kind); + const details = await provider.run(); + assert.equal(details.outcome, "input-required"); + assert.deepEqual(provider.statusCalls, kind === "refuses" ? [true, false] : [true]); + for (const key of [...CORRECTION_KEYS, ...NON_BLOCKING_KEYS]) assert.equal(Object.hasOwn(details, key), false, `${kind}: ${key}`); + const { collect: _collect, ...transition } = STATUS.next_transition; + assert.deepEqual(details.result.result, { ...STATUS, next_transition: transition }, `${kind}: the relayed STATUS keeps the correction request`); + assert.deepEqual(Object.keys(details).sort(), ["forecasts", "lineage_id", "metrics", "outcome", "result", "reviewer_runs", "schema"], `${kind}: the pre-T11 result keys`); + }); +} diff --git a/tests/review-coordinator-nonblocking-scope.test.ts b/tests/review-coordinator-nonblocking-scope.test.ts new file mode 100644 index 000000000..66c64eb4d --- /dev/null +++ b/tests/review-coordinator-nonblocking-scope.test.ts @@ -0,0 +1,162 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test, { after } from "node:test"; +import { createGentleAiExtension, __testing } from "../extensions/gentle-ai.ts"; +import { NativeReviewCliV216, type ExecFileAdapter } from "../lib/native-review-cli.ts"; + +// odd/tasks/review-one-call.md T11 (S5), L19: only the run asks the provider +// for the non-blocking findings; a direct gentle_review never does, and the +// run's result carries one bounded copy of them. + +type Body = Record; +interface CapturedAuthority { lineage_id: string } +interface CapturedStatus { authority: CapturedAuthority; non_blocking_findings?: Body } +interface FixtureTextContent { text: string } +interface FixtureResult { details: Body; content: FixtureTextContent[] } +interface FixtureTool { + name: string; + execute(callId: string, parameters: Body, signal: undefined, onUpdate: undefined, context: unknown): Promise; +} +const scratch = realpathSync(mkdtempSync(join(realpathSync(tmpdir()), "rdd-nbf-scope-"))); +after(() => rmSync(scratch, { recursive: true, force: true })); +execFileSync("git", ["init", "-q"], { cwd: scratch }); +writeFileSync(join(scratch, "app.ts"), "export const value = 1;\n"); +const executable = join(scratch, "gentle-ai-fixture"); +writeFileSync(executable, "fixture provider; never executed"); + +const captured = JSON.parse(readFileSync(join(process.cwd(), "tests", "fixtures", "devbinary", "status-v9-escalated-non-blocking.captured.json"), "utf8")) as CapturedStatus; +const lineage: string = captured.authority.lineage_id; +const mode = JSON.stringify({ schema: "gentle-ai.review-mode/v1", operation: "status", scope: "both", status: { schema: "gentle-ai.rdd-mode-status/v1", global: "on", clone_local: "", effective: "on", source: "global", revision: `sha256:${"f".repeat(64)}` } }); + +/** The captured escalated STATUS with `count` non-blocking findings of about 1.2 KB each. */ +function statusBody(count: number): CapturedStatus { + const body = structuredClone(captured); + body.non_blocking_findings!.findings = Array.from({ length: count }, (_, index) => ({ + id: `R3-W${String(index).padStart(3, "0")}`, lens: "reliability", location: `app.ts:${index + 1}`, severity: "WARNING", disposition: "informational", + claim: `Fixture claim ${index}; not real review output. ${"c".repeat(1150)}`, + })); + return body; +} + +/** A provider that knows --non-blocking-findings: the key appears only when the flag is passed. */ +function provider(count: number, mutate: (set: Body) => void = () => {}) { + const statusCalls: (readonly string[])[] = []; + const adapter: ExecFileAdapter = async (request) => { + const args = request.arguments; + let stdout: string; + if (args[0] === "review" && args[1] === "mode") stdout = mode; + else if (args[0] === "review" && args[1] === "status") { + statusCalls.push(args); + const body = statusBody(count); + if (!args.includes("--non-blocking-findings")) delete body.non_blocking_findings; + else mutate(body.non_blocking_findings!); + stdout = JSON.stringify(body); + } else throw new Error(`unexpected native invocation: ${args.join(" ")}`); + return { stdout, stderr: "", exitCode: 0, signal: null, timedOut: false, outputLimitExceeded: false }; + }; + const tools = new Map(); + createGentleAiExtension({ nativeReviewCli: new NativeReviewCliV216(adapter, executable, 30_000, 64 * 1024 * 1024), candidateViews: null, processEnv: { GENTLE_AI_TELEMETRY: "0" } })( + { on() {}, registerCommand() {}, registerTool: (tool: FixtureTool) => tools.set(tool.name, tool) } as never); + const context = { cwd: scratch, hasUI: false, sessionManager: { getSessionId: () => "fixture-scope", getBranch: () => [], getSessionFile: () => undefined } }; + const invoke = async (name: string, parameters: Body) => { + const result = await tools.get(name)!.execute("fixture-call", parameters, undefined, undefined, context); + return { details: result.details as Body, bytes: Buffer.byteLength((result.content as { text: string }[]).map((part) => part.text).join("")), content: result.content }; + }; + return { statusCalls, invoke, close: () => { __testing.setReviewHostRelayRunnerForTesting(); __testing.setReviewHostRelayGroupRunnersForTesting(); } }; +} + +test("a direct gentle_review status or inspect never asks for the non-blocking findings and relays none", async (t) => { + for (const parameters of [{ operation: "status", lineageId: lineage }, { operation: "status" }, { operation: "inspect" }]) { + const host = provider(300); + try { + const { details, content, bytes } = await host.invoke("gentle_review", parameters); + t.diagnostic(`${JSON.stringify(parameters)} content ${bytes} B`); + assert.ok(host.statusCalls.length >= 1, JSON.stringify(parameters)); + for (const args of host.statusCalls) assert.equal(args.includes("--non-blocking-findings"), false, `${JSON.stringify(parameters)}: ${args.join(" ")}`); + assert.doesNotMatch(JSON.stringify(content), /non_blocking_findings/, JSON.stringify(parameters)); + assert.doesNotMatch(JSON.stringify(details), /non_blocking_findings/, JSON.stringify(parameters)); + } finally { host.close(); } + } +}); + +test("a model-supplied attempt to opt in through tool input is refused, never sent", async () => { + for (const parameters of [{ operation: "status", lineageId: lineage, nonBlockingFindings: true }, { operation: "status", lineageId: lineage, input: JSON.stringify({ nonBlockingFindings: true }) }]) { + const host = provider(1); + try { + const { details } = await host.invoke("gentle_review", parameters).catch((error: Error) => ({ details: { thrown: error.message } as Body })); + for (const args of host.statusCalls) assert.equal(args.includes("--non-blocking-findings"), false, JSON.stringify(parameters)); + assert.doesNotMatch(JSON.stringify(details), /"non_blocking_findings"/, JSON.stringify(parameters)); + } finally { host.close(); } + } +}); + +const RUN_CEILING = 32 * 1024; + +test("the run asks for them and returns one bounded copy: the nested record omits the raw set, the list and claims are capped", async (t) => { + const host = provider(300); + try { + const { details, bytes } = await host.invoke("gentle_review_run", { lineageId: lineage }); + t.diagnostic(`run content ${bytes} B with 300 findings`); + assert.equal(details.outcome, "stopped", JSON.stringify(details).slice(0, 400)); + assert.ok(host.statusCalls.length >= 1); + for (const args of host.statusCalls) assert.equal(args.includes("--non-blocking-findings"), true, args.join(" ")); + assert.ok(bytes <= RUN_CEILING, `run content ${bytes} B exceeds ${RUN_CEILING} B`); + assert.equal(Object.hasOwn(details.result.result, "non_blocking_findings"), false, "the nested provider record omits the raw set"); + const { non_blocking_findings: _raw, ...rest } = statusBody(0); + for (const key of Object.keys(rest)) assert.deepEqual(details.result.result[key], rest[key], `nested ${key} unchanged`); + const listed = details.non_blocking_findings.findings as Body[]; + assert.equal(details.non_blocking_findings.statement, captured.non_blocking_findings.statement); + assert.ok(Buffer.byteLength(JSON.stringify(listed)) <= 16 * 1024, "the listed findings stay within 16 KiB"); + const expected = statusBody(300).non_blocking_findings.findings.map(({ claim: _claim, ...finding }: Body) => finding); + assert.deepEqual(listed, expected.slice(0, listed.length), "provider order and fields, unchanged"); + assert.equal(details.non_blocking_findings_omitted_count, 300 - listed.length); + assert.ok(listed.length > 0 && listed.length < 300); + const claims = details.non_blocking_finding_claims as Body; + assert.ok(Buffer.byteLength(Object.values(claims).join("")) <= 8192); + assert.deepEqual(Object.keys(claims), listed.slice(0, Object.keys(claims).length).map((finding) => finding.id), "claims only for listed ids, in order"); + assert.deepEqual(details.non_blocking_finding_claims_omitted, listed.slice(Object.keys(claims).length).map((finding) => finding.id), "the claim limit names the listed ids it left out"); + } finally { host.close(); } +}); + +test("a small set is returned whole with no omitted count", async () => { + const host = provider(3); + try { + const { details } = await host.invoke("gentle_review_run", { lineageId: lineage }); + assert.equal(details.outcome, "stopped"); + assert.equal(details.non_blocking_findings.findings.length, 3); + assert.equal(Object.hasOwn(details, "non_blocking_findings_omitted_count"), false); + assert.equal(Object.keys(details.non_blocking_finding_claims).length, 3); + assert.equal(Object.hasOwn(details.result.result, "non_blocking_findings"), false); + } finally { host.close(); } +}); + +test("a malformed set fails the strict STATUS decode and the run ends blocked; only whitespace-only strings pass the decoder and are dropped by the run", async () => { + const malformed: [string, (set: Body) => void][] = [ + ["disposition outside the set", (set) => { set.findings[0].disposition = "bogus"; }], + ["claim missing", (set) => { delete set.findings[0].claim; }], + ["extra key", (set) => { set.findings[0].proof_refs = []; }], + ["empty statement", (set) => { set.statement = ""; }], + ["findings null", (set) => { set.findings = null; }], + ["severity not a string", (set) => { set.findings[0].severity = 5; }], + ]; + for (const [label, mutate] of malformed) { + const host = provider(2, mutate); + try { + const { details } = await host.invoke("gentle_review_run", { lineageId: lineage }); + assert.equal(details.outcome, "blocked", label); + for (const key of ["non_blocking_findings", "non_blocking_finding_claims"]) assert.equal(Object.hasOwn(details, key), false, `${label}: ${key}`); + } finally { host.close(); } + } + for (const [label, mutate] of [["whitespace-only id", (set: Body) => { set.findings[0].id = " "; }], ["whitespace-only statement", (set: Body) => { set.statement = " "; }]] as const) { + const host = provider(2, mutate); + try { + const { details } = await host.invoke("gentle_review_run", { lineageId: lineage }); + assert.equal(details.outcome, "stopped", label); + for (const key of ["non_blocking_findings", "non_blocking_finding_claims", "non_blocking_findings_omitted_count"]) assert.equal(Object.hasOwn(details, key), false, `${label}: ${key}`); + assert.equal(Object.hasOwn(details.result.result, "non_blocking_findings"), false, `${label}: the raw set is still left out`); + } finally { host.close(); } + } +}); diff --git a/tests/review-one-call-reviewer-requests.test.ts b/tests/review-one-call-reviewer-requests.test.ts index 9af1e8add..f285aaaf5 100644 --- a/tests/review-one-call-reviewer-requests.test.ts +++ b/tests/review-one-call-reviewer-requests.test.ts @@ -187,7 +187,15 @@ for (const scenario of ["single", "group", "refuter"] as const) { const modeReads = run.providerOperations.filter(({ operation }) => operation === "review-mode"); assert.deepEqual(modeReads, run.steps.map(() => ({ operation: "review-mode", request: { cwd: repo, operation: "status", signal: "" } })), `${label}: one switch read per facade step`); assert.equal(direct.providerOperations.some(({ operation }) => operation === "review-mode"), false); - assert.deepEqual(run.providerOperations.filter(({ operation }) => operation !== "review-mode"), direct.providerOperations, `${label}: provider operations and arguments`); + // L19: only STATUS reads made for the run ask for the provider's + // non-blocking findings (gentle-ai#6088); a parent's direct calls never do. + // That opt-in reaches no reviewer; every other argument is identical. + const optedIn = run.providerOperations.filter(({ operation, request }) => operation === "target-status" && (request as Record).nonBlockingFindings === true); + assert.ok(optedIn.length > 0, `${label}: the run's STATUS asks for the non-blocking findings`); + assert.equal(direct.providerOperations.some(({ request }) => Object.hasOwn(request as object, "nonBlockingFindings")), false, `${label}: direct calls never ask`); + const withoutOptIn = run.providerOperations.filter(({ operation }) => operation !== "review-mode") + .map(({ operation, request }) => ({ operation, request: (({ nonBlockingFindings: _optIn, ...rest }) => rest)(request as Record) })); + assert.deepEqual(withoutOptIn, direct.providerOperations, `${label}: provider operations and arguments`); } t.diagnostic(`relay request fields: ${fieldsOf(direct.launches.map((launch) => launch.relayRequest)).join(", ")}`); t.diagnostic(`prepared request fields: ${fieldsOf(direct.launches.map((launch) => launch.preparedRequest)).join(", ")}`); diff --git a/tests/support/review-coordinator-runtime.ts b/tests/support/review-coordinator-runtime.ts index 93f161cbf..5cb21ed39 100644 --- a/tests/support/review-coordinator-runtime.ts +++ b/tests/support/review-coordinator-runtime.ts @@ -15,6 +15,8 @@ export interface CoordinatorRuntimeOptions { advisoryFindings?: unknown; /** Provider `reviewer_results` carried by the approved last-event closure. */ reviewerResults?: unknown; + /** Provider `findings` of the STATUS correction request (scenario "correction"). */ + correctionFindings?: unknown; /** Effective native review mode per read; the fixture defaults to on. */ mode?: (read: number, nativeCalls: readonly string[]) => "on" | "off"; /** @@ -31,8 +33,11 @@ export interface CoordinatorRuntimeOptions { * and fails. "provider-role": the "refuter" group, after which STATUS offers * an older provider's self-contained refuter vector (--execute=true) whose * capture returns the provider-role artifact, not a closure. + * "correction": STATUS asks for a correction plan (correction_plan_required) + * with a correction request carrying `correctionFindings`. "escalated": + * STATUS stops (native_stop_required) with the provider's escalation. */ - scenario?: "single" | "group" | "refuter" | "refuter-reoffers-group" | "untracked-selection" | "provider-role"; + scenario?: "single" | "group" | "refuter" | "refuter-reoffers-group" | "untracked-selection" | "provider-role" | "correction" | "escalated"; /** Passed to every tool execute as its abort signal. */ signal?: AbortSignal; /** Called right after a reviewer submission made the fixture's closure approved. */ @@ -117,6 +122,15 @@ export function createCoordinatorRuntime(cwd: string, variant: "baseline" | "coo const startReady = { ...reviewing, authority: undefined, applicability: "unrelated", action: "start", nextTransition: { kind: "execute", reasonCode: "start_required", execute: { operation: "review.start", command: "gentle-ai review start --fixture-only", arguments: [], preconditions: [] } }, raw: { schema: "gentle-ai.review-integration.status/v7", target_identity: target, next_transition: { kind: "execute", reason_code: "start_required", execute: { operation: "review.start" } } } } as unknown as ReviewStatusV3; + // A STATUS asking for the correction plan, with the provider's correction request. + const correctionRequest = { schema: "gentle-ai.review-correction-plan-request/v1", request_hash: `sha256:${"c".repeat(64)}`, lineage_id: lineage, expected_revision: revision, + target_identity: target, correction_budget: 10, fix_finding_ids: ["B1"], findings: options.correctionFindings }; + const correcting = { ...reviewing, nextTransition: { kind: "collect", reasonCode: "correction_plan_required", collect: reviewing.nextTransition!.collect }, + raw: { ...raw, next_transition: { kind: "collect", reason_code: "correction_plan_required", correction_request: correctionRequest } } } as unknown as ReviewStatusV3; + // A STATUS stop on an escalated review, with the provider's escalation diagnostic. + const escalation = { cause: "unknown_causality", finding_ids: ["B1"], refuter_outcomes: [{ finding_id: "B1", outcome: "corroborated", proof: "Fixture refuter proof; not real review output." }] }; + const escalated = { ...reviewing, nextTransition: { kind: "stop", reasonCode: "native_stop_required" }, + raw: { ...raw, authority: { ...raw.authority, state: "escalated" }, escalation, next_transition: { kind: "stop", reason_code: "native_stop_required" } } } as unknown as ReviewStatusV3; const startRequests: Body[] = []; const approvedClosure = (operation: string) => JSON.stringify({ schema: "gentle-ai.review-last-event-closure/v1", operation, lineage_id: lineage, state: "approved", action: "fixture approval; not real authority", store_revision: revision, ...(options.advisoryFindings === undefined ? {} : { advisory_findings: options.advisoryFindings }), @@ -144,6 +158,8 @@ export function createCoordinatorRuntime(cwd: string, variant: "baseline" | "coo options.onStatus?.(state); if (state === "approved") return approved; if (scenario === "untracked-selection") return request.intendedUntrackedSelection === undefined ? selecting : startReady; + if (scenario === "correction") return correcting; + if (scenario === "escalated") return escalated; if (scenario === "single") return reviewing; if (state === "refuting") { if (scenario === "provider-role") return roleVector; From bb1f7f56badf6f5bc29be450263b096b204fd5c9 Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Thu, 12 Nov 2026 17:03:48 +0100 Subject: [PATCH 3/4] test(review): type scope fixtures in their introduction slice --- tests/review-coordinator-nonblocking-scope.test.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/review-coordinator-nonblocking-scope.test.ts b/tests/review-coordinator-nonblocking-scope.test.ts index 66c64eb4d..aa83ed241 100644 --- a/tests/review-coordinator-nonblocking-scope.test.ts +++ b/tests/review-coordinator-nonblocking-scope.test.ts @@ -13,9 +13,16 @@ import { NativeReviewCliV216, type ExecFileAdapter } from "../lib/native-review- type Body = Record; interface CapturedAuthority { lineage_id: string } -interface CapturedStatus { authority: CapturedAuthority; non_blocking_findings?: Body } +interface FixtureFindings extends Body { statement: string; findings: Body[] } +interface CapturedStatus extends Body { authority: CapturedAuthority; non_blocking_findings?: FixtureFindings } +interface FixtureEnvelope { result: Body } +interface FixtureDetails extends Body { + result: FixtureEnvelope; + non_blocking_findings: FixtureFindings; + non_blocking_finding_claims: Body; +} interface FixtureTextContent { text: string } -interface FixtureResult { details: Body; content: FixtureTextContent[] } +interface FixtureResult { details: FixtureDetails; content: FixtureTextContent[] } interface FixtureTool { name: string; execute(callId: string, parameters: Body, signal: undefined, onUpdate: undefined, context: unknown): Promise; @@ -63,7 +70,7 @@ function provider(count: number, mutate: (set: Body) => void = () => {}) { const context = { cwd: scratch, hasUI: false, sessionManager: { getSessionId: () => "fixture-scope", getBranch: () => [], getSessionFile: () => undefined } }; const invoke = async (name: string, parameters: Body) => { const result = await tools.get(name)!.execute("fixture-call", parameters, undefined, undefined, context); - return { details: result.details as Body, bytes: Buffer.byteLength((result.content as { text: string }[]).map((part) => part.text).join("")), content: result.content }; + return { details: result.details, bytes: Buffer.byteLength((result.content as { text: string }[]).map((part) => part.text).join("")), content: result.content }; }; return { statusCalls, invoke, close: () => { __testing.setReviewHostRelayRunnerForTesting(); __testing.setReviewHostRelayGroupRunnersForTesting(); } }; } From 40c1545cd7bdf5149c6f565c0372998ee760611e Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Thu, 12 Nov 2026 21:28:25 +0100 Subject: [PATCH 4/4] docs(odd): record verified result delivery and scope integration --- odd/tasks/nonblocking-delivery-slices.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/odd/tasks/nonblocking-delivery-slices.md b/odd/tasks/nonblocking-delivery-slices.md index 22f71bb2d..c4590fad5 100644 --- a/odd/tasks/nonblocking-delivery-slices.md +++ b/odd/tasks/nonblocking-delivery-slices.md @@ -10,7 +10,7 @@ S3. Preserve run-only opt-in, direct/capture STATUS parity, bounded findings/cla ## Tasks T1 | S1-S3 | worker scope | done | run-only opt-in, bounds/isolation and correction-gate tests; integrated 46/46 PASS | commit: 55f3b664f T2 | S1-S3 | worker result | done | bounded coordinator result; RED 5 failures, GREEN 7/7 plus 27/27 existing tests; exact reference code | commit: 6e5825d28 -T3 | S1-S3 | parent | in_progress | earlier integrated proof retained; in-flight refusal race RED reproduced, generated fix and134 native tests GREEN; independent concurrency verification pending | commit: 332224982 + e9e9d0aa3 + b1971cb64 (race correction) +T3 | S1-S3 | parent | in_progress | refusal and bounded-result corrections independently verified and delivered; remaining integration must explain nested statement_truncated, then receive fresh checks | commit: 332224982 + e9e9d0aa3 + b1971cb64 (race correction) T4 | S1-S3 | parent | pending | publish <=400-line slices after provider prerequisite and required verification | commit: pending ## Log @@ -35,3 +35,7 @@ L12. Independent verifier26: baseline5PASS/5FAIL on each TS/runtime; candidate10 L14. #3212 integrated current main atcd34b10bc; public result files byte-identical to prior candidate,18tests/typecheck291PASS and297allCI PASS. Fresh302merge gate stopped on unresolved review4555320381. Parent public probe reproduced1MiB statement with emptyfindings, stopped outcome, exactlyoneSTATUS call and unchanged input;16KiB cap covers only list. Parent implementation decision under standing authorization: share1024UTF8-byte claim cap for statement, keep whole codepoints and short text unchanged, emit nested statement_truncated:true only on truncation. This is host-derived metadata, never a provider-schema/authority change. Explorer2c confirms nested marker survives coordinator/tool serialization; subsequent integration guidance/tests and docs slice must explain it. No additional top-level scope key needed. Regression/fix/independent verification and delivery pending. L15. Statement regression RED9PASS/3FAIL (1048576vs1024,1028vs1024,1027vs1023). Reused unchanged UTF8 helper/1024claim cap and added nested host-only statement_truncated:true on cuts; docs explain disclosure/provider-schema separation. Parent30654/54coordinator/native testsPASS, typecheck/diffPASS; inputs/calls/outcomes/claims preserved across stop/correction and empty/nonempty lists. Risk3 public result contract: independent proof and freshCI pending; later integration guidance must explain the marker. + +L16. #3211 merged5e592cf7a and retained probes#3219 merged20d147cee. #3212 statementfixe4ca47371: independent60/60 with30 differential compatibility cases identical; parent54/typecheckPASS,317allCI and319freshhead/320lines/allchecks/threadsresolved. Mergedca9993e3421029539168fcbd44f7dabb7bb61659 normally, no admin/force. Threadwasalreadyresolved315, no duplicate replyposted. #3213 integrated currentmain in nbf-scope-current-main; initial patch-equality check againstbb1f7f56's firstparent failed; no equality/SDK validation was established by that command. Parent322 was mistakenly launched despite failed setup; preserve its actual result and diagnose the correct comparison base before publishing. No new behavior or benchmark claim; checks/delivery pending. Remaining integration/docs must disclose statement_truncated. + +L17. Parent setup error: bb1's firstparent is only the fixture-typing commit, not original scope base; comparison against actual f091a0da3 proves all five scope files' added/removed patch lines identical to integrated main-relative delta, and coordinator unchanged from correctedmain. Two SDK lookup assumptions failed before link; actual importer pins1.5.2 despite >=1.2.0 manifest range. Parent32224/24PASS but typecheck path missing;323 validated lock/installed1.5.2, linked ignored local dependencies and typecheck/diffPASS. No source correction or dependency install. These setup failures remain recorded, not hidden by later PASS.