Repository navigation
Conversation
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This change replaces hand-wired production panel mounting with new shared registry and host infrastructure, while migrating several existing panels and their lifecycle/context behavior. Its broad runtime surface and substantial cross-cutting refactor warrant human review despite the accompanying regression tests. You can add or adjust custom eligibility rules. Learn more. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This refactor routes the Preview, Diff, Terminal and Device side panels through a shared registry and host. It is intended to keep the same visible behavior. The review found no concrete defects. Tests cover panel loading, launcher behavior and thread changes during asynchronous actions. It appears ready to merge after normal checks.
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 24 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Description check | |
The description provides detailed problem, change, scope, verification, evidence, and limitations. However, it explicitly states that the proposed direction has no maintainer approval, while the templ… | Obtain explicit maintainer approval for the panel-host scope extension and document the approval comment in the description. Rename or align the Fix and Evidence sections with the template's Change and Verification sections if required by r… |
✅ Passed checks (3 passed)
-
-
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. Title check The title clearly identifies the main change: opening the Device side panel through the panel host.
-
Full details: Description check
-
-
Explanation
The description provides detailed problem, change, scope, verification, evidence, and limitations. However, it explicitly states that the proposed direction has no maintainer approval, while the template requires explicit approval for this scope extension.
Resolution
Obtain explicit maintainer approval for the panel-host scope extension and document the approval comment in the description. Rename or align the Fix and Evidence sections with the template's Change and Verification sections if required by repository conventions.
-
✨ 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/04-device-panel
branch
4 times, most recently
from
October 6, 2026 16:03
4a7bd6d to
d1cecb7
Compare
Contributor
Author
|
Review requested
Logged so this PR shows when a maintainer was asked to review it. |
saphid
force-pushed
the
stack/04-device-panel
branch
5 times, most recently
from
October 10, 2026 02:13
8a43f85 to
01cfd65
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>
One Device panel instance stays mounted across a thread switch, so a device start or power-off that settled after the switch left its "Starting device…" spinner or its error in the next thread. The panel now resets its operation state when its thread changes, and a pick or power-off that settles after it moved to another thread (even back again) or unmounted no longer touches the panel's spinner or error. The tab it opens or closes still lands in the thread it started from, which is where the server opened or shut down the device. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Register Device in the bundled panel registry like Browser, Diff and Terminal. The body moves to panels/device and reads its thread and visibility from the panel host; the launcher row, tab title and icon read the definition, so the copy, letter and order are unchanged. The Device tests now mount it through the registered lazy path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
saphid
force-pushed
the
stack/04-device-panel
branch
from
October 10, 2026 04:05
01cfd65 to
2bc92f1
Compare
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.
Stacked on #16040 (and #15010). Review only the top commit: 2bc92f1.
Problem
The Device panel (live iOS Simulator / Android Emulator) is mounted by hand in
ChatViewwith its own lazy import andSuspense, and the + menu and empty launcher keep a hand-written Device row with its own letter, icon and copy, separate from the registered Browser, Diff and Terminal panels. This PR opens Device as a registered right panel on the panel host. There is no visible change.Why this qualifies
This continues the panel host proposed in Ideas discussion #14938; moving Device onto it is a scope extension of that proposal. No maintainer has agreed to that direction yet. It is stacked on the Device thread-switch fix PR (which stands on its own) and on the terminal panel PR (and through it on the Preview panel PR and #15010), and depends on them; review those first. If #14938 is declined, close this too. Previous PR in this stack: fix(web): keep device results in the thread that started them (#16040).
Fix
One commit (+110/−97, 10 files; the Device body and its test are git renames).
panels/bundledPanels.tsxregistersdevice(title "Device",Smartphone, letter M, "Available from a thread." / "Devices are only available from a thread.") with a lazyload.components/device/DevicePanel.tsxmoves topanels/device/DeviceSidePanel.tsxas the default export. It reads the thread and visibility from the panel host; the always-"embedded"modeprop and thethreadRef/visibleprops are gone. The body is otherwise unchanged.ChatViewmounts<RegisteredSidePanel id="device" …/>with the samekeyand the same close-then-show setup dismissal. The setup dialog, auto-float and mini-player effects are untouched.RightPanelTabs: the hand-written Device row becomesregistered("device")in the same slot (B T F D P L M); the tab fallback title and icon read the definition (Apple/Android icons per platform unchanged). The unuseddescriptionfield on launcher rows is removed (only Device set it and nothing rendered it). The pull-requests page lists Device as unavailable, as before.ChatViewdoes, through the registry's lazy load on the panel host.Evidence
Intended to look the same, so the same steps were captured before and after. Before = the Device thread-switch fix PR head
7a5afb4555, after =9b3ab22380. macOS 26.5.2, fresh isolated state (Thread A and Thread B), a dedicated Android emulator (API 36). Web: Playwright Chromium, 1440×1000, theme byprefers-color-scheme. Recordings are real time.Start, stream, input and screenshot
Remote browser over
vp run dev --share(after): a new, unpaired browser pairs with this server's own link. It then opens Device, streams the emulator, and taps the Phone icon, which opens Phone.All of these matched on both revisions:
/pull-requests, with "Devices are only available from a thread.";With the live stream area masked, 33 of 36 web right-panel still pairs are pixel-identical. The other 3 were captured mid-transition: a dialog fade, a hover highlight, and a device list still loading.
Stills (web, light and dark)
/pull-requests, Device unavailable (light)Built Electron app. Both revisions were built with
vp run build:desktopand launched against an isolated profile: a throwaway HOME, Chromium--use-mock-keychain. Theme was set withnativeTheme. The same steps were run as on web, and B1 to B9 matched. With the live stream area masked, 26 of 36 Electron right-panel still pairs are pixel-identical. The other 10 come from capture timing: Android version text that loaded later, device controls discovered later, and a different hover target. None of them is a layout change.Stills (Electron, light and dark)
Checks at this head (
9b3ab22380), re-run 2026-10-05 (CI=true, all exit 0):vp test run src/panels src/components/RightPanelTabs.test.tsx src/components/RightPanelTabs.terminal.test.tsx src/components/RightPanelTabs.browserProfile.test.tsx src/components/device(apps/web): 14 files, 57 tests pass. The 7 Device cases mount the real panel through the registered lazy path and check the device list, opening a device in its own thread, closing it on power-off, and the thread-switch cases from the fix PR below. Recorded during development, with this PR's source reverted to its parent, the suite loads and all 7 fail withUnknown panel id: device; the other 50 pass. The same cases against the old component pass on the parent, as expected for a move.vp run --filter @t3tools/web typecheckpasses; compile-only fixtures (Device without its surface or setup dismissal, Device props on Preview, Preview props on Device, host-ownedvisiblepassed as a prop) and the launcher fixtures produce 11 type errors against the parent.vp lint --report-unused-disable-directivesandvp fmt --checkon the touched files: pass; lint warnings equal the parent's.vp run knip:checkpasses.vp run --filter @t3tools/web buildpassed on the previous revision of this change with the same panel body; the Device body is its own 42 kB lazy chunk, referenced only by the launcher. The web build,vp run build:desktopandnode scripts/release-smoke.tspassed at the top of this stack (the example plugins PR), which contains this change.Surfaces
device_open) and the mini-player's return-to-panel are unchanged. There is no command palette entry or keybinding for Device today; this PR does not add one. The pull-requests page still shows Device unavailable.apps/web.--shareremote-browser pass above opened, streamed and tapped the device.Not verified
/pull-requestscheck was run on a route with no project, where the whole right panel is disabled on both revisions. The Device-unavailable tooltip is shown on web only.--shareover the tailnet).vp run build:desktoppassed at this head and at its parent; it built the Electron app used for the captures above.Claude Opus 5.5 (build), GPT-6.1 Sol (review) and GPT-6 Astra (captures) via T3 Code
🤖 Generated with Claude Code