Skip to content

fix(session): serialize canonical file access - #5383

Merged
chengyongru merged 2 commits into
HKUDS:mainfrom
chengyongru:codex/fix-windows-session-store-concurrency
Aug 14, 2026
Merged

fix(session): serialize canonical file access#5383
chengyongru merged 2 commits into
HKUDS:mainfrom
chengyongru:codex/fix-windows-session-store-concurrency

Conversation

@chengyongru

Copy link
Copy Markdown
Collaborator

Summary

  • serialize canonical session JSONL readers and mutators behind one sessions-directory lock
  • coordinate separate SessionManager instances that point at the same canonical directory
  • use unique, exclusively created temp files while preserving atomic os.replace and existing fsync behavior
  • route direct WebUI index scanning and Dream pruning through the store-owned boundary

Root cause

On Windows, an open reader for the destination JSONL file can deny os.replace(tmp, target) with [WinError 5] Access is denied.

The confirmed production path was internal and deterministic:

  1. /api/sessions ran _sessions_list_payload through asyncio.to_thread.
  2. session_list_index._scan_session_row opened a canonical session JSONL file.
  3. A peer turn completed and JsonlSessionStore.save tried to replace that same file.
  4. Windows rejected the replacement, so a successfully streamed answer was followed by the generic error response.

The store also used one predictable <session>.jsonl.tmp, so concurrent saves could share scratch state.

Design

JsonlSessionStore now owns one FileLock at .session-files.lock per canonical sessions directory. Load/read/metadata/list/save/delete/repair and migration/restore use it. The exceptional direct consumers—WebUI index reconciliation and Dream pruning—enter the same boundary through a narrow SessionManager.locked_session_files() context manager.

Different manager instances coordinate because they resolve the same lock path. Nested repair/delete operations reuse the same reentrant lock. Critical sections contain bounded local file work only; no LLM, provider, or network operation is locked.

Session and WebUI-index writers now use unique open(..., "x") temp files. Atomic replacement and the existing conditional file/directory fsync policy are unchanged.

Related work

PR #5382 retries os.replace on Windows. That may be useful defense against brief locks from uncoordinated external software, but it does not prevent the confirmed in-process reader/writer overlap, cover delete/repair/restore paths, coordinate multiple managers, or remove the shared temp-file race. This PR fixes the synchronization invariant at the persistence owner; a narrowly justified external-lock retry could still be layered inside that boundary later.

Issue #5290 and PR #5291 concern deduplicating atomic JSONL writer code and persisting subagent transcripts. They are related to the write idiom but do not define this session-file concurrency boundary.

Validation

  • relevant session/Dream/WebUI regression set: 116 passed, 1 skipped, 2 Windows-symlink tests deselected
  • deterministic interleaving test proves WebUI scanning and save cannot overlap while the canonical file handle is open
  • cross-manager lock and unique-temp regressions pass
  • Ruff: passed
  • BasedPyright on changed production files: 0 errors, 0 warnings
  • TypeScript + Vite production build: passed
  • isolated gateway/browser: session list and persisted /model thread returned 200 after refresh; post-refresh console clean; runtime cleanup complete

@chengyongru chengyongru added bug Something isn't working fix priority: p2 Normal backlog: minor bug, enhancement, docs, cleanup, edge case, or unvalidated proposal. test webui conflict labels Aug 13, 2026
@chengyongru
chengyongru force-pushed the codex/fix-windows-session-store-concurrency branch from 26b6daf to f49b4d0 Compare August 14, 2026 02:15
@chengyongru
chengyongru merged commit e226242 into HKUDS:main Aug 14, 2026
6 checks passed
@chengyongru
chengyongru deleted the codex/fix-windows-session-store-concurrency branch August 14, 2026 02:32
okwillbe pushed a commit to okwillbe/nanobot that referenced this pull request Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix priority: p2 Normal backlog: minor bug, enhancement, docs, cleanup, edge case, or unvalidated proposal. test webui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant