fix: avoid unrelated macOS OAuth local-lock collisions

[mohanagy]

Summary

• add a bounded deterministic macOS-only fallback set for OAuth local locks;
• use it only after the canonical legacy port identifies an unrelated listener;
• preserve exact legacy-holder and same-key serialization, including fail-closed ambiguous probes.

Security and compatibility

• The canonical port remains the first and only cross-version compatibility boundary.
• Matching greetings and incomplete or ambiguous probes never fall back.
• Contenders choose only the first available deterministic fallback and re-probe after a race, preventing one key from splitting across candidates.
• Linux and Windows strategy behavior is unchanged.

Closes #264

Validation

• TDD: new macOS collision and unrelated-listener tests failed before the implementation with OAuthLocalLockUnavailableError, then passed.
• npm test -- tests/oauth-local-lock.test.ts
• npm run test:oauth-console
• npm test: 1,720 passed, 27 skipped
• npm run test:core: 415 passed, 22 skipped
• npm run test:coverage: 1,720 passed, 27 skipped; thresholds passed
• npm run lint
• npm run typecheck
• npm run build
• npm run smoke:cli
• npm run check:pack
• npm run test:package: 24 passed

Summary by CodeRabbit

• Bug Fixes
  • Improved local lock acquisition on macOS when the primary lock endpoint is occupied by selecting an available fallback endpoint.
  • Added coordinated fallback behavior that avoids bypassing existing holders and fails safely when lock ownership is ambiguous.
  • Updated retry/wait behavior to prefer fallback availability over unnecessary waiting.
• Tests
  • Added expanded macOS regression coverage for deterministic fallback candidate probing, independence across keys, and protection against ambiguous or bypassed acquisitions.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a8bcdbe9-869c-4033-afbc-d5fffff46c18

📥 Commits

Reviewing files that changed from the base of the PR and between 9c4bf75 and e734176.

📒 Files selected for processing (2)

• src/oauth/local-lock.ts
• tests/oauth-local-lock.test.ts

📝 Walkthrough

Walkthrough

macOS OAuth local-lock acquisition now derives bounded deterministic fallback TCP endpoints, probes them after unrelated legacy-port collisions, preserves same-key and older-version coordination, and fails closed for ambiguous listeners. Tests cover collisions, fallback success, serialization, and holder detection.

Changes

macOS OAuth lock fallback

Layer / File(s)	Summary
Deterministic fallback probing src/oauth/local-lock.ts	macOS fallback endpoints are deterministically derived from the lock key, de-duplicated from the legacy endpoint, bounded in count, and probed with same-key coordination rules.
Fallback acquisition integration src/oauth/local-lock.ts	acquireLocalLock determines fallback eligibility, adjusts waiting behavior, and returns a release function for successful fallback acquisition.
macOS collision and serialization coverage tests/oauth-local-lock.test.ts	Test helpers and regressions cover deterministic ordering, concurrent probing, independent colliding keys, unrelated occupied listeners, same-key serialization, ambiguous listeners, and exact older holders.

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

Sequence Diagram(s)

sequenceDiagram
  participant OAuthOperation
  participant acquireLocalLock
  participant inspectLocalLockEndpoint
  participant tryAcquireMacOSFallback
  OAuthOperation->>acquireLocalLock: request lock
  acquireLocalLock->>inspectLocalLockEndpoint: inspect canonical endpoint
  inspectLocalLockEndpoint-->>acquireLocalLock: unrelated occupied state
  acquireLocalLock->>tryAcquireMacOSFallback: probe deterministic fallbacks
  tryAcquireMacOSFallback->>inspectLocalLockEndpoint: inspect fallback endpoints
  inspectLocalLockEndpoint-->>tryAcquireMacOSFallback: endpoint states
  tryAcquireMacOSFallback-->>acquireLocalLock: acquired fallback
  acquireLocalLock-->>OAuthOperation: release function

Loading

Possibly related PRs

• mohanagy/miftah#189: Related bounded deterministic TCP probe endpoint changes.
• mohanagy/miftah#194: Related OAuth local-lock acquisition changes.
• mohanagy/miftah#263: Related macOS fallback probing behavior.

Poem

A bunny found a port blocked tight,
Then hopped through fallbacks left and right.
Same keys stayed paired in lockstep true,
While strangers found a path anew.
Ambiguous gates said, “No entry tonight!”

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Docstring Coverage	❌ Error	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the macOS OAuth local-lock collision fix.
Description check	✅ Passed	The description covers the summary, compatibility notes, and validation results required by the template.
Linked Issues check	✅ Passed	The changes satisfy #264 by adding macOS-only deterministic fallbacks, preserving serialization, and failing closed on ambiguous probes.
Out of Scope Changes check	✅ Passed	The diff appears scoped to the requested macOS local-lock behavior and tests, with no unrelated changes called out.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/264-macos-oauth-lock-collisions

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/oauth/local-lock.ts`:
- Around line 7-9: Export macOSFallbackCandidateCount from
src/oauth/local-lock.ts as the shared source of truth. In
tests/oauth-local-lock.test.ts, import that symbol and replace the hardcoded 7
used as the loop bound; no direct changes are needed elsewhere.
- Around line 168-186: Update tryAcquireMacOSFallback to inspect all endpoints
concurrently, such as by collecting the inspectLocalLockEndpoint results with
Promise.all, while preserving the full-scan semantics: any “held” or “unknown”
result must return undefined, “occupied” candidates must be skipped, and the
first available endpoint must remain the selected candidate for
tryAcquireLocalLock.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 15231118-d2ad-4717-9360-0d3a7f5f80a1

📥 Commits

Reviewing files that changed from the base of the PR and between f846511 and cc14ec0.

📒 Files selected for processing (2)

• src/oauth/local-lock.ts
• tests/oauth-local-lock.test.ts

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/oauth/local-lock.ts`:
- Around line 211-218: Update the local-lock acquisition loop around
canUseMacOSFallback and the primary tryAcquireLocalLock call so an occupied
legacy endpoint with an available macOS fallback skips the doomed legacy bind
and proceeds directly to fallback scanning. Preserve the existing primary
acquisition behavior for all other endpoint states and platforms.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4fe30311-92a5-4c3a-8df3-09e1f87e141a

📥 Commits

Reviewing files that changed from the base of the PR and between cc14ec0 and 9c4bf75.

📒 Files selected for processing (2)

• src/oauth/local-lock.ts
• tests/oauth-local-lock.test.ts

[mohanagy]

Addressed the remaining CodeRabbit thread in e734176: when the canonical macOS legacy endpoint is positively identified as unrelated and occupied, Miftah now goes directly to the deterministic fallback scan. The focused regression failed before this change and passes after it; full tests, coverage, lint, typecheck, build, CLI smoke, and package validation are green locally.

[mohanagy]

Review-gate exception, documented before dismissal: CodeRabbit's current-head check for e734176 explicitly reports Review rate limited, so it cannot produce a new decision for this final commit. All three CodeRabbit threads are resolved, including the final occupied-legacy-bind regression. The full current-head matrix is green: Linux, macOS, Windows 20/22/24, Linux quality, and Verify. Dismissing only the two stale CodeRabbit change requests from earlier commits under the repository's documented rate-limit exception.

All associated threads are resolved; current-head CodeRabbit review is rate-limited.