fix: fall back to FETCH_HEAD when gh pr checkout fails for branch names with /#3232
Conversation
…ranch names with / Fixes superset-sh#3231 gh pr checkout internally runs `git checkout -b <branch> --track origin/<branch>`. When the branch name contains `/`, git cannot resolve the tracking ref inside a freshly created detached worktree, producing "starting point is not a branch". The fetch succeeds — only the tracking setup fails. Catch that specific error and fall back to `git checkout -b <localBranchName> --no-track FETCH_HEAD`. push.autoSetupRemote=true (already set after worktree creation) handles push tracking without needing --track.
📝 WalkthroughWalkthroughWraps the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR fixes a crash when creating a worktree linked to a PR whose branch name contains The fix wraps the
Confidence Score: 4/5Safe to merge — fixes a real, reproducible bug for the common case with one minor edge-case that is unlikely in practice The fix correctly handles the primary failure scenario (detached worktree + branch name containing apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts — specifically the fallback path in createWorktreeFromPr when branchExists is true Important Files Changed
Sequence DiagramsequenceDiagram
participant C as createWorktreeFromPr
participant W as git worktree
participant GH as gh CLI
participant G as git
C->>W: git worktree add (detached or with branch)
C->>GH: gh pr checkout number --branch localBranchName --force
alt success
GH-->>C: ok (branch + tracking configured)
else is not a branch error
GH-->>C: fatal: starting point origin/user/feature is not a branch
Note over GH,G: gh fetched remote before failing, FETCH_HEAD is valid
C->>G: git checkout -b localBranchName --no-track FETCH_HEAD
G-->>C: ok (branch created, no upstream tracking)
else other error
GH-->>C: re-throw original error
end
C->>G: git config push.autoSetupRemote true
Note over C,G: autoSetupRemote handles push tracking without --track
Reviews (1): Last reviewed commit: "fix: fall back to FETCH_HEAD checkout wh..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:1808">
P2: When the branch already exists locally (`branchExists === true`), this fallback will fail with `"A branch named '<localBranchName>' already exists"` because `checkout -b` requires the branch to not exist. That error won't be caught by the outer error handlers, surfacing as a generic failure.
Consider branching on `branchExists`: when true, use `git reset --hard FETCH_HEAD` (the branch is already checked out in the worktree); when false, use the current `checkout -b` path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
1793-1802: Consider logging when fallback path is taken.When the fallback is triggered, there's no indication in the logs. Adding a log statement would help with debugging and understanding which code path was executed.
Suggested improvement
} catch (ghError) { const ghMsg = ghError instanceof Error ? ghError.message : String(ghError); // `gh pr checkout` can fail with "is not a branch" when the branch name // contains '/' (e.g. "user/feature-branch"). Git has trouble resolving // "origin/user/feature-branch" as a tracking ref inside a worktree. // gh already fetched the remote successfully, so FETCH_HEAD points to // the right commit — fall back to creating the branch without tracking. if (!ghMsg.includes("is not a branch")) { throw ghError; } + console.log( + `[git] gh pr checkout failed with tracking error for PR #${prInfo.number}, falling back to FETCH_HEAD checkout`, + ); await execGitWithShellPath(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts` around lines 1793 - 1802, In the catch block that handles ghError (where ghMsg is derived) add a log statement when the fallback branch-creation path is taken (i.e., when ghMsg.includes("is not a branch")); log the fact that the "is not a branch" fallback is being used along with the branch name/PR identifier and ghMsg (and include ghError stack if available) using the project logger (e.g., processLogger.warn or similar) so it's easy to trace when the non-tracking branch creation is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts`:
- Around line 1803-1814: The fallback git checkout call inside
createWorktreeFromPr can fail when branchExists is true because the worktree may
already have the branch locally; update the execGitWithShellPath invocation that
passes ["-C", worktreePath, "checkout", "-b", localBranchName, "--no-track",
"FETCH_HEAD"] to use a force-replace flag (use "-B" instead of "-b") so the
checkout will succeed whether the branch already exists or not; locate the call
to execGitWithShellPath and replace the "-b" token with "-B" while keeping
localBranchName, "--no-track", and "FETCH_HEAD" intact.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts`:
- Around line 1793-1802: In the catch block that handles ghError (where ghMsg is
derived) add a log statement when the fallback branch-creation path is taken
(i.e., when ghMsg.includes("is not a branch")); log the fact that the "is not a
branch" fallback is being used along with the branch name/PR identifier and
ghMsg (and include ghError stack if available) using the project logger (e.g.,
processLogger.warn or similar) so it's easy to trace when the non-tracking
branch creation is used.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49546284-912e-47f9-aaf2-24d96a606a1d
📒 Files selected for processing (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)
1780-1818: LGTM! Well-structured fallback for the/branch name issue.The try/catch approach with specific error detection is a pragmatic solution. The comment clearly documents the rationale, and logging the fallback path aids debugging. The
-Bflag correctly handles both existing and non-existing branch scenarios.One optional hardening consideration: the error string check is case-sensitive. While git error messages are typically lowercase, a case-insensitive check would be slightly more defensive:
Optional: case-insensitive error matching
- if (!ghMsg.includes("is not a branch")) { + if (!ghMsg.toLowerCase().includes("is not a branch")) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts` around lines 1780 - 1818, Modify the error-message check to be case-insensitive by normalizing ghMsg before matching: when catching ghError in the gh pr checkout block (variables ghError/ghMsg, function execWithShellEnv), convert ghMsg to a single case (e.g., lowerCase) and test for the substring "is not a branch" against that normalized string; keep the same fallback path that calls execGitWithShellPath to checkout FETCH_HEAD into localBranchName and rethrow other errors unchanged (use prInfo.number in the log as currently done).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts`:
- Around line 1780-1818: Modify the error-message check to be case-insensitive
by normalizing ghMsg before matching: when catching ghError in the gh pr
checkout block (variables ghError/ghMsg, function execWithShellEnv), convert
ghMsg to a single case (e.g., lowerCase) and test for the substring "is not a
branch" against that normalized string; keep the same fallback path that calls
execGitWithShellPath to checkout FETCH_HEAD into localBranchName and rethrow
other errors unchanged (use prInfo.number in the log as currently done).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d4b53e48-3498-46d4-ba60-a311f34233cd
📒 Files selected for processing (1)
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
- Fix Biome formatting in git.ts (PR #3232 lint regression) - Update NotificationManager test to match renamed notification strings ('Awaiting Response' / 'is waiting for your reply') - Add DATABASE_URL and DATABASE_URL_UNPOOLED to marketing deploy workflows (production + preview) — the neon() client init crashes at module evaluation time when the env var is missing, which broke the marketing build since Apr 19 Co-Authored-By: Mastra Code (anthropic/claude-opus-4-6) <noreply@mastra.ai>
- Fix Biome formatting in git.ts (PR #3232 lint regression) - Update NotificationManager test to match renamed notification strings ('Awaiting Response' / 'is waiting for your reply') - Convert CTAButtons auth import to dynamic import — the top-level import of @superset/auth/server triggered neon() at module evaluation time during Next.js static page collection, crashing the marketing build when DATABASE_URL was absent. Dynamic import defers DB init to runtime inside a try-catch, so it fails gracefully. Co-Authored-By: Mastra Code (anthropic/claude-opus-4-6) <noreply@mastra.ai>
- Fix Biome formatting in git.ts (PR #3232 lint regression) - Update NotificationManager test to match renamed notification strings ('Awaiting Response' / 'is waiting for your reply') - Convert CTAButtons auth import to dynamic import to prevent neon() from crashing during Next.js static page collection at build time - Add DATABASE_URL to marketing deploy workflows (production + preview) so the session check works at runtime and logged-in users see the Dashboard link Co-Authored-By: Mastra Code (anthropic/claude-opus-4-6) <noreply@mastra.ai>
2 participants
Closes #3231
What
When creating a workspace linked to a PR whose branch name contains
/(e.g.user/feature-branch),gh pr checkoutinternally runsgit checkout -b <branch> --track origin/<branch>. Inside a freshly created detached worktree, git cannot resolve the remote tracking ref when the branch name has a/, producing:The fetch itself succeeds — only the
--tracksetup fails.Fix
Wrap the
gh pr checkoutcall in a try/catch. When the error contains "is not a branch", fall back to:Since
gh pr checkoutalready fetched the remote before failing,FETCH_HEADpoints to the correct commit.push.autoSetupRemote = true(already configured after worktree creation) handles push tracking without needing--track.Summary by cubic
Fixes PR checkout when branch names contain "/" by falling back to a local
FETCH_HEADcheckout ifgh pr checkoutfails. Restores workspace creation and logs the fallback path for easier debugging (fixes #3231).gh pr checkout; on "is not a branch", rungit checkout -B <localBranchName> --no-track FETCH_HEADto create/replace the local branch.gh;FETCH_HEADpoints to the right commit.push.autoSetupRemote=truesogit pushsets upstream without--track.Written for commit 919665d. Summary will update on new commits.
Summary by CodeRabbit