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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -181,6 +182,7 @@ export class Task extends EventEmitter<TaskEvents> 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
Expand Down
72 changes: 72 additions & 0 deletions src/core/task/__tests__/observationRegistry.spec.ts
Original file line number Diff line number Diff line change
@@ -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")
})
})
47 changes: 47 additions & 0 deletions src/core/task/observationRegistry.ts
Original file line number Diff line number Diff line change
@@ -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<string, FileObservation>()

/**
* 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
}
}
12 changes: 12 additions & 0 deletions src/core/tools/ReadFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

updateFileResult(relPath, {
nativeContent: `File: ${relPath}\n${result}`,
})
Expand Down Expand Up @@ -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}`)
Expand Down
113 changes: 113 additions & 0 deletions src/core/tools/__tests__/readFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -136,13 +142,17 @@ interface MockTaskOptions {
rooIgnoreAllowed?: boolean
maxImageFileSize?: number
maxTotalImageSize?: number
observationRegistry?: ObservationRegistry
}

function createMockTask(options: MockTaskOptions = {}) {
const { supportsImages = false, rooIgnoreAllowed = true, maxImageFileSize = 5, maxTotalImageSize = 20 } = options

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 },
Expand Down Expand Up @@ -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")
})
})
})
})
Loading
Loading