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 @@ -112,6 +112,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 @@ -292,6 +293,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
91 changes: 91 additions & 0 deletions src/core/task/__tests__/observationRegistry.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
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()
Comment thread
easonLiangWorldedtech marked this conversation as resolved.
try {
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)
} finally {
// Teardown in finally so a failing assertion cannot leak fake
// timers or the mocked Date into subsequent tests.
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("forget drops a warm entry so a later read cannot be guarded by a stale token", () => {
const reg = new ObservationRegistry()
reg.observe("/x.ts", "t1")

// The warm case: an entry from an earlier read must not survive a read that could not
// compute a token for the current on-disk version.
expect(reg.forget("/x.ts")).toBe(true)
expect(reg.has("/x.ts")).toBe(false)
expect(reg.get("/x.ts")).toBeUndefined()

// The cold case: forgetting an unobserved path is a no-op, not an error.
expect(reg.forget("/y.ts")).toBe(false)
expect(reg.size).toBe(0)
})

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")
})
})
56 changes: 56 additions & 0 deletions src/core/task/observationRegistry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/**
* 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() })
}

/**
* Drop the observation for a file. A read that cannot compute a version token must not
* leave an earlier entry behind, because the write guard would then compare against a
* token that no longer describes the last read.
*/
forget(absolutePath: string): boolean {
return this.entries.delete(absolutePath)
}

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
}
}
23 changes: 23 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 { 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,17 @@

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. It must also
// drop any earlier entry, otherwise the guard still holds a token from a previous
// read that no longer describes this one.
const version = await computeVersionToken(fullPath).catch(() => undefined)
if (version) {
task.observationRegistry.observe(fullPath, version)
} else {
task.observationRegistry.forget(fullPath)

Check warning on line 232 in src/core/tools/ReadFileTool.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/tools/ReadFileTool.ts:232: Survived CallExpression mutant (replacement: ;). See the job summary for the complete list and resolution guidance.
}

updateFileResult(relPath, {
nativeContent: `File: ${relPath}\n${result}`,
})
Expand Down Expand Up @@ -799,6 +811,17 @@

// 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, and drops any
// entry from an earlier read so the guard cannot compare against a stale token.
const version = await computeVersionToken(fullPath).catch(() => undefined)
if (version) {
task.observationRegistry.observe(fullPath, version)
} else {
task.observationRegistry.forget(fullPath)

Check warning on line 823 in src/core/tools/ReadFileTool.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/tools/ReadFileTool.ts:823: Survived CallExpression mutant (replacement: ;). See the job summary for the complete list and resolution guidance.
}
} catch (error) {
const errorMsg = error instanceof Error ? error.message : String(error)
results.push(`File: ${relPath}\nError: ${errorMsg}`)
Expand Down
Loading
Loading