[user-github-me]

Concurrent preference records can overwrite each other before a confirmed preference reaches the personal tier. Serialize each read-modify-write with the existing per-file lock so every project preference and personal sighting survives. Clean up temporary files without hiding the original write error.

The packaged and standalone copies stay identical. The media guide and skill reference explain abandoned-lock recovery, transient lock files, and why a locked personal tier leaves the project record intact.

Fixes #5333.

Validation:

• Integrated main f21c3e3e and regenerated the seven-skill manifest. The previously reviewed code, tests, guide and reference remain byte-identical.
• Fresh full workspace build and all 21 preference/concurrency tests passed, including held/abandoned locks and EPERM/EBUSY cleanup cases.
• Skill lint, mirror checks, manifest freshness against a clean Git export, normal commit hooks and both comment checks passed.
• Mint documentation validation and broken-link checks passed.

[jrusso1020]

Thanks, this fixes #5333 and I'd like to land it. I'll approve as soon as the conflict below is resolved.

What I checked:

• prefs-store.concurrent.test.mjs 5/5 passes at head, and 17/17 together with the existing prefs-store tests. With main's prefs-store.mjs swapped back in, all 5 new tests fail in 5 of 5 runs. The ready barrier keeps the 8-writer race tight, and the held-lock cases are deterministic.
• An out-of-tree probe ran 25 trials of 8 parallel node prefs.mjs record processes (2 projects × 4 keys). Main had 8 ENOENT exits, 18 lost project keys and 14 lost user sightings, with 13/25 trials losing data. This head had no failures, no lost keys or sightings, and no leftover .tmp/.lock files.
• The standalone and CLI prefs-store.mjs copies are byte-identical, and the manifest hash at head matches the tree.

Before merge: the branch conflicts with main only in skills-manifest.json. Please rebase and rerun bun run --cwd packages/cli gen:skills-manifest, since main's media-use hash has moved since your base.

Non-blocking notes:

• prefs-store.mjs:79-80: the finally rmSync(tmp, { force: true }) is unguarded. An EPERM/EBUSY there, most likely on Windows, would mask the original error or fail a write that already succeeded. Wrapping it in try/catch would be safer.
• withFileLock never takes over a lock. A record killed inside the critical section leaves .media/preferences.json.lock, and later records for that project fail with "Settings are locked…" until someone deletes the file. That matches the helper's config.json policy and your abandoned-lock test, so I'm fine with it. It may be worth a mention in the prefs docs, and maybe .media/*.lock in the project's ignore guidance, since .media/ is committed.

This supersedes #5460, which uses the same lock but only patches the standalone copy.

— Rames

[jrusso1020]

Approving at e4c6891f. Thanks for working through both notes. The conflict is resolved and both non-blocking points are addressed.

Since my last review at 4b778034:

• Merged current main (d5cdf418). The only conflict was skills-manifest.json, and gen-skills-manifest.ts --check now reports it in sync (21 skills). main hadn't touched prefs-store.mjs, config-lock.mjs or memory.md since your base, so the merge didn't change the fix.
• prefs-store.mjs:79-82: the finally cleanup is now wrapped in try/catch. I checked the four new EPERM/EBUSY tests in prefs-store.test.mjs by putting the unguarded rmSync back. All four failed: two because a successful write turned into a failure, and two because the cleanup error hid the ENOSPC. With your change they pass.
• skills/media-use/references/memory.md: it now documents recovery from an abandoned preferences.json.lock and the .media/*.lock ignore guidance. The claim that "a locked personal tier does not fail the project record" matches recordPreference, which catches the error from recordUserSighting (prefs-store.mjs:191-195).

At this head, the CLI and standalone prefs-store.mjs files are byte-identical, and prefs-store.test.mjs plus prefs-store.concurrent.test.mjs pass 21/21.

CI: the one red job, CLI: npx shim (ubuntu-latest), was cancelled because bun install hung for 10 minutes. Its macOS and Windows runs passed, so it only needs a rerun.

— Rames

[jrusso1020]

Thanks for this, @user-github-me. This PR now has merge conflicts with main. Could you rebase it onto the latest main? Once it's conflict-free we'll review it and get it into the merge queue.

[mintlify]

Preview approval required

This pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit e46f584.

Approve preview

[miguel-heygen]

Thanks for rebasing this. Since the earlier approval was on an older commit, I compared the fix itself across the two heads: both prefs-store.mjs copies, the tests and the skill reference are unchanged line for line, and the merge keeps main's newer media-home handling around them. The only new piece is the "Remember confirmed preferences" section in the media guide, and it matches the code: the lock file name and recovery steps, the .media/*.lock ignore advice, and the fact that a locked personal tier does not fail the project record.

CI has no failures at this head, including the Windows lanes, and the branch merges cleanly with main.

Verdict: APPROVE
Reasoning: The previously approved fix is unchanged after the rebase, the new docs section is accurate, and CI is green with no conflicts.

[miguel-heygen]

Re-checked at 6acf696b after the merge with main. Compared with f072486a, the only change in this PR's own diff is the regenerated media-use hash in skills-manifest.json, which is expected since main changed other media-use files. Both prefs-store.mjs copies, the tests and the docs are identical, and "Skills: manifest in sync" passes at this head. No failing checks so far; a few jobs were still running when I looked.

Verdict: APPROVE
Reasoning: The approved fix is unchanged; the new commits only bring in main and refresh the manifest hash, which CI confirms is in sync.

[jrusso1020]

This was approved, but it conflicts with main again, most likely the skills manifest hash. Could you rebase on current main, regenerate the manifest, and push? We'll re-approve at the new head.

— Rames

[miguel-heygen]

Re-checked at e46f584 (the changes since the earlier approvals are the rebase and the regenerated manifest). I ran prefs-store.concurrent.test.mjs and prefs-store.test.mjs: 21 passed. With both prefs-store.mjs copies put back to the merge base, 5 fail, so the tests guard the change.

Verdict: APPROVE
Reasoning: same fix as the approved head, tests fail without it, CI green.