fix(desktop): prevent orphan worktree branches and auto-recover unborn HEAD#2007
fix(desktop): prevent orphan worktree branches and auto-recover unborn HEAD#2007Kitenite wants to merge 1 commit intosuperset-sh:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces unborn HEAD detection and recovery mechanisms to handle git worktrees with invalid HEAD references. A new utility module detects unborn HEAD states and attempts recovery by resetting to the default branch. The utility is integrated into both the status router and the worktree creation logic with corresponding tests. Changes
Sequence Diagram(s)sequenceDiagram
participant StatusRouter as Status Router
participant UnbornDetector as Unborn HEAD Detector
participant GitInterface as Git Interface
StatusRouter->>UnbornDetector: detectAndRecoverUnbornHead(git, path, branch)
UnbornDetector->>GitInterface: raw(["rev-parse", "HEAD"])
alt HEAD is Valid
GitInterface-->>UnbornDetector: success
UnbornDetector-->>StatusRouter: false (no recovery)
else HEAD is Unborn
GitInterface-->>UnbornDetector: error
UnbornDetector->>GitInterface: raw(["reset", "origin/<branch>"])
alt Reset Succeeds
GitInterface-->>UnbornDetector: success
UnbornDetector-->>StatusRouter: true (recovered)
else Reset Fails
GitInterface-->>UnbornDetector: error
UnbornDetector-->>StatusRouter: false (recovery failed)
end
end
StatusRouter->>GitInterface: status()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.ts (1)
1-3: Consider exporting theGitRawExecutorinterface.The interface is useful for consumers to properly type their git objects, especially for testing. Currently tests in
status.test.tsdefine their own compatible object without explicit type annotation.♻️ Proposed change
-interface GitRawExecutor { +export interface GitRawExecutor { raw(args: string[]): Promise<string>; }🤖 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/utils/unborn-head.ts` around lines 1 - 3, Export the GitRawExecutor interface so consumers and tests can import and use the exact type instead of redefining compatible shapes; update the declaration of GitRawExecutor (the interface named GitRawExecutor in unborn-head.ts) to be exported (export interface GitRawExecutor { ... }) and ensure any local imports or tests reference this exported symbol for typing.apps/desktop/src/lib/trpc/routers/changes/status.test.ts (1)
1-71: Move test file to co-locate with the source module.Per coding guidelines, tests should be placed in the same directory as the component being tested. Since
detectAndRecoverUnbornHeadis defined in./utils/unborn-head.ts, this test file should be moved toapps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.test.ts.As per coding guidelines: "Co-locate tests with components - place test files in the same directory as the component (e.g.,
ComponentName.test.tsxinComponentName/folder)"🤖 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/status.test.ts` around lines 1 - 71, The test file for detectAndRecoverUnbornHead is currently not co-located with its module; move the test so it sits next to the source file unborn-head.ts and rename it to unborn-head.test.ts, update the test's import to reference detectAndRecoverUnbornHead from the local module (keeping the createGitWithRaw helper and test cases intact), and ensure the test runner picks up the new file name/location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/changes/status.test.ts`:
- Around line 1-71: The test file for detectAndRecoverUnbornHead is currently
not co-located with its module; move the test so it sits next to the source file
unborn-head.ts and rename it to unborn-head.test.ts, update the test's import to
reference detectAndRecoverUnbornHead from the local module (keeping the
createGitWithRaw helper and test cases intact), and ensure the test runner picks
up the new file name/location.
In `@apps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.ts`:
- Around line 1-3: Export the GitRawExecutor interface so consumers and tests
can import and use the exact type instead of redefining compatible shapes;
update the declaration of GitRawExecutor (the interface named GitRawExecutor in
unborn-head.ts) to be exported (export interface GitRawExecutor { ... }) and
ensure any local imports or tests reference this exported symbol for typing.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/changes/status.test.tsapps/desktop/src/lib/trpc/routers/changes/status.tsapps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.tsapps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.ts:13">
P2: Narrow the broad `catch {}` for `rev-parse HEAD`; currently every lookup failure is treated as unborn HEAD and can trigger an incorrect recovery reset.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts">
<violation number="1" location="apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:495">
P1: Use a hard reset when recovering an unborn HEAD so files are actually checked out; plain `git reset <startPoint>` can leave the worktree full of deletions.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| console.warn( | ||
| `[createWorktree] HEAD is unborn after creation, resetting to ${startPoint}`, | ||
| ); | ||
| await worktreeGit.raw(["reset", startPoint]); |
There was a problem hiding this comment.
P1: Use a hard reset when recovering an unborn HEAD so files are actually checked out; plain git reset <startPoint> can leave the worktree full of deletions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts, line 495:
<comment>Use a hard reset when recovering an unborn HEAD so files are actually checked out; plain `git reset <startPoint>` can leave the worktree full of deletions.</comment>
<file context>
@@ -480,6 +480,21 @@ export async function createWorktree(
+ console.warn(
+ `[createWorktree] HEAD is unborn after creation, resetting to ${startPoint}`,
+ );
+ await worktreeGit.raw(["reset", startPoint]);
+ }
+
</file context>
| await worktreeGit.raw(["reset", startPoint]); | |
| await worktreeGit.raw(["reset", "--hard", startPoint]); |
| try { | ||
| await git.raw(["rev-parse", "HEAD"]); | ||
| return false; | ||
| } catch { |
There was a problem hiding this comment.
P2: Narrow the broad catch {} for rev-parse HEAD; currently every lookup failure is treated as unborn HEAD and can trigger an incorrect recovery reset.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/lib/trpc/routers/changes/utils/unborn-head.ts, line 13:
<comment>Narrow the broad `catch {}` for `rev-parse HEAD`; currently every lookup failure is treated as unborn HEAD and can trigger an incorrect recovery reset.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>
<file context>
@@ -0,0 +1,28 @@
+ try {
+ await git.raw(["rev-parse", "HEAD"]);
+ return false;
+ } catch {
+ try {
+ console.warn(
</file context>
Summary\n- add post-creation HEAD validation in worktree creation and reset to start point when HEAD is unborn\n- add unborn HEAD detection/recovery during status refresh via reset to origin/{defaultBranch}\n- call recovery before ahead/behind and commit diff calculations so status is accurate\n- add focused unit tests for detect/recover behavior\n\n## Validation\n- bun test apps/desktop/src/lib/trpc/routers/changes/status.test.ts apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts\n- bun run lint\n- bun run typecheck\n
Summary by cubic
Prevents orphan worktree branches by validating HEAD after creation and auto-recovering unborn HEADs. Keeps status, ahead/behind, and diffs accurate by self-healing before computing them.
Written for commit 2b53134. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests