Skip to content

Keep one local project after concurrent registration - #26789

Merged
nwparker merged 1 commit into
mainfrom
nwparker/repository-registration-dedup-task1cc89eeb
Oct 9, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/repository-registration-dedup-task1cc89eeb

Conversation

@nwparker

@nwparker nwparker commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​124 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​123
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​17 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​16

ELI5

Adding the same local project several times at once now keeps one project instead of creating duplicates.

What Changed

Recheck the existing normalized input, repository-root, and linked-main-checkout identities after icon discovery, immediately before insertion. Git and folder projects retain their current host/path rules; SSH records are excluded. There is no await between the final check and insertion.

Why

The race predates async Git detection: registration already waited for icon discovery. Reusing the existing identity rules closes that gap without a lock, cache, or second ownership store.

Visual Proof

N/A — no layout change. Real hidden Electron tests launch six concurrent root/nested adds, verify exactly one authoritative row and eventual renderer ID, then verify linked-checkout reuse and missing-path refusal.

Testing

Head b4ce9df98f1f784e9fd79e5f0574b1eeed3c7c89, base main 69cf557a698bf25f4bae24d39c19fb1fd8bc13aa.

  • Three deterministic regression cases fail before the change; all 10 registration tests pass after it.

  • macOS hidden Electron concurrency test passes. Revised core containing this fix also passes the same six-call test on native Windows.

  • The original CI renderer-count assertion assumed an awaited catalog fetch always applied. A controlled real-store interleaving reproduces 0 immediately, then the exact winner ID after the latest fetch. The test now checks authoritative rows and uses the existing settlement boundary plus eventual renderer identity.

  • Changed-code quality against original baseline 6a15cd8972bf passes; CLI/web typechecks pass. Full typecheck has the baseline limitation below.

  • Manually exercised hidden Electron

  • Automated tests added/updated

Merge this PR first, then #26760, then its two independent consumer PRs.

Linked Issue

Part of the repository-detection audit in #20497; split from #26760. This does not close the broader audit.

Review

Review the focused diff against the named base. Original code and evidence are preserved at backup/pr26760-original-72208-task1cc89eeb.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer code or assets copied.

Notes

All app tests launch hidden with ORCA_BACKGROUND_LAUNCH=1. No foreground/focus tests, OS picker automation, release packaging, or full suite claim. New CI has run static checks; the baseline failure is recorded below.

Checklist

  • Focused change, user behavior, mechanism, and choice explained
  • Self-review and independent review completed
  • Cross-platform, SSH, WSL, and folder impact considered
  • Targeted tests and changed-code quality pass
  • Full typecheck: the unchanged main journal fixture fails at command-receipt-transaction.test.ts:107; reproduced on pristine main and each branch. CLI/web typechecks pass.

Split and merge order

  1. #26789: concurrent local registration, based on main.
  2. #26760: shared detection and registration responsiveness, based on Keep one local project after concurrent registration #26789.
  3. #26790: nested discovery/import and #26791: folder upgrade, independently based on Keep repository detection off the main registration thread #26760.

The original head 72208aa57e9b and its full evidence remain preserved; #26760 was narrowed through forward-only commits. No PR in this stack has been merged.

Current CI at this exact head: FAILURE in static analysis/typecheck on the unchanged main journal fixture: command-receipt-transaction.test.ts(107,39) / TS2345 (readonly JournalRow[] passed to JournalRow). This matches the pristine-main and local branch reproduction; aggregate verify fails with that prerequisite. Other pending/skipped lanes do not establish a full-suite pass. No unrelated journal fix is included in this PR.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b1010423-c6b4-45ad-aed3-fd2c6fd99324
📥 Commits

Reviewing files that changed from the base of the PR and between cbad641 and b4ce9df.

📒 Files selected for processing (3)
  • src/main/ipc/repos-add-linked-worktree.test.ts
  • src/main/ipc/repos/local-repo-registration.ts
  • tests/e2e/repo-registration-concurrency.spec.ts

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


📝 Walkthrough

Walkthrough

Local repository registration now checks for an existing repository after icon discovery and returns it instead of adding a duplicate. Unit and end-to-end tests cover concurrent registrations, linked worktrees, persisted repository identity, and missing paths.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b4ce9

The registration change is mergeable after normal checks; no actionable issue remains identified.

🚥 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 2 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check Passed The title clearly and concisely describes the primary change: preventing duplicate local projects during concurrent registration.
Description check Passed The description is detailed and covers the user impact, implementation, rationale, linked issue context, testing, review notes, cross-platform considerations, and known baseline typecheck failure.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · 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.

@nwparker
nwparker merged commit df922d3 into main Oct 9, 2026
23 of 25 checks passed
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