[terencecho]

What

Publish a complete share-lock owner record on filesystems that refuse hard links.

Why

The fallback created the final lock file and then wrote the owner. Killing the process between those operations left an empty file that subsequent share and export attempts could never attribute to a dead owner.

Related work

The share-specific change in #7474 also handles hard links refused with EACCES. This PR preserves that behavior using the new fallback.

How

Stage the owner record in a private sibling directory. Keep the regular-file hard-link path where supported. Otherwise, publish the nonempty directory with a single rename at the same lock path, which cannot replace a regular file or a nonempty lock directory.

A contender removes a directory owner only after proving its PID is dead on the same host. The marker name includes a unique token, and cleanup removes only an empty directory, so a delayed reaper cannot remove a replacement owner. Empty legacy files remain untouched because an older process might still write to one. Older clients may report a filesystem error if they encounter an active fallback directory. Windows retries transient rename errors while the lock path is absent, up to the existing acquisition deadline.

The crash-safe fallback and its documentation grew the published share package past both size limits. The share package budget is now 64,831 packed bytes and 257,736 unpacked bytes, using 5% headroom over the previous Linux CI packed size and the updated local unpacked size. The new tests stay outside the tarball. Previous Build result

Test plan

• Reproduced the lockout on the base by fault-injecting hard-link failure and killing a child immediately after lock publication.
• Verified a paused live owner stays exclusive, two contenders serialize after the kill, an empty legacy file stays in place, and an empty fallback directory left during release does not block the next writer.
• 55 share tests passed on Node 22. The workspace build, packed-manifest verification, share typecheck, lint, format and test reachability passed.
• Deterministically replaced a stale owner between the reaper’s read and cleanup. The live replacement survived. Mutation tests caught recursive cleanup, removal of the same-host guard, skipped draft cleanup and removal of Windows retries.
• Injected Windows rename failures with an absent lock path and checked both recovery and the error at an expired deadline.
• Updated the share package README.
• Native Windows and exFAT behavior still needs direct verification. These regressions use real macOS filesystem operations with injected hard-link and rename failures.

[miguel-heygen]

Reviewed at 6e7f572d. There are no other reviews on the PR yet.

The fix is right. Publishing the lock by renaming a directory that already holds the owner record closes the crash window. I checked each crash-safety claim in the body against the code, and #7474's EACCES handling still holds. One required check is red, and one of the body's claims isn't pinned by a test.

What's strong

• projectLock.mjs:143-151: rename(draft, lock) publishes the owner record atomically. On POSIX it can't replace a regular file (ENOTDIR) or a nonempty directory (ENOTEMPTY/EEXIST), and it replaces an empty directory atomically. That makes a lock left empty mid-release harmless without any extra code.
• projectLock.mjs:92-119: release and reaping only ever unlink <token>.json and rmdir. A reaper that read a stale owner can therefore only remove its own target. An unlink of the old marker inside a replacement directory gets ENOENT, and the rmdir gets ENOTEMPTY. If a hard-link owner has taken the path in the meantime, both calls get ENOTDIR. All of these are swallowed.
• tests/projectLock.test.mjs: the paused-child test is a real regression test. I ran it against main's projectLock.mjs and it fails there (2 of the 3 new tests fail), so the "reproduced on base" claim holds.

Blocker

• The required Build check fails at this head. The PR body doesn't mention or explain it. The failing step is verify:packed-manifests (job):

  @hyperframes/share packed size 61743 bytes is over its budget of 60820 by more than 1% / unpacked size 245020 bytes is over its budget of 240537

  • Build passes on the base (7afa05a7), so the growth comes from projectLock.mjs (3,949 → 6,088 bytes) plus the README. The script's own fix is to raise @hyperframes/share in scripts/package-size-budgets.json to "packed": 64831, "unpacked": 257271 (+5% headroom), and to state the reason in the PR body. The reason here is the crash-safe fallback. Everything else that has finished is green: 41 succeeded and 23 skipped. Two Windows engine-cli shards were still running when I posted.

Important

• The claim "cleanup removes only an empty directory, so a delayed reaper cannot remove a replacement owner" has no test. I swapped rmdir(path) for fs.rm(path, { recursive: true }) in removeEmptyDirectory (projectLock.mjs:94), and all 52 share tests still pass. That swap re-opens the exact two-owner race this PR is guarding against.
  • Here is a deterministic test I ran in a child-style script, using syncBuiltinESMExports the way pausedAcquisition does. Put a lock directory at the path holding a dead same-host owner's <token>.json, with fs.link failing with ENOTSUP. Wrap fs.readFile so that the first read of that marker removes the directory and renames in a live owner's directory, as if another reaper and a new owner got there first. Then call withProjectShareLock(root, "review", ..., { timeoutMs: 100 }).
  • At this head it rejects with ShareBusyError and the live owner's marker survives. With the fs.rm mutation it acquires the lock and the live owner's directory is gone. I can share the script if useful.

Nits

• The "same host" half of "dead PID on the same host" is unpinned. If I remove if (owner.hostname !== hostname()) return false; (:54), every test still passes. The foreign-owner case in kernel.test.mjs:198-205 uses process.pid, which is alive anyway. A dead PID with hostname() + "-foreign", in both the file shape and the directory shape, would pin it. This gap predates the PR, but the directory shape is new.
• Nothing checks that the draft is cleaned up after a successful directory-mode run. If I drop releaseDirectory(draft, marker) (:187), the tests still pass. Asserting that .hyperframes holds no *.tmp entry after the serialize tests would pin it.
• The EPERM/EACCES/EBUSY branch (:149) only treats the error as busy when the lock path exists. On Windows exFAT, where this fallback is the only path, antivirus holding a handle on the fresh marker can make rename fail with EPERM/EBUSY while the lock is absent. That throws instead of retrying until the deadline. I haven't verified this, and the PR already lists Windows and exFAT as untested. Retrying these codes until the deadline would be the conservative choice.

How this interacts with #7474 (EACCES)

• EACCES is in the hard-link fallback list (:136), and the kernel loop covers it (kernel.test.mjs:338). Removing EACCES from the list fails exactly the EACCES case. git merge-tree against #7474's head (05bd36e9) merges projectLock.mjs cleanly, with EACCES listed once. kernel.test.mjs hits a one-line conflict on the loop's test title only. Whichever PR lands second should keep the list as is and the new "directory fallback" title. #7474's other sites (core atomicFile, studio-server ownerLock) aren't touched here.

Other claims checked

• Unique-token marker: the reaper releases only <token>.json and checks that the name matches the record's token (:117). If I give every owner a constant marker name, the paused-owner test fails.
• Legacy empty files untouched: reapFile still returns early when there's no valid owner. A mutation that reaps ownerless files fails the new legacy test. A new contender's link gets EEXIST, and its rename onto the file gets ENOTDIR. The lock then waits and ends in ShareBusyError, which matches the README's manual-recovery wording.
• Mixed versions: an older client that meets a held directory gets EEXIST from its link or open. Its ownerAt then throws EISDIR, so it surfaces an error instead of stealing the lock. That matches the body and README. Nothing outside packages/share reads review.lock or review-export.lock. I grepped this repo and hyperframes-internal's main.
• Mutation results: the tests caught a marker written after publish, a skipped directory release, a check that treats every owner as dead, dropped EACCES, and reaping of ownerless files.

Verdict: REQUEST CHANGES
Reasoning: The code is correct, every crash-safety claim I traced holds, and #7474's EACCES handling still works. But the required Build check is red because of this PR's own size growth. The fix is a budget bump with a reason in the body, and I'd approve after that. The delayed-reaper test is strongly recommended but isn't gating.

[terencecho]

Addressed the package-size failure and the lock regressions in 7b311d19. I raised only the share package budget for the crash-safe fallback and documented the measurements in the PR body. The workspace build and packed-manifest verification passed locally.

The delayed-reaper test swaps in a live owner after the stale owner is read. It fails if cleanup is changed to recursive removal. The new tests also cover a foreign owner with a dead local PID in both lock formats, draft cleanup after failed acquisition, and transient Windows rename errors within the existing deadline. All 55 share tests passed on Node 22. The Windows cases use fault injection on macOS. Review

[miguel-heygen]

Re-review at 7b311d19bd3df14483ba5d7a91bd785447c4f706. Everything from my earlier review at 6e7f572d is addressed.

Blocker resolved

• scripts/package-size-budgets.json now sets @hyperframes/share to 64831/257736, and the PR body gives the reason: the crash-safe fallback and its docs. The unpacked figure differs a little from the one I suggested because it uses the updated local size, which the body explains. Build is green at this head, and so is every other required check (all 68 check runs finished: 46 success, 22 skipped, none failing; "Studio: edit accuracy gate" and "Skills: manifest in sync" skip by path).

Important resolved

• tests/projectLock.test.mjs, "a delayed reaper cannot remove a replacement owner's directory". I swapped rmdir for fs.rm(..., { recursive: true }) in removeEmptyDirectory, and this test fails while the rest pass. The test gets at the module's named readFile import through syncBuiltinESMExports, and that mutation result confirms the hook really fires.

Nits resolved

• Same-host check: the foreign-owner test now uses a PID that has exited, in both the file shape and the directory shape. Dropping the hostname() guard fails it.
• Draft cleanup: the legacy-file test now asserts that .hyperframes holds only review.lock. Dropping releaseDirectory(draft, marker) fails it.
• Windows rename retry (projectLock.mjs:154-158): the retry runs only on win32, only while the lock path is absent, and only until the existing deadline. After the deadline it rethrows the original error. Removing the retry fails the new test, and so does ignoring the deadline or treating every EPERM/EACCES/EBUSY as busy.

Verified locally

• 55/55 share tests pass on Node 22.
• Mutation run at this head: 14 of 17 caught. The three that survive can't change behaviour on Linux, and none is a gap:
  • Dropping the marker-name check: the unique token makes it redundant.
  • Dropping the empty-directory reap: on POSIX, rename replaces an empty directory anyway.
  • Reaping a directory with two or more entries: this code never creates one.
• #7474 overlap is unchanged. projectLock.mjs merges cleanly, with EACCES listed once. kernel.test.mjs hits a one-line conflict on the test title only, and whichever PR lands second should keep the "directory fallback" title.

As the body says, native Windows and exFAT still need a real-volume check after merge. The retry logic is deadline-bounded, so the worst case is the original error at the deadline, not a hang.

Verdict: APPROVE
Reasoning: The crash-safety claims hold and are now each pinned by a test that fails under the matching mutation. #7474's EACCES handling still works, and the required checks are green at this head.