From 131d18d9fcf4d2624867f6a57ed00e0c32860be8 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 27 Aug 2026 10:40:53 +0800 Subject: [PATCH 1/5] feat(file-safety): add version token for the guarded-write path (A1, #1375) Introduces the version token - dev:ino:size:mtimeNs:ctimeNs derived from a single fs.stat - a pure function of a file's on-disk state that every process computing from the same state agrees on. The compare-and-swap write guard (A2/A3) will compare the token observed at read time against the token recomputed before a write to detect stale or replaced files. No production callers yet: this is infrastructure for the file-write safety series (plan: easonLiangWorldedtech/Zoo-Code#33), part of upstream epic #1375. --- src/utils/__tests__/versionToken.spec.ts | 103 +++++++++++++++++++++++ src/utils/versionToken.ts | 57 +++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 src/utils/__tests__/versionToken.spec.ts create mode 100644 src/utils/versionToken.ts diff --git a/src/utils/__tests__/versionToken.spec.ts b/src/utils/__tests__/versionToken.spec.ts new file mode 100644 index 0000000000..56ce269244 --- /dev/null +++ b/src/utils/__tests__/versionToken.spec.ts @@ -0,0 +1,103 @@ +import * as fs from "fs/promises" +import * as os from "os" +import * as path from "path" +import type { Stats } from "fs" +import { afterEach, beforeEach, describe, expect, it } from "vitest" + +import { computeVersionToken, versionTokenOfStat } from "../versionToken" + +// Stats is a class-backed interface without a public constructor, so a plain-object +// test double is the only practical way to pin the token format without real files. +// Last-resort double assertion (test-local, per AGENTS.md). +function makeStats(overrides: Partial = {}): Stats { + const base: Partial = { + dev: 7, + ino: 4242, + size: 1234, + atimeMs: 1_700_000_000_000, + mtimeMs: 1_700_000_000_123.456, + ctimeMs: 1_700_000_000_789.999, + birthtimeMs: 1_700_000_000_000, + } + return { ...base, ...overrides } as unknown as Stats +} + +describe("versionTokenOfStat (A1, epic #1375)", () => { + it("is deterministic for an identical stat", () => { + expect(versionTokenOfStat(makeStats())).toBe(versionTokenOfStat(makeStats())) + }) + + it("matches the documented dev:ino:size:mtimeNs:ctimeNs format", () => { + const expected = [ + "7", + "4242", + "1234", + Math.round(1_700_000_000_123.456 * 1e6).toString(), + Math.round(1_700_000_000_789.999 * 1e6).toString(), + ].join(":") + expect(versionTokenOfStat(makeStats())).toBe(expected) + }) + + it("distinguishes size changes at identical timestamps", () => { + expect(versionTokenOfStat(makeStats({ size: 1235 }))).not.toBe(versionTokenOfStat(makeStats())) + }) + + it("distinguishes mtime changes at identical size", () => { + expect(versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_124 }))).not.toBe(versionTokenOfStat(makeStats())) + }) + + it("distinguishes a replaced file (dev/ino change) with identical content state", () => { + const replaced = makeStats({ dev: 8, ino: 999 }) + expect(versionTokenOfStat(replaced)).not.toBe(versionTokenOfStat(makeStats())) + }) + + it("preserves sub-ms mtime resolution in the ns field", () => { + const wholeMs = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123 })) + const halfMsLater = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123.5 })) + expect(halfMsLater).not.toBe(wholeMs) + // 0.5 ms = 500_000 ns. The float-derived ns field is quantized (~256 ns at + // this epoch), so allow a bounded drift instead of asserting an exact value. + const diff = Number(halfMsLater.split(":")[3]) - Number(wholeMs.split(":")[3]) + expect(Math.abs(diff - 500_000)).toBeLessThanOrEqual(512) + }) + + it("handles sizes beyond 32 bits without precision loss", () => { + const size = 5_000_000_000 // > 2^32 + const token = versionTokenOfStat(makeStats({ size })) + expect(token).toContain(`:4242:${size}:`) + }) +}) + +describe("computeVersionToken (A1, epic #1375)", () => { + let tmpDir: string + let file: string + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "version-token-")) + file = path.join(tmpDir, "seed.txt") + await fs.writeFile(file, "seed content", "utf8") + }) + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }) + }) + + it("derives the token from the on-disk state (single stat)", async () => { + const token = await computeVersionToken(file) + expect(token).toBe(versionTokenOfStat(await fs.stat(file))) + }) + + it("changes when the file content changes", async () => { + const before = await computeVersionToken(file) + // Different size + a new mtime — both must move the token. + await fs.writeFile(file, "seed content, extended", "utf8") + await new Promise((resolve) => setTimeout(resolve, 5)) + expect(await computeVersionToken(file)).not.toBe(before) + }) + + it("rejects with ENOENT for an absent file", async () => { + await expect(computeVersionToken(path.join(tmpDir, "absent.txt"))).rejects.toMatchObject({ + code: "ENOENT", + }) + }) +}) diff --git a/src/utils/versionToken.ts b/src/utils/versionToken.ts new file mode 100644 index 0000000000..ee8e9e82f1 --- /dev/null +++ b/src/utils/versionToken.ts @@ -0,0 +1,57 @@ +import { stat } from "fs/promises" +import type { Stats } from "fs" + +/** + * Version token for the compare-and-swap write guard (upstream epic #1375, phase A1). + * + * A token is a pure function of a file's on-disk state, derived from a single + * `fs.stat`, so every process that observes the same file state (a second VS Code + * window, the CLI, the user's own editor tooling) computes the same token. The + * downstream guard phases (A2/A3) compare the token observed at read time with the + * token recomputed just before a write to detect "the file changed since the read" + * (stale) or "the file was replaced by a different file" (dev/ino change). + * + * Format: `dev:ino:size:mtimeNs:ctimeNs` + * + * Resolution note: Node exposes modification/change times as float milliseconds, + * so the ns fields are derived as `Math.round(mtimeMs * 1e6)`. The integer-to-double + * conversion is correctly rounded, so the derivation is deterministic across + * processes, but it is quantized by double precision (~256 ns at the current epoch). + * Two file states whose timestamps differ by less than the quantum derive the same + * ns field; in practice distinct states differ by at least the OS clock resolution + * (and no write workload produces mtimes closer than that), so the guard contract + * holds: same disk state → same token; changed state → a different token in all + * realistic cases. dev, ino and size are exact integers, so any size or file + * identity change is always detected regardless of the timestamp quantum. + */ + +/** Derive an ns-scale field from Node's float milliseconds (see module docs). */ +function nsFromMs(ms: number): string { + return Math.round(ms * 1e6).toString() +} + +/** + * Build the version token from an already-fetched `Stats` — no I/O. + * + * Exported separately from {@link computeVersionToken} so tests can pin the exact + * format against synthetic stats. + */ +export function versionTokenOfStat(stats: Stats): string { + return [ + stats.dev.toString(), + stats.ino.toString(), + stats.size.toString(), + nsFromMs(stats.mtimeMs), + nsFromMs(stats.ctimeMs), + ].join(":") +} + +/** + * Compute the version token for a file (one `fs.stat`). + * + * Rejects with the underlying ENOENT (or equivalent) error when the file is absent; + * how an unobservable target is treated is decided by the guard layer (A3). + */ +export async function computeVersionToken(filePath: string): Promise { + return versionTokenOfStat(await stat(filePath)) +} From 13188d2e1e4b0f090b8c7faa76ee775c9157120b Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 27 Aug 2026 11:03:05 +0800 Subject: [PATCH 2/5] docs(file-safety): correct ino precision bounds in version token (A1, #1375) Review finding: 'ino is an exact integer' was overstated. Node exposes ino as a float64 number: exact for small POSIX inode numbers, but on modern Windows the file ID exceeds 2^53 so Node's own value is already rounded (verified on node v25: non-zero ino, isSafeInteger=false). It remains deterministic per file (same file -> same token), so the token contract is unchanged; change detection rests on exact dev/size plus the mtime/ctime ns fields. Document the bound instead of claiming exactness. --- src/utils/versionToken.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/utils/versionToken.ts b/src/utils/versionToken.ts index ee8e9e82f1..59a8b348fe 100644 --- a/src/utils/versionToken.ts +++ b/src/utils/versionToken.ts @@ -21,8 +21,15 @@ import type { Stats } from "fs" * ns field; in practice distinct states differ by at least the OS clock resolution * (and no write workload produces mtimes closer than that), so the guard contract * holds: same disk state → same token; changed state → a different token in all - * realistic cases. dev, ino and size are exact integers, so any size or file - * identity change is always detected regardless of the timestamp quantum. + * realistic cases. `dev` and `size` are exact integers. `ino` is Node's + * `number` (float64): exact for small POSIX inode numbers, but on modern Windows + * the underlying file ID exceeds 2^53, so Node's own value is already rounded — + * still deterministic per file (same file → same token), but not guaranteed + * injective across distinct files. Change detection therefore rests on size + + * mtime/ctime: any size change is always detected regardless of the timestamp + * quantum, and a replacement whose size and timestamps are indistinguishable is + * undetectable by any scheme reading the same Stats — the detect-and-reread + * stance (no lockfile) accepts that. */ /** Derive an ns-scale field from Node's float milliseconds (see module docs). */ From 2c1582bd9ea89521552e080eb1fc74e8e5189160 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 27 Aug 2026 11:27:00 +0800 Subject: [PATCH 3/5] fix(file-safety): derive the version token from exact BigInt stats (A1, #1375) CodeRabbit finding on this PR: the default numeric fs.stat() loses precision (values above 2^53 are rounded, including Windows file IDs) and the ms->ns derivation introduced a double-precision quantum. Fixed by fetching the stat with { bigint: true }: all five token fields (dev, ino, size, mtimeNs, ctimeNs) are exact BigInt values rendered as decimal strings, with no float anywhere. The sub-ms test now asserts an exact 1_000 ns delta instead of bounded drift, and a regression test pins a size of 10^16+1 (> Number.MAX_SAFE_INTEGER). --- src/utils/__tests__/versionToken.spec.ts | 82 ++++++++++++------------ src/utils/versionToken.ts | 62 +++++++----------- 2 files changed, 65 insertions(+), 79 deletions(-) diff --git a/src/utils/__tests__/versionToken.spec.ts b/src/utils/__tests__/versionToken.spec.ts index 56ce269244..3e2ca26b5c 100644 --- a/src/utils/__tests__/versionToken.spec.ts +++ b/src/utils/__tests__/versionToken.spec.ts @@ -1,25 +1,33 @@ import * as fs from "fs/promises" import * as os from "os" import * as path from "path" -import type { Stats } from "fs" +import type { BigIntStats } from "fs" import { afterEach, beforeEach, describe, expect, it } from "vitest" import { computeVersionToken, versionTokenOfStat } from "../versionToken" -// Stats is a class-backed interface without a public constructor, so a plain-object -// test double is the only practical way to pin the token format without real files. -// Last-resort double assertion (test-local, per AGENTS.md). -function makeStats(overrides: Partial = {}): Stats { - const base: Partial = { - dev: 7, - ino: 4242, - size: 1234, - atimeMs: 1_700_000_000_000, - mtimeMs: 1_700_000_000_123.456, - ctimeMs: 1_700_000_000_789.999, - birthtimeMs: 1_700_000_000_000, +// BigIntStats is a class-backed interface without a public constructor, so a +// plain-object test double is the only practical way to pin the token format +// without real files. Last-resort double assertion (test-local, per AGENTS.md). +function makeStats(overrides: Partial = {}): BigIntStats { + // This repo's @types/node models every StatsBase field (including the *Ms + // fields) as the parameter type T, so all values here are bigint literals; + // the token only reads the *Ns fields. Single-step downcast from Partial to + // the full type (BigIntStats has no public constructor). + const base: Partial = { + dev: 7n, + ino: 4242n, + size: 1234n, + atimeMs: 1_700_000_000_000n, + mtimeMs: 1_700_000_000_123n, + ctimeMs: 1_700_000_000_789n, + birthtimeMs: 1_700_000_000_000n, + atimeNs: 1_700_000_000_000_000_000n, + mtimeNs: 1_700_000_000_123_456_789n, + ctimeNs: 1_700_000_000_789_999_999n, + birthtimeNs: 1_700_000_000_000_000_000n, } - return { ...base, ...overrides } as unknown as Stats + return { ...base, ...overrides } as BigIntStats } describe("versionTokenOfStat (A1, epic #1375)", () => { @@ -27,44 +35,38 @@ describe("versionTokenOfStat (A1, epic #1375)", () => { expect(versionTokenOfStat(makeStats())).toBe(versionTokenOfStat(makeStats())) }) - it("matches the documented dev:ino:size:mtimeNs:ctimeNs format", () => { - const expected = [ - "7", - "4242", - "1234", - Math.round(1_700_000_000_123.456 * 1e6).toString(), - Math.round(1_700_000_000_789.999 * 1e6).toString(), - ].join(":") - expect(versionTokenOfStat(makeStats())).toBe(expected) + it("matches the documented dev:ino:size:mtimeNs:ctimeNs format with exact decimal fields", () => { + expect(versionTokenOfStat(makeStats())).toBe("7:4242:1234:1700000000123456789:1700000000789999999") }) it("distinguishes size changes at identical timestamps", () => { - expect(versionTokenOfStat(makeStats({ size: 1235 }))).not.toBe(versionTokenOfStat(makeStats())) + expect(versionTokenOfStat(makeStats({ size: 1235n }))).not.toBe(versionTokenOfStat(makeStats())) }) - it("distinguishes mtime changes at identical size", () => { - expect(versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_124 }))).not.toBe(versionTokenOfStat(makeStats())) + it("distinguishes a one-nanosecond mtime change", () => { + expect(versionTokenOfStat(makeStats({ mtimeNs: 1_700_000_000_123_456_790n }))).not.toBe( + versionTokenOfStat(makeStats()), + ) }) it("distinguishes a replaced file (dev/ino change) with identical content state", () => { - const replaced = makeStats({ dev: 8, ino: 999 }) + const replaced = makeStats({ dev: 8n, ino: 999n }) expect(versionTokenOfStat(replaced)).not.toBe(versionTokenOfStat(makeStats())) }) - it("preserves sub-ms mtime resolution in the ns field", () => { - const wholeMs = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123 })) - const halfMsLater = versionTokenOfStat(makeStats({ mtimeMs: 1_700_000_000_123.5 })) - expect(halfMsLater).not.toBe(wholeMs) - // 0.5 ms = 500_000 ns. The float-derived ns field is quantized (~256 ns at - // this epoch), so allow a bounded drift instead of asserting an exact value. - const diff = Number(halfMsLater.split(":")[3]) - Number(wholeMs.split(":")[3]) - expect(Math.abs(diff - 500_000)).toBeLessThanOrEqual(512) + it("renders nanosecond resolution exactly (no float quantization)", () => { + const base = versionTokenOfStat(makeStats()) + const plusOneMicrosecond = versionTokenOfStat(makeStats({ mtimeNs: 1_700_000_000_123_457_789n })) + // 1_000 ns apart — the BigInt derivation must keep the delta exact. + const baseNs = BigInt(base.split(":")[3]) + const microNs = BigInt(plusOneMicrosecond.split(":")[3]) + expect(microNs - baseNs).toBe(1_000n) }) - it("handles sizes beyond 32 bits without precision loss", () => { - const size = 5_000_000_000 // > 2^32 + it("handles sizes beyond Number.MAX_SAFE_INTEGER without precision loss", () => { + const size = 10_000_000_000_000_001n // 10^16 + 1 > 2^53 const token = versionTokenOfStat(makeStats({ size })) - expect(token).toContain(`:4242:${size}:`) + expect(token).toBe(`7:4242:${size}:1700000000123456789:1700000000789999999`) }) }) @@ -82,9 +84,9 @@ describe("computeVersionToken (A1, epic #1375)", () => { await fs.rm(tmpDir, { recursive: true, force: true }) }) - it("derives the token from the on-disk state (single stat)", async () => { + it("derives the token from the on-disk state (single bigint stat)", async () => { const token = await computeVersionToken(file) - expect(token).toBe(versionTokenOfStat(await fs.stat(file))) + expect(token).toBe(versionTokenOfStat(await fs.stat(file, { bigint: true }))) }) it("changes when the file content changes", async () => { diff --git a/src/utils/versionToken.ts b/src/utils/versionToken.ts index 59a8b348fe..1738280087 100644 --- a/src/utils/versionToken.ts +++ b/src/utils/versionToken.ts @@ -1,64 +1,48 @@ import { stat } from "fs/promises" -import type { Stats } from "fs" +import type { BigIntStats } from "fs" /** * Version token for the compare-and-swap write guard (upstream epic #1375, phase A1). * * A token is a pure function of a file's on-disk state, derived from a single - * `fs.stat`, so every process that observes the same file state (a second VS Code - * window, the CLI, the user's own editor tooling) computes the same token. The - * downstream guard phases (A2/A3) compare the token observed at read time with the - * token recomputed just before a write to detect "the file changed since the read" - * (stale) or "the file was replaced by a different file" (dev/ino change). + * `fs.stat(path, { bigint: true })`, so every process that observes the same file + * state (a second VS Code window, the CLI, the user's own editor tooling) computes + * the same token. The downstream guard phases (A2/A3) compare the token observed at + * read time with the token recomputed just before a write to detect "the file + * changed since the read" (stale) or "the file was replaced by a different file" + * (dev/ino change). * * Format: `dev:ino:size:mtimeNs:ctimeNs` * - * Resolution note: Node exposes modification/change times as float milliseconds, - * so the ns fields are derived as `Math.round(mtimeMs * 1e6)`. The integer-to-double - * conversion is correctly rounded, so the derivation is deterministic across - * processes, but it is quantized by double precision (~256 ns at the current epoch). - * Two file states whose timestamps differ by less than the quantum derive the same - * ns field; in practice distinct states differ by at least the OS clock resolution - * (and no write workload produces mtimes closer than that), so the guard contract - * holds: same disk state → same token; changed state → a different token in all - * realistic cases. `dev` and `size` are exact integers. `ino` is Node's - * `number` (float64): exact for small POSIX inode numbers, but on modern Windows - * the underlying file ID exceeds 2^53, so Node's own value is already rounded — - * still deterministic per file (same file → same token), but not guaranteed - * injective across distinct files. Change detection therefore rests on size + - * mtime/ctime: any size change is always detected regardless of the timestamp - * quantum, and a replacement whose size and timestamps are indistinguishable is - * undetectable by any scheme reading the same Stats — the detect-and-reread - * stance (no lockfile) accepts that. + * Precision: the stat is fetched in `bigint` mode, so all five fields are exact + * `BigInt` values rendered as decimal strings — no float is involved anywhere. + * There is therefore no precision loss for large sizes or inodes (a Windows file ID + * exceeds 2^53 and is still exact), and the ns timestamps are the kernel's exact + * nanosecond values rather than a ms→ns derivation (no ~256 ns double-precision + * quantum). Guarantee: same disk state → same token, deterministic across + * processes; any change to size, file identity, or mtime/ctime → a different token. + * + * Platform note: on POSIX `ctime` is the last file-status change; on Windows it is + * the file creation time. The token only requires it to move when the file's + * metadata is replaced, which holds on both. */ -/** Derive an ns-scale field from Node's float milliseconds (see module docs). */ -function nsFromMs(ms: number): string { - return Math.round(ms * 1e6).toString() -} - /** - * Build the version token from an already-fetched `Stats` — no I/O. + * Build the version token from an already-fetched `BigIntStats` — no I/O. * * Exported separately from {@link computeVersionToken} so tests can pin the exact * format against synthetic stats. */ -export function versionTokenOfStat(stats: Stats): string { - return [ - stats.dev.toString(), - stats.ino.toString(), - stats.size.toString(), - nsFromMs(stats.mtimeMs), - nsFromMs(stats.ctimeMs), - ].join(":") +export function versionTokenOfStat(stats: BigIntStats): string { + return [stats.dev, stats.ino, stats.size, stats.mtimeNs, stats.ctimeNs].map((value) => value.toString()).join(":") } /** - * Compute the version token for a file (one `fs.stat`). + * Compute the version token for a file (one `fs.stat` in bigint mode). * * Rejects with the underlying ENOENT (or equivalent) error when the file is absent; * how an unobservable target is treated is decided by the guard layer (A3). */ export async function computeVersionToken(filePath: string): Promise { - return versionTokenOfStat(await stat(filePath)) + return versionTokenOfStat(await stat(filePath, { bigint: true })) } From 2965ad18e867285318ae5fa3748ab94ce453782f Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Thu, 27 Aug 2026 14:52:58 +0800 Subject: [PATCH 4/5] feat(task): per-task file observation registry (A2, #1375) --- src/core/task/Task.ts | 2 + .../__tests__/observationRegistry.spec.ts | 72 +++++++++++ src/core/task/observationRegistry.ts | 47 ++++++++ src/core/tools/ReadFileTool.ts | 12 ++ src/core/tools/__tests__/readFileTool.spec.ts | 113 ++++++++++++++++++ 5 files changed, 246 insertions(+) create mode 100644 src/core/task/__tests__/observationRegistry.spec.ts create mode 100644 src/core/task/observationRegistry.ts diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 349d9c51d3..8977b60830 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -103,6 +103,7 @@ import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector" import { restoreTodoListForTask } from "../tools/UpdateTodoListTool" import { FileContextTracker } from "../context-tracking/FileContextTracker" import { RooIgnoreController } from "../ignore/RooIgnoreController" +import { ObservationRegistry } from "./observationRegistry" import { RooProtectedController } from "../protect/RooProtectedController" import { type AssistantMessageContent, presentAssistantMessage } from "../assistant-message" import { NativeToolCallParser } from "../assistant-message/NativeToolCallParser" @@ -181,6 +182,7 @@ export class Task extends EventEmitter implements TaskLike { readonly parentTask: Task | undefined = undefined readonly taskNumber: number readonly workspacePath: string + readonly observationRegistry = new ObservationRegistry() /** * The mode associated with this task. Persisted across sessions diff --git a/src/core/task/__tests__/observationRegistry.spec.ts b/src/core/task/__tests__/observationRegistry.spec.ts new file mode 100644 index 0000000000..51b73aabde --- /dev/null +++ b/src/core/task/__tests__/observationRegistry.spec.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, vi } from "vitest" + +import { ObservationRegistry } from "../observationRegistry" + +describe("ObservationRegistry", () => { + it("observe → get returns the recorded version and observedAt", () => { + const reg = new ObservationRegistry() + reg.observe("/a/b/c.ts", "1:2:300:4000000000:5000000000") + + const obs = reg.get("/a/b/c.ts") + expect(obs).toBeDefined() + expect(obs!.version).toBe("1:2:300:4000000000:5000000000") + expect(typeof obs!.observedAt).toBe("number") + }) + + it("re-observe replaces the entry with a fresh observedAt", () => { + vi.useFakeTimers() + const reg = new ObservationRegistry() + reg.observe("/a/b/c.ts", "v1") + const first = reg.get("/a/b/c.ts")! + expect(first.version).toBe("v1") + + vi.advanceTimersByTime(50) + reg.observe("/a/b/c.ts", "v2") + const second = reg.get("/a/b/c.ts")! + expect(second.version).toBe("v2") + expect(second.observedAt).toBeGreaterThan(first.observedAt) + + vi.useRealTimers() + }) + + it("has returns true for observed paths, false otherwise", () => { + const reg = new ObservationRegistry() + reg.observe("/x.ts", "t1") + expect(reg.has("/x.ts")).toBe(true) + expect(reg.has("/y.ts")).toBe(false) + }) + + it("size reflects the number of observed entries", () => { + const reg = new ObservationRegistry() + expect(reg.size).toBe(0) + reg.observe("/a.ts", "t1") + reg.observe("/b.ts", "t2") + expect(reg.size).toBe(2) + }) + + it("clear removes all entries and resets size to 0", () => { + const reg = new ObservationRegistry() + reg.observe("/a.ts", "t1") + reg.observe("/b.ts", "t2") + reg.clear() + expect(reg.size).toBe(0) + expect(reg.get("/a.ts")).toBeUndefined() + expect(reg.has("/b.ts")).toBe(false) + }) + + it("get on empty registry returns undefined", () => { + const reg = new ObservationRegistry() + expect(reg.get("/any.ts")).toBeUndefined() + }) + + it("separate instances are independent — observing in one does not appear in the other", () => { + const regA = new ObservationRegistry() + const regB = new ObservationRegistry() + regA.observe("/shared.ts", "v1") + expect(regA.get("/shared.ts")).toBeDefined() + expect(regB.get("/shared.ts")).toBeUndefined() + regB.observe("/shared.ts", "v2") + expect(regA.get("/shared.ts")!.version).toBe("v1") + expect(regB.get("/shared.ts")!.version).toBe("v2") + }) +}) diff --git a/src/core/task/observationRegistry.ts b/src/core/task/observationRegistry.ts new file mode 100644 index 0000000000..871f80225b --- /dev/null +++ b/src/core/task/observationRegistry.ts @@ -0,0 +1,47 @@ +/** + * Per-task file observation registry (upstream epic #1375, phase A2). + * + * Each Task owns its own instance so parent and subtask observations are + * independent. The S4 guarded-write will compare these versions against the + * token recomputed pre-write to detect stale reads or file replacement. + * + * Pure in-memory — zero I/O, no dependencies. No behavior change in this PR: + * observations are recorded but not consulted. + */ + +export interface FileObservation { + /** Version token derived from on-disk fs.stat (bigint mode). */ + version: string + /** Millisecond timestamp when the observation was recorded. */ + observedAt: number +} + +export class ObservationRegistry { + private readonly entries = new Map() + + /** + * Record an observation for a file at its absolute path. + * + * Re-observing replaces the entry with a fresh observedAt timestamp and + * the new version token. + */ + observe(absolutePath: string, version: string): void { + this.entries.set(absolutePath, { version, observedAt: Date.now() }) + } + + get(absolutePath: string): FileObservation | undefined { + return this.entries.get(absolutePath) + } + + has(absolutePath: string): boolean { + return this.entries.has(absolutePath) + } + + clear(): void { + this.entries.clear() + } + + get size(): number { + return this.entries.size + } +} diff --git a/src/core/tools/ReadFileTool.ts b/src/core/tools/ReadFileTool.ts index 2107cfe21b..6e222e1309 100644 --- a/src/core/tools/ReadFileTool.ts +++ b/src/core/tools/ReadFileTool.ts @@ -16,6 +16,7 @@ import type { ReadFileParams, ReadFileMode, ReadFileToolParams, FileEntry, LineR import { isLegacyReadFileParams, type ClineSayTool } from "@roo-code/types" import { Task } from "../task/Task" +import { computeVersionToken } from "../../utils/versionToken" import { formatResponse } from "../prompts/responses" import { RecordSource } from "../context-tracking/FileContextTrackerTypes" import { isPathOutsideWorkspace } from "../../utils/pathUtils" @@ -220,6 +221,11 @@ export class ReadFileTool extends BaseTool<"read_file"> { await task.fileContextTracker.trackFileContext(relPath, "read_tool" as RecordSource) + // A2 (plan #33 / epic #1375): record the observed on-disk version for the future write guard. + // A stat failure leaves the target unobserved and never fails the read. + const version = await computeVersionToken(fullPath).catch(() => undefined) + if (version) task.observationRegistry.observe(fullPath, version) + updateFileResult(relPath, { nativeContent: `File: ${relPath}\n${result}`, }) @@ -799,6 +805,12 @@ export class ReadFileTool extends BaseTool<"read_file"> { // Track file in context await task.fileContextTracker.trackFileContext(relPath, "read_tool") + + // A2 (plan #33 / epic #1375): mirror the native path — record the observed + // on-disk version so legacy-format reads also feed the future write guard. + // A stat failure leaves the target unobserved and never fails the read. + const version = await computeVersionToken(fullPath).catch(() => undefined) + if (version) task.observationRegistry.observe(fullPath, version) } catch (error) { const errorMsg = error instanceof Error ? error.message : String(error) results.push(`File: ${relPath}\nError: ${errorMsg}`) diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts index 6c9e177d38..6108e78151 100644 --- a/src/core/tools/__tests__/readFileTool.spec.ts +++ b/src/core/tools/__tests__/readFileTool.spec.ts @@ -13,10 +13,16 @@ */ import path from "path" +import type { Stats } from "fs" + +import type { LegacyReadFileParams } from "@roo-code/types" import { isBinaryFile } from "isbinaryfile" import { readFileTool, ReadFileTool } from "../ReadFileTool" +import type { Task } from "../../task/Task" +import { ObservationRegistry } from "../../task/observationRegistry" +import { computeVersionToken } from "../../../utils/versionToken" import { formatResponse } from "../../prompts/responses" import { validateImageForProcessing, @@ -136,6 +142,7 @@ interface MockTaskOptions { rooIgnoreAllowed?: boolean maxImageFileSize?: number maxTotalImageSize?: number + observationRegistry?: ObservationRegistry } function createMockTask(options: MockTaskOptions = {}) { @@ -143,6 +150,9 @@ function createMockTask(options: MockTaskOptions = {}) { return { cwd: "/test/workspace", + // Mirror Task: every task always owns an observation registry (A2, #1375). + // Tests asserting on observations pass their own instance via options. + observationRegistry: options.observationRegistry ?? new ObservationRegistry(), api: { getModel: vi.fn().mockReturnValue({ info: { supportsImages }, @@ -1489,5 +1499,108 @@ describe("ReadFileTool", () => { expect(mockTask.didToolFailInCurrentTurn).toBe(true) }) + + describe("observation registry", () => { + it("records an observation on successful read of an existing file", async () => { + const mockTask = createMockTask({ + observationRegistry: new ObservationRegistry(), + }) + const callbacks = createMockCallbacks() + + // Override the beforeEach default stat mock with proper BigIntStats. + mockedFsStat.mockResolvedValue({ + isDirectory: () => false, + dev: BigInt(1), + ino: BigInt(2), + size: BigInt(300), + mtimeNs: BigInt(4_000_000_000n), + ctimeNs: BigInt(5_000_000_000n), + // Cast: the mock only implements the members the tool and versionToken read. + } as unknown as Stats) + mockedIsBinaryFile.mockResolvedValue(false) + + // Spy on observe to capture the exact key used (Windows path.resolve may use backslashes). + const reg = mockTask.observationRegistry! + const observeSpy = vi.spyOn(reg, "observe") + + // Cast: the mock task only implements the members ReadFileTool.execute touches. + await readFileTool.execute({ path: "existing.ts" }, mockTask as unknown as Task, callbacks) + + // Verify the tool called observe exactly once with a valid token. + expect(observeSpy).toHaveBeenCalledTimes(1) + const [calledPath, calledVersion] = observeSpy.mock.calls[0] + expect(calledPath).toContain("existing.ts") + expect(calledVersion).toMatch(/^\d+:\d+:\d+:\d+:\d+$/) + + // Verify get() returns the same data using the spy-captured key. + const obs = reg.get(calledPath) + expect(obs).toBeDefined() + expect(obs!.version).toBe(calledVersion) + }) + + it("a failed read (absent path) leaves the registry size 0 and does not throw", async () => { + const mockTask = createMockTask({ + observationRegistry: new ObservationRegistry(), + }) + const callbacks = createMockCallbacks() + + mockedFsReadFile.mockRejectedValue(new Error("ENOENT")) + + // Cast: the mock task only implements the members ReadFileTool.execute touches. + await readFileTool.execute({ path: "missing.ts" }, mockTask as unknown as Task, callbacks) + + // observationRegistry is guaranteed present because we passed it in createMockTask. + const reg = mockTask.observationRegistry + expect(reg).toBeDefined() + expect(reg!.size).toBe(0) + }) + + it("records an observation for legacy-format reads of existing files", async () => { + const mockTask = createMockTask({ + observationRegistry: new ObservationRegistry(), + }) + const callbacks = createMockCallbacks() + + mockedFsStat.mockResolvedValue({ + isDirectory: () => false, + dev: BigInt(1), + ino: BigInt(2), + size: BigInt(300), + mtimeNs: BigInt(4_000_000_000n), + ctimeNs: BigInt(5_000_000_000n), + // Cast: the mock only implements the members the tool and versionToken read. + } as unknown as Stats) + mockedIsBinaryFile.mockResolvedValue(false) + + const reg = mockTask.observationRegistry! + const observeSpy = vi.spyOn(reg, "observe") + + // Typed legacy (pre-refactor) params: the multi-file format with the + // _legacyFormat discriminant (see LegacyReadFileParams). + const legacyParams: LegacyReadFileParams = { + files: [{ path: "legacy.ts" }], + _legacyFormat: true, + } + + // Cast: the mock task only implements the members ReadFileTool.execute touches. + await readFileTool.execute(legacyParams, mockTask as unknown as Task, callbacks) + + expect(observeSpy).toHaveBeenCalledTimes(1) + const [calledPath, calledVersion] = observeSpy.mock.calls[0] + expect(calledPath).toContain("legacy.ts") + expect(calledVersion).toMatch(/^\d+:\d+:\d+:\d+:\d+$/) + }) + + it("two separate Task-owned registries are independent", async () => { + const regA = new ObservationRegistry() + const regB = new ObservationRegistry() + regA.observe("/shared.ts", "v1") + expect(regA.get("/shared.ts")!.version).toBe("v1") + expect(regB.get("/shared.ts")).toBeUndefined() + regB.observe("/shared.ts", "v2") + expect(regA.get("/shared.ts")!.version).toBe("v1") + expect(regB.get("/shared.ts")!.version).toBe("v2") + }) + }) }) }) From a00eef8e0917856e9d790ff40cecb45a5bd828bc Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Sun, 30 Aug 2026 12:33:13 +0800 Subject: [PATCH 5/5] =?UTF-8?q?chore(ci):=20empty=20commit=20=E2=80=94=20r?= =?UTF-8?q?e-trigger=20CI=20and=20the=20CodeRabbit=20current-head=20review?= =?UTF-8?q?=20gate=20(no=20code=20change)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit