fix(lock): treat compromise-check I/O failure as a lost lock

[SebTardif]

What Problem This Solves

Fixes an issue where consumers using acquireFileLock / createSidecarLockManager with compromiseCheckIntervalMs and onCompromised would never learn that the lock could no longer be verified when the sidecar snapshot read failed with a filesystem I/O error (for example EIO). The interval promise had no rejection handler, so Node emitted an unhandled rejection on every tick and the compromise callback never ran.

This is the async sidecar lock surface (periodic ownership check), not path confinement or archive extraction.

Why This Change Was Made

The interval now maps verifyStillHeld() rejection to "not held" and fires onCompromised once, then stops the timer. That matches the existing fail-closed identity policy: if the library cannot confirm the sidecar still matches the acquisition snapshot, treat the lock as lost. Public verifyStillHeld() still rejects so callers who check it themselves see the I/O error. No API shape change.

User Impact

Holders that already register onCompromised now get that callback when a periodic snapshot read fails, instead of an unhandled rejection storm and a silent detector. Transient disk errors can produce a false-positive compromise signal; that is detection only, not automatic revocation of in-flight work. The callback still fires at most once.

Evidence

Live node against built dist/file-lock.js on origin/main vs this branch. Same script: acquire a lock with a 20ms compromise interval, then make verifyStillHeld() reject with EIO.

Before (origin/main f748844):

$ node /tmp/proof-fs-safe-f001.mjs /tmp/fs-safe-main-proof
{
  "packageRoot": "/tmp/fs-safe-main-proof",
  "fired": false,
  "unhandledRejections": [
    "Error: EIO during sidecar snapshot",
    "Error: EIO during sidecar snapshot"
  ],
  "waitMs": 1005
}

onCompromised never ran. The interval kept throwing (dozens of unhandled rejections in one second).

After (this branch):

$ node /tmp/proof-fs-safe-f001.mjs /tmp/fs-safe-f001
onCompromised {"lockPath":".../state.json.lock","normalizedTargetPath":".../state.json"}
{
  "packageRoot": "/tmp/fs-safe-f001",
  "fired": true,
  "unhandledRejections": [],
  "waitMs": 21
}

Callback fired once in 21ms. Zero unhandled rejections.

• Tests added or updated when behavior changed
• Security and compatibility impact considered
• CHANGELOG.md updated when release-relevant
• No credentials, private paths, private hosts, or sensitive contents included

Real behavior proof

• Behavior or issue addressed: Periodic sidecar compromise check dropped I/O failures as unhandled rejections and never called onCompromised.

• Real environment tested: macOS, Node v22, package built from this branch and from origin/main f748844 under /tmp/fs-safe-f001 and /tmp/fs-safe-main-proof.

• Exact steps or command run after this patch:

  node /tmp/proof-fs-safe-f001.mjs /tmp/fs-safe-main-proof
  node /tmp/proof-fs-safe-f001.mjs /tmp/fs-safe-f001

• Evidence after fix: terminal output from the patched build:

  onCompromised {"lockPath":".../state.json.lock","normalizedTargetPath":".../state.json"}
  {
    "packageRoot": "/tmp/fs-safe-f001",
    "fired": true,
    "unhandledRejections": [],
    "waitMs": 21
  }

• Observed result after fix: The interval treats a rejected snapshot as a lost lock, fires onCompromised once, and does not emit unhandled rejections.

• What was not tested: Synchronous acquireFileLockSync interval (separate throw-in-timer path). Live antivirus-induced EIO on a production volume.

Origin

The interval was added in b1132f91 via #92 (2026-08-02). The .then had no rejection handler from the start.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 14, 2026, 12:55 PM ET / 16:55 UTC.

ClawSweeper review

What this changes

This PR treats an asynchronous sidecar-lock verification failure as a lost lock, invokes the existing compromise callback once, and documents and tests that behavior.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR repairs a real current-main failure in periodic asynchronous lock verification. The narrow fail-closed handling, focused regression test, documentation update, and built-package terminal proof support normal maintainer review.

Priority: P2
Reviewed head: a62ba3a2641d48c25a91d3c73ebf84d8810318bf

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	A focused fail-closed repair with strong built-package proof and regression coverage.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.
Evidence reviewed	5 items	Current failure path: Current main attaches only a fulfillment handler to the Promise-returning periodic verifier, so a rejected snapshot read bypasses the compromise callback. Verifier contract: Direct callers retain the Promise-returning verifier; the branch catches failures only around the timer path. Focused regression test: The branch injects a snapshot EIO, waits for exactly one callback, and verifies no unhandled rejections after additional timer time.
Findings	None	None.
Security	None	None.

How this fits together

Sidecar locks create a lock file beside an application target and periodically compare it with the ownership snapshot. A mismatch or inability to verify ownership informs the application through its one-time compromise callback.

flowchart LR
  A[Application acquires lock] --> B[Sidecar lock and snapshot]
  B --> C[Periodic compromise check]
  C --> D{Snapshot verified?}
  D -->|yes| C
  D -->|mismatch or I/O failure| E[Stop timer]
  E --> F[One-time compromise callback]

Loading

Before merge

• Resolve merge risk (P1) - Existing applications that automatically stop work in onCompromised will now receive that callback for transient verification I/O failures; this is a deliberate documented fail-closed behavior change.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Change footprint	57 added, 9 removed across 4 files	The runtime adjustment is confined to the periodic verification path, with matching documentation and regression coverage.
Production versus test delta	production +3 net, tests +44	The small behavioral change is backed by focused failure-path coverage.

Merge-risk options

Maintainer options:

1. Accept the documented fail-closed callback expansion (recommended)
   Merge the change with the documented understanding that an application opting into onCompromised may now act on a transient verification I/O failure.

Technical review

Best possible solution:

Land the narrow timer-level failure handling with its regression coverage so configured compromise detection reports unverifiable ownership instead of producing unhandled rejections.

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

Yes. Current main leaves the periodic verifier rejection unhandled, and the supplied built-package Node comparison demonstrates the callback and rejection behavior before and after the patch.

Is this the best way to solve the issue?

Yes. Catching only the periodic verification failure preserves direct verifier rejection semantics while applying the repository’s fail-closed ownership policy to the asynchronous detector.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add P2: This fixes a bounded lock-integrity detection failure without evidence of an active emergency.
• add merge-risk: 🚨 compatibility: Consumers with automatic compromise handling will observe a new callback outcome for transient verification failures.
• add proof: sufficient: Contributor real behavior proof is sufficient. The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.
• add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.

Label justifications:

• P2: This fixes a bounded lock-integrity detection failure without evidence of an active emergency.
• merge-risk: 🚨 compatibility: Consumers with automatic compromise handling will observe a new callback outcome for transient verification failures.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR provides redacted before/after Node 22 terminal proof from built main and branch packages, showing one callback and no unhandled rejections after the fix.

Evidence

What I checked:

• Current failure path: Current main attaches only a fulfillment handler to the Promise-returning periodic verifier, so a rejected snapshot read bypasses the compromise callback. (src/sidecar-lock-acquire.ts:187, f74884494505)
• Verifier contract: Direct callers retain the Promise-returning verifier; the branch catches failures only around the timer path. (src/sidecar-lock-handle.ts:42, f74884494505)
• Focused regression test: The branch injects a snapshot EIO, waits for exactly one callback, and verifies no unhandled rejections after additional timer time. (test/sidecar-lock-acquire-failure.test.ts:223, a62ba3a2641d)
• History: Blame attributes the current timer implementation to the v0.5.5 release commit; the related merged lock refactor is commit b1132f9. (src/sidecar-lock-acquire.ts:187, b28ed6d426f4)
• Real behavior proof: The PR body contains before/after Node 22 output from built main and branch packages: main emits unhandled EIO rejections without a callback, while the branch calls back once with none. (a62ba3a2641d)

Likely related people:

• Peter Steinberger: Current timer lines are attributed to Peter's v0.5.5 release commit, and the related merged lock work was also authored by Peter. (role: recent area contributor; confidence: high; commits: b28ed6d426f4, b1132f915a7c; files: src/sidecar-lock-acquire.ts, docs/sidecar-lock.md)

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.