fix(desktop): align tracking remote handling across git calls#2531
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces remote-aware git operations that support tracking remotes beyond "origin". A new upstream-ref utility module parses and resolves tracking remote names, enabling functions like branch existence checks and push operations to work with worktree-specific remotes. Common logic is refactored into the new module and re-exported from existing APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GitOps as Git Operations
participant UpstreamRef as Upstream Ref Utils
participant Git as Git CLI
rect rgba(100, 200, 150, 0.5)
Note over Client,Git: Old Flow (Hard-coded Origin)
Client->>GitOps: pushWithSetUpstream(git, branch)
GitOps->>Git: push origin branch<br/>(hard-coded "origin")
Git-->>GitOps: success
GitOps-->>Client: result
end
rect rgba(150, 150, 200, 0.5)
Note over Client,Git: New Flow (Tracking Remote Resolution)
Client->>GitOps: pushWithSetUpstream(git, branch, remote?)
GitOps->>Git: rev-parse @{upstream}
Git-->>GitOps: upstream ref (e.g., upstream/main)
GitOps->>UpstreamRef: resolveTrackingRemoteName(upstreamRef)
UpstreamRef-->>GitOps: resolved remote (e.g., "upstream")
GitOps->>Git: push [resolved-remote] branch
Git-->>GitOps: success
GitOps-->>Client: result
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/lib/trpc/routers/changes/git-operations.ts (1)
35-44: Consider consolidating withgetTrackingRemoteNameForWorktree.This function duplicates logic from
getTrackingRemoteNameForWorktreeingit.ts. The only difference is the input type (SimpleGitvsworktreePath: string). Consider either:
- Reusing
getTrackingRemoteNameForWorktreeby extracting the worktree path from theSimpleGitinstance- Exporting a shared helper that both can use
This is a minor duplication concern and could be addressed in a follow-up.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/changes/git-operations.ts` around lines 35 - 44, The getTrackingRemote function duplicates logic already implemented in getTrackingRemoteNameForWorktree; update it to reuse shared logic by either extracting the worktree path from the SimpleGit instance and calling getTrackingRemoteNameForWorktree(worktreePath), or move the core logic that calls git.rev-parse and resolveTrackingRemoteName into a new exported helper (e.g., resolveTrackingRemoteFromRawUpstream) that both getTrackingRemote and getTrackingRemoteNameForWorktree call; ensure you continue to catch errors and default to "origin" as the current behavior.apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.test.ts (1)
26-30: Consider adding a test for custom fallback parameter.The
resolveTrackingRemoteNamefunction accepts a customfallbackparameter, but this isn't tested. Consider adding a test case to verify this behavior.💡 Suggested additional test case
test("uses custom fallback when provided", () => { expect(resolveTrackingRemoteName("", "upstream")).toBe("upstream"); expect(resolveTrackingRemoteName(null, "fork")).toBe("fork"); });🤖 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/upstream-ref.test.ts` around lines 26 - 30, Add a test that verifies resolveTrackingRemoteName respects its optional fallback parameter: update the test suite in upstream-ref.test.ts to include a new test (e.g., "uses custom fallback when provided") that calls resolveTrackingRemoteName with empty string/null and a custom fallback string (like "upstream" or "fork") and asserts the return equals that fallback; ensure both empty string and null cases are covered to mirror the existing fallback tests.
🤖 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/changes/git-operations.ts`:
- Around line 35-44: The getTrackingRemote function duplicates logic already
implemented in getTrackingRemoteNameForWorktree; update it to reuse shared logic
by either extracting the worktree path from the SimpleGit instance and calling
getTrackingRemoteNameForWorktree(worktreePath), or move the core logic that
calls git.rev-parse and resolveTrackingRemoteName into a new exported helper
(e.g., resolveTrackingRemoteFromRawUpstream) that both getTrackingRemote and
getTrackingRemoteNameForWorktree call; ensure you continue to catch errors and
default to "origin" as the current behavior.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.test.ts`:
- Around line 26-30: Add a test that verifies resolveTrackingRemoteName respects
its optional fallback parameter: update the test suite in upstream-ref.test.ts
to include a new test (e.g., "uses custom fallback when provided") that calls
resolveTrackingRemoteName with empty string/null and a custom fallback string
(like "upstream" or "fork") and asserts the return equals that fallback; ensure
both empty string and null cases are covered to mirror the existing fallback
tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d317cae6-988e-4a8f-8485-c09b7a4863bf
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/changes/git-operations.tsapps/desktop/src/lib/trpc/routers/changes/utils/pull-request-url.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.test.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.ts
z3thon
pushed a commit
to z3thon/superset
that referenced
this pull request
Mar 19, 2026
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
1 participant
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.
Summary
originorigin/<defaultBranch>unchanged, and add regression tests for upstream parsing plus remote-specific branch existenceTest plan
bun test apps/desktop/src/lib/trpc/routers/changes/git-operations.test.ts apps/desktop/src/lib/trpc/routers/changes/utils/pull-request-url.test.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.test.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.tsbunx @biomejs/biome@2.4.2 check apps/desktop/src/lib/trpc/routers/changes/git-operations.ts apps/desktop/src/lib/trpc/routers/changes/utils/pull-request-url.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.test.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/upstream-ref.tsFollow-up to #2517.
Summary by cubic
Make all desktop git operations use the current branch’s tracking remote instead of assuming
origin, so fetch/push/set-upstream and PR status checks work correctly with forks and non-originremotes.Bug Fixes
@{upstream}with a safe fallback tooriginand use it for fetch and--set-upstreampush.branchExistsOnRemoteaccepts a remote name; GitHub PR status now checks the branch on the tracking remote.origin/<defaultBranch>unchanged.Refactors
upstream-refutility (parseUpstreamRef,resolveTrackingRemoteName) and reused it in PR URL building and git ops.Written for commit 89a8738. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Refactor
Tests