[Feature] Polish right panel tabs (Terminal, Review, Status)#71
Merged
Conversation
Enforce min-one-terminal UI invariant: - Thread totalCount from terminal-panel to SortableTerminalTab - Hide × close button when totalCount === 1 - Hide context-menu "Close" item when totalCount === 1 Panel auto-close on PTY exit (via existing removeExited flow) is unchanged; only the explicit × path is gated. Ghostty-web font tweak deferred — no font rendering issue observed within scope. Closes part of #52.
Delete the vertical file-tree sub-panel that squeezed the diff viewer inside the Review tab (session-side-panel.tsx:482-561). The diff now fills the full Review pane. Workspace-tree navigation inside Review is removed by design — the Files tab handles artifact previews and the diff viewer's own per-file sticky headers cover navigation to each changed file. Switch the default diff style from "split" to "unified". The right panel minimum width (360px) cannot render split view legibly; users who want split can still toggle it via the existing control. Trim now-unused props (diffsReady, empty, activeDiff, focusReviewDiff), memos (treeWidth, diffFiles, kinds, nofiles), helpers (fileTreeTab, setFileTreeTabValue, focusReviewDiff), and the FileTree import. Closes part of #52.
Prerequisites for the Status side-panel redesign (#52). Nothing user-visible yet — adds pure logic + translation strings that the next commits wire into UI. - session-status-extractors.ts / .test.ts: two pure functions over a pre-flattened Part[]: extractTodos(parts): TodoItem[] — latest completed todowrite's todos, malformed items filtered. extractSources(parts): string[] — URLs from webfetch input.url and websearch output (URL regex tolerates balanced parens), deduped by scheme+host case-folding, capped at 20, first-seen order. Plus a string-presence sanity test that reads the canonical tool files in packages/opencode/src/tool/ to catch upstream rename drift. - i18n: new keys under status.summary.* (progress, sources + empty fallbacks) and status.connections.* (title, empty, state.{disabled, failed,needs_auth,needs_client_registration}) added to en / zh / zht with parity test green.
Three Solid components for the Status side-panel redesign (#52). Not yet mounted; next commit swaps them in at the Status tab. - SessionStatusSummary: Progress (todos from extractTodos with status-colored dots; cancelled gets strikethrough + muted text) and Sources (URLs from extractSources) with empty-state fallbacks. - SessionStatusConnections: Servers / MCP / LSP / Plugins as collapsible sections. Green reserved for confirmed live (anyHealthy / anyConnected); warn on failed / needs_auth / needs_client_registration / healthy=false / status=error; grey otherwise. Auto-expand fires only on transition into warn so a user's manual collapse sticks. Empty categories show a "None configured." hint. Row labels use min-w-0 truncation to ellipsize long identifiers inside the flex row. - Health polling has an in-flight guard against overlapping refreshes, per-probe try/catch so one failing server doesn't abort the batch, and an exception fallback to { healthy: false } so thrown checker bugs surface as red rather than silently-grey. - SessionStatusPanel: composes Summary + Connections; flattens the session's messages into Part[] via sync.data keyed by params.id so extractors stay as pure functions.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 14 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRedesigns the Status tab into grouped sections with periodic health polling, adds utilities and tests to extract todos and sources from session parts, hides terminal-close controls when only one terminal exists, removes the file-tree review explorer from the side panel, adds i18n keys, and changes default diff view to "unified". Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SessionStatusConnections
participant Sync as Sync Store
participant Server as ServerHealthProbe
participant Dialog as Manage Servers Dialog
Note over UI,Server: Poll loop (every 10s) when shown=true
UI->>UI: if shown && !inFlight -> start poll (inFlight=true)
loop for each configured server
UI->>Server: probe(serverConfig)
alt probe succeeds
Server-->>UI: { healthy: true/false }
else probe fails
Server-->>UI: { healthy: false }
end
UI->>Sync: update health[serverKey] = result
end
UI->>UI: compute aggregate categories ("ok"/"warn"/"empty")
UI->>UI: set inFlight=false
UI->>UI: expand section if transitioned to "warn"
UI->>Dialog: (on manage servers click) dynamic import -> open dialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a session status panel to display progress, sources, and connection statuses for servers, MCP, LSP, and plugins. It includes new extraction utilities, UI components, and updated translations. Additionally, terminal tab closing is now conditional on tab count, and the default review diff style is set to 'unified'. Feedback highlights a potential runtime error when accessing configuration data and suggests a performance optimization for URL extraction from large tool outputs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/app/src/context/layout.tsx (1)
794-794: Consider centralizing the review diff default in one constant.Behavior is correct, but the
"unified"default is duplicated across initialization, fallback, and lazy creation paths. A shared constant would reduce drift risk.♻️ Suggested cleanup
export type ReviewDiffStyle = "unified" | "split" +const DEFAULT_REVIEW_DIFF_STYLE: ReviewDiffStyle = "unified" // ... review: { - diffStyle: "unified" as ReviewDiffStyle, + diffStyle: DEFAULT_REVIEW_DIFF_STYLE, panelOpened: false, }, // ... review: { - diffStyle: createMemo(() => store.review?.diffStyle ?? "unified"), + diffStyle: createMemo(() => store.review?.diffStyle ?? DEFAULT_REVIEW_DIFF_STYLE), // ... function setReviewPanelOpened(next: boolean) { const current = store.review if (!current) { - setStore("review", { diffStyle: "unified" as ReviewDiffStyle, panelOpened: next }) + setStore("review", { diffStyle: DEFAULT_REVIEW_DIFF_STYLE, panelOpened: next }) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/src/context/layout.tsx` at line 794, Default review diff style "unified" is duplicated; define a single constant (e.g. DEFAULT_REVIEW_DIFF_STYLE) of type ReviewDiffStyle and replace hardcoded "unified" occurrences used in initialization, fallback, and lazy creation (including the call that uses setStore("review", { diffStyle: "unified" as ReviewDiffStyle, panelOpened: next })) so all codepaths reference the constant and avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/components/session/session-status-connections.tsx`:
- Around line 137-138: The plugins initializer currently dereferences
sync.data.config.plugin directly which can throw if config is undefined; update
the createMemo for the plugins constant to safely access the nested property
like the mcp and lsp initializers by using optional chaining and a default empty
array (e.g., access sync.data.config?.plugin ?? []) before mapping, so change
the createMemo that defines plugins to use sync.data.config?.plugin ?? [] and
preserve the existing mapping transform (also reference the createMemo and
plugins identifiers when making this change).
In `@packages/app/src/pages/session/session-side-panel.tsx`:
- Around line 271-273: The Status panel is still polling because
SessionStatusPanel is passed shown={open} regardless of which tab is active;
change it so polling is enabled only when the side panel is open AND the Status
tab is the active tab. Replace shown={open} on the SessionStatusPanel rendered
inside Tabs.Content value="status" with a boolean that checks both open and the
current Tabs value (e.g., shown={open && activeTab === "status"}), or read the
Tabs' active value/context and pass shown accordingly so
SessionStatusConnections only polls when the Status tab is visible.
---
Nitpick comments:
In `@packages/app/src/context/layout.tsx`:
- Line 794: Default review diff style "unified" is duplicated; define a single
constant (e.g. DEFAULT_REVIEW_DIFF_STYLE) of type ReviewDiffStyle and replace
hardcoded "unified" occurrences used in initialization, fallback, and lazy
creation (including the call that uses setStore("review", { diffStyle: "unified"
as ReviewDiffStyle, panelOpened: next })) so all codepaths reference the
constant and avoid drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51dc2b24-c25a-4e71-86e4-feb1aebd5f64
📒 Files selected for processing (14)
packages/app/src/components/session/session-sortable-terminal-tab.tsxpackages/app/src/components/session/session-status-connections.tsxpackages/app/src/components/session/session-status-panel.tsxpackages/app/src/components/session/session-status-summary.tsxpackages/app/src/context/layout.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/i18n/zht.tspackages/app/src/pages/session.tsxpackages/app/src/pages/session/session-side-panel.test.tsxpackages/app/src/pages/session/session-side-panel.tsxpackages/app/src/pages/session/session-status-extractors.test.tspackages/app/src/pages/session/session-status-extractors.tspackages/app/src/pages/session/terminal-panel.tsx
💤 Files with no reviewable changes (1)
- packages/app/src/pages/session.tsx
Two CI-surfaced regressions from the right-panel polish PR: - Status side panel: add a Manage servers button at the bottom of SessionStatusConnections. The old titlebar-popover-style StatusPanel had this affordance inside its Servers tab footer; my swap to SessionStatusPanel dropped it. Restored via a secondary button under the four collapsible category sections, lazy-loading DialogSelectServer via dynamic import like the popover does. - e2e/files/file-tree.spec.ts: the previous test asserted the persistent #file-tree-panel inside the Review tab. That pane was removed by design in this PR. Rewrote the spec to guard the inverse invariant (pane must NOT render, Review tab content area must still mount), matching the new layout.
CodeRabbit review: when the right panel is open but the user is on the Files / Review / Terminal tab, SessionStatusConnections still polls checkServerHealth every 10 seconds, wasting work on a tab nobody is looking at. Narrow the shown gate to require both open AND sidePanelTab() === "status". The reconcile-to-empty branch in the polling effect already handles the transition, so polling stops as soon as the user switches away from Status.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/e2e/files/file-tree.spec.ts`:
- Around line 33-34: The test uses rightPanel.getByRole("tabpanel").first()
(assigning to reviewContent) which relies on DOM order; instead, find the Review
tab element (e.g., the tab with name/text "Review" or the tab element used in
the test) and read its aria-controls attribute, then use that id to query the
specific tabpanel (document.getElementById or within rightPanel) to assert
visibility; update the code paths around rightPanel and reviewContent to derive
the panel via aria-controls rather than using .first().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bebe998d-f488-41ef-a99a-3778018acba2
📒 Files selected for processing (3)
packages/app/e2e/files/file-tree.spec.tspackages/app/src/components/session/session-status-connections.tsxpackages/app/src/pages/session/session-side-panel.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/app/src/components/session/session-status-connections.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app/src/pages/session/session-side-panel.tsx
Two CI-surfaced follow-ups: - packages/opencode/test/config/e2e-smoke-tagging.test.ts: Update the expected @smoke inventory entry for file-tree.spec.ts to match the new test title ("review tab no longer renders the legacy file-tree sub-panel"). The old title ("review keeps the persistent file-tree pane...") described behavior that was removed by design in this PR. - packages/app/e2e/files/file-tree.spec.ts: CodeRabbit review: session-side-panel has nested Tabs with two Tabs.Content value="review" elements (outer shell + inner review tabs), so .getByRole("tabpanel").first() is order-coupled. Locate the panel via the tab's aria-controls attribute instead.
This was referenced Apr 21, 2026
This was referenced Apr 28, 2026
This was referenced May 4, 2026
This was referenced May 13, 2026
This was referenced May 15, 2026
This was referenced May 24, 2026
8 tasks
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.
Summary
Closes #52. Five commits on
devthat polish the three tabs inside the session right panel:7d25a6e82— Terminal: hide the×button (and context-menu Close) on the lone terminal tab so the panel enforces its min-one invariant. ThreadedtotalCountfromterminal-panel.tsxthroughSortableTerminalTab.30e369e2e— Review: delete the vertical file-tree sub-panel that squeezed the diff viewer; the diff now fills the full Review pane. Flip the default diff style fromsplittounified(the right panel's 360px minimum cannot render split legibly; toggle still available). Trims now-dead props, memos, and helpers.5d2cd891b— Status (prereqs): pureextractTodos/extractSourcesmodule (reads a pre-flattenedPart[], dedupes URLs by scheme+host case-fold, caps at 20) plus i18n keys understatus.summary.*/status.connections.*inen/zh/zht.6e9421069— Status (components):SessionStatusSummary(Progress + Sources with empty-state fallbacks, strikethrough for cancelled todos),SessionStatusConnections(Servers / MCP / LSP / Plugins as collapsible sections; green reserved for confirmed-live; auto-expand transition-only into warn; polling with in-flight guard + per-probe try/catch; thrown probes surface as unhealthy, not unprobed),SessionStatusPanelcomposer.093264bfd— Status (mount): swap the Status tab body fromStatusPaneltoSessionStatusPanel. The titlebar Status popover is unchanged; both surfaces independently subscribe to the same sync signals.No
packages/ui/**files touched (upstream carveout respected).Why
Closes the three items in #52. The Review tab was eating horizontal width for a workspace-tree navigator that duplicated the already-horizontal Files tab; the Status tab only showed infrastructure, missing session-level narrative (progress + sources) that Codex surfaces well; the Terminal tab let users close the last remaining pane, leaving the tab strip empty.
Two follow-up issues filed from the smoke round:
Went through four rounds of
/crosscheck(Claude + Codex). Agreed findings fixed in-place during the polish cycle; disagreed items documented in commit messages with verified reasoning.Related Issue
Closes #52. Follow-ups: #65, #66.
How To Verify
Manual smoke in
bun run dev:desktop:×hidden on lone tab; click+→ both tabs show×; close one → remaining tab's×disappears;exitin shell → panel auto-dismisses.Screenshots or Recordings
Manual smoke walkthrough done during the polish cycle. No screenshots attached here — all visual changes are structural (file-tree panel removed, Status panel shape changed) and covered by the How To Verify steps above.
Checklist
devbranchSummary by CodeRabbit
New Features
Changes
Localization