Repository navigation
fix(desktop): classify detached-HEAD main-repo open as typed BAD_REQUEST - #6258
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe workspace creation procedure now reports detached HEAD states with a ChangesWorkspace error handling
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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/procedures/create.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts:748">
P2: Because getCurrentBranch swallows every git failure and returns null, a genuine error (not-a-git-repo, git binary/permission failure) now surfaces to the user as 'detached HEAD' and is no longer reported to Sentry — so unexpected failures on this path no longer reach Sentry 'as before' as the PR states. Consider distinguishing a real git error (e.g. let getCurrentBranch propagate or check repo validity) and only classify detached HEAD as BAD_REQUEST.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| input.branch || (await getCurrentBranch(project.mainRepoPath)); | ||
| if (!branch) { | ||
| throw new Error("Could not determine current branch"); | ||
| throw new TRPCError({ |
Contributor
There was a problem hiding this comment.
P2: Because getCurrentBranch swallows every git failure and returns null, a genuine error (not-a-git-repo, git binary/permission failure) now surfaces to the user as 'detached HEAD' and is no longer reported to Sentry — so unexpected failures on this path no longer reach Sentry 'as before' as the PR states. Consider distinguishing a real git error (e.g. let getCurrentBranch propagate or check repo validity) and only classify detached HEAD as BAD_REQUEST.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts, line 748:
<comment>Because getCurrentBranch swallows every git failure and returns null, a genuine error (not-a-git-repo, git binary/permission failure) now surfaces to the user as 'detached HEAD' and is no longer reported to Sentry — so unexpected failures on this path no longer reach Sentry 'as before' as the PR states. Consider distinguishing a real git error (e.g. let getCurrentBranch propagate or check repo validity) and only classify detached HEAD as BAD_REQUEST.</comment>
<file context>
@@ -745,7 +745,12 @@ export const createCreateProcedures = () => {
input.branch || (await getCurrentBranch(project.mainRepoPath));
if (!branch) {
- throw new Error("Could not determine current branch");
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message:
</file context>
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
added a commit
that referenced
this pull request
Aug 13, 2026
…y fix wave (#6437) * fix(desktop,host-service): address unreviewed findings from the sentry fix wave Four defects that automated reviewers flagged on PRs #6191, #6258, #6316 and #6318 and that were merged without anyone acting on them. - host-service-coordinator: crash reports redacted the child's own auth secret but not the cloud credential the coordinator also injects into its env, so a logged Authorization header could reach Sentry. Replace the single replacement with a per-instance redaction list. - workspaces/create: getCurrentBranch() returns null for a detached HEAD and for any git failure alike, so a missing repo or a permission wall was reported to the user as a detached HEAD and stopped being visible in Sentry. Confirm HEAD is genuinely detached first. - mesh-gradient: the WebGL probe leaked a context per mount, which can exhaust the browser's context budget and pin the real gradient to the static fallback. Release the probe and cache the result. - workspace-creation: existsSync() is true for a regular file, so a repo path pointing at a file passed the directory guard. * refactor(desktop): read HEAD once via a discriminated result instead of a second git client
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Opening a project's main repository as a workspace (
workspaces.openMainRepoWorkspace) throws a plainError("Could not determine current branch")when the repo has no current branch. The tRPCsentryMiddlewarereports this as anINTERNAL_SERVER_ERROR, so an expected user-repo state shows up in Sentry as a 500 (DESKTOP-JM, 3 events on 1.15.1).Root cause
When the main repo is on a detached HEAD (or an unborn branch),
getCurrentBranchreturns nothing and the guard at the top ofopenMainRepoWorkspacethrows an untypedError. Per the classification contract (#6164), expected domain states must be translated into non-500TRPCErrors at the throw site — this one never was.Fix
Replace the plain throw with a typed
TRPCError:code: "BAD_REQUEST"— matching the existing detached-HEAD classification ingetLocalBranchOrThrow(routers/changes/git-operations.ts), whose message style this also follows.cause: { kind: "DETACHED_HEAD" }— a plain-object, kind-discriminated cause that the existingerrorFormatterinsrc/lib/trpc/index.tsalready forwards to the renderer.No filter, no middleware change — genuine unexpected failures on this path still reach Sentry as before.
Verification
bunx biome check apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.tscd apps/desktop && bun run typecheckexternal-worktree-import.test.tstestsutils/gitonly), so no test run applies.Refs DESKTOP-JM
https://claude.ai/code/session_2bf90175-44a5-44e3-be07-aeed3182571a
Summary by cubic
Treat opening the main repo on a detached HEAD as a BAD_REQUEST instead of a 500, with a clear action message. Aligns with DESKTOP-JM and keeps this expected state out of Sentry server errors.
Written for commit 2315ab6. Summary will update on new commits.
Summary by CodeRabbit