refactor(desktop): derive v2 base branch from git config#3492
Merged
Conversation
The v2 right sidebar always rendered the repo default branch because the BaseBranchSelector was hardcoded to display defaultBranchName, and ensureSidebarWorkspaceRecord never seeded sidebarState.baseBranch from the pending workspace's selection. Thread the persisted value into the selector and seed it at creation time from pending.baseBranch.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR moves workspace base-branch state from local persisted sidebar state to remote TRPC-backed storage, adds TRPC endpoints to read/write Changes
Sequence DiagramsequenceDiagram
participant UI as UI Components
participant Hook as Hooks (useChangesTab / useGitStatus)
participant TRPC as TRPC Client
participant Server as Host-Service Server
participant Git as Git Config
UI->>Hook: render / request baseBranch
Hook->>TRPC: getBaseBranch.useQuery({ workspaceId })
TRPC->>Server: RPC getBaseBranch
Server->>Git: read git config branch.<current>.base
Git-->>Server: baseBranch or null
Server-->>TRPC: { baseBranch }
TRPC-->>Hook: query result (staleTime: ∞)
Hook->>UI: pass baseBranch prop
UI->>UI: render selector with baseBranch
Note over UI,Hook: user updates base branch
UI->>Hook: call setBaseBranch({ baseBranch })
Hook->>TRPC: setBaseBranch.mutate({ workspaceId, baseBranch })
TRPC->>Server: RPC setBaseBranch
Server->>Git: git config --set/--unset branch.<current>.base
Git-->>Server: success
Server-->>TRPC: { baseBranch }
TRPC->>Hook: mutation complete
TRPC->>TRPC: invalidate getBaseBranch, getStatus, listCommits, getDiff
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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)
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 two independent bugs in the v2 sidebar's base branch handling: a display bug where Key changes:
Confidence Score: 5/5This PR is safe to merge — both bugs are precisely fixed, backward compatibility is maintained, and all other callers of ensureWorkspaceInSidebar remain correct via the null default. The two bugs are tightly scoped and independently fixed. The display fix (baseBranch ?? defaultBranchName) is a one-line change that correctly handles null. The creation fix threads pending.baseBranch down exactly one call site. The guard in ensureSidebarWorkspaceRecord (if get(workspaceId) return) ensures all other callers don't interfere. The schema already had baseBranch: z.string().nullable().default(null), so no migration is needed. Documentation is thorough. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/hooks/useDashboardSidebarState/useDashboardSidebarState.ts | Core creation bug fix: ensureSidebarWorkspaceRecord and ensureWorkspaceInSidebar now accept baseBranch: string |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesHeader/ChangesHeader.tsx | Core display bug fix: replaces hardcoded defaultBranchName with baseBranch ?? defaultBranchName in BaseBranchSelector's currentValue prop, and adds baseBranch: string |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx | Passes pending.baseBranch (type string |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesTabContent/ChangesTabContent.tsx | Pass-through component updated to accept and forward baseBranch: string |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx | Reads sidebarState.baseBranch from the collection and threads it to ChangesTabContent; also uses it in listCommits query (existing correct behavior). |
| apps/desktop/docs/V2_BASE_BRANCH.md | New documentation clearly explaining V2 base branch storage, seeding contract, and rationale for the V1/V2 split. |
Sequence Diagram
sequenceDiagram
participant Pending as PendingWorkspacePage
participant DSS as useDashboardSidebarState
participant Col as v2WorkspaceLocalState Collection
participant UCT as useChangesTab
participant CH as ChangesHeader
participant BBS as BaseBranchSelector
Note over Pending: workspace.status === "succeeded"
Pending->>DSS: ensureWorkspaceInSidebar(workspaceId, projectId, baseBranch)
DSS->>Col: insert sidebarState with baseBranch
Note over Col: Record seeded with fork-time baseBranch
Pending->>Pending: navigate to /v2-workspace/$workspaceId
UCT->>Col: get(workspaceId).sidebarState.baseBranch
Col-->>UCT: baseBranch (string | null)
UCT->>CH: baseBranch={baseBranch}
CH->>BBS: currentValue={baseBranch ?? defaultBranchName}
Note over BBS: Displays persisted branch
BBS-->>UCT: onChange(newBranch)
UCT->>Col: update sidebarState.baseBranch = newBranch
Note over Col: Selection persisted
Reviews (1): Last reviewed commit: "fix(desktop): persist v2 sidebar base br..." | Re-trigger Greptile
…toring Replace sidebarState.baseBranch in v2WorkspaceLocalState with a host-service git.getBaseBranch query backed by git config (branch.<name>.base, the same key v1 uses). Writes go through a new git.setBaseBranch mutation that invalidates the dependent queries. Removes the duplicate store (and the seed-at-creation workaround) so the v2 sidebar reflects whatever git has, which is already per-branch and persists across worktree recreates. React Query's cache is the only cache we need — base branch changes rarely enough that an extra roundtrip on focus is imperceptible.
Kitenite
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts (1)
30-34:⚠️ Potential issue | 🟠 MajorInvalidate
getBaseBranchongit:changedto handle external branch switches.When the user changes branches outside the app (e.g., via terminal), the
git:changedevent fires but onlygetStatusis invalidated. SincegetBaseBranchreads the config for the current branch and usesstaleTime: Number.POSITIVE_INFINITY, the cached result becomes stale after an external branch switch—the UI would show the new branch name but the old branch's base.🐛 Proposed fix to invalidate getBaseBranch on git:changed
const invalidate = useCallback(() => { void utils.git.getStatus.invalidate({ workspaceId }); + void utils.git.getBaseBranch.invalidate({ workspaceId }); }, [utils, workspaceId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts` around lines 30 - 34, The git:changed handler currently only invalidates utils.git.getStatus via the invalidate callback; update that callback (used by useWorkspaceEvent("git:changed", ...)) to also invalidate utils.git.getBaseBranch so external branch switches refresh the base-branch cache. Locate the invalidate function in useGitStatus (the one calling utils.git.getStatus.invalidate with workspaceId) and add a second invalidate call to utils.git.getBaseBranch.invalidate passing the same workspaceId (or appropriate params) so both caches are invalidated on the git:changed event.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts (1)
19-27: Consider sequencing to avoid double-fetch ofgetStatus.Currently both queries run in parallel on mount. If a non-null
baseBranchis configured,getStatuswill fetch twice: once withundefined(whilebaseBranchQueryis loading), then again with the resolved value when the query key changes.This is minor since it's only on initial mount, but you could avoid the extra request by gating
getStatusonbaseBranchQuerycompletion:♻️ Optional: wait for baseBranch before fetching status
const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery( { workspaceId }, { staleTime: Number.POSITIVE_INFINITY, enabled: Boolean(workspaceId) }, ); const baseBranch = baseBranchQuery.data?.baseBranch ?? null; +const baseBranchReady = baseBranchQuery.isSuccess; const query = workspaceTrpc.git.getStatus.useQuery( { workspaceId, baseBranch: baseBranch ?? undefined }, - { refetchOnWindowFocus: true, enabled: Boolean(workspaceId) }, + { refetchOnWindowFocus: true, enabled: Boolean(workspaceId) && baseBranchReady }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts` around lines 19 - 27, The getStatus query is firing twice because it runs before getBaseBranch (baseBranchQuery) settles; change the workspaceTrpc.git.getStatus.useQuery call to wait for baseBranchQuery completion by adding its state to the enabled option (e.g., enabled: Boolean(workspaceId) && baseBranchQuery.isSuccess or baseBranchQuery.isFetched) so getStatus only runs after baseBranchQuery resolves, and keep passing baseBranch ?? undefined for the query variables; update the enabled logic in the getStatus useQuery invocation accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts`:
- Around line 30-34: The git:changed handler currently only invalidates
utils.git.getStatus via the invalidate callback; update that callback (used by
useWorkspaceEvent("git:changed", ...)) to also invalidate
utils.git.getBaseBranch so external branch switches refresh the base-branch
cache. Locate the invalidate function in useGitStatus (the one calling
utils.git.getStatus.invalidate with workspaceId) and add a second invalidate
call to utils.git.getBaseBranch.invalidate passing the same workspaceId (or
appropriate params) so both caches are invalidated on the git:changed event.
---
Nitpick comments:
In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts`:
- Around line 19-27: The getStatus query is firing twice because it runs before
getBaseBranch (baseBranchQuery) settles; change the
workspaceTrpc.git.getStatus.useQuery call to wait for baseBranchQuery completion
by adding its state to the enabled option (e.g., enabled: Boolean(workspaceId)
&& baseBranchQuery.isSuccess or baseBranchQuery.isFetched) so getStatus only
runs after baseBranchQuery resolves, and keep passing baseBranch ?? undefined
for the query variables; update the enabled logic in the getStatus useQuery
invocation accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 908dc355-8bf3-4f7b-8dd1-af692a595df2
📒 Files selected for processing (5)
apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tspackages/host-service/src/trpc/router/git/git.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
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="packages/host-service/src/trpc/router/git/git.ts">
<violation number="1" location="packages/host-service/src/trpc/router/git/git.ts:281">
P2: Do not swallow errors when unsetting the base branch config; this can report success even when persistence fails.
(Based on your team's feedback about handling async errors explicitly and avoiding silent empty catches.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts">
<violation number="1" location="apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts:21">
P1: `getBaseBranch` is cached with infinite staleness but never invalidated on `git:changed`, so branch switches can leave status comparisons using a stale base branch.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.ts:22">
P2: Caching `git.getBaseBranch` with infinite staleness can keep an outdated base branch after checkout changes, so sidebar diffs may compare against the wrong base.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx:35">
P2: `getBaseBranch` is cached forever here, but this value depends on the current checked-out branch and is not invalidated on general git change events. That can leave the sidebar/commit comparison using an outdated base branch after branch switches.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } else { | ||
| await git | ||
| .raw(["config", "--unset", `branch.${currentBranch}.base`]) | ||
| .catch(() => {}); |
Contributor
There was a problem hiding this comment.
P2: Do not swallow errors when unsetting the base branch config; this can report success even when persistence fails.
(Based on your team's feedback about handling async errors explicitly and avoiding silent empty catches.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/git/git.ts, line 281:
<comment>Do not swallow errors when unsetting the base branch config; this can report success even when persistence fails.
(Based on your team's feedback about handling async errors explicitly and avoiding silent empty catches.) </comment>
<file context>
@@ -231,6 +231,58 @@ export const gitRouter = router({
+ } else {
+ await git
+ .raw(["config", "--unset", `branch.${currentBranch}.base`])
+ .catch(() => {});
+ }
+ return { baseBranch: input.baseBranch };
</file context>
Suggested change
| .catch(() => {}); | |
| .catch((error) => { | |
| if (`${error}`.includes("No such section or key")) return; | |
| throw error; | |
| }); |
|
|
||
| const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery( | ||
| { workspaceId }, | ||
| { staleTime: Number.POSITIVE_INFINITY }, |
Contributor
There was a problem hiding this comment.
P2: Caching git.getBaseBranch with infinite staleness can keep an outdated base branch after checkout changes, so sidebar diffs may compare against the wrong base.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useSidebarDiffRef/useSidebarDiffRef.ts, line 22:
<comment>Caching `git.getBaseBranch` with infinite staleness can keep an outdated base branch after checkout changes, so sidebar diffs may compare against the wrong base.</comment>
<file context>
@@ -15,7 +16,12 @@ export function useSidebarDiffRef(workspaceId: string): DiffRef {
+
+ const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery(
+ { workspaceId },
+ { staleTime: Number.POSITIVE_INFINITY },
+ );
+ const baseBranch = baseBranchQuery.data?.baseBranch ?? null;
</file context>
|
|
||
| const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery( | ||
| { workspaceId }, | ||
| { staleTime: Number.POSITIVE_INFINITY }, |
Contributor
There was a problem hiding this comment.
P2: getBaseBranch is cached forever here, but this value depends on the current checked-out branch and is not invalidated on general git change events. That can leave the sidebar/commit comparison using an outdated base branch after branch switches.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx, line 35:
<comment>`getBaseBranch` is cached forever here, but this value depends on the current checked-out branch and is not invalidated on general git change events. That can leave the sidebar/commit comparison using an outdated base branch after branch switches.</comment>
<file context>
@@ -24,12 +24,17 @@ export function useChangesTab({
+
+ const baseBranchQuery = workspaceTrpc.git.getBaseBranch.useQuery(
+ { workspaceId },
+ { staleTime: Number.POSITIVE_INFINITY },
+ );
+ const baseBranch = baseBranchQuery.data?.baseBranch ?? null;
</file context>
The v2 `create` mutation used the selected base branch only as the `git worktree add` start point and never wrote it to `branch.<newBranch>.base`. Combined with the v2 sidebar deriving the base branch from that config, a freshly forked workspace's Changes tab always compared against the repo default — the user's fork-time selection was effectively dropped the moment creation finished. Record `startPoint.shortName` into the config right after the worktree is created. Skipped for "head" start points where no meaningful base exists.
V2's create and checkout mutations produced worktrees without push.autoSetupRemote set, so the first terminal `git push` failed with "no upstream branch" unless the user had the config globally. V1's equivalent path already writes this per-worktree; match that. Safe against wrong upstream targeting: --no-track (create) guarantees no upstream at first push, so auto-create always wins and always sets <remote>/<branch-name>, never the base branch. Checkout from a remote branch already has upstream via --track, so the config is a no-op there. Also rewrite the --no-track comment at the create site — it had incorrectly tied --no-track to push targeting, when --no-track is actually about suppressing `git pull` / ahead-behind counts against the start point; push targeting is governed independently by push.autoSetupRemote.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts">
<violation number="1" location="packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts:669">
P2: Using `git config --local` here applies `push.autoSetupRemote` to the entire repository, not just the new worktree. That introduces cross-worktree side effects and does not match the per-worktree behavior described in the comment.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…fig scope Address PR review: 1. P1 (CodeRabbit / cubic): getBaseBranch was cached with staleTime: Infinity but never invalidated on git:changed. An external branch switch (terminal `git checkout`) would leave the sidebar comparing against the previous branch's base. Add invalidation alongside getStatus in useGitStatus — since useGitStatus is always mounted while the Changes UI is visible, this refreshes the shared cache for all consumers (useChangesTab, useSidebarDiffRef). 2. P2 (cubic): my earlier comment claimed push.autoSetupRemote was set "per-worktree" but `git config --local` in a linked worktree writes to the shared repo config, not a worktree-specific one. Rewrite the comments to note the repo-wide scope is intentional — every superset-managed worktree wants the same ergonomics.
This was referenced Apr 16, 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
The v2 right sidebar always rendered the repo default base branch. Two independent bugs caused it:
BaseBranchSelector'scurrentValuewas hardcoded todefaultBranchNameinChangesHeader.tsx, so the user's selection never appeared in the UI.sidebarState.baseBranchfield onv2WorkspaceLocalState, never seeded at creation and divergent from the v1 DB / git-config source of truth.Fix
useChangesTab→ChangesTabContent→ChangesHeader→BaseBranchSelectorso the selector renders the real value (falls back todefaultBranchNamewhen unset).sidebarState.baseBranchentirely. Add two host-service procedures,git.getBaseBranch(readsbranch.<current>.basegit config) andgit.setBaseBranch(writes it). V2's three readers (useChangesTab,useGitStatus,useSidebarDiffRef) use the query; the selector writes via the mutation and invalidates dependent queries.Why git config as the store
branch.<name>.base) v1 already uses viagetBranchBaseConfig, so v1 and v2 stay aligned without bridging code.workspace-init.ts) already writes the config.Test plan
bun run typecheck --filter=@superset/desktop --filter=@superset/host-servicepasses.Summary by CodeRabbit
New Features
Refactor