fix(review): preserve exact candidate owner filesystem identity

lib/review-candidate-view-owner.ts

@@ -286,14 +286,15 @@ function posixPrivateModeProbe(path: string): boolean {
 }
 
 function directory(path: string, privateMode = false, platform: NodeJS.Platform = process.platform, classifyParentPrivacy = false): string {
-	const stat = lstatSync(path);
+	// 64-bit dev/ino may collide as Numbers; validate metadata and identity from one exact observation.
+	const stat = lstatSync(path, { bigint: true });
 	if (!stat.isDirectory() || stat.isSymbolicLink() || !samePath(realpathSync(path), path, platform)) throw new Error("Unsafe candidate owner directory");
 	if (privateMode && platform !== "win32") {
 		const uid = process.getuid?.();
 		if (uid === undefined) throw new Error("Unsafe candidate owner directory");
-		if (stat.uid !== uid || (stat.mode & 0o077) !== 0) {
+		if (stat.uid !== BigInt(uid) || (stat.mode & 0o077n) !== 0n) {
 			if (classifyParentPrivacy) {
-				if (stat.uid === uid && (stat.mode & 0o077) !== 0 && !posixPrivateModeProbe(path)) throw new PosixCandidateOwnerParentChmodIneffectiveError();
+				if (stat.uid === BigInt(uid) && (stat.mode & 0o077n) !== 0n && !posixPrivateModeProbe(path)) throw new PosixCandidateOwnerParentChmodIneffectiveError();
 				throw new PosixCandidateOwnerParentPrivacyError();
 			}
 			throw new Error("Unsafe candidate owner directory");
@@ -329,10 +330,11 @@ export function prepareCandidateOwnerParent(commonDir: string, platform: NodeJS.
 }
 
 function regular(path: string, privateMode = false, platform: NodeJS.Platform = process.platform): string {
-	const stat = lstatSync(path);
+	// Keep 64-bit identity exact without a second stat lookup racing the privacy checks.
+	const stat = lstatSync(path, { bigint: true });
 	const uid = process.getuid?.();
-	if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1 || !samePath(realpathSync(path), path, platform) || stat.size > 16384 ||
-		(privateMode && platform !== "win32" && (uid === undefined || stat.uid !== uid || (stat.mode & 0o777) !== 0o600))) throw new Error("Unsafe candidate owner file");
+	if (!stat.isFile() || stat.isSymbolicLink() || stat.nlink !== 1n || !samePath(realpathSync(path), path, platform) || stat.size > 16384n ||
+		(privateMode && platform !== "win32" && (uid === undefined || stat.uid !== BigInt(uid) || (stat.mode & 0o777n) !== 0o600n))) throw new Error("Unsafe candidate owner file");
 	if (privateMode && platform === "win32") privateWindowsDacl(path, "file", false);
 	return `${stat.dev}:${stat.ino}`;
 }

odd/tasks/candidate-owner-file-identity.md

@@ -0,0 +1,30 @@
+# Exact candidate-owner filesystem identity
+
+Refs #343 (approved cleanup tracker; remains closed). Independent follow-up to merged #672; blocks Windows check on coordination PR #1736. No main merge, auto-merge or runtime reload authorized.
+
+## Invariant and scope
+Rollback and reclamation must preserve a replaced marker/root even when two filesystem identifiers exceed Number's safe range. Use exact dev/ino from the same filesystem observation; no new authority representation, retry or gate. Limit changes to the owner module, focused regression and this tracker. Forecast <150 changed lines; dedicated `fix/candidate-owner-file-identity` worktree, current main `a87192ee`.
+
+## Evidence before correction
+Observed Windows CI run `37159261739`, job `111309064512`: original replacement-preservation test failed because marker was absent. Actual NTFS identifiers were not logged, so that exact CI cause is inferred, not measured.
+
+On clean current main, a deterministic actual-registry/real-filesystem fixture assigns two distinct 64-bit marker identities (`2^53`, `2^53+1`) which collide when represented as Number. It observes one controlled fsync replacement, no worktree addition, preserved original `.saved` and no lock, but rollback deletes the replacement. Focused command: `node --experimental-strip-types --test --test-name-pattern='candidate owner publication.*replaced|64-bit|precision' tests/review-candidate-view.test.ts`: one passed, one failed. This is modeled precision loss, not native NTFS measurement.
+
+## Plan
+- [x] Current-main behavior RED and approved-root/duplicate audit.
+- [x] Exact bigint identity, same-stat privacy/type/size validation and GREEN.
+- [x] Relevant candidate/repository suite and types/runtime validation.
+- [ ] Independent read-only verification by parent (high-risk deletion authority); no semantic approval or native verdict claimed.
+- [ ] Separate PR and actual Windows CI; propagate fix without main merge and verify stacked checks.
+
+## Correction verification
+Both `regular()` and `directory()` now request `{ bigint: true }` once; their existing template-string dev/ino identity API stays unchanged. UID/mode/nlink/size validation uses that same observation with bigint comparisons; the unrelated numeric chmod probe remains unchanged. No marker JSON, ACL, path, registration or PID-death changes.
+
+Additional controlled root-replacement regression retains the Git pointer and intercepts removal without executing it, protecting fixture replacement data even on RED. Before correction, the focused command above observed 3 tests: 1 pass, 2 fail (marker replacement deleted; directory replacement reached Git removal). After correction: 3 pass, 0 fail.
+
+- `node --experimental-strip-types --test tests/review-candidate-view.test.ts tests/review-repository.test.ts`: 173 tests, 165 pass, 8 Windows-only skips, 0 fail; existing privacy, symlink, replacement, failed-fsync/removal and death-proof controls pass.
+- `node scripts/check-types.mjs`: 186 recorded diagnostics, no regressions; 12 file/code pairs improved (no baseline update).
+- `node scripts/build-runtime-modules.mjs --check`: 8 generated modules match; one-shot metrics sources validated.
+- `git diff --check`: passed. Native Windows CI is unavailable locally; modeled precision evidence does not measure the original NTFS identifiers. Review-ready uncommitted diff only; delivery and independent verification remain parent-owned.
+
+Rollback boundary: owner-module precision changes and their regression. Marker JSON, public APIs, privacy/ACL/registration/dead-owner gates must remain unchanged. CodeGraph at parent cwd indexes gentle-ai, not this repository; child CodeGraph unavailable/uninitialized, scoped known-path fallback used.

tests/review-candidate-view.test.ts

@@ -364,6 +364,53 @@ test("candidate owner publication never removes a replaced marker", (t) => {
 	assert.equal(readdirSync(parent).some((name) => name.endsWith(".reaper-lock")), false);
 });
 
+test("candidate owner publication preserves a replaced marker with 64-bit identity precision loss", (t) => {
+	mockWindowsAcl(t);
+	const cwd = repository(t), parent = join(cwd, ".git", "gentle-ai", "candidate-views");
+	const originalLstat = fs.lstatSync, fsync = fs.fsyncSync, write = fs.writeFileSync;
+	// Modeled identities, not measured NTFS values: distinct 64-bit indices round to one Number.
+	const high = 2n ** 53n;
+	assert.notEqual(high, high + 1n);
+	assert.equal(Number(high), Number(high + 1n));
+	let marker: string | undefined, replaced = false, faults = 0, adds = 0;
+	let savedContent: string | undefined;
+	t.mock.method(fs, "lstatSync", (path: Parameters<typeof fs.lstatSync>[0], ...args: unknown[]) => {
+		const stat = (originalLstat as (...arguments_: unknown[]) => ReturnType<typeof fs.lstatSync>)(path, ...args);
+		if (marker === undefined && typeof path === "string" && path.endsWith(".owner.json") && path === join(parent, readdirSync(parent).find((name) => name.endsWith(".owner.json")) ?? "")) marker = path;
+		if (path !== marker) return stat;
+		const bigint = (args[0] as { bigint?: boolean } | undefined)?.bigint === true;
+		const ino = replaced ? high + 1n : high;
+		// Retain real metadata and Stats methods; only the identity pair is synthetic.
+		return Object.assign(Object.create(Object.getPrototypeOf(stat)), stat, {
+			dev: bigint ? high : Number(high), ino: bigint ? ino : Number(ino),
+		});
+	});
+	t.mock.method(fs, "fsyncSync", (fd: number) => {
+		if (replaced) return fsync(fd);
+		assert.ok(marker, "owned marker identity must be captured before the fault");
+		faults++;
+		savedContent = readFileSync(marker, "utf8");
+		renameSync(marker, `${marker}.saved`);
+		write(marker, "replacement", { mode: 0o600 });
+		replaced = true;
+		throw new Error("fixture marker fsync failure after 64-bit identity replacement");
+	});
+	syncBuiltinESMExports();
+	t.after(() => { t.mock.restoreAll(); syncBuiltinESMExports(); });
+	const registry = new CandidateViewRegistry((file, args, options) => {
+		if (args[0] === "worktree" && args[1] === "add") adds++;
+		return execFileSync(file, args, options);
+	}, "win32");
+	assert.throws(() => registry.create({ contributorRoot: cwd }), (error: unknown) => error instanceof CandidateViewError && error.reason === "candidate-owner-preparation-failed");
+	assert.equal(faults, 1, "replacement fault must be reached exactly once");
+	assert.equal(adds, 0);
+	assert.ok(marker);
+	assert.equal(readFileSync(`${marker}.saved`, "utf8"), savedContent);
+	assert.equal(readdirSync(parent).some((name) => name.endsWith(".reaper-lock")), false);
+	assert.equal(existsSync(marker), true, "publication rollback deleted the replacement after distinct 64-bit identities rounded to the same Number");
+	assert.equal(readFileSync(marker, "utf8"), "replacement");
+});
+
 test("public POSIX candidate-views parent reports bounded privacy guidance without changing permissions or adding a worktree", { skip: process.platform === "win32" }, (t) => {
 	const cwd = repository(t);
 	const parent = join(cwd, ".git", "gentle-ai", "candidate-views");
@@ -721,6 +768,47 @@ for (const race of ["root", "registration", "lock"] as const) {
 	});
 }
 
+test("ordinary cleanup preserves a replaced root with 64-bit identity precision loss", (t) => {
+	mockWindowsAcl(t);
+	const cwd = repository(t), high = 2n ** 53n;
+	const originalLstat = fs.lstatSync;
+	let root = "", replaced = false, lists = 0, removes = 0;
+	const registry = new CandidateViewRegistry((file, args, options) => {
+		// Never allow Git to delete the replacement, even on the failing implementation.
+		if (args[0] === "worktree" && args[1] === "remove") { removes++; return ""; }
+		const result = execFileSync(file, args, options);
+		if (root && args[0] === "worktree" && args[1] === "list" && ++lists === 2) {
+			const pointer = readFileSync(join(root, ".git"));
+			renameSync(root, `${root}.saved`);
+			mkdirSync(root);
+			writeFileSync(join(root, ".git"), pointer);
+			writeFileSync(join(root, "replacement.txt"), "replacement");
+			replaced = true;
+		}
+		return result;
+	});
+	const view = registry.create({ contributorRoot: cwd });
+	root = view.root;
+	t.mock.method(fs, "lstatSync", (path: Parameters<typeof fs.lstatSync>[0], ...args: unknown[]) => {
+		const stat = (originalLstat as (...arguments_: unknown[]) => ReturnType<typeof fs.lstatSync>)(path, ...args);
+		if (path !== root) return stat;
+		const bigint = (args[0] as { bigint?: boolean } | undefined)?.bigint === true;
+		const ino = replaced ? high + 1n : high;
+		return Object.assign(Object.create(Object.getPrototypeOf(stat)), stat, {
+			dev: bigint ? high : Number(high), ino: bigint ? ino : Number(ino),
+		});
+	});
+	syncBuiltinESMExports();
+	t.after(() => { t.mock.restoreAll(); syncBuiltinESMExports(); });
+	assert.throws(() => view.cleanup());
+	assert.equal(replaced, true);
+	assert.equal(readFileSync(join(root, "replacement.txt"), "utf8"), "replacement");
+	assert.equal(existsSync(`${root}.saved`), true);
+	assert.equal(existsSync(ownerMarker(root)), true);
+	assert.equal(existsSync(`${root}.reaper-lock`), false);
+	assert.equal(removes, 0, "distinct 64-bit directory identities must stop cleanup before Git removal");
+});
+
 test("ordinary cleanup never unlinks a replaced sidecar after Git removal", (t) => {
 	const cwd = repository(t);
 	let marker = "";