Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions lib/orchestrator-scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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" };
Expand All @@ -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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.key = key;
return this.snapshot;
}
Expand Down
20 changes: 20 additions & 0 deletions odd/tasks/issue-2020-review-split.md
Original file line number Diff line number Diff line change
@@ -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.
66 changes: 66 additions & 0 deletions tests/orchestrator-scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand Down
Loading