Skip to content

Commit f2d5e8d

Browse files
anandgupta42claude
andcommitted
test: [AI-450] add tests for stale file recovery + append failure context to error
- Add comprehensive test suite for `StaleFileError` class and recovery logic: - `instanceof` checks (typed error vs regular Error) - Small file auto-re-read with content in response - Large file rejection with size message - Missing file graceful handling - Null/undefined error safety - Regex-like error text on regular Error does NOT trigger recovery - Backtick fencing handles files containing markdown code blocks - FilePath preserved for paths with spaces/special chars - Append read failure context to error message when auto-re-read fails, so the model knows why re-reading didn't work Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b567494 commit f2d5e8d

2 files changed

Lines changed: 223 additions & 0 deletions

File tree

packages/opencode/src/session/processor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ export namespace SessionProcessor {
237237
}
238238
} catch (readErr) {
239239
log.warn("failed to auto-re-read stale file", { file: staleFilePath, error: readErr })
240+
errorStr += `\n\nAttempted to auto-re-read the file but failed: ${String(readErr)}`
240241
}
241242
}
242243
// altimate_change end
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
// altimate_change start — tests for stale file auto-re-read recovery (#450)
2+
import { describe, test, expect } from "bun:test"
3+
import * as fs from "fs/promises"
4+
import * as path from "path"
5+
import { StaleFileError } from "../../src/file/time"
6+
import { FileTime } from "../../src/file/time"
7+
import { Filesystem } from "../../src/util/filesystem"
8+
import { Instance } from "../../src/project/instance"
9+
10+
async function tmpdir() {
11+
const dir = await fs.mkdtemp(path.join(import.meta.dir, ".tmp-"))
12+
return {
13+
path: dir,
14+
[Symbol.asyncDispose]: async () => {
15+
await fs.rm(dir, { recursive: true, force: true })
16+
},
17+
}
18+
}
19+
20+
describe("StaleFileError", () => {
21+
test("extends Error", () => {
22+
const err = new StaleFileError("/path/to/file.ts", "File was modified")
23+
expect(err).toBeInstanceOf(Error)
24+
expect(err).toBeInstanceOf(StaleFileError)
25+
expect(err.name).toBe("StaleFileError")
26+
})
27+
28+
test("carries filePath property", () => {
29+
const err = new StaleFileError("/some/path/file.sql", "modified since last read")
30+
expect(err.filePath).toBe("/some/path/file.sql")
31+
expect(err.message).toBe("modified since last read")
32+
})
33+
34+
test("works with instanceof check", () => {
35+
const err: Error = new StaleFileError("/test", "msg")
36+
if (err instanceof StaleFileError) {
37+
expect(err.filePath).toBe("/test")
38+
} else {
39+
throw new Error("instanceof check failed")
40+
}
41+
})
42+
43+
test("preserves stack trace", () => {
44+
const err = new StaleFileError("/file", "error")
45+
expect(err.stack).toBeDefined()
46+
expect(err.stack).toContain("StaleFileError")
47+
})
48+
})
49+
50+
describe("FileTime.assert throws StaleFileError", () => {
51+
test("throws StaleFileError when file modified since read", async () => {
52+
await using tmp = await tmpdir()
53+
const filepath = path.join(tmp.path, "test.txt")
54+
await fs.writeFile(filepath, "original", "utf-8")
55+
56+
await Instance.provide({
57+
directory: tmp.path,
58+
fn: async () => {
59+
FileTime.read("test-session", filepath)
60+
61+
// Wait and modify externally
62+
await new Promise((r) => setTimeout(r, 100))
63+
await fs.writeFile(filepath, "modified", "utf-8")
64+
65+
try {
66+
await FileTime.assert("test-session", filepath)
67+
throw new Error("should have thrown")
68+
} catch (e) {
69+
expect(e).toBeInstanceOf(StaleFileError)
70+
expect((e as StaleFileError).filePath).toBe(filepath)
71+
expect((e as StaleFileError).message).toContain("modified since it was last read")
72+
}
73+
},
74+
})
75+
})
76+
77+
test("does not throw StaleFileError for unread files", async () => {
78+
await using tmp = await tmpdir()
79+
const filepath = path.join(tmp.path, "test.txt")
80+
await fs.writeFile(filepath, "content", "utf-8")
81+
82+
await Instance.provide({
83+
directory: tmp.path,
84+
fn: async () => {
85+
try {
86+
await FileTime.assert("test-session-2", filepath)
87+
throw new Error("should have thrown")
88+
} catch (e) {
89+
// This should be a regular Error, NOT StaleFileError
90+
expect(e).toBeInstanceOf(Error)
91+
expect(e).not.toBeInstanceOf(StaleFileError)
92+
expect((e as Error).message).toContain("You must read file")
93+
}
94+
},
95+
})
96+
})
97+
})
98+
99+
describe("stale file recovery logic", () => {
100+
// These tests replicate the recovery logic from processor.ts in isolation,
101+
// following the same pattern as processor.test.ts for telemetry tests.
102+
103+
const MAX_AUTO_READ_BYTES = 50 * 1024
104+
105+
async function simulateRecovery(opts: {
106+
error: unknown
107+
sessionID: string
108+
filePath?: string
109+
fileContent?: string
110+
fileSize?: number
111+
fileExists?: boolean
112+
}) {
113+
let errorStr = String(opts.error ?? "Unknown error")
114+
115+
if (opts.error instanceof StaleFileError) {
116+
const staleFilePath = opts.error.filePath
117+
try {
118+
if (opts.fileExists === false) {
119+
throw new Error("ENOENT: no such file or directory")
120+
}
121+
const size = opts.fileSize ?? Buffer.byteLength(opts.fileContent ?? "", "utf-8")
122+
if (size > MAX_AUTO_READ_BYTES) {
123+
errorStr += `\n\nThe file has been modified (${Math.round(size / 1024)}KB). It is too large to include here — please use the Read tool to view it.`
124+
} else {
125+
const freshContent = opts.fileContent ?? ""
126+
const fence = "````"
127+
errorStr += `\n\nThe file has been auto-re-read. Here is the current content:\n\n${fence}\n${freshContent}\n${fence}`
128+
}
129+
} catch (readErr) {
130+
errorStr += `\n\nAttempted to auto-re-read the file but failed: ${String(readErr)}`
131+
}
132+
}
133+
134+
return errorStr
135+
}
136+
137+
test("only triggers for StaleFileError, not regular errors", async () => {
138+
const regularError = new Error("some other tool error")
139+
const result = await simulateRecovery({
140+
error: regularError,
141+
sessionID: "s1",
142+
})
143+
expect(result).toBe("Error: some other tool error")
144+
expect(result).not.toContain("auto-re-read")
145+
})
146+
147+
test("appends file content for small files", async () => {
148+
const err = new StaleFileError("/test/file.sql", "modified since last read")
149+
const result = await simulateRecovery({
150+
error: err,
151+
sessionID: "s1",
152+
fileContent: "SELECT * FROM orders;",
153+
})
154+
expect(result).toContain("auto-re-read")
155+
expect(result).toContain("SELECT * FROM orders;")
156+
expect(result).toContain("````")
157+
})
158+
159+
test("rejects large files with size message", async () => {
160+
const err = new StaleFileError("/test/big.sql", "modified")
161+
const result = await simulateRecovery({
162+
error: err,
163+
sessionID: "s1",
164+
fileSize: 100 * 1024, // 100KB
165+
})
166+
expect(result).toContain("too large to include here")
167+
expect(result).toContain("please use the Read tool")
168+
expect(result).not.toContain("auto-re-read")
169+
})
170+
171+
test("handles missing file gracefully", async () => {
172+
const err = new StaleFileError("/test/deleted.sql", "modified")
173+
const result = await simulateRecovery({
174+
error: err,
175+
sessionID: "s1",
176+
fileExists: false,
177+
})
178+
expect(result).toContain("Attempted to auto-re-read the file but failed")
179+
expect(result).toContain("ENOENT")
180+
})
181+
182+
test("handles null/undefined errors safely", async () => {
183+
// Both null and undefined should produce "Unknown error" via ?? coalescing
184+
const result1 = await simulateRecovery({ error: null, sessionID: "s1" })
185+
expect(result1).toBe("Unknown error")
186+
187+
const result2 = await simulateRecovery({ error: undefined, sessionID: "s1" })
188+
expect(result2).toBe("Unknown error")
189+
})
190+
191+
test("does not trigger for errors with similar text", async () => {
192+
// A regular Error with stale-file-like text should NOT trigger recovery
193+
const trickyError = new Error("File /etc/passwd has been modified since it was last read")
194+
const result = await simulateRecovery({
195+
error: trickyError,
196+
sessionID: "s1",
197+
})
198+
expect(result).not.toContain("auto-re-read")
199+
expect(result).not.toContain("too large")
200+
})
201+
202+
test("file content with backticks does not break fencing", async () => {
203+
const err = new StaleFileError("/test/file.md", "modified")
204+
const content = "```python\nprint('hello')\n```"
205+
const result = await simulateRecovery({
206+
error: err,
207+
sessionID: "s1",
208+
fileContent: content,
209+
})
210+
// Uses ```` (4 backticks) so inner ``` (3 backticks) don't break it
211+
expect(result).toContain("````")
212+
expect(result).toContain(content)
213+
})
214+
215+
test("filePath is extracted from StaleFileError, not parsed from message", async () => {
216+
// Path with spaces and special chars
217+
const weirdPath = "/Users/test user/my project/models/stg orders.sql"
218+
const err = new StaleFileError(weirdPath, "some error message without the path")
219+
expect(err.filePath).toBe(weirdPath)
220+
})
221+
})
222+
// altimate_change end

0 commit comments

Comments
 (0)