fix(update): publish the sync lock with an atomic temp-write + hard-link

[dvd233]

What

Fixes #760 — under contention, acquireLock can hand one lock to two live processes.

Root cause

exclusiveCreate wrote the payload directly with writeFile(..., { flag: 'wx' }). The lock file exists empty/partial between the open and the write. A concurrent isLockStale reading that window treats the live lock as unparseable (i.e. reclaimable); the reclaimer then renames over the live holder, and both sides believe they hold the lock. The issue's 16×2000 stress showed ~1% of acquisitions overlapping another holder.

Fix

Publish creation with a unique sibling temp file + hard-link: link(2) is atomic and fails with EEXIST when the target already exists, so every on-disk observation of the lock is a complete, parseable payload. Stale detection, the reclaim sentinel, and owner-verified release are unchanged — this only changes how a new lock file comes into existence.

Verification

Regression test (red → green): new lock-atomic.test.ts case — concurrent create/read must never observe a partially written lock.

• Without the fix: fails with SyntaxError: Unexpected end of JSON input (the partial write is observed).
• With the fix: passes; 13/13 stable across repeated runs.

Real-filesystem A/B stress (multi-process): 8 child processes × 30 acquire/release rounds on one lock path, with a detector in the reclaim branch that flags any rename whose target was created during the reclaim (another live creator's lock being overwritten). To make the microsecond-scale window observable, the unfixed build's publish window was artificially widened (the holder's create sleeps 50 ms before writing, mirroring the issue's interleaving):

• Unfixed: 5 overwrite-of-live-lock events in 240 attempts.
• Fixed: 0.

Unit & contract: update.test.ts / lock-atomic.test.ts / update-policy.test.ts 80/80 pass — the fs-extra mock gained link, and the assertions that pinned the old wx-write implementation were realigned to the temp-write + link mechanism (behavior contract unchanged).

Full suite: npx vitest run — 254 files / 4,202 tests passed; 37 files / 94 tests fail, the identical failure set as unmodified main on this Windows machine (pre-existing symlink/permission/environment failures, none related to the lock path).

Build & types: npm run build (tsup) and npx tsc --noEmit clean; git diff --check clean.

Note: the change touches only exclusiveCreate; CLI user-facing output is untouched, so no docs/skill-data wording is affected.

Follow-up validation (commit 2c3502b)

Addressed the review findings:

• Added the missing fs-extra.link type mock.
• Added a regression test for filesystems that reject hard links.
• Added a fallback to the historical wx exclusive create when hard links are unavailable, so updates do not fail outright on FAT/exFAT or network filesystems.
• Updated stale comments that still described the removed hard-link/O_EXCL mechanism.

Additional verification on the updated head:

• npx tsc --noEmit
• npx vitest run src/__tests__/update.test.ts src/__tests__/lock-atomic.test.ts — 69 passed
• npm run build
• node dist/index.js --help
• node dist/index.js update --help
• git diff --check

The focused ESLint command was not available in this checkout because the repository does not provide an ESLint config/package; npx resolved ESLint 10, which reported no eslint.config.*. No lint result is claimed.

Follow-up validation (commit ad65c88)

Addressed the remaining race concern in the unsupported-filesystem fallback:

• Fresh unparseable locks are treated as busy for a 2-second grace window, so a partial wx write cannot be reclaimed while its creator may still be writing.
• Legacy malformed locks older than the grace window remain reclaimable.
• Updated the contention tests to make link(tmp, target) return EEXIST, matching the actual target-lock race rather than an implausible temporary-name collision.

The updated head again passes npx tsc --noEmit, the focused 69-test suite, npm run build, real CLI help smoke checks, and git diff --check.

Follow-up validation (commit d5a19f6)

Reworked the unsupported-filesystem path to remove the remaining partial-write race:

• Hard-link-capable filesystems keep the temp-write + hard-link publication.
• Other filesystems now use an atomically-created lock directory with a .owner payload file; no lock target file is ever partially written.
• Stale detection and owner-verified release now understand both file locks and directory locks.
• A live reclaim sentinel blocks fresh acquirers while a stale directory lock is replaced.

The updated head passes npx tsc --noEmit, the focused 69-test suite, npm run build, real CLI help smoke checks, and git diff --check.

Follow-up validation (commit 32b9032)

Closed the remaining publication-window concern:

• On filesystems without hard-link support, the lock directory is created atomically, then its .owner payload is written to a sibling temp file and atomically renamed into place.
• A directory without a published owner is treated as busy during the grace period; metadata/stat failures no longer imply that a live lock is stale.
• Updated the fallback test and real-filesystem comments to cover directory-lock publication rather than the old wx race.

The updated head passes npx tsc --noEmit, the focused 69-test suite, npm run build, real CLI help smoke checks, and git diff --check.

[github-actions]

Findings

• [P1 blocking] src/__tests__/update.test.ts:506 — mockedFse.link is used, but the typed mock at src/__tests__/update.test.ts:118 does not declare link. Because tsconfig.json includes all of src/**/*, npx tsc --noEmit fails with Property 'link' does not exist, contradicting the PR’s verification record.
• [P1 blocking] src/update.ts:269 — Every lock acquisition now requires hard-link support. For project locks on filesystems that reject hard links (for example FAT/exFAT or some network-mounted filesystems), link() fails, acquireLock() returns false, and pull/push/migration incorrectly report permanent contention. The previous implementation worked on these filesystems.
• [P1 blocking] The PR description has no real-CLI end-to-end verification record. The filesystem stress harness is useful but does not satisfy the repository requirement to build and exercise the actual CLI; no Agent/provider Test Plan is documented.
• [P2 non-blocking] src/update.ts:329 and src/__tests__/lock-atomic.test.ts:9 still describe the implementation as writeFile(..., { flag: 'wx' })/O_EXCL, although this PR replaces it with temp-write plus hard-link. Update these comments to avoid documenting the removed mechanism.

No earlier findings were provided to mark as resolved.

[github-actions]

Findings

• [P1 blocking] src/update.ts:275 — The unsupported-filesystem fallback uses the original writeFile(..., { flag: 'wx' }) mechanism, reintroducing the exact partial-write race this PR fixes. On FAT/exFAT or affected network filesystems, a concurrent reader can still classify a live, partially written lock as stale and grant the lock twice.
• [P1 blocking] The PR description still lacks the required real-CLI end-to-end verification and Agent/provider Test Plan. Filesystem stress tests, unit tests, builds, and type checks do not demonstrate exercising the built CLI with the required agents and providers.
• [P2 non-blocking] src/__tests__/update.test.ts:781 — The live-holder test makes the unique temporary-file write return EEXIST, rather than making link(tmp, target) return EEXIST. This models an implausible UUID-temp collision and does not test the actual target-lock contention path. The same issue appears at src/__tests__/update.test.ts:522.

Resolved Earlier Findings

• The typed fs-extra mock now declares link.
• Hard-link rejection no longer causes unconditional permanent contention, although its fallback has the blocking correctness issue above.
• The stale O_EXCL implementation comments previously identified in src/update.ts and src/__tests__/lock-atomic.test.ts are corrected.

[github-actions]

Findings

• [P1 blocking] src/update.ts:255 — The two-second grace period does not make the wx fallback safe. If the writer stalls for over two seconds after creating the file but before completing the payload—especially plausible on the unsupported/network filesystems using this fallback—a contender again classifies the partial lock as stale, replaces it, and both processes return success.
• [P1 blocking] src/update.ts:241 — Promise.all([readFile, stat]) treats any stat failure as a stale lock, even when readFile successfully returned a valid live-owner payload. Repeated metadata errors can therefore cause a live lock to be replaced. Parse the content first and request metadata only for unparseable content; arbitrary stat failures must not imply staleness.
• [P1 blocking] The PR description still lacks the required real-CLI end-to-end verification and Agent/provider Test Plan. Running node dist/index.js --help and update --help does not exercise the changed locking workflow, and no verification is recorded for Claude, Codex, CodeBuddy, OpenCode or the git, gitlab, and github providers.

Resolved Earlier Findings

• The typed fs-extra mock now declares link.
• Unsupported hard-link filesystems no longer fail unconditionally, although the fallback remains unsafe as described above.
• The previously identified stale implementation comments are corrected.
• Contention tests now model EEXIST from link(tmp, target) rather than an implausible temporary-file UUID collision.

[github-actions]

Findings

• [P1 blocking] src/update.ts:291 — The directory fallback publishes the directory before writing .owner. If that write stalls for over two seconds, another process treats the live lock as stale, removes it, and acquires the lock while the original creator still returns success. This recreates the double-holder race on filesystems without hard-link support.
• [P1 blocking] src/update.ts:242 — A transient stat failure still immediately marks the lock stale without attempting to read its valid owner payload. A live lock can therefore be reclaimed during metadata errors; the earlier finding remains unresolved.
• [P1 blocking] The PR description still lacks the required real-CLI end-to-end verification and Agent/provider Test Plan. --help smoke checks do not exercise locking, and there is no recorded validation for Claude, Codex, CodeBuddy, OpenCode or the git, gitlab, and github providers.
• [P2 non-blocking] src/__tests__/update.test.ts:723 — The test-suite comment still says real filesystem tests exercise the wx exclusive-create race, although the implementation now uses hard links or directory locks.

Resolved Earlier Findings

• The typed fs-extra mock declares link.
• Hard-link rejection no longer causes unconditional contention.
• Contention tests now model EEXIST from link(tmp, target).
• The previously identified stale implementation comments in src/update.ts and src/__tests__/lock-atomic.test.ts are corrected.

[SaulMoro]

Thanks for this. The temp-write + link create is right, and #761 now does the same. It covers only one of #760's two holes, though:

hole 1  lock exists empty between open and write     → #764 fixes · #761 fixes
hole 2  lock vanishes during the stale check          → #764 still renames over it · #761 retries the create
        (A releases, C re-creates, B renames over C)

Same harness as #760, 16 processes × 2000 attempts, 3 runs:

	#764 (d5a19f6)	#761 (f99f100)
renames over a live lock	3 / 11 / 5	0 / 0 / 0
double holders	1 / 0 / 3	0 / 0 / 0

#761 also treats an EPERM owner (alive, another user) as live, and falls back to wx without hard links instead of a directory lock. I suggest closing this in favour of #761; happy to credit the link approach there.

[dvd233]

Closing this PR in favor of #761 per maintainer feedback. #761 now covers both stale-check re-creation and cross-user owner handling more completely. The changes on this branch were not merged, and no further writes are needed here.

[github-actions]

Findings

• [P1 blocking] src/update.ts:292 — The directory fallback still publishes the directory before its ownership is established. If the first creator’s .owner write starts after the two-second grace period, a reclaimer can remove and recreate the directory; the original creator can then write into that replacement directory, overwrite its .owner, and also return success. The same exclusiveCreate race affects reclaim sentinels.
• [P1 blocking] docs/designs/data-directory-layout.md:121 — The required design documentation was not updated. It still specifies writeFile(..., { flag: 'wx' }), file-only locks, and atomic rename takeover, while the PR introduces hard-link publication, directory locks, and grace-period handling. This violates the repository rule requiring behavior changes to update affected docs/designs/ documentation.
• [P1 blocking] The PR description still lacks the required real-CLI end-to-end verification and Agent/provider Test Plan. --help smoke checks do not exercise lock acquisition through Claude, Codex, CodeBuddy, or OpenCode, nor the git, gitlab, and github providers.
• [P2 non-blocking] src/update.ts:367 — The comments claim every stale-lock takeover finishes with an atomic rename, but the directory branch at src/update.ts:412 removes the old lock and recreates it.
• [P2 non-blocking] src/update.ts:234 — The helper documentation says a missing lock is stale, but the stat catch returns false for ENOENT along with all other metadata errors.

Resolved Earlier Findings

• The typed fs-extra mock declares link.
• Hard-link rejection no longer causes unconditional contention.
• The unsafe wx fallback was removed, although its directory replacement remains racy as described above.
• Transient stat failures no longer mark live locks stale.
• Contention tests now model EEXIST from link(tmp, target).
• The stale O_EXCL test-suite comment was corrected.