feat(tools): wire guarded writes into the diff-view save paths (S4b, #1375)

src/core/task/Task.ts

@@ -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"
@@ -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

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")
+	})
+})

src/core/task/observationRegistry.ts

@@ -0,0 +1,49 @@
+/**
+ * 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 observations ARE consulted:
+ * guardedWrite reads this registry before publishing (src/core/tools/guardedWrite.ts)
+ * and compares the recorded version token against the token recomputed from disk, so
+ * a stale read or an out-of-band replacement is rejected instead of published over.
+ */
+
+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
+	}
+}

src/core/tools/ApplyDiffTool.ts

@@ -8,6 +8,7 @@ import { getReadablePath } from "../../utils/path"
 import { Task } from "../task/Task"
 import { formatResponse } from "../prompts/responses"
 import { fileExistsAtPath } from "../../utils/fs"
+import { versionTokenOfStat } from "../../utils/versionToken"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
 import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
@@ -68,7 +69,22 @@ 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
+			// diff view happens to stat afterwards (there is no diff view on the
+			// focus-disruption path). Same contract as ApplyPatchTool's hunk read: stat
+			// around the read and observe only when the file did not change underneath it.
+			// Without it the saveDirectly("edit") below has no observation for the path and the
+			// guarded publish fails with "File not read yet -- read the file, then retry."
+			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)) {
+					task.observationRegistry.observe(absolutePath, preReadToken)
+				}
+			}
 
 			// Apply the diff to the original content
 			const diffResult = (await task.diffStrategy?.applyDiff(
@@ -173,7 +189,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(
@@ -182,6 +199,7 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
 					false,
 					diagnosticsEnabled,
 					writeDelayMs,
+					"edit",
 				)
 			} else {
 				// Original behavior with diff view

src/core/tools/ApplyPatchTool.ts

@@ -11,6 +11,7 @@ import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { fileExistsAtPath } from "../../utils/fs"
 import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
 import { sanitizeUnifiedDiff, computeDiffStats } from "../diff/stats"
+import { versionTokenOfStat } from "../../utils/versionToken"
 import { BaseTool, ToolCallbacks } from "./BaseTool"
 import type { ToolUse } from "../../shared/tools"
 import { parsePatch, ParseError, processAllHunks } from "./apply-patch"
@@ -85,10 +86,25 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
 				return
 			}
 
-			// Process each hunk
+			// Process each hunk. The read doubles as the S2 observation for the
+			// guarded publish (ReadFileTool contract: stat before and after the
+			// read, observe only when the on-disk version is unchanged between the
+			// two stats). Without it, the in-place modify publish is an unobserved
+			// write and the composed chat-diff default rejects it ("File already
+			// exists ... and was not read before this write") even though this tool
+			// just read the exact content the patch was applied to.
 			const readFile = async (filePath: string): Promise<string> => {
 				const absolutePath = path.resolve(task.cwd, filePath)
-				return await fs.readFile(absolutePath, "utf8")
+				const preReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
+				const content: string = await fs.readFile(absolutePath, "utf8")
+				const postReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
+				if (preReadStats && postReadStats) {
+					const preReadToken = versionTokenOfStat(preReadStats)
+					if (preReadToken === versionTokenOfStat(postReadStats)) {
+						task.observationRegistry.observe(absolutePath, preReadToken)
+					}
+				}
+				return content
 			}
 
 			let changes: ApplyPatchFileChange[]
@@ -214,7 +230,16 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
 
 		// Save the changes
 		if (isPreventFocusDisruptionEnabled) {
-			await task.diffViewProvider.saveDirectly(relPath, newContent, true, diagnosticsEnabled, writeDelayMs)
+			// Guarded publish: the patch supplies the complete new content, so create-guard
+			// semantics apply (an unobserved existing target is rejected, not overwritten).
+			await task.diffViewProvider.saveDirectly(
+				relPath,
+				newContent,
+				true,
+				diagnosticsEnabled,
+				writeDelayMs,
+				"create",
+			)
 		} else {
 			await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
 		}
@@ -408,12 +433,14 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
 
 			// Save new content to the new path
 			if (isPreventFocusDisruptionEnabled) {
+				// The move destination is published with the complete new content.
 				await task.diffViewProvider.saveDirectly(
 					change.movePath,
 					newContent,
 					false,
 					diagnosticsEnabled,
 					writeDelayMs,
+					"create",
 				)
 			} else {
 				// Write to new path and delete old file
@@ -433,7 +460,16 @@ export class ApplyPatchTool extends BaseTool<"apply_patch"> {
 		} else {
 			// Save changes to the same file
 			if (isPreventFocusDisruptionEnabled) {
-				await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs)
+				// Guarded publish: the patched file content is complete, so create-guard
+				// semantics apply (stale observed versions are rejected with a re-read hint).
+				await task.diffViewProvider.saveDirectly(
+					relPath,
+					newContent,
+					false,
+					diagnosticsEnabled,
+					writeDelayMs,
+					"create",
+				)
 			} else {
 				await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
 			}

src/core/tools/EditFileTool.ts

@@ -4,6 +4,7 @@ import path from "path"
 import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 import { getReadablePath } from "../../utils/path"
+import { versionTokenOfStat } from "../../utils/versionToken"
 import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { Task } from "../task/Task"
 import { formatResponse } from "../prompts/responses"
@@ -231,10 +232,23 @@ export class EditFileTool extends BaseTool<"edit_file"> {
 			// Read file or determine if creating new
 			if (fileExists) {
 				try {
+					// The guarded saveDirectly below authorizes the publish against the version
+					// this read saw, and the focus-disruption path has no diff view that could
+					// observe the file. Same contract as ApplyDiffTool/ApplyPatchTool: stat around
+					// the read and observe only when the file did not change underneath it. Without
+					// it the saveDirectly("edit") below rejects with "File not read yet".
+					const preReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
 					currentContent = await fs.readFile(absolutePath, "utf8")
+					const postReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
 					originalEol = detectLineEnding(currentContent)
 					// Normalize line endings to LF for matching
 					currentContentLF = normalizeToLF(currentContent)
+					if (preReadStats && postReadStats) {
+						const preReadToken = versionTokenOfStat(preReadStats)
+						if (preReadToken === versionTokenOfStat(postReadStats)) {
+							task.observationRegistry.observe(absolutePath, preReadToken)
+						}
+					}
 				} catch (error) {
 					task.consecutiveMistakeCount++
 					task.didToolFailInCurrentTurn = true
@@ -436,13 +450,16 @@ export class EditFileTool extends BaseTool<"edit_file"> {
 
 			// Save the changes
 			if (isPreventFocusDisruptionEnabled) {
-				// Direct file write without diff view or opening the file
+				// Direct file write without diff view or opening the file. In-place edits
+				// use edit-guard semantics (a prior read is required); new-file creation
+				// keeps create-guard semantics.
 				await task.diffViewProvider.saveDirectly(
 					relPath,
 					newContent,
 					isNewFile,
 					diagnosticsEnabled,
 					writeDelayMs,
+					isNewFile ? "create" : "edit",
 				)
 			} else {
 				// Call saveChanges to update the DiffViewProvider properties

src/core/tools/EditTool.ts

@@ -4,6 +4,7 @@ import path from "path"
 import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 import { getReadablePath } from "../../utils/path"
+import { versionTokenOfStat } from "../../utils/versionToken"
 import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { Task } from "../task/Task"
 import { formatResponse } from "../prompts/responses"
@@ -89,9 +90,22 @@ export class EditTool extends BaseTool<"edit"> {
 
 			let fileContent: string
 			try {
+				// The guarded saveDirectly below authorizes the publish against the version this
+				// read saw, and the focus-disruption path has no diff view that could observe
+				// the file. Same contract as ApplyDiffTool/ApplyPatchTool: stat around the read
+				// and observe only when the file did not change underneath it. Without it the
+				// saveDirectly("edit") below rejects with "File not read yet".
+				const preReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
 				fileContent = await fs.readFile(absolutePath, "utf8")
+				const postReadStats = await fs.stat(absolutePath, { bigint: true }).catch(() => undefined)
 				// Normalize line endings to LF for consistent matching
 				fileContent = fileContent.replace(/\r\n/g, "\n")
+				if (preReadStats && postReadStats) {
+					const preReadToken = versionTokenOfStat(preReadStats)
+					if (preReadToken === versionTokenOfStat(postReadStats)) {
+						task.observationRegistry.observe(absolutePath, preReadToken)
+					}
+				}
 			} catch (error) {
 				task.consecutiveMistakeCount++
 				task.recordToolError("edit")
@@ -211,8 +225,16 @@ export class EditTool extends BaseTool<"edit"> {
 
 			// Save the changes
 			if (isPreventFocusDisruptionEnabled) {
-				// Direct file write without diff view or opening the file
-				await task.diffViewProvider.saveDirectly(relPath, newContent, false, diagnosticsEnabled, writeDelayMs)
+				// Direct file write without diff view or opening the file. This tool only
+				// edits existing files, so edit-guard semantics require a prior read.
+				await task.diffViewProvider.saveDirectly(
+					relPath,
+					newContent,
+					false,
+					diagnosticsEnabled,
+					writeDelayMs,
+					"edit",
+				)
 			} else {
 				// Call saveChanges to update the DiffViewProvider properties
 				await task.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)