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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
}
}
34 changes: 34 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 { versionTokenOfStat } from "../../utils/versionToken"
import { formatResponse } from "../prompts/responses"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
Expand Down Expand Up @@ -214,12 +215,29 @@ export class ReadFileTool extends BaseTool<"read_file"> {
// Read text file content with lossy UTF-8 conversion
// Reading as Buffer first allows graceful handling of non-UTF8 bytes
// (they become U+FFFD replacement characters instead of throwing)
// A2 (epic #1375): capture the on-disk token before the read so a mutation
// landing mid-read is detected by the post-read stat below.
const preReadStats = await fs.stat(fullPath, { bigint: true }).catch(() => undefined)
const buffer = await fs.readFile(fullPath)
const fileContent = buffer.toString("utf-8")
const result = this.processTextFile(fileContent, entry)

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.
// The token is captured before AND after the read; the target is observed only
// when both match — a mutation between the two stats means the content the model
// received is not the on-disk state, and observing it would let a later write
// match a token the model never saw. A stat failure leaves the target
// unobserved and never fails the read.
const postReadStats = await fs.stat(fullPath, { bigint: true }).catch(() => undefined)
if (preReadStats && postReadStats) {
const preReadToken = versionTokenOfStat(preReadStats)
if (preReadToken === versionTokenOfStat(postReadStats)) {
task.observationRegistry.observe(fullPath, preReadToken)
}
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
updateFileResult(relPath, {
nativeContent: `File: ${relPath}\n${result}`,
})
Expand Down Expand Up @@ -768,6 +786,9 @@ export class ReadFileTool extends BaseTool<"read_file"> {
}

// Read text file
// A2 (epic #1375): capture the on-disk token before the read so a mutation
// landing mid-read is detected by the post-read stat below.
const preReadStats = await fs.stat(fullPath, { bigint: true }).catch(() => undefined)
const rawContent = await fs.readFile(fullPath, "utf8")

// Handle line ranges if specified
Expand Down Expand Up @@ -799,6 +820,19 @@ 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.
// Observe only when the pre-read and post-read tokens match (a mutation between
// them means the returned content is not the on-disk state). A stat failure
// leaves the target unobserved and never fails the read.
const postReadStats = await fs.stat(fullPath, { bigint: true }).catch(() => undefined)
if (preReadStats && postReadStats) {
const preReadToken = versionTokenOfStat(preReadStats)
if (preReadToken === versionTokenOfStat(postReadStats)) {
task.observationRegistry.observe(fullPath, preReadToken)
}
}
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error)
results.push(`File: ${relPath}\nError: ${errorMsg}`)
Expand Down
Loading
Loading