Skip to content

Commit b567494

Browse files
anandgupta42claude
andcommitted
fix: [AI-450] address code review — typed StaleFileError, bounded reads, safe stringification
Addresses all findings from multi-model code review: - CRITICAL: Replace regex-based error detection with typed `StaleFileError` class that extends `Error` with a `filePath` property. Use `instanceof` check in processor instead of brittle string parsing. - MAJOR: Add 50KB size limit before auto-reading stale files to prevent OOM and token explosion. Files over limit get a message to use the Read tool instead. - MAJOR: Handle missing files (deleted between error and re-read) gracefully. - MINOR: Use `String(value.error ?? "Unknown error")` for null-safe stringification. - MINOR: Use markdown code fences instead of `<file>` XML tags to prevent prompt injection via unescaped content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d4aa51d commit b567494

2 files changed

Lines changed: 34 additions & 12 deletions

File tree

packages/opencode/src/file/time.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,17 @@ import { Log } from "../util/log"
33
import { Flag } from "../flag/flag"
44
import { Filesystem } from "../util/filesystem"
55

6+
// altimate_change start — typed error for stale file detection (#450)
7+
export class StaleFileError extends Error {
8+
public readonly filePath: string
9+
constructor(filePath: string, message: string) {
10+
super(message)
11+
this.name = "StaleFileError"
12+
this.filePath = filePath
13+
}
14+
}
15+
// altimate_change end
16+
617
export namespace FileTime {
718
const log = Log.create({ service: "file.time" })
819
// Per-session read times plus per-file write locks.
@@ -63,9 +74,12 @@ export namespace FileTime {
6374
const mtime = Filesystem.stat(filepath)?.mtime
6475
// Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing
6576
if (mtime && mtime.getTime() > time.getTime() + 50) {
66-
throw new Error(
77+
// altimate_change start — use typed StaleFileError (#450)
78+
throw new StaleFileError(
79+
filepath,
6780
`File ${filepath} has been modified since it was last read.\nLast modification: ${mtime.toISOString()}\nLast read: ${time.toISOString()}\n\nPlease read the file again before modifying it.`,
6881
)
82+
// altimate_change end
6983
}
7084
}
7185
}

packages/opencode/src/session/processor.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ import type { SessionID, MessageID } from "./schema"
1919
// altimate_change start — import Telemetry for per-generation token tracking
2020
import { Telemetry } from "@/altimate/telemetry"
2121
// altimate_change end
22-
// altimate_change start — import FileTime and Filesystem for stale file recovery (#450)
22+
// altimate_change start — import StaleFileError and Filesystem for stale file recovery (#450)
23+
import { StaleFileError } from "@/file/time"
2324
import { FileTime } from "@/file/time"
2425
import { Filesystem } from "@/util/filesystem"
2526
// altimate_change end
@@ -216,17 +217,24 @@ export namespace SessionProcessor {
216217
const match = toolcalls[value.toolCallId]
217218
if (match && match.state.status === "running") {
218219
// altimate_change start — auto-read stale files so model sees current content (#450)
219-
let errorStr = (value.error as any).toString()
220-
const staleFileMatch =
221-
errorStr.match(/File (.+) has been modified since it was last read/) ??
222-
errorStr.match(/You must read file (.+) before overwriting it/)
223-
if (staleFileMatch) {
224-
const staleFilePath = staleFileMatch[1].trim()
220+
let errorStr = String(value.error ?? "Unknown error")
221+
if (value.error instanceof StaleFileError) {
222+
const staleFilePath = value.error.filePath
225223
try {
226-
const freshContent = await Filesystem.readText(staleFilePath)
227-
FileTime.read(input.sessionID, staleFilePath)
228-
errorStr += `\n\nThe file has been auto-re-read. Here is the current content:\n<file path="${staleFilePath}">\n${freshContent}\n</file>`
229-
log.info("stale file auto-re-read", { file: staleFilePath, sessionID: input.sessionID })
224+
const stat = Filesystem.stat(staleFilePath)
225+
const MAX_AUTO_READ_BYTES = 50 * 1024
226+
if (!stat) {
227+
errorStr += "\n\nNote: The file no longer exists on disk."
228+
} else if (Number(stat.size) > MAX_AUTO_READ_BYTES) {
229+
FileTime.read(input.sessionID, staleFilePath)
230+
errorStr += `\n\nThe file has been modified (${Math.round(Number(stat.size) / 1024)}KB). It is too large to include here — please use the Read tool to view it.`
231+
} else {
232+
const freshContent = await Filesystem.readText(staleFilePath)
233+
FileTime.read(input.sessionID, staleFilePath)
234+
const fence = "````"
235+
errorStr += `\n\nThe file has been auto-re-read. Here is the current content:\n\n${fence}\n${freshContent}\n${fence}`
236+
log.info("stale file auto-re-read", { file: staleFilePath, sessionID: input.sessionID })
237+
}
230238
} catch (readErr) {
231239
log.warn("failed to auto-re-read stale file", { file: staleFilePath, error: readErr })
232240
}

0 commit comments

Comments
 (0)