fix(desktop,host-service): tolerate slow or failing post-checkout hooks during worktree creation

Merged
KI
3mo ago
+468-75

Summary

  • Workspace creation no longer fails when a repo's post-checkout hook exits non-zero or runs long: if the worktree is verifiably in place (registered + valid HEAD/branch), the hook error is demoted to a warning and creation succeeds.
  • Worktree-add / PR-checkout timeouts raised from 2 min to 10 min so hooks that install dependencies on fresh worktrees can finish.
  • The tolerance helper moved to @superset/shared/git-hook-tolerance and now also guards the host-service (v2) addBranchWorktree path, so both creation surfaces behave identically.

Why / Context

A user (drew) hit "Workspace setup failed" on every creation attempt in a repo whose post-checkout hook installs dependencies and builds packages (Fresh worktree detected — installing dependencies… Done in 1m 5.7s Building packages…). Two compounding bugs:

  1. The 120s exec timeout killed git mid-hook. Checkout of ~11k files + a 1m+ install blows past 120s; Node SIGTERMs git and creation reports failure — with the worktree fully checked out on disk.
  2. The existing hook tolerance (SUPER-594) was gated backwards. It pattern-matched the error text for the literal string post-checkout before checking whether the worktree actually exists. A timeout kill (or any hook that doesn't print that marker) never engages it.

Each failure stranded a fully valid worktree + branch, so retries failed on "already exists". These failures are also invisible in telemetry: updateProgress("failed") is never captured, only thrown tRPC errors are — the sibling PR-checkout path shows the same fatal: '<path>' already exists loop in PostHog (one user hit it 42×).

How It Works

  • runWithPostCheckoutHookTolerance (now in @superset/shared/git-hook-tolerance) consults didSucceed() on any error and tolerates when ground truth confirms the operation landed. The post-checkout text gate is gone.
  • Desktop execWorktreeAdd: ground truth = worktree registered and rev-parse --verify HEAD resolves (a half-created worktree can stay registered); default timeout 600s. Same bump for gh pr checkout and its FETCH_HEAD fallback, which also run hooks.
  • Host-service addBranchWorktree: both worktree-add variants wrapped; ground truth = existing findWorktreeAtPath(git, worktreePath, plan.branch) (registered at path with the planned branch — stricter than the desktop probe; this path never detaches). No timeout change needed there: simple-git never had one, so slow hooks were never killed on v2.

Manual QA (CDP, dev desktop from this branch, before/after evidence gate)

Repro repo with a switchable post-checkout hook; "before" = this fix stashed, "after" = this branch. Desktop runs on the v1 surface (drew's surface — the error format Command failed: git -C … is the desktop execFile path).

Before (reproduces both report screenshots):

  • Hook exits non-zero (no "post-checkout" marker in output) → step: failed in 2s; error text matches the user report byte-for-byte; failure screen pixel-matches the screenshot
  • Hook sleeps 130s → step: failed at 122s (timeout kill), output truncated mid-hook — the second report screenshot
  • Ground truth both times: git worktree list shows a fully valid worktree with the branch checked out — the app failed anyway and stranded it

After (same scenarios, same observations):

  • Hook exits non-zero → step: ready in 2s
  • Hook sleeps 130s → step: ready at 132s (hook simply completes inside the new ceiling; no tolerance needed)
  • Stranded before-workspace recovers via a real-UI "Retry Setup" click → ready in 2s, workspace fully functional (old code looped on "already exists" here)
  • Main-process log shows the tolerance warning firing exactly where expected, and nowhere else
  • Host-service v2 E2E (post-refactor): project.create + workspaces.create via host tRPC against the failing-hook repo → workspace created, worktree registered with the planned branch; [hs:…] … (non-fatal) log line proves the shared helper ran in the host-service process

Creation was triggered via the same tRPC mutations the UI forms call (the folder picker is a native dialog CDP can't drive); navigation, the failure screen, the Retry Setup click, and screenshots were real UI.

Testing

  • bun run typecheck (repo-wide, exit 0)
  • bun run lint (exit 0)
  • bun test on the three touched suites: 44 pass — desktop git.test.ts (4 new edge cases: no-marker hook failure, git SIGTERM'd mid-hook with worktree intact, invalid start point still throws, pre-existing plain dir still throws), shared git-hook-tolerance.test.ts, new host-service worktree-add-tolerance.test.ts (new-branch + existing-branch tolerance, invalid start point)
  • Mutation-verified: re-adding the old text-pattern gate fails the timeout-shape test; disabling didSucceed fails exactly the two host-service tolerance tests

Design Decisions

  • Ground truth over pattern matching: error-text heuristics can never enumerate every hook failure shape (timeout kills carry no marker at all). Checking the worktree's actual state is exact, and conservative in the right direction — when the probe itself fails, the original error is rethrown.
  • Share the policy, not the git clients: desktop (shell-PATH execFile) and host-service (simple-git factory with credential/env handling) keep their own plumbing; with v1 sunsetting, unifying the exec layers is churn with no user benefit. Only the decision logic is shared.
  • Exported addBranchWorktree/BranchSourcePlan so the host-service behavior is testable against real git repos.

Known Limitations

  • A timeout kill can orphan the hook's child processes (e.g. a package install continuing in the background). The 10-min ceiling makes this rare; creation now succeeds regardless.
  • Stranded detached worktrees (PR flow killed mid-hook before gh pr checkout ran) do not self-heal on retry: a detached leftover is indistinguishable from a foreign checkout, so the "already exists" error surfaces — same as before this PR. Branch-add strandings (the common case) do recover. Safe PR-retry self-healing needs a provenance marker; follow-up.
  • If the timeout kills git before checkout completes, nothing is registered, didSucceed is false, and the error surfaces as before — genuine failures still fail.

Follow-ups

  • Move user hooks out of the creation critical path entirely (core.hooksPath= during add, then run the hook as a streamed, non-fatal setup step).
  • workspace_init_failed telemetry — these failures are currently invisible; this bug was only found via user screenshots.
  • Clean up stranded worktrees/branches from historical failed attempts and make plain Retry self-heal (dedup at creation time instead of the post-failure button).
  • Humanize the failure view: map common git fatals to one-line actionable messages; the raw command spew is line-clamped to 3 lines, which hides the actual fatal: line.

Summary by cubic

Tolerates slow or failing post-checkout hooks during worktree creation and PR checkout by verifying ground truth (registered worktree, expected branch, resolvable HEAD, and PR head OID) so setup succeeds when checkout actually completed. Timeouts are raised to 600s, and the logic is shared via @superset/shared/git-hook-tolerance across desktop and host-service.

  • Bug Fixes

    • Demote errors only when ground truth confirms success:
      • Branch adds: worktree is registered at the path with the requested branch; HEAD resolves. Detached adds only count when this call registered the path (detached retries are not tolerated).
      • PR checkout: tolerate only when the local branch is checked out and HEAD equals the PR’s headRefOid (covers gh pr checkout and the FETCH_HEAD fallback).
  • Refactors

    • Moved tolerance helper to @superset/shared/git-hook-tolerance and adopted in desktop and host-service; host-service now also requires a resolvable HEAD.
    • Exported addBranchWorktree and BranchSourcePlan; added targeted tests in desktop, host-service, and shared packages.

Summary by CodeRabbit

  • New Features

    • Worktree creation now succeeds when post-checkout hooks fail after the worktree is otherwise valid.
    • Supports new branches, existing branches, detached worktrees, and pull request checkouts.
    • Longer operations have increased time limits.
  • Bug Fixes

    • Prevents incomplete or mismatched worktrees from being marked successful.
    • Invalid references and conflicting worktrees continue to fail safely.
    • Successful operations with hook failures now show non-fatal warnings.