From 0e522d55cd45eb26572af247aad9fcc3f664121d Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 01:52:20 +0200 Subject: [PATCH 1/8] fix(review): preflight cheap candidate-view invariants before inspect advertises START (#2196) Inspect advertised a ready review.start for candidates whose frozen-view materialization was already known to fail, so STATUS and START disagreed about operations that cannot begin (gentle-ai#2196). Before a ready advertisement leaves the controller, inspect now runs the same read-only checks START verifies: the POSIX candidate-owner-parent privacy assert and, when the frozen candidate tree tracks 120000 entries, a native symlink capability probe. A deterministic failure withholds the advertisement as candidate-view-preflight-blocked carrying the exact typed diagnostics the START refusal would produce (candidate-owner-parent-privacy, candidate-owner-parent-chmod-ineffective, symlink-materialization-failed), applied at all three inspect return paths. Message constants move to shared exports so inspect and START can never drift apart. Root causes and the audit that scoped this fix are reporter work credited in the tracked issue (Derillo, andresm1991, deaquino and the occurrence reporters); the current-main variant audit landed there as issuecomment-6226523271. --- docs/readme-reference.md | 4 +- extensions/gentle-ai.ts | 110 ++++++++++++++++++++++++- lib/review-candidate-view.ts | 7 +- tests/review-inspect-preflight.test.ts | 94 +++++++++++++++++++++ 4 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 tests/review-inspect-preflight.test.ts diff --git a/docs/readme-reference.md b/docs/readme-reference.md index ea0f968cc..c63d65cc1 100644 --- a/docs/readme-reference.md +++ b/docs/readme-reference.md @@ -816,9 +816,9 @@ Contract `/v2` replaces the Base64 `candidate_diff` reviewer transport of `/v1` Target status owns `current_target`, `unrelated`, `ambiguous`, and `corrupted` applicability and returns one native action. Pi does not reconstruct ordinary authority from provider-private files or choose a lineage from repository-wide history. Restart recovery rebuilds only the derived candidate view from the native Git/content projection, including intended-untracked paths, symlinks, and immutable gitlink identities. Native failure envelopes retain their exact mutation outcome, replayability, required inputs, request digest, and next action. After an unknown or lost mutating result, Pi calls target status before any replay decision and returns only the provider-declared action. -Candidate views materialize tracked Git symlinks from their frozen blobs even when `core.symlinks=false`, including unchanged links outside the changed scope. Unsafe targets fail before any link is created; a host without native symlink capability fails closed with `symlink-materialization-failed`. Pi does not alter the contributor's Git configuration or install symlink privileges. +Candidate views materialize tracked Git symlinks from their frozen blobs even when `core.symlinks=false`, including unchanged links outside the changed scope. Unsafe targets fail before any link is created; a host without native symlink capability fails closed with `symlink-materialization-failed`. Pi does not alter the contributor's Git configuration or install symlink privileges. Before `inspect` advertises a ready `review.start`, it runs the same read-only capability check over the frozen candidate tree, so a host that cannot create the tracked symlinks sees the refusal at inspect time instead of a START that cannot begin (gentle-ai#2196). -On POSIX, if START rejects a group- or world-accessible `.git/gentle-ai/candidate-views` parent, Pi reports `candidate-owner-parent-privacy` before native START. When a sanitized probe shows the filesystem cannot represent private POSIX modes (for example WSL DrvFS mounts without `metadata`), Pi instead reports `candidate-owner-parent-chmod-ineffective` with guidance to move the Git common directory to a POSIX-metadata filesystem or enable metadata support. Inspect that parent's ownership and permissions and correct them out of band before retrying; Pi does not change them automatically. Other owner-preparation failures retain a generic diagnostic rather than exposing filesystem errors. +On POSIX, if START rejects a group- or world-accessible `.git/gentle-ai/candidate-views` parent, Pi reports `candidate-owner-parent-privacy` before native START. When a sanitized probe shows the filesystem cannot represent private POSIX modes (for example WSL DrvFS mounts without `metadata`), Pi instead reports `candidate-owner-parent-chmod-ineffective` with guidance to move the Git common directory to a POSIX-metadata filesystem or enable metadata support. Inspect that parent's ownership and permissions and correct them out of band before retrying; Pi does not change them automatically. Other owner-preparation failures retain a generic diagnostic rather than exposing filesystem errors. `inspect` runs the same read-only privacy assert before advertising a ready `review.start` and withholds the advertisement with the identical typed diagnostic under `candidate-view-preflight-blocked`, so STATUS and START stop disagreeing about a candidate whose view cannot materialize (gentle-ai#2196). Once the source checkout's pinned gentle-ai runtime (currently v4.2.0) has written review authority, rollback MUST preserve every native store and receipt and MUST NOT run a downgraded binary against that repository. Disable the Pi route or roll forward to a compatible authority-aware release instead; deleting authority data or reinstalling an older binary is not a rollback path. diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index ee533f992..4c013c60d 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -22,10 +22,13 @@ import { existsSync, lstatSync, mkdirSync, + mkdtempSync, readdirSync, readFileSync, realpathSync, + rmSync, statSync, + symlinkSync, writeFileSync, } from "node:fs"; import { @@ -35,7 +38,7 @@ import { readdir, writeFile, } from "node:fs/promises"; -import { homedir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { discoverAgents, loadAgentsConfig, unmatchedProfileKeys } from "../lib/agents-config.ts"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; @@ -169,7 +172,8 @@ import { import { GentleAiElapsedTimingLedger } from "../lib/gentle-ai-elapsed-store.ts"; import { renderGentleAiLifecycleCall, renderGentleAiResult, type GentleAiRenderContext } from "../lib/gentle-ai-renderer.ts"; import { sanitizeTerminalText, stripAnsi } from "../lib/terminal-theme.ts"; -import { BASE_REF_ACCEPTED_FORMS, CandidateViewError, CandidateViewRegistry, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, isProviderCandidateBaseTree, type CandidateView } from "../lib/review-candidate-view.ts"; +import { BASE_REF_ACCEPTED_FORMS, CandidateViewError, CandidateViewRegistry, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, isProviderCandidateBaseTree, CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE, CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE, CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, type CandidateView } from "../lib/review-candidate-view.ts"; +import { assertCandidateOwnerParent, PosixCandidateOwnerParentChmodIneffectiveError, PosixCandidateOwnerParentPrivacyError } from "../lib/review-candidate-view-owner.ts"; import { GentleAiDevBinaryOverrideError, GENTLE_AI_INSTALL_RECOVERY_COMMAND, @@ -7421,6 +7425,87 @@ function nativePreLineageCandidateIdentity( : undefined; } +// gentle-ai#2196: inspect must not advertise a START whose candidate-view +// materialization is already known to fail. START materializes the frozen +// view and then verifies cheap non-mutating invariants; when one is +// deterministically false (the POSIX candidate owner parent is not private, +// or the frozen tree tracks symlinks this process cannot create natively), +// inspect runs the same read-only checks first and withholds the ready +// advertisement with the exact typed diagnostics the START refusal would +// carry, so STATUS and START stop disagreeing about operations that cannot +// begin. The Windows owner boundary keeps its START-time DACL validation for +// now; this preflight only answers questions a read-only check settles. +let candidateViewSymlinkProbe: (() => void) | undefined; + +function setCandidateViewSymlinkProbeForTesting(probe: (() => void) | undefined): void { + candidateViewSymlinkProbe = probe; +} + +function defaultCandidateViewSymlinkProbe(): void { + const directory = mkdtempSync(join(tmpdir(), "gentle-symlink-probe-")); + try { + symlinkSync("probe-target", join(directory, "probe"), process.platform === "win32" ? "file" : undefined); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +} + +function candidateTreeTracksSymlinks(cwd: string, candidateTree: string): boolean { + // Discovery must describe the selected directory, never an ambient Git + // override pointing at a different repository or worktree. + const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_"))); + try { + const listing = execFileSync("git", ["ls-tree", "-r", "--full-tree", candidateTree], { cwd, env, encoding: "utf8", windowsHide: true, stdio: ["ignore", "pipe", "pipe"] }); + return listing.split("\n").some((line) => line.startsWith("120000 ")); + } catch { + // An unreadable tree is not a deterministic materialization failure; + // START owns reporting it. + return false; + } +} + +function candidateViewPreflightRejection(status: ReviewStatusV3, diagnostics: Record): Record { + return { + operation: REVIEW_CONTROLLER_OPERATION.INSPECT, + status: "blocked", + outcome: "candidate-view-preflight-blocked", + result: status.raw, + diagnostics, + mutation_performed: false, + mutation_outcome: "none", + lineage_created: false, + next_action: "resolve-native-operation-failure", + }; +} + +function inspectCandidateViewPreflightBlocked(status: ReviewStatusV3, cwd: string, commonDir: string | undefined, platform: NodeJS.Platform = process.platform): Record | undefined { + if (status.action !== "start") return undefined; + if (platform !== "win32" && commonDir !== undefined) { + try { + assertCandidateOwnerParent(commonDir, platform); + } catch (error) { + // A missing parent is not a deterministic failure: START's prepare + // creates it private. Only the typed privacy findings withhold the + // advertisement; anything else stays START's to report. + const diagnostics = error instanceof PosixCandidateOwnerParentPrivacyError + ? { code: "candidate-owner-parent-privacy" as const, message: CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE } + : error instanceof PosixCandidateOwnerParentChmodIneffectiveError + ? { code: "candidate-owner-parent-chmod-ineffective" as const, message: CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE } + : undefined; + if (diagnostics !== undefined) return candidateViewPreflightRejection(status, diagnostics); + } + } + const candidateTree = status.projection.currentCandidateTree; + if (isCanonicalProcessString(candidateTree) && candidateTreeTracksSymlinks(cwd, candidateTree)) { + try { + (candidateViewSymlinkProbe ?? defaultCandidateViewSymlinkProbe)(); + } catch { + return candidateViewPreflightRejection(status, { code: "symlink-materialization-failed" as const, message: CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE }); + } + } + return undefined; +} + function readRetainedPreLineageNativeUntrackedSelection( selections: Map, workspaceRoot: string, @@ -8811,6 +8896,12 @@ async function executeReviewControllerOperation( retainNativeStatusSelection(retainedUntrackedSelections, reviewLifecycleStorageKey(defaultCwd, ""), Object.freeze({ ...readyCandidateIdentity, baseRef: canonicalBaseRef, committedOnly: true as const })); } } + // gentle-ai#2196: a ready advertisement must survive the same cheap + // candidate-view invariants START verifies, or be withheld. + if (plainMapped.status === "ready") { + const preflightBlocked = inspectCandidateViewPreflightBlocked(status, defaultCwd, workspaceResolution.identity?.commonDir); + if (preflightBlocked !== undefined) return { ...preflightBlocked, ...(includeWorkspaceRoot ? { workspace_root: defaultCwd } : {}) }; + } // gentle-pi#706: the stop alone never tells the caller what to do next. return { ...plainMapped, @@ -8821,12 +8912,18 @@ async function executeReviewControllerOperation( }; } const input = reviewIntendedUntrackedInput(status); - if (input === undefined) + if (input === undefined) { + // gentle-ai#2196: same ready-advertisement preflight as the plain path. + if (plainMapped.status === "ready") { + const preflightBlocked = inspectCandidateViewPreflightBlocked(status, defaultCwd, workspaceResolution.identity?.commonDir); + if (preflightBlocked !== undefined) return { ...preflightBlocked, ...(includeWorkspaceRoot ? { workspace_root: defaultCwd } : {}) }; + } return { ...plainMapped, untracked_selection: "not-required", ...(includeWorkspaceRoot ? { workspace_root: defaultCwd } : {}), }; + } const eligibleJson = exactCollectArgument(input, "eligible_paths_json"), inventory = exactCollectArgument(input, "expected_untracked_inventory"); let eligible: unknown; @@ -8910,6 +9007,12 @@ async function executeReviewControllerOperation( undefined, includeWorkspaceRoot ? defaultCwd : undefined, ); + // gentle-ai#2196: the resolved selection path advertises START under the + // same cheap candidate-view invariants as the plain path. + if (resolvedMapped.status === "ready") { + const preflightBlocked = inspectCandidateViewPreflightBlocked(resolvedStatus, defaultCwd, workspaceResolution.identity?.commonDir); + if (preflightBlocked !== undefined) return { ...preflightBlocked, ...(includeWorkspaceRoot ? { workspace_root: defaultCwd } : {}) }; + } return { ...resolvedMapped, ...("selectionBinding" in resolvedMapped @@ -9718,6 +9821,7 @@ async function executeReviewControllerOperation( /** @internal */ export const __testing = { + setCandidateViewSymlinkProbeForTesting, parseReviewControllerParameters, parseReviewCaptureParameters, parseReviewCaptureGroupParameters, diff --git a/lib/review-candidate-view.ts b/lib/review-candidate-view.ts index a3118a3ec..bcc54d6aa 100644 --- a/lib/review-candidate-view.ts +++ b/lib/review-candidate-view.ts @@ -242,8 +242,9 @@ export interface CandidateOwnerChmodIneffectiveDiagnostic { message: typeof CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE; } -const CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE = "candidate-views parent must be owned by the current user and inaccessible to group and others; inspect its ownership and permissions, then correct them out of band before retrying START" as const; -const CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE = "candidate-views parent filesystem does not honor POSIX permission changes; move this repository's Git common directory to a POSIX-metadata filesystem or enable metadata support before retrying START" as const; +export const CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE = "candidate-views parent must be owned by the current user and inaccessible to group and others; inspect its ownership and permissions, then correct them out of band before retrying START" as const; +export const CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE = "candidate-views parent filesystem does not honor POSIX permission changes; move this repository's Git common directory to a POSIX-metadata filesystem or enable metadata support before retrying START" as const; +export const CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE = "candidate view cannot create native symlinks; enable symlink capability out of band before retrying START" as const; export interface CandidateViewDiagnostic { phase: typeof CANDIDATE_VIEW_DIAGNOSTIC_PHASE; @@ -872,7 +873,7 @@ function materializeSymlinkEntries(root: string, entries: readonly CandidateTree const targetType = process.platform === "win32" && lstatSync(targetPath, { throwIfNoEntry: false })?.isDirectory() ? "dir" : "file"; symlinkSync(link.target, path, targetType); } catch (error) { - throw new CandidateViewError("candidate view cannot create native symlinks; enable symlink capability out of band before retrying START", "symlink-materialization-failed", undefined, { cause: error }); + throw new CandidateViewError(CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, "symlink-materialization-failed", undefined, { cause: error }); } } return new Map(links.map((link) => [link.path, createHash("sha256").update(Buffer.from(link.target, "utf8")).digest("hex")])); diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts new file mode 100644 index 000000000..b54ab4e17 --- /dev/null +++ b/tests/review-inspect-preflight.test.ts @@ -0,0 +1,94 @@ +import assert from "node:assert/strict"; +import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { __testing } from "../extensions/gentle-ai.ts"; +import type { NativeReviewCli } from "../lib/native-review-cli.ts"; + +// gentle-ai#2196: inspect must not advertise a review.start whose candidate-view +// materialization is already known to fail. START verifies cheap non-mutating +// invariants (candidate owner parent privacy, symlink capability for trees that +// track symlinks); inspect runs the same read-only checks before a ready +// advertisement leaves the controller, with the typed codes START would emit. + +function git(cwd: string, ...arguments_: string[]): string { + return execFileSync("git", [...arguments_], { cwd, encoding: "utf8" }).trim(); +} + +function fixtureRepository(tracksSymlink: boolean): { root: string; candidateTree: string } { + const root = realpathSync(mkdtempSync(join(tmpdir(), "inspect-preflight-"))); + git(root, "init", "--quiet", "-b", "main"); + writeFileSync(join(root, "tracked.txt"), "base\n"); + if (tracksSymlink) symlinkSync("tracked.txt", join(root, "link")); + git(root, "add", "-A"); + execFileSync("git", ["-c", "user.email=t@example.com", "-c", "user.name=T", "commit", "--quiet", "-m", "base"], { cwd: root, encoding: "utf8" }); + return { root, candidateTree: git(root, "rev-parse", "HEAD^{tree}") }; +} + +function readyNative(candidateTree: string): NativeReviewCli { + return { + reviewMode: async () => ({ operation: "status", status: { effective: "on", source: "global", global: "on", cloneLocal: "" } }), + targetStatus: async () => ({ + action: "start", + applicability: "unrelated", + targetIdentity: `sha256:${"a".repeat(64)}`, + projection: { projection: "workspace", baseTree: candidateTree, initialReviewTree: candidateTree, currentCandidateTree: candidateTree, paths: [] }, + raw: { schema: "gentle-ai.review-integration.status/v7", action: "start", reason_code: "fresh_target_ready" }, + nextTransition: { kind: "execute", execute: { operation: "review.start", arguments: [] } }, + }), + } as unknown as NativeReviewCli; +} + +const inspect = (root: string, native: NativeReviewCli) => + __testing.executeReviewControllerOperation({ operation: "inspect" }, root, native, undefined, null) as Promise>; + +const posix = process.platform !== "win32"; + +for (const quota of [{ name: "plain", untrackedScope: undefined }] as const) { + test(`inspect withholds a ready START advertisement when the candidate owner parent is not private (${quota.name}, gentle-ai#2196)`, { skip: posix ? false : "POSIX privacy modes are unavailable on Windows" }, async t => { + const { root } = fixtureRepository(false); + t.after(() => rmSync(root, { recursive: true, force: true })); + const views = join(root, ".git", "gentle-ai", "candidate-views"); + mkdirSync(views, { recursive: true, mode: 0o700 }); + chmodSync(views, 0o777); + const result = await inspect(root, readyNative(git(root, "rev-parse", "HEAD^{tree}"))); + assert.equal(result.status, "blocked"); + assert.equal(result.outcome, "candidate-view-preflight-blocked"); + assert.equal((result.diagnostics as { code?: string }).code, "candidate-owner-parent-privacy"); + assert.equal(result.mutation_performed, false); + assert.equal(result.lineage_created, false); + }); +} + +test("inspect withholds a ready START advertisement when the frozen tree tracks symlinks this process cannot create (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { + const { root, candidateTree } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + __testing.setCandidateViewSymlinkProbeForTesting(() => { throw new Error("EPERM (probe)"); }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "blocked"); + assert.equal(result.outcome, "candidate-view-preflight-blocked"); + assert.equal((result.diagnostics as { code?: string }).code, "symlink-materialization-failed"); + assert.match(String((result.diagnostics as { message?: string }).message), /enable symlink capability out of band before retrying START/); +}); + +test("a frozen tree without symlinks never consults the symlink capability probe (gentle-ai#2196)", async t => { + const { root, candidateTree } = fixtureRepository(false); + t.after(() => rmSync(root, { recursive: true, force: true })); + let probed = 0; + __testing.setCandidateViewSymlinkProbeForTesting(() => { probed++; throw new Error("EPERM (probe)"); }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(probed, 0, "no symlink entries in the frozen tree means no capability probe"); + assert.equal(result.status, "ready", "a healthy candidate keeps its advertisement"); +}); + +test("a healthy worktree keeps the ready START advertisement (gentle-ai#2196)", { skip: posix ? false : "real symlink probe depends on runner capability" }, async t => { + const { root, candidateTree } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "ready"); + assert.equal(result.outcome, undefined); +}); From f77add74a3e8e815c856ac4da76ef6c5a474fce8 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 11:55:12 +0200 Subject: [PATCH 2/8] fix(review): keep the symlink preflight honest: bounded scan, view-filesystem probe (#2196) The preflight's symlink checks could still guess wrong in three ways (gentle-ai#2196 review findings): the ls-tree child had no timeout and used Node's default output limit, so a slow or oversized tree hung the event loop, and a buffer-limit failure was reported as a confirmed symlink-free tree; and the capability probe sampled tmpdir instead of the filesystem that will hold the candidate view, so it could pass where START fails and vice versa. scanCandidateTreeForSymlinks now returns a tri-state and reuses the candidate-view Git bounds from the lib (resolveCandidateGitTimeoutMs, CANDIDATE_GIT_MAX_BUFFER_BYTES): a timed-out, over-limit, or unreadable listing is "unconfirmed", never evidence of a symlink-free tree, and START keeps owning unreadable trees. The default probe samples the deepest existing ancestor of /gentle-ai/candidate-views, so it tests the filesystem that will actually hold the view; when that filesystem cannot be probed the capability stays unconfirmed and inspect withholds the ready advertisement with the new typed code symlink-capability-unconfirmed instead of guessing from another filesystem. --- extensions/gentle-ai.ts | 103 ++++++++++++++++++++----- lib/review-candidate-view.ts | 5 +- tests/review-inspect-preflight.test.ts | 42 ++++++++++ 3 files changed, 130 insertions(+), 20 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 4c013c60d..01865dc22 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -38,7 +38,7 @@ import { readdir, writeFile, } from "node:fs/promises"; -import { homedir, tmpdir } from "node:os"; +import { homedir } from "node:os"; import { discoverAgents, loadAgentsConfig, unmatchedProfileKeys } from "../lib/agents-config.ts"; import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; @@ -172,7 +172,7 @@ import { import { GentleAiElapsedTimingLedger } from "../lib/gentle-ai-elapsed-store.ts"; import { renderGentleAiLifecycleCall, renderGentleAiResult, type GentleAiRenderContext } from "../lib/gentle-ai-renderer.ts"; import { sanitizeTerminalText, stripAnsi } from "../lib/terminal-theme.ts"; -import { BASE_REF_ACCEPTED_FORMS, CandidateViewError, CandidateViewRegistry, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, isProviderCandidateBaseTree, CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE, CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE, CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, type CandidateView } from "../lib/review-candidate-view.ts"; +import { BASE_REF_ACCEPTED_FORMS, CANDIDATE_GIT_MAX_BUFFER_BYTES, CandidateViewError, CandidateViewRegistry, CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE, CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE, CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE, CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, resolveCandidateGitTimeoutMs, isProviderCandidateBaseTree, type CandidateView } from "../lib/review-candidate-view.ts"; import { assertCandidateOwnerParent, PosixCandidateOwnerParentChmodIneffectiveError, PosixCandidateOwnerParentPrivacyError } from "../lib/review-candidate-view-owner.ts"; import { GentleAiDevBinaryOverrideError, @@ -7434,15 +7434,30 @@ function nativePreLineageCandidateIdentity( // advertisement with the exact typed diagnostics the START refusal would // carry, so STATUS and START stop disagreeing about operations that cannot // begin. The Windows owner boundary keeps its START-time DACL validation for -// now; this preflight only answers questions a read-only check settles. -let candidateViewSymlinkProbe: (() => void) | undefined; - -function setCandidateViewSymlinkProbeForTesting(probe: (() => void) | undefined): void { +// now; this preflight only answers questions a read-only check settles. The +// symlink scan never claims a confirmed symlink-free tree from an incomplete +// listing (timeout, output limit, unreadable tree), and the capability probe +// samples the filesystem that will hold the candidate view — when that +// filesystem cannot be probed, the capability stays unconfirmed and the +// advertisement is withheld instead of guessing from another filesystem. +let candidateViewSymlinkProbe: ((probeRoot: string) => void) | undefined; + +function setCandidateViewSymlinkProbeForTesting(probe: ((probeRoot: string) => void) | undefined): void { candidateViewSymlinkProbe = probe; } -function defaultCandidateViewSymlinkProbe(): void { - const directory = mkdtempSync(join(tmpdir(), "gentle-symlink-probe-")); +// Thrown when no existing directory of the candidate-view filesystem can host +// the probe: the capability stays unconfirmed instead of being reported as +// incapable, so an unverifiable filesystem never masquerades as a pass. +class CandidateViewSymlinkProbeUnavailableError extends Error {} + +function defaultCandidateViewSymlinkProbe(probeRoot: string): void { + let directory: string; + try { + directory = mkdtempSync(join(probeRoot, "gentle-symlink-probe-")); + } catch (error) { + throw new CandidateViewSymlinkProbeUnavailableError("cannot create a probe directory on the candidate-view filesystem", { cause: error }); + } try { symlinkSync("probe-target", join(directory, "probe"), process.platform === "win32" ? "file" : undefined); } finally { @@ -7450,20 +7465,66 @@ function defaultCandidateViewSymlinkProbe(): void { } } -function candidateTreeTracksSymlinks(cwd: string, candidateTree: string): boolean { +// The deepest existing ancestor of the candidate-view parent: probing there +// samples the filesystem that will actually hold the candidate view. Before +// the first START only the Git common directory exists; after one, the probe +// lands inside the real candidate-views parent. +function nearestExistingAncestorDirectory(path: string): string | undefined { + let current = path; + for (;;) { + try { + if (statSync(current).isDirectory()) return current; + } catch { + // Missing or unreadable: keep walking up. + } + const parent = dirname(current); + if (parent === current) return undefined; + current = parent; + } +} + +type CandidateTreeSymlinkScan = "tracks-symlinks" | "no-symlinks" | "unconfirmed"; + +function scanCandidateTreeForSymlinks(cwd: string, candidateTree: string): CandidateTreeSymlinkScan { // Discovery must describe the selected directory, never an ambient Git // override pointing at a different repository or worktree. const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !key.startsWith("GIT_"))); try { - const listing = execFileSync("git", ["ls-tree", "-r", "--full-tree", candidateTree], { cwd, env, encoding: "utf8", windowsHide: true, stdio: ["ignore", "pipe", "pipe"] }); - return listing.split("\n").some((line) => line.startsWith("120000 ")); + const listing = execFileSync("git", ["ls-tree", "-r", "--full-tree", candidateTree], { + cwd, + env, + encoding: "utf8", + windowsHide: true, + stdio: ["ignore", "pipe", "pipe"], + // Same bounds the candidate-view materialization uses: a bounded + // child that cannot hang the event loop, and a listing past the + // output limit is an incomplete scan, never a symlink-free verdict. + timeout: resolveCandidateGitTimeoutMs(), + maxBuffer: CANDIDATE_GIT_MAX_BUFFER_BYTES, + }); + return listing.split("\n").some((line) => line.startsWith("120000 ")) ? "tracks-symlinks" : "no-symlinks"; } catch { - // An unreadable tree is not a deterministic materialization failure; - // START owns reporting it. - return false; + // A timed-out, over-limit, or otherwise unreadable tree is an + // incomplete scan, not a deterministic materialization failure; + // START owns reporting unreadable trees. + return "unconfirmed"; } } +type CandidateViewSymlinkCapability = "capable" | "incapable" | "unconfirmed"; + +function candidateViewSymlinkCapability(commonDir: string | undefined): CandidateViewSymlinkCapability { + if (commonDir === undefined) return "unconfirmed"; + const probeRoot = nearestExistingAncestorDirectory(join(commonDir, "gentle-ai", "candidate-views")); + if (probeRoot === undefined) return "unconfirmed"; + try { + (candidateViewSymlinkProbe ?? defaultCandidateViewSymlinkProbe)(probeRoot); + } catch (error) { + return error instanceof CandidateViewSymlinkProbeUnavailableError ? "unconfirmed" : "incapable"; + } + return "capable"; +} + function candidateViewPreflightRejection(status: ReviewStatusV3, diagnostics: Record): Record { return { operation: REVIEW_CONTROLLER_OPERATION.INSPECT, @@ -7496,10 +7557,15 @@ function inspectCandidateViewPreflightBlocked(status: ReviewStatusV3, cwd: strin } } const candidateTree = status.projection.currentCandidateTree; - if (isCanonicalProcessString(candidateTree) && candidateTreeTracksSymlinks(cwd, candidateTree)) { - try { - (candidateViewSymlinkProbe ?? defaultCandidateViewSymlinkProbe)(); - } catch { + if (isCanonicalProcessString(candidateTree) && scanCandidateTreeForSymlinks(cwd, candidateTree) === "tracks-symlinks") { + const capability = candidateViewSymlinkCapability(commonDir); + if (capability === "unconfirmed") { + // The tree needs native symlinks and the target filesystem could not + // be probed: advertising START would only move the same uncertainty + // into a materialization refusal (gentle-ai#2196). + return candidateViewPreflightRejection(status, { code: "symlink-capability-unconfirmed" as const, message: CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE }); + } + if (capability === "incapable") { return candidateViewPreflightRejection(status, { code: "symlink-materialization-failed" as const, message: CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE }); } } @@ -9822,6 +9888,7 @@ async function executeReviewControllerOperation( /** @internal */ export const __testing = { setCandidateViewSymlinkProbeForTesting, + CandidateViewSymlinkProbeUnavailableError, parseReviewControllerParameters, parseReviewCaptureParameters, parseReviewCaptureGroupParameters, diff --git a/lib/review-candidate-view.ts b/lib/review-candidate-view.ts index bcc54d6aa..cab476002 100644 --- a/lib/review-candidate-view.ts +++ b/lib/review-candidate-view.ts @@ -12,7 +12,7 @@ export type ReviewLens = (typeof REVIEW_LENS)[number]; const CANDIDATE_GIT_TIMEOUT_MS = 10_000; const CANDIDATE_GIT_TIMEOUT_MAX_MS = 120_000; const CANDIDATE_GIT_TIMEOUT_ENV = "GENTLE_PI_CANDIDATE_GIT_TIMEOUT_MS"; -const CANDIDATE_GIT_MAX_BUFFER_BYTES = 64 * 1024 * 1024; +export const CANDIDATE_GIT_MAX_BUFFER_BYTES = 64 * 1024 * 1024; // Keep the copied index at least one timestamp tick behind its source. The // two-second target also clears filesystems whose mtime granularity is a second // or coarser without needlessly assigning an arbitrary historical timestamp. @@ -21,7 +21,7 @@ const PRIVATE_INDEX_RACY_BACKDATE_NS = 2_000_000_000n; // Candidate views may materialize full repository trees. Large repositories can // raise this bounded deadline without creating an unbounded child process. -function resolveCandidateGitTimeoutMs(environment: NodeJS.ProcessEnv = process.env): number { +export function resolveCandidateGitTimeoutMs(environment: NodeJS.ProcessEnv = process.env): number { const value = environment[CANDIDATE_GIT_TIMEOUT_ENV]; if (value === undefined || !/^[1-9]\d*$/.test(value)) return CANDIDATE_GIT_TIMEOUT_MS; const parsed = Number(value); @@ -245,6 +245,7 @@ export interface CandidateOwnerChmodIneffectiveDiagnostic { export const CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE = "candidate-views parent must be owned by the current user and inaccessible to group and others; inspect its ownership and permissions, then correct them out of band before retrying START" as const; export const CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE = "candidate-views parent filesystem does not honor POSIX permission changes; move this repository's Git common directory to a POSIX-metadata filesystem or enable metadata support before retrying START" as const; export const CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE = "candidate view cannot create native symlinks; enable symlink capability out of band before retrying START" as const; +export const CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE = "candidate-view symlink capability could not be verified on the filesystem that will hold the candidate view; make sure the Git common directory is writable and can create symlinks before retrying START" as const; export interface CandidateViewDiagnostic { phase: typeof CANDIDATE_VIEW_DIAGNOSTIC_PHASE; diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index b54ab4e17..55465fb0b 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -92,3 +92,45 @@ test("a healthy worktree keeps the ready START advertisement (gentle-ai#2196)", assert.equal(result.status, "ready"); assert.equal(result.outcome, undefined); }); + +test("the symlink capability probe samples the filesystem that will hold the candidate view, never tmpdir (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { + const { root, candidateTree } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + let probeRoot: string | undefined; + __testing.setCandidateViewSymlinkProbeForTesting((directory: string) => { + probeRoot = directory; + }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "ready"); + assert.ok(probeRoot !== undefined, "a symlink-tracking frozen tree consults the capability probe"); + assert.ok(probeRoot!.startsWith(join(root, ".git")), `probe must run inside the candidate-view filesystem, saw: ${probeRoot}`); +}); + +test("inspect withholds the ready START advertisement when the symlink capability stays unconfirmed (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { + const { root, candidateTree } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + __testing.setCandidateViewSymlinkProbeForTesting(() => { + throw new __testing.CandidateViewSymlinkProbeUnavailableError("no existing ancestor could host the probe"); + }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "blocked"); + assert.equal(result.outcome, "candidate-view-preflight-blocked"); + assert.equal((result.diagnostics as { code?: string }).code, "symlink-capability-unconfirmed"); + assert.equal(result.mutation_performed, false); + assert.equal(result.lineage_created, false); +}); + +test("an incomplete symlink scan never claims a symlink-free tree and keeps the advertisement; START owns unreadable trees (gentle-ai#2196)", async t => { + const { root } = fixtureRepository(false); + t.after(() => rmSync(root, { recursive: true, force: true })); + let probed = 0; + __testing.setCandidateViewSymlinkProbeForTesting(() => { + probed++; + }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative("f".repeat(40))); + assert.equal(result.status, "ready", "an unconfirmed scan withholds no advertisement"); + assert.equal(probed, 0, "an unconfirmed scan never consults the capability probe"); +}); From 2b8bbbd9f433d766fb71d1a85ba770493fddd0a9 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 12:46:14 +0200 Subject: [PATCH 3/8] fix(review): pin the scan tri-state with a testing seam and aim the unconfirmed message at the candidate-views filesystem (#2196) Review follow-up on 7fefd271e (gentle-ai#2196): the unconfirmed-capability message told users to fix the Git common directory, but the unconfirmed state arises on the filesystem that will hold the candidate-views parent, so the guidance now names that filesystem. The incomplete-scan rule was only observable through inspect's kept advertisement; expose scanCandidateTreeForSymlinks through the testing seam and pin all three classifications directly (tracks-symlinks, no-symlinks, unconfirmed for an unreadable tree). --- extensions/gentle-ai.ts | 1 + lib/review-candidate-view.ts | 2 +- tests/review-inspect-preflight.test.ts | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 01865dc22..779c42802 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -9889,6 +9889,7 @@ async function executeReviewControllerOperation( export const __testing = { setCandidateViewSymlinkProbeForTesting, CandidateViewSymlinkProbeUnavailableError, + scanCandidateTreeForSymlinks, parseReviewControllerParameters, parseReviewCaptureParameters, parseReviewCaptureGroupParameters, diff --git a/lib/review-candidate-view.ts b/lib/review-candidate-view.ts index cab476002..b368abeba 100644 --- a/lib/review-candidate-view.ts +++ b/lib/review-candidate-view.ts @@ -245,7 +245,7 @@ export interface CandidateOwnerChmodIneffectiveDiagnostic { export const CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE = "candidate-views parent must be owned by the current user and inaccessible to group and others; inspect its ownership and permissions, then correct them out of band before retrying START" as const; export const CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE = "candidate-views parent filesystem does not honor POSIX permission changes; move this repository's Git common directory to a POSIX-metadata filesystem or enable metadata support before retrying START" as const; export const CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE = "candidate view cannot create native symlinks; enable symlink capability out of band before retrying START" as const; -export const CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE = "candidate-view symlink capability could not be verified on the filesystem that will hold the candidate view; make sure the Git common directory is writable and can create symlinks before retrying START" as const; +export const CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE = "candidate-view symlink capability could not be verified on the filesystem that will hold the candidate-views parent; make sure that filesystem is writable and can create symlinks before retrying START" as const; export interface CandidateViewDiagnostic { phase: typeof CANDIDATE_VIEW_DIAGNOSTIC_PHASE; diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index 55465fb0b..af50b807b 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -134,3 +134,13 @@ test("an incomplete symlink scan never claims a symlink-free tree and keeps the assert.equal(result.status, "ready", "an unconfirmed scan withholds no advertisement"); assert.equal(probed, 0, "an unconfirmed scan never consults the capability probe"); }); + +test("the scan classification itself is a tri-state: unreadable trees are unconfirmed, never symlink-free (gentle-ai#2196)", async t => { + const { root } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + assert.equal(__testing.scanCandidateTreeForSymlinks(root, git(root, "rev-parse", "HEAD^{tree}")), "tracks-symlinks"); + const bare = fixtureRepository(false); + t.after(() => rmSync(bare.root, { recursive: true, force: true })); + assert.equal(__testing.scanCandidateTreeForSymlinks(bare.root, git(bare.root, "rev-parse", "HEAD^{tree}")), "no-symlinks"); + assert.equal(__testing.scanCandidateTreeForSymlinks(bare.root, "f".repeat(40)), "unconfirmed", "an unreadable tree is never classified symlink-free"); +}); From 84a802005916eb83751943476d9ce415dca78748 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 16:19:38 +0200 Subject: [PATCH 4/8] ci: retrigger verify after runner-side git clone maintenance.lock flake From f719356e6b83b23970c7d778d2b5ca0cc65bbdb8 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 17:10:11 +0200 Subject: [PATCH 5/8] fix(review): withhold the native START transition from blocked inspect results (#2196) candidateViewPreflightRejection forwarded the native STATUS body unchanged, and that body carries the executable review.start next_transition that consumers read from result.next_transition. A blocked inspect now strips it while keeping the remaining native status data. The test fixture now mirrors the real native body shape. --- extensions/gentle-ai.ts | 5 ++++- tests/review-inspect-preflight.test.ts | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 779c42802..94b09bbc0 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -7526,11 +7526,14 @@ function candidateViewSymlinkCapability(commonDir: string | undefined): Candidat } function candidateViewPreflightRejection(status: ReviewStatusV3, diagnostics: Record): Record { + // The native body carries the executable START transition; consumers read the + // nested result.next_transition, so a blocked inspect must not forward it. + const { next_transition: _withheldTransition, ...blockedResult } = status.raw; return { operation: REVIEW_CONTROLLER_OPERATION.INSPECT, status: "blocked", outcome: "candidate-view-preflight-blocked", - result: status.raw, + result: blockedResult, diagnostics, mutation_performed: false, mutation_outcome: "none", diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index af50b807b..c92d464a7 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -27,6 +27,8 @@ function fixtureRepository(tracksSymlink: boolean): { root: string; candidateTre return { root, candidateTree: git(root, "rev-parse", "HEAD^{tree}") }; } +const START_TRANSITION = { kind: "execute", execute: { operation: "review.start", arguments: [] } } as const; + function readyNative(candidateTree: string): NativeReviewCli { return { reviewMode: async () => ({ operation: "status", status: { effective: "on", source: "global", global: "on", cloneLocal: "" } }), @@ -35,7 +37,8 @@ function readyNative(candidateTree: string): NativeReviewCli { applicability: "unrelated", targetIdentity: `sha256:${"a".repeat(64)}`, projection: { projection: "workspace", baseTree: candidateTree, initialReviewTree: candidateTree, currentCandidateTree: candidateTree, paths: [] }, - raw: { schema: "gentle-ai.review-integration.status/v7", action: "start", reason_code: "fresh_target_ready" }, + // raw is the full native STATUS body, which carries the executable transition. + raw: { schema: "gentle-ai.review-integration.status/v7", action: "start", reason_code: "fresh_target_ready", next_transition: START_TRANSITION }, nextTransition: { kind: "execute", execute: { operation: "review.start", arguments: [] } }, }), } as unknown as NativeReviewCli; @@ -74,6 +77,18 @@ test("inspect withholds a ready START advertisement when the frozen tree tracks assert.match(String((result.diagnostics as { message?: string }).message), /enable symlink capability out of band before retrying START/); }); +test("a blocked inspect result carries no executable START transition but keeps the native status data (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { + const { root, candidateTree } = fixtureRepository(true); + t.after(() => rmSync(root, { recursive: true, force: true })); + __testing.setCandidateViewSymlinkProbeForTesting(() => { throw new Error("EPERM (probe)"); }); + t.after(() => __testing.setCandidateViewSymlinkProbeForTesting(undefined)); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "blocked"); + assert.equal(result.next_transition, undefined); + assert.deepEqual(result.result, { schema: "gentle-ai.review-integration.status/v7", action: "start", reason_code: "fresh_target_ready" }); + assert.equal(JSON.stringify(result).includes("review.start"), false, "no nested field may still advertise review.start"); +}); + test("a frozen tree without symlinks never consults the symlink capability probe (gentle-ai#2196)", async t => { const { root, candidateTree } = fixtureRepository(false); t.after(() => rmSync(root, { recursive: true, force: true })); From 13636e2929afdedd4ab98a11597b4922c2ce94fc Mon Sep 17 00:00:00 2001 From: danielgap Date: Sat, 17 Oct 2026 17:29:13 +0200 Subject: [PATCH 6/8] fix(review): withhold START from inspect on any owner failure START would refuse (#2196) Inspect only withheld the advertisement for the typed privacy findings, so a symlinked, non-directory, or unreadable candidate-views ancestor still advertised a review.start that START then refused. Only ENOENT stays advertised, because START creates the missing parent private. Every other owner failure is classified through the exported candidateOwnerPreparationError mapping instead of a duplicated one. --- extensions/gentle-ai.ts | 19 +++++++++---------- lib/review-candidate-view.ts | 2 +- tests/review-inspect-preflight.test.ts | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 94b09bbc0..fc6397738 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -172,8 +172,8 @@ import { import { GentleAiElapsedTimingLedger } from "../lib/gentle-ai-elapsed-store.ts"; import { renderGentleAiLifecycleCall, renderGentleAiResult, type GentleAiRenderContext } from "../lib/gentle-ai-renderer.ts"; import { sanitizeTerminalText, stripAnsi } from "../lib/terminal-theme.ts"; -import { BASE_REF_ACCEPTED_FORMS, CANDIDATE_GIT_MAX_BUFFER_BYTES, CandidateViewError, CandidateViewRegistry, CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE, CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE, CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE, CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, resolveCandidateGitTimeoutMs, isProviderCandidateBaseTree, type CandidateView } from "../lib/review-candidate-view.ts"; -import { assertCandidateOwnerParent, PosixCandidateOwnerParentChmodIneffectiveError, PosixCandidateOwnerParentPrivacyError } from "../lib/review-candidate-view-owner.ts"; +import { BASE_REF_ACCEPTED_FORMS, CANDIDATE_GIT_MAX_BUFFER_BYTES, candidateOwnerPreparationError, CandidateViewError, CandidateViewRegistry, CANDIDATE_SYMLINK_CAPABILITY_UNCONFIRMED_MESSAGE, CANDIDATE_SYMLINK_MATERIALIZATION_MESSAGE, injectReviewCandidateView, readCandidateContextManifestPage, resolveCanonicalCandidateBase, resolveCandidateGitTimeoutMs, isProviderCandidateBaseTree, type CandidateView } from "../lib/review-candidate-view.ts"; +import { assertCandidateOwnerParent } from "../lib/review-candidate-view-owner.ts"; import { GentleAiDevBinaryOverrideError, GENTLE_AI_INSTALL_RECOVERY_COMMAND, @@ -7549,14 +7549,13 @@ function inspectCandidateViewPreflightBlocked(status: ReviewStatusV3, cwd: strin assertCandidateOwnerParent(commonDir, platform); } catch (error) { // A missing parent is not a deterministic failure: START's prepare - // creates it private. Only the typed privacy findings withhold the - // advertisement; anything else stays START's to report. - const diagnostics = error instanceof PosixCandidateOwnerParentPrivacyError - ? { code: "candidate-owner-parent-privacy" as const, message: CANDIDATE_OWNER_PARENT_PRIVACY_MESSAGE } - : error instanceof PosixCandidateOwnerParentChmodIneffectiveError - ? { code: "candidate-owner-parent-chmod-ineffective" as const, message: CANDIDATE_OWNER_PARENT_CHMOD_INEFFECTIVE_MESSAGE } - : undefined; - if (diagnostics !== undefined) return candidateViewPreflightRejection(status, diagnostics); + // creates it private. Every other owner failure makes START refuse too, + // so withhold the advertisement with the owner-preparation classification. + if ((error as NodeJS.ErrnoException | undefined)?.code !== "ENOENT") { + const failure = candidateOwnerPreparationError(error); + const typed = failure.diagnostics !== undefined && "code" in failure.diagnostics ? failure.diagnostics : undefined; + return candidateViewPreflightRejection(status, typed ?? { code: failure.reason, message: failure.message }); + } } } const candidateTree = status.projection.currentCandidateTree; diff --git a/lib/review-candidate-view.ts b/lib/review-candidate-view.ts index b368abeba..00de3b330 100644 --- a/lib/review-candidate-view.ts +++ b/lib/review-candidate-view.ts @@ -689,7 +689,7 @@ function makeWritableForCleanup(path: string): void { chmodSync(path, 0o644); } -function candidateOwnerPreparationError(error: unknown): CandidateViewError { +export function candidateOwnerPreparationError(error: unknown): CandidateViewError { // Surface the bounded errno of the underlying failure directly in the // message so CI logs are self-diagnosing without TAP printing the cause. const code = (error as NodeJS.ErrnoException | undefined)?.code; diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index c92d464a7..f79f3ac25 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -65,6 +65,21 @@ for (const quota of [{ name: "plain", untrackedScope: undefined }] as const) { }); } +test("inspect withholds a ready START advertisement for any other candidate owner failure START would repeat (gentle-ai#2196)", { skip: posix ? false : "POSIX symlinked owner parent fixture" }, async t => { + const { root, candidateTree } = fixtureRepository(false); + t.after(() => rmSync(root, { recursive: true, force: true })); + const elsewhere = join(root, ".git", "elsewhere"); + mkdirSync(elsewhere, { mode: 0o700 }); + mkdirSync(join(root, ".git", "gentle-ai"), { mode: 0o700 }); + symlinkSync(elsewhere, join(root, ".git", "gentle-ai", "candidate-views")); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "blocked"); + assert.equal(result.outcome, "candidate-view-preflight-blocked"); + assert.equal((result.diagnostics as { code?: string }).code, "candidate-owner-preparation-failed"); + assert.equal(result.mutation_performed, false); + assert.equal(result.lineage_created, false); +}); + test("inspect withholds a ready START advertisement when the frozen tree tracks symlinks this process cannot create (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { const { root, candidateTree } = fixtureRepository(true); t.after(() => rmSync(root, { recursive: true, force: true })); From 07231503d389fdabf5c90f278578953fdf556597 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sun, 18 Oct 2026 16:21:18 +0200 Subject: [PATCH 7/8] fix(review): withhold START when the missing owner parent cannot be created (#2196) A missing candidate-views parent is only harmless while START can create it under the nearest existing ancestor. Check write and search access there and withhold the ready START advertisement with candidate-owner-preparation-failed when the access check fails. --- extensions/gentle-ai.ts | 23 ++++++++++++++++++----- tests/review-inspect-preflight.test.ts | 23 ++++++++++++++++++++++- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index fc6397738..5c0304748 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -19,6 +19,8 @@ import { execFileSync } from "node:child_process"; import { createHash, randomUUID, timingSafeEqual } from "node:crypto"; import { isProxy } from "node:util/types"; import { + accessSync, + constants as fsConstants, existsSync, lstatSync, mkdirSync, @@ -7548,11 +7550,22 @@ function inspectCandidateViewPreflightBlocked(status: ReviewStatusV3, cwd: strin try { assertCandidateOwnerParent(commonDir, platform); } catch (error) { - // A missing parent is not a deterministic failure: START's prepare - // creates it private. Every other owner failure makes START refuse too, - // so withhold the advertisement with the owner-preparation classification. - if ((error as NodeJS.ErrnoException | undefined)?.code !== "ENOENT") { - const failure = candidateOwnerPreparationError(error); + // A missing parent is not a deterministic failure while START's prepare + // can still create it private under the nearest existing ancestor. Every + // other owner failure makes START refuse too, so withhold the + // advertisement with the owner-preparation classification. + let failureCause: unknown = error; + if ((error as NodeJS.ErrnoException | undefined)?.code === "ENOENT") { + failureCause = undefined; + const ancestor = nearestExistingAncestorDirectory(join(commonDir, "gentle-ai", "candidate-views")); + try { + if (ancestor !== undefined) accessSync(ancestor, fsConstants.W_OK | fsConstants.X_OK); + } catch (accessError) { + failureCause = accessError; + } + } + if (failureCause !== undefined) { + const failure = candidateOwnerPreparationError(failureCause); const typed = failure.diagnostics !== undefined && "code" in failure.diagnostics ? failure.diagnostics : undefined; return candidateViewPreflightRejection(status, typed ?? { code: failure.reason, message: failure.message }); } diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index f79f3ac25..638de33e3 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { chmodSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -80,6 +80,27 @@ test("inspect withholds a ready START advertisement for any other candidate owne assert.equal(result.lineage_created, false); }); +const unprivileged = posix && process.getuid?.() !== 0; + +test("inspect withholds a ready START advertisement when the missing owner parent cannot be created under its nearest existing ancestor (gentle-ai#2196)", { skip: unprivileged ? false : "needs POSIX permission bits enforced for this process" }, async t => { + const { root, candidateTree } = fixtureRepository(false); + const commonDir = join(root, ".git"); + t.after(() => { + chmodSync(commonDir, 0o755); + rmSync(root, { recursive: true, force: true }); + }); + // The owner parent is missing, so START would try to create it under .git; + // without write access there that creation fails, and START with it. + chmodSync(commonDir, 0o555); + const result = await inspect(root, readyNative(candidateTree)); + assert.equal(result.status, "blocked"); + assert.equal(result.outcome, "candidate-view-preflight-blocked"); + assert.deepEqual(result.diagnostics, { code: "candidate-owner-preparation-failed", message: "candidate view owner preparation failed (EACCES)" }); + assert.equal(result.mutation_performed, false); + assert.equal(result.lineage_created, false); + assert.equal(existsSync(join(commonDir, "gentle-ai")), false, "the preflight creates nothing"); +}); + test("inspect withholds a ready START advertisement when the frozen tree tracks symlinks this process cannot create (gentle-ai#2196)", { skip: posix ? false : "fixture needs a tracked symlink" }, async t => { const { root, candidateTree } = fixtureRepository(true); t.after(() => rmSync(root, { recursive: true, force: true })); From b24ff27341001d80d1c6481675b487902a6c8ec5 Mon Sep 17 00:00:00 2001 From: danielgap Date: Thu, 22 Oct 2026 23:34:26 +0200 Subject: [PATCH 8/8] docs(review): describe inspect preflight checks as non-persistent (#2196) The symlink capability probe creates and removes a temporary directory under the candidate-views parent, so calling the preflight read-only overstated it. Reword the reference docs and code comments. --- docs/readme-reference.md | 2 +- extensions/gentle-ai.ts | 5 +++-- tests/review-inspect-preflight.test.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/readme-reference.md b/docs/readme-reference.md index c63d65cc1..b41c21257 100644 --- a/docs/readme-reference.md +++ b/docs/readme-reference.md @@ -816,7 +816,7 @@ Contract `/v2` replaces the Base64 `candidate_diff` reviewer transport of `/v1` Target status owns `current_target`, `unrelated`, `ambiguous`, and `corrupted` applicability and returns one native action. Pi does not reconstruct ordinary authority from provider-private files or choose a lineage from repository-wide history. Restart recovery rebuilds only the derived candidate view from the native Git/content projection, including intended-untracked paths, symlinks, and immutable gitlink identities. Native failure envelopes retain their exact mutation outcome, replayability, required inputs, request digest, and next action. After an unknown or lost mutating result, Pi calls target status before any replay decision and returns only the provider-declared action. -Candidate views materialize tracked Git symlinks from their frozen blobs even when `core.symlinks=false`, including unchanged links outside the changed scope. Unsafe targets fail before any link is created; a host without native symlink capability fails closed with `symlink-materialization-failed`. Pi does not alter the contributor's Git configuration or install symlink privileges. Before `inspect` advertises a ready `review.start`, it runs the same read-only capability check over the frozen candidate tree, so a host that cannot create the tracked symlinks sees the refusal at inspect time instead of a START that cannot begin (gentle-ai#2196). +Candidate views materialize tracked Git symlinks from their frozen blobs even when `core.symlinks=false`, including unchanged links outside the changed scope. Unsafe targets fail before any link is created; a host without native symlink capability fails closed with `symlink-materialization-failed`. Pi does not alter the contributor's Git configuration or install symlink privileges. Before `inspect` advertises a ready `review.start`, it runs the same non-persistent capability check over the frozen candidate tree (the probe creates and removes a temporary directory under the candidate-views parent, or its nearest existing ancestor), so a host that cannot create the tracked symlinks sees the refusal at inspect time instead of a START that cannot begin (gentle-ai#2196). On POSIX, if START rejects a group- or world-accessible `.git/gentle-ai/candidate-views` parent, Pi reports `candidate-owner-parent-privacy` before native START. When a sanitized probe shows the filesystem cannot represent private POSIX modes (for example WSL DrvFS mounts without `metadata`), Pi instead reports `candidate-owner-parent-chmod-ineffective` with guidance to move the Git common directory to a POSIX-metadata filesystem or enable metadata support. Inspect that parent's ownership and permissions and correct them out of band before retrying; Pi does not change them automatically. Other owner-preparation failures retain a generic diagnostic rather than exposing filesystem errors. `inspect` runs the same read-only privacy assert before advertising a ready `review.start` and withholds the advertisement with the identical typed diagnostic under `candidate-view-preflight-blocked`, so STATUS and START stop disagreeing about a candidate whose view cannot materialize (gentle-ai#2196). diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 5c0304748..4cf4a378f 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -7432,11 +7432,12 @@ function nativePreLineageCandidateIdentity( // view and then verifies cheap non-mutating invariants; when one is // deterministically false (the POSIX candidate owner parent is not private, // or the frozen tree tracks symlinks this process cannot create natively), -// inspect runs the same read-only checks first and withholds the ready +// inspect runs the same non-persistent checks first and withholds the ready // advertisement with the exact typed diagnostics the START refusal would // carry, so STATUS and START stop disagreeing about operations that cannot // begin. The Windows owner boundary keeps its START-time DACL validation for -// now; this preflight only answers questions a read-only check settles. The +// now; this preflight only answers questions a non-persistent check settles +// (the capability probe creates and removes a temporary directory). The // symlink scan never claims a confirmed symlink-free tree from an incomplete // listing (timeout, output limit, unreadable tree), and the capability probe // samples the filesystem that will hold the candidate view — when that diff --git a/tests/review-inspect-preflight.test.ts b/tests/review-inspect-preflight.test.ts index 638de33e3..e15cc711f 100644 --- a/tests/review-inspect-preflight.test.ts +++ b/tests/review-inspect-preflight.test.ts @@ -10,7 +10,7 @@ import type { NativeReviewCli } from "../lib/native-review-cli.ts"; // gentle-ai#2196: inspect must not advertise a review.start whose candidate-view // materialization is already known to fail. START verifies cheap non-mutating // invariants (candidate owner parent privacy, symlink capability for trees that -// track symlinks); inspect runs the same read-only checks before a ready +// track symlinks); inspect runs the same non-persistent checks before a ready // advertisement leaves the controller, with the typed codes START would emit. function git(cwd: string, ...arguments_: string[]): string {