Skip to content

fix(config): reject ambiguous Windows migration publications - #222

Merged
mohanagy merged 2 commits into
developmentfrom
fix/221-config-migration-windows
Jul 25, 2026
Merged

fix(config): reject ambiguous Windows migration publications#222
mohanagy merged 2 commits into
developmentfrom
fix/221-config-migration-windows

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Fixes #221\n\n## Root cause\n\nThe post-link migration checks compared default-number filesystem IDs. On Windows, those IDs can be lossy at JavaScript Number precision, allowing a concurrent replacement of the public candidate path to be mistaken for the private hard-linked candidate.\n\n## Change\n\n- Verify both backup and candidate hard-link publications with BigInt filesystem IDs.\n- Fail closed when the exact inode identity is unavailable.\n- Keep the existing held-source recovery and bounded CONFIG_MIGRATION_WRITE_FAILED behavior unchanged.\n\n## TDD evidence\n\n- Added a deterministic regression that aliases all default-number metadata after a concurrent target replacement while preserving the exact BigInt identity.\n- The regression resolved before the exact-identity change and rejects afterward.\n- Passed: config-migration focused suite, typecheck, lint.\n\nWindows CI remains the authoritative validation for the original intermittent path.

Summary by CodeRabbit

  • Bug Fixes
    • Improved configuration migration safety on filesystems where file identity values may lose precision.
    • Prevents incorrect overwrites when a configuration file is replaced during migration.
    • Ensures the original source and backup contents are preserved when concurrent file changes are detected.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

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: 42 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: 586f01ee-59d0-4eba-8c2b-67ad8bbeed05

📥 Commits

Reviewing files that changed from the base of the PR and between 601e1aa and cd64929.

📒 Files selected for processing (2)
  • src/cli/migrate-config.ts
  • tests/config-migration.test.ts
📝 Walkthrough

Walkthrough

Config migration now uses bigint-capable filesystem identities to validate backups and published candidates, preventing lossy number metadata from masking concurrent replacements. Tests simulate aliased metadata and verify the migration retains the held source and backup content.

Changes

Config migration race handling

Layer / File(s) Summary
BigInt file identity verification
src/cli/migrate-config.ts
Adds bigint dev/ino comparisons, regular-file checks, and bigint lstat calls for backup and candidate verification.
Aliased metadata regression coverage
tests/config-migration.test.ts
Mocks aliased stat metadata and verifies rollback behavior when a concurrent replacement wins publication.

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

Possibly related PRs

  • mohanagy/miftah#73: Introduced related config-migration race and file-identity behavior refined by this change.

Poem

I’m a bunny guarding each file,
With bigint tracks across the aisle.
If rivals hop in during flight,
Backup stays tucked safe and right.
The held source wins its careful mile.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main Windows config-migration fix.
Description check ✅ Passed The description covers the root cause, fix, and validation, though it omits the template's Security impact section.
Linked Issues check ✅ Passed The change matches #221 by failing closed on ambiguous publication identity and preserving the existing write-failure recovery path.
Out of Scope Changes check ✅ Passed The PR stays focused on config-migration race handling and its regression test, with no obvious unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/221-config-migration-windows

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

@mohanagy
mohanagy force-pushed the fix/221-config-migration-windows branch from 601e1aa to cd64929 Compare July 25, 2026 00:46
@mohanagy
mohanagy merged commit 065ed70 into development Jul 25, 2026
12 checks passed
@mohanagy
mohanagy deleted the fix/221-config-migration-windows branch July 25, 2026 00:52
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.

bug: preserve config migration failure when concurrent replacement wins on Windows

1 participant