Skip to content

fix(desktop): compare changes against worktree base branch - #1366

Merged
Kitenite merged 8 commits into
mainfrom
kitenite/base-branch
Feb 17, 2026
Merged

Kitenite merged 8 commits into
mainfrom
kitenite/base-branch

Conversation

@Kitenite

@Kitenite Kitenite commented Feb 10, 2026 •

Copy link
Copy Markdown
Member

Summary

  • When creating a workspace from a non-default base branch (e.g. feature-x instead of main), the changes view incorrectly compared against origin/main, showing all inherited commits from the base branch
  • Added worktreeBaseBranch field to the getBranches query response by looking up the worktree's stored baseBranch from the local DB
  • Updated the effectiveBaseBranch fallback chain in all 5 renderer locations to: store override → worktree base branch → repo default branch → "main"

Test plan

  • Create a workspace from main → changes should show 0 "against base" items (unchanged behavior)
  • Create workspace A from main, make a commit → changes show 1 commit against main
  • Create workspace B from workspace A's branch as base → changes should show 0 "against base" items and 0 commits
  • In workspace B, manually switch base branch via the selector to main → should show inherited commits (store override still works)

Summary by CodeRabbit

  • New Features
    • Per-branch base-branch can be set or cleared from the UI; diffs and status views prefer the configured base branch.
  • Refactor
    • Base-branch tracking now persists in Git config (per-branch) rather than local workspace fields; new worktrees set this config when created.
  • Bug Fixes
    • Secrets settings effect dependency fixed so mutation updates trigger reliably.

… repo default

When creating a workspace from a non-default base branch (e.g. feature-x),
the changes view compared against origin/main instead of origin/feature-x,
causing inherited commits to appear. Now the getBranches query returns the
worktree's stored baseBranch, and all effectiveBaseBranch computations use
it as a fallback before the repo default branch.
@coderabbitai

coderabbitai Bot commented Feb 10, 2026 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Reads and writes per-worktree base branch via git config (branch..base); exposes worktreeBaseBranch in branches API and adds updateBaseBranch mutation. UI and hooks now prefer worktreeBaseBranch; DB-backed baseBranch fields and detection logic were removed.

Changes

Cohort / File(s) Summary
Branches API & mutation
apps/desktop/src/lib/trpc/routers/changes/branches.ts
Adds `worktreeBaseBranch: string
UI: base-branch preference updates
apps/desktop/src/renderer/screens/main/components/WorkspaceView/.../useFileContent/useFileContent.ts, apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/ChangesView.tsx, apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx, apps/desktop/src/renderer/screens/main/hooks/useGitChangesStatus/useGitChangesStatus.ts
Resolve effective base branch by preferring branchData?.worktreeBaseBranch before defaultBranch/"main". ChangesHeader now uses updateBaseBranch mutation (invalidates branch list); useGitChangesStatus returns refetch and uses worktreeBaseBranch.
Workspace init/create & worktree base handling
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts, apps/desktop/src/lib/trpc/routers/workspaces/procedures/init.ts, apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts, apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts
Removes baseBranch/baseBranchWasExplicit from workspace payloads and init params; moves persistence to git config (set branch.<branch>.base on create/open) and removes DB-based detection/update logic. workspace-init now reads git config and exposes baseBranchWasConfigured.
GitHub PR resolution simplification
apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts
Drops commit-based PR lookup (findPRByHeadCommit); getPRForBranch relies solely on branch-tracking lookup and returns null when nothing found.
Projects init branch detection
apps/desktop/src/lib/trpc/routers/projects/projects.ts
Uses getCurrentBranch(input.path) (fallback to "main") instead of git.branch() when initializing a repo.
Misc UI dependency fix
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/SecretsSettings.tsx
Adds linkToNeon to a useEffect dependency array so the effect re-runs when the mutation object reference changes.

Sequence Diagram(s)

sequenceDiagram
  participant User as "User (UI)"
  participant UI as "Renderer UI"
  participant API as "TRPC Router"
  participant Git as "Local Git"
  participant DB as "Local DB"

  User->>UI: set/unset base branch
  UI->>API: updateBaseBranch({ worktreePath, baseBranch })
  API->>Git: getCurrentBranch(worktreePath)
  API->>Git: git config set|--unset branch.{current}.base
  Git-->>API: result
  API-->>UI: { success: true }

  UI->>API: getBranches / getStatus
  API->>DB: read worktree record
  API->>Git: git config branch.{current}.base (read)
  DB-->>API: worktree record
  Git-->>API: worktreeBaseBranch value or none
  API-->>UI: branches[] with worktreeBaseBranch
  UI->>UI: effectiveBaseBranch = worktreeBaseBranch ?? defaultBranch ?? "main"
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I nibble configs, soft and spry,
I tuck a base branch where branches lie.
Worktrees hum with their chosen name,
UI asks git and gets the same.
Hooray — a hop, then off I fly.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing the comparison of changes against the worktree base branch instead of the default branch.
Description check ✅ Passed The description comprehensively covers the problem, solution, changes made, and provides a detailed test plan matching the template's key sections.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch kitenite/base-branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)

42-59: ⚠️ Potential issue | 🔴 Critical

Bug: Selecting the repo default branch from the dropdown is silently overridden by worktreeBaseBranch.

When the user explicitly picks the default branch (e.g. main) from the selector, handleBranchSelect sets the store to null (Line 55). With the new fallback chain, null ?? worktreeBaseBranch ?? defaultBranch resolves to worktreeBaseBranch — not the branch the user just selected. The checkmark indicator (Line 99) will also land on the wrong item.

This applies to the test plan scenario: "In workspace B, manually switch base branch via the selector to main → should show inherited commits" — that won't work when worktreeBaseBranch is something other than main.

The fix is to store the explicit branch name when it differs from the worktree base branch, and only clear to null when the user picks the worktree base branch (the new natural default):

Proposed fix
 const handleBranchSelect = (branch: string) => {
-  if (branch === branchData?.defaultBranch) {
+  if (branch === (branchData?.worktreeBaseBranch ?? branchData?.defaultBranch)) {
     setBaseBranch(worktreePath, null);
   } else {
     setBaseBranch(worktreePath, branch);
   }
 };
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useFileContent/useFileContent.ts (1)

33-34: Pre-existing inconsistency: store-level base-branch override is not consulted here.

The other four components use baseBranch ?? worktreeBaseBranch ?? defaultBranch ?? "main", where baseBranch comes from useChangesStore().getBaseBranch(...). This hook skips the store entirely, so if a user manually switches the base branch via the dropdown in ChangesHeader, the file diff for "against-base" will still compare against the worktree base branch rather than the user's choice.

Consider threading the store's baseBranch into this hook (e.g. as an optional param) for full consistency.

# Conflicts:
#	apps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsx
#	apps/desktop/src/renderer/screens/main/components/WorkspaceView/ChangesContent/ChangesContent.tsx
#	apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

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/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/SecretsSettings.tsx (1)

59-66: ⚠️ Potential issue | 🔴 Critical

Adding linkToNeon to the dependency array will cause repeated/infinite effect executions.

useMutation() returns a new object reference whenever its internal state changes (e.g., isPending, data). Including the whole linkToNeon object in the deps means:

  1. Effect fires → calls linkToNeon.mutate()
  2. Mutation starts → isPending flips → new linkToNeon reference
  3. Effect re-fires → calls mutate() again → loop

linkToNeon.mutate (already listed) is the only part the effect consumes and is the stable reference you need.

Proposed fix
-	}, [suggestedMatch, linkToNeon.mutate, projectId, linkToNeon]);
+	}, [suggestedMatch, linkToNeon.mutate, 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/cloud/secrets/components/SecretsSettings/SecretsSettings.tsx
around lines 59 - 66, The effect in useEffect that calls linkToNeon.mutate when
suggestedMatch exists currently includes the entire linkToNeon object in its
dependency array which can change identity on mutation state and cause a re-run
loop; change the deps to only include suggestedMatch, linkToNeon.mutate, and
projectId (remove linkToNeon) so the effect only triggers when the actual inputs
used (suggestedMatch and projectId) or the stable mutate function change,
preserving the existing call to linkToNeon.mutate inside the effect.
🤖 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/changes/branches.ts`:
- Around line 103-130: The updateBaseBranch mutation currently writes an empty
string into git config which later gets treated as unset by getBranches; change
updateBaseBranch in apps/desktop/src/lib/trpc/routers/changes/branches.ts
(function updateBaseBranch) to treat an empty-string baseBranch as a request to
unset: if input.baseBranch === "" then call git.raw(["config", "--unset",
`branch.${currentBranch}.base`]) (instead of writing it) or validate and throw
when baseBranch is an empty string if you prefer to enforce non-empty values;
ensure the code path that currently writes input.baseBranch (git.raw(["config",
`branch.${currentBranch}.base`, input.baseBranch])) only runs for non-empty,
non-null strings.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/cloud/secrets/components/SecretsSettings/SecretsSettings.tsx:
- Around line 59-66: The effect in useEffect that calls linkToNeon.mutate when
suggestedMatch exists currently includes the entire linkToNeon object in its
dependency array which can change identity on mutation state and cause a re-run
loop; change the deps to only include suggestedMatch, linkToNeon.mutate, and
projectId (remove linkToNeon) so the effect only triggers when the actual inputs
used (suggestedMatch and projectId) or the stable mutate function change,
preserving the existing call to linkToNeon.mutate inside the effect.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts (1)

92-129: Stale comments still reference removed commit-based fallback.

The JSDoc on Line 95 ("undefined (branch name mismatch — caller should try commit-based)") and the inline comment on Line 124 ("signal caller to try commit-based") describe the old two-step lookup that was just removed. Now that the caller simply returns null, these comments are misleading.

Consider also simplifying getPRByBranchTracking to return null instead of undefined for the "no PR found" case, since the distinction no longer matters after the fallback removal.

♻️ Suggested cleanup

Update the JSDoc:

 /**
  * Looks up a PR using `gh pr view` (no args), which matches via the branch's
  * tracking ref. Essential for fork PRs that track refs/pull/XXX/head.
- * Returns the PR, null (no match), or undefined (branch name mismatch — caller should try commit-based).
+ * Returns the PR or null if no matching PR is found.
  */
 async function getPRByBranchTracking(
   worktreePath: string,
-): Promise<GitHubStatus["pr"] | undefined> {
+): Promise<GitHubStatus["pr"]> {

Update the catch block:

     } catch (error) {
       if (
         error instanceof Error &&
         error.message.includes("no pull requests found")
       ) {
-        // Branch name didn't match any PR — signal caller to try commit-based
-        return undefined;
+        return null;
       }
       throw error;
     }

And simplify the caller:

-  return branchResult ?? null;
+  return branchResult;
🤖 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/github/github.ts` around
lines 92 - 129, The JSDoc and inline comments in getPRByBranchTracking are
outdated (they reference a removed commit-based fallback) and the function
currently returns undefined in the catch path; update getPRByBranchTracking's
JSDoc to state it returns a PR object or null (no match/branch mismatch) and
remove references to "try commit-based", change the catch branch that checks
error.message.includes("no pull requests found") to return null instead of
undefined, and update any callers that inspect undefined to treat null as the
single "no PR found" sentinel (references: function getPRByBranchTracking,
parsePRResponse, sharesAncestry, formatPRData).
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)

210-218: Remove the dead baseBranch writes from the worktrees table.

The baseBranch field is written in three places (lines 216, 432, 815) but never read from the database—no code accesses worktree.baseBranch after loading from the DB. Since workspace-init.ts reads exclusively from git config (branch.<name>.base), these writes serve no purpose.

Original snippet
	const worktree = localDb
		.insert(worktrees)
		.values({
			projectId: project.id,
			path: worktreePath,
			branch: localBranchName,
			baseBranch: defaultBranch,
			gitStatus: null,
		})
🤖 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 210 - 218, The code is writing a dead baseBranch field into the worktrees
table (seen in the localDb.insert(worktrees).values call that assigns
baseBranch) although nothing reads worktree.baseBranch; remove all assignments
of baseBranch from worktree writes (the localDb.insert(worktrees).values call
that produces worktree and any other insert/update calls that set baseBranch) so
the DB writes no longer include that property; also update any related object
literals or type usages in this file that construct the worktree payload to omit
baseBranch (adjusting types if needed).
🤖 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/changes/branches.ts`:
- Around line 98-126: The mutation updateBaseBranch currently uses rev-parse to
get currentBranch and will read/write branch.HEAD.base when in a detached HEAD;
guard against detached HEAD by checking the resolved currentBranch value (the
variable currentBranch from the git.raw call) and aborting early (throwing a
meaningful error or returning { success: false }) when it equals "HEAD" (or
otherwise indicates detached state). Keep the existing assertRegisteredWorktree
and simpleGit usage, perform the detached check immediately after computing
currentBranch, and only run the git.config/read/write logic if the branch is not
"HEAD".

---

Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 210-218: The code is writing a dead baseBranch field into the
worktrees table (seen in the localDb.insert(worktrees).values call that assigns
baseBranch) although nothing reads worktree.baseBranch; remove all assignments
of baseBranch from worktree writes (the localDb.insert(worktrees).values call
that produces worktree and any other insert/update calls that set baseBranch) so
the DB writes no longer include that property; also update any related object
literals or type usages in this file that construct the worktree payload to omit
baseBranch (adjusting types if needed).

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts`:
- Around line 92-129: The JSDoc and inline comments in getPRByBranchTracking are
outdated (they reference a removed commit-based fallback) and the function
currently returns undefined in the catch path; update getPRByBranchTracking's
JSDoc to state it returns a PR object or null (no match/branch mismatch) and
remove references to "try commit-based", change the catch branch that checks
error.message.includes("no pull requests found") to return null instead of
undefined, and update any callers that inspect undefined to treat null as the
single "no PR found" sentinel (references: function getPRByBranchTracking,
parsePRResponse, sharesAncestry, formatPRData).

Comment on lines +98 to +126
updateBaseBranch: publicProcedure
.input(
z.object({
worktreePath: z.string(),
baseBranch: z.string().nullable(),
}),
)
.mutation(async ({ input }): Promise<{ success: boolean }> => {
assertRegisteredWorktree(input.worktreePath);

const git = simpleGit(input.worktreePath);
const currentBranch = (
await git.raw(["rev-parse", "--abbrev-ref", "HEAD"])
).trim();

if (input.baseBranch) {
await git.raw([
"config",
`branch.${currentBranch}.base`,
input.baseBranch,
]);
} else {
await git
.raw(["config", "--unset", `branch.${currentBranch}.base`])
.catch(() => {});
}

return { success: true };
}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Detached HEAD would write a nonsensical branch.HEAD.base config key.

If the worktree is in a detached HEAD state, rev-parse --abbrev-ref HEAD returns the literal string "HEAD". The mutation would then read/write branch.HEAD.base in git config, which is meaningless and pollutes the config. Consider guarding against this:

Proposed guard
 			.mutation(async ({ input }): Promise<{ success: boolean }> => {
 				assertRegisteredWorktree(input.worktreePath);
 
 				const git = simpleGit(input.worktreePath);
 				const currentBranch = (
 					await git.raw(["rev-parse", "--abbrev-ref", "HEAD"])
 				).trim();
 
+				if (currentBranch === "HEAD") {
+					throw new Error("Cannot update base branch while in detached HEAD state");
+				}
+
 				if (input.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/changes/branches.ts` around lines 98 - 126,
The mutation updateBaseBranch currently uses rev-parse to get currentBranch and
will read/write branch.HEAD.base when in a detached HEAD; guard against detached
HEAD by checking the resolved currentBranch value (the variable currentBranch
from the git.raw call) and aborting early (throwing a meaningful error or
returning { success: false }) when it equals "HEAD" (or otherwise indicates
detached state). Keep the existing assertRegisteredWorktree and simpleGit usage,
perform the detached check immediately after computing currentBranch, and only
run the git.config/read/write logic if the branch is not "HEAD".

…current

simpleGit's .branch(["-a"]).current returns an empty string, causing
git config lookups with an empty branch name. Replace all usages with
the existing getCurrentBranch() helper that uses rev-parse. Also clean
up stale comments.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/lib/trpc/routers/workspaces/utils/workspace-init.ts (1)

140-146: ⚠️ Potential issue | 🟠 Major

Bug: base_branch should be effectiveBaseBranch, not branch.

In the useExistingBranch path, branch is the workspace's own branch — not its base. Compare with line 439 which correctly uses effectiveBaseBranch. This will report misleading analytics for workspaces created from existing branches.

Proposed fix
 		track("workspace_initialized", {
 			workspace_id: workspaceId,
 			project_id: projectId,
 			branch,
-			base_branch: branch,
+			base_branch: effectiveBaseBranch,
 			use_existing_branch: true,
 		});
🤖 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/workspace-init.ts` around
lines 140 - 146, The analytics event call to track("workspace_initialized", ...)
incorrectly sets base_branch to branch in the useExistingBranch path; change
base_branch to use effectiveBaseBranch (the variable used elsewhere, e.g., at
line 439) so the event uses the workspace's actual base branch instead of its
own branch—update the object passed to track in the useExistingBranch path to
set base_branch: effectiveBaseBranch while keeping workspace_id: workspaceId,
project_id: projectId, branch, and use_existing_branch: true.
🧹 Nitpick comments (3)
apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts (1)

85-90: Consider aligning getPRByBranchTracking return type now that the fallback is removed.

With findPRByHeadCommit gone, the null vs undefined distinction in getPRByBranchTracking is vestigial. You could simplify by returning null on line 123 (instead of undefined) and having getPRForBranch call it directly without the ?? null coercion. This also makes the return type annotation (GitHubStatus["pr"] | undefined) more honest, since the function already returns null on lines 108 and 114.

♻️ Suggested simplification
 async function getPRByBranchTracking(
 	worktreePath: string,
-): Promise<GitHubStatus["pr"] | undefined> {
+): Promise<GitHubStatus["pr"]> {
 	try {
 		...
 	} catch (error) {
 		if (
 			error instanceof Error &&
 			error.message.includes("no pull requests found")
 		) {
-			return undefined;
+			return null;
 		}
 		throw error;
 	}
 }

Then in getPRForBranch:

 async function getPRForBranch(
 	worktreePath: string,
 ): Promise<GitHubStatus["pr"]> {
-	const branchResult = await getPRByBranchTracking(worktreePath);
-	return branchResult ?? null;
+	return getPRByBranchTracking(worktreePath);
 }

Also applies to: 96-127

🤖 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/github/github.ts` around
lines 85 - 90, getPRByBranchTracking currently mixes null/undefined and its
signature includes undefined; change getPRByBranchTracking to consistently
return null (replace any bare returns or implicit undefined with return null)
and update its return type to Promise<GitHubStatus["pr"]> so the type matches
the actual outputs; then simplify getPRForBranch by removing the null-coercion
(drop the "?? null") and change getPRForBranch's return type to
Promise<GitHubStatus["pr"]> so it simply returns await
getPRByBranchTracking(worktreePath).
apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts (1)

240-242: Silent .catch(() => {}) swallows git-config failures — consider at least logging.

This is the core mechanism for base-branch tracking introduced by this PR. If git config fails (e.g., permissions, corrupt config file), the feature silently degrades with no diagnostic trace. The same pattern repeats at lines 464–466 and 843–845.

Suggested improvement
-	await simpleGit(project.mainRepoPath)
-		.raw(["config", `branch.${localBranchName}.base`, defaultBranch])
-		.catch(() => {});
+	await simpleGit(project.mainRepoPath)
+		.raw(["config", `branch.${localBranchName}.base`, defaultBranch])
+		.catch((err) => {
+			console.warn(`Failed to set branch.${localBranchName}.base config:`, err);
+		});

Apply the same pattern at the other two call sites.

🤖 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 240 - 242, The silent .catch(() => {}) on the
simpleGit(...).raw(["config", `branch.${localBranchName}.base`, defaultBranch])
call hides failures; replace the empty catch with a handler that logs the error
(including repo path project.mainRepoPath, localBranchName and defaultBranch)
via the existing logger (e.g., processLogger.warn or processLogger.error) so you
get diagnostics when git-config fails; apply the same change to the two other
occurrences that call simpleGit(...).raw(...).catch(() => {}) so none of those
git-config failures are swallowed.
apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts (1)

249-251: Silent swallow of git config write failure is acceptable but consider logging.

The .catch(() => {}) silently suppresses errors when persisting the fallback base branch to git config. If this write fails, the in-memory effectiveBaseBranch is still correct for this run, but subsequent workspace inits will re-derive the fallback. Consider logging a warning so config write failures are observable in diagnostics.

Optional: log on failure
 			await simpleGit(mainRepoPath)
 				.raw(["config", `branch.${branch}.base`, result.fallbackBranch])
-				.catch(() => {});
+				.catch((e) => {
+					console.warn(`[workspace-init] Failed to persist base branch to git config:`, e);
+				});
🤖 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/workspace-init.ts` around
lines 249 - 251, The git config write for branch fallback is being silently
swallowed by .catch(() => {}); update the catch to log a warning including
context (branch, mainRepoPath, result.fallbackBranch) so failures are visible in
diagnostics—e.g. replace .catch(() => {}) with .catch(err => { (use existing
logger like processLogger.warn if available, otherwise console.warn) and include
the error message and the branch/mainRepoPath/result.fallbackBranch values });
keep the original behavior (do not rethrow) so in-memory effectiveBaseBranch
remains unchanged.
🤖 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/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 140-146: The analytics event call to
track("workspace_initialized", ...) incorrectly sets base_branch to branch in
the useExistingBranch path; change base_branch to use effectiveBaseBranch (the
variable used elsewhere, e.g., at line 439) so the event uses the workspace's
actual base branch instead of its own branch—update the object passed to track
in the useExistingBranch path to set base_branch: effectiveBaseBranch while
keeping workspace_id: workspaceId, project_id: projectId, branch, and
use_existing_branch: true.

---

Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 464-466: The .catch(()=>{}) on the simpleGit raw config call
silently swallows errors; update the handler on the
simpleGit(project.mainRepoPath).raw(["config", `branch.${branch}.base`,
targetBranch]) call to catch the error and log it with context (include
project.mainRepoPath, branch and targetBranch) or surface it appropriately
rather than ignoring it; reference the simpleGit call and the variables
project.mainRepoPath, branch and targetBranch when adding the error
logging/handling.
- Around line 843-845: The silent .catch() on the simpleGit(...) .raw(...) call
swallows errors; update the error handling in the create workspace logic so
failures are logged and/or handled instead of ignored: in the block that calls
simpleGit(project.mainRepoPath).raw(["config", `branch.${input.branch}.base`,
defaultBranch]) capture the thrown error (e.g., try/catch or .catch(err => ...))
and call the existing logger (or throw) with contextual details including
input.branch and project.mainRepoPath; ensure behavior matches other similar
places (no silent catches).

---

Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/workspaces/procedures/create.ts`:
- Around line 240-242: The silent .catch(() => {}) on the
simpleGit(...).raw(["config", `branch.${localBranchName}.base`, defaultBranch])
call hides failures; replace the empty catch with a handler that logs the error
(including repo path project.mainRepoPath, localBranchName and defaultBranch)
via the existing logger (e.g., processLogger.warn or processLogger.error) so you
get diagnostics when git-config fails; apply the same change to the two other
occurrences that call simpleGit(...).raw(...).catch(() => {}) so none of those
git-config failures are swallowed.

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/github/github.ts`:
- Around line 85-90: getPRByBranchTracking currently mixes null/undefined and
its signature includes undefined; change getPRByBranchTracking to consistently
return null (replace any bare returns or implicit undefined with return null)
and update its return type to Promise<GitHubStatus["pr"]> so the type matches
the actual outputs; then simplify getPRForBranch by removing the null-coercion
(drop the "?? null") and change getPRForBranch's return type to
Promise<GitHubStatus["pr"]> so it simply returns await
getPRByBranchTracking(worktreePath).

In `@apps/desktop/src/lib/trpc/routers/workspaces/utils/workspace-init.ts`:
- Around line 249-251: The git config write for branch fallback is being
silently swallowed by .catch(() => {}); update the catch to log a warning
including context (branch, mainRepoPath, result.fallbackBranch) so failures are
visible in diagnostics—e.g. replace .catch(() => {}) with .catch(err => { (use
existing logger like processLogger.warn if available, otherwise console.warn)
and include the error message and the branch/mainRepoPath/result.fallbackBranch
values }); keep the original behavior (do not rethrow) so in-memory
effectiveBaseBranch remains unchanged.

@Kitenite
Kitenite merged commit 95c6e16 into main Feb 17, 2026
5 checks passed
@Kitenite
Kitenite deleted the kitenite/base-branch branch February 17, 2026 08:38
@github-actions

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ⚠️ Neon database branch
  • ⚠️ Electric Fly.io app
  • ⚠️ Streams Fly.io app

Thank you for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant