feat(read-file): record read scope and report clipping separately (U4, #1375)

src/core/task/Task.ts

@@ -111,6 +111,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"
@@ -286,6 +287,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

src/core/task/__tests__/observationRegistry.spec.ts

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

src/core/task/observationRegistry.ts

@@ -0,0 +1,59 @@
+/**
+ * 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)
+	}
+
+	clear(): void {
+		this.entries.clear()
+	}
+
+	get size(): number {
+		return this.entries.size
+	}
+}

src/core/tools/ReadFileTool.ts

@@ -16,13 +16,14 @@
 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"
 import { getReadablePath } from "../../utils/path"
 import { extractTextFromFile, addLineNumbers, getSupportedBinaryFormats } from "../../integrations/misc/extract-text"
 import { readWithIndentation, readWithSlice } from "../../integrations/misc/indentation-reader"
-import { DEFAULT_LINE_LIMIT } from "../prompts/tools/native-tools/read_file"
+import { DEFAULT_LINE_LIMIT, MAX_LINE_LENGTH } from "../prompts/tools/native-tools/read_file"
 import type { ToolUse, PushToolResult } from "../../shared/tools"
 
 import {
@@ -214,14 +215,36 @@
 					// 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)
+					// A lossy decode is not the whole file: the model never saw those bytes.
+					const lossyDecode = !Buffer.from(fileContent).equals(buffer)
+					// S4b follow-up (#46 / epic #1375): processTextFile reports whether the
+					// returned content is the whole file; the observation below records that
+					// scope so the write guard can deny full-file updates built on a partial view.
+					const processed = 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, processed.complete && !lossyDecode)
+						}
+					}
+
 					updateFileResult(relPath, {
-						nativeContent: `File: ${relPath}\n${result}`,
+						nativeContent: `File: ${relPath}\n${processed.content}`,
 					})
 				} catch (error) {
 					const errorMsg = error instanceof Error ? error.message : String(error)
@@ -265,8 +288,14 @@
 
 	/**
 	 * Process a text file according to the requested mode.
+	 *
+	 * Returns the content string plus whether that content is the complete
+	 * file (S4b follow-up #46 / epic #1375): slice mode is complete only
+	 * when it starts at line 1, returns every line, and was not truncated;
+	 * indentation mode is never complete because it returns semantic blocks
+	 * of the file, not the file itself.
 	 */
-	private processTextFile(content: string, entry: InternalFileEntry): string {
+	private processTextFile(content: string, entry: InternalFileEntry): { content: string; complete: boolean } {
 		const mode = entry.mode || "slice"
 
 		if (mode === "indentation") {
@@ -299,7 +328,8 @@
 				output += `\n\nIncluded ranges: ${rangeStr} (total: ${result.totalLines} lines)`
 			}
 
-			return output
+			// Indentation mode returns semantic blocks: never a complete file view.
+			return { content: output, complete: false }
 		}
 
 		// Slice mode (default): simple offset/limit reading
@@ -322,11 +352,27 @@
 	To read more: Use the read_file tool with offset=${nextOffset} and limit=${limit}.
 
 	${result.content}`
+			if (result.hasClippedLines) {
+				// The slice cut lines off and also clipped long lines inside it, so both
+				// notices belong to the response.
+				output += `\nNote: Some lines in this view exceed ${MAX_LINE_LENGTH} characters and were clipped in this view.`
+			}
+		} else if (result.hasClippedLines) {
+			// Every line was returned, so there is no later offset to read: report the
+			// clipping without a next-offset hint, and keep the read incomplete so a
+			// full-file replacement cannot be built from a clipped line.
+			output = `IMPORTANT: Some lines exceed ${MAX_LINE_LENGTH} characters and were clipped in this view. ${offset1 === 1 ? "The file was read in full" : `The returned slice starts at line ${offset1} and reaches the end of the file`}, but the clipped lines were not shown in full.\n${result.content}`
 		} else if (result.returnedLines === 0) {
 			output = "Note: File is empty"
 		}
 
-		return output
+		// Complete only when the slice starts at line 1, returned every line, and
+		// showed every line in full (returnedLines === totalLines follows from the
+		// first two conditions): a partial start, a truncated tail, or a clipped
+		// line means the model did not see the whole file.
+		const complete = offset0 === 0 && !result.wasTruncated && !result.hasClippedLines
+
+		return { content: output, complete }
 	}
 
 	/**
@@ -768,9 +814,20 @@
 				}
 
 				// Read text file
-				const rawContent = await fs.readFile(fullPath, "utf8")
+				// 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 rawBuffer = await fs.readFile(fullPath)
+				const rawContent = rawBuffer.toString("utf-8")
+				// Same contract: a lossy decode is a partial view.
+				const lossyDecode = !Buffer.from(rawContent).equals(rawBuffer)
 
 				// Handle line ranges if specified
+				// S4b follow-up (#46 / epic #1375): a line-range read returns only the requested
+				// ranges, and a slice truncated to DEFAULT_LINE_LIMIT returns only the head of
+				// the file — record such observations as partial so the write guard denies a
+				// full-file update built on them.
+				let readComplete = false
 				let content: string
 				if (entry.lineRanges && entry.lineRanges.length > 0) {
 					const lines = rawContent.split("\n")
@@ -790,15 +847,36 @@
 					// Read with default limits using slice mode
 					const result = readWithSlice(rawContent, 0, DEFAULT_LINE_LIMIT)
 					content = result.content
+					readComplete = !result.wasTruncated && !result.hasClippedLines
 					if (result.wasTruncated) {
 						content += `\n\n[File truncated: showing ${result.returnedLines} of ${result.totalLines} total lines]`
+						if (result.hasClippedLines) {
+							// Both notices: the slice was truncated and a line inside it was
+							// clipped.
+							content += `\n\n[Some lines exceed the per-line length cap and were clipped in this view]`
+						}
+					} else if (result.hasClippedLines) {
+						content += `\n\n[Some lines exceed the per-line length cap and were clipped in this view]`
 					}
 				}
 
 				results.push(`File: ${relPath}\n${content}`)
 
 				// 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, readComplete && !lossyDecode)
+					}
+				}
 			} catch (error) {
 				const errorMsg = error instanceof Error ? error.message : String(error)
 				results.push(`File: ${relPath}\nError: ${errorMsg}`)