Repository navigation
Conversation
github-actions
Bot
added
vouch:trusted
Contributor
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, tested bug fix that keeps checkout terminals aligned with their actual cwd instead of inheriting a thread worktree after the thread changes. It does not alter APIs, product configuration defaults, deployment behavior, or static-analysis settings. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 31.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 22 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Title check | |
The title describes a checkout-terminal worktree fix, but the provided changeset primarily refactors the web right-panel terminal into the panel-host and registry architecture. The title does not summ… | Rename the PR with a concise title that reflects the panel-host refactor, such as "refactor(web): open the right-panel terminal through the panel host". |
| Description check | |
The description is detailed but describes a checkout-terminal resolver fix that is absent from the provided changeset and conflicts with the stated PR objectives. It also does not provide the required… | Rewrite the description to document the panel-host refactor, its problem and expected behavior, the affected components, the related issue or maintainer approval, and verification results for the actual changeset. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Title check
-
Explanation
The title describes a checkout-terminal worktree fix, but the provided changeset primarily refactors the web right-panel terminal into the panel-host and registry architecture. The title does not summarize the main change.
Resolution
Rename the PR with a concise title that reflects the panel-host refactor, such as "refactor(web): open the right-panel terminal through the panel host".
Full details: Description check
-
Explanation
The description is detailed but describes a checkout-terminal resolver fix that is absent from the provided changeset and conflicts with the stated PR objectives. It also does not provide the required scope and approval information for the panel-host refactor.
Resolution
Rewrite the description to document the panel-host refactor, its problem and expected behavior, the affected components, the related issue or maintainer approval, and verification results for the actual changeset.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
saphid
force-pushed
the
stack/03-terminal-panel
branch
3 times, most recently
from
October 6, 2026 16:03
bdb5405 to
fe0a090
Compare
Contributor
Author
|
Review requested
Logged so this PR shows when a maintainer was asked to review it. |
saphid
force-pushed
the
stack/03-terminal-panel
branch
5 times, most recently
from
October 10, 2026 02:13
8701762 to
cdb4f58
Compare
saphid
force-pushed
the
stack/03-terminal-panel
branch
2 times, most recently
from
October 10, 2026 07:06
a77d211 to
e34c8a4
Compare
Preview becomes the second panel on the side-panel registry that Diff started. Each definition now also carries the panel's title, icon, launcher letter, client support and unavailable copy, so the tabs, the empty launcher and the add menu read one ordered list instead of three hand-kept ones. Labels, letters, order and copy are unchanged. Panel props are inferred from each lazily loaded body, and the caller is a closed union, so another panel's props, unknown ids and widened ids do not compile. ChatView lends the rendered panel a small host (thread, right panel visibility, composer draft target, workspace mutation id and the annotation send) instead of drilling the same props into each body; the annotation send keeps the per-render closure it had before, and PreviewView still drops a pick that settles after a thread switch. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ChatView built a new PanelHost on every render, so every usePanelHost consumer re-rendered even when no host field changed. Memoize it on its fields and send annotations through onSendRef so the sender stays stable. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Plain server threads reuse one ChatView, so the memoized panel host's sender could resolve to the next thread's composer when a pick settled after a switch. The latest sender now carries its thread key, and each host forwards only to a sender for its own thread. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ChatView replaced the panel host's annotation sender while rendering. If React threw that render away, an in-flight preview pick could still call its onSend, for example one that edits a queued message instead of sending a turn. Update the sender in a layout effect so only committed renders lend it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
PersistentThreadTerminalPanel, PersistentThreadTerminalDrawer, their two reconciliation helpers and the terminal launch-context types now live in apps/web/src/panels/terminal. The moved code is unchanged apart from the added export keywords; ChatView imports them and its call sites, props, memo boundaries and callbacks are untouched. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The right-panel terminal is now a registered side panel. Its body reads the thread and visibility from the panel host and keybindings from the server keybindings atom, then hands them to the unchanged memoized terminal, so ChatView renders that leave its inputs alone still skip it. ChatView passes only the terminal surface, launch context, focus request, callbacks and shortcut labels. Launcher copy, letter, order and availability are unchanged; the bottom drawer stays mounted by ChatView. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tree A terminal launch context with a null worktree path means the terminal was launched on the local checkout. The right-panel terminal treated that null as missing and fell back to the thread's worktree, so a thread that gained a worktree after the launch gave the drawer a worktree path and runtime env that did not match its cwd. Use the launch context whenever one exists, as the persistent drawer already does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…hread's worktree A terminal summary's null worktree path means the server opened the terminal on the project checkout. Without a launch context the right-panel terminal treated that null as missing and fell back to the thread's worktree, so a thread that gained a worktree later gave the drawer a checkout cwd with a worktree path and runtime env. Fall back to the thread's worktree only when there is neither a launch context nor a summary. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
saphid
force-pushed
the
stack/03-terminal-panel
branch
from
October 12, 2026 00:11
e34c8a4 to
ca03345
Compare
This PR now carries only the checkout-terminal worktree fix. The panel-host layers it was stacked on (pingdotgg#15010, pingdotgg#16038 and the terminal panel move) wait on Ideas discussion #14938; the fix does not need them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s worktree A null worktree path on a terminal launch context or server summary means the terminal runs on the project checkout. The right-panel terminal read both with `??`, so it treated that null as missing and fell back to the thread's worktree. Once a thread gained a worktree (first worktree-mode send, or switching the thread onto a worktree), a terminal still running on the checkout got the worktree as its worktreePath and T3CODE_WORKTREE_PATH, and the server tracked and re-spawned it with that mismatched context whenever it reopened the session. Use the launch context whenever there is one, then the summary, and fall back to the thread's worktree only when neither exists, as the bottom drawer already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mobile terminal screen resolves its location from the session summary, where a null worktree path means the terminal runs on the project checkout. resolveTerminalOpenLocation read that field with `??`, so once the thread had a worktree it resolved a checkout terminal to the checkout cwd with the thread's worktree as its worktreePath. Attaching to a running session ignores the location, but when the server has to open a new session for it, that session records the wrong worktree and takes its workspace lease on the worktree instead of the checkout. Use the session's location whenever there is a session, and fall back to the thread's worktree only when there is none, matching the web fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
saphid
changed the title
github-actions
Bot
added
size:M
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…inal off the thread's worktree Preserve the scoped contribution against upstream 70d60e0. Focused checks and read-only integration review recorded in the PR. GPT-6.1 Sol (high), Codex harness through T3 Code.
macroscopeapp
Bot
dismissed
their stale review
October 17, 2026 03:04
Dismissing prior approval to re-evaluate 4d3a85e
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Model: Claude Opus 5.5. Narrowed to the checkout-terminal fix by Claude Opus 5.5 on 2026-10-14.
Problem
A terminal can run on the project checkout while its thread uses a worktree. Two common ways:
worktreePath: null.worktreePath: null.nullhere means "the project checkout". Then the thread gains a worktree, either from its first send in worktree mode or by being moved onto one.On
main, the web right-panel terminal (PersistentThreadTerminalPanelinPersistentThreadTerminals.tsx) resolveslaunchContext?.worktreePath ?? summary?.worktreePath ?? threadWorktreePath, at two sites. The??treats the authoritativenullas missing and falls back to the thread's worktree. The terminal keeps its checkout cwd, but it is described to the server with the thread's worktree:projectScriptRuntimeEnv(packages/shared/src/projectScripts.ts) setsT3CODE_WORKTREE_PATHto the unrelated worktree.The bottom terminal drawer already checks
launchContext !== nulland is not affected. Mobile has the same??fallback inresolveTerminalOpenLocation(apps/mobile/src/features/terminal/terminalLaunchContext.ts). Attaching to a running session ignores the location, so mobile only goes wrong when the server has to open a new session for that terminal.Why this qualifies
A small, focused fix for an obvious bug: a terminal on the checkout is told it is in a worktree. It changes no contract;
TerminalSummary.worktreePathis already a required nullable field wherenullmeans the checkout. It includes each client that has the same defect.Fix
resolveTerminalWorktreePathinChatView.logic.tsuses the launch context if there is one, then the summary, and only then the thread's worktree. Both right-panel call sites inPersistentThreadTerminals.tsxuse it.resolveTerminalOpenLocationuses the session's location as a whole whenever a session exists, and the thread's worktree only when there is none. The cwd result is unchanged.Verification
At 4d3a85ec8c, integrated with upstream 70d60e03a7 on macOS, 2026-10-17:
vp test run apps/mobile/src/features/terminal/terminalLaunchContext.test.ts apps/web/src/components/ChatView.logic.test.ts: 144 passed (144).apps/mobile,apps/web.Checkrun was not discovered within the one-minute window.Review rate limited, so no fresh CodeRabbit review is claimed.Earlier verification
Run on Linux (lxso2):
vp test run apps/web/src/components/ChatView.logic.test.ts -t resolveTerminalWorktreePath: 3 new cases pass. With the resolver set tomain's??expression, 2 fail (expected '/repo/.worktrees/feature' to be null).vp test run apps/mobile/src/features/terminal/terminalLaunchContext.test.ts: 7/7 pass. Onmain's file, the new case fails (worktreePathwas"/repo/worktrees/feature", expectednull).ChatView.logic.test.ts,ThreadTerminalDrawer.test.ts,ThreadTerminalDrawer.permissions.test.tsxand the mobile test: all pass.tsc --noEmitforapps/webandapps/mobile, plusvp lintandvp fmt --checkon the changed files: pass. Lint has no warnings on changed lines.No screenshots: nothing changes visually. The fix changes the worktree path and environment a checkout terminal is opened with.
Not verified
Surfaces
Claude Opus 5.5 (build) and GPT-6.1 Sol (review) via T3 Code
🤖 Generated with Claude Code
Integration and publication: GPT-6.1 Sol (high), Codex harness through T3 Code.