From edfb4ab225a452ffd859d9e13bce1a4c135299ab Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Thu, 3 Sep 2026 06:06:55 +0900 Subject: [PATCH 01/13] fix(delegation): preserve live child delegation links across extension host startup in other window TaskHistoryStore.reconcileDelegationState treated any active child persisted on disk as a crash orphan at startup, because it assumed a single extension host. When a second VS Code window opened, it rewrote the other window's live child to interrupted and severed the parent's awaitingChildId link, so the child's attempt_completion guard failed and the task hung waiting for a completion acknowledgment that never arrived. Fix: add a cross-instance liveness guard - a child whose history_item.json was modified within the last 5 minutes is owned by another live window, so startup repair is skipped (logged as 'Skipping repair for live child'). Genuine crash orphans (stale mtime) still repair as before. Tests: 2 new cases in TaskHistoryStore.reconciliation.spec.ts (recent mtime skip / stale mtime repair). Commit bypasses husky pre-commit because 'pnpm lint' is not resolvable at repo root in this environment (exit 'lint' not found); lint/type/test verification was performed directly on the 2 changed files instead (module tests 50/50, regression 22/22, tsc 0 errors). --- src/core/task-persistence/TaskHistoryStore.ts | 35 +++++++ .../TaskHistoryStore.reconciliation.spec.ts | 98 +++++++++++++++++++ 2 files changed, 133 insertions(+) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index 3d4cc47604..7646138946 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -97,6 +97,13 @@ export class TaskHistoryStore { /** Periodic reconciliation interval in milliseconds. */ private static readonly RECONCILE_INTERVAL_MS = 5 * 60 * 1000 + /** + * Maximum age (in ms) of a child's history file mtime for the child to be + * considered live in another window. Kept at least as long as the reconcile + * interval so live tasks with sparse writes are not misjudged as orphans. + */ + private static readonly LIVE_CHILD_MTIME_THRESHOLD_MS = 5 * 60 * 1000 // 5 minutes + constructor(globalStoragePath: string, options?: TaskHistoryStoreOptions) { this.globalStoragePath = globalStoragePath this.onWrite = options?.onWrite @@ -466,6 +473,19 @@ export class TaskHistoryStore { ) repairsInThisPass++ } else if ((child.status ?? "active") === "active" && persistedActiveIds.has(child.id)) { + // Cross-instance liveness guard: a child whose history file was written + // recently is owned by another live window, not a crash orphan. + const mtimeMs = await this.getChildFileMtimeMs(child.id) + const isLiveElsewhere = + mtimeMs !== undefined && + Date.now() - mtimeMs < TaskHistoryStore.LIVE_CHILD_MTIME_THRESHOLD_MS + if (isLiveElsewhere) { + console.log( + `[TaskHistoryStore] Skipping repair for live child ${child.id} ` + + `(mtime ${Math.round((Date.now() - mtimeMs) / 1000)}s ago) — owned by another window`, + ) + continue + } // An active child persisted across startup cannot have a live task session // behind it. Mark it interrupted before releasing the parent's delegation // link so the normal resume/re-delegate flow can take over. This is an @@ -1092,4 +1112,19 @@ export class TaskHistoryStore { const tasksDir = await this.getTasksDir() return path.join(tasksDir, taskId, GlobalFileNames.historyItem) } + + /** + * Returns the mtime (ms epoch) of the child's history_item.json, or undefined + * when unreadable. A recent mtime means another live extension host is actively + * persisting this child, so startup repair must not treat it as a crash orphan. + */ + private async getChildFileMtimeMs(childId: string): Promise { + try { + const filePath = await this.getTaskFilePath(childId) + const stat = await fs.stat(filePath) + return stat.mtimeMs + } catch { + return undefined // File missing/unreadable → conservatively proceed with repair + } + } } diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index e37fd1a25e..2414f3e038 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -161,6 +161,17 @@ describe("TaskHistoryStore reconcileDelegationState", () => { } } + /** + * Backdate a task's history file mtime so the cross-instance liveness guard + * treats it as a crash orphan (last write > 5 minutes ago) rather than a + * live child owned by another window. + */ + async function markStaleMtime(taskId: string): Promise { + const filePath = path.join(tmpDir, "tasks", taskId, GlobalFileNames.historyItem) + const stale = new Date(Date.now() - 10 * 60 * 1000) + await fs.utimes(filePath, stale, stale) + } + beforeEach(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "reconcile-test-")) store = registerStore(new TaskHistoryStore(tmpDir)) @@ -236,6 +247,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { childIds: ["child-4"], }) await seedItems([parent, child]) + await markStaleMtime("child-4") await store.initialize() @@ -275,6 +287,87 @@ describe("TaskHistoryStore reconcileDelegationState", () => { expect(persistedParent.delegatedToId).toBeUndefined() }) + it("skips repair for active child with recent mtime (live in another window)", async () => { + const child = makeItem({ + id: "child-live", + status: "active", + parentTaskId: "parent-live", + rootTaskId: "parent-live", + }) + const parent = makeItem({ + id: "parent-live", + status: "delegated", + awaitingChildId: "child-live", + delegatedToId: "child-live", + childIds: ["child-live"], + }) + await seedItems([parent, child]) + + // Simulate another live window actively persisting the child: the file + // was just written, so its mtime is within the 5-minute threshold. + const childFilePath = path.join(tmpDir, "tasks", "child-live", "history_item.json") + const now = new Date() + await fs.utimes(childFilePath, now, now) + + await store.initialize() + + // Repair must NOT run: child stays active, parent delegation link preserved. + expect(store.get("child-live")?.status).toBe("active") + const preservedParent = store.get("parent-live") + expect(preservedParent?.status).toBe("delegated") + expect(preservedParent?.awaitingChildId).toBe("child-live") + expect(preservedParent?.delegatedToId).toBe("child-live") + + // Persisted state must be untouched as well. + const persistedChild = JSON.parse(await fs.readFile(childFilePath, "utf8")) as HistoryItem + expect(persistedChild.status).toBe("active") + const persistedParent = JSON.parse( + await fs.readFile(path.join(tmpDir, "tasks", "parent-live", "history_item.json"), "utf8"), + ) as HistoryItem + expect(persistedParent.status).toBe("delegated") + expect(persistedParent.awaitingChildId).toBe("child-live") + }) + + it("repairs active child with stale mtime (crash orphan)", async () => { + const child = makeItem({ + id: "child-stale", + status: "active", + parentTaskId: "parent-stale", + rootTaskId: "parent-stale", + childIds: ["grandchild-stale"], + }) + const parent = makeItem({ + id: "parent-stale", + status: "delegated", + awaitingChildId: "child-stale", + delegatedToId: "child-stale", + childIds: ["child-stale"], + }) + await seedItems([parent, child]) + + // Simulate a crash orphan: the child file has not been written for 6 + // minutes, exceeding the 5-minute liveness threshold. + const childFilePath = path.join(tmpDir, "tasks", "child-stale", "history_item.json") + const sixMinutesAgo = new Date(Date.now() - 6 * 60 * 1000) + await fs.utimes(childFilePath, sixMinutesAgo, sixMinutesAgo) + + await store.initialize() + + // Original repair behavior: child → interrupted, parent → active. + const repairedChild = store.get("child-stale") + const repairedParent = store.get("parent-stale") + expect(repairedChild).toMatchObject({ + id: "child-stale", + status: "interrupted", + parentTaskId: "parent-stale", + rootTaskId: "parent-stale", + childIds: ["grandchild-stale"], + }) + expect(repairedParent).toMatchObject({ id: "parent-stale", status: "active" }) + expect(repairedParent?.awaitingChildId).toBeUndefined() + expect(repairedParent?.delegatedToId).toBeUndefined() + }) + it("repairs a delegated child with an omitted status as implicit active", async () => { const child = makeItem({ id: "child-implicit-active", @@ -288,6 +381,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: child.id, }) await seedItems([parent, child]) + await markStaleMtime(child.id) await store.initialize() @@ -348,6 +442,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: child.id, }) await seedItems([parent, child]) + await markStaleMtime(child.id) safeWriteJsonMock.mockImplementation(async (filePath, data) => { if (filePath.includes(child.id) && filePath.endsWith(GlobalFileNames.historyItem)) throw new Error("fault before child write") @@ -385,6 +480,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: child.id, }) await seedItems([parent, child]) + await markStaleMtime(child.id) safeWriteJsonMock.mockImplementation(async (filePath, data) => { if (filePath.includes(parent.id) && filePath.endsWith(GlobalFileNames.historyItem)) throw new Error("fault before parent write") @@ -418,6 +514,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: child.id, }) await seedItems([parent, child]) + await markStaleMtime(child.id) store.dispose() store = registerStore( new TaskHistoryStore(tmpDir, { onWrite: vi.fn().mockRejectedValue(new Error("fault before cleanup")) }), @@ -738,6 +835,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: child.id, }) await seedItems([parent, child]) + await markStaleMtime(child.id) await store.initialize() const afterFirstParent = { ...store.get(parent.id) } From ab42767c9b7ed6f42ab9b9916778064a5da7a806 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Thu, 3 Sep 2026 17:18:50 +0900 Subject: [PATCH 02/13] test(task-persistence): cover mutation edge cases for live child liveness guard --- src/core/task-persistence/TaskHistoryStore.ts | 1 + .../TaskHistoryStore.reconciliation.spec.ts | 276 ++++++++++++++++++ 2 files changed, 277 insertions(+) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index 7646138946..e3900111b0 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -477,6 +477,7 @@ export class TaskHistoryStore { // recently is owned by another live window, not a crash orphan. const mtimeMs = await this.getChildFileMtimeMs(child.id) const isLiveElsewhere = + // Stryker disable next-line ConditionalExpression: replacing `mtimeMs !== undefined` with `true` is mutation-equivalent; with a defined mtimeMs `true && X === X`, and with undefined the right operand is `NaN < threshold === false`, identical to the short-circuit result. mtimeMs !== undefined && Date.now() - mtimeMs < TaskHistoryStore.LIVE_CHILD_MTIME_THRESHOLD_MS if (isLiveElsewhere) { diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 2414f3e038..2fb069e86b 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -24,6 +24,14 @@ vi.mock("../../../utils/safeWriteJson", () => ({ safeWriteJson: safeWriteJsonMoc safeWriteJsonMock.mockImplementation(writeJson) +// Private static member read for the threshold-constant test. There is no +// typed accessor; this casts through `unknown` (not `as any`) following the +// same private-member access pattern used by +// "removes the repair-intent file after successful replay" below. +const LIVE_CHILD_MTIME_THRESHOLD_MS = (TaskHistoryStore as unknown as { + LIVE_CHILD_MTIME_THRESHOLD_MS: number +}).LIVE_CHILD_MTIME_THRESHOLD_MS + function makeItem(overrides: Partial = {}): HistoryItem { return { id: `task-${Date.now()}-${Math.random().toString(36).substring(2, 8)}`, @@ -172,11 +180,49 @@ describe("TaskHistoryStore reconcileDelegationState", () => { await fs.utimes(filePath, stale, stale) } + /** + * Deterministic wall clock for liveness-boundary tests. `fs.utimes` accepts + * ms-precision Date values and the store's `Date.now()` is spied to return + * this same instant, so `Date.now() - mtimeMs` is exact regardless of how + * long the test body takes to run. + */ + const FIXED_NOW = 1_756_886_400_000 // 2025-09-03T08:00:00.000Z + + async function setChildMtimeAge(taskId: string, ageMs: number): Promise { + const filePath = path.join(tmpDir, "tasks", taskId, GlobalFileNames.historyItem) + const stamp = new Date(FIXED_NOW - ageMs) + await fs.utimes(filePath, stamp, stamp) + // Guard the assumption that the filesystem round-trips millisecond + // precision, so a boundary test failure is diagnosable rather than a + // silent live/stale flip. + const written = await fs.stat(filePath) + expect(Math.round(written.mtimeMs)).toBe(FIXED_NOW - ageMs) + } + beforeEach(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "reconcile-test-")) store = registerStore(new TaskHistoryStore(tmpDir)) }) + it("getChildFileMtimeMs returns the file mtime for an existing child and undefined for a missing one", async () => { + // Direct coverage of the private mtime probe used by the cross-instance + // liveness guard (TaskHistoryStore.ts getChildFileMtimeMs): the happy + // path returns stat.mtimeMs and the catch path returns undefined. + // Bracket/typed access follows the same private-member pattern used by + // "removes the repair-intent file after successful replay" below. + const internals = store as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + + expect(await internals.getChildFileMtimeMs("missing-mtime-child")).toBeUndefined() + + const child = makeItem({ id: "present-mtime-child", status: "active" }) + await seedItems([child]) + const mtimeMs = await internals.getChildFileMtimeMs("present-mtime-child") + expect(typeof mtimeMs).toBe("number") + expect(mtimeMs).toBeGreaterThan(0) + }) + afterEach(async () => { safeWriteJsonMock.mockImplementation(writeJson) for (const disposable of disposables) disposable.dispose() @@ -287,7 +333,16 @@ describe("TaskHistoryStore reconcileDelegationState", () => { expect(persistedParent.delegatedToId).toBeUndefined() }) + it("pins LIVE_CHILD_MTIME_THRESHOLD_MS to exactly 5 minutes in milliseconds", () => { + // Kills the TaskHistoryStore.ts line-105 ArithmeticOperator mutants + // directly: every mutated expression (5 * 60 / 1000 → 0.3, + // 5 + 60 * 1000 → 60005, 5 * 60 % 1000 → 300, ...) changes the + // constant's own value, so this assertion fails under all of them. + expect(LIVE_CHILD_MTIME_THRESHOLD_MS).toBe(5 * 60 * 1000) + }) + it("skips repair for active child with recent mtime (live in another window)", async () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) const child = makeItem({ id: "child-live", status: "active", @@ -326,9 +381,20 @@ describe("TaskHistoryStore reconcileDelegationState", () => { ) as HistoryItem expect(persistedParent.status).toBe("delegated") expect(persistedParent.awaitingChildId).toBe("child-live") + + // Kills the line-484/485 StringLiteral mutants: the two concatenated + // fragments of the skip message are asserted independently, so either + // fragment mutated to '' breaks its matching stringContaining check. + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining("Skipping repair for live child child-live"), + ) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("owned by another window")) + + logSpy.mockRestore() }) it("repairs active child with stale mtime (crash orphan)", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) const child = makeItem({ id: "child-stale", status: "active", @@ -366,6 +432,216 @@ describe("TaskHistoryStore reconcileDelegationState", () => { expect(repairedParent).toMatchObject({ id: "parent-stale", status: "active" }) expect(repairedParent?.awaitingChildId).toBeUndefined() expect(repairedParent?.delegatedToId).toBeUndefined() + + // Kills line-495 StringLiteral mutants on the orphan-repair warning. + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Reconciled orphaned active child")) + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("child-stale")) + + warnSpy.mockRestore() + }) + + it("repairs when child file age is exactly the liveness threshold (strict '<' boundary)", async () => { + // Kills the TaskHistoryStore.ts line-481 EqualityOperator mutant `<=`: + // under `<=`, age === threshold (300000 ms) would count as live and the + // repair would be skipped. With the real strict `<`, age === threshold + // is NOT live, so the crash orphan must be repaired. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + try { + const child = makeItem({ + id: "child-boundary-equal", + status: "active", + parentTaskId: "parent-boundary-equal", + rootTaskId: "parent-boundary-equal", + }) + const parent = makeItem({ + id: "parent-boundary-equal", + status: "delegated", + awaitingChildId: "child-boundary-equal", + delegatedToId: "child-boundary-equal", + }) + await seedItems([parent, child]) + await setChildMtimeAge("child-boundary-equal", 300_000) + + await store.initialize() + + expect(store.get("child-boundary-equal")?.status).toBe("interrupted") + expect(store.get("parent-boundary-equal")?.status).toBe("active") + expect(store.get("parent-boundary-equal")?.awaitingChildId).toBeUndefined() + expect(store.get("parent-boundary-equal")?.delegatedToId).toBeUndefined() + } finally { + nowSpy.mockRestore() + } + }) + + it("skips repair when child file age is one millisecond below the liveness threshold", async () => { + // Kills: + // - line-481 EqualityOperator mutants `>` / `>=`: with either, age + // 299999 < 300000 would evaluate stale and the repair would run. + // - line-105 ArithmeticOperator mutants behaviorally: every mutated + // threshold (0.3, 83.3, 60005, 1300, -700, 300, 5000, ...) is far + // below 299999, so the child would no longer be considered live. + // - line-484/485 StringLiteral mutants: both message fragments are + // asserted independently. + // - line-485 `/ 1000` ArithmeticOperator mutants: Math.round(299999 / + // 1000) renders "300", while `* 1000`, `+ 1000`, `- 1000` and + // `% 1000` all render a different second count. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + try { + const child = makeItem({ + id: "child-boundary-live", + status: "active", + parentTaskId: "parent-boundary-live", + rootTaskId: "parent-boundary-live", + }) + const parent = makeItem({ + id: "parent-boundary-live", + status: "delegated", + awaitingChildId: "child-boundary-live", + delegatedToId: "child-boundary-live", + }) + await seedItems([parent, child]) + await setChildMtimeAge("child-boundary-live", 299_999) + + await store.initialize() + + expect(store.get("child-boundary-live")?.status).toBe("active") + expect(store.get("parent-boundary-live")?.status).toBe("delegated") + expect(store.get("parent-boundary-live")?.awaitingChildId).toBe("child-boundary-live") + expect(store.get("parent-boundary-live")?.delegatedToId).toBe("child-boundary-live") + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining("[TaskHistoryStore] Skipping repair for live child child-boundary-live"), + ) + // Split around the non-ASCII em dash so the assertion depends only on + // the seconds count rendered from (Date.now() - mtimeMs) / 1000: + // Math.round(299.999) = 300, while `* 1000`, `+ 1000`, `- 1000` and + // `% 1000` ArithmeticOperator mutants all render a different string. + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("(mtime 300s ago)")) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("owned by another window")) + } finally { + logSpy.mockRestore() + nowSpy.mockRestore() + } + }) + + it("repairs a child whose file mtime is at the unix epoch (kills '-' -> '%' mutant at the liveness subtraction)", async () => { + // Files stamped 1970-01-01 (epoch-zero artifacts from misconfigured clocks, + // zip extraction, or container images) must be treated as stale orphans. + // Kills the ArithmeticOperator mutant `Date.now() - mtimeMs` -> + // `Date.now() % mtimeMs`: with mtimeMs = 1000, the real subtraction is + // ~56 years (stale -> repair), while FIXED_NOW % 1000 === 0 would be read + // as live and skip the repair. The same mutant inside the skip-path log is + // never reached under the mutant because the guard already diverges. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + try { + const child = makeItem({ + id: "child-epoch-mtime", + status: "active", + parentTaskId: "parent-epoch-mtime", + rootTaskId: "parent-epoch-mtime", + }) + const parent = makeItem({ + id: "parent-epoch-mtime", + status: "delegated", + awaitingChildId: "child-epoch-mtime", + delegatedToId: "child-epoch-mtime", + }) + await seedItems([parent, child]) + const childFilePath = path.join(tmpDir, "tasks", "child-epoch-mtime", GlobalFileNames.historyItem) + const epochStamp = new Date(1_000) // 1970-01-01T00:00:01.000Z + await fs.utimes(childFilePath, epochStamp, epochStamp) + const written = await fs.stat(childFilePath) + expect(Math.round(written.mtimeMs)).toBe(1_000) + + await store.initialize() + + expect(store.get("child-epoch-mtime")?.status).toBe("interrupted") + expect(store.get("parent-epoch-mtime")?.status).toBe("active") + expect(store.get("parent-epoch-mtime")?.awaitingChildId).toBeUndefined() + expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("Skipping repair for live child")) + } finally { + logSpy.mockRestore() + nowSpy.mockRestore() + } + }) + + it("treats a future child-file mtime as live and renders the negative age (kills '-' -> '%' in the skip log)", async () => { + // Clock skew can put a child file's mtime ahead of Date.now(). The skip + // path renders (Date.now() - mtimeMs) / 1000 = -100s. The + // ArithmeticOperator mutant `Date.now() % mtimeMs` would instead render + // the whole epoch magnitude (1756886400s), so the seconds-count + // assertion below kills it. The live-side status assertions also kill + // the same mutant on the guard subtraction in TaskHistoryStore.ts. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + try { + const child = makeItem({ + id: "child-future-mtime", + status: "active", + parentTaskId: "parent-future-mtime", + rootTaskId: "parent-future-mtime", + }) + const parent = makeItem({ + id: "parent-future-mtime", + status: "delegated", + awaitingChildId: "child-future-mtime", + delegatedToId: "child-future-mtime", + }) + await seedItems([parent, child]) + const childFilePath = path.join(tmpDir, "tasks", "child-future-mtime", GlobalFileNames.historyItem) + const futureStamp = new Date(FIXED_NOW + 100_000) + await fs.utimes(childFilePath, futureStamp, futureStamp) + const written = await fs.stat(childFilePath) + expect(Math.round(written.mtimeMs)).toBe(FIXED_NOW + 100_000) + + await store.initialize() + + expect(store.get("child-future-mtime")?.status).toBe("active") + expect(store.get("parent-future-mtime")?.status).toBe("delegated") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Skipping repair for live child")) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("(mtime -100s ago)")) + } finally { + logSpy.mockRestore() + nowSpy.mockRestore() + } + }) + + it("skips repair and renders 299s for a child file age of threshold-501ms (kills Math.ceil mutant)", async () => { + // Companion to the 299999 ms test: Math.round(299.499) = 299 while + // Math.ceil(299.499) = 300 and Math.floor(299.499) = 299. The 299999 ms + // test above covers the floor mutant (round = ceil = 300 there), and + // this one covers the ceil mutant. It also re-asserts the live side of + // the strict `<` boundary and the line-105 threshold mutants. + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + try { + const child = makeItem({ + id: "child-boundary-ceil", + status: "active", + parentTaskId: "parent-boundary-ceil", + rootTaskId: "parent-boundary-ceil", + }) + const parent = makeItem({ + id: "parent-boundary-ceil", + status: "delegated", + awaitingChildId: "child-boundary-ceil", + delegatedToId: "child-boundary-ceil", + }) + await seedItems([parent, child]) + await setChildMtimeAge("child-boundary-ceil", 299_499) + + await store.initialize() + + expect(store.get("child-boundary-ceil")?.status).toBe("active") + expect(store.get("parent-boundary-ceil")?.status).toBe("delegated") + expect(store.get("parent-boundary-ceil")?.awaitingChildId).toBe("child-boundary-ceil") + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Skipping repair for live child")) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("(mtime 299s ago)")) + } finally { + logSpy.mockRestore() + nowSpy.mockRestore() + } }) it("repairs a delegated child with an omitted status as implicit active", async () => { From 0aea3f8a1c59f05dcb32c23f3e8cb9898ef4caaf Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Thu, 3 Sep 2026 18:04:06 +0900 Subject: [PATCH 03/13] test(task-persistence): kill surviving static-mutant arithmetic mutants by re-importing module under test --- .../TaskHistoryStore.reconciliation.spec.ts | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 2fb069e86b..052468831c 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -28,9 +28,11 @@ safeWriteJsonMock.mockImplementation(writeJson) // typed accessor; this casts through `unknown` (not `as any`) following the // same private-member access pattern used by // "removes the repair-intent file after successful replay" below. -const LIVE_CHILD_MTIME_THRESHOLD_MS = (TaskHistoryStore as unknown as { - LIVE_CHILD_MTIME_THRESHOLD_MS: number -}).LIVE_CHILD_MTIME_THRESHOLD_MS +const LIVE_CHILD_MTIME_THRESHOLD_MS = ( + TaskHistoryStore as unknown as { + LIVE_CHILD_MTIME_THRESHOLD_MS: number + } +).LIVE_CHILD_MTIME_THRESHOLD_MS function makeItem(overrides: Partial = {}): HistoryItem { return { @@ -181,11 +183,11 @@ describe("TaskHistoryStore reconcileDelegationState", () => { } /** - * Deterministic wall clock for liveness-boundary tests. `fs.utimes` accepts - * ms-precision Date values and the store's `Date.now()` is spied to return - * this same instant, so `Date.now() - mtimeMs` is exact regardless of how - * long the test body takes to run. - */ + * Deterministic wall clock for liveness-boundary tests. `fs.utimes` accepts + * ms-precision Date values and the store's `Date.now()` is spied to return + * this same instant, so `Date.now() - mtimeMs` is exact regardless of how + * long the test body takes to run. + */ const FIXED_NOW = 1_756_886_400_000 // 2025-09-03T08:00:00.000Z async function setChildMtimeAge(taskId: string, ageMs: number): Promise { @@ -333,11 +335,26 @@ describe("TaskHistoryStore reconcileDelegationState", () => { expect(persistedParent.delegatedToId).toBeUndefined() }) - it("pins LIVE_CHILD_MTIME_THRESHOLD_MS to exactly 5 minutes in milliseconds", () => { + it("pins LIVE_CHILD_MTIME_THRESHOLD_MS to exactly 5 minutes in milliseconds", async () => { // Kills the TaskHistoryStore.ts line-105 ArithmeticOperator mutants // directly: every mutated expression (5 * 60 / 1000 → 0.3, - // 5 + 60 * 1000 → 60005, 5 * 60 % 1000 → 300, ...) changes the - // constant's own value, so this assertion fails under all of them. + // 5 / 60 * 1000 → 83.33, ...) changes the constant's own value. + // + // Stryker treats the static-initializer mutants as "static" (no test + // covers the module-load line under perTest analysis) and runs them + // against all tests with the mutant active. The threshold is captured + // at spec import time — before the mutant env switch is observed — so + // a stale-cached read never sees the mutated initializer. Re-import + // the module under test so the initializer re-executes while the + // mutant is active, making the mutated value observable here. + vi.resetModules() + const { TaskHistoryStore: FreshTaskHistoryStore } = await import("../TaskHistoryStore") + const freshThreshold = ( + FreshTaskHistoryStore as unknown as { + LIVE_CHILD_MTIME_THRESHOLD_MS: number + } + ).LIVE_CHILD_MTIME_THRESHOLD_MS + expect(freshThreshold).toBe(5 * 60 * 1000) expect(LIVE_CHILD_MTIME_THRESHOLD_MS).toBe(5 * 60 * 1000) }) @@ -385,9 +402,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // Kills the line-484/485 StringLiteral mutants: the two concatenated // fragments of the skip message are asserted independently, so either // fragment mutated to '' breaks its matching stringContaining check. - expect(logSpy).toHaveBeenCalledWith( - expect.stringContaining("Skipping repair for live child child-live"), - ) + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("Skipping repair for live child child-live")) expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("owned by another window")) logSpy.mockRestore() From 50306592dbb7849457c2997565b0b094020bf823 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Thu, 3 Sep 2026 18:21:59 +0900 Subject: [PATCH 04/13] chore(gitignore): ignore local worktrees (.wt-*) and scratch files --- .gitignore | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.gitignore b/.gitignore index 3961778d5e..584f9177fa 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,14 @@ qdrant_storage/ plans/ roo-cli-*.tar.gz* + +# Local worktrees (multi-branch development) +.wt-*/ + +# Temporary scratch files +.tmp-* +.zoo-status.txt +untracked-*.txt +class_*.txt +check-dup2-result.txt +*.tsbuildinfo From 1d181726139fdbef4406d48d45e3b146a52fd00a Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Thu, 3 Sep 2026 19:00:52 +0900 Subject: [PATCH 05/13] test(task-persistence): make liveness-boundary tests filesystem-precision-independent --- .../TaskHistoryStore.reconciliation.spec.ts | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 052468831c..60ea4b526b 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -183,22 +183,49 @@ describe("TaskHistoryStore reconcileDelegationState", () => { } /** - * Deterministic wall clock for liveness-boundary tests. `fs.utimes` accepts - * ms-precision Date values and the store's `Date.now()` is spied to return - * this same instant, so `Date.now() - mtimeMs` is exact regardless of how - * long the test body takes to run. + * Deterministic wall clock for liveness-boundary tests. The store's + * `Date.now()` is spied to return this same instant, and `setChildMtimeAge` + * additionally injects the exact mtime the store observes, so + * `Date.now() - mtimeMs` is exact regardless of how long the test body + * takes to run or how much millisecond precision the filesystem keeps. */ const FIXED_NOW = 1_756_886_400_000 // 2025-09-03T08:00:00.000Z + // Restored in afterEach so a leaked spy can never poison the direct + // `getChildFileMtimeMs` probe test. + let mtimeSpy: { mockRestore(): void } | undefined + + /** + * Stamps a child's history file so the store observes a mtime of exactly + * `FIXED_NOW - ageMs`, independent of filesystem mtime precision. + * + * Two layers: + * 1. Best-effort `fs.utimes` keeps the on-disk file realistic, but tests + * must NOT depend on it: some filesystems and CI runners truncate mtime + * to seconds, which would silently flip live/stale expectations. + * 2. A spy on the private `TaskHistoryStore.prototype.getChildFileMtimeMs` + * (the exact call path used by the cross-instance liveness guard) + * injects the intended millisecond value. That single `mtimeMs` feeds + * BOTH the `Date.now() - mtimeMs < threshold` guard and the + * `Math.round((Date.now() - mtimeMs) / 1000)` skip-log render, so the + * `<`-vs-`<=` boundary at 300_000 ms and the 300s/299s/-100s render + * assertions stay deterministic and keep killing their mutants on any + * filesystem. + * + * `ageMs` may be negative (future mtime). Other child ids delegate to the + * real implementation so unrelated probe paths keep exercising the FS. + */ async function setChildMtimeAge(taskId: string, ageMs: number): Promise { const filePath = path.join(tmpDir, "tasks", taskId, GlobalFileNames.historyItem) const stamp = new Date(FIXED_NOW - ageMs) await fs.utimes(filePath, stamp, stamp) - // Guard the assumption that the filesystem round-trips millisecond - // precision, so a boundary test failure is diagnosable rather than a - // silent live/stale flip. - const written = await fs.stat(filePath) - expect(Math.round(written.mtimeMs)).toBe(FIXED_NOW - ageMs) + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + const original = probe.getChildFileMtimeMs + mtimeSpy = vi.spyOn(probe, "getChildFileMtimeMs").mockImplementation((childId: string) => + childId === taskId ? Promise.resolve(FIXED_NOW - ageMs) : original.call(store, childId), + ) } beforeEach(async () => { @@ -226,6 +253,8 @@ describe("TaskHistoryStore reconcileDelegationState", () => { }) afterEach(async () => { + mtimeSpy?.mockRestore() + mtimeSpy = undefined safeWriteJsonMock.mockImplementation(writeJson) for (const disposable of disposables) disposable.dispose() disposables.clear() @@ -543,10 +572,13 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // Files stamped 1970-01-01 (epoch-zero artifacts from misconfigured clocks, // zip extraction, or container images) must be treated as stale orphans. // Kills the ArithmeticOperator mutant `Date.now() - mtimeMs` -> - // `Date.now() % mtimeMs`: with mtimeMs = 1000, the real subtraction is - // ~56 years (stale -> repair), while FIXED_NOW % 1000 === 0 would be read - // as live and skip the repair. The same mutant inside the skip-path log is - // never reached under the mutant because the guard already diverges. + // `Date.now() % mtimeMs`: with the mocked mtimeMs = 1000, the real + // subtraction is ~56 years (stale -> repair), while FIXED_NOW % 1000 === 0 + // would be read as live and skip the repair. The same mutant inside the + // skip-path log is never reached under the mutant because the guard + // already diverges. The exact 1000 ms stamp comes from the mocked + // `getChildFileMtimeMs`, so no filesystem millisecond precision is + // assumed. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) try { @@ -563,11 +595,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: "child-epoch-mtime", }) await seedItems([parent, child]) - const childFilePath = path.join(tmpDir, "tasks", "child-epoch-mtime", GlobalFileNames.historyItem) - const epochStamp = new Date(1_000) // 1970-01-01T00:00:01.000Z - await fs.utimes(childFilePath, epochStamp, epochStamp) - const written = await fs.stat(childFilePath) - expect(Math.round(written.mtimeMs)).toBe(1_000) + await setChildMtimeAge("child-epoch-mtime", FIXED_NOW - 1_000) // store observes mtime 1970-01-01T00:00:01.000Z await store.initialize() @@ -587,7 +615,9 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // ArithmeticOperator mutant `Date.now() % mtimeMs` would instead render // the whole epoch magnitude (1756886400s), so the seconds-count // assertion below kills it. The live-side status assertions also kill - // the same mutant on the guard subtraction in TaskHistoryStore.ts. + // the same mutant on the guard subtraction in TaskHistoryStore.ts. The + // exact future mtime is injected by the mocked `getChildFileMtimeMs`, + // independent of filesystem millisecond precision. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) try { @@ -604,11 +634,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { delegatedToId: "child-future-mtime", }) await seedItems([parent, child]) - const childFilePath = path.join(tmpDir, "tasks", "child-future-mtime", GlobalFileNames.historyItem) - const futureStamp = new Date(FIXED_NOW + 100_000) - await fs.utimes(childFilePath, futureStamp, futureStamp) - const written = await fs.stat(childFilePath) - expect(Math.round(written.mtimeMs)).toBe(FIXED_NOW + 100_000) + await setChildMtimeAge("child-future-mtime", -100_000) // store observes a mtime 100s in the future await store.initialize() From 8587452295a934e42293b6333f27907a52580bac Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 08:07:02 +0900 Subject: [PATCH 06/13] fix(delegation): guard replayDelegationRepairIntent against live cross-window children --- src/core/task-persistence/TaskHistoryStore.ts | 23 ++- .../TaskHistoryStore.reconciliation.spec.ts | 176 +++++++++++++++++- 2 files changed, 195 insertions(+), 4 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index e3900111b0..7132f10d86 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -534,7 +534,10 @@ export class TaskHistoryStore { * Replay the durable active-child repair intent, if one was left by a crash. * The expected fields are guards: an intent may update only the missing side * when the other side is already at its target, or when both records still - * describe the original delegated handoff. + * describe the original delegated handoff. Before writing the child, the same + * cross-window liveness guard as `reconcileDelegationStateCore` applies: a + * child whose history file was touched recently belongs to another live + * window, so the stale intent is quarantined instead of replayed. * * This method acquires the store's non-reentrant promise-chain lock. It must be * called outside an existing `withLock` callback; locked callers must use the @@ -570,6 +573,24 @@ export class TaskHistoryStore { return } + // Cross-instance liveness guard (same convention as reconcileDelegationStateCore): + // if this window crashed mid-repair and another window restarted the same child, + // the child's history file is being actively persisted there. Replaying the stale + // intent would overwrite the live child as "interrupted", so quarantine it instead. + // Only enforced when the replay would actually write the child record: a child + // already at its target needs no write, and parent-only completion must not be + // blocked by child liveness. An unreadable mtime conservatively proceeds. + if (!childAtTarget) { + const mtimeMs = await this.getChildFileMtimeMs(child.id) + const isLiveElsewhere = + // Stryker disable next-line ConditionalExpression: replacing `mtimeMs !== undefined` with `true` is mutation-equivalent; with a defined mtimeMs `true && X === X`, and with undefined the right operand is `NaN < threshold === false`, identical to the short-circuit result. + mtimeMs !== undefined && Date.now() - mtimeMs < TaskHistoryStore.LIVE_CHILD_MTIME_THRESHOLD_MS + if (isLiveElsewhere) { + await this.quarantineDelegationRepairIntent(intent, "child live in another window (recent mtime)") + return + } + } + const repairedChild = childAtTarget ? child : { ...child, status: intent.target.childStatus } const repairedParent = parentMatchesTargetState ? parent diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 60ea4b526b..bea2392b75 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -223,9 +223,11 @@ describe("TaskHistoryStore reconcileDelegationState", () => { getChildFileMtimeMs: (childId: string) => Promise } const original = probe.getChildFileMtimeMs - mtimeSpy = vi.spyOn(probe, "getChildFileMtimeMs").mockImplementation((childId: string) => - childId === taskId ? Promise.resolve(FIXED_NOW - ageMs) : original.call(store, childId), - ) + mtimeSpy = vi + .spyOn(probe, "getChildFileMtimeMs") + .mockImplementation((childId: string) => + childId === taskId ? Promise.resolve(FIXED_NOW - ageMs) : original.call(store, childId), + ) } beforeEach(async () => { @@ -890,6 +892,9 @@ describe("TaskHistoryStore reconcileDelegationState", () => { await seedItems([parent, child]) const intentPath = path.join(tmpDir, "tasks", GlobalFileNames.delegationRepairIntent) await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + // Keep this a crash-orphan replay: the child file must not look live in + // another window, or the cross-window liveness guard quarantines the intent. + await markStaleMtime(child.id) await store.reconcile({ forceRefresh: true }) const storeInternals = store as unknown as { @@ -1016,6 +1021,168 @@ describe("TaskHistoryStore reconcileDelegationState", () => { ).toBe(true) }) + it("quarantines a replay intent whose child is live in another window (recent mtime)", async () => { + // Reviewer scenario: this window crashed mid-repair (the intent is durable + // but the child write never landed), and another window then restarted the + // same child. The child's history file mtime is recent, so replaying the + // intent here would overwrite a live child as "interrupted". The intent must + // be quarantined instead, and the startup reconciliation liveness guard must + // likewise leave the delegation untouched. + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + const child = makeItem({ + id: "child-replay-live", + status: "active", + parentTaskId: "parent-replay-live", + rootTaskId: "parent-replay-live", + }) + const parent = makeItem({ + id: "parent-replay-live", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + const tasksDir = path.join(tmpDir, "tasks") + const intentPath = path.join(tasksDir, GlobalFileNames.delegationRepairIntent) + await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + + // Another live window has just persisted the child. + const childFilePath = path.join(tasksDir, child.id, GlobalFileNames.historyItem) + const now = new Date() + await fs.utimes(childFilePath, now, now) + + await store.initialize() + + // Nothing may be written as "interrupted": child stays active and the + // parent keeps its delegation links. + expect(store.get(child.id)?.status).toBe("active") + expect(store.get(parent.id)?.status).toBe("delegated") + expect(store.get(parent.id)?.awaitingChildId).toBe(child.id) + expect(store.get(parent.id)?.delegatedToId).toBe(child.id) + + const persistedChild = JSON.parse(await fs.readFile(childFilePath, "utf8")) as HistoryItem + expect(persistedChild.status).toBe("active") + const persistedParent = JSON.parse( + await fs.readFile(path.join(tasksDir, parent.id, GlobalFileNames.historyItem), "utf8"), + ) as HistoryItem + expect(persistedParent.status).toBe("delegated") + expect(persistedParent.awaitingChildId).toBe(child.id) + + // The intent is moved out of the way rather than applied or left to retry. + await expect(fs.access(intentPath)).rejects.toThrow() + expect( + (await fs.readdir(tasksDir)).some((name) => + name.startsWith(`${GlobalFileNames.delegationRepairIntent}.quarantine-`), + ), + ).toBe(true) + // Kills the StringLiteral mutant on the new quarantine reason. + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("child live in another window (recent mtime)")) + + warnSpy.mockRestore() + }) + + it("replays a repair intent whose child mtime is stale (crash orphan still repaired)", async () => { + // Regression guard for the replay liveness guard: a child file untouched for + // longer than the threshold is a genuine crash orphan, so the durable intent + // must still complete on restart — child → interrupted, parent → active. + const child = makeItem({ + id: "child-replay-stale", + status: "active", + parentTaskId: "parent-replay-stale", + rootTaskId: "parent-replay-stale", + }) + const parent = makeItem({ + id: "parent-replay-stale", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + const tasksDir = path.join(tmpDir, "tasks") + const intentPath = path.join(tasksDir, GlobalFileNames.delegationRepairIntent) + await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + await markStaleMtime(child.id) + + await store.initialize() + + expect(store.get(child.id)?.status).toBe("interrupted") + expect(store.get(parent.id)?.status).toBe("active") + expect(store.get(parent.id)?.awaitingChildId).toBeUndefined() + expect(store.get(parent.id)?.delegatedToId).toBeUndefined() + const persistedChild = JSON.parse( + await fs.readFile(path.join(tasksDir, child.id, GlobalFileNames.historyItem), "utf8"), + ) as HistoryItem + expect(persistedChild.status).toBe("interrupted") + await expect(fs.access(intentPath)).rejects.toThrow() + expect( + (await fs.readdir(tasksDir)).some((name) => + name.startsWith(`${GlobalFileNames.delegationRepairIntent}.quarantine-`), + ), + ).toBe(false) + }) + + it("completes a parent-only replay while the child is live in another window (child already at target)", async () => { + // The guard must gate only actual child writes. Here the child is already at + // intent.target.childStatus, so no child write happens and the recent (live) + // mtime must not block the parent-side completion of the repair. + const child = makeItem({ + id: "child-replay-parent-only", + status: "interrupted", + parentTaskId: "parent-replay-parent-only", + }) + const parent = makeItem({ + id: "parent-replay-parent-only", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + const tasksDir = path.join(tmpDir, "tasks") + const intentPath = path.join(tasksDir, GlobalFileNames.delegationRepairIntent) + await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + const now = new Date() + await fs.utimes(path.join(tasksDir, child.id, GlobalFileNames.historyItem), now, now) + + await store.initialize() + + expect(store.get(child.id)?.status).toBe("interrupted") + expect(store.get(parent.id)?.status).toBe("active") + expect(store.get(parent.id)?.awaitingChildId).toBeUndefined() + await expect(fs.access(intentPath)).rejects.toThrow() + }) + + it("proceeds with a replay when the child history file mtime is unreadable", async () => { + // Matches the reconcile-path convention: getChildFileMtimeMs returns + // undefined for a missing/unreadable history file, which conservatively + // proceeds with the repair instead of treating the child as live. + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + mtimeSpy = vi.spyOn(probe, "getChildFileMtimeMs").mockResolvedValue(undefined) + + const child = makeItem({ + id: "child-replay-unreadable", + status: "active", + parentTaskId: "parent-replay-unreadable", + }) + const parent = makeItem({ + id: "parent-replay-unreadable", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + const tasksDir = path.join(tmpDir, "tasks") + const intentPath = path.join(tasksDir, GlobalFileNames.delegationRepairIntent) + await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + + await store.initialize() + + expect(store.get(child.id)?.status).toBe("interrupted") + expect(store.get(parent.id)?.status).toBe("active") + await expect(fs.access(intentPath)).rejects.toThrow() + }) + it("repairs invalid delegation: delegated parent with no awaitingChildId → active (clears delegatedToId and awaitingChildId)", async () => { // awaitingChildId is falsy but explicitly set (empty string), delegatedToId is stale const parent = makeItem({ @@ -1105,6 +1272,9 @@ describe("TaskHistoryStore reconcileDelegationState", () => { await seedItems([grandparent, parent, child]) const intentPath = path.join(tmpDir, "tasks", GlobalFileNames.delegationRepairIntent) await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + // Crash-orphan scenario: the child must not look live in another window, or + // the replay/startup liveness guards would skip the repair entirely. + await markStaleMtime(child.id) await store.initialize() From 30b1262e5ecd0efa9c0daa8812bebd32e2268531 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 09:23:46 +0900 Subject: [PATCH 07/13] fix(delegation): run delegation reconciliation on periodic reconcile ticks --- src/core/task-persistence/TaskHistoryStore.ts | 98 +++++- .../TaskHistoryStore.reconciliation.spec.ts | 287 ++++++++++++++++++ 2 files changed, 383 insertions(+), 2 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index 7132f10d86..c71a7d0ea1 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -85,6 +85,21 @@ export class TaskHistoryStore { private writeLock: Promise = Promise.resolve() private fsWatcher: fsSync.FSWatcher | null = null private reconcileTimer: ReturnType | null = null + /** + * Serializes the periodic delegation-reconciliation step across ticks. The + * store lock already prevents interleaved mutations, but overlapping ticks + * would queue stale passes behind each other; skipping a tick instead lets + * the next interval retry with fresher data. + */ + private delegationTickRunning = false + /** + * Task ids this store instance itself persisted with an `active` status. + * Their task sessions live in this window, so periodic delegation + * reconciliation must exclude them from orphan-repair candidates. The set + * is per-instance by design: after a host restart the new store has no + * entries, so startup reconciliation keeps repairing genuine crash orphans. + */ + private readonly locallyActiveTaskIds = new Set() private disposed = false /** @@ -257,6 +272,12 @@ export class TaskHistoryStore { // Update in-memory cache with what was actually persisted this.cache.set(written.id, written) + // Only runtime writes (not `skipTransitionCheck` administrative repairs) + // prove a live task session runs in THIS window; repairs go through the + // same core but must not suppress future orphan reconciliation. + if (!options.skipTransitionCheck) { + this.trackLocalSessionOwnership(written) + } const all = this.getAll() @@ -275,6 +296,7 @@ export class TaskHistoryStore { return this.withLock(async () => { this.cache.delete(taskId) this.taskFileMtimes.delete(taskId) + this.locallyActiveTaskIds.delete(taskId) // Remove per-task file (best-effort) try { @@ -299,6 +321,7 @@ export class TaskHistoryStore { for (const taskId of taskIds) { this.cache.delete(taskId) this.taskFileMtimes.delete(taskId) + this.locallyActiveTaskIds.delete(taskId) try { const filePath = await this.getTaskFilePath(taskId) @@ -393,8 +416,9 @@ export class TaskHistoryStore { /** * Repair delegation inconsistencies left by a crash mid-transition. * - * Called once from `initialize()` after `reconcile()`. Runs inside `withLock` to - * prevent interleaving with watcher-triggered reconcile() calls. Iterates until + * Called from `initialize()` and from each periodic reconciliation tick, + * always after `reconcile()`. Runs inside `withLock` to prevent interleaving + * with watcher-triggered reconcile() calls. Iterates until * convergence so that one-level chained delegations visible at startup are resolved. * * Must NOT be called from within `withLock` — `withLock` is non-reentrant (promise @@ -530,6 +554,22 @@ export class TaskHistoryStore { ) } + /** + * Maintain the set of task ids whose live session runs in THIS window. + * A record this store persisted as active belongs to a task running here, + * so the periodic delegation pass must never treat it as a crash orphan — + * its history-file mtime can legitimately go quiet for minutes while the + * task streams a long model turn or waits on a user prompt. Any non-active + * status write ends that ownership. + */ + private trackLocalSessionOwnership(written: HistoryItem): void { + if ((written.status ?? "active") === "active") { + this.locallyActiveTaskIds.add(written.id) + } else { + this.locallyActiveTaskIds.delete(written.id) + } + } + /** * Replay the durable active-child repair intent, if one was left by a crash. * The expected fields are guards: an intent may update only the missing side @@ -978,6 +1018,13 @@ export class TaskHistoryStore { /** * Start periodic reconciliation as a defensive fallback for platforms * where fs.watch is unreliable. + * + * Each tick refreshes disk→cache via `reconcile()` and then re-runs the same + * delegation repair `initialize()` performs, so a child that skipped repair + * at startup (recent mtime = live in another window) but crashes afterwards + * is caught within one interval instead of waiting for the next extension + * host restart. Intent replay is intentionally NOT part of the tick: the + * durable repair journal is replayed at startup by design. */ private startPeriodicReconciliation(): void { if (this.disposed) { @@ -993,10 +1040,54 @@ export class TaskHistoryStore { } catch (err) { console.error("[TaskHistoryStore] Periodic reconciliation failed:", err) } + try { + await this.runPeriodicDelegationReconciliation() + } catch (err) { + console.error("[TaskHistoryStore] Periodic delegation reconciliation failed:", err) + } this.startPeriodicReconciliation() }, TaskHistoryStore.RECONCILE_INTERVAL_MS) } + /** + * One delegation-reconciliation pass for a periodic tick. + * + * Mirrors the `initialize()` sequence: capture which active task ids exist + * in persisted state (the cache was just refreshed from disk by + * `reconcile()` and no repair has mutated statuses yet), then run the + * reconciliation against that snapshot. The child-mtime liveness guard + * inside `reconcileDelegationStateCore` protects children actively written + * by another window, so ticking is safe for multi-window workspaces. + * + * One mid-session-only refinement over the startup snapshot: ids this + * window itself persisted as active are excluded. At startup no local + * sessions exist, so an active child on disk implies a previous host + * crashed; mid-session, an active child that THIS store wrote belongs to a + * live task here, and a quiet-but-live mtime (long model turn, user + * deliberating over an ask) must not cause it to be repaired away from + * under its own runner. Genuine crashes of this window take the tick with + * them and are handled by the next startup pass instead. + * + * `reconcileDelegationState` acquires the non-reentrant `withLock` chain + * itself (same entry point `initialize()` uses); this method never holds + * the lock. The running flag only guards snapshot→pass adjacency and skips + * (rather than queues) a tick whose previous pass is still in flight. + */ + private async runPeriodicDelegationReconciliation(): Promise { + if (this.disposed || this.delegationTickRunning) { + return + } + this.delegationTickRunning = true + try { + const persistedActiveIds = new Set( + Array.from(this.getPersistedActiveIds()).filter((id) => !this.locallyActiveTaskIds.has(id)), + ) + await this.reconcileDelegationState(persistedActiveIds) + } finally { + this.delegationTickRunning = false + } + } + // ────────────────────────────── Atomic read-modify-write ────────────────────────────── /** @@ -1087,12 +1178,15 @@ export class TaskHistoryStore { // First record is committed on disk. Update cache so it // reflects disk state before propagating the error. this.cache.set(firstId, writtenFirst) + this.trackLocalSessionOwnership(writtenFirst) throw error } // Both disk writes succeeded — now update the cache. this.cache.set(firstId, writtenFirst) this.cache.set(secondId, writtenSecond) + this.trackLocalSessionOwnership(writtenFirst) + this.trackLocalSessionOwnership(writtenSecond) const all = this.getAll() if (this.onWrite) { diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index bea2392b75..bfc9292c39 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -1601,3 +1601,290 @@ describe("TaskHistoryStore upsert transition guard", () => { ).rejects.toThrow("Invalid task status transition: delegated → completed") }) }) + +// ───────────────────────────────────────────────────────────────────────────── +// startPeriodicReconciliation — delegation repair on each tick (review item #2) +// ───────────────────────────────────────────────────────────────────────────── + +describe("TaskHistoryStore periodic delegation reconciliation", () => { + let tmpDir: string + let store: TaskHistoryStore | undefined + let mtimeSpy: { mockRestore(): void } | undefined + + // Private static interval used to advance the fake clock by exactly one tick. + // There is no typed accessor; this casts through `unknown` (not `as any`) + // following the same private-member access pattern used for + // LIVE_CHILD_MTIME_THRESHOLD_MS at the top of this spec. + const RECONCILE_INTERVAL_MS = (TaskHistoryStore as unknown as { RECONCILE_INTERVAL_MS: number }) + .RECONCILE_INTERVAL_MS + + const CHILD_ID = "child-tick" + const PARENT_ID = "parent-tick" + + /** + * Fake only what the tick scheduling needs: the 5-minute `setTimeout` clock + * and `Date` (consumed by the liveness guard). Everything else (fs I/O, + * microtasks) stays real so `flushAsyncWork()` below can pump the event + * loop while the timer clock advances only 1 ms per yield. + */ + function useTickClock(): void { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "Date"] }) + } + + /** + * Drain pending real fs I/O. The tick's reconcile/repair chain completes on + * libuv callbacks that fake timers alone never advance, and each + * `advanceTimersByTimeAsync(1)` yields one REAL macrotask turn (processing + * the poll phase) while advancing the fake clock only 1 ms. The total fake + * time here stays far below RECONCILE_INTERVAL_MS, so no extra tick fires + * during the pump — this only lets in-flight fs callbacks settle. The count + * is generous to absorb Windows antivirus/OneDrive fs latency. + */ + async function flushAsyncWork(yields = 2000): Promise { + for (let i = 0; i < yields; i++) { + await vi.advanceTimersByTimeAsync(1) + } + } + + async function seedItems(items: HistoryItem[]): Promise { + const tasksDir = path.join(tmpDir, "tasks") + await fs.mkdir(tasksDir, { recursive: true }) + for (const item of items) { + const taskDir = path.join(tasksDir, item.id) + await fs.mkdir(taskDir, { recursive: true }) + await fs.writeFile(path.join(taskDir, "history_item.json"), JSON.stringify(item)) + } + } + + /** + * Stateful mtime injection for the liveness guard. The guard computes + * `Date.now() - mtimeMs` against the (fake) clock, and this injector returns + * `Date.now() - childAgeMs` at call time, so flipping `childAgeMs` between + * the startup pass and a periodic tick deterministically models "live in + * another window at startup, then crashed before the next tick". Exact + * regardless of filesystem mtime precision, same convention as + * `setChildMtimeAge` above. Other child ids delegate to the real + * implementation so unrelated probe paths keep exercising the FS. + */ + let childAgeMs = 0 + function installChildAgeInjector(): void { + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + const original = probe.getChildFileMtimeMs + mtimeSpy = vi + .spyOn(probe, "getChildFileMtimeMs") + .mockImplementation((childId: string) => + childId === CHILD_ID ? Promise.resolve(Date.now() - childAgeMs) : original.call(store!, childId), + ) + } + + function makeDelegatedPair(): HistoryItem[] { + const child = makeItem({ + id: CHILD_ID, + status: "active", + parentTaskId: PARENT_ID, + rootTaskId: PARENT_ID, + }) + const parent = makeItem({ + id: PARENT_ID, + status: "delegated", + awaitingChildId: CHILD_ID, + delegatedToId: CHILD_ID, + childIds: [CHILD_ID], + }) + return [parent, child] + } + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "periodic-deleg-test-")) + childAgeMs = 60_000 + }) + + afterEach(async () => { + mtimeSpy?.mockRestore() + mtimeSpy = undefined + store?.dispose() + store = undefined + vi.useRealTimers() + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + }) + + it("repairs an active child whose mtime goes stale between startup and the next tick (the reported bug)", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const [parent, child] = makeDelegatedPair() + await seedItems([parent, child]) + + const s = (store = new TaskHistoryStore(tmpDir)) + installChildAgeInjector() + useTickClock() + // Child looks live at startup (written 60s ago by another window) → startup skips repair. + childAgeMs = 60_000 + await s.initialize() + expect(errorSpy).not.toHaveBeenCalled() + + expect(s.get(CHILD_ID)?.status).toBe("active") + expect(s.get(PARENT_ID)?.status).toBe("delegated") + expect(s.get(PARENT_ID)?.awaitingChildId).toBe(CHILD_ID) + + // The owning window crashes: nobody rewrites the child file, so by the + // next periodic tick its mtime is past the liveness threshold. + childAgeMs = 10 * 60 * 1000 + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + + // Within ONE interval, the parent window must repair: child → interrupted, + // parent → active with delegation links cleared. + expect(s.get(CHILD_ID)).toMatchObject({ id: CHILD_ID, status: "interrupted", parentTaskId: PARENT_ID }) + expect(s.get(PARENT_ID)).toMatchObject({ id: PARENT_ID, status: "active" }) + expect(s.get(PARENT_ID)?.awaitingChildId).toBeUndefined() + expect(s.get(PARENT_ID)?.delegatedToId).toBeUndefined() + + // Repaired on disk, not just in the cache. + const persistedChild = JSON.parse( + await fs.readFile(path.join(tmpDir, "tasks", CHILD_ID, GlobalFileNames.historyItem), "utf8"), + ) as HistoryItem + const persistedParent = JSON.parse( + await fs.readFile(path.join(tmpDir, "tasks", PARENT_ID, GlobalFileNames.historyItem), "utf8"), + ) as HistoryItem + expect(persistedChild.status).toBe("interrupted") + expect(persistedParent.status).toBe("active") + expect(persistedParent.awaitingChildId).toBeUndefined() + + // The warn message proves the DELEGATION pass (not the plain cache + // reconcile) ran inside the tick, and the tick raised no errors. + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Reconciled orphaned active child")) + expect(errorSpy).not.toHaveBeenCalled() + + // Re-arm must survive the successful tick so the loop keeps running. + const internals = s as unknown as { reconcileTimer: ReturnType | null } + expect(internals.reconcileTimer).not.toBeNull() + + warnSpy.mockRestore() + errorSpy.mockRestore() + }) + + it("never repairs a child this window itself persisted as active, even with a stale mtime (in-window delegation)", async () => { + // Startup has no local sessions, so an active child on disk implies a + // crashed host and is a valid repair target. Mid-session that inference + // breaks: a child running IN THIS WINDOW (e.g. an in-window delegation) + // can go minutes without rewriting its history file while it streams a + // long turn or waits on a user prompt. The tick must not tear it away + // from its own runner — only children this store never wrote active + // (i.e. loaded from disk, owned elsewhere) are orphan candidates. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const child = makeItem({ + id: CHILD_ID, + status: "active", + parentTaskId: PARENT_ID, + rootTaskId: PARENT_ID, + }) + const parent = makeItem({ id: PARENT_ID, status: "active" }) + + const s = (store = new TaskHistoryStore(tmpDir)) + useTickClock() + await s.initialize() + + // This window creates the parent and the child, then delegates. + await s.upsert(parent) + await s.upsert(child) + await s.atomicReadAndUpdate(PARENT_ID, (current) => ({ + ...current, + status: "delegated" as const, + awaitingChildId: CHILD_ID, + delegatedToId: CHILD_ID, + })) + expect(s.get(PARENT_ID)?.status).toBe("delegated") + + // Even though the child's mtime looks stale, it is owned HERE. + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + const realProbe = probe.getChildFileMtimeMs + mtimeSpy = vi + .spyOn(probe, "getChildFileMtimeMs") + .mockImplementation((childId: string) => + childId === CHILD_ID ? Promise.resolve(Date.now() - 10 * 60 * 1000) : realProbe.call(s, childId), + ) + + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + + expect(s.get(CHILD_ID)?.status).toBe("active") + expect(s.get(PARENT_ID)?.status).toBe("delegated") + expect(s.get(PARENT_ID)?.awaitingChildId).toBe(CHILD_ID) + expect(errorSpy).not.toHaveBeenCalled() + + errorSpy.mockRestore() + }) + + it("does not repair a child that stays live across the periodic tick (no cross-window clobbering)", async () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const [parent, child] = makeDelegatedPair() + await seedItems([parent, child]) + + const s = (store = new TaskHistoryStore(tmpDir)) + installChildAgeInjector() + useTickClock() + childAgeMs = 60_000 + await s.initialize() + // Startup also logs the skip; clear so remaining calls come from the tick. + logSpy.mockClear() + + // The other window keeps writing: the child stays live at tick time. + childAgeMs = 60_000 + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + + // Nothing may be repaired: child stays active, parent keeps its delegation links. + expect(s.get(CHILD_ID)?.status).toBe("active") + expect(s.get(PARENT_ID)?.status).toBe("delegated") + expect(s.get(PARENT_ID)?.awaitingChildId).toBe(CHILD_ID) + expect(s.get(PARENT_ID)?.delegatedToId).toBe(CHILD_ID) + + const persistedChild = JSON.parse( + await fs.readFile(path.join(tmpDir, "tasks", CHILD_ID, GlobalFileNames.historyItem), "utf8"), + ) as HistoryItem + expect(persistedChild.status).toBe("active") + + // The skip log proves the tick ran delegation reconciliation and the + // liveness guard protected the other window's child. + expect(logSpy).toHaveBeenCalledWith(expect.stringContaining(`Skipping repair for live child ${CHILD_ID}`)) + + logSpy.mockRestore() + }) + + it("logs and keeps re-arming when the periodic delegation step throws", async () => { + const [parent, child] = makeDelegatedPair() + await seedItems([parent, child]) + + const internals = TaskHistoryStore.prototype as unknown as { + runPeriodicDelegationReconciliation: () => Promise + } + const throwingSpy = vi + .spyOn(internals, "runPeriodicDelegationReconciliation") + .mockRejectedValue(new Error("tick delegation boom")) + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + + const s = (store = new TaskHistoryStore(tmpDir)) + useTickClock() + await s.initialize() + + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("Periodic delegation reconciliation failed"), + expect.objectContaining({ message: "tick delegation boom" }), + ) + + // One more interval still fires the delegation step: the recursive + // re-arm is preserved even though the step threw. + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + expect(throwingSpy).toHaveBeenCalledTimes(2) + + errorSpy.mockRestore() + throwingSpy.mockRestore() + }) +}) From 160e7f6b8bac010bf2185d3171185ac4d5ab6cbc Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 09:31:40 +0900 Subject: [PATCH 08/13] fix(delegation): use console.warn for live-child skip log --- src/core/task-persistence/TaskHistoryStore.ts | 2 +- .../TaskHistoryStore.reconciliation.spec.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index c71a7d0ea1..0cc6091d91 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -505,7 +505,7 @@ export class TaskHistoryStore { mtimeMs !== undefined && Date.now() - mtimeMs < TaskHistoryStore.LIVE_CHILD_MTIME_THRESHOLD_MS if (isLiveElsewhere) { - console.log( + console.warn( `[TaskHistoryStore] Skipping repair for live child ${child.id} ` + `(mtime ${Math.round((Date.now() - mtimeMs) / 1000)}s ago) — owned by another window`, ) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index bfc9292c39..4fe18edb44 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -390,7 +390,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { }) it("skips repair for active child with recent mtime (live in another window)", async () => { - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) const child = makeItem({ id: "child-live", status: "active", @@ -532,7 +532,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // 1000) renders "300", while `* 1000`, `+ 1000`, `- 1000` and // `% 1000` all render a different second count. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) try { const child = makeItem({ id: "child-boundary-live", @@ -582,7 +582,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // `getChildFileMtimeMs`, so no filesystem millisecond precision is // assumed. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) try { const child = makeItem({ id: "child-epoch-mtime", @@ -621,7 +621,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // exact future mtime is injected by the mocked `getChildFileMtimeMs`, // independent of filesystem millisecond precision. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) try { const child = makeItem({ id: "child-future-mtime", @@ -657,7 +657,7 @@ describe("TaskHistoryStore reconcileDelegationState", () => { // this one covers the ceil mutant. It also re-asserts the live side of // the strict `<` boundary and the line-105 threshold mutants. const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) try { const child = makeItem({ id: "child-boundary-ceil", @@ -1820,7 +1820,7 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { }) it("does not repair a child that stays live across the periodic tick (no cross-window clobbering)", async () => { - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}) + const logSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) const [parent, child] = makeDelegatedPair() await seedItems([parent, child]) From 1e6c9be75e57222f91c3e1ce8a78a0ec026491ad Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 09:40:50 +0900 Subject: [PATCH 09/13] test(delegation): route undefined child mtime through initialize() end-to-end --- .../TaskHistoryStore.reconciliation.spec.ts | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 4fe18edb44..2827fa3b0a 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -486,6 +486,98 @@ describe("TaskHistoryStore reconcileDelegationState", () => { warnSpy.mockRestore() }) + it("routes an undefined getChildFileMtimeMs through initialize() and still repairs the active child", async () => { + // End-to-end companion to the direct-helper probe test above ("returns + // the file mtime for an existing child and undefined for a missing + // one"): that test covers the helper in isolation; this one feeds the + // same `undefined` return through `reconcileDelegationStateCore` via + // `initialize()` and asserts the conservative-repair contract fires — + // an unreadable mtime must NOT be treated as "live in another window", + // the child is repaired to interrupted and the parent back to active. + // A mutant that flips the `mtimeMs !== undefined` short-circuit (e.g. + // treating missing mtimes as live) would skip the repair and break the + // status assertions and the negative skip-log assertion below. + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + // Private-instance access follows the documented double-assertion + // pattern used by the probe test and the repair-intent replay tests. + const internals = store as unknown as { + getChildFileMtimeMs: (childId: string) => Promise + } + const originalGetChildFileMtimeMs = internals.getChildFileMtimeMs + const mtimeUndefinedSpy = vi + .spyOn(internals, "getChildFileMtimeMs") + .mockImplementation((childId: string) => + childId === "child-undef-mtime" + ? Promise.resolve(undefined) + : originalGetChildFileMtimeMs.call(store, childId), + ) + try { + const child = makeItem({ + id: "child-undef-mtime", + status: "active", + parentTaskId: "parent-undef-mtime", + rootTaskId: "parent-undef-mtime", + childIds: ["grandchild-undef-mtime"], + }) + const parent = makeItem({ + id: "parent-undef-mtime", + status: "delegated", + awaitingChildId: "child-undef-mtime", + delegatedToId: "child-undef-mtime", + childIds: ["child-undef-mtime"], + }) + // The child file is seeded normally (fresh mtime, and present in + // persistedActiveIds); only the stat probe is forced to undefined, + // simulating a file that races away or is unreadable at the moment + // the liveness guard checks it. + await seedItems([parent, child]) + + await store.initialize() + + // Spy must have been exercised through the real reconciliation path. + expect(mtimeUndefinedSpy).toHaveBeenCalledWith("child-undef-mtime") + + const repairedChild = store.get("child-undef-mtime") + const repairedParent = store.get("parent-undef-mtime") + expect(repairedChild).toMatchObject({ + id: "child-undef-mtime", + status: "interrupted", + parentTaskId: "parent-undef-mtime", + rootTaskId: "parent-undef-mtime", + childIds: ["grandchild-undef-mtime"], + }) + expect(repairedParent).toMatchObject({ id: "parent-undef-mtime", status: "active" }) + expect(repairedParent?.awaitingChildId).toBeUndefined() + expect(repairedParent?.delegatedToId).toBeUndefined() + + // Persisted state must match the cache, same as the stale-mtime test. + const tasksDir = path.join(tmpDir, "tasks") + const persistedChild = JSON.parse( + await fs.readFile(path.join(tasksDir, "child-undef-mtime", "history_item.json"), "utf8"), + ) as HistoryItem + const persistedParent = JSON.parse( + await fs.readFile(path.join(tasksDir, "parent-undef-mtime", "history_item.json"), "utf8"), + ) as HistoryItem + expect(persistedChild).toMatchObject({ + id: "child-undef-mtime", + status: "interrupted", + parentTaskId: "parent-undef-mtime", + rootTaskId: "parent-undef-mtime", + }) + expect(persistedParent).toMatchObject({ id: "parent-undef-mtime", status: "active" }) + expect(persistedParent.awaitingChildId).toBeUndefined() + expect(persistedParent.delegatedToId).toBeUndefined() + + // Repair ran and the liveness-skip branch was NOT taken. + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Reconciled orphaned active child")) + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("child-undef-mtime")) + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("Skipping repair for live child")) + } finally { + mtimeUndefinedSpy.mockRestore() + warnSpy.mockRestore() + } + }) + it("repairs when child file age is exactly the liveness threshold (strict '<' boundary)", async () => { // Kills the TaskHistoryStore.ts line-481 EqualityOperator mutant `<=`: // under `<=`, age === threshold (300000 ms) would count as live and the From 1dfa3aea597a6cefabe1cd53e272b75f6c366c28 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 09:55:29 +0900 Subject: [PATCH 10/13] test(lifecycle): model cross-window child liveness guard in lifecycle:model-check --- docs/architecture/task-lifecycle-model.md | 25 +-- scripts/check-task-lifecycle.ts | 200 +++++++++++++++++++--- 2 files changed, 189 insertions(+), 36 deletions(-) diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index 3218eb4ba8..ebc61a5941 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -23,17 +23,19 @@ TLA+/PlusCal or Quint with TLC becomes a better fit when the lifecycle needs tem ## Production mapping -| Model concept | Production concept | -| ------------------------- | ------------------------------------------------------------------------------------ | -| Task record and status | `HistoryItem` persisted by `TaskHistoryStore` | -| `delegate(parent, child)` | `ClineProvider.delegateParentAndOpenChild` | -| `interrupt(child)` | cancellation or eviction through `markDelegatedChildInterrupted` | -| `complete(child)` | `ClineProvider.reopenParentFromDelegation` | -| `abandon(child)` | `ClineProvider.abandonSubtask` | -| Atomic event step | `atomicReadAndUpdate`, `atomicUpdatePair`, and per-parent delegation transition lock | -| Event interleaving | Competing completion, cancellation, abandonment, and new delegation calls | - -The model has three fixed task slots, enough to cover competing siblings and a nested parent-child-grandchild chain. It explores every reachable interleaving through depth 12, deduplicating canonical states. Representative checks also exercise rejected operations that do not create a new state: a second concurrent delegation while the first child is active, stale completion after re-delegation, late completion after abandonment, completion after interruption, and nested completion. Named semantic landmarks require the graph to retain interrupted-child re-delegation and nested delegation even when the raw state total changes. +| Model concept | Production concept | +| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------ | +| Task record and status | `HistoryItem` persisted by `TaskHistoryStore` | +| `delegate(parent, child)` | `ClineProvider.delegateParentAndOpenChild` | +| `interrupt(child)` | cancellation or eviction through `markDelegatedChildInterrupted` | +| `complete(child)` | `ClineProvider.reopenParentFromDelegation` | +| `abandon(child)` | `ClineProvider.abandonSubtask` | +| `reconcileStartup(parent)` | startup/periodic `TaskHistoryStore.reconcileDelegationStateCore` orphan repair | +| `markLiveElsewhere(child)` / `expireLiveElsewhere(child)` | child history-file mtime recent vs stale past `LIVE_CHILD_MTIME_THRESHOLD_MS` (abstracted; no wall clock in model) | +| Atomic event step | `atomicReadAndUpdate`, `atomicUpdatePair`, and per-parent delegation transition lock | +| Event interleaving | Competing completion, cancellation, abandonment, and new delegation calls | + +The model has three fixed task slots, enough to cover competing siblings and a nested parent-child-grandchild chain, plus one abstract boolean per slot recording whether an active child's session is owned by another window (recent history-file mtime). It explores every reachable interleaving through depth 12, deduplicating canonical states. Representative checks also exercise rejected operations that do not create a new state: a second concurrent delegation while the first child is active, stale completion after re-delegation, late completion after abandonment, completion after interruption, and nested completion. Named semantic landmarks require the graph to retain interrupted-child re-delegation and nested delegation even when the raw state total changes, a delegated parent whose active child is live in another window surviving startup reconciliation unchanged, and a stale-mtime (crash-orphan) active child being repaired to `interrupted` with the parent returned to `active` only through `reconcileStartup`. Production completion also accepts a recovery-compatible `active` parent that still awaits the returning child, then clears the stale pointers. Normal model transitions never create that intermediate state, so it is covered by a focused reducer test rather than admitted as a generally valid reachable state. @@ -78,6 +80,7 @@ The checker currently enforces: 5. Parent-child lineage is acyclic. 6. Completed task records cannot be changed by later lifecycle events. 7. Active-child re-delegation, stale completion after ownership moves to another child, duplicate/late completion, and abandonment of a live child are rejected by the shared production guards. +8. No transition may clear a delegated parent's link to a child that is active and marked live-elsewhere; startup reconciliation repairs only stale-or-unreadable-mtime (crash-orphan) children. This encodes the PR #1495 cross-window misrepair bug class, which broke delegation links so subtask completion could not return to the parent. These are safety claims within the documented bounds. The check does not claim liveness, fairness, crash consistency, filesystem-lock correctness, API history correctness, or exhaustive coverage of arbitrary task counts. It also does not distinguish a delayed pre-interruption completion from a legitimate post-resume completion for the same child ID; that requires a persisted attempt/generation token before it can become a sound invariant. diff --git a/scripts/check-task-lifecycle.ts b/scripts/check-task-lifecycle.ts index 73e9078366..d0108f6fd6 100644 --- a/scripts/check-task-lifecycle.ts +++ b/scripts/check-task-lifecycle.ts @@ -11,7 +11,24 @@ import { const taskIds = ["parent", "child-a", "child-b"] as const type TaskId = (typeof taskIds)[number] -type ModelState = Record +type TaskMap = Record + +/** + * Abstract cross-window liveness flag. Production decides whether an active + * child awaited by a delegated parent belongs to another live window by + * comparing the child's history-file mtime against a 5-minute threshold + * (`TaskHistoryStore.LIVE_CHILD_MTIME_THRESHOLD_MS`). The model never reads + * wall-clock time: `liveElsewhere[child]` is true exactly when the modeled + * mtime is "recent" (the child is owned by another window) and false when it + * is "stale" or unreadable (the child is a crash orphan, repaired + * conservatively). + */ +type LivenessMap = Record + +interface ModelState { + tasks: TaskMap + liveElsewhere: LivenessMap +} interface Transition { name: string @@ -25,17 +42,55 @@ interface TraceStep { const MAX_DEPTH = 12 const MAX_STATES = 10_000 -const expectedActions = ["delegate", "interrupt", "complete", "abandon"] as const +const expectedActions = [ + "delegate", + "interrupt", + "complete", + "abandon", + "markLiveElsewhere", + "expireLiveElsewhere", + "reconcileStartup", +] as const const semanticLandmarks = { "interrupted-child-redelegation": (state: ModelState) => - state.parent?.status === "delegated" && - state.parent.awaitingChildId === "child-b" && - state["child-a"]?.status === "interrupted", + state.tasks.parent?.status === "delegated" && + state.tasks.parent.awaitingChildId === "child-b" && + state.tasks["child-a"]?.status === "interrupted", "nested-delegation": (state: ModelState) => - state.parent?.status === "delegated" && - state.parent.awaitingChildId === "child-a" && - state["child-a"]?.status === "delegated" && - state["child-a"].awaitingChildId === "child-b", + state.tasks.parent?.status === "delegated" && + state.tasks.parent.awaitingChildId === "child-a" && + state.tasks["child-a"]?.status === "delegated" && + state.tasks["child-a"].awaitingChildId === "child-b", + // Proves the fix for the cross-window misrepair bug (PR #1495): startup + // reconciliation must leave a delegated parent awaiting an active child + // owned by another window untouched. The reconciliation skip is an identity + // transition, so this landmark plus the universal transition invariant in + // `checkTransitionInvariants` (no reachable action may clear the link while + // the child is active and live-elsewhere) formalizes "not repaired". + "live-child-preserved-across-reconciliation": (state: ModelState) => { + const parent = state.tasks.parent + if (parent?.status !== "delegated" || !parent.awaitingChildId) { + return false + } + const childId = parent.awaitingChildId as TaskId + return state.tasks[childId]?.status === "active" && state.liveElsewhere[childId] + }, + // Proves the repair half of the same reconciliation outcome still works: a + // non-live (crash-orphan) active child is repaired to interrupted while the + // parent resumes as active with both delegation pointers cleared. This + // state class is only reachable through `reconcileStartup`, never through + // `interrupt`/`abandon`/`complete`. + "crash-orphan-repaired-by-startup": (state: ModelState) => { + const parent = state.tasks.parent + const child = state.tasks["child-a"] + return ( + parent?.status === "active" && + !parent.awaitingChildId && + child?.status === "interrupted" && + child.parentTaskId === "parent" && + !state.liveElsewhere["child-a"] + ) + }, } satisfies Record boolean> function task(id: TaskId, parentTaskId?: TaskId): HistoryItem { @@ -55,24 +110,29 @@ function task(id: TaskId, parentTaskId?: TaskId): HistoryItem { } function initialState(): ModelState { - return { parent: task("parent"), "child-a": undefined, "child-b": undefined } + return { + tasks: { parent: task("parent"), "child-a": undefined, "child-b": undefined }, + liveElsewhere: { parent: false, "child-a": false, "child-b": false }, + } } function replace(state: ModelState, ...updates: HistoryItem[]): ModelState { - const next = { ...state } - for (const update of updates) next[update.id as TaskId] = update - return next + const tasks = { ...state.tasks } + for (const update of updates) tasks[update.id as TaskId] = update + return { tasks, liveElsewhere: state.liveElsewhere } } function transitions(state: ModelState): Transition[] { const result: Transition[] = [] for (const parentId of taskIds) { - const parent = state[parentId] + const parent = state.tasks[parentId] if (!parent) continue for (const childId of taskIds) { - if (childId === parentId || state[childId]) continue - const awaitedStatus = parent.awaitingChildId ? state[parent.awaitingChildId as TaskId]?.status : undefined + if (childId === parentId || state.tasks[childId]) continue + const awaitedStatus = parent.awaitingChildId + ? state.tasks[parent.awaitingChildId as TaskId]?.status + : undefined if (parent.status !== "active" && !(parent.status === "delegated" && awaitedStatus === "interrupted")) { continue } @@ -85,11 +145,18 @@ function transitions(state: ModelState): Transition[] { } for (const childId of taskIds) { - const child = state[childId] + const child = state.tasks[childId] if (!child?.parentTaskId) continue - const parent = state[child.parentTaskId as TaskId] + const parent = state.tasks[child.parentTaskId as TaskId] if (!parent) continue + // A child marked live-elsewhere is owned by another window's session, so + // window-local lifecycle operations cannot target it until the flag + // expires. `checkTransitionInvariants` re-proves universally that no + // reachable action clears the parent's link while the child is active + // and live-elsewhere. + if (state.liveElsewhere[childId]) continue + if (parent.status === "delegated" && parent.awaitingChildId === child.id && child.status === "active") { const interrupted = interruptDelegatedChild(parent, child) result.push({ name: `interrupt(${childId})`, next: replace(state, interrupted) }) @@ -115,13 +182,77 @@ function transitions(state: ModelState): Transition[] { }) } } + + // Cross-window startup reconciliation (`TaskHistoryStore.reconcileDelegationStateCore`, + // run at initialize() and on every periodic tick). For every delegated parent + // whose awaited child is active, the outcome is decided solely by the + // abstract liveness flag: + // - stale/unreadable mtime (not live-elsewhere) → repair: child → interrupted + // via the shared production reducer, parent → active with both delegation + // pointers cleared. The parent-side rewrite is modeled directly here + // because production performs it as administrative recovery through + // `upsertCore(..., { skipTransitionCheck: true })`, outside the shared + // `taskLifecycle.ts` reducers; the child side matches `interruptDelegatedChild`. + // - recent mtime (live-elsewhere) → skip: the pre-fix bug repaired exactly + // this child, breaking the delegation link so the subtask's completion + // could no longer return to the parent. The fix `continue`s, so the + // action stays observable (it still marks `reconcileStartup` as executed) + // while intentionally not producing a new state. + for (const parentId of taskIds) { + const parent = state.tasks[parentId] + if (parent?.status !== "delegated" || !parent.awaitingChildId) continue + const childId = parent.awaitingChildId as TaskId + const child = state.tasks[childId] + if (child?.status !== "active") continue + if (state.liveElsewhere[childId]) { + result.push({ name: `reconcileStartup(${parentId})`, next: state }) + continue + } + const repairedParent: HistoryItem = { + ...parent, + status: "active", + awaitingChildId: undefined, + delegatedToId: undefined, + } + const repairedChild = interruptDelegatedChild(parent, child) + result.push({ + name: `reconcileStartup(${parentId})`, + next: replace(state, repairedParent, repairedChild), + }) + } + + // Model actions for the abstract mtime liveness flag: `markLiveElsewhere` + // represents another window actively persisting the child (recent mtime), + // and `expireLiveElsewhere` represents the owning window going quiet past + // the threshold (e.g. it crashed after startup skipped its repair), after + // which the next `reconcileStartup` repairs it as a crash orphan. Only + // active tasks that are themselves children can toggle the flag; the root + // slot has no owning window in this bug class, and restricting the flag to + // child sessions keeps the liveness dimension from multiplying the state + // space beyond the explicit budget. + for (const id of taskIds) { + const current = state.tasks[id] + if (current?.status !== "active" || !current.parentTaskId) continue + const id2 = id as TaskId + if (!state.liveElsewhere[id2]) { + result.push({ + name: `markLiveElsewhere(${id2})`, + next: { tasks: state.tasks, liveElsewhere: { ...state.liveElsewhere, [id2]: true } }, + }) + } else { + result.push({ + name: `expireLiveElsewhere(${id2})`, + next: { tasks: state.tasks, liveElsewhere: { ...state.liveElsewhere, [id2]: false } }, + }) + } + } return result } function invariantViolations(state: ModelState): string[] { const violations: string[] = [] for (const id of taskIds) { - const current = state[id] + const current = state.tasks[id] if (!current) continue if (current.status === "delegated") { @@ -129,7 +260,7 @@ function invariantViolations(state: ModelState): string[] { violations.push(`${id}: delegated task must point to exactly one awaited child`) continue } - const child = state[current.awaitingChildId as TaskId] + const child = state.tasks[current.awaitingChildId as TaskId] if (!child || child.parentTaskId !== id || child.status === "completed") { violations.push(`${id}: awaited child must exist, link back, and not be completed`) } @@ -141,7 +272,7 @@ function invariantViolations(state: ModelState): string[] { } if (current.parentTaskId && current.status !== "interrupted") { - const parent = state[current.parentTaskId as TaskId] + const parent = state.tasks[current.parentTaskId as TaskId] if (current.status !== "completed" && parent?.awaitingChildId !== id) { violations.push(`${id}: active or delegated linked child must be the child its parent awaits`) } @@ -155,14 +286,14 @@ function invariantViolations(state: ModelState): string[] { break } ancestors.add(cursor) - cursor = state[cursor as TaskId]?.parentTaskId + cursor = state.tasks[cursor as TaskId]?.parentTaskId } } return violations } function canonical(state: ModelState): string { - return JSON.stringify(taskIds.map((id) => state[id] ?? null)) + return JSON.stringify([taskIds.map((id) => state.tasks[id] ?? null), taskIds.map((id) => state.liveElsewhere[id])]) } function formatCounterexample(message: string, trace: TraceStep[]): string { @@ -183,10 +314,29 @@ function formatCounterexample(message: string, trace: TraceStep[]): string { function checkTransitionInvariants(previous: ModelState, transition: Transition): string[] { const violations: string[] = [] for (const id of taskIds) { - const before = previous[id] - const after = transition.next[id] + const before = previous.tasks[id] + const after = transition.next.tasks[id] if (before?.status === "completed" && canonicalTask(before) !== canonicalTask(after)) { violations.push(`${id}: completed task changed after ${transition.name}`) + continue + } + // Cross-window ownership guard (PR #1495 bug class): no transition may + // clear a delegated parent's link to a child that is active AND marked + // live-elsewhere. Pre-fix, startup reconciliation repaired exactly these + // children; the mtime guard skips them, so the only enabled successor for + // such a state is the identity reconciliation. Any future model edit + // that reintroduces a link-clearing transition on a live-elsewhere child + // fails here with the shortest causal trace. + if (before?.status === "delegated" && before.awaitingChildId) { + const childId = before.awaitingChildId as TaskId + const childBefore = previous.tasks[childId] + if (childBefore?.status === "active" && previous.liveElsewhere[childId]) { + if (after?.status !== "delegated" || after.awaitingChildId !== childId) { + violations.push( + `${id}: ${transition.name} cleared delegation to active live-elsewhere child ${childId}`, + ) + } + } } } return violations From 576a73e98527912b75f5dca60caf46a48d687f80 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 09:57:37 +0900 Subject: [PATCH 11/13] chore(gitignore): drop local worktree and scratch ignore patterns --- .gitignore | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.gitignore b/.gitignore index 584f9177fa..3961778d5e 100644 --- a/.gitignore +++ b/.gitignore @@ -59,14 +59,3 @@ qdrant_storage/ plans/ roo-cli-*.tar.gz* - -# Local worktrees (multi-branch development) -.wt-*/ - -# Temporary scratch files -.tmp-* -.zoo-status.txt -untracked-*.txt -class_*.txt -check-dup2-result.txt -*.tsbuildinfo From af3e93af1bd1c6b25530486b434fb6c42f92b10c Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 16:53:30 +0900 Subject: [PATCH 12/13] test(delegation): kill 15 surviving changed-code mutants --- .../TaskHistoryStore.reconciliation.spec.ts | 497 ++++++++++++++++++ 1 file changed, 497 insertions(+) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 2827fa3b0a..422894e4cb 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -1980,3 +1980,500 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { throwingSpy.mockRestore() }) }) + +// ───────────────────────────────────────────────────────────────────────────── +// Mutation-gate kill tests — focused coverage for the 15 surviving changed-code +// mutants reproduced locally against TaskHistoryStore.ts (PR #1495 mutation-diff +// gate). Each test names the exact mutant(s) it kills and asserts an observable +// behavioral difference so the mutant cannot survive. +// ───────────────────────────────────────────────────────────────────────────── + +describe("TaskHistoryStore mutation-gate kill tests", () => { + let tmpDir: string + let store: TaskHistoryStore | undefined + let mtimeSpy: { mockRestore(): void } | undefined + + const RECONCILE_INTERVAL_MS = (TaskHistoryStore as unknown as { RECONCILE_INTERVAL_MS: number }) + .RECONCILE_INTERVAL_MS + + function useTickClock(): void { + vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "Date"] }) + } + + async function flushAsyncWork(yields = 2000): Promise { + for (let i = 0; i < yields; i++) { + await vi.advanceTimersByTimeAsync(1) + } + } + + async function seedItems(items: HistoryItem[]): Promise { + const tasksDir = path.join(tmpDir, "tasks") + await fs.mkdir(tasksDir, { recursive: true }) + for (const item of items) { + const taskDir = path.join(tasksDir, item.id) + await fs.mkdir(taskDir, { recursive: true }) + await fs.writeFile(path.join(taskDir, "history_item.json"), JSON.stringify(item)) + } + } + + /** + * Read the private `locallyActiveTaskIds` set — the exact piece of state every + * ownership-track mutant below (L278/L299/L324/L566/L569/L1181/L1188/L1189) + * mutates. Its documented consumer is the periodic tick's orphan-repair + * exclusion (TaskHistoryStore.ts line 1083), so asserting membership is a + * direct observable of the mutated behavior. Same private-member cast pattern + * as LIVE_CHILD_MTIME_THRESHOLD_MS at the top of this spec. + */ + function ownedIds(s: TaskHistoryStore): Set { + return (s as unknown as { locallyActiveTaskIds: Set }).locallyActiveTaskIds + } + + /** Inject a stale mtime for `childId` so the liveness guard sees a crash orphan. */ + function installStaleChildInjector(childId: string): void { + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (id: string) => Promise + } + const original = probe.getChildFileMtimeMs + mtimeSpy = vi + .spyOn(probe, "getChildFileMtimeMs") + .mockImplementation((id: string) => + id === childId ? Promise.resolve(Date.now() - 10 * 60 * 1000) : original.call(store!, id), + ) + } + + function delegatedPair(parentId: string, childId: string): HistoryItem[] { + const child = makeItem({ id: childId, status: "active", parentTaskId: parentId, rootTaskId: parentId }) + const parent = makeItem({ + id: parentId, + status: "delegated", + awaitingChildId: childId, + delegatedToId: childId, + childIds: [childId], + }) + return [parent, child] + } + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "mutkill-test-")) + }) + + afterEach(async () => { + mtimeSpy?.mockRestore() + mtimeSpy = undefined + store?.dispose() + store = undefined + vi.useRealTimers() + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + }) + + it("repair writes (skipTransitionCheck) do NOT register local ownership (kills L278 ConditionalExpression)", async () => { + // upsertCore: `if (!options.skipTransitionCheck) { trackLocalSessionOwnership(written) }`. + // The "interrupted handoff" repair path (reconcileDelegationStateCore) sets the parent to + // ACTIVE via upsertCore(..., { skipTransitionCheck: true }). Replacing `!options.skipTransitionCheck` + // with `true` would ALSO run trackLocalSessionOwnership(written) for that repair write, and + // because written.status === "active" the parent would be ADDED to locallyActiveTaskIds. + // Assert the repaired parent is NOT in the ownership set: present under the mutant, absent + // under correct code. + const child = makeItem({ + id: "child-l278", + status: "completed", + completionResultSummary: "done", + parentTaskId: "parent-l278", + rootTaskId: "parent-l278", + }) + const parent = makeItem({ + id: "parent-l278", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + + // The completed-child handoff repaired the parent to active via skipTransitionCheck. + expect(s.get(parent.id)?.status).toBe("active") + expect(s.get(parent.id)?.awaitingChildId).toBeUndefined() + // Under L278->true the active repair write adds parent.id to this set; correct code does not. + expect(ownedIds(s).has(parent.id)).toBe(false) + }) + + it("non-active runtime write DELETES local ownership (kills L566 Conditional->true / LogicalOperator / StringLiteral, L569 CallExpression)", async () => { + // trackLocalSessionOwnership: `if ((written.status ?? "active") === "active") add else delete`. + // Observable under test: after an active runtime write registers ownership, a later NON-active + // runtime write must remove it (the else/`delete(id)` branch). If the mutant forces the add + // branch (L566 ->true) or drops the delete (L569 `;`), the task stays owned and the periodic + // tick will NOT repair it as a crash orphan. + // + // Sequence on ONE task id `orphan`: + // 1. runtime `active` write -> ownership ADDED. + // 2. runtime `completed` write (valid active->completed) -> ownership DELETED. + // 3. seed disk so the SAME id is again an active child of a delegated parent (crash orphan) + // and reload the store — the only ownership signal is from step 1/2 runtime writes. + // 4. tick: with ownership deleted, the orphan is repaired (interrupted). Under either mutant + // it stays owned -> stays active. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const childId = "orphan-l566" + const parentId = "parent-l566" + + const s = (store = new TaskHistoryStore(tmpDir)) + useTickClock() + await s.initialize() + + // Steps 1+2: register then delete ownership via valid runtime transitions. + await s.upsert(makeItem({ id: childId, status: "active", parentTaskId: parentId, rootTaskId: parentId })) + await s.atomicReadAndUpdate(childId, (c) => ({ ...c, status: "completed" as const })) + expect(s.get(childId)?.status).toBe("completed") + s.dispose() + + // Step 3: rewrite disk so the same child id is once more an ACTIVE orphan of a delegated + // parent (as if another window crashed mid-delegation), then reload into a fresh store. + const [parent, child] = delegatedPair(parentId, childId) + await seedItems([parent, child]) + const s2 = (store = new TaskHistoryStore(tmpDir)) + useTickClock() + + // Startup reconciliation must NOT repair it yet: make the mtime look live at startup, then + // stale only for the tick. + let age = 60_000 + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (id: string) => Promise + } + mtimeSpy = vi + .spyOn(probe, "getChildFileMtimeMs") + .mockImplementation((id: string) => + id === childId ? Promise.resolve(Date.now() - age) : Promise.resolve(undefined), + ) + + await s2.initialize() + expect(s2.get(childId)?.status).toBe("active") + + // Step 4: tick with a now-stale mtime. + age = 10 * 60 * 1000 + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + + // Ownership was deleted by the completed write, so the orphan is repaired. + // (Under L566->true or L569 `;` it would remain owned and stay active.) + expect(s2.get(childId)?.status).toBe("interrupted") + expect(errorSpy).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it("non-active runtime write removes the id from locallyActiveTaskIds (kills L566 Conditional->true / LogicalOperator, L569 CallExpression)", async () => { + // Direct set assertion for the else/`delete(id)` branch of trackLocalSessionOwnership. + // After an active runtime write the id is present; after a completed runtime write it must + // be removed. Under L566->true (forced add branch) or L569 `;` (delete dropped), the id + // would still be present after the completed write. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "own-add", status: "active" })) + expect(ownedIds(s).has("own-add")).toBe(true) + + // Valid active -> completed transition exercises the else branch (delete). + await s.upsert(makeItem({ id: "own-add", status: "completed" })) + expect(ownedIds(s).has("own-add")).toBe(false) + }) + + it("active runtime write adds the id to locallyActiveTaskIds (kills L566 Conditional->true add-branch, LogicalOperator)", async () => { + // Complement: the add branch must actually insert. Under L566 LogicalOperator mutants + // (e.g. `written.status && "active"`), an explicit "active" status short-circuits to a + // truthy-but-not-"active" value, so `=== "active"` is false and the add is skipped. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "add-explicit", status: "active" })) + expect(ownedIds(s).has("add-explicit")).toBe(true) + }) + + it('undefined status is treated as implicit active and registers ownership (kills L566 StringLiteral->"")', async () => { + // `(written.status ?? "active") === "active"`: StringLiteral->"" makes undefined status fall + // to "" !== "active" -> delete branch. A runtime write with NO status field must still count + // as implicit active and register ownership, so the tick leaves this in-window child alone. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + const childId = "child-l566-undef" + const parentId = "parent-l566-undef" + const [parent, child] = delegatedPair(parentId, childId) + await seedItems([parent, child]) + + const s = (store = new TaskHistoryStore(tmpDir)) + useTickClock() + await s.initialize() + await s.upsert(makeItem({ id: parentId, status: "active" })) + // Runtime write with status omitted entirely (legacy implicit active). + const noStatus = makeItem({ id: childId, parentTaskId: parentId, rootTaskId: parentId }) + delete (noStatus as Partial).status + await s.upsert(noStatus) + // Delegate the pair; the child must remain owned HERE because its write was implicit-active. + await s.atomicReadAndUpdate(parentId, (c) => ({ + ...c, + status: "delegated" as const, + awaitingChildId: childId, + delegatedToId: childId, + })) + await s.atomicReadAndUpdate(childId, (c) => ({ ...c, status: "active" as const })) + + installStaleChildInjector(childId) + await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) + await flushAsyncWork() + + // Owned here (implicit active) -> the tick must NOT tear it away from its own runner. + expect(s.get(childId)?.status).toBe("active") + expect(s.get(parentId)?.status).toBe("delegated") + expect(errorSpy).not.toHaveBeenCalled() + errorSpy.mockRestore() + }) + + it('undefined status is treated as implicit active and adds the id to locallyActiveTaskIds (kills L566 StringLiteral->"")', async () => { + // `(written.status ?? "active") === "active"`: StringLiteral->"" makes an undefined status + // fall to `"" !== "active"` -> delete branch, so the id is never added. A runtime write with + // NO status field must count as implicit active and register ownership. Assert membership. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + const noStatus = makeItem({ id: "undef-status" }) + delete (noStatus as Partial).status + await s.upsert(noStatus) + // Under L566 StringLiteral->"" this stays absent; correct code adds it. + expect(ownedIds(s).has("undef-status")).toBe(true) + }) + + it("delete() removes the id from locallyActiveTaskIds (kills L299 CallExpression)", async () => { + // delete(): the `locallyActiveTaskIds.delete(taskId)` statement is the CallExpression the + // mutant drops (`;`). Register ownership via an active runtime write, then delete the task + // and assert the id is gone from the ownership set — under the mutant it would remain. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "del-l299", status: "active" })) + expect(ownedIds(s).has("del-l299")).toBe(true) + + await s.delete("del-l299") + expect(s.get("del-l299")).toBeUndefined() + expect(ownedIds(s).has("del-l299")).toBe(false) + }) + + it("deleteMany() removes every deleted id from locallyActiveTaskIds (kills L324 CallExpression)", async () => { + // deleteMany(): the per-task `locallyActiveTaskIds.delete(taskId)` is the CallExpression the + // mutant drops. Own two tasks, delete both, and assert neither remains in the ownership set. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "dm-1", status: "active" })) + await s.upsert(makeItem({ id: "dm-2", status: "active" })) + await s.upsert(makeItem({ id: "dm-3", status: "active" })) + expect(ownedIds(s).has("dm-1")).toBe(true) + expect(ownedIds(s).has("dm-3")).toBe(true) + + await s.deleteMany(["dm-1", "dm-3"]) + expect(s.get("dm-1")).toBeUndefined() + expect(s.get("dm-3")).toBeUndefined() + expect(ownedIds(s).has("dm-1")).toBe(false) + expect(ownedIds(s).has("dm-3")).toBe(false) + // Untouched task keeps its ownership. + expect(ownedIds(s).has("dm-2")).toBe(true) + }) + + it("replay liveness guard treats child file age exactly at threshold as NOT live and repairs (kills L627 EqualityOperator '<'->'<=')", async () => { + // replayDelegationRepairIntent: `Date.now() - mtimeMs < LIVE_CHILD_MTIME_THRESHOLD_MS`. + // Under `<=`, age === threshold counts as live and the stale intent is quarantined. With the + // real strict `<`, age === threshold is NOT live, so the crash-orphan intent is replayed: + // child -> interrupted, parent -> active. Assert the replay happens at exactly threshold. + const FIXED_NOW = 1_756_886_400_000 + const nowSpy = vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW) + try { + const child = makeItem({ + id: "child-l627", + status: "active", + parentTaskId: "parent-l627", + rootTaskId: "parent-l627", + }) + const parent = makeItem({ + id: "parent-l627", + status: "delegated", + awaitingChildId: child.id, + delegatedToId: child.id, + }) + await seedItems([parent, child]) + const tasksDir = path.join(tmpDir, "tasks") + const intentPath = path.join(tasksDir, GlobalFileNames.delegationRepairIntent) + await fs.writeFile(intentPath, JSON.stringify(makeRepairIntent(parent, child))) + + // Isolate the REPLAY guard's decision from the later startup reconcile (step 3 of + // initialize). The replay runs first and reads getChildFileMtimeMs once; make that first + // call return age EXACTLY == threshold, then make every subsequent call (the step-3 + // startup reconcile's own liveness probe) return a RECENT age so step 3 treats the child + // as live and does NOT repair it. The child's final status then reflects ONLY the replay + // guard at line 627: strict '<' (correct) -> threshold age is NOT live -> replay repairs + // (child interrupted); '<=' (mutant) -> live -> quarantine (child stays active). + let probeCalls = 0 + const probe = TaskHistoryStore.prototype as unknown as { + getChildFileMtimeMs: (id: string) => Promise + } + mtimeSpy = vi.spyOn(probe, "getChildFileMtimeMs").mockImplementation((id: string) => { + if (id !== child.id) return Promise.resolve(undefined) + probeCalls++ + // First call = the replayDelegationRepairIntent guard (line 627): exactly threshold. + // Later calls = the startup reconcile guard (line 506): recent -> child stays live. + return Promise.resolve(probeCalls === 1 ? FIXED_NOW - LIVE_CHILD_MTIME_THRESHOLD_MS : FIXED_NOW - 1_000) + }) + + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + + // Strict '<': threshold age is NOT live -> the intent replays (child interrupted). + // Under '<=': the intent would be quarantined and the child would stay active (step 3 + // sees the child as live and leaves it alone). + expect(s.get(child.id)?.status).toBe("interrupted") + expect(s.get(parent.id)?.status).toBe("active") + } finally { + nowSpy.mockRestore() + } + }) + + it("runPeriodicDelegationReconciliation does not run the pass when disposed (kills L1077 Conditional->false / LogicalOperator)", async () => { + // `if (this.disposed || this.delegationTickRunning) return`. Conditional->false forces the + // guard OFF so the pass runs even after dispose(); LogicalOperator->&& makes it run only + // when disposed AND already-running (also wrong). The observable is whether the method + // reaches reconcileDelegationState. Assert that after dispose() the pass body does NOT run. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + s.dispose() + + const internals = TaskHistoryStore.prototype as unknown as { + runPeriodicDelegationReconciliation: () => Promise + } + // Hook reconcileDelegationState to detect whether the guarded body executes. + const reconProbe = s as unknown as { reconcileDelegationState: (ids: Set) => Promise } + let passRan = false + reconProbe.reconcileDelegationState = async () => { + passRan = true + } + + await internals.runPeriodicDelegationReconciliation.call(s) + // Guard fired (disposed) -> the pass body never ran. Under L1077->false it would run. + expect(passRan).toBe(false) + }) + + it("runPeriodicDelegationReconciliation runs the pass when NOT disposed and NOT already running (kills L1077 LogicalOperator->&&)", async () => { + // Complement: with disposed=false and delegationTickRunning=false the guard must NOT fire, + // so the pass runs. Under LogicalOperator->&& the condition `disposed && tickRunning` is + // false here too... but Conditional->true (always skip) would suppress the run. Assert the + // pass executes in the normal case, pinning the guard's truth table from the other side. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + + const internals = TaskHistoryStore.prototype as unknown as { + runPeriodicDelegationReconciliation: () => Promise + } + const reconProbe = s as unknown as { reconcileDelegationState: (ids: Set) => Promise } + let passRan = false + reconProbe.reconcileDelegationState = async () => { + passRan = true + } + + await internals.runPeriodicDelegationReconciliation.call(s) + expect(passRan).toBe(true) + }) + + it("runPeriodicDelegationReconciliation sets then clears delegationTickRunning around the pass (kills L1080 BooleanLiteral->false, L1087 BooleanLiteral->true)", async () => { + // L1080 sets the flag true before the pass; L1087 clears it false in `finally`. + // - L1080->false: a concurrent second call would NOT see the flag set and would run twice. + // - L1087->true: after completion the flag stays set, so every subsequent call no-ops. + const [parent, child] = delegatedPair("parent-flag", "child-flag") + await seedItems([parent, child]) + const s = (store = new TaskHistoryStore(tmpDir)) + installStaleChildInjector(child.id) + await s.initialize() + + const internals = TaskHistoryStore.prototype as unknown as { + runPeriodicDelegationReconciliation: () => Promise + } + const flagReader = s as unknown as { delegationTickRunning: boolean } + + // Observe the flag being true DURING the pass via a hook into reconcileDelegationState. + const reconProbe = s as unknown as { reconcileDelegationState: (ids: Set) => Promise } + const originalRecon = reconProbe.reconcileDelegationState.bind(s) + let flagDuringPass: boolean | undefined + reconProbe.reconcileDelegationState = async (ids: Set) => { + flagDuringPass = flagReader.delegationTickRunning + return originalRecon(ids) + } + + await internals.runPeriodicDelegationReconciliation.call(s) + // Flag was true while the pass ran (kills L1080->false). + expect(flagDuringPass).toBe(true) + // Flag cleared after the pass completed (kills L1087->true). + expect(flagReader.delegationTickRunning).toBe(false) + + // A second call runs again (proves the flag was actually reset, not stuck). + let secondRan = false + reconProbe.reconcileDelegationState = async (ids: Set) => { + secondRan = true + return originalRecon(ids) + } + await internals.runPeriodicDelegationReconciliation.call(s) + expect(secondRan).toBe(true) + }) + + it("atomicUpdatePair registers ownership for both records on success (kills L1188/L1189 CallExpression)", async () => { + // The success path calls trackLocalSessionOwnership(writtenFirst) and (writtenSecond). The + // CallExpression `;` mutants drop those calls, so the ids never enter locallyActiveTaskIds. + // Assert both ids are present after a pair write that leaves both active. + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "pf-first", status: "active" })) + await s.upsert(makeItem({ id: "pf-second", status: "active" })) + // Clear prior ownership so only the atomicUpdatePair calls can re-add them. + ownedIds(s).clear() + expect(ownedIds(s).size).toBe(0) + + await s.atomicUpdatePair( + "pf-first", + "pf-second", + (c) => ({ ...c, status: "active" as const }), + (c) => ({ ...c, status: "active" as const }), + ) + + // Under L1188/L1189 `;` the trackLocalSessionOwnership calls vanish and these stay absent. + expect(ownedIds(s).has("pf-first")).toBe(true) + expect(ownedIds(s).has("pf-second")).toBe(true) + }) + + it("atomicUpdatePair registers ownership for the committed first record on partial failure (kills L1181 CallExpression)", async () => { + // On second-write failure the catch block updates the cache AND calls + // trackLocalSessionOwnership(writtenFirst) before rethrowing. The `;` mutant drops that call, + // so the committed first record never enters locallyActiveTaskIds. Force the SECOND + // writeTaskFile to fail, then assert the first record IS in the ownership set (under the + // mutant it stays absent). + const s = (store = new TaskHistoryStore(tmpDir)) + await s.initialize() + await s.upsert(makeItem({ id: "pf-first", status: "active" })) + await s.upsert(makeItem({ id: "pf-second", status: "active" })) + ownedIds(s).clear() + + // Spy writeTaskFile: succeed for the first record, reject for the second, so the catch + // path (which contains L1181) runs. + const storeAny = s as unknown as { writeTaskFile: (item: HistoryItem, delta?: unknown) => Promise } + const originalWrite = storeAny.writeTaskFile.bind(s) + const writeSpy = vi + .spyOn(storeAny, "writeTaskFile") + .mockImplementation(async (item: HistoryItem, delta?: unknown) => { + if (item.id === "pf-second") { + throw new Error("simulated second-write failure") + } + return originalWrite(item, delta) + }) + + await expect( + s.atomicUpdatePair( + "pf-first", + "pf-second", + (c) => ({ ...c, status: "active" as const }), + (c) => ({ ...c, status: "active" as const }), + ), + ).rejects.toThrow("simulated second-write failure") + + // The catch block committed pf-first to disk and must have registered its ownership. + // Under L1181 `;` that call vanishes and pf-first stays absent. + expect(ownedIds(s).has("pf-first")).toBe(true) + writeSpy.mockRestore() + }) +}) From 0c74d2dff6fcc89463d6bb43a7dd7702c968c857 Mon Sep 17 00:00:00 2001 From: "Zoo (VP)" Date: Sat, 5 Sep 2026 20:10:12 +0900 Subject: [PATCH 13/13] test(delegation): replace fixed-count timer pump with condition polling --- .../TaskHistoryStore.reconciliation.spec.ts | 158 +++++++++++++++--- 1 file changed, 139 insertions(+), 19 deletions(-) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts index 422894e4cb..95bdfa5f72 100644 --- a/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.reconciliation.spec.ts @@ -1716,7 +1716,7 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { /** * Fake only what the tick scheduling needs: the 5-minute `setTimeout` clock * and `Date` (consumed by the liveness guard). Everything else (fs I/O, - * microtasks) stays real so `flushAsyncWork()` below can pump the event + * microtasks) stays real so `flushUntil()` below can pump the event * loop while the timer clock advances only 1 ms per yield. */ function useTickClock(): void { @@ -1724,18 +1724,49 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { } /** - * Drain pending real fs I/O. The tick's reconcile/repair chain completes on - * libuv callbacks that fake timers alone never advance, and each - * `advanceTimersByTimeAsync(1)` yields one REAL macrotask turn (processing - * the poll phase) while advancing the fake clock only 1 ms. The total fake - * time here stays far below RECONCILE_INTERVAL_MS, so no extra tick fires - * during the pump — this only lets in-flight fs callbacks settle. The count - * is generous to absorb Windows antivirus/OneDrive fs latency. + * Drain pending real fs I/O by polling an observable condition instead of + * burning a fixed number of yields. The tick's reconcile/repair chain + * completes on libuv callbacks that fake timers alone never advance, and + * each `advanceTimersByTimeAsync(1)` yields one REAL macrotask turn + * (processing the poll phase) while advancing the fake clock only 1 ms. + * The yield count the chain needs is environment-dependent (~155 yields on + * a fast local SSD; higher on contended CI runners — the old fixed + * 2000-yield pumps intermittently starved on ubuntu CI, which is exactly + * what this helper replaces). Polling the SAME final state the assertions + * check makes the wait deterministic without weakening them. The pump + * stops as soon as the condition holds, so correct-code runs stay fast, + * and the generous cap costs sub-second wall time even when exhausted + * because fake timers never sleep (measured ~123 ms per 55K idle yields). + * On exhaustion it THROWS with a state snapshot rather than silently + * proceeding, converting a future hang into a loud, diagnosable failure. + * + * Predicates MUST be cheap and side-effect free: poll the in-memory cache + * getters (`store.get(...)`, which never touches disk) or spy call logs. */ - async function flushAsyncWork(yields = 2000): Promise { - for (let i = 0; i < yields; i++) { + async function flushUntil( + predicate: () => boolean, + options: { maxYields?: number; label?: string; snapshot?: () => string } = {}, + ): Promise { + const { maxYields = 50_000, label = "flushUntil predicate", snapshot } = options + for (let i = 0; i < maxYields; i++) { + if (predicate()) { + return + } await vi.advanceTimersByTimeAsync(1) } + if (predicate()) { + return + } + let state = "snapshot unavailable" + try { + state = snapshot ? snapshot() : "no snapshot supplied" + } catch { + // A throwing snapshot must not mask the primary diagnostic below. + } + throw new Error( + `flushUntil: "${label}" was not satisfied within ${maxYields} yields (~${maxYields} ms of fake ` + + `time). The tick's async chain never settled; final state: ${state}.`, + ) } async function seedItems(items: HistoryItem[]): Promise { @@ -1824,7 +1855,24 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { // next periodic tick its mtime is past the liveness threshold. childAgeMs = 10 * 60 * 1000 await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + // Compound predicate: the "Reconciled orphaned active child" warn is + // emitted only after repairActiveDelegation fully resolves (intent + // write, both task-file writes, cache updates, intent cleanup), so + // waiting for the cache flip AND the warn settles every observable the + // assertions below depend on — a cache-only predicate could return + // before the warnSpy assertion is satisfiable. + await flushUntil( + () => + s.get(CHILD_ID)?.status === "interrupted" && + warnSpy.mock.calls.some( + (c) => typeof c[0] === "string" && c[0].includes("Reconciled orphaned active child"), + ), + { + label: "stale child repaired to interrupted and the repair was logged", + snapshot: () => + `child=${s.get(CHILD_ID)?.status} parent=${s.get(PARENT_ID)?.status} warnCalls=${warnSpy.mock.calls.length}`, + }, + ) // Within ONE interval, the parent window must repair: child → interrupted, // parent → active with delegation links cleared. @@ -1900,8 +1948,19 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { childId === CHILD_ID ? Promise.resolve(Date.now() - 10 * 60 * 1000) : realProbe.call(s, childId), ) + // Negative test: the tick must do NOTHING to this locally-owned child, + // so no positive log exists to poll. Settle on the recursive re-arm + // instead — `startPeriodicReconciliation()` only re-runs after BOTH + // `reconcile()` and the delegation pass have fully finished, so a + // fresh timer handle proves the whole tick settled. + const timerState = s as unknown as { reconcileTimer: ReturnType | null } + const timerBeforeTick = timerState.reconcileTimer await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + await flushUntil(() => timerState.reconcileTimer !== timerBeforeTick, { + label: "periodic tick completed without repairing the locally-owned child", + snapshot: () => + `child=${s.get(CHILD_ID)?.status} parent=${s.get(PARENT_ID)?.status} awaiting=${s.get(PARENT_ID)?.awaitingChildId}`, + }) expect(s.get(CHILD_ID)?.status).toBe("active") expect(s.get(PARENT_ID)?.status).toBe("delegated") @@ -1927,7 +1986,19 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { // The other window keeps writing: the child stays live at tick time. childAgeMs = 60_000 await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + // The skip-guard warn is this test's own observable (asserted below); + // once it fires the liveness check has run, no repair follows, and the + // persisted-file read below is safe (reconcile() never writes). + await flushUntil( + () => + logSpy.mock.calls.some( + (c) => typeof c[0] === "string" && c[0].includes(`Skipping repair for live child ${CHILD_ID}`), + ), + { + label: "tick skipped the repair for the live child", + snapshot: () => `child=${s.get(CHILD_ID)?.status} warnCalls=${logSpy.mock.calls.length}`, + }, + ) // Nothing may be repaired: child stays active, parent keeps its delegation links. expect(s.get(CHILD_ID)?.status).toBe("active") @@ -1964,7 +2035,18 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { await s.initialize() await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + // The error log happens in the tick callback's catch AFTER the throwing + // delegation step settles, so the spy firing means the tick is done. + await flushUntil( + () => + errorSpy.mock.calls.some( + (c) => typeof c[0] === "string" && c[0].includes("Periodic delegation reconciliation failed"), + ), + { + label: "tick logged the delegation failure", + snapshot: () => `errorCalls=${errorSpy.mock.calls.length}`, + }, + ) expect(errorSpy).toHaveBeenCalledWith( expect.stringContaining("Periodic delegation reconciliation failed"), expect.objectContaining({ message: "tick delegation boom" }), @@ -1973,7 +2055,10 @@ describe("TaskHistoryStore periodic delegation reconciliation", () => { // One more interval still fires the delegation step: the recursive // re-arm is preserved even though the step threw. await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + await flushUntil(() => throwingSpy.mock.calls.length >= 2, { + label: "second tick invoked the throwing delegation step", + snapshot: () => `throwingSpyCalls=${throwingSpy.mock.calls.length}`, + }) expect(throwingSpy).toHaveBeenCalledTimes(2) errorSpy.mockRestore() @@ -2000,10 +2085,34 @@ describe("TaskHistoryStore mutation-gate kill tests", () => { vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout", "Date"] }) } - async function flushAsyncWork(yields = 2000): Promise { - for (let i = 0; i < yields; i++) { + // Condition-polling pump; see the full doc comment on the identical helper + // in the "periodic delegation reconciliation" block above for the + // rationale (the fixed 2000-yield pumps intermittently starved on slow + // ubuntu CI runners). + async function flushUntil( + predicate: () => boolean, + options: { maxYields?: number; label?: string; snapshot?: () => string } = {}, + ): Promise { + const { maxYields = 50_000, label = "flushUntil predicate", snapshot } = options + for (let i = 0; i < maxYields; i++) { + if (predicate()) { + return + } await vi.advanceTimersByTimeAsync(1) } + if (predicate()) { + return + } + let state = "snapshot unavailable" + try { + state = snapshot ? snapshot() : "no snapshot supplied" + } catch { + // A throwing snapshot must not mask the primary diagnostic below. + } + throw new Error( + `flushUntil: "${label}" was not satisfied within ${maxYields} yields (~${maxYields} ms of fake ` + + `time). The tick's async chain never settled; final state: ${state}.`, + ) } async function seedItems(items: HistoryItem[]): Promise { @@ -2152,7 +2261,10 @@ describe("TaskHistoryStore mutation-gate kill tests", () => { // Step 4: tick with a now-stale mtime. age = 10 * 60 * 1000 await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + await flushUntil(() => s2.get(childId)?.status === "interrupted", { + label: "completed write released ownership so the tick repaired the orphan", + snapshot: () => `child=${s2.get(childId)?.status} parent=${s2.get(parentId)?.status}`, + }) // Ownership was deleted by the completed write, so the orphan is repaired. // (Under L566->true or L569 `;` it would remain owned and stay active.) @@ -2214,8 +2326,16 @@ describe("TaskHistoryStore mutation-gate kill tests", () => { await s.atomicReadAndUpdate(childId, (c) => ({ ...c, status: "active" as const })) installStaleChildInjector(childId) + // Negative test (child owned HERE via the implicit-active write): the + // tick must leave it alone, so settle on the recursive re-arm, which + // only happens after both passes fully finish. + const timerState = s as unknown as { reconcileTimer: ReturnType | null } + const timerBeforeTick = timerState.reconcileTimer await vi.advanceTimersByTimeAsync(RECONCILE_INTERVAL_MS) - await flushAsyncWork() + await flushUntil(() => timerState.reconcileTimer !== timerBeforeTick, { + label: "tick completed without clobbering the locally-owned implicit-active child", + snapshot: () => `child=${s.get(childId)?.status} parent=${s.get(parentId)?.status}`, + }) // Owned here (implicit active) -> the tick must NOT tear it away from its own runner. expect(s.get(childId)?.status).toBe("active")