Skip to content

fix: publish complete cross-process write locks - #121

Merged
cmdaltctr merged 1 commit into
mainfrom
fix/cross-process-lock-publication
Oct 7, 2026
Merged

cmdaltctr merged 1 commit into
mainfrom
fix/cross-process-lock-publication

Conversation

@cmdaltctr

@cmdaltctr cmdaltctr commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

Release 4.13.2 failed its macOS 26 storage check: all 50 memory rows survived, but shard metadata counted 48. The other five platforms passed, and npm publishing was skipped.

Exclusive file creation exposed an empty lock before its process ID was written. A contender could delete that file as corrupt and enter alongside the original writer. The deterministic regression reproduced two active writer callbacks before the fix.

This fix prepares a complete payload in a unique temporary file, then publishes it with an exclusive hard link. It keeps the existing lock path and JSON fields. Windows hard-link refusals use the bounded retry policy from TDR-034. Added regression tests and TDR-039.

The existing stale-owner takeover logic stays unchanged. This restores the shared-store safety requirement; no new OpenSpec behaviour change is introduced.

Validation

  • PASS: regression failed before the fix (two active writers, expected one), then passed after the fix.
  • PASS: six focused lock tests and three real-storage tests.
  • PASS: 20 consecutive real-storage runs, covering 60 cases.
  • PASS: bun run ci:local, rerun after commit and before push (295 test files).
  • PASS: pre-commit and pre-push checks, plus git diff --check.
  • PASS: openspec validate host-neutral-memory-core --type spec --strict.
  • NOT RUN: native Windows and the six-platform smoke for this fix.
  • NOT RUN: independent audit (not requested).

Bun emitted its existing web tsconfig directory warnings; every test file passed.

Before merge

  • Run Platform Package Smoke on this branch with maintainer approval and confirm all six platforms pass.
  • Confirm pull-request checks pass.

Failed release run: https://github.com/cmdaltctr/omms/actions/runs/37684125173

Summary by CodeRabbit

  • Bug Fixes

    • Improved cross-process write-lock reliability by ensuring a complete lock record is published before another process can inspect it.
    • Preserved recovery of locks held by inactive processes and added bounded retries for temporary publication failures on Windows.
    • Lock files are cleaned up after use, including when the protected operation fails.
  • Documentation

    • Added a proposed technical decision record documenting the locking behavior and its trade-offs.

Prepare each PID payload before publishing the lock with an exclusive hard link. This prevents concurrent writers from reclaiming a partially written lock and corrupting shard counts.

Add deterministic regression tests, bounded Windows publication retries, and TDR-039.
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: cmdaltctr/omms/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 699361aa-d5fe-4580-a5d4-a8fe0c0cd9cd
📥 Commits

Reviewing files that changed from the base of the PR and between 390483d and 7892a55.

📒 Files selected for processing (5)
  • docs/shared-core.md
  • docs/tdr/039-publish-complete-cross-process-write-locks.md
  • docs/tdr/README.md
  • src/services/turso/cross-process-write-lock.ts
  • tests/cross-process-write-lock.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The cross-process write lock now writes a complete PID payload to a unique temporary file and publishes it with a hard link. The change retains stale-lock recovery and the contention deadline, adds Windows publication retries, and includes tests and a proposed TDR.

Changes

Cross-process write-lock publication

Layer / File(s) Summary
Publication decision and documented constraints
docs/tdr/039-publish-complete-cross-process-write-locks.md, docs/shared-core.md, docs/tdr/README.md
The proposed TDR records the hard-link publication approach, Windows retry delays, filesystem constraints, alternatives, and validation steps. The shared-core description and TDR index include the proposal.
Lock publication and behavior tests
src/services/turso/cross-process-write-lock.ts, tests/cross-process-write-lock.test.ts
The lock writes to a unique temporary file before publishing the lock path. Windows retries specified link errors. Tests cover contention, cleanup, publication errors, writer failure, and recovery from a dead owner.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant WriterProcess
  participant LockFileSystem
  participant ContenderProcess
  WriterProcess->>LockFileSystem: Write complete payload to unique temporary file
  WriterProcess->>LockFileSystem: Publish lock path with linkSync
  ContenderProcess->>LockFileSystem: Attempt lock publication
  LockFileSystem-->>ContenderProcess: Return EEXIST
  ContenderProcess->>LockFileSystem: Check holder PID and poll
  WriterProcess->>WriterProcess: Run callback while holding lock
  WriterProcess->>LockFileSystem: Remove acquired lock and temporary file
Loading

Merge Risk: ⚪ Minimal · up to 7892a

No actionable issue is established for this change. The planned platform smoke test remains a before-merge validation step.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: publishing complete cross-process write locks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (3 skipped: 3 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cmdaltctr
cmdaltctr merged commit beae596 into main Oct 7, 2026
5 checks passed
@cmdaltctr
cmdaltctr deleted the fix/cross-process-lock-publication branch October 7, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant