Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
426f809
split unit U1 of PR 1833 (issue 1375)
Oct 5, 2026
4528075
fix(file-safety): close the pre-merge findings on the publish primiti…
Oct 5, 2026
9207cee
fix(file-safety): propagate a non-ENOENT lstat failure in resolvePubl…
Oct 5, 2026
5d02948
fix(file-safety): keep the rollback pair typed and the mock stand-ins…
Oct 5, 2026
1e7c390
rebuild unit u2 on the fixed chain
Oct 5, 2026
86ae014
chore(lint): prune the safeWriteJson suppression this unit earns
Oct 5, 2026
cbe99cc
rebuild unit u3 on the fixed chain
Oct 5, 2026
7559543
fix(task): declare the observation registry on Task in this unit
Oct 5, 2026
0fcf305
rebuild unit u4 on the fixed chain
Oct 5, 2026
7e0533e
rebuild unit u5 on the fixed chain
Oct 5, 2026
2f42d5c
rebuild unit U8 on U5 (corrected merge order)
Oct 5, 2026
47dc492
rebuild unit u6 in the corrected order
Oct 5, 2026
b5a213a
fix(apply-patch): a full hunk read is a complete observation
Oct 5, 2026
cf751fb
rebuild unit u7 on the corrected u6
Oct 5, 2026
38d99ed
rebuild unit U7 on the corrected U6
Oct 5, 2026
6a46b6b
fix(task): keep main's abort re-check in ask
Oct 5, 2026
9adf66f
fix(file-safety): inherit unit 1 committed guard
Oct 5, 2026
8647b7d
fix(file-safety): keep the publish error message in RollbackFailureError
Oct 5, 2026
3ba741a
test(file-safety): assert the exact staging directory removed after a…
Oct 5, 2026
7ad01ae
test: re-run mocked e2e and the ubuntu lane - no source change since …
Oct 5, 2026
14ec1fb
fix(file-safety): give the Windows DACL dump a per-write name
Oct 5, 2026
b27f4c8
fix(tools): carry U1's hunk-read completeness rule into this unit
Oct 6, 2026
46c1cfb
fix(tools): bring U7 onto the current file-safety core
Oct 7, 2026
ba10fd4
chore: trigger a fresh review pass at this head
Oct 7, 2026
1b2f187
fix(integrations): preview observation must not authorize a save; asy…
Oct 7, 2026
f4f917f
fix(tools): ApplyDiffTool must observe the version its diff was built on
Oct 7, 2026
fd1c719
test(tools): type the apply_diff observation doubles for check-types
Oct 7, 2026
d6446b5
fix(integrations): never adopt an autosaved match for an unauthorized…
Oct 7, 2026
d2d9e9d
fix(tools): never refresh a model observation the tool read on a diff…
Oct 7, 2026
160ddb5
fix(safety): confine before the lock, keep the post-commit backup
Oct 8, 2026
0f8c6c0
fix(safety): confine before mkdir, report the retained backup, stop p…
Oct 8, 2026
483ed65
fix(tools): honor an approval for a write outside the task root
Oct 8, 2026
8fc2c5b
fix(u7): no containment decision without a canonical workspace, and n…
Oct 8, 2026
183de84
fix(file-safety): one lock key whether the parent directory exists yet
Oct 8, 2026
e06bed1
test(file-safety): a backup copy that fails part-way must not survive…
Oct 8, 2026
3187e58
test(tools): pin which bracketing stat fails in the diff-read observa…
Oct 8, 2026
3183810
fix(diff-view): put the save's post-publish cleanup inside the same t…
Oct 8, 2026
c8caff0
fix(diff-view): let only the teardown that started a pass finalize th…
Oct 9, 2026
c434b0b
fix(diff-view): restore preview tabs for the pass that owns a rejecte…
Oct 9, 2026
9e59f01
fix(tools): pass the outside-workspace approval flag in its own position
Oct 9, 2026
cd5bfdc
fix(guardedWrite): one unresolvable workspace folder must not veto ev…
Oct 9, 2026
6b68a57
fix(guardedWrite): bind an approved outside-workspace write to the id…
Oct 9, 2026
510effe
fix(diff-view): keep the teardown guard held through the finalization…
Oct 9, 2026
376a7b9
fix(write-to-file): resolve the approved identity inside the tool's e…
Oct 9, 2026
d63ba73
test(tools): keep the outside-workspace flag reset on the failure path
Oct 9, 2026
800916e
fix(diff-view): finalize the session when a cancellation lands during…
Oct 9, 2026
13fa14b
fix(diff-view): one caller owns the session's finalization, by claim …
Oct 9, 2026
0b0c15b
fix(test): clear the merge artifacts on the U9 branch
Oct 9, 2026
da2e0ef
test(safety,tools) and refactor(apply-patch): make four tests prove w…
Oct 10, 2026
6d8f077
style: format the fifteen files the formatting gate flags on this PR
Oct 10, 2026
bc43841
fix(u7): align the unit with main's lint re-enable and format gate; d…
easonLiangWorldedtech Oct 14, 2026
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
5 changes: 5 additions & 0 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import { buildNativeToolsArrayWithRestrictions } from "./build-tools"
import { ToolRepetitionDetector } from "../tools/ToolRepetitionDetector"
import { restoreTodoListForTask } from "../tools/UpdateTodoListTool"
import { FileContextTracker } from "../context-tracking/FileContextTracker"
import { ObservationRegistry } from "./observationRegistry"
import { RooIgnoreController } from "../ignore/RooIgnoreController"
import { RooProtectedController } from "../protect/RooProtectedController"
import { type AssistantMessageContent, presentAssistantMessage } from "../assistant-message"
Expand Down Expand Up @@ -282,6 +283,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
readonly instanceId: string
readonly metadata: TaskMetadata

// The observed on-disk version of each file this task has read. Declared here so the
// read tools can record it; a write guard later compares a token against this registry.
readonly observationRegistry = new ObservationRegistry()

todoList?: TodoItem[]

readonly rootTask: Task | undefined = undefined
Expand Down
108 changes: 108 additions & 0 deletions src/core/task/__tests__/observationRegistry.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
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")
})

describe("completeness scope (S4b follow-up #46)", () => {
it("defaults to a complete observation when the read scope is not given", () => {
const reg = new ObservationRegistry()
reg.observe("/a/b/c.ts", "v1")

expect(reg.get("/a/b/c.ts")!.complete).toBe(true)
})

it("records a partial observation when the read only returned a view of the file", () => {
const reg = new ObservationRegistry()
reg.observe("/a/b/c.ts", "v1", false)

expect(reg.get("/a/b/c.ts")!.complete).toBe(false)
})

it("re-observing replaces the entry's completeness with the new read's scope", () => {
const reg = new ObservationRegistry()
reg.observe("/a/b/c.ts", "v1", false)
reg.observe("/a/b/c.ts", "v2")

const obs = reg.get("/a/b/c.ts")!
expect(obs.version).toBe("v2")
expect(obs.complete).toBe(true)
})

it("re-observing with a partial scope downgrades a previously complete entry", () => {
const reg = new ObservationRegistry()
reg.observe("/a/b/c.ts", "v1")
reg.observe("/a/b/c.ts", "v2", false)

const obs = reg.get("/a/b/c.ts")!
expect(obs.version).toBe("v2")
expect(obs.complete).toBe(false)
})
})
})
69 changes: 69 additions & 0 deletions src/core/task/observationRegistry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* 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. The S4 guarded-write consults
* these observations for the version check and for the completeness check that
* gates a full-file replacement.
*/

export interface FileObservation {
/** Version token derived from on-disk fs.stat (bigint mode). */
version: string
/** Millisecond timestamp when the observation was recorded. */
observedAt: number
/**
* Whether the read that produced this observation returned the complete
* file. A slice, line-range, truncated, or indentation-block read returns
* only a view of the file; such an observation authorizes targeted edits
* on the view the model saw, but never a full-file replacement.
*/
complete: boolean
}

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, the
* new version token, and the read's completeness. `complete` defaults to
* true for callers that read the whole file themselves (spec doubles,
* WriteToFileTool). A caller whose read is internal to a targeted edit must
* carry the model's prior completeness instead, so the tool's own read cannot
* upgrade a partial read into authority for a full-file replacement.
*/
observe(absolutePath: string, version: string, complete: boolean = true): void {
this.entries.set(absolutePath, { version, observedAt: Date.now(), complete })
}

get(absolutePath: string): FileObservation | undefined {
return this.entries.get(absolutePath)
}

has(absolutePath: string): boolean {
return this.entries.has(absolutePath)
}

/**
* Drop the observation for one path. A caller that must revoke an
* authorization it did not earn - a preview that observed a version the model
* never read - needs this instead of clear(), which would also discard the
* observations other reads of the same task still rely on.
*/
forget(absolutePath: string): boolean {
return this.entries.delete(absolutePath)
}

clear(): void {
this.entries.clear()
}

get size(): number {
return this.entries.size
}
}
32 changes: 30 additions & 2 deletions src/core/tools/ApplyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"

import { getReadablePath } from "../../utils/path"
import { versionTokenOfStat } from "../../utils/versionToken"
import { Task } from "../task/Task"
import { formatResponse } from "../prompts/responses"
import { fileExistsAtPath } from "../../utils/fs"
Expand Down Expand Up @@ -68,7 +69,32 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
return
}

// The diff below is built from this exact read, so the save that follows must be
// authorized against the version captured here - not against whatever version the
// preview happens to stat afterwards. Same contract as ApplyPatchTool's hunk read:
// stat around the read and observe only when the file did not change underneath it.
const preReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
const originalContent: string = await fs.readFile(absolutePath, "utf-8")
const postReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
if (preReadStats && postReadStats) {
const preReadToken = versionTokenOfStat(preReadStats)
if (preReadToken === versionTokenOfStat(postReadStats)) {
// A tool read is not a model read. With no prior observation this stays a
// partial observation of the version the diff was computed against - the only
// authorization the save can have, since apply_diff computes its hunks from
// this read. When the model already observed the file, keep the completeness it
// earned, but only on the version it was earned on: refreshing an OLDER
// observation to the current version would let content the model built from a
// stale read pass the compare-and-swap, so an out-of-date observation is left
// alone and the save fails with the re-read remediation.
const prior = task.observationRegistry.get(absolutePath)
if (prior === undefined) {
task.observationRegistry.observe(absolutePath, preReadToken, false)
} else if (prior.version === preReadToken) {
task.observationRegistry.observe(absolutePath, preReadToken, prior.complete === true)
}
}
}

// Apply the diff to the original content
const diffResult = (await task.diffStrategy?.applyDiff(
Expand Down Expand Up @@ -173,7 +199,8 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
return
}

// Save directly without showing diff view or opening the file
// Save directly without showing diff view or opening the file. The diff is
// applied to an existing file, so edit-guard semantics require a prior read.
task.diffViewProvider.editType = "modify"
task.diffViewProvider.originalContent = originalContent
await task.diffViewProvider.saveDirectly(
Expand All @@ -182,6 +209,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
false,
diagnosticsEnabled,
writeDelayMs,
"edit",
)
} else {
// Original behavior with diff view
Expand Down Expand Up @@ -221,7 +249,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
}

// Call saveChanges to update the DiffViewProvider properties
await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs, "edit")
}

// Track file edit operation
Expand Down
Loading
Loading