fix(cli): recover stale config locks

[h1-hunt]

Summary

• serialize concurrent config read/modify/write operations so independent wallet and session updates are preserved
• recover same-host locks whose recorded owner PID is proven dead on Linux and macOS
• serialize stale-lock reclamation with a crash-released exclusive advisory guard and revalidate the canonical owner under that guard
• preserve ownerless, malformed, foreign-host, live-PID, and unverifiable locks with actionable manual recovery guidance
• keep normal lock acquire/release independent from the native guard so guard import/runtime failures cannot strand successful writes
• document the narrow shared-hostname/PID-namespace safety envelope
• issue closure: closes fix(cli): recover from stale config locks after an interrupted write #72

Technical notes

• lock owner metadata is versioned and records PID, hostname, creation time, and a random ownership token
• same-host process.kill(pid, 0) liveness treats only ESRCH as proof of death; PID reuse degrades conservatively to manual recovery
• contenders use fs-native-extensions only after observing a reclaimable dead owner
• while holding .config.lock.guard, reclamation re-reads .config.lock/owner.json and removes the canonical lock only when the token is unchanged and still dead
• production reclamation never renames the canonical lock path, so a stale observer cannot displace a live successor and expose a gap to a third writer
• native import and tryLock() runtime failures preserve the stale lock and surface manual recovery guidance
• normal owner release remains a direct ownership-token check and is not coupled to native guard availability
• the persistent guard file contains no owner data, is validated as a file on every platform, and is tightened to mode 0600 on POSIX; the kernel lock is released on descriptor close/process death
• ownerless legacy locks are intentionally not age-reclaimed because ownership cannot be proven

Verification

• deterministic stale-observer A → live successor B → third writer C regression
• simultaneous dead-owner reclaimers preserve all independent updates
• real lock-holder hard-kill recovery
• real reclamation-guard-holder SIGKILL blocks before kill and recovers after descriptor close
• native module import failure preserves the lock with manual guidance
• native tryLock() runtime failure preserves the lock without affecting normal release
• ownerless, foreign-host, and live-PID locks are never auto-deleted
• focused config/guard suites: 28/28
• full suite: 182/182 (168 CLI + 14 core)
• typecheck, lint, diff check
• production dependency audit: 0 vulnerabilities
• build, pack verification, clean tarball install smoke

No production config, wallet signing, provider call, or payment was used during verification.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f2c95a44a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Guard owner-write cleanup against successor locks

If this process is paused after mkdir(lockDir) but before owner.json is written for longer than the legacy grace period, another contender can classify the metadata-less directory as stale, remove it, and create a successor lock. When this original process resumes, writeFile(..., flag: "wx") can fail with EEXIST against the successor's owner file, and this cleanup removes the successor lock unconditionally, breaking mutual exclusion and allowing overlapping config writes. The cleanup needs to verify it is still deleting the directory it created before removing it.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Expire abandoned reclamation claims

When a process is killed after creating .config.lock.reclaim-* but before the finally releases it, every later attempt to reclaim the same stale lock hits this deterministic claim path and returns false forever, so the dead config lock times out instead of recovering automatically. Since the change is specifically handling hard-killed writers, the claim file needs its own stale-owner handling or the stale lock remains unreclaimable after a killed reclaimer.

Useful? React with 👍 / 👎.

[h1-hunt]

Review follow-up

Addressed the abandoned reclamation-claim finding in commit 2c35cc9.

• Replaced identity-specific claim files with a persistent kernel-backed operation guard.
• The guard is automatically released on file-descriptor close or process death, so a hard-killed reclaimer cannot leave another permanent lock.
• Reclamation and ownership-checked release now use the same guard and revalidate the current lock identity before removal.
• Added a child-process regression that kills the guard holder and proves the next writer reclaims the dead config owner and completes successfully.

Verification: targeted config tests, full test suite, typecheck, lint, build, pack verification, clean-pack smoke test, and production dependency audit all pass.

[sebayaki]

Opus review — needs-rework

The lock protocol itself is carefully done (kernel-guarded reclaim serialization, token-checked release, re-inspect under guard closes the TOCTOU, strong regression matrix including the SIGKILL'd guard-holder test), and CI is green. Rejecting on platform coverage and proportionality, not on a correctness bug:

1. No automatic recovery on macOS/Windows — the filed failure stays permanent on primary platforms (blocking)

inspectLockOwner() can only prove an owner dead via the Linux identity matrix — readLinuxHostIdentity() returns undefined off Linux, so every dead-owner lock on macOS resolves to "no process identity that can be verified on this platform" → reclaimable: false → manual rm guidance. Accordingly, all three dead-owner recovery tests are it.skipIf(process.platform !== "linux").

macOS is one of the two supported wallet/signing platforms. Issue #72's scenario — crash mid-write → every later auth/wallet config write blocked — still ends in a hard failure there; on macOS this PR delivers a better error message, not recovery. The acceptance criterion "Reclaim an abandoned lock safely without deleting a live process's lock" is met only on Linux.

A safe dead-check that covers both platforms needs no native code: same-hostname + process.kill(pid, 0) → ESRCH proves no process with that PID exists now. A live owner can never yield ESRCH, so the unsafe direction is impossible; PID reuse merely degrades to today's conservative manual path. Keep the Linux boot-id/PID-namespace/start-ticks checks as a refinement on top if you want the extra precision there — but macOS must recover automatically for this to close #72.

2. A native runtime dependency + ~330-line bespoke protocol is disproportionate for ~/.h402/.config.lock (blocking)

fs-native-extensions (with transitive bare-addon-resolve/require-addon/which-runtime from the Holepunch ecosystem) becomes a production dependency of the npm-published CLI, solely so concurrent reclaimers/releasers serialize on a flock guard. The CLI already carries a documented native-binding platform matrix for OWS (#71); adding a second native addon on the config path compounds that surface for a corner case.

The race the guard closes (two contenders observing the same dead owner; reclaim-vs-release) can be closed dependency-free with an atomic claim: rename(lockDir, lockDir + ".reclaim-" + token) — exactly one contender wins the rename (losers get ENOENT), re-verify the renamed dir's owner.json still matches the observed dead owner, then rm (rename back on the rare mismatch). The config.test.ts assertions about .config.lock.reclaim-* leftovers suggest this shape was already contemplated. Alternatively, mtime-plus-refresh staleness (the proper-lockfile model) is the battle-tested answer for sub-second writes. Either direction removes the dependency, the .d.ts shim, and most of the protocol while extending coverage to macOS.

Non-blocking notes

• If release() cannot acquire the operation guard within ~5s, it throws after the config write already succeeded and leaves its own live-owner lock behind — a new way to strand a lock, introduced by the recovery mechanism itself. A rename-claim design has no such path.
• ~/.h402/.config.lock.guard is created and never removed (asserted in tests) — a permanent artifact in the config dir.
• Keeping the ownerless/legacy lock un-reclaimable is a defensible stance, but note it means the literal reproduction in fix(cli): recover from stale config locks after an interrupted write #72 still fails (with guidance) after this PR — worth stating explicitly in the PR body.

Rework direction: keep the v2 owner metadata, token-checked release, and the test matrix (un-skipped on macOS); replace kernel-flock + Linux-only identity with hostname + ESRCH liveness (optionally retaining the Linux start-ticks refinement) and an atomic rename-claim for deletion; drop fs-native-extensions.

[clawd800]

GPT review — changes requested

Reviewed head 2c35cc99c2dee77be137044e83bec76baef01397.

1. [P1] Runtime native-lock failures can leave a successful config write permanently locked. loadNativeLockApi degrades only when the addon import fails. If the addon imports but tryLock() throws (for example, an unsupported filesystem/locking backend), the release path propagates after config.json has already been written and leaves .config.lock/owner.json in place. An injected ENOTSUP-style failure reproduced exactly that outcome and blocked subsequent updates. Runtime locking failures need deliberate fallback and handle cleanup as well: reclamation should preserve the lock with manual guidance, while normal owner release must not strand a successful write.

2. [P2] Automatic stale-owner recovery remains unavailable on macOS. Only Linux process identity is recorded, and every non-Linux owner is classified as unverifiable. On macOS, killing a real lock holder with SIGKILL caused the next saveConfig() to time out after about 5.45 seconds and require manual deletion. macOS is a supported CLI platform, but all automatic-recovery tests are Linux-only and CI runs only Ubuntu. Please either use the crash-released kernel lock as the primary writer lock on Darwin or record a verifiable Darwin process identity, with a macOS recovery regression.

The two older Codex findings are addressed at this head: ownerless locks are no longer auto-reclaimed, and the fd-scoped operation guard no longer leaves a permanent claim when a reclaimer dies. CI, CodeQL, build, lint, packaging checks, and existing tests pass.

[sebayaki]

Review follow-up

Reworked in commit feb8611 to address all three review threads (Opus ×2, GPT ×2); the PR body is updated to describe the new design.

1. macOS now recovers automatically (Opus Restructure into @h402/core + @h402/cli npm workspaces #1, GPT P2). Owner liveness is same-hostname + signal-0 ESRCH — platform-independent, no native code. The dead-owner recovery, hard-killed-holder, foreign-host, live-PID, and claim-sweep regressions run on every platform (verified locally on macOS: 25/25) instead of being skipIf(!linux). The hard-killed real-lock-holder test is the macOS regression GPT asked for.

2. fs-native-extensions is gone (Opus Add release packaging guard so npm tarballs always include dist #2), along with the .d.ts shim, the guard file, and ~110 lines of protocol. Reclaim serialization now uses an atomic rename claim: exactly one contender wins the rename, the claimed dir's token is re-verified before deletion, and a lock recreated in the gap is renamed back instead of deleted. Abandoned claims are inert (never the lock path) and swept by age on the next acquire — the crash-abandoned-claim concern from the earlier Codex round stays closed.

3. The runtime native-lock failure mode no longer exists (GPT P1) — there are no native lock calls left, so a successful write can no longer end in a stranded lock via a throwing tryLock(); release is back to the plain token-checked refusal.

Safety envelope note: a PID reused by another process or a foreign hostname degrades to the conservative manual-guidance path, never to deletion; a sibling container sharing both this config dir and this hostname across PID namespaces is documented as out of envelope (previously covered by the Linux identity matrix, at the cost of Linux-only recovery).

Verification: npm test (config suite 25/25 on macOS incl. previously-skipped recovery paths), npm run typecheck, npm run lint, npm run build all pass; CI runs the same plus pack checks on Linux.

[h1-hunt]

Final review

Verdict: REJECT — blocking mutual-exclusion race

Reviewed head feb8611a407d2a33486382db0584f5de54b143f9 against current main 2d335d93a0c487da00db26b8c1a0fdc1be318d84.

Blocker

packages/cli/src/config-lock.ts:116-137 — a stale reclaimer can move a live successor lock out of the lock path, allowing a third writer to enter concurrently.

The post-rename token check prevents deletion of a successor, but it happens only after rename(lockDir, claimDir) has already displaced that successor. A valid interleaving is:

1. R1 and R2 both inspect dead lock A and remember token A.
2. R1 renames/removes A, then acquires successor lock B and enters its config critical section.
3. R2 resumes with its stale observation and successfully renames B to its claim path.
4. R2 sees token B !== token A, but before it can rename B back, .config.lock is absent.
5. Writer C creates .config.lock, acquires lock C, and enters while B's holder is still active.
6. R2 cannot restore B because C now occupies the path; B remains displaced and B's eventual release fails.

I reproduced this deterministically in a disposable worktree by adding pause hooks around the existing rename/token-check sequence. The observed result was overlap: true: the lock path was absent after B was displaced, C acquired a different owner token while B's release handle was still live, and B later failed release with owner metadata is missing or invalid.

The current 8-writer regression does not force this stale-observer/successor interleaving. Please use a reclamation protocol that cannot move an unverified successor out of the coordination path (or otherwise keeps reclamation serialized continuously), and add a deterministic three-party regression covering A → B displacement → C acquisition.

Verification performed

• Fresh merge into current main: clean
• Config suite: 25/25 passed
• npm run typecheck: passed
• npm run lint: passed
• npm run build: passed
• npm run verify:pack: passed
• GitHub CI and CodeQL: green
• Deterministic stale-observer/successor race probe: reproduced overlap

No production config, wallet, paid call, or provider call was used.

[h1-hunt]

Review follow-up

Addressed the deterministic stale-observer → live-successor → third-writer race in commit a018104.

• Production reclamation no longer renames or otherwise removes an unverified canonical lock path.
• Dead-owner reclamation is serialized by a crash-released exclusive advisory guard. After acquiring it, the contender re-reads .config.lock/owner.json and removes the lock only when the token is unchanged and the owner is still proven dead.
• The native guard is used only for stale reclamation. Normal uncontended acquire/release does not depend on it, so an import or runtime lock failure cannot strand a successful config write.
• Native import and tryLock() runtime failures fail conservatively: the stale lock is preserved and the timeout retains manual recovery guidance.
• Added deterministic A → B → C ordering coverage, native import/runtime failure coverage, and a real child-process regression proving the guard blocks reclamation, is released by SIGKILL, and then allows recovery.

Verification:

• focused config/guard suites: 28/28
• full suite: 182/182 (168 CLI + 14 core)
• typecheck, lint, and diff check
• production dependency audit: 0 vulnerabilities
• build, pack verification, and clean tarball install smoke
• independent concurrency review: pass

fs-native-extensions@1.5.0 ships Linux and macOS x64/arm64 prebuilds. The persistent guard path is validated as a file on every platform and tightened to mode 0600 on POSIX; descriptor close releases its advisory lock even when a process is killed. No production config, wallet signing, provider call, or payment was used.

[h1-hunt]

GPT Reviewer

Verdict: PASS

Reviewed head a018104398a2688cf1e638f4073f815fca0b9204 against current base 2d335d93a0c487da00db26b8c1a0fdc1be318d84 using a bounded, tool-free review bundle.

No actionable findings. Stale-lock reclamation is serialized and revalidated under the advisory guard, normal acquisition and release remain independent of native-guard failures, and the supplied validation passes.

Findings

None.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.769s)
• typecheck: PASS (3.687s)
• lint: PASS (2.845s)
• test: PASS (18.301s)
• build: PASS (1.922s)
• No paid h402/provider calls were made.

[h1-hunt]

Opus Reviewer

Verdict: PASS

Reviewed head a018104398a2688cf1e638f4073f815fca0b9204 against current base 2d335d93a0c487da00db26b8c1a0fdc1be318d84 using a bounded, tool-free review bundle.

This PR replaces the pure-mkdir config lock with a crash-recovering protocol: versioned owner metadata (PID/hostname/createdAt/token), same-host process.kill(pid,0) ESRCH liveness, and stale-lock reclamation serialized by a crash-released fs-native-extensions advisory guard on a persistent .config.lock.guard file. The final commit (a018104) specifically fixes the deterministic stale-observer→live-successor→third-writer race that blocked the prior dependency-free version.

I verified the fix is sound. Mutual exclusion rests on atomic mkdir; reclamation deletes a lock only after (a) acquiring the exclusive guard and (b) re-reading owner.json under the guard and confirming the token is unchanged and the PID is still dead. Because the canonical path is never renamed/moved, a stale reclaimer can never displace a live successor B: on re-inspect it observes token-B (live PID → refuse), B's ownerless init window (ownerless is never reclaimable → refuse), or an absent lock (→ refuse). I traced each interleaving (config-lock-race.test.ts models exactly A→B→C) and found no path that admits overlapping writers or wrongly deletes a live/successor lock.

Prior review threads are all resolved: Codex P1 (owner-write cleanup deleting a successor) is prevented by removing age-based ownerless reclamation; Codex P2 (abandoned claim files) is gone since the guard is kernel-released on descriptor close/SIGKILL; GPT P1 (native runtime failure stranding a write) cannot occur because normal acquire/release performs no native calls and the guard is used only for reclamation — import/tryLock failures preserve the stale lock with actionable manual guidance and leave the token-checked release intact. Token-checked release cannot delete a successor (throws instead). PID-reuse, foreign-host, live-PID, and unverifiable owners all degrade conservatively to manual recovery (the unsafe direction — declaring a live process dead — is impossible under ESRCH semantics). The guard fd lifecycle closes the handle on every non-acquired branch (no leaks).

Observations considered and deemed non-actionable: the persistent guard file is a correctness requirement (unlinking it would reintroduce an inode-swap race), not a stray artifact; the native dependency is install-safe (no build/install scripts; runtime addon resolution with graceful degradation; 0 vulnerabilities) and justified over the proven-racy rename-claim alternative, scoped strictly to reclamation; one dead-owner test omits skipIf(win32), but Windows is outside the supported/CI platform matrix and the config-write paths require OWS bindings that also exclude it. No stdout/stderr or JSON error-contract regressions (errors propagate as normal Errors with actionable text; no direct console writes). Shared validation is fully green (typecheck, lint, 168 CLI + 14 core tests, build, 0-vuln audit). No actionable findings.

Findings

None.

Shared exact-snapshot validation

• diff-check: PASS (0.0s)
• npm-ci: PASS (1.769s)
• typecheck: PASS (3.687s)
• lint: PASS (2.845s)
• test: PASS (18.301s)
• build: PASS (1.922s)
• No paid h402/provider calls were made.