fix(pairing): backport upstream mobile pairing and tailscale endpoint fixes

[enisze]

Our fork is 653 commits behind pingdotgg/t3code (152 ahead). While chasing a phone that couldn't pair with the desktop, I checked what upstream already fixed in this area and found four commits we simply never picked up. This backports them rather than reinventing them — a full 653-commit merge conflicts in 86 files / 300 hunks (including Migrations.ts, the orchestration decider, and ChatView.tsx at 45 hunks alone), so that stays a separate job.

Backported (cherry-picked with -x, original authors preserved)

Upstream	What it fixes
#4990 fix(mobile): default bare IP pairing to HTTP	Typing 192.168.1.21:3773 built an https URL against a plain-HTTP desktop, so the handshake failed and reported an opaque transport error
#7086 fix(mobile): stop a directly-saved backend from hiding its T3 Connect environment	A directly-saved backend masked the same machine's T3 Connect entry
#6487 fix(mobile): recover the QR pairing scanner when camera access is denied	Permanently-denied camera left the scanner dead with no route to Settings
#7116 fix(desktop): keep tailscale spawn defects from breaking advertised endpoints	A tailscale spawn defect took down the whole advertised-endpoint list — which matters here, since Tailscale is the answer for pairing from cellular

One conflict, in ConnectionsNewRouteScreen.tsx: upstream's hunk also imported useRef for a commit we haven't taken. Resolved by keeping our hook set and taking only Linking, which #6487 actually needs.

Also included: a one-line fixture fix (sidebarV2GroupByProject missing from DesktopClientSettings.test.ts). That test was already failing on main and typecheck already flagged the object as incomplete — unrelated to the backports, but the suite had to be green to verify them.

Verification

• apps/mobile 556, apps/desktop 406 (was 405 + 1 pre-existing failure), packages/tailscale 14, packages/shared 323 — all pass.
• Lint and format clean across the touched paths; desktop typecheck reports no errors.
• Two pre-existing mobile typecheck errors remain in Stack.tsx and archivedThreadList.test.ts. Both files are untouched by this branch — confirmed against the diff — so they predate it and are out of scope here.
• arm64 DMG built from this branch.

Relationship to the open PRs

#68 independently reimplemented #4990's scheme fix before I knew upstream had it. Upstream's version is canonical, so #68 should be trimmed to just its transport-error hint (which upstream lacks) and #69 rebased on top.

What this does not fix

Not the original report. The desktop serves http://192.168.1.21:3773 correctly, the firewall is off, Android cleartext HTTP is already enabled by withAndroidCleartextTraffic.cjs, iOS declares NSLocalNetworkUsageDescription — and with the correct address entered, no connection from the phone reaches this machine. That points at the network path between phone and Mac, not at app code.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches persistence migrations, git/PR merge RPC authorization, and checkpoint cwd selection alongside user-facing pairing; most pairing changes are localized, but the bundled server and projection changes affect core orchestration paths.

Overview
Improves mobile–desktop pairing and connection UX by defaulting schemeless IP hosts to HTTP in buildPairingUrl, documenting transport rules in the mobile test skill, and only treating relay-managed saves as occupying a T3 Connect slot so a direct backend does not hide the matching cloud environment. The QR flow now distinguishes revocable vs permanently denied camera permission and offers Open Settings when the user cannot be prompted again.

On desktop, getState and getAdvertisedEndpoints re-resolve the LAN address from current interfaces (without rebinding the backend), so pairing URLs and QR codes stay correct after DHCP or Wi‑Fi changes; tests cover roaming and local-only fallbacks.

The same diff also carries a large project/orchestration slice: new projected project fields (GitHub account, worktree branch defaults/prefix, review model, preview port, worktree copy files) with DB migrations, cwd-based account and default-model routing, document attachments written into the worktree on turn start, checkpoint baselines aligned with the session-runtime repo for turn diffs, git merge PR support and richer PR status, worktree file copying on PR-thread setup, and mobile parity (thread list sorted by recent activity, project worktree defaults in git/task flows).

^{Reviewed by Cursor Bugbot for commit 0763091. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Fix mobile pairing URL scheme defaults and harden tailscale spawn error handling

• buildPairingUrl in pairing.ts now defaults to http for bare IP literals and https for bare hostnames, preserving explicit schemes when provided
• readTailscaleStatus and runTailscaleCommand in tailscale.ts wrap spawn(...) with Effect.catchDefect to convert synchronous spawn defects (e.g. ENOTDIR) into typed TailscaleCommandSpawnError failures instead of letting them escape uncaught
• DesktopServerExposure.ts now re-resolves advertisedHost and endpointUrl from current network interfaces on each getState call, so LAN IP changes are reflected without relaunch
• Risk: parseGitHubAuthStatus in gitHubAuthStatus.ts was rewritten to parse human-readable gh auth status output only; JSON parsing support is removed, which may affect older or custom gh wrappers that relied on JSON output

📊 Macroscope summarized 0763091. 162 files reviewed, 74 issues evaluated, 54 issues filtered, 15 comments posted

🗂️ Filtered Issues

apps/server/src/attachmentStore.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 81: attachmentRelativePath now stores a document under its original extension (for example, <id>.pdf), but resolveAttachmentPathById only probes image extensions plus .bin. Asset access calls that ID resolver, so document attachment URLs consistently resolve as missing instead of serving the uploaded document. [ Cross-file consolidated ]

apps/server/src/orchestration/Layers/ProviderCommandReactor.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 73: safeDocumentBaseName permits Windows reserved device basenames such as CON, NUL, and COM1 (and names ending in .). A document with such a valid user-provided name reaches writeFile at line 672, where Windows cannot create an ordinary file with that name; the error is swallowed and the attachment is silently omitted from the model prompt. The helper's stated portable-filename contract is therefore not met. [ Out of scope (post-validation triage) ]

apps/server/src/orchestration/Normalizer.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 149: The document branch makes checkpoint reverts delete even documents that belong to retained messages. collectThreadAttachmentRelativePaths only adds image attachments, so the prune step receives no document paths and removes every matching document file from attachmentsDir; the persisted retained message still references the now-missing document. [ Already posted ]

apps/server/src/sourceControl/GitHubSourceControlProvider.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 132: withReviewState reports the value returned by readPullRequestReviewState as the total unresolved-thread count, but that lookup only fetches reviewThreads(first:100) and does not paginate. A PR with more than 100 review threads and unresolved threads outside that first page will show an understated (potentially zero) unresolved-comment warning. [ Cross-file consolidated ]

apps/server/src/sourceControl/gitHubPullRequests.ts — 2 comments posted, 4 evaluated, 2 filtered

• line 88: classifyMergeability returns "unknown" for mergeStateStatus: "DRAFT" (and "BEHIND"), although GitHub reports DRAFT specifically as a blocked merge state. Consequently an attempt to merge a draft PR follows the generic failure path rather than reporting it as blocked, despite this helper being used to provide the concrete merge-block reason. [ Out of scope (post-validation triage) ]
• line 151: A completed check with conclusion NEUTRAL is classified as pending at line 151. GitHub treats neutral completed checks as successful for dependent checks, and the GitHub CLI itself classifies NEUTRAL as passing, so this leaves successful neutral checks shown as indefinitely pending. [ Out of scope (post-validation triage) ]

apps/server/src/terminal/Manager.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 1870: When a project has an attached account whose token cannot be minted, resolveForCwd returns _tag: "unavailable", but this branch returns {}. The spawned terminal consequently retains the ambient gh credentials and inherited git credential helpers, so any git push or gh command can act as the wrong account rather than the project-selected account. The resolver explicitly distinguishes this state to prevent that fallback. [ Exceeded comment limit ]

apps/server/src/vcs/GitVcsDriverCore.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 2400: readUntrackedReviewDiffs applies the new 10 MB limit independently to every untracked path, then retains every patch in diffs and concatenates them with join. A workspace containing many untracked files (for example, 200 files each just below 10 MB) can make a single preview allocate and return gigabytes of patch text, despite the stated 10 MB review budget, and can OOM the server process. [ Exceeded comment limit ]

apps/server/src/vcs/VcsProcess.ts — 1 comment posted, 2 evaluated, 1 filtered

• line 130: The bare normalized.includes("at least") condition is not specific to a blocked merge and applies to every VcsProcess command. Any unrelated CLI failure that says, for example, that a command "requires at least" one argument is surfaced as merge-blocked; GitHub callers then present it as a PR mergeability problem rather than the actual command failure. Restrict this match to the platform's approval-review message. [ Out of scope (post-validation triage) ]

apps/server/src/workspace/WorkspaceSearchIndex.ts — 0 comments posted, 2 evaluated, 2 filtered

• line 220: The containment check treats every relative path beginning with ".." as outside cwd. A valid workspace directory such as ..config containing .env produces ..config/.env from NodePath.relative, so line 220 drops that in-workspace env file and the supplemental listing fails to surface it. Check an actual parent traversal segment (for example relativePath === ".." || relativePath.startsWith("../")) instead. [ Out of scope (post-validation triage) ]
• line 349: list invokes collectEnvEntries without any cap on discovered .env* files. ENV_WALK_MAX_DIRS only bounds directories, so a workspace containing a single directory with a very large number of .env.* files causes the walk to retain every path (and readdir to materialize every dirent) before the later slice(0, 25_000). A repository/workspace can therefore make a list request consume unbounded memory and stall or terminate the server despite the advertised bounded walk. [ Exceeded comment limit ]

apps/server/src/ws.ts — 0 comments posted, 3 evaluated, 3 filtered

• line 382: buildContinuationTranscript does not actually enforce CONTINUATION_TRANSCRIPT_MAX_CHARS. It budgets only the raw tail lines, then appends two "\n\n" separators and the "[… earlier conversation omitted …]" marker; moreover, if the first nonempty message alone is over the limit it is emitted intact. Thus long transcripts can still exceed the promised 24,000-character bound and send an oversized request to the continuation summarizer. [ Exceeded comment limit ]
• line 934: runCopyProjectFilesProgram only invokes copyForThread when bootstrap.runSetupScript is true. runSetupScript is optional and can explicitly be false while prepareWorktree still creates a fresh worktree, so configured worktreeCopyFiles (such as .env.local) are silently omitted for those worktrees even though copying is independent of running a setup script. [ Exceeded comment limit ]
• line 988: The duplicate-create recovery continues into prepareWorktree even when created is false. A retried draft turn carries the same newRefName; createWorktree explicitly treats an existing branch with -b as a failure, so a retry after the original worktree was created still fails rather than continuing the existing thread as this path intends. [ Exceeded comment limit ]

apps/web/src/components/BranchToolbar.logic.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 268: For a remote ref whose corresponding local branch already exists but is not checked out, this always supplies that local name as newRefName (for example origin/feature -> feature). The ref list retains non-origin remotes even when they match a local branch, while createWorktree invokes git worktree add -b whenever newRefName is present and explicitly cannot reuse an existing branch in that case. Clicking “Open this exact branch” for that remote therefore fails instead of reusing/opening the available local branch. [ Exceeded comment limit ]

apps/web/src/components/ChatView.tsx — 0 comments posted, 7 evaluated, 7 filtered

• line 757: worktreeSettled is false for a local-mode draft because it has neither a server thread nor a worktreePath. The terminal toggle still opens a session using the project root, but this drawer computes cwd as null and returns null, so users cannot see or interact with terminals in an unsent local draft. Gate only drafts that are actually awaiting a worktree, not every draft without one. [ Exceeded comment limit ]
• line 1624: Mounted terminal drawers are reconciled and made visible using activeThreadKey, while terminal UI state is now keyed by workspaceThreadRef. On a non-representative chat in a shared worktree, this mounts a drawer for the sibling key; PersistentThreadTerminalDrawer then reads that sibling's default closed state, so the shared terminal drawer disappears when switching sibling chats. [ Exceeded comment limit ]
• line 2440: addComposerFiles validates images and documents in separate calls against stale state. On a mixed drop/paste, addComposerImages permits up to the image-only limit and addComposerDocuments reads composerImagesRef before those additions are committed, so users can attach up to 10 images plus 10 documents despite the documented shared PROVIDER_SEND_TURN_MAX_ATTACHMENTS cap. [ Exceeded comment limit ]
• line 2741: toggleTerminalVisibility stores the new terminal under the worktree representative (workspaceThreadRef) but opens it with threadId: activeThreadId. When a non-representative sibling chat is active, later terminal queries are filtered to workspaceThreadId, so the new session is not visible as part of the shared workspace and can be removed from the shared UI on reconciliation. [ Exceeded comment limit ]
• line 3234: startConflictResolutionInNewChat creates a forced draft without preservePreviousDraft. If this project already has an unsent draft (for example, another worktree chat tab), useNewThreadHandler replaces the logical-project draft mapping and deletes that prior draft's composer state. Starting conflict resolution therefore silently destroys the user's unsent prompt and attachments. [ Exceeded comment limit ]
• line 4663: The empty-worktree path intentionally bypasses the no-provider send guard, but still dispatches a createThread request with ctxSelectedModelSelection. With no configured provider this is NO_PROVIDER_MODEL_SELECTION, whose definition explicitly says it must never be persisted or dispatched. Thus the enabled “Create worktree” action sends a placeholder provider/model as real thread metadata instead of requiring a usable model selection. [ Exceeded comment limit ]
• line 4681: The empty-worktree path creates an on-disk worktree before calling createThread, but if createThread fails it only reports the error and returns. The newly-created worktree/branch is never deleted, leaving orphaned worktrees that users must manually clean up after a transient thread-creation failure. [ Exceeded comment limit ]

apps/web/src/components/ProjectDefaultAgentField.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 71: entries only filters on isProviderInstancePickerVisible, which accepts every enabled instance, including entries with isAvailable === false or an unusable probe state. Selecting such an entry persists it as the project's default; useHandleNewThread then applies that project selection directly to each new draft, although the existing selection resolver explicitly excludes unavailable instances. Thus a provider unavailable in the current build can be selected as the project's agent and new chats/reviews are routed to an unusable provider instead of a selectable fallback. [ Exceeded comment limit ]

apps/web/src/components/ProjectDefaultWorktreeBranchField.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 55: branchNames includes the remote-tracking ref name (for example origin/main) and onValueChange persists it unchanged as defaultWorktreeBranch. The VCS list explicitly returns a remote-only default as origin/<branch> when no local equivalent exists, while this setting is also forwarded unchanged as the PR/MR base branch. Thus choosing the displayed repository default on such a clone makes PR creation target origin/main, which is not a branch on the hosting service and is rejected by gh pr create; the base must be normalized to main before persistence/use as a change-request target. [ Exceeded comment limit ]

apps/web/src/components/ProjectScriptsField.tsx — 0 comments posted, 4 evaluated, 3 filtered

• line 78: persist commits the project script update before attempting the separate keybinding mutation. If upsertKeybinding then fails, the dialog reports the save as failed even though the script is already stored; retrying an add appends a second script (with a different generated id), causing duplicate actions despite the reported failure. [ Exceeded comment limit ]
• line 90: The Electron check does not restrict keybinding persistence to the primary desktop environment: when this settings field edits a remote project's script, it still calls upsertKeybinding with that remote environmentId. This contradicts the intended primary-server-only reconciliation and saves a shortcut into the wrong environment instead of leaving remote scripts without a shortcut. [ Exceeded comment limit ]
• line 92: When editing a script to a different nonempty shortcut, persist upserts only the new rule and supplies no replace target. upsertKeybindingRule removes only an identical key-and-command rule unless replace is given, so the former shortcut remains registered and both shortcuts run the script. [ Exceeded comment limit ]

apps/web/src/components/ProjectWorktreeCopyFilesField.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 14: parseWorktreeCopyFiles does not enforce the contract's 512-character maximum for an individual path. Pasting a non-empty line longer than 512 characters returns it unchanged and onBlur sends it through onChange; ProjectWorktreeCopyFiles rejects that value, so the entire settings update fails rather than saving the other valid entries or showing a local validation error. [ Out of scope (post-validation triage) ]

apps/web/src/components/Sidebar.logic.ts — 0 comments posted, 2 evaluated, 2 filtered

• line 923: mergeWorktreeSiblingRunningStatus only projects the session and attention flags, leaving snoozedUntil on the earliest representative. Thus, if that representative is snoozed while another sibling in the same collapsed worktree is active/running, the subsequent sidebar partition classifies the merged row as snoozed and removes the active worktree from the inbox (conversely snoozing a non-representative has no visible effect). The collapsed row needs group-level snooze handling or snoozing must apply to the group. [ Exceeded comment limit ]
• line 923: The projected running session belongs to a sibling, but the returned row keeps the representative's latestTurn. WorkingDuration calls resolveWorkingStartedAt on that mixed row: when the representative's prior turn is complete, it falls back to the sibling session's updatedAt instead of the running sibling's startedAt/requestedAt. A collapsed worktree can therefore display an incorrect (typically too short) elapsed Working timer. [ Out of scope (post-validation triage) ]

apps/web/src/components/SidebarV2.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 1013: When a project has a previously selected GitHub account but discovery returns no authenticated accounts (for example, its token was revoked), the accounts.length === 0 branch renders only the "No GitHub accounts" message. It omits the "Use default account" option, so the user cannot clear the now-invalid member.gitHubAccount from this settings UI despite this being the selection's recovery path. [ Exceeded comment limit ]

apps/web/src/components/chat/ChangedFilesTree.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 393: The row's onKeyDown handles Space/Enter after events bubble from the nested Checkbox. Consequently, activating a focused viewed checkbox with the keyboard also calls onOpenFile(file.path) (and Space is prevented at the row), so keyboard users cannot mark a file viewed without navigating to that file. Stop keyboard propagation from the checkbox or have the row ignore events originating from interactive descendants. [ Exceeded comment limit ]

apps/web/src/components/chat/ChatHeader.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 37: Replacing gitCwd with previewPort removes the header's GitActionsControl input and its rendering. The only remaining GitActionsControl is passed as RightPanelTabs header content, which is mounted only while the right panel is open. Consequently, when the panel is closed, users have no access to git commit/push/PR actions that were previously available in the chat header. [ Exceeded comment limit ]

apps/web/src/components/chat/HeaderOverflowMenu.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 74: Placing OpenInPicker inside PopoverPopup means it is unmounted whenever the overflow is closed (the Base UI Popover.Portal defaults keepMounted to false). OpenInPicker installs the global editor.openFavorite keydown listener in its mount effect, so the favorite-editor shortcut no longer works during normal use until the user opens the overflow menu. [ Exceeded comment limit ]

apps/web/src/components/files/FilePreviewPanel.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 399: EditableFileSurface no longer supplies enableGutterUtility, enableLineSelection, onLineSelectionEnd, or renderAnnotation to the editable File. Consequently, selecting a source-file line in the file panel can no longer open a LocalCommentAnnotation or save a file review comment to composerDraftTarget; the prop is still passed into this surface but is now entirely unused. This removes the existing inline review-comment workflow for editable file previews. [ Exceeded comment limit ]

apps/web/src/components/preview/addBrowserSurface.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 22: addBrowserSurface now routes through openBrowserPreviewInChat, but that helper determines the worktree solely with readThreadShell. A newly created client-side draft that already targets an existing worktree has no server shell (the workspace resolver explicitly obtains its worktree from the draft store), so opening a browser tab from that draft falls back to the right panel instead of creating the chat-column content tab. This makes the new preview routing fail until the draft has been sent/promoted. [ Exceeded comment limit ]

apps/web/src/components/preview/openBrowserPreviewInChat.ts — 0 comments posted, 2 evaluated, 2 filtered

• line 19: openBrowserPreviewInChat derives the worktree exclusively from readThreadShell. A newly created client-side draft has no server shell, even when its composer draft targets a worktree, so opening a preview from that draft always takes the right-panel fallback rather than adding the preview to its worktree content-tab strip. This contradicts the draft worktree behavior used elsewhere and leaves the new preview inaccessible from the intended chat-column surface. [ Cross-file consolidated ]
• line 26: openBrowserPreviewInChat records the tab under the caller's threadRef, but then stores it in a worktree-shared content strip. For a non-representative sibling chat, ChatView renders that strip using workspaceThreadRef and reads activePreviewState.sessions from that representative, so the newly created session (owned by the sibling) is absent; the preview tab is blank and closing it also targets the wrong thread/session. [ Exceeded comment limit ]

apps/web/src/components/settings/ProjectSettingsPanel.tsx — 0 comments posted, 2 evaluated, 1 filtered

• line 229: An invalid nonempty preview-port edit is converted to null and persisted, silently clearing the currently configured port. For example, a project set to 5173 loses that setting when the user enters 70000 (which users can type despite max) and blurs; the existing settings handler instead rejects out-of-range values and preserves the current port. [ Exceeded comment limit ]

apps/web/src/components/settings/ProviderUsageSection.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 98: The footer always appends ago, but formatElapsedDurationLabel returns the complete phrase just now for a newly fetched (or clock-skewed future) timestamp. Fresh provider usage therefore renders as Updated just now ago, which is visibly incorrect until enough time has elapsed to produce a numeric label. [ Out of scope (post-validation triage) ]

apps/web/src/components/settings/SettingsUsagePill.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 60: SettingsUsagePill always reads primaryServerProvidersAtom, but chat routes can target a non-primary environment (threads are explicitly scoped by environmentId). When a remote chat is active, its modelSelection.instanceId is searched only in the primary server's providers, so the pill shows — (and refreshes the primary server) instead of that chat's usage. This contradicts the component's active-chat summary behavior for every remote environment. [ Exceeded comment limit ]

apps/web/src/components/tasks/TasksDock.tsx — 0 comments posted, 1 evaluated, 1 filtered

• line 70: maxBodyHeight() is only consulted when a stored height is read or during a drag. After the dock is sized at a large viewport, shrinking the browser window leaves bodyHeight unchanged and it is rendered as a fixed pixel height, so the task dock can exceed the new 70%-viewport cap and obscure the chat area until the user resizes the dock manually. [ Out of scope (post-validation triage) ]

apps/web/src/hooks/useStaleArchivedWorktreeCleanup.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 155: The eligibility check is only performed on the data snapshot captured before the confirmation dialog. If the user starts or unarchives a chat using one of the listed worktrees while that dialog is open, the loop never rechecks liveThreads and sends force: true to removeWorktree; it can therefore delete the newly active worktree and its uncommitted files. [ Exceeded comment limit ]

apps/web/src/projectScripts.ts — 0 comments posted, 3 evaluated, 3 filtered

• line 44: appendProjectScript can append a duplicate script id after the suffix search is exhausted. With a 64-character normalized name and existing ids through -9999, nextProjectScriptId falls back to ```${baseId}-${Date.now()}.slice(0, 64)``, which is exactly the already-taken baseId`; the newly appended script then collides with an existing one, breaking keyed rendering and script/keybinding selection. [ Already posted ]
• line 44: appendProjectScript can append a duplicate script id after the suffix search is exhausted. With a 24-character normalized name and existing ids through -9999, nextProjectScriptId falls back to ```${baseId}-${Date.now()}.slice(0, MAX_SCRIPT_ID_LENGTH)``, which truncates back to the already-taken baseId`; the new script then collides with an existing script and its keyed UI/keybinding command. [ Already posted ]
• line 150: isTaskTerminalId classifies every task- id as task-owned, but TasksDock only initializes shellIds with task-shell and does not restore additional task-shell-N tabs after it unmounts. Closing the right panel unmounts the dock while its extra shell sessions remain alive; on reopening, those sessions are no longer represented by the dock and this filter also removes them from the general terminal drawer, leaving them inaccessible. [ Exceeded comment limit ]

apps/web/src/state/query.ts — 0 comments posted, 2 evaluated, 1 filtered

• line 46: recoveredRef is scoped to the hook instance rather than selectedAtom, and is never reset when the caller switches to a different query atom. After any interrupted query sets it to true, a later parameter/environment change can produce a different interrupted query; its effect returns at line 46 without refreshing it, so that new query is also hidden and remains permanently pending. [ Exceeded comment limit ]

apps/web/src/workspaceContentTabsStore.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 172: closeTab removes the preview tab from the worktree strip before the caller's asynchronous closePreviewSession completes. If the preview-close RPC fails, closePreviewSession explicitly restores the preview session, but nothing restores the removed content tab; the still-live preview is therefore no longer accessible from the chat-column tab strip until the user opens it again. [ Exceeded comment limit ]

packages/client-runtime/src/state/server.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 70: The snapshot branch replaces config.providers directly instead of applying retainModelsAcrossTimeouts. Every subscribeServerConfig subscription begins with a snapshot, so after a reconnect/reload where the server's current provider probe is timed out (and therefore has models: []), this overwrites the cached/live catalogue and the picker again loses all models—the precise timeout case the new helper is meant to preserve. [ Exceeded comment limit ]

packages/contracts/src/orchestration.ts — 0 comments posted, 2 evaluated, 2 filtered

• line 187: ChatDocumentAttachment allows filenames such as report.pdf, which are persisted using that .pdf extension. Existing resolveAttachmentPathById only searches image extensions and .bin, so the asset endpoint cannot resolve the document by its attachment ID; consequently useAssetUrls returns no preview/download URL for normal document attachments. Add document extensions to ID lookup or persist documents as .bin. [ Exceeded comment limit ]
• line 198: UploadChatDocumentAttachment permits a 25 MiB document, but the enclosing ClientThreadTurnStartCommand.message.attachments array has no maximum-length check. The normalizer iterates every supplied attachment and writes each decoded document to disk before later provider validation; a client can therefore submit arbitrarily many valid 25 MiB document data URLs in one turn and exhaust server disk space (the declared PROVIDER_SEND_TURN_MAX_ATTACHMENTS limit is only enforced by some clients/provider input, not this ingress schema). [ Exceeded comment limit ]

packages/shared/src/git.ts — 0 comments posted, 3 evaluated, 3 filtered

• line 22: The wildcard prefix makes every single-segment branch ending in eight hex characters look like an app-created temporary branch, not just a branch made with the configured prefix. For example, a user worktree on release/deadbeef that is used to start a new thread is accepted here; maybeGenerateAndRenameWorktreeBranchForFirstTurn then renames it on the first user message. This can unexpectedly rename an existing user branch and update the thread to the generated branch. [ Exceeded comment limit ]
• line 124: sanitizeWorktreeBranchPrefix leaves internal dots unchanged, so a valid user setting such as team..wip returns team..wip and buildTemporaryWorktreeBranchName produces team..wip/<token>. Git explicitly rejects refnames containing .., so attempts to create a new worktree with that prefix fail even though this helper promises a valid ref segment. [ Exceeded comment limit ]
• line 130: The sanitizer also accepts a prefix whose segment ends in .lock: sanitizeWorktreeBranchPrefix("team.lock") returns team.lock, producing team.lock/<token>. Git rejects any ref component ending in .lock, so that accepted worktree-prefix setting makes all new worktree creation attempts fail. [ Already posted ]

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 683c9ece-cf3a-4322-9b30-3cda66179883

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[enisze]

Opened by mistake against upstream — these four commits are already in upstream/main; this was meant as a backport PR on a fork. Closing, no review needed. Sorry for the noise.

[macroscopeapp]

Effect service conventions: a few issues in the new GitHub account/merge code.

• Effect.catchTag is used in four new places; the convention is Effect.catchTags({ ... }) even for a single tag.
• GitHubAccountNotLoggedInError manufactures an Error purely to fill a required cause.

The new ProjectWorktreeFileCopier and GitHubAccountResolver service modules otherwise follow the expected shape (inline Context.Service interface, make + layer, dependencies acquired via yield*, optional-service resolution at consumers).

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

GitHubAccountNotLoggedInError is a pure domain refusal — nothing failed underneath, so this synthetic Error exists only to satisfy the required cause. Consider making cause optional on gitHubAccountFailureFields (cause: Schema.optional(Schema.Defect()), it is used only by this error) and omitting it here; the structural host/login/cwd fields already carry the full story.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Prefer Effect.catchTags over catchTag for a statically known tag.

Suggested change

	Effect.catchTag("OrchestrationCommandInvariantError", (error) =>
	isThreadAlreadyExistsInvariantError(error, command.threadId)
	? Effect.succeed(false)
	: Effect.fail(error),
	),
	Effect.catchTags({
	OrchestrationCommandInvariantError: (error) =>
	isThreadAlreadyExistsInvariantError(error, command.threadId)
	? Effect.succeed(false)
	: Effect.fail(error),
	}),

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Convention is Effect.catchTags({ ... }) for statically known tags, including a single tag.

Suggested change

	Effect.catchTag("GitHubProviderUnavailableError", (error) =>
	attemptsLeft <= 1
	? Effect.fail(error)
	: Effect.sleep(TRANSIENT_RETRY_INTERVAL).pipe(
	Effect.flatMap(() => retryProviderUnavailable(effect, attemptsLeft - 1)),
	),
	),
	Effect.catchTags({
	GitHubProviderUnavailableError: (error) =>
	attemptsLeft <= 1
	? Effect.fail(error)
	: Effect.sleep(TRANSIENT_RETRY_INTERVAL).pipe(
	Effect.flatMap(() => retryProviderUnavailable(effect, attemptsLeft - 1)),
	),
	}),

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Same here — prefer Effect.catchTags.

Suggested change

	Effect.catchTag("GitHubMergeBlockedError", (error) =>
	Effect.fail(
	new GitHubMergeBlockedError({
	command: error.command,
	cwd: error.cwd,
	cause: error.cause,
	mergeability,
	}),
	),
	),
	Effect.catchTags({
	GitHubMergeBlockedError: (error) =>
	Effect.fail(
	new GitHubMergeBlockedError({
	command: error.command,
	cwd: error.cwd,
	cause: error.cause,
	mergeability,
	}),
	),
	}),

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Same here — prefer Effect.catchTags.

Suggested change

	Effect.catchTag("GitHubMergeBlockedError", (error) =>
	attemptsLeft <= 1
	? Effect.fail(error)
	: Effect.sleep(MERGE_RETRY_INTERVAL).pipe(
	Effect.flatMap(() => attemptMerge(cwd, args, attemptsLeft - 1)),
	),
	),
	Effect.catchTags({
	GitHubMergeBlockedError: (error) =>
	attemptsLeft <= 1
	? Effect.fail(error)
	: Effect.sleep(MERGE_RETRY_INTERVAL).pipe(
	Effect.flatMap(() => attemptMerge(cwd, args, attemptsLeft - 1)),
	),
	}),

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

UI Consistency: 3 issues found

Three issues in the changed web UI:

1. SidebarV2.tsx reimplements the new shared ProjectGitHubAccountField instead of importing it, so the same per-project control renders with different labels, option text and sentinel values in the two sidebars.
2. TasksDock.tsx terminal-tab close button is hover-only — no focus-visible reveal/ring and no coarse-pointer fallback, unlike the sibling tab strips (WorktreeThreadTabs, RightPanelTabs) added in the same PR.
3. GitActionsControl.tsx leaves the Publish-repository flow unreachable after the git-actions menu was replaced by a single primary button; the dialog is still rendered but nothing can open it.

Nothing else flagged: diff theming is routed consistently through useDiffThemeName, tab strips reuse ScrollArea with the shared fade contract, and no CSS/theme files changed.

^{Posted via Macroscope — UI Consistency}

[macroscopeapp]

This close control is opacity-0 and only revealed by group-hover, with no focus-visible styling: keyboard-focusing it leaves an invisible, unringed target, and on a coarse pointer (no hover) it can't be revealed at all. The tab strip added alongside it in this PR (WorktreeThreadTabs) already handles both cases on its close button (focus-visible:opacity-100 focus-visible:outline-hidden focus-visible:ring-1 focus-visible:ring-ring max-sm:opacity-100).

Suggested change

	className="flex size-4 shrink-0 items-center justify-center rounded text-muted-foreground opacity-0 hover:bg-muted hover:text-foreground group-hover:opacity-100"
	className="flex size-4 shrink-0 items-center justify-center rounded text-muted-foreground opacity-0 transition-opacity hover:bg-muted hover:text-foreground focus-visible:opacity-100 focus-visible:outline-hidden focus-visible:ring-1 focus-visible:ring-ring max-sm:opacity-100 group-hover:opacity-100"

The adjacent “New terminal” + button (line 729) is also a raw button with no focus ring — either give it focus-visible:ring-2 focus-visible:ring-ring or render it as Button variant="ghost" size="icon-sm", which already carries the ring, cursor and disabled semantics.

^{Posted via Macroscope — UI Consistency}

[macroscopeapp]

Replacing the Group + Menu block with this single primary button drops the only two entry points into the publish flow (the open_publish quick action and the “Publish repository…” menu item). setIsPublishDialogOpen(true) now only exists inside runQuickAction, which is itself no longer called, so <PublishRepositoryDialog> at the bottom of this component can never open — a repo with no remote is left with a “Create PR” button that will fail.

Same block also dropped the only surface for gitStatusError and the detached-HEAD warning, so git status failures are now silent in the header.

Could you either re-expose publish (a secondary button when canPublishRepository, or keep the overflow menu) or remove the now-dead code with it? Currently Group/GroupSeparator, Menu/MenuItem/MenuPopup/MenuTrigger, gitActionMenuItems, openDialogForMenuItem, getMenuActionDisabledReason, GitActionItemIcon, GitQuickActionIcon, quickAction/quickActionDisabledReason/runQuickAction and canPublishRepository are all unreferenced.

^{Posted via Macroscope — UI Consistency}

[macroscopeapp]

This PR adds a shared ProjectGitHubAccountField (apps/web/src/components/ProjectGitHubAccountField.tsx) and uses it from Sidebar.tsx, but SidebarV2 reimplements the same control locally with a different prop shape and drifting presentation:

• trigger label login · host vs login (host)
• placeholder Default account vs Use default account
• active account marked with a " (default)" text suffix vs a muted default chip
• different sentinel/value encoding (__default__ + \u0000 join vs the shared module's own key format)

Same project, two sidebars, two different pickers. Consider importing the shared component here and folding this version's extras (the gh auth login empty state and the stale-token warning) into it, so both surfaces stay identical.

The inline Worktree branch prefix and Preview port fields below have the same problem against the new ProjectSettingsPanel, and they have already drifted: this dialog falls back to the global worktreeBranchPrefix setting for the placeholder/helper text, while ProjectSettingsPanel always shows the built-in WORKTREE_BRANCH_PREFIX, so the two surfaces advertise a different inherited default for the same project.

^{Posted via Macroscope — UI Consistency}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 0763091. Configure here.}

[cursor]

Document attachments unresolvable by id

Medium Severity

Document attachments now persist as {id}.pdf / {id}.xlsx (and similar) via inferDocumentExtension, but resolveAttachmentPathById still only probes image extensions plus .bin. Asset URL creation and serving go through that id-only lookup, so a PDF or spreadsheet in chat cannot be found on disk after persist. Worktree copy still works because it uses the full attachment object; the chat asset path does not.

Additional Locations (1)

• apps/server/src/attachmentStore.ts#L56-L84

 

^{Reviewed by Cursor Bugbot for commit 0763091. Configure here.}

[macroscopeapp]

🟡 Medium Layers/ProjectionSnapshotQuery.ts:395

getDefaultModelSelectionForCwd returns the parent project's model for a cwd inside a nested project whose default_model_selection_json is NULL, instead of returning None. Because the SQL query omits unset projects, the nested /repos/child route cannot shadow the /repos route; include unset project routes and select the longest match before converting a null selection to None.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts around line 395:

`getDefaultModelSelectionForCwd` returns the parent project's model for a `cwd` inside a nested project whose `default_model_selection_json` is `NULL`, instead of returning `None`. Because the SQL query omits unset projects, the nested `/repos/child` route cannot shadow the `/repos` route; include unset project routes and select the longest match before converting a null selection to `None`.

[macroscopeapp]

🟡 Medium sourceControl/gitHubPullRequests.ts:147

A completed check with conclusion STALE is reported as "passing" with failedCheckCount: 0, so a stale required check can make the PR appear healthy even though it has not succeeded and may block merging. Add STALE to the non-passing conclusion handling.

-              check.status?.toUpperCase() !== "COMPLETED" ||
+              check.status?.toUpperCase() !== "COMPLETED" ||
                 !check.conclusion ||
-                check.conclusion.toUpperCase() === "NEUTRAL",
+                check.conclusion.toUpperCase() === "NEUTRAL" ||
+                check.conclusion.toUpperCase() === "STALE",

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/sourceControl/gitHubPullRequests.ts around lines 147-152:

A completed check with conclusion `STALE` is reported as `"passing"` with `failedCheckCount: 0`, so a stale required check can make the PR appear healthy even though it has not succeeded and may block merging. Add `STALE` to the non-passing conclusion handling.

[macroscopeapp]

🟡 Medium sourceControl/gitHubPullRequests.ts:149

Legacy StatusContext entries are always reported as pending, so pull requests whose checks use commit statuses can never be classified as passing or failing. These entries expose state (such as SUCCESS or FAILURE) rather than status/conclusion; include state in the schema and use it when counting failures and determining completion.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/sourceControl/gitHubPullRequests.ts around line 149:

Legacy `StatusContext` entries are always reported as `pending`, so pull requests whose checks use commit statuses can never be classified as `passing` or `failing`. These entries expose `state` (such as `SUCCESS` or `FAILURE`) rather than `status`/`conclusion`; include `state` in the schema and use it when counting failures and determining completion.

[macroscopeapp]

🟠 High sourceControl/GitHubAccountResolver.ts:342

When listAccountRoutes() fails, findAccountForCwd returns no account, so resolveForCwd produces _tag: "ambient" and GitHubCli or remote git operations use the machine-global credentials. This bypasses the selected-account safety boundary during projection failures and can perform pushes, fetches, or PR creation as the wrong account. Do not convert this failure to an empty route list; propagate the failure or return a refusal outcome so callers fail closed.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/sourceControl/GitHubAccountResolver.ts around line 342:

When `listAccountRoutes()` fails, `findAccountForCwd` returns no account, so `resolveForCwd` produces `_tag: "ambient"` and `GitHubCli` or remote git operations use the machine-global credentials. This bypasses the selected-account safety boundary during projection failures and can perform pushes, fetches, or PR creation as the wrong account. Do not convert this failure to an empty route list; propagate the failure or return a refusal outcome so callers fail closed.

[macroscopeapp]

🟠 High project/ProjectWorktreeFileCopier.ts:189

copyFile follows symlinks in the destination path, so a tracked path such as config pointing outside worktreePath lets config/local.json overwrite an external file. The lexical resolveRelativePathWithinRoot check does not detect this, breaking the stated containment guarantee; validate destination ancestors with symlink-aware filesystem operations or otherwise copy without following destination symlinks.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/project/ProjectWorktreeFileCopier.ts around line 189:

`copyFile` follows symlinks in the destination path, so a tracked path such as `config` pointing outside `worktreePath` lets `config/local.json` overwrite an external file. The lexical `resolveRelativePathWithinRoot` check does not detect this, breaking the stated containment guarantee; validate destination ancestors with symlink-aware filesystem operations or otherwise copy without following destination symlinks.

[macroscopeapp]

🟡 Medium vcs/VcsProcess.ts:108

Rate-limit responses containing only HTTP 403 are classified as permission-denied, so GitHubCli.fromVcsError raises GitHubPermissionError instead of indicating a temporary failure that should be retried. The generic normalized.includes("403") match causes this; remove the status-only match or detect rate-limit text before the permission rule.

    normalized.includes("forbidden") ||
-    normalized.includes("403") ||
    normalized.includes("resource not accessible") ||

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/vcs/VcsProcess.ts around lines 108-110:

Rate-limit responses containing only HTTP `403` are classified as `permission-denied`, so `GitHubCli.fromVcsError` raises `GitHubPermissionError` instead of indicating a temporary failure that should be retried. The generic `normalized.includes("403")` match causes this; remove the status-only match or detect rate-limit text before the permission rule.

[macroscopeapp]

🟡 Medium Layers/ProviderCommandReactor.ts:760

A document-only turn is sent without input when writeWorktreeDocuments returns no paths, so ProviderService.sendTurn rejects the request even though document-copy failures are intended to be best-effort. Add fallback input text for this case.

-      ...(providerInput ? { input: providerInput } : {}),
+      ...(providerInput
+        ? { input: providerInput }
+        : documentAttachments.length > 0 && providerAttachments.length === 0
+          ? { input: "The user attached documents, but they could not be made available in the worktree." }
+          : {}),

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/orchestration/Layers/ProviderCommandReactor.ts around line 760:

A document-only turn is sent without `input` when `writeWorktreeDocuments` returns no paths, so `ProviderService.sendTurn` rejects the request even though document-copy failures are intended to be best-effort. Add fallback input text for this case.

[macroscopeapp]

🟠 High orchestration/Normalizer.ts:149

Document attachments persisted by normalizeDispatchCommand are never removed when a thread is deleted or a checkpoint is reverted, so their data remains in attachmentsDir and orphaned files accumulate. collectReferencedAttachmentRelativePaths skips every attachment whose type is not image; update that cleanup to include document attachments as well.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/orchestration/Normalizer.ts around line 149:

Document attachments persisted by `normalizeDispatchCommand` are never removed when a thread is deleted or a checkpoint is reverted, so their data remains in `attachmentsDir` and orphaned files accumulate. `collectReferencedAttachmentRelativePaths` skips every attachment whose `type` is not `image`; update that cleanup to include `document` attachments as well.

[macroscopeapp]

🟡 Medium

t3code/apps/server/src/git/GitManager.ts

Line 1869 in 0763091

resolvePullRequestWorktreeLocalBranchName(pullRequestWithRemoteInfo);

When an existing worktree is found for a cross-repository PR, preparePullRequestThread returns the synthetic localPullRequestBranch (for example, t3code/pr-83/feature/foo) even though the reused worktree is actually checked out on feature/foo. Consumers therefore receive and persist a branch name that does not exist; return the reused branch's actual name instead.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/git/GitManager.ts around line 1869:

When an existing worktree is found for a cross-repository PR, `preparePullRequestThread` returns the synthetic `localPullRequestBranch` (for example, `t3code/pr-83/feature/foo`) even though the reused worktree is actually checked out on `feature/foo`. Consumers therefore receive and persist a branch name that does not exist; return the reused branch's actual `name` instead.

[macroscopeapp]

🟠 High vcs/GitVcsDriverCore.ts:470

isNonFastForwardRejection classifies any push rejection containing fetch first as remote-ahead, so a server-side hook that emits that phrase for an unrelated policy failure triggers runPushWithAutoRebase to fetch and rewrite the user's local commits before retrying. Restrict this check to Git's actual non-fast-forward rejection status instead of matching arbitrary remote output.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/vcs/GitVcsDriverCore.ts around line 470:

`isNonFastForwardRejection` classifies any push rejection containing `fetch first` as remote-ahead, so a server-side hook that emits that phrase for an unrelated policy failure triggers `runPushWithAutoRebase` to fetch and rewrite the user's local commits before retrying. Restrict this check to Git's actual non-fast-forward rejection status instead of matching arbitrary remote output.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This is a large cross-cutting feature bundle, not just a mobile-pairing backport: it changes authentication and source-control side effects, persistence, orchestration, worktree/file handling, and major chat/sidebar workflows. Auth-sensitive changes and unresolved risks around credential routing, attachment handling, and worktree safety require human review.

Not approved because:

• 15 blocking correctness issues found at or above your repo's Minimum Blocking Severity

^{Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.}