Skip to content

fix: resolve history-shadow.ts race condition (#479)#480

Merged
bradygaster merged 1 commit intobradygaster:devfrom
diberry:squad/479-history-shadow-race-fix
Mar 22, 2026
Merged

fix: resolve history-shadow.ts race condition (#479)#480
bradygaster merged 1 commit intobradygaster:devfrom
diberry:squad/479-history-shadow-race-fix

Conversation

@diberry
Copy link
Copy Markdown
Collaborator

@diberry diberry commented Mar 20, 2026

Summary

\�ppendToHistory()\ in \history-shadow.ts\ uses a read → regex insert → write pattern with no synchronization. When two concurrent agents append to the same \history.md, the last write wins and the other agent's entry is silently lost.

Root Cause

The function reads the entire file, performs a regex-based section insertion, then writes the entire file back. Between the read and write, another caller can read the same (stale) version, compute its own update, and overwrite the first caller's changes.

Fix

  1. In-process async mutex (\withFileLock) — serializes concurrent callers targeting the same file path. Uses a promise-chain pattern (zero dependencies). Different files run in parallel.
  2. Atomic writes (\�tomicWriteFile) — writes to a temp file then renames, preventing concurrent readers from seeing partial content.

Zero new dependencies — pure Node.js built-ins only.

Tests

14 new tests in \ est/history-shadow.test.ts:

  • Regression: single-caller append, sequential multi-append, section creation
  • Race condition: 5 concurrent appends, 10 concurrent appends, cross-section concurrent appends — all verify zero data loss
  • Cleanup: no lock files or temp files left behind
  • API contract: create, read, delete, exists, error on missing shadow

All 14 pass. Existing agent tests (65 tests) unaffected.

Fixes #479

appendToHistory() used readFile → regex insert → writeFile with no
synchronization.  Two concurrent agents appending to the same
history.md caused last-write-wins data loss.

Fix: wrap the read-modify-write cycle in an in-process async mutex
(keyed by resolved file path) and use atomic writes (temp file +
rename) to prevent partial reads.  Zero new dependencies — uses
only Node.js built-ins.

- Adds withFileLock() async mutex that serializes concurrent callers
  per file while allowing parallelism across different files.
- Adds atomicWriteFile() using temp file + fs.rename.
- Adds 14 tests covering regression, concurrent race conditions
  (5 and 10 simultaneous appends), cross-section concurrency,
  lock/temp file cleanup, and full API contract verification.

Fixes bradygaster#479

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bradygaster bradygaster force-pushed the squad/479-history-shadow-race-fix branch from 266efa4 to ea7f4d9 Compare March 22, 2026 06:54
Copy link
Copy Markdown
Owner

@bradygaster bradygaster left a comment

Choose a reason for hiding this comment

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

EECOM review: Race condition fix looks correct. Promise-chain mutex + atomic write via temp-file rename — solid pattern, zero new deps. 14 new tests cover regression, concurrency (5 and 10 parallel appends), cleanup, and API contract. Rebased on dev (includes #483 CI timeout fix).

@bradygaster bradygaster merged commit dfa513a into bradygaster:dev Mar 22, 2026
1 of 2 checks passed
chrislomonico pushed a commit to clomonico/squad that referenced this pull request Mar 26, 2026
…adygaster#480)

* feat: session persistence across shell restarts (bradygaster#459)

Add session store module that saves and restores shell message history
across shell restarts. Sessions are persisted as JSON files in
.squad/sessions/ with unique IDs and safe timestamps.

Changes:
- New session-store.ts: createSession, saveSession, loadLatestSession,
  listSessions, loadSessionById functions
- Auto-resume recent sessions (<24h) on shell start
- Auto-save session on each message exchange
- Final save on shell exit
- /sessions command to list past sessions
- /resume command to restore a specific session by ID prefix
- 13 new tests for session store module
- Package export for session-store subpath

Closes bradygaster#459

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: use unwrapped addMessage in onRestoreSession to avoid duplicates

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: history-shadow.ts has read-modify-write race condition (data loss)

3 participants