Skip to content

fix(review): name the check that refused an intended-untracked selection #942

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

Open
L4XB wants to merge 1 commit into
base: main
Choose a base branch
from
+120 −6
Open

fix(review): name the check that refused an intended-untracked selection #942

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
47 changes: 45 additions & 2 deletions extensions/gentle-ai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5015,6 +5015,49 @@ function validateNativeStartUntrackedSelection(value: Record<string, unknown>):
};
}

// gentle-pi#941: the pre-lineage selection had eight ways to be refused and one
// opaque outcome for all of them, so a caller holding provider-issued bytes
// could not tell a moved status from an ineligible path -- and the only route
// left was to send the same call again. START rejections have carried
// `reason`/`field` since they were introduced (nativeStartRejection above);
// name the failing check here the same way. The checks keep their original
// order, so which one is reported is the first that fails, exactly as the
// short-circuiting condition decided before.
const INTENDED_UNTRACKED_BINDING_FIELDS = ["target_identity", "projection", "base_tree", "candidate_tree"] as const;

function intendedUntrackedSelectionRejection(
input: ReviewCollectInputV3 | undefined,
canonicalBinding: string,
status: ReviewStatusV3,
eligible: unknown,
selected: NativeStartUntrackedSelection,
): { readonly reason: string; readonly field?: string } | undefined {
if (input === undefined) return { reason: "selection-input-absent" };
if (canonicalReviewCaptureBinding(input) !== canonicalBinding) return { reason: "binding-mismatch" };
const bound: Record<(typeof INTENDED_UNTRACKED_BINDING_FIELDS)[number], string> = {
target_identity: status.targetIdentity,
projection: status.projection.projection,
base_tree: status.projection.baseTree,
candidate_tree: status.projection.currentCandidateTree,
};
// A field that no longer matches the live status means the binding was
// issued against a different target, projection or tree -- the selection is
// stale rather than wrong, and a fresh inspect is the forward route.
for (const field of INTENDED_UNTRACKED_BINDING_FIELDS) {
if (exactCollectArgument(input, field) !== bound[field]) return { reason: "status-field-mismatch", field };
}
// The eligible list is the provider's own `eligible_paths_json`; unreadable
// here means it was absent, duplicated, or not valid JSON in the status.
if (!Array.isArray(eligible)) return { reason: "eligible-paths-unreadable" };
if (selected.reason !== undefined) return { reason: selected.reason };
// Echoes back a path the caller itself submitted -- no information the
// caller did not already have, and with a large inventory it is the only
// way to tell which entry of a long selection was refused.
const ineligible = selected.intendedUntracked!.find((path) => !eligible.includes(path));
if (ineligible !== undefined) return { reason: "path-not-eligible", field: ineligible };
return undefined;
}

function nativeStartRejection(reason: string, field?: string): Record<string, unknown> {
return {
operation: REVIEW_CONTROLLER_OPERATION.START,
Expand Down Expand Up @@ -7319,8 +7362,8 @@ async function executeReviewControllerOperation(
try { eligible = JSON.parse(eligibleJson ?? ""); } catch { eligible = undefined; }
const scope = parameters.intendedUntracked!.length === 0 ? NATIVE_START_UNTRACKED_SCOPE.EXCLUDE : NATIVE_START_UNTRACKED_SCOPE.SELECT;
const selected = validateNativeStartUntrackedSelection({ untrackedScope: scope, expectedUntrackedInventory: inventory, intendedUntracked: parameters.intendedUntracked });
const rejected = input === undefined || canonicalReviewCaptureBinding(input) !== canonicalBinding || exactCollectArgument(input, "target_identity") !== status.targetIdentity || exactCollectArgument(input, "projection") !== status.projection.projection || exactCollectArgument(input, "base_tree") !== status.projection.baseTree || exactCollectArgument(input, "candidate_tree") !== status.projection.currentCandidateTree || !Array.isArray(eligible) || selected.reason !== undefined || selected.intendedUntracked!.some((path) => !eligible.includes(path));
if (rejected) return { operation: parameters.operation, status: "blocked", outcome: "intended-untracked-selection-binding-rejected", mutation_performed: false, mutation_outcome: "none" };
const rejected = intendedUntrackedSelectionRejection(input, canonicalBinding, status, eligible, selected);
if (rejected !== undefined) return { operation: parameters.operation, status: "blocked", outcome: "intended-untracked-selection-binding-rejected", reason: rejected.reason, ...(rejected.field === undefined ? {} : { field: rejected.field }), mutation_performed: false, mutation_outcome: "none" };
const submission = { argumentTokens: input.submission!.argumentTokens, value: JSON.stringify({ schema: "gentle-ai.review-intended-untracked-selection/v1", untracked_scope: scope, expected_untracked_inventory: inventory, intended_untracked: selected.intendedUntracked }) };
const result = await executeReviewControllerOperation({ operation: REVIEW_CONTROLLER_OPERATION.START, ...(parameters.workspaceRoot === undefined ? {} : { workspaceRoot: parameters.workspaceRoot }), input: JSON.stringify({ mode: REVIEW_MODE.ORDINARY, untrackedScope: scope, expectedUntrackedInventory: inventory, intendedUntracked: selected.intendedUntracked }) }, sessionCwd, nativeReviewCli, signal, candidateViews, context, retainedUntrackedSelections, pendingReviewConsentRegistry, pendingReviewConsentFallbackKey, reviewConsentNow, reviewConsentScheduleTimer, submission);
return { ...result, operation: parameters.operation };
Expand Down
79 changes: 75 additions & 4 deletions tests/review-controller-native-routing.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,13 +938,19 @@ for (const statusSchema of ["gentle-ai.review-integration.status/v6", "gentle-ai
assert.deepEqual(requests.map((request) => request.cwd), [cwd, cwd, cwd]);
assert.equal(starts[0]!.cwd, cwd);
assert.deepEqual(starts[0]!.intendedUntrackedSelection, { argumentTokens: selection.submission!.argumentTokens, value: JSON.stringify({ schema: "gentle-ai.review-intended-untracked-selection/v1", untracked_scope: "select", expected_untracked_inventory: SHA, intended_untracked: [eligible] }) });
for (const invalid of [
{ selectionBinding: selectionBinding.replace(eligible, "docs/stale.md"), intendedUntracked: [eligible] },
{ selectionBinding, intendedUntracked: [eligible, eligible] }, { selectionBinding, intendedUntracked: ["docs/unknown.md"] },
]) {
// gentle-pi#941: every refusal named its own check, so a caller holding
// provider-issued bytes can tell a stale binding from an ineligible path
// instead of retrying the same call.
for (const { reason, field, ...invalid } of [
{ selectionBinding: selectionBinding.replace(eligible, "docs/stale.md"), intendedUntracked: [eligible], reason: "binding-mismatch" },
{ selectionBinding, intendedUntracked: [eligible, eligible], reason: "untracked-selection-invalid" },
{ selectionBinding, intendedUntracked: ["docs/unknown.md"], reason: "path-not-eligible", field: "docs/unknown.md" },
] as ReadonlyArray<{ reason: string; field?: string; selectionBinding: string; intendedUntracked: readonly string[] }>) {
const rejection = await __testing.executeReviewControllerOperation({ operation: "select-intended-untracked", ...invalid } as never, cwd, native, undefined, undefined, undefined, retained);
assert.equal(rejection.status, "blocked", JSON.stringify(invalid));
assert.equal(rejection.outcome, "intended-untracked-selection-binding-rejected", JSON.stringify(invalid));
assert.equal(rejection.reason, reason, JSON.stringify(rejection));
assert.equal(rejection.field, field, JSON.stringify(rejection));
assert.equal(rejection.mutation_performed, false, JSON.stringify(invalid));
assert.equal(rejection.mutation_outcome, "none", JSON.stringify(invalid));
}
Expand All @@ -953,6 +959,71 @@ for (const statusSchema of ["gentle-ai.review-integration.status/v6", "gentle-ai
assert.equal(requests.every((request) => !("lineageId" in request)), true);
});

// gentle-pi#941: a selection refused because the live status moved names the field
// that moved, and an empty selection -- the only honest answer when the candidate
// touches no untracked file -- is accepted rather than refused.
for (const scenario of [
{ name: "target_identity", moved: (status: ReviewStatusV3) => ({ ...status, targetIdentity: `sha256:${"d".repeat(64)}` }) },
{ name: "candidate_tree", moved: (status: ReviewStatusV3) => ({ ...status, projection: { ...status.projection, currentCandidateTree: "e".repeat(40) } }) },
]) test(`a moved ${scenario.name} refuses the selection by name rather than opaquely`, async (t) => {
const cwd = realpathSync(repository(t)), eligible = "selected.md";
writeFileSync(join(cwd, eligible), "selected\n");
const initialTarget = startStatus(cwd, undefined, [eligible]);
const selection = intendedUntrackedSelection(initialTarget, [eligible]);
const initial = { ...initialTarget, nextTransition: { kind: "collect", reasonCode: "intended_untracked_selection_required", collect: { inputs: [selection] } }, raw: { schema: "gentle-ai.review-integration.status/v7" } } as ReviewStatusV3;
let statusCalls = 0;
const starts: Array<Record<string, unknown>> = [], retained = new Map();
const native = {
reviewMode: async () => ({ operation: "status", scope: "clone", status: { global: "on", cloneLocal: "on", effective: "on", source: "clone_local" } }),
// The binding is issued against the first status and revalidated against
// the second, which is where a real repository drifts under the caller.
targetStatus: async () => (++statusCalls === 1 ? initial : scenario.moved(initial)),
start: async (request: Record<string, unknown>) => { starts.push(request); return { lineageId: "unreachable", state: "reviewing", riskLevel: "low", selectedLenses: [], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: false, riskReasons: [], raw: {} }; },
} as unknown as NativeReviewCli;
const listed = await __testing.executeReviewControllerOperation({ operation: "status", workspaceRoot: cwd }, cwd, native, undefined, undefined, undefined, retained);
const rejection = await __testing.executeReviewControllerOperation({ operation: "select-intended-untracked", selectionBinding: listed.selectionBinding as string, intendedUntracked: [eligible], workspaceRoot: cwd } as never, cwd, native, undefined, undefined, undefined, retained);
assert.equal(rejection.outcome, "intended-untracked-selection-binding-rejected", JSON.stringify(rejection));
assert.equal(rejection.reason, "status-field-mismatch", JSON.stringify(rejection));
assert.equal(rejection.field, scenario.name, JSON.stringify(rejection));
assert.equal(starts.length, 0);
});

// The reporter's own case in gentle-pi#941: the candidate touches only tracked
// files while the repository carries an unrelated untracked inventory, so the
// only honest selection is the empty one. It must start, not be refused.
test("an empty intended-untracked selection excludes every eligible path and starts", async (t) => {
const cwd = realpathSync(repository(t)), sessionCwd = repository(t);
writeFileSync(join(cwd, "qa screenshot 1.png"), "noise\n");
writeFileSync(join(cwd, "mcp.log"), "noise\n");
const initialTarget = startStatus(cwd), target = startStatus(cwd);
const selection = intendedUntrackedSelection(initialTarget, ["qa screenshot 1.png", "mcp.log"]);
const initial = { ...initialTarget, nextTransition: { kind: "collect", reasonCode: "intended_untracked_selection_required", collect: { inputs: [selection] } }, raw: { schema: "gentle-ai.review-integration.status/v7" } } as ReviewStatusV3;
const starts: Array<Record<string, unknown>> = [], retained = new Map();
const native = {
reviewMode: async () => ({ operation: "status", scope: "clone", status: { global: "on", cloneLocal: "on", effective: "on", source: "clone_local" } }),
targetStatus: async (request: Record<string, unknown>) => ("intendedUntrackedSelection" in request ? target : initial),
start: async (request: Record<string, unknown>) => { starts.push(request); return { lineageId: "excluded", state: "reviewing", riskLevel: "low", selectedLenses: [], changedFiles: 1, changedLines: 1, correctionBudget: 1, action: "created", lensesRequired: false, riskReasons: [], raw: {} }; },
} as unknown as NativeReviewCli;
const listed = await __testing.executeReviewControllerOperation({ operation: "status", workspaceRoot: cwd }, sessionCwd, native, undefined, undefined, undefined, retained);
const result = await __testing.executeReviewControllerOperation({ operation: "select-intended-untracked", selectionBinding: listed.selectionBinding as string, intendedUntracked: [], workspaceRoot: cwd } as never, sessionCwd, native, undefined, undefined, undefined, retained);
assert.equal(starts.length, 1, JSON.stringify(result));
// EXCLUDE, with an empty selection: nothing untracked joins the candidate.
assert.deepEqual(starts[0]!.intendedUntrackedSelection, { argumentTokens: selection.submission!.argumentTokens, value: JSON.stringify({ schema: "gentle-ai.review-intended-untracked-selection/v1", untracked_scope: "exclude", expected_untracked_inventory: SHA, intended_untracked: [] }) });
assert.equal(result.reason, undefined, JSON.stringify(result));
});

function intendedUntrackedSelection(status: ReviewStatusV3, eligible: readonly string[]): ReviewCollectInputV3 {
return {
name: "intended_untracked_selection", schema: "gentle-ai.review-intended-untracked-selection/v1", captureOperation: "external.select_intended_untracked",
arguments: [
{ name: "target_identity", value: SHA }, { name: "projection", value: "workspace" },
{ name: "base_tree", value: status.projection.baseTree }, { name: "candidate_tree", value: status.projection.currentCandidateTree },
{ name: "eligible_paths_json", value: JSON.stringify([...eligible]) }, { name: "expected_untracked_inventory", value: SHA },
],
submission: { operationToken: "status", argumentTokens: ["--contract=gentle-ai.review-integration/v2", "--next-transition=true", "--agent=pi", "--projection=workspace", "--intended-untracked-selection={{value}}"], values: [{ slot: "intended_untracked_selection", domain: "schema_bound_json", schema: "gentle-ai.review-intended-untracked-selection/v1", substitutionLocation: 4 }] },
} as unknown as ReviewCollectInputV3;
}

// gentle-pi#706: inspect names the exact continuation for the intended-untracked
// stop and can resolve it in one call through top-level untrackedScope.
function untrackedStopFixture(
Expand Down