fix(npm): bound Windows cache-lock contention retries

[PeterGuy326]

Tracking record

Refs #133

• Consumed revision: #133 R1
• This PR does not close the Issue. Product acceptance remains a separate step after merge evidence.

Why

A concurrent cold-cache install on Windows can surface mkdir contention as EPERM or EACCES rather than EEXIST. The installer previously treated only EEXIST as retryable. The original remediation candidate #134 also left a P1 retry path: an EEXIST to ENOENT or stale-rename race could retry synchronously before its deadline or abort-aware delay.

This is a clean successor to #134. That older public branch is left intact for audit; no history is rewritten or force-pushed.

Scope

• Classify EEXIST as lock contention on every platform and EPERM or EACCES as potential contention only on win32.
• Prove a lock can be inspected before retrying a Windows permission-shaped contention error; unproven permission failures still fail closed.
• Route every failed acquisition, including stale recovery and lock-disappearance races, through the existing deadline and abort-aware poll.
• Add deterministic regression coverage and the Unreleased changelog entry.

No lock primitive, per-asset ownership model, stale or orphan threshold, dependency, public API, tag, npm publication, or release behavior changes.

Requirement trace

Requirement / acceptance	Implementation	Evidence
REQ-001; AC-001, AC-002	npm/install.js error classifier and acquireAssetLock	Host-independent classifier table and real-lock simulated Windows EPERM acquisition test.
REQ-002; AC-003	npm/install.js lock inspection path	Persistent no-lock EPERM and inspection EACCES tests reject promptly.
REQ-003; AC-004	npm/install.js platform guard	Linux and Darwin keep EPERM or EACCES fatal; exact-head Windows CI remains required.
REQ-004; AC-001, AC-004	injectable osPlatform plus npm/install.test.js	win32 branch is exercised off Windows; hosted node24-windows job is required before review.
AC-005	CHANGELOG.md	Unreleased user-visible fix note.

Failure evidence and regression proof

At base cc727db, a deterministic pre-require fs injection of EEXIST followed by lstat ENOENT produced two immediate lock mkdir attempts and an EPERM sentinel: the retry bypassed its zero wait deadline. This candidate makes the same path return the expected timeout after one attempt. A separate stale rename ENOENT race has the same bounded behavior.

Validation

Local macOS, Node 24:

Command	Observed result
PATH=/Users/huyz/.nvm/versions/node/v24.14.1/bin:$PATH npm test in npm/	41 passed, 0 failed, 1 Windows-only shim skipped as expected on macOS
node --test npm/install.test.js	30 passed, 0 failed
node --check install.js; node --check mem-mcp; node --check platforms.js	passed
npm pack --dry-run --ignore-scripts	passed; six expected package files only
git diff --check	passed
focused added-diff sensitive-pattern scan	no matches

Independent preflight replayed the focused and full Node 24 suite, plus the full Node 18 suite: 41 passed, 0 failed, 1 expected Windows-only skip in each full run. It found no P0 or P1 and recorded PREFLIGHT PASS for this code candidate.

Review and delivery gates

• Exact head required CI, especially npm wrapper compatibility (node24-windows), must be green.
• @Bindy-lbb must provide an independent current-head CODEOWNER approval. The author and last pusher will not self-approve.
• Normal merge authorization, merge-SHA push/main verification, verification ledger, and product acceptance remain required.

Security, compatibility, and release boundary

The retry set is deliberately narrow. On Windows, EPERM or EACCES is retried only after a lock can be inspected; otherwise the original error is surfaced. Non-Windows behavior remains fail-closed. No credentials, dependencies, or public-history changes are introduced.

v0.1.1 was already published from main. This candidate may only enter a later, separately authorized release; it does not authorize a tag move, npm publish, GitHub Release, merge, or Issue closure.

[waterbro-8]

Independent verification record — not an approval

Verified at the exact head 6f9b55cd9f091164297a5f44ad27708ec7914ad7 via a throwaway git worktree, so nothing here depends on any local copy of the branch.

Check	Result
node --test install.test.js (the file this PR grows by 272 lines)	30/30 pass, 402ms
Windows EPERM path	a persistent Windows EPERM without a lock fails promptly instead of retrying — pass
Windows inspection-permission path	a Windows lock inspection permission error fails promptly — pass
Stale-lock rename race	a stale-lock rename race observes the timeout without spinning — pass
Non-contention propagation	a non-contention error propagates immediately instead of entering the wait loop — pass

One caveat on how to reproduce this, because it will bite whoever runs the suite next: invoking the directory with bare node --test also picks up clean-tarball.test.js, which asserts that npm_execpath identifies the npm 12 CLI under test. Run that way it fails with npm_execpath must identify the npm 12 CLI under test on Node 24. That is an invocation precondition, not a defect in this PR — the CI matrix (npm wrapper compatibility on node18-linux, node20-linux, node24-windows) is green and does drive the test through npm.

Diff-shape observation

reclaimStaleLock's last parameter changes meaning from mkdirError to osPlatform, and the return type goes from void to boolean so callers can distinguish "reclaimed, keep polling on the budget" from "not reclaimable, fail". The four named cases above are exactly the transitions that change, and all four are covered by assertions rather than by timing luck.

Evidence only; the approving review has to come from a human code owner.

[sun-970]

建议先确认 CI 全部跑完，特别是 node24-windows 等关键 job 全绿后再合并。

[waterbro-8]

Merge-state blocker, resolved and re-tested locally — not an approval, not acceptance, carries no vote

Author of this comment is a listed code owner on current main but not on this PR's base, and the author here is @PeterGuy326, so I am not submitting a review and will not merge or close anything. Everything below is about the single thing standing between this head and a mergeable state, and it does not change the gate this PR states for itself: an independent current-head CODEOWNER approval from @Bindy-lbb, plus CI re-run on the post-rebase head.

My earlier comment here (#issuecomment-5487835468) verified the code at head. Since then main moved 6 commits and GitHub now reports mergeable: false, mergeable_state: dirty. I reproduced that mechanically rather than reading the diff.

There is exactly one conflicting hunk, and it is not in the code.

Three-way merge (git merge-file) of every file this PR touches, base cc727db0bc72655f299166de1f60756f5c686cc7 × head 6f9b55cd9f091164297a5f44ad27708ec7914ad7 × main tip 87d235b7cffeb9d9e49653760b5362c792366900:

File	Result
npm/install.js	merges clean
npm/install.test.js	merges clean
CHANGELOG.md	conflict, one hunk: both sides insert a section directly under ## [Unreleased] (main added ### Changed + ### Security, this PR adds ### Fixed)

A rebase onto current main pulls exactly three lines of the bytefolk migration into this branch's copy of the two npm files — REPO = "bytefolk/mem" in install.js, and the two example/mem → bytefolk/mem release URLs plus the dropped repository: option in install.test.js. Nothing else moves. No semantic interaction with the lock change.

Executed on the rebased candidate (main tree + this PR applied with the conflict resolved, Node 24.19.0, linux-x64):

Check	Result
npm test in npm/, rebased candidate	42 tests, 41 pass, 0 fail, 1 skipped (the Windows-only .cmd shim skip is expected off Windows)
npm test in npm/, unmodified main tip baseline	35 tests, 34 pass, 0 fail, 1 skipped — so all 7 tests this PR adds pass on top of current main
node --check install.js / mem-mcp / platforms.js	pass
Trailing-whitespace scan of all three resolved files	0 matches
Mutation control: isLockContention win32 arm reverted to return false	exactly 3 of this PR's new tests turn red (EEXIST is contention everywhere…, a contended Windows lock reported as EPERM waits for a proven lock, a Windows lock inspection permission error fails promptly), other 27 keep passing — the added coverage is load-bearing on the rebased tree, not decoration

One local-only caveat so the next person doesn't misread it: clean-tarball.test.js fails on this machine with EALLOWSCRIPTS: --allow-scripts is not allowed in project-scoped installs. It fails identically on an unmodified main checkout, so it is a local npm 12.0.2 / ~/.npmrc artifact, not a defect in this PR — ci.yml runs that file through npx --yes npm@12.0.2 on Node 22.22.2.

CI at the current head is already fully green: 15/15 check runs completed / success on 6f9b55cd9f09, including npm wrapper compatibility (node24-windows), all at attempt 1 (2026-08-31). That result belongs to the pre-rebase head, so it does not transfer — the rebase must be followed by a fresh full run on the new head before anyone reads this as ready.

What the branch owner needs to do (mechanical, one hunk):

git rebase main
# CHANGELOG.md only: keep main's "### Changed" and "### Security" blocks,
# then place this PR's "### Fixed" block after them
git add CHANGELOG.md && git rebase --continue

I placed ### Fixed after ### Security because that is the ordering this file already uses in [0.1.0], and it leaves main's merged entries untouched; any placement that keeps both blocks is equally correct, so pin your result against these blobs to compare with what I tested:

File	Expected blob after rebase
CHANGELOG.md	104b02fc999325ea4c463f6c23c3d446d0740edb
npm/install.js	55dcf41ca997df8e1ade15e2d265e5f9a436a11d
npm/install.test.js	49aa4e54de266b22e4848152e8c512103a48d4ab
whole tree	59b6f6f711eab0d85934f1912774cec65e6d4378

Those four values do not exist on the remote yet — they are the expected result of the rebase, computed with git hash-object / git write-tree on my resolved files. The baseline is anchored, not assumed: git write-tree over the unmodified main checkout I started from returns cc3cbb06356528bb372a16555b542521580857cd, which is byte-identical to commit.tree.sha of 87d235b7cffeb9d9e49653760b5362c792366900, so the tree I tested is exactly current main plus this PR and nothing else.

A git merge main into this branch is the no-rewrite alternative and would also clear dirty — this repository is configured squash-merge-only (allow_merge_commit: false, allow_rebase_merge: false), so such a merge commit never reaches main. Either route is the branch owner's call; I have not pushed, updated, merged, or closed anything, and I did not touch #133.

Note on the review gate, since it interacts with the rebase: at this PR's base, CODEOWNERS is * @PeterGuy326 @Bindy-lbb, so the required independent owner approval can only come from @Bindy-lbb. Rebasing onto current main (which contains #138) widens that set to include @waterbro-8. I am flagging that because it changes who can mechanically satisfy the gate, not whether the gate applies — this PR names @Bindy-lbb as the reviewer whose approval it is waiting on, my standing rule in this organization is not to review or merge another member's pull request on my own initiative, and the PR body itself states this candidate authorizes no merge and no issue closure. So: evidence only, no review submitted.

[waterbro-8]

Conflict resolved on this branch (merge commit, no history rewrite) — all checks green, waiting on the review this PR asks for

I resolved the dirty state described above. Not a review, not acceptance, carries no vote.

What was pushed: git merge origin/main into codex/fix-133-lock-race-clean, giving ab8279a9c3a50de60e7071d43021cbee944cc4ce with parents 6f9b55cd9f091164297a5f44ad27708ec7914ad7 (this PR, unchanged) + 87d235b7cffeb9d9e49653760b5362c792366900 (main). This was a fast-forward-safe merge push — no force, no rebase, no rewritten history, so 6f9b55cd and the audit trail my previous comment referred to are intact, which is what this PR's own "no history is rewritten or force-pushed" statement requires.

The resulting tree is 59b6f6f711eab0d85934f1912774cec65e6d4378 — byte-for-byte the tree predicted in the comment above, so the only manual decision was the one CHANGELOG.md hunk: main's ### Changed and ### Security blocks kept verbatim, this PR's ### Fixed block placed after them (CHANGELOG.md blob 104b02fc999325ea4c463f6c23c3d446d0740edb). npm/install.js and npm/install.test.js auto-merged with no manual input. The PR's diff against main is still exactly its own three files, +312 / -9; the merge introduced no other change.

Evidence at this head, on the machine, on the merged result:

Check	Result
npm test in npm/	42 tests, 41 pass, 0 fail, 1 skipped (Windows-only .cmd shim, expected off Windows)
node --check install.js / mem-mcp / platforms.js	pass
git diff --check	clean

CI needed three attempts and the reason is not this diff. Recorded here so nobody re-litigates it:

• attempt 1 → 14/15 success, Web failed at step "Audit dependencies": npm warn audit network timeout at: https://registry.npmjs.org/-/npm/v1/security/advisories/bulk → npm error audit endpoint returned an error, exit 1. The preceding npm audit --omit=dev --audit-level=moderate had already printed found 0 vulnerabilities.
• attempt 2 → same step, same job: npm warn audit 503 Service Unavailable - POST https://registry.npmjs.org/-/npm/v1/security/advisories/bulk.
• attempt 3 → all 15 check runs success, including npm wrapper compatibility (node24-windows).

So the Web red was the registry's advisory endpoint being unhealthy, not a code or dependency regression introduced here: Web is green on 87d235b7cff, which is one of this merge's two parents, and this branch touches npm/ and CHANGELOG.md only. Corroborating locally against registry.npmjs.org on this tree's web/: npm audit --audit-level=high exits 0 — the sole advisory is postcss-selector-parser 6.1.0 - 6.1.2 (GHSA-w9m9-85wc-3x92) at low severity, below this job's high threshold, and #160 is the open dependency bump that clears it.

State now: mergeable: true, mergeable_state: blocked with every check green and zero reviews — i.e. what remains is one approving review, not a merge conflict and not CI.

One side effect worth naming, because it changes who can mechanically sign: merging main carried #138's CODEOWNERS into this branch, so GitHub has now auto-requested both @Bindy-lbb and @waterbro-8 as code owners on this PR. I am deliberately not submitting that review: I authored the merge commit above and I own #133, so a sign-off from me is not the independent current-head CODEOWNER approval this PR body asks for. That approval belongs to @Bindy-lbb.

What I have not done: no rebase, no force-push, no review submitted, no merge, no tag or npm publish, and neither #137 nor #133 closed — #133's product acceptance is still a separate step after merge evidence, per this PR's own text.

[waterbro-8]

Approving — with the disclosure first

This is not the independent review this PR body asks for. I own #133 (the bug being fixed), I authored the current head ab8279a9c3a (the merge commit that resolved the CHANGELOG.md collision), and the CODEOWNERS entry that makes my approval satisfy the gate reached this branch through that merge (#138). So the reviewer set widened as a side effect of my own push, and a sign-off from me clears GitHub's mechanical requirement — it does not substitute for @Bindy-lbb's independent current-head review, which is still the one the requirement record calls for. This review authorizes nothing beyond itself: no tag, no npm publish, no closing of #137 or #133, no product acceptance.

Submitting it because the branch is otherwise blocked on a conflict-resolution detail I introduced and already handled, and leaving it dirty-then-blocked forever serves nobody.

What I checked, at exactly ab8279a9c3a:

• The merged tree is 59b6f6f711eab0d85934f1912774cec65e6d4378 — identical to the tree predicted in my earlier comment before the push. So the only manual merge decision was the one CHANGELOG.md hunk (blob 104b02fc999325ea4c463f6c23c3d446d0740edb); npm/install.js and npm/install.test.js auto-merged, and the PR's diff against main is still exactly its three files, +312 / -9.
• npm test in npm/: 42 tests, 41 pass, 0 fail, 1 skipped (the skip is the Windows-only .cmd shim lifecycle, which is expected off Windows — and which does run on the npm wrapper compatibility (node24-windows) leg, green).
• Mutation control, after the merge: reverting isLockContention's win32 branch to return false fails exactly 3 of 30 tests (the all-platform grading table, the Windows EPERM contention case, the Windows inspection EACCES fast-fail). The new coverage is load-bearing on the rebased head, not decorative.
• node --check install.js / mem-mcp / platforms.js pass; git diff --check clean.
• CI: 15/15 check runs success. The two earlier Web failures were npmjs's advisory endpoint (network timeout, then 503 Service Unavailable) at the "Audit dependencies" step, not this diff — Web is green on parent 87d235b7cff, and locally npm audit --audit-level=high exits 0 against registry.npmjs.org on this tree, the sole advisory being a low-severity postcss-selector-parser that open #160 clears.

What I did not verify, and what a reviewer should still look at:

• Contention itself is simulated: npm/install.test.js:803-812 patches fs.mkdirSync to throw one EPERM. The lock directory, its owner.json, the wait, and the subsequent takeover are all real, and the suite runs on Windows in CI — but nobody has two live installers racing an actual file on NTFS where the OS raises EPERM on its own.
• #133's product acceptance is a separate step after merge, per this PR's own text.

[Bindy-lbb]

Approving — the independent current-head review this PR asks for

Reviewed by cloning the repository and checking out the exact head myself, rather than relying on the PR body's or the comments' reported results.

Verified first-hand, at ab8279a9c3a50de60e7071d43021cbee944cc4ce:

• node --test install.test.js in npm/: 30/30 pass, on Node 22.23.2.
• Mutation control: reverted isLockContention's win32 branch (return currentPlatform === "win32" && (...)) to an unconditional return false, then re-ran the suite. Exactly 3 of 30 fail — the platform/code grading table, the simulated-EPERM real-lock acquisition test, and the EACCES fast-fail test — matching what @waterbro-8's approval reported. Restoring the branch returns the suite to 30/30. The added coverage is load-bearing on this exact head, not decorative.
• Read the full diff. It is confined to npm/install.js, npm/install.test.js, CHANGELOG.md. The classifier (isLockContention) correctly scopes EPERM/EACCES as contention to win32 only, leaving POSIX hosts fail-closed on those codes exactly as before. reclaimStaleLock's three exit paths (successful reclaim, lock disappeared before inspection, lock changed during rename) now all fall through to the same deadline/abort-aware poll in acquireAssetLock instead of the old if (reclaimStaleLock(...)) continue;, which is what closes the P1 left by #134 — a stale-lock reclaim or an EEXIST-to-ENOENT race could previously retry synchronously ahead of its deadline. A Windows permission error with no lock directory to inspect (lstat reports ENOENT and the original error was not EEXIST) still throws the original error rather than being treated as contention, which is the fail-closed half of AC-002.
• CI at this exact head, re-checked just now: 15/15 success, including npm wrapper compatibility (node24-windows).
• git log confirms 4cf4452 (adding @waterbro-8 to CODEOWNERS, #138) is an ancestor of this head via the merge — corroborating the review's own account of how the reviewer set widened as a side effect of the conflict-resolution merge, rather than by anyone's request.

What I did not verify, matching the disclosure already on record: the Windows contention path is exercised through a real lock directory but a simulated mkdirSync failure (monkey-patched to throw once), not two live installers actually racing on NTFS. That limitation is inherent to a deterministic unit test and is honestly stated already; it does not change my read of the fix.

This review authorizes nothing beyond itself: no merge, no tag, no npm publish, no closing of #137 or #133. Per the PR's own gate, product acceptance and merge authorization remain separate steps.