AntFleet

Disagreement · 44bd7a66-anthropic-3

saveSession is not atomic on Windows when target exists, and not concurrency-safe across processes

mismatch
repo 56f59a0d·PR #1·reviewed 4 days ago

Primary finding

saveSession is not atomic on Windows when target exists, and not concurrency-safe across processes

mediumdata-losshigh
  • src/session.ts:44-52
SESSION_TMP and SESSION_FILE are constants shared across all sessions for a user. Two mythos processes running concurrently for the same user will race on latest.tmp — one process can overwrite the other's tmp file between writeFileSync and renameSync, producing a corrupted/incorrect SESSION_FILE that mixes data from two sessions. Even single-process: if writeFileSync throws partway, latest.tmp is left on disk indefinitely (no cleanup). The header comment claims 'atomic writes' but the tmp path is not unique per call (no PID/UUID suffix), violating the typical atomic-write contract.

Recommendation

Use a unique tmp filename per call (e.g., `latest.${process.pid}.${Date.now()}.tmp` or crypto.randomUUID()), wrap in try/finally to unlink the tmp on failure, and consider an advisory file lock (proper-lockfile) for multi-process safety.

Counterpart finding

Session save uses a single fixed temp filename causing race/lost-update risk across concurrent saves

mediumconcurrencyhigh
  • src/session.ts:14-15
  • src/session.ts:46-49
All writers use the same temp path (latest.tmp). Concurrent calls (from multiple processes or threads) can overwrite each other's temp file and final rename, leading to lost updates or leaving a stray latest.tmp if a crash occurs between write and rename.

Recommendation

Use a unique tmp filename per save (e.g., latest.<pid>.<timestamp>.<random>.tmp) and then rename to latest.json. Optionally fsync the directory after rename for stronger durability and clean up any stale tmp files on startup.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →

From the same review

These findings passed the unanimous gate on the same PR review. The disagreement above was filtered out; the findings below were posted.