[codex] Fix PR worktree checkout materialization

Merged
KI
6mo ago
+2637-828

Summary

  • Replace the v2 PR workspace flow that created a detached placeholder worktree and then ran gh pr checkout.
  • Add a materialize-first helper that fetches and verifies the PR head against GitHub's headRefOid, creates/configures the local branch, then lets workspaces.create add the worktree from that branch.
  • Start materialized PR branches from verified commit OIDs, not mutable fetched refs; synthetic fallback fetches reuse one stable internal ref per PR.
  • Remove the now-unused gh pr checkout recovery helper/tests.
  • Configure cross-repo PR branches with a Superset-managed fork remote and push refspec so plain git push targets the contributor branch instead of refs/pull/<n>/head.
  • Return materialization warnings from workspaces.create and surface them in the desktop workspace-create flow.
  • Serialize concurrent creates for the same project/PR so repeated clicks reuse the first workspace instead of racing git worktree creation.
  • Normalize adopted local PR branches through the same tracking/push setup as freshly materialized branches, and roll back newly created PR branches if later workspace registration fails.
  • Update CLI/MCP/SDK/docs text so --pr no longer claims the host runs gh pr checkout.
  • Document the current setup, Worktrunk/T3Code comparison, implemented fix, and remaining follow-ups in plans/done/20260516-pr-worktree-checkout-flow.md.

Why

The old flow performed two checkouts in the target path. git worktree add --detach can run hooks, filters, or watchers that dirty tracked files such as package-lock.json; the later gh pr checkout --force can still fail with local changes would be overwritten by checkout.

The new flow avoids the dirty-placeholder state entirely: it materializes the verified branch first, then performs one worktree checkout.

Known follow-ups

  • v1 desktop still uses the old detached worktree + gh pr checkout path and should get a separate backport.
  • Add richer fork permission handling (maintainerCanModify, deleted forks, inaccessible forks) if exact GitHub CLI parity is needed for every fork push edge case.
  • Renderer surfacing for hook-induced dirty-status warnings is still deferred; materialization/setup warnings now reach the desktop flow.

Validation

  • bun test packages/host-service/src/trpc/router/workspace-creation/utils/pr-branch-materialize.test.ts packages/host-service/test/integration/workspace-create-pr.integration.test.ts packages/host-service/test/integration/pr-branch-materialize.integration.test.ts (21 pass, 0 fail)
  • bun test packages/host-service/test/integration (206 pass, 8 existing todos, 0 fail)
  • bun run --cwd packages/host-service test:e2e (12 pass, 0 fail)
  • bun run typecheck
  • bun run lint
Open in Stage

Summary by cubic

Fix PR workspace creation by materializing the verified PR head before creating the worktree. This removes the detached worktree + gh pr checkout flow, prevents “local changes would be overwritten by checkout” errors, and configures fork pushes for cross‑repo PRs.

  • Bug Fixes

    • Fetch and verify the PR head against headRefOid, create/configure the local branch, then run one git worktree add <path> <branch>.
    • Support same‑repo PRs via refs/heads/<branch> and cross‑repo or deleted head branches via refs/pull/<n>/head; abort on OID mismatch. For cross‑repo PRs with fork metadata, add a superset-pr-<n> remote and set pushRemote with HEAD:refs/heads/<headRefName>; warn when fork metadata is missing or when falling back to the synthetic PR ref.
    • Handle edge cases: normalize tracking for existing branches, detect conflicts, and abort with a clear error; on failure, delete only a newly created PR branch that still points at the verified head. Return warnings from the host; desktop now surfaces them as toasts.

  • Refactors

    • Added materializePrBranch, normalizePrBranchTracking, and cleanup utilities with unit/integration tests covering edge cases; workspaces.create now materializes the branch then runs git worktree add. fetchPrMetadata uses injected ctx.execGh; integration tests confirm the host never runs gh pr checkout in worktree mode.
    • Updated CLI/@mcp-v2/@superset/sdk/docs: --pr “checks out the verified PR head” and workspaces.create returns a warnings array. Added design doc plans/done/20260516-pr-worktree-checkout-flow.md. Removed the old gh pr checkout recovery helper and its tests.

Summary by CodeRabbit

  • New Features

    • Server now materializes and checks out the verified PR head, derives the workspace branch, returns optional warnings, and serializes concurrent PR workspace creation.

  • Bug Fixes

    • Verifies PR head before adding worktrees to avoid dirty-file/checkout failures and improves safe rollback and conflict handling.

  • Documentation

    • Updated CLI, SDK, tooling and help text to reflect the new PR checkout behavior.

  • Tests

    • Added unit and integration tests for PR branch materialization and worktree flows; removed legacy checkout-recovery tests.

  • UI

    • Warnings from the server are surfaced to users via toast notifications.

Review Change Stack