fix(server): record the checked-out branch on threads created without one

[NoahHendrickson]

Problem

Threads started from the phone in "Current checkout" mode never show a branch on web or desktop. Upstream's mobile composer displays the current branch but only records one when a branch is tapped in the picker, so the untouched default sends branch: null. The server stores exactly that, and the sidebar row has nothing to draw. Every one of the 17 blank threads in the live database since Aug 20 came from the phone this way; the fork's own mobile fence (mobile-current-checkout-branch) never reaches the phone because the fork ships no mobile builds and the phone runs the App Store build.

Fix

The bootstrap turn-start handler in apps/server/src/ws.ts now fills in the branch when the client named none: it looks up the project's workspace root (or the thread's existing worktree path), reads GitWorkflowService.localStatus, and records the checked-out ref before dispatching thread.create. An explicit client branch always wins, a worktree being prepared is left to its own flow, and a detached HEAD, non-repo project, or status failure logs a warning and stays null rather than failing the turn start. This covers every client, old or new, without waiting on a mobile release.

Fork bookkeeping: fenced hunk, server-local-thread-branch manifest entry, guard test in apps/web/src/__fork_guards__/.

Verification

• apps/server: typecheck clean; the eleven bootstrap tests in server.test.ts pass, including the new one covering project-root, existing-worktree, and explicit-branch cases.
• apps/web: new guard test and its mobile sibling pass; .fork/lint-owned.mjs reports no blocking warnings.

Existing blank threads are not backfilled; only threads created after this ships get a label.

Claude Fable 5.1 via Claude Code (T3 Code).

🤖 Generated with Claude Code

[NoahHendrickson]

Reviewed commit 7978c584f7988db2b1fe9c67f2296d35744d64a2. No blocking findings.

The fallback preserves explicit client branches, reads an existing worktree before the project root, and leaves branch assignment for newly prepared worktrees to the existing flow. I traced the mobile bootstrap payload, the Git status implementation's null results for detached HEAD/non-repositories, and the bootstrap cleanup path. This is shared server behavior for web, desktop, and mobile and does not require provider-specific changes or a wire-contract update.

Non-blocking test suggestion: extend the new server test with a failing localStatus and assert that both thread.create (with branch: null) and thread.turn.start still dispatch. The current runtime test covers successful inference and explicit-branch precedence; the guard's Effect.as(null) string assertion does not exercise failure recovery. A null-status case would also pin detached/non-repository behavior at the bootstrap boundary.

Validation on the PR checkout:

• vp test run apps/server/src/server.test.ts -t 'bootstrap|checked-out branch': 11 passed, 171 skipped.
• vp test run apps/web/src/__fork_guards__/serverLocalThreadBranch.test.ts apps/web/src/__fork_guards__/mobileCurrentCheckoutBranch.test.ts: 3 passed.
• Commit diff whitespace check passed.

No browser/mobile UI verification performed. CI was still running at review time. Submitted as a comment review because the authenticated GitHub account is the PR author.

Reviewed by GPT-6 via Codex.

[cursor]

The fill policy is right. The placement is not.

resolveCheckedOutBranch plus the prepareWorktree ? null : yield* ternary is a self-contained policy (explicit wins, skip a worktree being prepared, cwd is worktreePath else workspaceRoot, localStatus, failures → null). It does not belong as a nested helper inside dispatchBootstrapTurnStart (~290 lines, already inside makeWsRpcLayer, already inside a 3055-line file).

This is the first server-side fenced customization. Last-resort fences are supposed to stay small. A new server file upstream has never seen does not conflict on sync — put the policy there (files:), and leave a one-line call in ws.ts (watch:). Do not grow GitWorkflowService; that is git I/O, not thread-create policy.

The 123-line router-seam test and the guard that regex-locks the nested ternary make the inline shape load-bearing. After extraction, unit-test the resolver (including prepareWorktree skip, status failure, missing project, detached HEAD) and pin the outcome, not the spaghetti.

The manifest line that a bare thread.create "carries no cwd context" is false — the command already has projectId and worktreePath. Bootstrap-only can remain a call-site choice. Burying the helper in the bootstrap nest makes it a structural one.

Please extract, shrink the ws.ts fence, and move the tests. Behavior can stay exactly as specified.

  

_{Sent by Cursor Automation: Thermo nuke 4.6}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	13.6 KiB	13.6 KiB	−11 B (−0.1%)	15.1 KiB	✅
Codex	Thread snapshot wire	7.0 KiB	7.0 KiB	+5 B (+0.1%)	7.3 KiB	✅
Codex	Live turn WebSocket wire	6.6 KiB	6.6 KiB	−16 B (−0.2%)	7.8 KiB	✅
Codex	Live turn WebSocket decoded	57.1 KiB	57.1 KiB	0 B (0.0%)	66.4 KiB	✅
Codex	Live turn messages	10	10	0 (0.0%)	21	✅
Claude	Total thread wire	13.6 KiB	13.6 KiB	−19 B (−0.1%)	15.1 KiB	✅
Claude	Thread snapshot wire	7.0 KiB	7.0 KiB	0 B (0.0%)	7.3 KiB	✅
Claude	Live turn WebSocket wire	6.6 KiB	6.5 KiB	−19 B (−0.3%)	7.8 KiB	✅
Claude	Live turn WebSocket decoded	57.9 KiB	57.8 KiB	−44 B (−0.1%)	66.4 KiB	✅
Claude	Live turn messages	10	9	−1 (−10.0%)	21	✅

Baseline: 31e5bb2 · PR result: ebd1ec3 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 113.9 KiB
• Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[NoahHendrickson]

Reviewed 7978c584f7988db2b1fe9c67f2296d35744d64a2. CI is green (Check, Test, Test Server 1–3, Release Smoke), the manifest entry is well formed and matches the seven existing watch-only entries, and a guard in apps/web/src/__fork_guards__/ reading server source has precedent (forkAppIdentity.test.ts). The fill policy itself is right: explicit client branch wins, a worktree being prepared names its own branch, worktree path before project root, failures degrade to null.

Four things worth deciding on before this merges. The first is the only one I'd call a defect; the rest are consequences the PR body doesn't name.

1. Temporary worktree branches are not excluded

apps/server/src/ws.ts:1120 records whatever localStatus reports, including a t3/… placeholder branch. Both existing consumers of a checked-out ref refuse to adopt that name: CheckpointReactor.ts:553 (isTemporaryWorktreeBranch — "a temporary placeholder checkout means the first-turn auto-rename is still in flight, don't race it") and ProviderCommandReactor.ts:912.

Failure scenario: a thread is bootstrapped with worktreePath pointing at a worktree still sitting on its placeholder branch — either mid-rename, or permanently, because maybeGenerateAndRenameWorktreeBranchForFirstTurn returns early when branch-name generation yields nothing (ProviderCommandReactor.ts:934, if (!generated) return). Before this PR that thread's branch stayed null and the rename path no-opped. Now it records t3/ab12cd, so on its first turn maybeGenerateAndRenameWorktreeBranchForFirstTurn fires and renames that worktree's branch from this thread's message — the reactor gates on branch + worktreePath only, never on worktree ownership. The thread that actually owns the worktree keeps a recorded branch that no longer exists.

Reachability is narrow (needs worktreePath non-null with branch null, and that worktree on a placeholder), but the fix is one call and matches two existing precedents:

const local = yield* gitWorkflow.localStatus({ cwd });
return local.refName === null || isTemporaryWorktreeBranch(local.refName) ? null : local.refName;

isTemporaryWorktreeBranch is exported from @t3tools/shared/git.

2. This turns on more than a sidebar label

The intent block and PR body both frame this as "the sidebar row has nothing to draw". A non-null thread.branch is load-bearing in four other places, and every current-checkout thread — on web and desktop too, not only phone-created ones — now crosses those gates:

• apps/web/src/components/BranchToolbar.logic.ts:231 — the mismatch resolver returns null while activeThreadBranch is null. These threads will now show the composer's "Branch changed — was X" banner whenever the shared checkout has moved on, plus a Restore branch button that runs a real git checkout in the project root (ChatView.tsx:5599).
• apps/web/src/components/Sidebar.tsx:753 — the live git-status lease is gated on thread.branch != null || thread.worktreePath !== null. Current-checkout rows start leasing a status query they previously skipped.
• apps/server/src/orchestration/ThreadPullRequestReactor.ts:128 — these threads enter PR resolution. Bounded by the [projectId, worktreePath, branch] grouping, so it's one extra lookup per branch group rather than per thread, but it is new outbound gh traffic for a class of threads that had none.
• The label is what PR attribution matches against, so a stale one attributes the wrong PR badge.

None of that is wrong — it is exactly upstream's semantics for a branch-recorded local thread — but it is a behavior change well past "the row draws a label", and someone reading the manifest intent in six months should find it there. Worth a sentence in the intent block, and worth a conscious yes on the mismatch banner in particular: it appears on threads whose users never picked a branch.

3. Cost sits on the turn-start critical path

localStatus is awaited before thread.create dispatches, so it holds the dispatchCommand RPC and the first turn behind it. For one ref name it runs, via readStatusDetailsLocal (GitVcsDriverCore.ts:1586): rev-parse --git-path index, an index-lock exists check, status --porcelain=2 --branch, diff HEAD --numstat, default-branch and remote lookups, then two config reads for hosting-provider detection. Cache TTL is 1s (GitManager.ts:130), so a phone creating a thread against a server that isn't currently polling that cwd pays the cold path; default git timeout is 30s (GitVcsDriverCore.ts:41).

Small repo or warm cache, this is noise. Large dirty monorepo, diff HEAD --numstat is the expensive one and the user watches it before their first turn starts. If that trade isn't wanted, filling in after the create via thread.meta.update (the shape followWorktreeBranchDrift already uses, expectedBranch and all) keeps the turn start untouched.

4. One-shot, and silently permanent when it misses

The fill-in happens once, at creation. readStatusDetailsLocal fails outright when index.lock is present (GitVcsDriverCore.ts:1600-1621), so a thread created while any other git operation holds the index logs a warning and stays null forever — followWorktreeBranchDrift bails on thread.worktreePath === null (CheckpointReactor.ts:565), which is precisely the class this PR targets. Web self-heals on the next send through the mismatch path (ChatView.tsx:6989); mobile has no equivalent, so a phone thread that loses the race stays blank — the exact symptom this PR is fixing. Low priority, but it argues for the fill-in being repeatable rather than a one-shot.

5. Tests

Agreeing with the Cursor note rather than repeating it: the three cases pinned (project root, existing worktree, explicit branch) are the ones that can't regress silently anyway; the unpinned ones — prepareWorktree skip, status failure → null, missing project, detached HEAD → null — are the load-bearing branches, and every one of them is cheap to test against a resolver that doesn't need the WS harness.

The guard also pins formatting rather than behavior: expect(hunks).toContain("Effect.as(null)") and the whitespace-sensitive \?\?\s*\(Option\.isSome regex both fail on a behavior-identical rewrite (Effect.map(() => null), a prettier reflow after a rename). Asserting the resolver's outputs from a unit test, with the guard reduced to "the fenced hunk still calls the resolver before dispatch", survives sync churn better.

Findings 1–4 are about the policy, not its address, so they carry over unchanged if the resolver gets extracted.

Reviewed with Claude Opus 5 (1M context) via Claude Code (T3 Code).

[NoahHendrickson]

Addressed in ebd1ec38c4fbbe67665ac2cf16a715c9cf4a5aac (and the extraction in e454feee0):

1. Placeholder branches — fixed. The resolver now returns null for a t3/… name via isTemporaryWorktreeBranch, same as the checkpoint and provider reactors, with a unit test built on buildTemporaryWorktreeBranchName.
2. More than a label — agreed, and intended. The manifest intent now spells out that a filled thread crosses every gate keyed on thread.branch: the "Branch changed" banner and its Restore button, the sidebar's status lease, PR resolution and badge attribution. A phone thread should behave like the same thread started on the desktop; that is the point of recording a real branch rather than drawing one.
3. Cost on the turn-start path — kept as is, recorded as an accepted trade-off. It is the same cache-served status the open desktop already polls for every labelled row on that checkout, so it is warm in practice. A cold read on a large dirty repo is the price of the label being present from the first event instead of arriving in a second one.
4. One-shot — kept, recorded as a known limitation. A repeatable backfill would be a second mechanism for an index-lock race that is rare in practice; web self-heals on its next send, mobile does not.
5. Tests — done with the extraction: eight unit tests next to the resolver cover every path, the router test is one thin seam case, and the guard pins the call site and outcomes rather than the inline shape.

The only red check on the previous head was the known-flaky mobile shikiReviewHighlighter "without a warmup" case; this push re-runs it.