Skip to content

fix(desktop): make git hook failures non-fatal during worktree creation#1437

Merged
Kitenite merged 5 commits into
mainfrom
kitenite/setup-script-should-also-get-paths
Feb 14, 2026
Merged

fix(desktop): make git hook failures non-fatal during worktree creation#1437
Kitenite merged 5 commits into
mainfrom
kitenite/setup-script-should-also-get-paths

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Feb 12, 2026

Summary

  • Git hooks (e.g., Husky post-checkout) that fail during git worktree add no longer block workspace creation
  • Introduces execWorktreeAdd() helper that tolerates hook failures when the worktree was successfully created on disk
  • Works in conjunction with the revert of core.hooksPath=/dev/null (Revert "fix(desktop): disable git hooks during worktree creation" #1434) to restore legitimate hook execution

Changes

  • apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts:
    • Added pathExists() helper using stat from node:fs/promises
    • Added execWorktreeAdd() — wraps execFileAsync("git", ...) for worktree creation; on failure, checks if the worktree exists on disk (hook failed but worktree created) and logs a warning instead of throwing
    • Updated createWorktree() to use execWorktreeAdd()
    • Updated createWorktreeFromExistingBranch() to use execWorktreeAdd() (both local and remote branch paths)
    • Updated createWorktreeFromPr() to use execWorktreeAdd() (both existing-branch and new-branch paths)

Test Plan

  • bun test src/lib/trpc/routers/workspaces/utils/git.test.ts — 12 pass
  • bun test src/lib/trpc/routers/workspaces/utils/teardown.test.ts — 11 pass
  • bun run typecheck — clean
  • Manual: create a workspace in a repo with Husky hooks and verify hooks run + workspace creation succeeds even if hooks fail

Closes #961, closes #1432

Summary by CodeRabbit

  • Bug Fixes
    • More reliable worktree creation and branch checkouts by treating certain post-checkout hook failures as non-fatal, verifying the operation succeeded, and ensuring proper registration before proceeding.
  • Tests
    • Added unit and integration tests for post-checkout-hook tolerance, success verification, and worktree creation edge cases.

When git hooks (e.g., Husky post-checkout) fail during worktree
creation, the worktree is still created on disk but execFileAsync
throws due to a non-zero exit code. This caused workspace creation
to fail entirely even though the worktree was ready.

Introduces execWorktreeAdd() helper that checks if the worktree
exists on disk after a failure — if it does, the hook failure is
logged as a warning and creation continues normally.

Closes #961, closes #1432
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 12, 2026

📝 Walkthrough

Walkthrough

Adds a post-checkout-hook tolerance layer and guarded worktree-add wrapper, replaces direct exec calls for worktree operations, integrates tolerance into branch switch logic, and adds unit tests validating tolerant and error propagation behaviors.

Changes

Cohort / File(s) Summary
Worktree git utilities
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts
Adds isWorktreeRegistered, execWorktreeAdd(...), and checkoutBranchWithHookTolerance(...); replaces direct execFileAsync calls for worktree add/checkout with guarded wrapper that verifies registration and tolerates non-fatal post-checkout hook failures.
Post-checkout hook tolerance util & tests
apps/desktop/src/lib/trpc/routers/utils/git-hook-tolerance.ts, apps/desktop/src/lib/trpc/routers/utils/git-hook-tolerance.test.ts
New module providing runWithPostCheckoutHookTolerance, getErrorText, isPostCheckoutHookFailure, and tests covering success-with-hook-failure, failure propagation when operation didn't succeed, and non-hook failure propagation.
Branch switching integration
apps/desktop/src/lib/trpc/routers/changes/security/git-commands.ts
Adds internal isCurrentBranch and wraps gitSwitchBranch with the post-checkout-hook tolerance wrapper; retains legacy branch-switch fallbacks but verifies success via didSucceed.
Worktree tests
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.test.ts
Adds tests that simulate a failing post-checkout hook to assert worktree creation succeeds under tolerated hook failures and checks collision behavior for existing destination paths.

Sequence Diagram(s)

sequenceDiagram
  participant App as Desktop App
  participant Wrapper as execWorktreeAdd / runWithPostCheckoutHookTolerance
  participant Git as Git CLI
  participant FS as Filesystem
  participant Hook as post-checkout hook

  App->>Wrapper: request worktree add / checkout (params, env)
  Wrapper->>Git: run `git worktree add` / `git checkout`
  Git->>Hook: invoke post-checkout hook
  alt Hook succeeds
    Git-->>Wrapper: success
    Wrapper->>FS: check isWorktreeRegistered(worktreePath)
    FS-->>Wrapper: registered
    Wrapper-->>App: success
  else Hook fails (hook-related error)
    Git-->>Wrapper: error (hook failure)
    Wrapper->>Wrapper: detect post-checkout hook failure
    Wrapper->>FS: poll isWorktreeRegistered (retry until true or timeout)
    alt Registered becomes true
      FS-->>Wrapper: registered
      Wrapper-->>App: treat as success (non-fatal hook failure)
    else Not registered / timeout
      Wrapper-->>App: rethrow original git error
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰
I nudged the hook with careful paws,
If checkout whimpered, I checked the cause.
The tree stood ready, roots all aligned,
A gentle hop — no crash to find. 🥕✨

🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: making git hook failures non-fatal during worktree creation, which is the primary objective of this PR.
Description check ✅ Passed The PR description follows the template with clear sections (Summary, Changes, Test Plan) and includes related issue links. It adequately explains the problem, solution, and testing status.
Linked Issues check ✅ Passed The PR implementation fully addresses both linked issues: #961 (preventing Husky failures from blocking worktree creation) and #1432 (tolerating non-fatal hook failures while preserving hook execution).
Out of Scope Changes check ✅ Passed All changes are directly related to the stated objectives. New utilities for hook tolerance and worktree creation helpers, plus corresponding tests, are all in-scope.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ 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/setup-script-should-also-get-paths

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts (1)

36-68: Consider extracting path-normalization logic or consolidating duplication with worktreeExists.

isWorktreeRegistered and the exported worktreeExists (line 841) both parse git worktree list --porcelain, but with a key difference: isWorktreeRegistered normalizes paths with resolve() before comparing, while worktreeExists uses direct string matching (line.trim() === worktreePrefix). This makes worktreeExists fragile—it will fail to match paths that differ only by trailing slashes or symlink resolution.

Since worktreeExists is actively exported and used across the codebase (5 call sites in create.ts and delete.ts), extracting the normalized path-comparison logic into a shared helper would benefit both functions and reduce duplication. Alternatively, if error-handling semantics require separate functions, at least backport the resolve() normalization to worktreeExists to fix the latent path-matching bug.


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.

@Kitenite Kitenite merged commit 8f731db into main Feb 14, 2026
5 of 6 checks passed
@Kitenite Kitenite deleted the kitenite/setup-script-should-also-get-paths branch February 14, 2026 08:53
@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.

[bug] Git hooks silently disabled during worktree creation [bug] Superset fails with Husky hooks when using NVM-managed package managers

1 participant