Skip to content

fix: prevent duplicate URL group for local-only repos#613

Merged
PureWeen merged 3 commits intomainfrom
fix/no-url-group-for-local-only-repo
Apr 18, 2026
Merged

fix: prevent duplicate URL group for local-only repos#613
PureWeen merged 3 commits intomainfrom
fix/no-url-group-for-local-only-repo

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

Problem

After PR #607 fixed group name destruction during promotion, a second duplicate group issue appeared: GetOrCreateRepoGroup created a URL-based group for repos that only exist as local folder repos (e.g., dotnet-maui-local-66cccd41). This caused two "maui" groups in the sidebar — the local folder group (maui3) and a newly auto-created URL group (maui).

Root Cause

The "heal stranded sessions" loop in ReconcileOrganization calls GetOrCreateRepoGroup for sessions in local folder groups whose worktree paths are outside the group's LocalPath. For local-only repos (IDs containing -local-), this created a redundant URL-based group with the same name as the local folder group's repo name.

GetOrCreateRepoGroup skips IsLocalFolder groups when searching for existing groups (by design — they coexist). But for repos that only exist as local folder repos, this creates a duplicate.

Fix

Add a guard in GetOrCreateRepoGroup: when a local folder group already covers a repo with a -local- ID suffix, return null instead of creating a redundant URL-based group. Repos without the -local- suffix (i.e., repos with both URL and local entries) still allow URL group creation for the heal-stranded-sessions scenario.

Testing

  • 1 new regression test: LocalOnlyRepo_DoesNotCreateUrlGroup_ForCentralizedWorktree
  • Existing HealsStrandedSessions tests still pass (URL repos with both local + managed worktrees)
  • Full suite: 3480 tests pass, 0 failures

GetOrCreateRepoGroup created a duplicate URL-based group for repos
that only exist as local folder repos (e.g., dotnet-maui-local-*).
This caused two 'maui' groups to appear in the sidebar — the local
folder group and a newly created URL-based group with the same name.

The fix: when a local folder group already covers a repo with a
'-local-' ID suffix, GetOrCreateRepoGroup returns null instead of
creating a redundant URL-based group. Repos with managed bare clones
(same repoId for both local and URL groups) still allow URL group
creation for the heal-stranded-sessions scenario.

Adds regression test verifying that local-only repos don't get
duplicate URL-based groups during ReconcileOrganization.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen
Copy link
Copy Markdown
Owner Author

PureWeen commented Apr 18, 2026

🔍 Multi-Model Code Review — PR #613 (v3 Re-Review)

Reviewed by: 3 independent reviewers with adversarial consensus
Diff reviewed: +117/−8, 3 files (3 commits)
CI Status: ⚠️ No checks configured for this branch


All Findings — Final Status

# Finding v1 v2 v3
1 Test repo ID didn't contain "-local-" — guard never exercised 🔴 ✅ FIXED
2 localRepo != null check created silent failure mode 🟡 ✅ FIXED
3 Magic-string "-local-" detection is fragile 🟡 ⚠️ ACKNOWLEDGED FIXEDRepoManager.LocalRepoIdInfix constant + IsLocalOnlyRepoId() helper; all 5 usages updated
4 Heal-stranded-sessions behavioral change untested 🟡 ✅ FIXED
5 No diagnostic log when heal-stranded is blocked 🟢 🟢 FIXEDDebug(...) else-branch added
6 No shared constant for "-local-" infix 🟢 🟢 FIXED — same as #3

v3 Changes (commit d97f0c69)

Production code:

  • RepoManager.cs: Added LocalRepoIdInfix constant and IsLocalOnlyRepoId() static helper. Replaced all 4 inline "-local-" usages with the constant.
  • CopilotService.Organization.cs: Guard now uses RepoManager.IsLocalOnlyRepoId(repoId) instead of inline Contains. Added else Debug(...) branch for when heal-stranded is intentionally blocked.

No new test changes needed — existing tests already cover the behavior; only the implementation was refactored.


Recommended Action

✅ Approve

All 6 findings across 3 review rounds are resolved. Guard logic is correct, well-documented via shared constant/helper, and covered by unit + integration tests. 3482 tests pass, 0 failures.

PureWeen and others added 2 commits April 18, 2026 13:32
- Fix test repo ID to use real '-local-' format ('dotnet-maui-local-a1b2c3d4')
  so the guard code is actually exercised (was 'local-repo-abc123' which
  doesn't contain '-local-' substring)
- Set IsInitialized + allowPruning:false in integration test so
  ReconcileOrganization doesn't early-exit before reaching heal loop
- Remove unnecessary localRepo != null check — repoId.Contains('-local-')
  alone is sufficient, avoids silent failure when repo missing from state
- Simplify guard: check '-local-' first, then verify local folder group
  exists (clearer intent, fewer LINQ scans on non-local repos)
- Add direct GetOrCreateRepoGroup unit test for local-only repos (null)
- Add direct GetOrCreateRepoGroup unit test for non-local repos (allowed)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add RepoManager.LocalRepoIdInfix constant ("-local-") and
  IsLocalOnlyRepoId() helper method
- Replace all 5 inline "-local-" usages across RepoManager.cs and
  CopilotService.Organization.cs with the constant/helper
- Add diagnostic Debug log when heal-stranded-sessions is intentionally
  blocked for local-only repos (aids troubleshooting)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PureWeen PureWeen merged commit b197462 into main Apr 18, 2026
@PureWeen PureWeen deleted the fix/no-url-group-for-local-only-repo branch April 18, 2026 20:05
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