Skip to content

fix(runtime): swap plugin cache links atomically - #134

Merged
NagyVikt merged 2 commits into
mainfrom
agent/claude/atomic-plugin-cache-symlink-2026-08-07-12-19
Aug 7, 2026
Merged

fix(runtime): swap plugin cache links atomically#134
NagyVikt merged 2 commits into
mainfrom
agent/claude/atomic-plugin-cache-symlink-2026-08-07-12-19

Conversation

@NagyVikt

@NagyVikt NagyVikt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • verified locally

NagyVikt added 2 commits August 7, 2026 12:27
A Stop hook died mid-session with "Plugin directory does not exist:
<runtime>/plugins/cache/thedotmack/claude-mem/13.13.1 — run /plugin to
reinstall". The plugin was not missing: its payload had been on disk since
2026-08-04, and only the runtime's `plugins/cache` symlink was gone,
recreated ten seconds later while hooks were firing.

linkPluginCache replaced each entry with rm() then symlink(). Between those
two awaits the path does not exist, and runtimes are re-materialized while
sessions are live — so a hook resolving a plugin installPath in that window
hits the exact error this function's doc comment says it prevents.

Stage the new link beside the target and rename() it over instead. rename(2)
within one directory is atomic, so a concurrent reader sees the old entry or
the new one, never neither. rename refuses to clobber a real directory, so
remove-then-create stays as the fallback for Claude's lazy empty cache/ copy
on a first materialization — no session can be reading it yet. On failure the
staged link is cleaned up and the existing entry is left alone.

The window was not theoretical. The new race test polls the path across 300
swaps: 441 ENOENTs against the old implementation, 0 against this one. It
cannot false-fail, since an atomic rename has no window in which the entry is
absent.
The fallback branch can't be atomic — POSIX has no way to replace a
directory with a symlink in one step. Say so, instead of claiming no
session can be reading it.
@NagyVikt

NagyVikt commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

AI review (gate): no CRITICAL/HIGH.

One finding, fixed in 3f2a-follow-up commit:

  • The fallback branch's comment claimed "no session is reading it yet, so the non-atomic path is safe here". That is not guaranteed — Claude can recreate cache/ as a real directory at any point, and POSIX has no atomic directory→symlink replacement. Comment corrected to state the limitation rather than assert safety. The branch is still strictly better than before in every case, and the steady-state path (target is already a symlink) is now atomic — that is the one that was failing hooks.

Validation:

  • tsc --noEmit clean; biome lint clean on both touched files (3 warnings in runtime-materializer.ts are pre-existing, lines 286/1518/1530).
  • CI green on all four checks (test, lint, Profiles e2e ubuntu + macos); base main also green.
  • Local bun test full suite: 33 failures, byte-identical set on base and branch (diff empty) — pre-existing and environment-specific, since CI's test job is green on both.

The race assertion is the load-bearing test: polling the target across 300 swaps yields 441 ENOENT against the old rm-then-symlink implementation and 0 against this one. Verified by reverting the implementation and re-running.

@NagyVikt
NagyVikt marked this pull request as ready for review August 7, 2026 10:31
@NagyVikt
NagyVikt merged commit ca492fb into main Aug 7, 2026
4 checks passed
@NagyVikt
NagyVikt deleted the agent/claude/atomic-plugin-cache-symlink-2026-08-07-12-19 branch August 7, 2026 10:31
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.

1 participant