fix(worker): resolve cache version by semver, not mtime#3354
Open
jamincollins wants to merge 1 commit into
Open
fix(worker): resolve cache version by semver, not mtime#3354jamincollins wants to merge 1 commit into
jamincollins wants to merge 1 commit into
Conversation
…mack#2424) cacheWorkerScriptCandidates() picked the worker-service.cjs to spawn by sorting cache/thedotmack/claude-mem/<version> directories on mtime. An in-place plugin update extracts both the old and new version directories in the same pass, frequently leaving them with identical mtimes — at that point Array.prototype.sort's tie-break is whatever order readdirSync happened to return, which is not version order. Observed in the wild: with 13.10.2 and 13.11.0 both present and mtime-tied, the resolver kept picking 13.10.2. Each worker restart re-ran the same resolution, so the worker crash-looped by design (checkVersionMatch detected the plugin/worker mismatch, requested a recycle, and the recycle spawn hit the identical stale resolution again) — restarting every few seconds and leaking an orphaned chroma-mcp/uvx process pair per cycle (~150 processes / several GB RSS accumulated in under 10 minutes on the machine this was diagnosed on). Fix: sort candidates by the version directory name itself via a small semver-descending comparator, not by filesystem mtime. The directory name *is* the version identifier, so this is deterministic regardless of extraction order or mtime ties. Issue thedotmack#2424 previously proposed this and was closed as not planned; filing this PR because the failure mode reproduced live rather than theoretically.
Contributor
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.
Problem
cacheWorkerScriptCandidates()(src/shared/worker-utils.ts) picks whichcache/thedotmack/claude-mem/<version>/scripts/worker-service.cjsto spawnby sorting version directories on
mtimeMs. An in-place plugin updateextracts the old and new version directories in the same pass, so they
frequently end up with identical mtimes — at that point
Array.prototype.sort's comparator returns0for every pair, and theresulting order is whatever
readdirSynchappened to return, not versionorder.
This is exactly the scenario in #2424, which was closed as not planned. I'm
reopening it as a PR because it reproduced live rather than theoretically:
What I observed
On a machine with both
13.10.2and13.11.0cached (mtimes tied to thesecond), the resolver kept picking
13.10.2. Every worker restart re-ranthe same resolution, so:
13.10.2script.checkVersionMatch()notices the plugin (13.11.0) and running worker(
13.10.2) disagree and requests a recycle.picks
13.10.2again.Each cycle left behind an orphaned chroma-mcp/uvx process pair (the old
worker's
ChromaMcpManagernever got a chance to tear its subprocess downcleanly before being replaced). Over roughly 10 minutes this produced ~75
orphaned pairs (~150 processes, several GB RSS) before I killed the loop
manually — a variant of the accumulation described in #499/#789/#803/#1090,
but caused by the resolver, not by
ChromaMcpManager's own cleanup paths.Fix
Sort candidates by the version directory name itself (semver
descending) rather than by mtime. The directory name is the version
identifier, so this is deterministic regardless of extraction order or
mtime ties — no filesystem timestamp involved at all.
Testing
tests/shared/worker-utils-cache-version-resolution.test.ts:unit tests for the new comparator (major/minor/patch ordering, equal
versions, a realistic multi-version directory listing).
bun test tests/shared/— 126 pass, 0 fail (no regressions).tsc --noEmitclean.I didn't touch the equivalent mtime-sort in
src/build/hook-shell-template.ts(the generated shell/Node launcher embedded in
hooks.json/.mcp.json/codex hooks) — that path has its own byte-matching generator test and is a
separate, larger surface; happy to follow up there if this approach looks
right to you.