fix(agents): avoid repeated Git probes on recency reorders

lib/orchestrator-scope.ts

@@ -41,9 +41,14 @@ export function validRecordedScope(value: any): value is RecordedScope {
 /** One bounded derivative snapshot, not a registry or authority. The caller supplies
  * only admitted, unfinished, non-restored owned children and durable registry roots.
  * Membership/cwd changes invalidate even unknown facts; session replacement clears.
- * No resolution runs on a stable heartbeat/token update. */
+ * Recency reorders rebuild caller-ordered snapshots without refreshing visible facts.
+ * Resolution stays bounded to the host and first eight tasks/registered roots;
+ * newly visible paths are resolved lazily, including after a reorder. */
 export class OrchestratorScopeCache {
 	private key = "";
+	private membershipKey = "";
+	private facts = new Map<string, RepositoryFact>();
+	private resolvedAt = 0;
 	private snapshot?: RecordedScope;
 	private resolver: WorktreeResolver;
 	private now: () => number;
@@ -53,14 +58,24 @@ export class OrchestratorScopeCache {
 	}
 	clear() {
 		this.key = "";
+		this.membershipKey = "";
+		this.facts.clear();
 		this.snapshot = undefined;
 	}
 	project(cwd: string, tasks: readonly { id: string; cwd: string }[], roots: readonly string[]): RecordedScope {
 		const registered = [...new Set(roots)];
 		const key = createHash("sha256").update(JSON.stringify([cwd, tasks.map(t => [t.id, t.cwd]), registered])).digest("hex");
 		if (key === this.key && this.snapshot) return this.snapshot;
-		const resolvedAt = this.now();
-		const facts = new Map<string, RepositoryFact>();
+		const membershipKey = createHash("sha256").update(JSON.stringify([
+			cwd, tasks.map(t => JSON.stringify([t.id, t.cwd])).sort(), [...registered].sort(),
+		])).digest("hex");
+		if (membershipKey !== this.membershipKey) {
+			this.facts.clear();
+			this.resolvedAt = this.now();
+			this.membershipKey = membershipKey;
+		}
+		const resolvedAt = this.resolvedAt;
+		const facts = this.facts;
 		const fact = (path: string) => {
 			if (facts.has(path)) return facts.get(path)!;
 			const unknown: RepositoryFact = { root: null, cloneHash: null, resolvedAt, source: "recorded-workspace/git" };
@@ -74,6 +89,9 @@ export class OrchestratorScopeCache {
 		this.snapshot = { host: fact(cwd), tasks: tasks.slice(0, 8).map(t => ({ id: t.id, repository: fact(t.cwd) })),
 			registered: registered.slice(0, 8).map(fact), omittedTasks: Math.max(0, tasks.length - 8),
 			omittedRegistered: Math.max(0, registered.length - 8), complete: tasks.length <= 8 && registered.length <= 8 };
+		// Keep fact retention bounded even when an oversized task list rotates.
+		const visible = new Set([cwd, ...tasks.slice(0, 8).map(t => t.cwd), ...registered.slice(0, 8)]);
+		for (const path of facts.keys()) if (!visible.has(path)) facts.delete(path);
 		this.key = key;
 		return this.snapshot;
 	}

lib/review-candidate-view-owner.ts

@@ -208,19 +208,19 @@ function enforcePrivateWindowsDacl(path: string, identity: WindowsAclIdentity =
 	assertPrivateWindowsDacl(path, "directory", true, identity);
 }
 
-function privateWindowsDacl(path: string, kind: WindowsObjectKind, protectedDacl: boolean, enforce = false): void {
+function privateWindowsDacl(path: string, kind: WindowsObjectKind, protectedDacl: boolean, enforce = false, windowsIdentity?: WindowsAclIdentity): void {
 	if (testingWindowsAclAuthority !== undefined) return testingWindowsAclAuthority(path);
-	if (enforce) enforcePrivateWindowsDacl(path);
-	else assertPrivateWindowsDacl(path, kind, protectedDacl);
+	if (enforce) enforcePrivateWindowsDacl(path, windowsIdentity);
+	else assertPrivateWindowsDacl(path, kind, protectedDacl, windowsIdentity);
 }
 
-function privateWindowsCandidateOwnerBoundary(commonDir: string, enforce = false): void {
+function privateWindowsCandidateOwnerBoundary(commonDir: string, enforce = false, windowsIdentity?: WindowsAclIdentity): void {
 	const boundary = [commonDir, join(commonDir, "gentle-ai"), join(commonDir, "gentle-ai", "candidate-views")];
 	if (testingWindowsAclAuthority !== undefined) {
 		for (const path of boundary) testingWindowsAclAuthority(path);
 		return;
 	}
-	const identity = windowsAclIdentity();
+	const identity = windowsIdentity ?? windowsAclIdentity();
 	if (enforce) {
 		for (const path of boundary) validatePrivateWindowsOwner(windowsOwnerSid(path, "directory"), identity.user);
 		for (const path of boundary) enforcePrivateWindowsDacl(path, identity);
@@ -304,16 +304,23 @@ function directory(path: string, privateMode = false, platform: NodeJS.Platform
 	return `${stat.dev}:${stat.ino}`;
 }
 
-export function assertCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
+function checkedCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform): { parent: string; windowsIdentity?: WindowsAclIdentity } {
 	directory(commonDir, false, platform);
 	const control = join(commonDir, "gentle-ai");
 	directory(control, false, platform);
 	const parent = join(commonDir, "gentle-ai", "candidate-views");
 	if (platform === "win32") {
 		directory(parent, false, platform);
-		privateWindowsCandidateOwnerBoundary(commonDir);
-	} else directory(parent, true, platform, true);
-	return parent;
+		const windowsIdentity = testingWindowsAclAuthority === undefined ? windowsAclIdentity() : undefined;
+		privateWindowsCandidateOwnerBoundary(commonDir, false, windowsIdentity);
+		return { parent, windowsIdentity };
+	}
+	directory(parent, true, platform, true);
+	return { parent };
+}
+
+export function assertCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
+	return checkedCandidateOwnerParent(commonDir, platform).parent;
 }
 
 export function prepareCandidateOwnerParent(commonDir: string, platform: NodeJS.Platform = process.platform): string {
@@ -329,13 +336,13 @@ export function prepareCandidateOwnerParent(commonDir: string, platform: NodeJS.
 	return assertCandidateOwnerParent(commonDir, platform);
 }
 
-function regular(path: string, privateMode = false, platform: NodeJS.Platform = process.platform): string {
+function regular(path: string, privateMode = false, platform: NodeJS.Platform = process.platform, windowsIdentity?: WindowsAclIdentity): string {
 	// 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 !== 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);
+	if (privateMode && platform === "win32") privateWindowsDacl(path, "file", false, false, windowsIdentity);
 	return `${stat.dev}:${stat.ino}`;
 }
 
@@ -347,47 +354,50 @@ function syncDirectory(path: string): void {
 	try { fsyncSync(fd); } finally { closeSync(fd); }
 }
 
-function removeExactPrivateFile(path: string, identity: string, platform: NodeJS.Platform): void {
+function removeExactPrivateFile(path: string, identity: string, platform: NodeJS.Platform, windowsIdentity?: WindowsAclIdentity): void {
 	try {
-		if (regular(path, true, platform) !== identity) return;
+		if (regular(path, true, platform, windowsIdentity) !== identity) return;
 		unlinkSync(path);
 		syncDirectory(dirname(path));
 	} catch { /* An unproven or replaced file survives. */ }
 }
 
-function exclusiveFile(path: string, content: string, platform: NodeJS.Platform = process.platform, rollbackOnFailure = false): string {
+function exclusiveFile(path: string, content: string, platform: NodeJS.Platform = process.platform, rollbackOnFailure = false, windowsIdentity?: WindowsAclIdentity): string {
 	let identity: string | undefined;
 	try {
 		const fd = openSync(path, "wx", 0o600);
 		try {
-			identity = regular(path, true, platform);
+			identity = regular(path, true, platform, windowsIdentity);
 			writeFileSync(fd, content);
 			fsyncSync(fd);
 		} finally { closeSync(fd); }
 		syncDirectory(dirname(path));
 		return identity;
 	} catch (error) {
-		if (rollbackOnFailure && identity !== undefined) removeExactPrivateFile(path, identity, platform);
+		if (rollbackOnFailure && identity !== undefined) removeExactPrivateFile(path, identity, platform, windowsIdentity);
 		throw error;
 	}
 }
 
 function markerPath(root: string): string { return `${root}.owner.json`; }
 
 export function createCandidateOwner(commonDir: string, root: string, platform: NodeJS.Platform = process.platform): CandidateViewOwner {
-	const parent = assertCandidateOwnerParent(commonDir, platform);
+	// Reuse only identity within this module-controlled synchronous creation.
+	// Owner/DACL observations stay fresh, including rollback. Nothing is retained
+	// across calls or passed into callback-based removal/sweeping operations.
+	const { parent, windowsIdentity } = checkedCandidateOwnerParent(commonDir, platform);
 	const uuid = basename(root);
 	if (!UUID.test(uuid) || root !== join(parent, uuid) || lstatSync(root, { throwIfNoEntry: false })) throw new Error("Unsafe candidate owner root");
 	const owner: CandidateViewOwner = { version: 1, uuid, token: randomUUID(), pid: process.pid, host: localHost(), root, commonDir };
 	// Any write/fsync failure aborts creation BEFORE Git can register the view.
 	const marker = markerPath(root);
 	let markerIdentity: string | undefined;
 	try {
-		markerIdentity = exclusiveFile(marker, JSON.stringify(owner), platform, true);
+		markerIdentity = exclusiveFile(marker, JSON.stringify(owner), platform, true, windowsIdentity);
 		syncDirectory(dirname(parent));
 		syncDirectory(commonDir);
 	} catch (error) {
-		if (markerIdentity !== undefined) removeExactPrivateFile(marker, markerIdentity, platform);
+		if (markerIdentity !== undefined) removeExactPrivateFile(marker, markerIdentity, platform, windowsIdentity);
 		throw error;
 	}
 	return Object.freeze(owner);

tests/orchestrator-scope.test.ts

@@ -59,6 +59,40 @@ test("recorded host/owned child/registered scope correlates clones, isolates amb
 	assert.equal(new OrchestratorScopeCache(() => ({ root: "/repo\nother", commonDir: "/clone" })).project(repo, [], []).host.root, null);
 });
 
+test("recency reorders preserve repository facts but retain caller order", () => {
+	const host = join(tmpdir(), "scope-host"), a = join(tmpdir(), "scope-a"), b = join(tmpdir(), "scope-b");
+	let calls = 0, now = 1;
+	const cache = new OrchestratorScopeCache(path => {
+		calls++;
+		if (path === b) throw new Error("unavailable");
+		return { root: path, commonDir: host };
+	}, () => now++);
+	const tasks = [{ id: "a", cwd: a }, { id: "b", cwd: b }];
+	const first = cache.project(host, tasks, [a, b]);
+	assert.equal(calls, 3);
+	const reordered = cache.project(host, [...tasks].reverse(), [b, a]);
+	assert.equal(calls, 3, "pure reorders must not probe Git, including cached unknowns");
+	assert.deepEqual(reordered.tasks, [...first.tasks].reverse());
+	assert.deepEqual(reordered.registered, [...first.registered].reverse());
+	assert.deepEqual(reordered.host, first.host);
+	assert.equal(reordered.tasks[0].repository.root, null);
+	assert.equal(validRecordedScope(reordered), true);
+	assert.strictEqual(cache.project(host, [...tasks].reverse(), [b, a]), reordered);
+
+	const added = cache.project(host, [...tasks, { id: "c", cwd: host }], [a, b]);
+	assert.equal(calls, 6, "membership changes refresh even existing facts");
+	assert.ok(added.host.resolvedAt > first.host.resolvedAt);
+	cache.project(host, tasks, [a, b]);
+	assert.equal(calls, 9, "removal refreshes facts");
+	cache.project(host, [{ id: "a", cwd: host }, tasks[1]], [a, b]);
+	assert.equal(calls, 12, "cwd changes refresh facts");
+	cache.project(host, [{ id: "a", cwd: host }, tasks[1]], [a]);
+	assert.equal(calls, 15, "registered membership changes refresh facts");
+	cache.clear();
+	cache.project(host, tasks, [a, b]);
+	assert.equal(calls, 18, "session replacement clears facts");
+});
+
 test("literal recorded roots never alias Unicode separators to another real repository", (t) => {
 	const base = realpathSync(mkdtempSync(join(tmpdir(), "scope-literal-")));
 	t.after(() => rmSync(base, { recursive: true, force: true }));

tests/review-owner-identity.test.ts

@@ -0,0 +1,132 @@
+import assert from "node:assert/strict";
+import childProcess from "node:child_process";
+import { randomUUID } from "node:crypto";
+import fs from "node:fs";
+import { syncBuiltinESMExports } from "node:module";
+import { dirname, join, resolve } from "node:path";
+import test from "node:test";
+import { createCandidateOwner, prepareCandidateOwnerParent } from "../lib/review-candidate-view-owner.ts";
+
+function fixture(t: test.TestContext) {
+	fs.mkdirSync(resolve("tmp"), { recursive: true });
+	const base = fs.realpathSync(fs.mkdtempSync(join(resolve("tmp"), "owner-identity-")));
+	const commonDir = join(base, "common");
+	const parent = join(commonDir, "gentle-ai", "candidate-views");
+	fs.mkdirSync(parent, { recursive: true });
+	const boundary = [commonDir, dirname(parent), parent];
+	let user = "S-1-5-21-1-2-3-1001";
+	let identityFailure = "";
+	let replacedMarker = false;
+	const probes = { user: 0, administrator: 0, owner: [] as string[], dacl: [] as string[], enforce: [] as string[] };
+	const native = fs.realpathSync.native;
+	t.mock.method(fs.realpathSync, "native", (path: string) =>
+		path.startsWith("\\\\?\\GLOBALROOT\\SystemRoot\\System32") ? path : native(path));
+	t.mock.method(childProcess, "execFileSync", (command: string, args: string[], options: any) => {
+		const name = command.split(/[\\/]/).at(-1);
+		if (name === "whoami.exe") {
+			probes.user++;
+			return identityFailure === "user" ? "unavailable" : user;
+		}
+		if (name === "icacls.exe") {
+			probes.dacl.push(args[0]);
+			const directory = fs.lstatSync(args[0]).isDirectory();
+			const flags = directory ? "OICI" : "ID";
+			const dacl = `D:${directory ? "P" : ""}(A;${flags};FA;;;${user})(A;${flags};FA;;;SY)(A;${flags};FA;;;BA)`;
+			fs.writeFileSync(join(options.cwd, args[2]), dacl, "utf16le");
+			return "";
+		}
+		if (name === "powershell.exe") {
+			const script = Buffer.from(args.at(-1)!, "base64").toString("utf16le");
+			if (script.includes("RawSecurityDescriptor")) {
+				probes.administrator++;
+				return identityFailure === "administrator" ? "unavailable" : "S-1-5-21-1-2-3-500";
+			}
+			if (script.includes("SetAccessControl")) {
+				probes.enforce.push(options.env.GENTLE_PI_CANDIDATE_ACL_PATH);
+				return "";
+			}
+			const path = options.env.GENTLE_PI_CANDIDATE_OWNER_PATH;
+			probes.owner.push(path);
+			return replacedMarker && path.endsWith(".owner.json") ? "S-1-5-21-9-8-7-1002" : user;
+		}
+		throw new Error(`Unexpected subprocess: ${name}`);
+	});
+	syncBuiltinESMExports();
+	t.after(() => {
+		t.mock.restoreAll();
+		syncBuiltinESMExports();
+		fs.rmSync(base, { recursive: true, force: true });
+	});
+	return { commonDir, parent, boundary, probes,
+		changeUser: () => { user = "S-1-5-21-4-5-6-1002"; },
+		failIdentity: (kind: string) => { identityFailure = kind; },
+		replaceMarker: () => { replacedMarker = true; } };
+}
+
+test("Windows creation reuses identity only within one operation and preserves every filesystem probe", (t) => {
+	const f = fixture(t);
+	for (let operation = 1; operation <= 2; operation++) {
+		if (operation === 2) f.changeUser();
+		const root = join(f.parent, randomUUID());
+		const owner = createCandidateOwner(f.commonDir, root, "win32");
+		const marker = `${root}.owner.json`;
+		assert.equal(owner.root, root);
+		assert.deepEqual(JSON.parse(fs.readFileSync(marker, "utf8")), owner);
+		assert.equal(fs.existsSync(root), false);
+		assert.equal(f.probes.user, operation, "one fresh user SID per creation");
+		assert.equal(f.probes.administrator, operation, "one fresh administrator SID per creation");
+		assert.deepEqual(f.probes.owner.slice((operation - 1) * 4), [...f.boundary, marker]);
+		assert.deepEqual(f.probes.dacl.slice((operation - 1) * 4), [...f.boundary, marker]);
+		assert.deepEqual(f.probes.enforce, []);
+	}
+});
+
+test("Windows preparation retains fresh pre-write and post-write owner/DACL checks", (t) => {
+	const f = fixture(t);
+	assert.equal(prepareCandidateOwnerParent(f.commonDir, "win32"), f.parent);
+	assert.equal(f.probes.user, 1);
+	assert.equal(f.probes.administrator, 1);
+	assert.deepEqual(f.probes.owner, [...f.boundary, ...f.boundary.flatMap(path => [path, path])]);
+	assert.deepEqual(f.probes.dacl, f.boundary);
+	assert.deepEqual(f.probes.enforce, f.boundary);
+});
+
+for (const kind of ["user", "administrator"]) test(`Windows creation fails closed on fresh ${kind} identity failure`, (t) => {
+	const f = fixture(t);
+	const first = createCandidateOwner(f.commonDir, join(f.parent, randomUUID()), "win32");
+	const marker = `${first.root}.owner.json`;
+	const before = fs.readFileSync(marker, "utf8");
+	const stored = fs.readdirSync(f.parent);
+	f.failIdentity(kind);
+	const root = join(f.parent, randomUUID());
+	assert.throws(() => createCandidateOwner(f.commonDir, root, "win32"), /SID is unavailable/);
+	assert.equal(fs.existsSync(root), false);
+	assert.equal(fs.existsSync(`${root}.owner.json`), false);
+	assert.equal(fs.readFileSync(marker, "utf8"), before);
+	assert.deepEqual(fs.readdirSync(f.parent), stored);
+	assert.equal(f.probes.owner.length, 4);
+	assert.equal(f.probes.dacl.length, 4);
+});
+
+test("Windows creation rollback rechecks a replaced marker owner before removal", (t) => {
+	const f = fixture(t);
+	const write = fs.writeFileSync;
+	const root = join(f.parent, randomUUID());
+	const marker = `${root}.owner.json`;
+	t.mock.method(fs, "writeFileSync", (...args: any[]) => {
+		if (typeof args[0] === "number") {
+			f.replaceMarker();
+			throw new Error("fixture write failure");
+		}
+		return (write as any)(...args);
+	});
+	syncBuiltinESMExports();
+	assert.throws(() => createCandidateOwner(f.commonDir, root, "win32"), /fixture write failure/);
+	assert.equal(f.probes.user, 1);
+	assert.equal(f.probes.administrator, 1);
+	assert.deepEqual(f.probes.owner, [...f.boundary, marker, marker]);
+	assert.deepEqual(f.probes.dacl, [...f.boundary, marker]);
+	assert.equal(fs.existsSync(marker), true, "unproven marker survives rollback");
+	assert.equal(fs.readFileSync(marker, "utf8"), "");
+	assert.equal(fs.existsSync(root), false);
+});