Skip to content

fix(review): support untracked assess declarations #1266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Alan-TheGentleman merged 1 commit into from
Sep 20, 2026
Merged

fix(review): support untracked assess declarations #1266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 14 additions & 4 deletions extensions/gentle-ai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ import {
nativeReviewRecoverAuthorization,
normalizeNativeReviewCwd,
NativeReviewCliError,
nativeUntrackedSelection,
NativeReviewConsentBindingError,
NativeReviewConsentRequiredError,
NativeReviewIntegrationError,
Expand Down Expand Up @@ -1023,18 +1024,26 @@ async function resolveReviewAssessmentPlan(

let assessment: ReviewAssessmentV1 | undefined;
let unassessableDetail: string | undefined;
let unassessableCode = "native-assess-unavailable";
if (nativeReviewCli?.assess === undefined) {
unassessableDetail = "native review assess is unavailable: the installed gentle-ai binary does not expose the assess command.";
} else {
try {
const request: NativeReviewAssessRequest = {
cwd,
...nativeUntrackedSelection(input),
...(input.baseRef === undefined ? {} : { baseRef: input.baseRef, committedOnly: true as const }),
...(signal === undefined ? {} : { signal }),
};
assessment = await nativeReviewCli.assess(request);
} catch (error) {
unassessableDetail = `native review assess failed: ${error instanceof Error ? error.message : String(error)}`;
const nativeError = asNativeReviewCliError(error);
unassessableCode = nativeError?.code ?? unassessableCode;
// Only the sanitized process surface may supply native evidence.
// Arbitrary thrown messages can contain argv or environment values.
unassessableDetail = nativeError?.diagnostics.stderr
? `native review assess failed: ${nativeError.diagnostics.stderr}`
: "native review assess failed; no sanitized stderr diagnostic is available.";
}
}

Expand All @@ -1050,7 +1059,7 @@ async function resolveReviewAssessmentPlan(
return {
schema: "gentle-pi.review-assessment-plan/v1",
risk,
reasons: assessment?.reasons ?? (unassessableDetail === undefined ? [] : [{ code: "native-assess-unavailable", path: "", detail: unassessableDetail }]),
reasons: assessment?.reasons ?? (unassessableDetail === undefined ? [] : [{ code: unassessableCode, path: "", detail: unassessableDetail }]),
changedPaths: assessment?.changedPaths ?? 0,
changedLines: assessment?.changedLines ?? 0,
candidate: assessment === undefined ? null : { kind: assessment.candidate.kind, baseRef: assessment.candidate.baseRef },
Expand Down Expand Up @@ -4826,7 +4835,7 @@ interface ReviewScopeParameters {
// as `gentle_review` operation `assess` (not a dedicated tool), taking its
// optional fields through the controller's existing generic `input` JSON
// string, exactly like START's `{"mode":...,"baseRef":...}`.
interface ReviewAssessInput {
interface ReviewAssessInput extends Pick<NativeReviewAssessRequest, "untrackedScope" | "expectedUntrackedInventory" | "intendedUntracked"> {
baseRef?: string;
committedOnly?: boolean;
writerModelId?: string;
Expand All @@ -4845,7 +4854,7 @@ function isNativeReviewOutcome(value: unknown): value is NativeReviewOutcome {
function parseReviewAssessInput(operation: ReviewControllerOperation, raw: string | undefined): ReviewAssessInput {
if (raw === undefined) return {};
const value = parseControllerJson(raw, operation);
const allowed = new Set(["baseRef", "committedOnly", "writerModelId", "writerEffort", "nativeReviewOutcome"]);
const allowed = new Set(["baseRef", "committedOnly", "writerModelId", "writerEffort", "nativeReviewOutcome", "untrackedScope", "expectedUntrackedInventory", "intendedUntracked"]);
const unexpected = Object.keys(value).find((key) => !allowed.has(key));
if (unexpected !== undefined) throw new Error(`Review controller ${operation} input does not accept ${unexpected}`);
const { baseRef, committedOnly, writerModelId, writerEffort, nativeReviewOutcome } = value;
Expand All @@ -4859,6 +4868,7 @@ function parseReviewAssessInput(operation: ReviewControllerOperation, raw: strin
return {
...(baseRef === undefined ? {} : { baseRef: baseRef as string }),
...(committedOnly === undefined ? {} : { committedOnly: committedOnly as boolean }),
...nativeUntrackedSelection(value),
...(writerModelId === undefined ? {} : { writerModelId: writerModelId as string }),
...(writerEffort === undefined ? {} : { writerEffort: writerEffort as string }),
...(nativeReviewOutcome === undefined ? {} : { nativeReviewOutcome: nativeReviewOutcome as NativeReviewOutcome }),
Expand Down
39 changes: 29 additions & 10 deletions lib/native-review-cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export interface NativeReviewModeRequest {
// explicit `committedOnly` acknowledgement, exactly like Native START's
// baseRef/committedOnly pairing, because both select a committed range
// instead of the ambient working tree.
export interface NativeReviewAssessRequest {
export interface NativeReviewAssessRequest extends NativeUntrackedSelection {
cwd: string;
baseRef?: string;
committedOnly?: boolean;
Expand Down Expand Up @@ -705,7 +705,11 @@ function isNativeUntrackedPath(value: unknown): value is string {
&& value.split("/").every((segment) => segment.length > 0 && segment !== "." && segment !== "..");
}

function nativeUntrackedSelection(request: NativeUntrackedSelectionRequest): NativeUntrackedSelection {
export function nativeUntrackedSelection(request: {
untrackedScope?: unknown;
expectedUntrackedInventory?: unknown;
intendedUntracked?: unknown;
}): NativeUntrackedSelection {
const { untrackedScope, expectedUntrackedInventory, intendedUntracked } = request;
const declared = untrackedScope !== undefined || expectedUntrackedInventory !== undefined || intendedUntracked !== undefined;
if (!declared) return {};
Expand All @@ -716,16 +720,19 @@ function nativeUntrackedSelection(request: NativeUntrackedSelectionRequest): Nat
) {
throw new TypeError("Native untracked selection must declare one scope, one inventory digest, and unique repository-relative paths");
}
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.EXCLUDE && (intendedUntracked?.length ?? 0) > 0) {
// The guard above establishes the array and element types for both typed
// native requests and untyped facade input.
const paths = intendedUntracked as readonly string[] | undefined;
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.EXCLUDE && (paths?.length ?? 0) > 0) {
throw new TypeError("Native exclude untracked selection cannot include paths");
}
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.SELECT && (intendedUntracked?.length ?? 0) === 0) {
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.SELECT && (paths?.length ?? 0) === 0) {
throw new TypeError("Native select untracked selection requires at least one path");
}
return {
untrackedScope,
expectedUntrackedInventory,
intendedUntracked: intendedUntracked === undefined ? undefined : [...intendedUntracked],
intendedUntracked: paths === undefined ? undefined : [...paths],
};
}

Expand Down Expand Up @@ -1102,8 +1109,19 @@ function decodeSelectedLenses(value: unknown, riskLevel: string, lensesRequired:
function enumString(value: unknown, allowed: readonly string[]): string { const parsed = stringValue(value); if (!allowed.includes(parsed)) throw new Error("unsupported enum"); return parsed; }
const NATIVE_DIAGNOSTIC_TEXT_LIMIT = 4_096;

function sanitizeNativeDiagnosticText(value: string, limit = NATIVE_DIAGNOSTIC_TEXT_LIMIT): string {
const normalized = value
function sanitizeNativeDiagnosticText(value: string, limit = NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation?: NativeReviewOperation): string {
// ASSESS diagnostics are projected into a public verification plan. Retain
// native guidance, not local paths or environment assignment values.
const input = operation === NATIVE_REVIEW_OPERATION.ASSESS
? value
.replace(/(?<![\w-])[a-z_][a-z0-9_]*=(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s]+)/gi, "[REDACTED ENV]")
.replace(/--(?:password|token|secret|authorization|cookie|private[_-]key|access[_-]token|[a-z0-9_-]+[_-]token|api[_-]?key)[ \t]+(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s]+)/gi, "[REDACTED CREDENTIAL]")
// Quoted paths have a clear boundary. For an unquoted path, the
// remaining line is ambiguous (spaces may belong to the filename).
// Redact that suffix rather than leak trailing path components.
.replace(/"(?:[A-Za-z]:[\\/]|\/)[^"\r\n]*"|'(?:[A-Za-z]:[\\/]|\/)[^'\r\n]*'|(?:[A-Za-z]:[\\/]|\/)[^\r\n]*/g, "[REDACTED PATH]")
: value;
const normalized = input
.replace(/\x1b](?:[^\x07\x1b]|\x1b(?!\\))*?(?:\x07|\x1b\\)/g, "[REDACTED CONTROL]")
.replace(/\x1b[PX^_][\s\S]*?\x1b\\/g, "[REDACTED CONTROL]")
.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, "[REDACTED CONTROL]")
Expand Down Expand Up @@ -1137,7 +1155,7 @@ export function sanitizeForeignNativeReviewDiagnostics(value: unknown): NativeRe
timed_out: booleanValue(raw.timed_out),
output_limit_exceeded: booleanValue(raw.output_limit_exceeded),
...(maxBufferBytes === undefined ? {} : { max_buffer_bytes: maxBufferBytes, configuration_hint: configurationHint! }),
...(raw.stderr === undefined ? {} : { stderr: sanitizeNativeDiagnosticText(stringValue(raw.stderr)) }),
...(raw.stderr === undefined ? {} : { stderr: sanitizeNativeDiagnosticText(stringValue(raw.stderr), NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation) }),
};
} catch { return undefined; }
}
Expand All @@ -1154,7 +1172,7 @@ function nativeProcessDiagnostics(operation: NativeReviewOperation, code: Native
...(code === NATIVE_REVIEW_ERROR_CODE.OUTPUT_LIMIT && maxBufferBytes !== undefined
? { max_buffer_bytes: maxBufferBytes, configuration_hint: NATIVE_REVIEW_MAX_BUFFER_CONFIGURATION_HINT }
: {}),
...(result?.stderr.trim() ? { stderr: sanitizeNativeDiagnosticText(result.stderr) } : {}),
...(result?.stderr.trim() ? { stderr: sanitizeNativeDiagnosticText(result.stderr, NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation) } : {}),
};
}

Expand Down Expand Up @@ -2535,14 +2553,15 @@ export class NativeReviewCliV216 implements NativeReviewCli {
// rejects -- callers (the `gentle_review` tool's `assess` operation) fail
// closed to `high`.
async assess(request: NativeReviewAssessRequest): Promise<ReviewAssessmentV1> {
const selection = nativeUntrackedSelection(request);
if (request.baseRef !== undefined && !isCanonicalProcessString(request.baseRef)) throw new TypeError("Native ASSESS baseRef must be a non-empty, trimmed, NUL-free string");
if (request.baseRef !== undefined && request.committedOnly !== true) throw new TypeError("Native ASSESS baseRef requires explicit committedOnly acknowledgement");
if (request.baseRef === undefined && request.committedOnly !== undefined) throw new TypeError("Native ASSESS committedOnly requires an explicit baseRef");
const cwd = await canonicalNativeReviewCwd(request.cwd);
const execution = await this.invoke(
NATIVE_REVIEW_OPERATION.ASSESS,
cwd,
["review", "assess", "--cwd", cwd, ...(request.baseRef === undefined ? [] : ["--base-ref", request.baseRef, "--committed-only"]), "--json"],
["review", "assess", "--cwd", cwd, ...(request.baseRef === undefined ? [] : ["--base-ref", request.baseRef, "--committed-only"]), ...nativeUntrackedSelectionArguments(selection), "--json"],
false,
request.signal,
this.executablePath(NATIVE_REVIEW_OPERATION.ASSESS, false),
Expand Down
37 changes: 28 additions & 9 deletions runtime/native-review-cli.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,11 @@ function isNativeUntrackedPath(value ) {
&& value.split("/").every((segment) => segment.length > 0 && segment !== "." && segment !== "..");
}

function nativeUntrackedSelection(request ) {
export function nativeUntrackedSelection(request



) {
const { untrackedScope, expectedUntrackedInventory, intendedUntracked } = request;
const declared = untrackedScope !== undefined || expectedUntrackedInventory !== undefined || intendedUntracked !== undefined;
if (!declared) return {};
Expand All @@ -717,16 +721,19 @@ function nativeUntrackedSelection(request )
) {
throw new TypeError("Native untracked selection must declare one scope, one inventory digest, and unique repository-relative paths");
}
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.EXCLUDE && (intendedUntracked?.length ?? 0) > 0) {
// The guard above establishes the array and element types for both typed
// native requests and untyped facade input.
const paths = intendedUntracked ;
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.EXCLUDE && (paths?.length ?? 0) > 0) {
throw new TypeError("Native exclude untracked selection cannot include paths");
}
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.SELECT && (intendedUntracked?.length ?? 0) === 0) {
if (untrackedScope === NATIVE_UNTRACKED_SCOPE.SELECT && (paths?.length ?? 0) === 0) {
throw new TypeError("Native select untracked selection requires at least one path");
}
return {
untrackedScope,
expectedUntrackedInventory,
intendedUntracked: intendedUntracked === undefined ? undefined : [...intendedUntracked],
intendedUntracked: paths === undefined ? undefined : [...paths],
};
}

Expand Down Expand Up @@ -1103,8 +1110,19 @@ function decodeSelectedLenses(value , riskLevel , lensesRequired
function enumString(value , allowed ) { const parsed = stringValue(value); if (!allowed.includes(parsed)) throw new Error("unsupported enum"); return parsed; }
const NATIVE_DIAGNOSTIC_TEXT_LIMIT = 4_096;

function sanitizeNativeDiagnosticText(value , limit = NATIVE_DIAGNOSTIC_TEXT_LIMIT) {
const normalized = value
function sanitizeNativeDiagnosticText(value , limit = NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation ) {
// ASSESS diagnostics are projected into a public verification plan. Retain
// native guidance, not local paths or environment assignment values.
const input = operation === NATIVE_REVIEW_OPERATION.ASSESS
? value
.replace(/(?<![\w-])[a-z_][a-z0-9_]*=(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s]+)/gi, "[REDACTED ENV]")
.replace(/--(?:password|token|secret|authorization|cookie|private[_-]key|access[_-]token|[a-z0-9_-]+[_-]token|api[_-]?key)[ \t]+(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s]+)/gi, "[REDACTED CREDENTIAL]")
// Quoted paths have a clear boundary. For an unquoted path, the
// remaining line is ambiguous (spaces may belong to the filename).
// Redact that suffix rather than leak trailing path components.
.replace(/"(?:[A-Za-z]:[\\/]|\/)[^"\r\n]*"|'(?:[A-Za-z]:[\\/]|\/)[^'\r\n]*'|(?:[A-Za-z]:[\\/]|\/)[^\r\n]*/g, "[REDACTED PATH]")
: value;
const normalized = input
.replace(/\x1b](?:[^\x07\x1b]|\x1b(?!\\))*?(?:\x07|\x1b\\)/g, "[REDACTED CONTROL]")
.replace(/\x1b[PX^_][\s\S]*?\x1b\\/g, "[REDACTED CONTROL]")
.replace(/\x1b\[[0-?]*[ -/]*[@-~]/g, "[REDACTED CONTROL]")
Expand Down Expand Up @@ -1138,7 +1156,7 @@ export function sanitizeForeignNativeReviewDiagnostics(value )
timed_out: booleanValue(raw.timed_out),
output_limit_exceeded: booleanValue(raw.output_limit_exceeded),
...(maxBufferBytes === undefined ? {} : { max_buffer_bytes: maxBufferBytes, configuration_hint: configurationHint }),
...(raw.stderr === undefined ? {} : { stderr: sanitizeNativeDiagnosticText(stringValue(raw.stderr)) }),
...(raw.stderr === undefined ? {} : { stderr: sanitizeNativeDiagnosticText(stringValue(raw.stderr), NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation) }),
};
} catch { return undefined; }
}
Expand All @@ -1155,7 +1173,7 @@ function nativeProcessDiagnostics(operation , code
...(code === NATIVE_REVIEW_ERROR_CODE.OUTPUT_LIMIT && maxBufferBytes !== undefined
? { max_buffer_bytes: maxBufferBytes, configuration_hint: NATIVE_REVIEW_MAX_BUFFER_CONFIGURATION_HINT }
: {}),
...(result?.stderr.trim() ? { stderr: sanitizeNativeDiagnosticText(result.stderr) } : {}),
...(result?.stderr.trim() ? { stderr: sanitizeNativeDiagnosticText(result.stderr, NATIVE_DIAGNOSTIC_TEXT_LIMIT, operation) } : {}),
};
}

Expand Down Expand Up @@ -2536,14 +2554,15 @@ export class NativeReviewCliV216 {
// rejects -- callers (the `gentle_review` tool's `assess` operation) fail
// closed to `high`.
async assess(request ) {
const selection = nativeUntrackedSelection(request);
if (request.baseRef !== undefined && !isCanonicalProcessString(request.baseRef)) throw new TypeError("Native ASSESS baseRef must be a non-empty, trimmed, NUL-free string");
if (request.baseRef !== undefined && request.committedOnly !== true) throw new TypeError("Native ASSESS baseRef requires explicit committedOnly acknowledgement");
if (request.baseRef === undefined && request.committedOnly !== undefined) throw new TypeError("Native ASSESS committedOnly requires an explicit baseRef");
const cwd = await canonicalNativeReviewCwd(request.cwd);
const execution = await this.invoke(
NATIVE_REVIEW_OPERATION.ASSESS,
cwd,
["review", "assess", "--cwd", cwd, ...(request.baseRef === undefined ? [] : ["--base-ref", request.baseRef, "--committed-only"]), "--json"],
["review", "assess", "--cwd", cwd, ...(request.baseRef === undefined ? [] : ["--base-ref", request.baseRef, "--committed-only"]), ...nativeUntrackedSelectionArguments(selection), "--json"],
false,
request.signal,
this.executablePath(NATIVE_REVIEW_OPERATION.ASSESS, false),
Expand Down
Loading
Loading