Conversation
SessionCatalog.save() calls sync() on an opened directory handle after
atomically replacing sessions.json. On Windows, Node returns EPERM when
syncing a directory handle, so session persistence failed even though the
file itself was writable and file-level fsync succeeded:
Error: EPERM: operation not permitted, fsync
syscall: fsync
Keep the temporary-file sync before the atomic rename and skip only the
directory sync on win32. Platforms that support directory fsync keep the
existing durability behavior.
Adds a regression test that injects the Windows EPERM on the directory
handle and confirms the save succeeds on win32 (and still fails elsewhere).
…platform Attempt the directory sync everywhere and treat only EPERM, EINVAL, and ENOTSUP as "this platform or filesystem cannot fsync a directory handle". Any other error still fails the save. This fixes Windows without a platform check, covers the same failure on filesystems that reject directory fsync, and drops the injected-platform constructor parameter. Regression tests inject each tolerated code on the directory handle and assert the save succeeds and both handles are synced and closed once, plus a case asserting EIO from directory sync still fails the save.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Browser Control is currently unusable on Windows
Every relay-backed command that creates or updates a session (
execute,session new,session adopt, …) fails on Windows with:The relay starts and the extension connects fine, but the first session write fails, so nothing useful can run. This affects 0.7.0 and, as far as I can tell, every release since the durable session catalog was introduced.
Cause
SessionCatalog.save()callssync()on an opened directory handle after atomically replacingsessions.json. Windows cannotfsynca directory handle: libuv implementsfsyncwithFlushFileBuffers, which requires write access, and Node opens the directory read-only. The result isEPERMon every save.Confirmed on Windows:
sessions.jsonis writable; the user's ACL grants Full Control.fsync()on the temporary file succeeds.sync().Minimal reproduction (no Browser Control involved)
The same sequence
save()runs — write temp file, fsync it, rename into place, fsync the directory — fails at the last step on Windows:The same script prints
directory fsync: okon Linux (node v24.13.0).Fix
Attempt the directory sync everywhere, and treat only the codes that mean "this platform or filesystem cannot fsync a directory handle" as non-fatal:
EPERM(Windows),EINVALandENOTSUP(some Linux filesystems and network mounts). Any other error from the directory sync (EIO,EBADF, …) still fails the save.The temporary-file
sync()before the atomic rename is untouched, so on platforms that cannot fsync a directory the save still gets the strongest durability guarantee available. Platforms that support directoryfsynckeep the exact behavior documented inPLAN.md(updated to note the exception). This is the same approach SQLite and Postgres take for directory fsync.No platform check and no API change:
SessionCatalog's constructor is unchanged.Verification
Tested on Windows against the installed 0.7.0 by applying the equivalent guard to
dist/cli.js. Before the patch everyexecutefailed with the error above; after it:succeeds.
Tests
EPERM,EINVAL,ENOTSUP) injects the error on the directory handle'ssync()and asserts the save succeeds, the catalog round-trips, and both the file and directory handles are synced and closed exactly once.EIOon the directory sync and asserts the save still fails with the existing error envelope.directory-syncfailure-injection case (plainError, no code) is unchanged and still rejects.pnpm typecheck,pnpm test(799 passing),pnpm check:unused, andpnpm check:localsare green.