Skip to content

fix: [altimate-code] core_failure::edit — file modified since last read (100/2h) (autofix #486) #489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kulvirgit wants to merge 1 commit into from
Closed

fix: [altimate-code] core_failure::edit — file modified since last read (100/2h) (autofix #486) #489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions packages/opencode/src/file/time.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,29 @@ import { Filesystem } from "../util/filesystem"

export namespace FileTime {
const log = Log.create({ service: "file.time" })
// altimate_change start — track the file snapshot seen at read time so edit freshness checks compare against the observed mtime instead of Date.now(), avoiding false positives from clock skew or delayed mtime updates
type ReadState = {
at: Date
mtime: Date | undefined
}

function snapshot(file: string): ReadState {
const at = new Date()
const mtime = Filesystem.stat(file)?.mtime
return {
at,
mtime: mtime ? new Date(mtime.getTime()) : undefined,
}
}
// altimate_change end
// Per-session read times plus per-file write locks.
// All tools that overwrite existing files should run their
// assert/read/write/update sequence inside withLock(filepath, ...)
// so concurrent writes to the same file are serialized.
export const state = Instance.state(() => {
const read: {
[sessionID: string]: {
[path: string]: Date | undefined
[path: string]: ReadState | undefined
}
} = {}
const locks = new Map<string, Promise<void>>()
Expand All @@ -26,11 +41,15 @@ export namespace FileTime {
log.info("read", { sessionID, file })
const { read } = state()
read[sessionID] = read[sessionID] || {}
read[sessionID][file] = new Date()
// altimate_change start — store both the wall-clock read time and the mtime seen for that read so later asserts can compare against the file version that was actually observed
read[sessionID][file] = snapshot(file)
// altimate_change end
}

export function get(sessionID: string, file: string) {
return state().read[sessionID]?.[file]
// altimate_change start — preserve the existing public API by returning the read timestamp while keeping the richer snapshot internal
return state().read[sessionID]?.[file]?.at
// altimate_change end
}

export async function withLock<T>(filepath: string, fn: () => Promise<T>): Promise<T> {
Expand Down Expand Up @@ -58,14 +77,17 @@ export namespace FileTime {
return
}

const time = get(sessionID, filepath)
// altimate_change start — compare the current mtime against the mtime captured when the file was last read, which avoids rejecting unchanged files when filesystem mtimes run slightly ahead of Date.now()
const time = state().read[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
const baseline = time.mtime ?? time.at
// Allow a 50ms tolerance for Windows NTFS timestamp fuzziness / async flushing
if (mtime && mtime.getTime() > time.getTime() + 50) {
if (mtime && mtime.getTime() > baseline.getTime() + 50) {
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.`,
`File ${filepath} has been modified since it was last read.\nLast modification: ${mtime.toISOString()}\nLast read: ${time.at.toISOString()}\n\nPlease read the file again before modifying it.`,
)
}
// altimate_change end
}
}
3 changes: 3 additions & 0 deletions packages/opencode/src/tool/edit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ export const EditTool = Tool.define("edit", {
},
})

// altimate_change start — re-check file freshness after the permission prompt so an external save during approval cannot be overwritten with a stale edit
await FileTime.assert(ctx.sessionID, filePath)
// altimate_change end
Comment on lines +111 to +113
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if write.ts and apply_patch.ts have the same FileTime.assert pattern after ctx.ask

echo "=== write.ts pattern (ctx.ask followed by write) ==="
rg -n -A15 'ctx\.ask' packages/opencode/src/tool/write.ts | head -30

echo ""
echo "=== apply_patch.ts pattern (ctx.ask followed by write) ==="
rg -n -A30 'ctx\.ask' packages/opencode/src/tool/apply_patch.ts | head -50

Repository: AltimateAI/altimate-code

Length of output: 1813

🏁 Script executed:

# Check edit.ts context around lines 111-113 to see FileTime.withLock() usage
echo "=== edit.ts context around the FileTime.assert() (lines 100-120) ==="
rg -n -B10 -A5 'FileTime.assert' packages/opencode/src/tool/edit.ts

echo ""
echo "=== Check if write.ts and apply_patch.ts use FileTime.withLock() ==="
rg -n 'FileTime.withLock\|FileTime.assert' packages/opencode/src/tool/write.ts
rg -n 'FileTime.withLock\|FileTime.assert' packages/opencode/src/tool/apply_patch.ts

Repository: AltimateAI/altimate-code

Length of output: 1585

🏁 Script executed:

# Check the function signatures to see if write.ts and apply_patch.ts use FileTime.withLock
echo "=== write.ts export/function definition ==="
head -60 packages/opencode/src/tool/write.ts | tail -20

echo ""
echo "=== apply_patch.ts export/function definition ==="
head -60 packages/opencode/src/tool/apply_patch.ts | tail -20

Repository: AltimateAI/altimate-code

Length of output: 1285

🏁 Script executed:

# Search for the actual function implementation in write.ts that contains ctx.ask
echo "=== write.ts full function containing ctx.ask ==="
rg -n -B20 -A20 'ctx\.ask' packages/opencode/src/tool/write.ts | head -60

Repository: AltimateAI/altimate-code

Length of output: 1667

Address race condition inconsistency across edit, write, and apply_patch tools.

The fix correctly prevents file overwrites during the approval window in edit.ts by re-checking freshness after ctx.ask(). However, write.ts (line 45) and apply_patch.ts (lines 198, 203) perform the same sequence—approval prompt followed by file write—but lack an equivalent FileTime.assert() check.

In write.ts, FileTime.read() is called after the write, not before. This records the current file time rather than validating that the file hasn't been externally modified during the approval delay. Similarly, apply_patch.ts contains no freshness checks at all.

Both tools should receive the same protection that edit.ts now has: a FileTime.assert() call immediately after ctx.ask() and before writing, to prevent stale edits from overwriting external modifications made during user approval.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/src/tool/edit.ts` around lines 111 - 113, Add the same
freshness assertion used in edit.ts to the write and apply_patch flows: after
the user approval prompt (ctx.ask) and before performing any filesystem
mutation, call FileTime.assert(ctx.sessionID, filePath) to verify the file
wasn't changed during the approval delay; in write.ts also move/replace the
current FileTime.read() usage so it does not run only after write but instead
the assert runs before writeFile, and in apply_patch.ts add the FileTime.assert
call before running the patch application logic (applyPatch / patch write
function) to prevent overwriting external modifications.

await Filesystem.write(filePath, contentNew)
await Bus.publish(File.Event.Edited, {
file: filePath,
Expand Down
27 changes: 27 additions & 0 deletions packages/opencode/test/file/time.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,33 @@ describe("file/time", () => {
})
})

test("passes when unchanged file mtime is ahead of the local read timestamp", async () => {
await using tmp = await tmpdir()
const filepath = path.join(tmp.path, "file.txt")
await fs.writeFile(filepath, "content", "utf-8")

await Instance.provide({
directory: tmp.path,
fn: async () => {
const originalStat = Filesystem.stat
const futureMtime = new Date(Date.now() + 1000)
;(Filesystem as { stat: typeof Filesystem.stat }).stat = ((file) => {
if (file !== filepath) return originalStat(file)
return { mtime: futureMtime } as ReturnType<typeof Filesystem.stat>
}) as typeof Filesystem.stat

try {
FileTime.read(sessionID, filepath)

// Should not throw because the file snapshot observed at read time is unchanged.
await FileTime.assert(sessionID, filepath)
} finally {
;(Filesystem as { stat: typeof Filesystem.stat }).stat = originalStat
}
},
})
})

test("throws when file was not read first", async () => {
await using tmp = await tmpdir()
const filepath = path.join(tmp.path, "file.txt")
Expand Down
33 changes: 33 additions & 0 deletions packages/opencode/test/tool/edit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,39 @@ describe("tool.edit", () => {
})
})

test("does not overwrite external changes made during the approval window", async () => {
await using tmp = await tmpdir()
const filepath = path.join(tmp.path, "file.txt")
await fs.writeFile(filepath, "original content", "utf-8")

await Instance.provide({
directory: tmp.path,
fn: async () => {
FileTime.read(ctx.sessionID, filepath)

const edit = await EditTool.init()
await expect(
edit.execute(
{
filePath: filepath,
oldString: "original content",
newString: "edited content",
},
{
...ctx,
ask: async () => {
await new Promise((resolve) => setTimeout(resolve, 100))
await fs.writeFile(filepath, "modified externally", "utf-8")
},
},
),
).rejects.toThrow("modified since it was last read")

expect(await fs.readFile(filepath, "utf-8")).toBe("modified externally")
},
})
})

test("replaces all occurrences with replaceAll option", async () => {
await using tmp = await tmpdir()
const filepath = path.join(tmp.path, "file.txt")
Expand Down
Loading