Skip to content

fix(storage): retry Windows session replacement under read locks - #946

Merged
likun666661 merged 1 commit into
apache:mainfrom
zhiiw:codex/fix-windows-session-atomic-replace
Jul 14, 2026
Merged

likun666661 merged 1 commit into
apache:mainfrom
zhiiw:codex/fix-windows-session-atomic-replace

Conversation

@zhiiw

@zhiiw zhiiw commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • retry Windows session-file replacement when a short-lived reader causes EPERM or EACCES
  • give atomic-write temp files UUID-qualified names
  • clean up temp files after both successful and failed replacement attempts
  • add a Windows regression test that holds the destination open while a header update begins

Root cause

FileSessionStore.updateHeader() safely rewrites session.jsonl through a temporary file and atomic rename. On Windows, replacing the destination fails with EPERM while another reader briefly has session.jsonl open. Session list and preview reads can overlap this header update, so a transient read lock could fail a turn before the provider request was made and surface as missing_terminal_event.

The replacement now retries only the Windows lock-related error codes, with a small bounded backoff. Persistent permission errors still surface after the retry budget; non-Windows behavior remains a single attempt.

Validation

  • node --test packages/storage/dist/__tests__/session-store.test.js — 40 passed
  • npm run typecheck
  • git diff --check

The regression test uses a real Windows file handle rather than mocking the error: the old implementation fails with EPERM, while the retry succeeds after the reader releases the file.

@zhiiw
zhiiw marked this pull request as ready for review July 14, 2026 03:01
@likun666661
likun666661 merged commit 5f559cd into apache:main Jul 14, 2026
3 checks passed
@zhiiw
zhiiw deleted the codex/fix-windows-session-atomic-replace branch July 14, 2026 04:35
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.

2 participants