fix(lock): keep callback errors out of acquisition retries

[steipete]

Summary

Keep application callback failures out of lock-acquisition retry handling. A saved filesystem Error could retain its old failure brand: throwing that same object later from parsePayload on the same path caused repeated parser calls, and replaying a historical Root-open error could turn the original error into file_lock_timeout even while a foreign sidecar remained present.

This is the focused follow-up to the late replay finding from PR #189. The reproduced baseline is main commit 59b9cc4ea1b12defd7e693aa3af8d56581d52e9f; the proof uses unpublished main-snapshot packages, not a claim about every released version. No lock takeover or deletion of foreign bytes was demonstrated by these reproductions.

Production delta: +100 / −56 / net +44 lines. No new dependency, public API, version change, or archive change.

What changed and why

Payload evaluation, JSON serialization (including toJSON), and parsing now run outside the acquisition I/O retry catches. Shared raw snapshot readers finish descriptor cleanup before invoking application parsing. Actual open failures set attempt-local denial flags rather than conferring reusable authority on an Error object.

Root identity, opened-path resolution, open, and unlink evidence now belongs to an observation-local receipt set carried by AsyncLocalStorage. Each Root observation gets a fresh set, including nested and concurrent observations. The permanent Error WeakMap/WeakSet brands are removed; an error saved from another observation cannot supply current open/unlink evidence.

The safety boundary is unchanged: a genuine current Windows lock-file open denial can still retry at most eight times within caller retry/deadline limits, returning the original denial on exhaustion. Every retry still requires fresh exclusive creation. Generic Root reads, parent/descriptor/ancestry checks, creator-byte/token admission, and identity-conditioned release/reclaim remain strict. Historical exception identity grants no ownership or removal authority.

Executed before/after proof

The same frozen public-consumer scripts were run against baseline and candidate packages. Both were physically installed with npm, with no source/workspace symlinks. Candidate root tarball integrity:

sha512-HJxy8iAGAjmrT7S/VuyHoaoOBnGacZ9H7Ag226Z+VN8wQOy3uINzGC4i97qJ0+7pNfcRfYK0NoxSG2Dg121XOQ==

Native Windows, Node 24.20.0: the parser control injects an EPERM at the Node open call to retain a genuine operation-classified Error, then rethrows that object from application parsing. This is explicitly controlled open-error injection, not a claim of a naturally timed Windows denial; Windows execution itself is real (platformOverride:false). With two retries, same-path parsing changed from three calls to one, for both sync and async acquisition. Fresh matching errors and different-path replays remained one call. The exact original Error and foreign bytes were preserved, and no handle was returned.

Before:

{"surface":"sync","control":"same-path-replayed-error","parserCalls":3,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"async","control":"same-path-replayed-error","parserCalls":3,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"proof":"read-only-error-replay","actualPlatform":"win32","node":"v24.20.0","platformOverride":false,"injectedOpenError":true,"cases":6,"passed":false}

After:

{"surface":"sync","control":"fresh-matching-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"sync","control":"same-path-replayed-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"sync","control":"other-path-replayed-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"async","control":"fresh-matching-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"async","control":"same-path-replayed-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"surface":"async","control":"other-path-replayed-error","parserCalls":1,"caughtOriginalError":true,"acquired":false,"foreignBytesPreserved":true,"asyncHeldEntries":0}
{"proof":"read-only-error-replay","actualPlatform":"win32","node":"v24.20.0","platformOverride":false,"injectedOpenError":true,"cases":6,"passed":true}

Root historical-evidence control: a real earlier missing-file error from public Root.open() is replayed through a wrapper around that public method after creating a foreign sidecar. No filesystem error or platform is injected in this control. The old code discarded the observation and changed the error to a timeout; the candidate propagates the same original error, leaves the manager empty, and preserves foreign bytes through drain. Fresh/cross-path errors are controls. A separate real unlink immediately before the current open still recovers through a second, fresh exclusive creation, verifies ownership, and releases successfully.

Actual Windows output:

{"proof":"root-observation-replay","platform":"win32","node":"v24.20.0","case":"same-path","callbackCalls":1,"originalErrorPreserved":false,"code":"file_lock_timeout","heldRegistrations":0,"foreignBytesPreserved":true}
{"proof":"root-observation-replay","platform":"win32","node":"v24.20.0","case":"same-path","callbackCalls":1,"originalErrorPreserved":true,"code":"not-found","heldRegistrations":0,"foreignBytesPreserved":true}
{"proof":"root-observation-replay","platform":"win32","node":"v24.20.0","replayScheduling":"public-Root-open-wrapper","injectedFilesystemErrors":false,"cases":4,"currentLossRecoveredByFreshCreate":true,"passed":true}
{"proof":"windows-installed-replay-candidate","tree":"ea9a84e961f3d9caa88c4c30e046ea243d7c6d47","parserCases":6,"rootCases":4,"parserCalls":1,"originalErrorsPreserved":true,"foreignBytesPreserved":true,"currentMissingOpenRecoveryPassed":true,"platformOverride":false,"passed":true}

The identical six-parser/four-Root controls also passed candidate consumers on macOS with Node 22.23.2 and 24.20.0, after reproducing baseline failures. The Mac parser controls explicitly simulate Windows classification; the Root controls do not. The Linux installed-consumer parser gate also passed.

Validation

• New regression RED proof on unchanged base: 11 callback cases failed / 28 passed; all four new Root history/isolation cases failed.
• Frozen candidate pnpm check: 6,317 passed / 81 skipped, 179 test files passed / one skipped, including build, documentation and package checks.
• pnpm native:test: 63 passed; pnpm test:security: 78 passed.
• Native Windows focused callback/Root suite: 113 passed / 33 skipped, with the existing platform-specific skips reported rather than counted as passes.
• node scripts/sidecar-contention-proof.mjs off, auto, and require: 1,200 contended acquisitions passed on Linux, with release-retry controls.
• pnpm package:smoke: passed root-only npm/pnpm resolution, missing-binary and omitted-optionals cases. Foreign platform-filtering fixtures are synthetic, not foreign execution proof.
• Isolated Codex autoreview: scoped-clean at the configured P0 threshold, no accepted/actionable findings.
• git diff --check: passed.

An initial wrong test-helper import was corrected before the valid RED run. Five unchanged 5-second Root tests then timed out on the loaded Mac; they were not counted as passing. The same frozen candidate passed the isolated Linux full gate and Windows focused gate. No test deadline or invariant was relaxed.

Validated tree: ea9a84e961f3d9caa88c4c30e046ea243d7c6d47; head: d9d1a2c4211c07d24a14945bd419b4c4131b59f1. Hosted exact-head CI and review must pass before merge; this body does not claim pending hosted jobs have passed.

Release boundary

Only the current 0.7.1 Unreleased notes change. Package versions remain 0.7.0; dependency lockfiles, dated 0.7.0 notes, tags, npm packages, and GitHub Release artifacts are untouched. This PR does not publish a release.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: passed. Reviewed September 1, 2026, 5:05 AM ET / 09:05 UTC.

ClawSweeper review

What this changes

The PR prevents lock payload and parser callback errors from entering filesystem retry handling, and makes Root failure receipts local to one sidecar-lock observation.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open: this is a focused, unique fix not present on current main, with no actionable correctness or security finding in the introduced patch.

Priority: P2
Reviewed head: d9d1a2c4211c07d24a14945bd419b4c4131b59f1

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Strong installed-consumer proof and focused regression coverage support a clean, narrowly scoped lock-safety correction.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The changed production owners are the async/sync acquisition retry paths and Root observation wrapper; supplied installed-consumer Windows before/after traces at the reviewed head exercise replayed errors, retain the original error and foreign bytes, and show the intended recovery only after a fresh create.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owners are the async/sync acquisition retry paths and Root observation wrapper; supplied installed-consumer Windows before/after traces at the reviewed head exercise replayed errors, retain the original error and foreign bytes, and show the intended recovery only after a fresh create.
Evidence reviewed	5 items	Callback failures are outside retry catches: Both async payload serialization and parser invocation occur outside the acquisition I/O catch; only an attempt-local flag from the actual lock-file open permits a denial retry. Root receipts are observation-local: A fresh observation map encloses Root.open, and the subsequent decision reads only that map rather than reusable Error-object brands. Focused regression coverage: The new tests cover replayed payload, serialization, and parser errors across sync, async, and Root modes, plus sequential, nested, and interleaved Root observations.
Findings	None	None.
Security	None	None.

How this fits together

Sidecar locks store a small lock record beside a protected target. Acquisition either creates that record or safely observes a competing one, applying bounded retry only to verified filesystem failures before returning a release handle.

flowchart LR
  A[Caller payload or parser] --> B[Sidecar lock acquisition]
  B --> C[Create or observe lock record]
  C --> D[Root identity verification]
  D --> E[Retry only verified I/O denial]
  E --> F[Lock handle or original error]

Loading

Before merge

• Complete next step (P2) - No discrete repair remains for automation; the PR is already the focused implementation and can use its exact-head automerge path.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and regression delta	production +100/-56; tests +183; docs +7	The production receipt refactor is accompanied by focused replay and concurrency regression coverage.

Technical review

Best possible solution:

Land the exact reviewed head through the normal exact-head merge gates, retaining the scoped-receipt design and its replay regressions.

Do we have a high-confidence way to reproduce the issue?

Yes. The supplied exact-base installed-consumer traces reproduce callback replay and historical Root-error reinterpretation, with focused regressions covering the same paths.

Is this the best way to solve the issue?

Yes. Moving application callbacks outside retry catches and binding filesystem receipts to one observation removes reusable error authority while preserving retries for freshly observed lock-file denials.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 59b9cc4ea1b1.

Labels

Label changes:

• add status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The changed production owners are the async/sync acquisition retry paths and Root observation wrapper; supplied installed-consumer Windows before/after traces at the reviewed head exercise replayed errors, retain the original error and foreign bytes, and show the intended recovery only after a fresh create.
• remove status: 👀 ready for maintainer look: Current PR status label is status: 🚀 automerge armed.

Label justifications:

• P2: This corrects a bounded but security-sensitive lock-acquisition error path without evidence of an active broad outage.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Sufficient (terminal): The changed production owners are the async/sync acquisition retry paths and Root observation wrapper; supplied installed-consumer Windows before/after traces at the reviewed head exercise replayed errors, retain the original error and foreign bytes, and show the intended recovery only after a fresh create.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owners are the async/sync acquisition retry paths and Root observation wrapper; supplied installed-consumer Windows before/after traces at the reviewed head exercise replayed errors, retain the original error and foreign bytes, and show the intended recovery only after a fresh create.

Evidence

What I checked:

• Callback failures are outside retry catches: Both async payload serialization and parser invocation occur outside the acquisition I/O catch; only an attempt-local flag from the actual lock-file open permits a denial retry. (src/sidecar-lock-acquire.ts:158, d9d1a2c4211c)
• Root receipts are observation-local: A fresh observation map encloses Root.open, and the subsequent decision reads only that map rather than reusable Error-object brands. (src/sidecar-lock-root.ts:38, d9d1a2c4211c)
• Focused regression coverage: The new tests cover replayed payload, serialization, and parser errors across sync, async, and Root modes, plus sequential, nested, and interleaved Root observations. (test/file-lock-error-replay.test.ts:56, d9d1a2c4211c)
• Baseline behavior and ownership history: The merged baseline retained open, resolution, and unlink evidence in module-level WeakMap/WeakSet state; the current PR replaces those reusable brands. The immediately preceding merged lock work introduced the Root sidecar helper. (src/opened-file-failure.ts, 59b9cc4ea1b1)
• Real behavior proof: The supplied exact-head PR body includes installed-consumer before/after Windows traces showing replayed parser errors fall from three calls to one and a historical Root error propagates unchanged while foreign lock bytes remain intact. (d9d1a2c4211c)

Likely related people:

• Peter Steinberger: Raw commit 59b9cc4 adds src/sidecar-lock-root.ts:34 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 59b9cc4ea1b1; files: src/sidecar-lock-root.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-01T08:53:43.665Z sha d9d1a2c :: needs maintainer review before merge. :: none

[steipete]

@clawsweeper automerge

Special instructions:
Land only PR 190 at exact reviewed head d9d1a2c4211c07d24a14945bd419b4c4131b59f1 through normal merge gates. Maintainer review is complete: the durable exact-head review has sufficient installed-consumer proof and no actionable code/security findings, and the complete hosted CI/coverage/native/package/audit matrix is green.

The main body contains the real Windows before/after callback and Root-observation traces, unchanged Node 22/24 consumer controls, preserved foreign bytes, and a genuine current-loss control that still requires fresh exclusive creation. Frozen-tree Linux validation passed 6,317 tests, 63 Rust tests, 78 security tests, package smoke, and 1,200 contended acquisitions. Windows focused validation passed 113 tests with platform skips explicitly reported. Codex autoreview is scoped-clean at the configured threshold; the earlier local Root timeouts were resolved by unchanged-source isolated proof, not increased deadlines.

Do not change source, rebase, bypass checks, start unrelated work, bump versions, create tags, publish packages, or modify 0.7.0 artifacts. If the head changes or a gate regresses, stop rather than merging a different candidate. This authorizes landing the existing reviewed follow-up only; release remains separate.

[clawsweeper]

🦞✅
ClawSweeper merged this PR after the passing review.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=d9d1a2c4211c07d24a14945bd419b4c4131b59f1)
Merge status: merged by ClawSweeper automerge
Merged at: 2026-09-01T09:07:19Z
Merge commit: 790472f73bb4

What merged:

• The PR prevents lock payload and parser callback errors from entering filesystem retry handling, and makes Root failure receipts local to one sidecar-lock observation.

Automerge notes:

• No ClawSweeper repair was needed after automerge opt-in.

The automerge loop is complete.

Automerge progress:

• 2026-09-01 09:01:20 UTC review queued d9d1a2c4211c (queued)

• 2026-09-01 09:06:15 UTC review passed d9d1a2c4211c (structured ClawSweeper verdict: pass (sha=d9d1a2c4211c07d24a14945bd419b4c4131b5...)

• 2026-09-01 09:07:21 UTC merged d9d1a2c4211c (merged by ClawSweeper automerge)

[steipete]

Landed as 790472f73bb499ec8eb0ee7a823de1e98b0a94c0. The merged tree exactly matches reviewed/tested tree ea9a84e961f3d9caa88c4c30e046ea243d7c6d47; the maintainer worktree is synchronized to clean main.

The installed-consumer before/after traces in the main body verify the changed effect directly: saved parser errors fall from three calls to one, historical Root errors retain their original identity instead of becoming timeouts, foreign sidecar bytes survive, and a genuine current-open loss still recovers only through fresh exclusive creation. These passed on real Windows and on Node 22/24 Mac consumers; synthetic Windows classification and controlled open-error injection are explicitly labeled where used. The focused Windows suite passed 113 tests, with 33 existing platform-specific skips reported.

The frozen Linux gate passed pnpm check (6,317 passed / 81 skipped), pnpm native:test (63 passed), pnpm test:security (78 passed), pnpm package:smoke, all three modes of scripts/sidecar-contention-proof.mjs (1,200 contended acquisitions), and the installed replay gate. Final CI, coverage, and CodeQL passed on exact head d9d1a2c4211c07d24a14945bd419b4c4131b59f1. Codex autoreview was scoped-clean, and exact-head PR review found no actionable code/security issue. Earlier Mac Root-test timeouts were resolved by unchanged-source isolated proof; no test deadlines or safety invariants were relaxed.

Production delta is net +44 lines (+100/-56). Both task-owned proof VMs are released. Public APIs, dependencies, package versions, and published 0.7.0 artifacts remain unchanged; this is an unreleased source fix, not a package release.