fix: eliminate stale file race condition and fix error misclassification

- Use file's actual `mtime` instead of `new Date()` in `FileTime.read()` to
  eliminate clock drift between Node.js and filesystem (WSL, networked drives)
- Increase staleness tolerance from 50ms to 2s (p50 gap was 651ms, caused
  782-retry loops on WSL)
- Split `file_stale` out of `validation` error class for cleaner triage
- Move `http_error` pattern before `validation` and add `HTTP 4xx` keywords
  to prevent WebFetch 404s from being misclassified as validation errors
- Update tests to use `fs.utimes()` for deterministic mtime manipulation

Closes #610

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: anandgupta42

packages/opencode/src/altimate/telemetry/index.ts

@@ -475,7 +475,7 @@ export namespace Telemetry {
         session_id: string
         tool_name: string
         tool_category: string
-        error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "http_error" | "file_not_found" | "edit_mismatch" | "not_configured" | "resource_exhausted" | "unknown"
+        error_class: "parse_error" | "connection" | "timeout" | "validation" | "internal" | "permission" | "http_error" | "file_not_found" | "file_stale" | "edit_mismatch" | "not_configured" | "resource_exhausted" | "unknown"
         error_message: string
         input_signature: string
         masked_args?: string
@@ -761,19 +761,31 @@ export namespace Telemetry {
     // altimate_change end
     { class: "timeout", keywords: ["timeout", "etimedout", "bridge timeout", "timed out"] },
     { class: "permission", keywords: ["permission", "access denied", "permission denied", "unauthorized", "forbidden", "authentication"] },
+    // altimate_change start — http_error before validation to prevent "HTTP 404" matching "invalid"/"missing"
+    {
+      class: "http_error",
+      keywords: ["status code: 4", "status code: 5", "request failed with status", "http 404", "http 410", "http 429", "http 451", "http 403"],
+    },
+    // altimate_change end
+    // altimate_change start — split file_stale out of validation; remove "does not exist" (was catching HTTP 404s)
+    {
+      class: "file_stale",
+      keywords: [
+        "must read file",
+        "has been modified since",
+        "before overwriting",
+      ],
+    },
     {
       class: "validation",
       keywords: [
         "invalid params",
         "invalid",
         "missing",
         "required",
-        "must read file",
-        "has been modified since",
-        "does not exist",
-        "before overwriting",
       ],
     },
+    // altimate_change end
     { class: "internal", keywords: ["internal", "assertion"] },
     // altimate_change start — resource_exhausted class for OOM/quota errors
     {
@@ -788,10 +800,6 @@ export namespace Telemetry {
       ],
     },
     // altimate_change end
-    {
-      class: "http_error",
-      keywords: ["status code: 4", "status code: 5", "request failed with status"],
-    },
   ]
   // altimate_change end
 

packages/opencode/src/file/time.ts

@@ -26,7 +26,12 @@ export namespace FileTime {
     log.info("read", { sessionID, file })
     const { read } = state()
     read[sessionID] = read[sessionID] || {}
-    read[sessionID][file] = new Date()
+    // Use the file's actual mtime instead of wall-clock time to avoid
+    // clock drift between Node.js and the filesystem (especially on WSL,
+    // networked drives, and macOS APFS). This eliminates the race condition
+    // where mtime > new Date() due to filesystem clock skew.
+    const mtime = Filesystem.stat(file)?.mtime
+    read[sessionID][file] = mtime ?? new Date()
   }
 
   export function get(sessionID: string, file: string) {
@@ -61,8 +66,11 @@ export namespace FileTime {
     const time = get(sessionID, filepath)
     if (!time) throw new Error(`You must read file ${filepath} before overwriting it. Use the Read tool first`)
     const mtime = Filesystem.stat(filepath)?.mtime
-    // Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing
-    if (mtime && mtime.getTime() > time.getTime() + 50) {
+    // Allow a 2s tolerance for filesystem clock drift.
+    // WSL (NTFS-over-9P) and networked drives routinely show 400ms–1.2s gaps
+    // between Node.js Date.now() and filesystem mtime. The previous 50ms
+    // tolerance caused massive retry loops (782 retries in one session).
+    if (mtime && mtime.getTime() > time.getTime() + 2000) {
       throw new Error(
         `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.`,
       )

packages/opencode/test/file/time.test.ts

@@ -112,11 +112,10 @@ describe("file/time", () => {
         fn: async () => {
           FileTime.read(sessionID, filepath)
 
-          // Wait to ensure different timestamps
-          await new Promise((resolve) => setTimeout(resolve, 100))
-
-          // Modify file after reading
+          // Modify file and set mtime 5s in the future to exceed 2s tolerance
           await fs.writeFile(filepath, "modified content", "utf-8")
+          const future = new Date(Date.now() + 5000)
+          await fs.utimes(filepath, future, future)
 
           await expect(FileTime.assert(sessionID, filepath)).rejects.toThrow("modified since it was last read")
         },
@@ -132,8 +131,10 @@ describe("file/time", () => {
         directory: tmp.path,
         fn: async () => {
           FileTime.read(sessionID, filepath)
-          await new Promise((resolve) => setTimeout(resolve, 100))
           await fs.writeFile(filepath, "modified", "utf-8")
+          // Set mtime 5 seconds in the future to exceed the 2s tolerance
+          const future = new Date(Date.now() + 5000)
+          await fs.utimes(filepath, future, future)
 
           let error: Error | undefined
           try {
@@ -346,9 +347,10 @@ describe("file/time", () => {
 
           const originalStat = Filesystem.stat(filepath)
 
-          // Wait and modify
-          await new Promise((resolve) => setTimeout(resolve, 100))
           await fs.writeFile(filepath, "modified", "utf-8")
+          // Set mtime 5 seconds in the future to exceed the 2s tolerance
+          const future = new Date(Date.now() + 5000)
+          await fs.utimes(filepath, future, future)
 
           const newStat = Filesystem.stat(filepath)
           expect(newStat!.mtime.getTime()).toBeGreaterThan(originalStat!.mtime.getTime())

packages/opencode/test/telemetry/telemetry.test.ts

@@ -1628,11 +1628,13 @@ describe("telemetry.classifyError", () => {
     expect(Telemetry.classifyError("Invalid dialect specified")).toBe("validation")
     expect(Telemetry.classifyError("Missing required field")).toBe("validation")
     expect(Telemetry.classifyError("Required parameter 'query' not provided")).toBe("validation")
-    // altimate_change start — expanded validation patterns
-    expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("validation")
-    expect(Telemetry.classifyError("File has been modified since it was last read")).toBe("validation")
-    expect(Telemetry.classifyError("error: column foo does not exist")).toBe("validation")
-    expect(Telemetry.classifyError("You must read file before overwriting it. Use the Read tool first")).toBe("validation")
+    // altimate_change start — file_stale split out from validation; "does not exist" moved to http_error
+    // These are now classified as file_stale, not validation
+    expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("file_stale")
+    expect(Telemetry.classifyError("File has been modified since it was last read")).toBe("file_stale")
+    expect(Telemetry.classifyError("You must read file before overwriting it. Use the Read tool first")).toBe("file_stale")
+    // HTTP 404 "does not exist" is now http_error
+    expect(Telemetry.classifyError("HTTP 404: https://example.com/page does not exist")).toBe("http_error")
     // altimate_change end
   })
 
@@ -1651,12 +1653,25 @@ describe("telemetry.classifyError", () => {
     expect(Telemetry.classifyError("Assertion failed: x > 0")).toBe("internal")
   })
 
+  // altimate_change start — file_stale class tests
+  test("classifies file_stale errors", () => {
+    expect(Telemetry.classifyError("You must read file /path/to/file before overwriting it")).toBe("file_stale")
+    expect(Telemetry.classifyError("File /foo.ts has been modified since it was last read")).toBe("file_stale")
+    expect(Telemetry.classifyError("Read the file before overwriting it")).toBe("file_stale")
+  })
+  // altimate_change end
+
   // altimate_change start — http_error class and priority ordering tests
   test("classifies http errors", () => {
     expect(Telemetry.classifyError("Request failed with status code: 404 (example.com)")).toBe("http_error")
     expect(Telemetry.classifyError("Request failed with status code: 500")).toBe("http_error")
     expect(Telemetry.classifyError("status code: 403")).toBe("http_error")
     expect(Telemetry.classifyError("Request failed with status")).toBe("http_error")
+    // altimate_change start — HTTP status codes in webfetch error messages
+    expect(Telemetry.classifyError("HTTP 404: https://example.com does not exist. Do NOT retry")).toBe("http_error")
+    expect(Telemetry.classifyError("HTTP 410: https://example.com has been permanently removed")).toBe("http_error")
+    expect(Telemetry.classifyError("HTTP 429: Rate limited by example.com")).toBe("http_error")
+    // altimate_change end
   })
 
   test("priority ordering: earlier patterns win over later ones", () => {

packages/opencode/test/tool/edit.test.ts

@@ -238,11 +238,11 @@ describe("tool.edit", () => {
           // Read first
           FileTime.read(ctx.sessionID, filepath)
 
-          // Wait a bit to ensure different timestamps
-          await new Promise((resolve) => setTimeout(resolve, 100))
-
-          // Simulate external modification
+          // Simulate external modification with mtime 5s in the future
+          // to exceed the 2s tolerance in FileTime.assert()
           await fs.writeFile(filepath, "modified externally", "utf-8")
+          const future = new Date(Date.now() + 5000)
+          await fs.utimes(filepath, future, future)
 
           // Try to edit with the new content
           const edit = await EditTool.init()