feat(workspaces): add workspace base branch functionality#1562
Merged
Kitenite merged 16 commits intoFeb 20, 2026
Conversation
- Introduced `workspaceBaseBranch` field in the project schema to allow setting a default base branch for new workspaces. - Updated project creation and workspace handling logic to utilize the new base branch setting. - Implemented utility function `resolveWorkspaceBaseBranch` to determine the effective base branch based on user preferences and existing branches. - Added tests for the new base branch resolution logic. - Enhanced UI components to support selection and display of the workspace base branch.
- Added utility functions for managing base branch configurations, including `getBranchBaseConfig`, `setBranchBaseConfig`, and `unsetBranchBaseConfig`. - Updated the branches router to utilize the new base branch configuration methods for retrieving and setting base branches. - Refactored workspace creation procedures to leverage the new base branch utilities, ensuring consistent handling of base branch settings across workspaces. - Enhanced the workspace initialization logic to check and apply base branch configurations effectively.
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:
📝 WalkthroughWalkthroughAdd per-project Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Renderer UI
participant TRPC as TRPC Router
participant Resolver as BaseBranch Resolver
participant GitCfg as Branch Config Helpers
participant DB as Local DB
UI->>TRPC: create/open workspace (projectId, explicitBaseBranch?)
TRPC->>DB: SELECT project (includes workspace_base_branch) + branch info
DB-->>TRPC: project, defaultBranch, knownBranches
TRPC->>Resolver: resolveWorkspaceBaseBranch({ explicit, workspaceBaseBranch, defaultBranch, knownBranches })
Resolver-->>TRPC: resolved baseBranch
TRPC->>GitCfg: maybe setBranchBaseConfig(repoPath, branch, baseBranch, isExplicit)
GitCfg-->>TRPC: OK
TRPC->>DB: INSERT/UPDATE workspace/worktree with base_branch = resolved
DB-->>TRPC: OK
TRPC->>UI: return workspace/worktree (includes baseBranch)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.ts (1)
1-6: Consider exportingResolveWorkspaceBaseBranchParams.The interface is currently internal, which is fine for current call sites. Exporting it improves ergonomics for any future callers that want to construct and pass the params object in a typed way.
♻️ Proposed change
-interface ResolveWorkspaceBaseBranchParams { +export interface ResolveWorkspaceBaseBranchParams {🤖 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/base-branch.ts` around lines 1 - 6, The interface ResolveWorkspaceBaseBranchParams is currently internal; export it so callers can import and construct typed params — add the export keyword to the interface declaration (export interface ResolveWorkspaceBaseBranchParams { ... }) in base-branch.ts and update any imports where external code will use it; rebuild/typecheck to ensure no further changes are needed.apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
730-735: MovegetKnownBranchesSafepast the early-exit checks inopenExternalWorktree.
baseBranchis only consumed in the final "new worktree" insert path (Lines 840–900). The two common early-return paths —!exists(Line 742) andexistingWorktree(Lines 756–838) — both return without ever using it, so the async branch-listing call is wasted I/O in those cases.♻️ Proposed refactor
.mutation(async ({ input }) => { const project = getProject(input.projectId); if (!project) { throw new Error(`Project ${input.projectId} not found`); } - const knownBranches = await getKnownBranchesSafe(project.mainRepoPath); - const baseBranch = resolveWorkspaceBaseBranch({ - workspaceBaseBranch: project.workspaceBaseBranch, - defaultBranch: project.defaultBranch, - knownBranches, - }); const exists = await worktreeExists( project.mainRepoPath, input.worktreePath, ); if (!exists) { throw new Error("Worktree no longer exists on disk"); } const existingWorktree = localDb.select()...get(); if (existingWorktree) { // ... existing handling, early returns } + const knownBranches = await getKnownBranchesSafe(project.mainRepoPath); + const baseBranch = resolveWorkspaceBaseBranch({ + workspaceBaseBranch: project.workspaceBaseBranch, + defaultBranch: project.defaultBranch, + knownBranches, + }); + const worktree = localDb.insert(worktrees).values({ ..., baseBranch, ... });🤖 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/procedures/create.ts` around lines 730 - 735, The call to getKnownBranchesSafe (and the subsequent resolveWorkspaceBaseBranch into baseBranch) should be moved inside openExternalWorktree so it runs only when creating the new worktree: delay calling getKnownBranchesSafe(project.mainRepoPath) and resolveWorkspaceBaseBranch(...) until after the early-exit checks for repository existence (!exists) and for existingWorktree are performed and those checks decide to proceed to the "new worktree" insert path; update code around openExternalWorktree to remove the eager async call and instead compute knownBranches/baseBranch just before the insert logic that uses baseBranch.apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.test.ts (1)
4-43: Add tests for the offline (noknownBranches) and ultimate "main" fallback cases.The four existing tests cover the happy path and the stale-preference case well. Two important scenarios are missing:
- Offline case (
knownBranchesomitted): whengetKnownBranchesSafereturnsundefined(branch listing fails),workspaceBaseBranchshould still be returned as-is — this is the production safety guarantee but it's currently untested.- "main" fallback: when neither
workspaceBaseBranchnordefaultBranchis set, the function should return"main". This verifies the hardcoded last-resort constant is actually reached.✅ Suggested additional tests
+ test("uses workspace base branch when knownBranches is unavailable (offline)", () => { + const resolved = resolveWorkspaceBaseBranch({ + workspaceBaseBranch: "feature/long-lived", + defaultBranch: "main", + // knownBranches intentionally omitted — simulates branch-listing failure + }); + + expect(resolved).toBe("feature/long-lived"); + }); + + test('falls back to "main" when no defaultBranch or workspaceBaseBranch is provided', () => { + const resolved = resolveWorkspaceBaseBranch({}); + + expect(resolved).toBe("main"); + });🤖 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/base-branch.test.ts` around lines 4 - 43, Add two unit tests to apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.test.ts for resolveWorkspaceBaseBranch: one that omits knownBranches (simulating offline/getKnownBranchesSafe returning undefined) and verifies that the provided workspaceBaseBranch is returned unchanged, and another that omits both workspaceBaseBranch and defaultBranch and verifies the function returns the hardcoded "main" fallback; put them alongside existing tests and use the same resolveWorkspaceBaseBranch call pattern and expect(...) assertions.apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
65-68:getBranchesis invalidated on every project update, not only workspace-base-branch changes.
onSettledfires for allupdateProjectmutations (name renames, color changes,hideImagetoggles, etc.).getBranchesruns several git operations —git.branch -a,for-each-ref, and potentiallyrefreshDefaultBranchwhich may hit the network. Invalidating it unconditionally causes unnecessary git I/O on settings changes that have nothing to do with branches.♻️ Suggested approach: conditional invalidation
Move the
getBranchesinvalidation intohandleWorkspaceBaseBranchChangeinstead ofonSettled, or use a separate mutation for the base-branch update:const updateProject = electronTrpc.projects.update.useMutation({ onSettled: () => { utils.projects.get.invalidate({ id: projectId }); - utils.projects.getBranches.invalidate({ projectId }); utils.workspaces.getAllGrouped.invalidate(); }, });const handleWorkspaceBaseBranchChange = (value: string) => { updateProject.mutate({ id: projectId, patch: { workspaceBaseBranch: value === REPO_DEFAULT_BASE_BRANCH ? null : value, }, }); + utils.projects.getBranches.invalidate({ projectId }); };Also applies to: 88-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx around lines 65 - 68, The getBranches query is being invalidated on every updateProject onSettled, causing unnecessary git work; instead, remove the unconditional invalidation from the updateProject onSettled and perform conditional invalidation only when the workspace base branch actually changes (move the invalidate call into your handleWorkspaceBaseBranchChange handler) or create a dedicated mutation for updating the base branch that calls utils.projects.getBranches.invalidate on success; target symbols: electronTrpc.projects.getBranches.useQuery, the updateProject mutation onSettled block, and handleWorkspaceBaseBranchChange (or the new base-branch mutation) so the expensive git queries run only when the base branch is modified.
🤖 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/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 132-136: effectiveBaseBranch currently uses
project?.workspaceBaseBranch without validating it against the available
branches, causing a stale "from X" label; update the calculation of
effectiveBaseBranch to check that project?.workspaceBaseBranch exists in
branchData?.branches (e.g., by matching branch name or id) before selecting it,
otherwise fall back to branchData?.defaultBranch or null so the displayed "from
X" matches what the server will resolve; modify the logic around
effectiveBaseBranch (and any helper where project?.workspaceBaseBranch is read)
to perform this inclusion check against branchData?.branches.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/project/`$projectId/page.tsx:
- Around line 105-109: The UI uses project?.workspaceBaseBranch directly when
computing effectiveBaseBranch (const effectiveBaseBranch = baseBranch ??
project?.workspaceBaseBranch ?? branchData?.defaultBranch ?? null), which can be
stale if that branch was deleted; update the logic to verify that
project.workspaceBaseBranch exists in branchData?.branches (or matches
branchData?.defaultBranch) before using it, otherwise fall back to
branchData?.defaultBranch (and then null). Apply the same guard to the
equivalent computation in NewWorkspaceModal (the workspace base branch
preference check around lines 132–136) so both places only show a "from X" label
when the branch actually exists in branchData.branches.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 205-209: The computed workspaceBaseBranchMissing currently treats
undefined branchData as "missing"; update the logic in the ProjectSettings
component so workspaceBaseBranchMissing only evaluates true when branchData is
available and the branch truly doesn't exist — e.g. guard on branchData (or
branchData.branches) before calling .some, or check a loading state (branchData
=== undefined or isLoading) and return false in that case; adjust the expression
that defines workspaceBaseBranchMissing (referencing workspaceBaseBranchMissing,
project.workspaceBaseBranch, and branchData?.branches) accordingly so the
`(missing)` label isn't shown while the query is in flight.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 730-735: The call to getKnownBranchesSafe (and the subsequent
resolveWorkspaceBaseBranch into baseBranch) should be moved inside
openExternalWorktree so it runs only when creating the new worktree: delay
calling getKnownBranchesSafe(project.mainRepoPath) and
resolveWorkspaceBaseBranch(...) until after the early-exit checks for repository
existence (!exists) and for existingWorktree are performed and those checks
decide to proceed to the "new worktree" insert path; update code around
openExternalWorktree to remove the eager async call and instead compute
knownBranches/baseBranch just before the insert logic that uses baseBranch.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.test.ts`:
- Around line 4-43: Add two unit tests to
apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.test.ts for
resolveWorkspaceBaseBranch: one that omits knownBranches (simulating
offline/getKnownBranchesSafe returning undefined) and verifies that the provided
workspaceBaseBranch is returned unchanged, and another that omits both
workspaceBaseBranch and defaultBranch and verifies the function returns the
hardcoded "main" fallback; put them alongside existing tests and use the same
resolveWorkspaceBaseBranch call pattern and expect(...) assertions.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch.ts`:
- Around line 1-6: The interface ResolveWorkspaceBaseBranchParams is currently
internal; export it so callers can import and construct typed params — add the
export keyword to the interface declaration (export interface
ResolveWorkspaceBaseBranchParams { ... }) in base-branch.ts and update any
imports where external code will use it; rebuild/typecheck to ensure no further
changes are needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 65-68: The getBranches query is being invalidated on every
updateProject onSettled, causing unnecessary git work; instead, remove the
unconditional invalidation from the updateProject onSettled and perform
conditional invalidation only when the workspace base branch actually changes
(move the invalidate call into your handleWorkspaceBaseBranchChange handler) or
create a dedicated mutation for updating the base branch that calls
utils.projects.getBranches.invalidate on success; target symbols:
electronTrpc.projects.getBranches.useQuery, the updateProject mutation onSettled
block, and handleWorkspaceBaseBranchChange (or the new base-branch mutation) so
the expensive git queries run only when the base branch is modified.
…on logic - Added `resolveEffectiveWorkspaceBaseBranch` utility to determine the most appropriate base branch based on user-defined preferences and existing branches. - Updated `NewWorkspaceModal` and `ProjectPage` components to utilize the new base branch resolution logic for improved workspace handling. - Introduced tests for the new utility to ensure correct functionality across various scenarios. - Refactored related components to enhance clarity and maintainability.
…eate procedures - Moved base branch resolution logic to a more appropriate location within the workspace creation procedures for improved clarity and maintainability. - Updated tests for `resolveWorkspaceBaseBranch` to cover additional scenarios, ensuring robust handling of base branch selection. - Adjusted ProjectSettings component to correctly invalidate branch data after updates, enhancing data consistency.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
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/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
374-384:⚠️ Potential issue | 🟡 Minor
"from null"/"from "renders wheneffectiveBaseBranchisnullduring branch loading.
resolveEffectiveWorkspaceBaseBranchreturnsstring | null. WhilebranchDatais still loading, all three inputs (explicitBaseBranch,workspaceBaseBranchpresence in branches,defaultBranch) can be absent, causingnull. React rendersnullas empty text, so the label shows "from " with nothing after it.🐛 Proposed fix
- <span className="text-muted-foreground/60"> - from {effectiveBaseBranch} - </span> + {effectiveBaseBranch && ( + <span className="text-muted-foreground/60"> + from {effectiveBaseBranch} + </span> + )}
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
165-173:getBranchesinvalidation is unnecessary here.Changing
workspaceBaseBranchdoes not alter the repository's branch list.updateProject.onSettledalready invalidatesprojects.get, which carries the updatedworkspaceBaseBranch. ThegetBranchesinvalidation causes a spurious network round-trip.♻️ Proposed fix
const handleWorkspaceBaseBranchChange = (value: string) => { updateProject.mutate({ id: projectId, patch: { workspaceBaseBranch: value === REPO_DEFAULT_BASE_BRANCH ? null : value, }, }); - utils.projects.getBranches.invalidate({ projectId }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx around lines 165 - 173, The call to utils.projects.getBranches.invalidate inside handleWorkspaceBaseBranchChange is unnecessary and causes an extra network request; remove that invalidate call and keep the updateProject.mutate call as-is so updateProject.onSettled can rely on its existing invalidation of projects.get to propagate the updated workspaceBaseBranch. Specifically, edit the handleWorkspaceBaseBranchChange function to drop utils.projects.getBranches.invalidate({ projectId }) and only perform updateProject.mutate({ id: projectId, patch: { workspaceBaseBranch: value === REPO_DEFAULT_BASE_BRANCH ? null : value } });.apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)
978-983: All imported external worktrees receive the same resolvedbaseBranch.For pre-existing DB worktrees (loop at lines 994–1026) the DB
baseBranchis preserved (no update), which is correct. For external worktrees not yet in DB, the project-levelbaseBranchis the only available signal, so the approximation is the best achievable here. Worth a comment to document the intent.📝 Suggested inline comment
const knownBranches = await getKnownBranchesSafe(project.mainRepoPath); const baseBranch = resolveWorkspaceBaseBranch({ workspaceBaseBranch: project.workspaceBaseBranch, defaultBranch: project.defaultBranch, knownBranches, }); + // baseBranch is a project-level approximation; individual worktree origins are unknown for external imports.🤖 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/procedures/create.ts` around lines 978 - 983, The code resolves a single project-level baseBranch using getKnownBranchesSafe and resolveWorkspaceBaseBranch which is then applied to all imported external worktrees; add a concise inline comment near the block that computes baseBranch (around the getKnownBranchesSafe / resolveWorkspaceBaseBranch calls) explaining that for external worktrees not yet present in the DB we must approximate with the project-level baseBranch and that pre-existing DB worktrees keep their stored baseBranch (the loop handling DB entries preserves baseBranch), so the shared resolved value is intentional and not a bug.apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch-config.ts (1)
13-16: Consider exportingBranchBaseConfigfor consumer type annotations.The interface is the return type of the exported
getBranchBaseConfigbut is not exported itself. Callers that want to explicitly type-annotate the result (e.g., in destructuring patterns like atworkspace-init.tsline 68) must inline the shape or useAwaited<ReturnType<typeof getBranchBaseConfig>>.♻️ Proposed fix
-interface BranchBaseConfig { +export interface BranchBaseConfig { baseBranch: string | null; isExplicit: boolean; }🤖 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/base-branch-config.ts` around lines 13 - 16, Export the BranchBaseConfig interface so callers can use it for explicit type annotations instead of inlining or using Awaited<ReturnType<typeof getBranchBaseConfig>>; update the interface declaration for BranchBaseConfig to be exported and keep getBranchBaseConfig's signature intact so existing consumers can import BranchBaseConfig and annotate results (e.g., const config: BranchBaseConfig = await getBranchBaseConfig(...)).apps/desktop/src/renderer/lib/workspaceBaseBranch/workspaceBaseBranch.test.ts (1)
36-42: Consider adding edge-case tests forbranches: []and isolateddefaultBranch.Two realistic scenarios currently untested:
Empty branches array — a consumer passes
branches: [](e.g., branches haven't been fetched yet). SinceworkspaceBaseBranchrequires an existence match inbranches, the outcome would differ frombranches: undefined, but both are currently covered only by the latter.
defaultBranchnot inbranches— the current test always hasdefaultBranch: "main"present inbranches. The function description showsdefaultBranchis returned vianormalizeBranchregardless of whether it exists inbranches, which is asymmetric to howworkspaceBaseBranchis handled. A test would document this intentional asymmetry.✅ Suggested additional tests
+ test("falls back to default branch when branches is empty", () => { + const resolved = resolveEffectiveWorkspaceBaseBranch({ + workspaceBaseBranch: "feature/preferred", + defaultBranch: "main", + branches: [], + }); + + expect(resolved).toBe("main"); + }); + + test("returns default branch even when it is absent from branches list", () => { + const resolved = resolveEffectiveWorkspaceBaseBranch({ + workspaceBaseBranch: "feature/deleted", + defaultBranch: "main", + branches: [{ name: "feature/other" }], + }); + + expect(resolved).toBe("main"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/workspaceBaseBranch/workspaceBaseBranch.test.ts` around lines 36 - 42, Add two unit tests for resolveEffectiveWorkspaceBaseBranch: one where the input has branches: [] and workspaceBaseBranch set (e.g., "feature/deleted") asserting it returns null (to cover the case where branches are present but empty), and another where defaultBranch is set to a name not present in branches (e.g., defaultBranch: "main", branches: [{name: "other"}]) asserting the function returns normalizeBranch(defaultBranch) (to document the asymmetry between defaultBranch and workspaceBaseBranch handling). Use the existing test structure and reference resolveEffectiveWorkspaceBaseBranch and normalizeBranch when adding assertions.
🤖 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/workspace-init.ts`:
- Around line 249-254: resolveLocalRef currently calls setBranchBaseConfig to
update git config when effectiveBaseBranch falls back to result.fallbackBranch,
but it doesn't persist that fallback into the DB so worktrees.baseBranch remains
stale; after calling setBranchBaseConfig (the block updating git config for
repoPath/mainRepoPath, branch and baseBranch: result.fallbackBranch), also
update the worktree record in the DB to set worktrees.baseBranch =
result.fallbackBranch (i.e., persist the configuredBaseBranch into
persistedBaseBranch) so that the DB mirrors the git config and
getBranches/persistedBaseBranch stay consistent if the git config is later
cleared.
In `@apps/desktop/src/renderer/lib/workspaceBaseBranch/workspaceBaseBranch.ts`:
- Around line 28-33: The renderer currently falls back to defaultBranch when
branches is undefined because preferredExists uses branches?.some(...); change
the logic in the workspaceBaseBranch resolution so that if
normalizeBranch(workspaceBaseBranch) returns a preferred value and branches is
undefined (loading/unavailable) you return that preferred value
immediately—i.e., treat missing branches the same as the server's
resolveWorkspaceBaseBranch by checking for branches presence first and only
performing the branches.some(...) check when branches is defined; update the
code around normalizeBranch, preferredExists, and the return path so preferred
is returned when branches === undefined or when branches contains the preferred
branch.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 978-983: The code resolves a single project-level baseBranch using
getKnownBranchesSafe and resolveWorkspaceBaseBranch which is then applied to all
imported external worktrees; add a concise inline comment near the block that
computes baseBranch (around the getKnownBranchesSafe /
resolveWorkspaceBaseBranch calls) explaining that for external worktrees not yet
present in the DB we must approximate with the project-level baseBranch and that
pre-existing DB worktrees keep their stored baseBranch (the loop handling DB
entries preserves baseBranch), so the shared resolved value is intentional and
not a bug.
In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/base-branch-config.ts`:
- Around line 13-16: Export the BranchBaseConfig interface so callers can use it
for explicit type annotations instead of inlining or using
Awaited<ReturnType<typeof getBranchBaseConfig>>; update the interface
declaration for BranchBaseConfig to be exported and keep getBranchBaseConfig's
signature intact so existing consumers can import BranchBaseConfig and annotate
results (e.g., const config: BranchBaseConfig = await getBranchBaseConfig(...)).
In
`@apps/desktop/src/renderer/lib/workspaceBaseBranch/workspaceBaseBranch.test.ts`:
- Around line 36-42: Add two unit tests for resolveEffectiveWorkspaceBaseBranch:
one where the input has branches: [] and workspaceBaseBranch set (e.g.,
"feature/deleted") asserting it returns null (to cover the case where branches
are present but empty), and another where defaultBranch is set to a name not
present in branches (e.g., defaultBranch: "main", branches: [{name: "other"}])
asserting the function returns normalizeBranch(defaultBranch) (to document the
asymmetry between defaultBranch and workspaceBaseBranch handling). Use the
existing test structure and reference resolveEffectiveWorkspaceBaseBranch and
normalizeBranch when adding assertions.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 165-173: The call to utils.projects.getBranches.invalidate inside
handleWorkspaceBaseBranchChange is unnecessary and causes an extra network
request; remove that invalidate call and keep the updateProject.mutate call
as-is so updateProject.onSettled can rely on its existing invalidation of
projects.get to propagate the updated workspaceBaseBranch. Specifically, edit
the handleWorkspaceBaseBranchChange function to drop
utils.projects.getBranches.invalidate({ projectId }) and only perform
updateProject.mutate({ id: projectId, patch: { workspaceBaseBranch: value ===
REPO_DEFAULT_BASE_BRANCH ? null : value } });.
- Bump the version of the @superset/desktop package in bun.lock to ensure compatibility with the latest features and fixes. - Refactor workspace base branch resolution logic in workspace-init.ts for improved clarity. - Enhance NewWorkspaceModal to handle undefined effectiveBaseBranch gracefully. - Add tests for edge cases in resolveEffectiveWorkspaceBaseBranch to ensure robust functionality. - Update ProjectSettings to notify users when the specified workspace base branch is missing.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
65-69:enabled: !!projectIdis redundant —projectIdis always a non-emptystring.
ProjectSettingsPropsdeclaresprojectId: stringas a required prop, so!!projectIdis alwaystrue. The guard adds noise without protecting against a real case.♻️ Proposed simplification
- const { data: branchData, isLoading: isBranchDataLoading } = - electronTrpc.projects.getBranches.useQuery( - { projectId }, - { enabled: !!projectId }, - ); + const { data: branchData, isLoading: isBranchDataLoading } = + electronTrpc.projects.getBranches.useQuery({ projectId });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx around lines 65 - 69, The query call uses a redundant boolean cast for the enabled option: remove the unnecessary guard by changing electronTrpc.projects.getBranches.useQuery({ projectId }, { enabled: !!projectId }) to simply pass enabled: true (or omit the enabled option) because ProjectSettingsProps defines projectId as a required string; update the call site in ProjectSettings (the electronTrpc.projects.getBranches.useQuery invocation) so it no longer uses !!projectId.
🤖 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/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 287-310: The Select can show a blank value when branches are
loading because no SelectItem matches workspaceBaseBranchValue; update the
Select's disabled prop to also disable while branch list is loading by changing
disabled={updateProject.isPending} to disabled={updateProject.isPending ||
isBranchDataLoading} (in the Select that uses workspaceBaseBranchValue and
handleWorkspaceBaseBranchChange) so the control is not interactive or rendered
with a missing selection until branchData.branches is available.
---
Duplicate comments:
In `@apps/desktop/src/renderer/lib/workspaceBaseBranch/workspaceBaseBranch.ts`:
- Around line 28-36: The offline resolver should mirror the server behavior by
returning the normalized preferred branch when the branch list is missing;
ensure the early-return guard in workspaceBaseBranch (use normalizeBranch to get
preferred, then if (preferred) { if (!branches) return preferred; if
(branches.some(branch => branch.name === preferred)) return preferred; })
remains intact so we short-circuit to preferred when branches is undefined and
only fall back to defaultBranch when preferred is not present in branches.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 205-211: The earlier flashing issue is resolved: the computation
for workspaceBaseBranchMissing now correctly guards with !isBranchDataLoading
and !!branchData before calling branchData.branches.some(), so no change is
required—confirm the variable workspaceBaseBranchMissing (and the related guards
isBranchDataLoading and branchData) remain as shown and remove any outdated
TODO/concern comments referencing the flashing label.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 65-69: The query call uses a redundant boolean cast for the
enabled option: remove the unnecessary guard by changing
electronTrpc.projects.getBranches.useQuery({ projectId }, { enabled: !!projectId
}) to simply pass enabled: true (or omit the enabled option) because
ProjectSettingsProps defines projectId as a required string; update the call
site in ProjectSettings (the electronTrpc.projects.getBranches.useQuery
invocation) so it no longer uses !!projectId.
- Added logic to update the base branch of the worktree in the local database during workspace initialization. - Ensures that the correct fallback branch is set for the worktree, improving workspace setup reliability.
- Updated the ProjectSettings component to disable the workspace base branch selection when branch data is loading, improving user experience and preventing potential errors during updates.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
27-27: Sentinel collision if a branch is named__repo_default__.A branch named
"__repo_default__"would appear as a regular<SelectItem>alongside the sentinel item — both sharing the samevalue. When selected,handleWorkspaceBaseBranchChangemaps it tonull, silently discarding the user's intended branch choice. Filtering the sentinel name out of the mapped list eliminates the collision at zero runtime cost.♻️ Proposed fix
- {(branchData?.branches ?? []).map((branch) => ( - <SelectItem key={branch.name} value={branch.name}> - {branch.name} - </SelectItem> - ))} + {(branchData?.branches ?? []) + .filter((branch) => branch.name !== REPO_DEFAULT_BASE_BRANCH) + .map((branch) => ( + <SelectItem key={branch.name} value={branch.name}> + {branch.name} + </SelectItem> + ))}Also applies to: 296-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx at line 27, The sentinel constant REPO_DEFAULT_BASE_BRANCH("__repo_default__") collides with a real branch name; update the selection handling in handleWorkspaceBaseBranchChange (and the mapped selection logic around the SelectItem rendering at lines ~296-309) to first filter out any values equal to REPO_DEFAULT_BASE_BRANCH from the selected values array, then map the remaining values to branches (or null only when the filtered array is empty) so a real branch named "__repo_default__" is preserved and the sentinel is only used as UI placeholder.
🤖 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/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 287-295: Select currently shows blank because SelectValue has no
placeholder while branchData?.branches is empty during isBranchDataLoading;
update the Select usage (the component rendering Select, SelectValue and the
SelectItem list) to pass a placeholder to SelectValue (e.g., "Loading branches…"
or a localized string) when isBranchDataLoading or when workspaceBaseBranchValue
is not found in branchData?.branches, so the control displays a loading hint
instead of empty; reference Select, SelectValue, workspaceBaseBranchValue,
isBranchDataLoading, and branchData?.branches to locate where to add the
conditional placeholder logic.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Line 27: The sentinel constant REPO_DEFAULT_BASE_BRANCH("__repo_default__")
collides with a real branch name; update the selection handling in
handleWorkspaceBaseBranchChange (and the mapped selection logic around the
SelectItem rendering at lines ~296-309) to first filter out any values equal to
REPO_DEFAULT_BASE_BRANCH from the selected values array, then map the remaining
values to branches (or null only when the filtered array is empty) so a real
branch named "__repo_default__" is preserved and the sentinel is only used as UI
placeholder.
- Updated the ProjectSettings component to display a loading placeholder in the branch selection when branch data is being loaded, enhancing user feedback during data retrieval.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
802-802: Consider adding.min(1)to prevent empty-string persistence.The
z.string().nullable().optional()schema accepts empty strings. While consuming code (normalizeBranch()) treats empty/falsy values as undefined at resolution time, it's cleaner to reject them at the validation boundary for consistency with other branch-name fields.Suggested improvement
- workspaceBaseBranch: z.string().nullable().optional(), + workspaceBaseBranch: z.string().min(1).nullable().optional(),The Drizzle
.set()call and DB column (workspace_base_branchin@superset/local-db) are correctly wired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` at line 802, The schema for workspaceBaseBranch currently allows empty strings because it's declared as workspaceBaseBranch: z.string().nullable().optional(); update this validation to reject empty strings by adding .min(1) (e.g., z.string().min(1).nullable().optional()) so empty-string values are rejected at the validation boundary; ensure this change is applied where the workspaceBaseBranch schema is defined and that any callers relying on normalizeBranch() still handle nullable/optional values.
🤖 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/projects/projects.ts`:
- Line 802: The schema for workspaceBaseBranch currently allows empty strings
because it's declared as workspaceBaseBranch: z.string().nullable().optional();
update this validation to reject empty strings by adding .min(1) (e.g.,
z.string().min(1).nullable().optional()) so empty-string values are rejected at
the validation boundary; ensure this change is applied where the
workspaceBaseBranch schema is defined and that any callers relying on
normalizeBranch() still handle nullable/optional values.
- Updated the ProjectSettings component to show a loading message instead of a placeholder when branch data is being loaded, enhancing user feedback during data retrieval.
switchBranch was updating worktrees.branch without clearing worktrees.baseBranch, causing the persisted fallback in getBranches to return the previous branch's base branch after switching.
# Conflicts: # packages/local-db/drizzle/meta/0028_snapshot.json # packages/local-db/drizzle/meta/_journal.json
This was referenced Mar 18, 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
2 participants
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.
workspaceBaseBranchfield in the project schema to allow setting a default base branch for new workspaces.resolveWorkspaceBaseBranchto determine the effective base branch based on user preferences and existing branches.Screen.Recording.2026-02-19.at.3.58.12.PM.mov
Description
Fixes #1426
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
UI
Data & Sync
Tests
Chores