diff --git a/lib/orchestrator-scope.ts b/lib/orchestrator-scope.ts index 0cdfc78e8..6e1d36806 100644 --- a/lib/orchestrator-scope.ts +++ b/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(); + 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(); + 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; } diff --git a/odd/tasks/issue-2020-review-split.md b/odd/tasks/issue-2020-review-split.md new file mode 100644 index 000000000..22b7dd064 --- /dev/null +++ b/odd/tasks/issue-2020-review-split.md @@ -0,0 +1,20 @@ +# Issue 2020 review follow-up +Objective: add visibility-boundary coverage and separate Windows owner identity from PR #2068. +Branch: dnlrsls/issue-2020-subagent-git-overhead; delivery: separate focused PRs, no force-push or merge; runner: node --experimental-strip-types --test. + +## Specs +S1. User: "sip", accepting "agregar ese test y separar Windows antes del merge". Add the accepted nine-distinct-task-path visibility test: move the ninth path into view, out of view, and back; assert resolver calls and unknown-result caching while visible. +S2. User: "sip", accepting "agregar ese test y separar Windows antes del merge". Limit PR #2068 to issue #2020's repository-fact cache; preserve the Windows work for a separate relevant issue and PR. + +## Tasks +T1 | S1 | inline | done | commit ecebf79de | Coverage test and existing reorder regression passed 2/2; typecheck reported 0 diagnostics; diff check clean. Existing production eviction implements the rule, so no production RED was invented. +T2 | S2 | inline | in_progress | commit pending | Windows revert staged; owner source byte-equivalent to original baseline. Existing retry tests 7/7 pass, typecheck 0 diagnostics; native assessment medium, reviewDue=false. Commit, update description and publish cache-only scope next; do not rewrite history. +T3 | S2 | inline | pending | commit pending | Preserve Windows as an independent same-clone worktree/branch; create relevant issue and request exact issue approval before its PR. Observe checks and report any missing authorization/proof. + +## Log +L1. User original request: "sip". +L2. Accepted parent recommendation: "Recomiendo **agregar ese test y separar Windows antes del merge**. No hice cambios." +L3. Existing commits: cache 913c740381405dccaaaef3d6c2cb43ec7a6f75ea; Windows 1efccb5a01445d84ca554e0dea9f733362c4a8c8. PR #2068 currently contains both; nine CI checks passed, but CodeRabbit flagged visibility coverage and Windows scope. Native approval applies only to the original unchanged target, not future edits. Local full-suite/cache/catalog limitations remain recorded. +L4. Forecast: approximately 240 authored source/test diff lines plus task notes across work-unit commits, under the 400-line delivery heuristic. Windows separation uses independent default-base work, not stacked dependency. No docstring expansion or unrelated production fix is authorized. +L5. Job25: selected first-eight visibility test and prior reorder regression passed 2/2 (no skips); typecheck 0 recorded diagnostics/no regressions; diff check clean. Exact resolver trace proves only the newly visible/evicted path resolves at each transition and the continuously visible unknown resolves once. +L6. Job27: Windows owner source matches baseline 2d7ab4 exactly after revert; retry tests 7/7 pass, typecheck 0 diagnostics, staged diff clean. Native current-change assessment: medium, 3 paths/184 diff lines, large runtime writer, reviewDue=false under_budget; own verification stands, no separate verifier required. This is not native approval of the changed target. diff --git a/tests/orchestrator-scope.test.ts b/tests/orchestrator-scope.test.ts index d5f5b2709..2e1fc20db 100644 --- a/tests/orchestrator-scope.test.ts +++ b/tests/orchestrator-scope.test.ts @@ -59,6 +59,72 @@ 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("first-eight visibility evicts hidden facts and retains visible unknowns", () => { + const host = join(tmpdir(), "scope-visible-host"); + const tasks = Array.from({ length: 9 }, (_, i) => ({ id: `t${i}`, cwd: join(tmpdir(), `scope-visible-task-${i}`) })); + const calls: string[] = []; + const cache = new OrchestratorScopeCache(path => { + calls.push(path); + if (path === tasks[0].cwd) throw new Error("unavailable"); + return { root: path, commonDir: host }; + }); + const first = cache.project(host, tasks, []); + assert.deepEqual(calls, [host, ...tasks.slice(0, 8).map(t => t.cwd)]); + assert.equal(first.tasks[0].repository.root, null); + const unknown = first.tasks[0].repository; + const ninthVisible = [tasks[8], ...tasks.slice(0, 8)]; + const expectedCalls = [...calls]; + for (const [order, newlyVisible] of [[ninthVisible, tasks[8]], [tasks, tasks[7]], [ninthVisible, tasks[8]]] as const) { + const snapshot = cache.project(host, order, []); + expectedCalls.push(newlyVisible.cwd); + assert.deepEqual(calls, expectedCalls, "each visibility transition resolves only its evicted/newly visible path"); + assert.deepEqual(snapshot.tasks.map(t => t.id), order.slice(0, 8).map(t => t.id)); + assert.equal(snapshot.tasks.length, 8); + assert.equal(snapshot.omittedTasks, 1); + assert.equal(snapshot.complete, false); + assert.equal(validRecordedScope(snapshot), true); + assert.strictEqual(snapshot.tasks.find(t => t.id === "t0")!.repository, unknown, "visible unknown facts survive every reorder"); + assert.strictEqual(cache.project(host, order, []), snapshot); + assert.deepEqual(calls, expectedCalls, "stable snapshots never resolve again"); + } + assert.equal(calls.filter(path => path === tasks[8].cwd).length, 2, "ninth path is resolved again after eviction"); + assert.equal(calls.filter(path => path === tasks[0].cwd).length, 1, "visible unknown is not retried"); +}); + 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 }));