Repository navigation
Add Dock sidebar TUI controls - #3217
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a right‑sidebar "Dock" UI and runtime with config discovery (project → global → builtin) and trust gating, rebrands the sidebar from Feed→Dock, separates dock terminal focus routing, adds Changes
Sequence DiagramsequenceDiagram
participant User as User
participant RightSidebar as RightSidebar
participant DockPanel as DockPanel
participant ConfigLoader as ConfigLoader
participant FileSystem as FileSystem
participant TerminalPanel as TerminalPanel
participant FocusCoordinator as FocusCoordinator
User->>RightSidebar: Switch to Dock mode
RightSidebar->>DockPanel: init(rootDirectory)
DockPanel->>ConfigLoader: loadConfig()
ConfigLoader->>FileSystem: check project .cmux/dock.json
alt project config found
FileSystem-->>ConfigLoader: return project config
else
ConfigLoader->>FileSystem: check ~/.config/cmux/dock.json
alt global config found
FileSystem-->>ConfigLoader: return global config
else
ConfigLoader-->>DockPanel: use built-in control
end
end
ConfigLoader->>DockPanel: parsed controls + fingerprint
alt untrusted project config
DockPanel->>User: show trust prompt
User-->>DockPanel: accept/reject
end
DockPanel->>TerminalPanel: create panels (focusPlacement=rightSidebarDock)
TerminalPanel->>FocusCoordinator: register & request focus (dock-aware)
User->>DockPanel: keyboard action (navigate/restart/open/reload)
DockPanel->>TerminalPanel: execute control / route focus
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR introduces the Dock right-sidebar panel: a configurable list of TUI controls (project/global Two issues need attention before merge:
Confidence Score: 3/5Not safe to merge as-is due to a snapshot boundary violation that can cause a CPU spin loop and a TUI input bug that drops keypresses. Two P1 findings: the @ObservedObject-inside-ForEach pattern is explicitly prohibited by the team's CLAUDE.md rule (risk of 100% CPU spin loop identical to issue #2586), and the ESC key blocking causes silent keypress loss in the new TUI. Both affect core paths introduced by this PR. Sources/DockPanelView.swift (snapshot boundary) and CLI/cmux.swift (ESC key handling, CJK truncation) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant DockPanelView
participant DockControlsStore
participant DockControlRuntime
participant TerminalPanel
participant SocketClient
DockPanelView->>DockControlsStore: reload(rootDirectory)
DockControlsStore->>DockControlsStore: resolve() — project / global / built-in
alt project config and untrusted
DockControlsStore-->>DockPanelView: trustRequest set → show DockTrustView
User->>DockPanelView: Trust and Start
DockPanelView->>DockControlsStore: trustAndReload()
end
DockControlsStore->>DockControlRuntime: init(definition, baseDirectory)
DockControlRuntime->>TerminalPanel: makePanel(definition) focusPlacement=.rightSidebarDock
DockPanelView->>DockPanelView: render DockControlSectionView per control
User->>DockPanelView: click Focus button
DockPanelView->>DockControlRuntime: focus()
DockControlRuntime->>TerminalPanel: ensureFocus(...)
User->>SocketClient: cmux feed tui
SocketClient->>SocketClient: feed.list(pending_only: false)
SocketClient-->>User: render TUI items sorted pending-first
User->>SocketClient: keypress e.g. Enter / d / f
SocketClient->>SocketClient: feed.permission.reply / feed.exit_plan.reply / feed.question.reply
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce0ce26956
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| private struct DockControlSectionView: View { | ||
| @ObservedObject var runtime: DockControlRuntime |
There was a problem hiding this comment.
The project guideline in /workspace/cmux/AGENTS.md forbids holding ObservableObject references below ForEach row boundaries because it has previously caused full-row invalidation loops and high CPU in sidebar panels. DockControlSectionView keeps DockControlRuntime as @ObservedObject under a ForEach, reintroducing that exact invalidation risk instead of the required immutable snapshot + action closure pattern.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
docs/feed.md (1)
3-3: Consider tightening wording to avoid “inline” vs “Dock” ambiguity.The current sentence reads slightly conflicting with the new Dock-first model.
Doc wording tweak
-Feed is cmux's inline surface for AI agent decisions. The keyboard-first Feed TUI can run in the right-sidebar [Dock](dock.md) with `cmux feed tui`. It shows three things that need a human response: +Feed is cmux’s AI-agent decision surface. The keyboard-first Feed TUI runs in the right-sidebar [Dock](dock.md) via `cmux feed tui`. It shows three things that need a human response:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/feed.md` at line 3, The wording "Feed is cmux's inline surface for AI agent decisions" conflicts with the Dock-first model—locate the sentence containing "Feed is cmux's inline surface for AI agent decisions" and the following mention of "cmux feed tui" and rephrase to remove "inline" ambiguity (for example: "Feed is cmux's surface for AI agent decisions; the keyboard-first Feed TUI can run in the right‑sidebar Dock via `cmux feed tui`"), keeping the same meaning but clarifying that Feed is a UI surface that can run inline or in the Dock.Sources/MainWindowFocusController.swift (1)
354-359: Extract duplicated feed/dock host focus logic into one helper.The same feed/dock focusing sequence is implemented twice (Line 354 and Line 553 paths). Pulling it into a private helper will reduce drift risk.
♻️ Suggested refactor
@@ case .feed: - if focusFirstItem { - feedHost?.focusFirstItemFromCoordinator() - dockHost?.focusFirstItemFromCoordinator() - } - modeResult = feedHost?.focusHostFromCoordinator() == true || - dockHost?.focusHostFromCoordinator() == true + modeResult = focusFeedLikeHostsFromCoordinator(focusFirstItem: focusFirstItem) @@ case .feed: - feedHost?.focusFirstItemFromCoordinator() - dockHost?.focusFirstItemFromCoordinator() - result = feedHost?.focusHostFromCoordinator() == true || - dockHost?.focusHostFromCoordinator() == true + result = focusFeedLikeHostsFromCoordinator(focusFirstItem: true) } @@ + private func focusFeedLikeHostsFromCoordinator(focusFirstItem: Bool) -> Bool { + if focusFirstItem { + feedHost?.focusFirstItemFromCoordinator() + dockHost?.focusFirstItemFromCoordinator() + } + return feedHost?.focusHostFromCoordinator() == true || + dockHost?.focusHostFromCoordinator() == true + }Also applies to: 554-557
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/MainWindowFocusController.swift` around lines 354 - 359, Extract the duplicated feed/dock focusing sequence into a private helper method (e.g. a method on MainWindowFocusController like applyFocusSequence(focusFirstItem: Bool) -> Bool) that performs the conditional calls to feedHost?.focusFirstItemFromCoordinator(), dockHost?.focusFirstItemFromCoordinator() when focusFirstItem is true, then returns the combined result of feedHost?.focusHostFromCoordinator() == true || dockHost?.focusHostFromCoordinator() == true; replace the duplicated blocks that set modeResult with calls to this new helper in both locations.Sources/DockPanelView.swift (2)
391-406: Consider logging encoding failures in trust descriptor generation.Line 394 silently falls back to empty
Data()if encoding fails. While unlikely for validDockControlDefinitionarrays, a silent failure here could cause unexpected trust behavior. Consider logging the error in DEBUG builds.🛠️ Suggested improvement
private static func trustDescriptor(for resolution: DockConfigResolution) -> CmuxActionTrustDescriptor { let encoder = JSONEncoder() encoder.outputFormatting = [.sortedKeys] - let data = (try? encoder.encode(DockConfigFile(controls: resolution.controls))) ?? Data() + let data: Data + do { + data = try encoder.encode(DockConfigFile(controls: resolution.controls)) + } catch { + `#if` DEBUG + cmuxDebugLog("DockControlsStore: trust descriptor encoding failed: \(error)") + `#endif` + data = Data() + } let commandFingerprint = String(data: data, encoding: .utf8) ?? ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/DockPanelView.swift` around lines 391 - 406, The JSON encoding in trustDescriptor(for resolution: DockConfigResolution) currently swallows errors by using (try? encoder.encode(...)) ?? Data(), so update this function to capture the thrown error when encoding DockConfigFile(controls: resolution.controls) fails and log it in debug builds (e.g., using `#if` DEBUG) before falling back; specifically, replace the try? pattern with do/catch around encoder.encode, log the caught error along with context (resolution.sourceURL or resolution.baseDirectory) via your existing logging facility, and then continue to use an empty Data() as the fallback so behavior remains unchanged in release builds.
362-369: Consider documenting the file-to-parent fallback behavior.When
rawPathpoints to an existing file (not directory), this returns the parent directory. This is a reasonable convenience, but the implicit behavior could surprise callers who expect a file path to fail validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/DockPanelView.swift` around lines 362 - 369, Add a clear doc comment above the private static func existingDirectory(_ rawPath: String) explaining its fallback behavior: that it expands tildes, checks existence, returns nil when path doesn't exist, returns the directory if rawPath is itself a directory, and — importantly — when rawPath points to an existing file it returns the file's parent directory (not nil). Reference the function name existingDirectory and parameter rawPath in the comment so callers understand this implicit file-to-parent fallback and expected return semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 17560-17588: renderFeedTUI currently always displays
items.prefix(bodyRows) so when selectedIndex exceeds bodyRows the highlighted
selection can scroll off-screen; change rendering to compute a sliding window
(start..end) of items centered on selectedIndex (clamped to 0..max(0,
items.count-bodyRows)) and iterate over items[start..<min(items.count,
start+bodyRows]) instead of items.prefix(bodyRows), updating the displayed index
math so the selected row maps to the correct printed line; ensure
feedTUIItem(in:at:) and truncateForTerminal continue to receive the actual
selected item and statusLine uses selectedHelp correctly for the visible
selection.
- Around line 17371-17421: The parser drops the multi-select flag so
multi-select questions are handled as single-choice; update FeedTUIItem and
FeedTUIOption to include a multiSelect/allowMultiple Bool (or similar) and
populate it in FeedTUIItem.parse from the incoming payload (e.g., the question
payload key that indicates multi-select), ensure questionOptions preserves that
flag, then update the TUI resolver logic that submits answers (the resolver that
reads FeedTUIItem.questionOptions and sends request ids) to either (A) accept
and send an array of selected ids for multiSelect questions or (B) block/disable
resolution for items where multiSelect == true until UI selection for multiple
choices is implemented—make the change in the parse(_:), FeedTUIItem,
FeedTUIOption, and the resolver/submit path that currently sends a single option
id.
In `@Sources/Feed/FeedCoordinator.swift`:
- Around line 546-561: The .question branch in FeedCoordinator.swift currently
only serializes the first question (case .question, variables requestId and
questions) which drops questions[1...]; update this block to also produce a
backward-compatible "questions" array in dict by mapping every question in
questions into a dictionary (including prompt, multiSelect and options) while
keeping the existing top-level "question_prompt", "question_multi_select", and
"question_options" fields derived from firstQuestion for compatibility; ensure
each option mapping (option.id, option.label, optional option.description) is
reused for all questions so socket consumers receive the full questions payload.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 11181-11209: The right-sidebar dock fast-path in the if block that
checks surfaceView.terminalSurface and focusPlacement == .rightSidebarDock
returns early and bypasses the shared focus guards (e.g.,
respectForeignFirstResponder and the search-focus restoration path); update this
block in ensureFocus so that before returning you invoke the same shared-guard
logic used by the non-dock path: call the existing respectForeignFirstResponder
check (and act on its result) and run the search-focus restoration/ownership
checks (the code paths that call reassertTerminalSurfaceFocus and
isSurfaceViewFirstResponder) instead of an unconditional return, or move the
return to after those checks so docked terminals respect foreign first
responders and search-focus state.
---
Nitpick comments:
In `@docs/feed.md`:
- Line 3: The wording "Feed is cmux's inline surface for AI agent decisions"
conflicts with the Dock-first model—locate the sentence containing "Feed is
cmux's inline surface for AI agent decisions" and the following mention of "cmux
feed tui" and rephrase to remove "inline" ambiguity (for example: "Feed is
cmux's surface for AI agent decisions; the keyboard-first Feed TUI can run in
the right‑sidebar Dock via `cmux feed tui`"), keeping the same meaning but
clarifying that Feed is a UI surface that can run inline or in the Dock.
In `@Sources/DockPanelView.swift`:
- Around line 391-406: The JSON encoding in trustDescriptor(for resolution:
DockConfigResolution) currently swallows errors by using (try?
encoder.encode(...)) ?? Data(), so update this function to capture the thrown
error when encoding DockConfigFile(controls: resolution.controls) fails and log
it in debug builds (e.g., using `#if` DEBUG) before falling back; specifically,
replace the try? pattern with do/catch around encoder.encode, log the caught
error along with context (resolution.sourceURL or resolution.baseDirectory) via
your existing logging facility, and then continue to use an empty Data() as the
fallback so behavior remains unchanged in release builds.
- Around line 362-369: Add a clear doc comment above the private static func
existingDirectory(_ rawPath: String) explaining its fallback behavior: that it
expands tildes, checks existence, returns nil when path doesn't exist, returns
the directory if rawPath is itself a directory, and — importantly — when rawPath
points to an existing file it returns the file's parent directory (not nil).
Reference the function name existingDirectory and parameter rawPath in the
comment so callers understand this implicit file-to-parent fallback and expected
return semantics.
In `@Sources/MainWindowFocusController.swift`:
- Around line 354-359: Extract the duplicated feed/dock focusing sequence into a
private helper method (e.g. a method on MainWindowFocusController like
applyFocusSequence(focusFirstItem: Bool) -> Bool) that performs the conditional
calls to feedHost?.focusFirstItemFromCoordinator(),
dockHost?.focusFirstItemFromCoordinator() when focusFirstItem is true, then
returns the combined result of feedHost?.focusHostFromCoordinator() == true ||
dockHost?.focusHostFromCoordinator() == true; replace the duplicated blocks that
set modeResult with calls to this new helper in both locations.
🪄 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
Run ID: 12d3beed-7a60-468a-ad22-b806d23334ec
📒 Files selected for processing (14)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/DockPanelView.swiftSources/Feed/FeedCoordinator.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettings.swiftSources/MainWindowFocusController.swiftSources/Panels/TerminalPanel.swiftSources/RightSidebarPanelView.swiftcmuxUITests/FeedSidebarUITests.swiftdocs/dock.mddocs/feed.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8211dff0c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (index, item) in items.prefix(bodyRows).enumerated() { | ||
| let selected = index == selectedIndex |
There was a problem hiding this comment.
The list view always renders items.prefix(bodyRows) from index 0 while navigation updates selectedIndex across the full array. Once there are more rows than fit the terminal, selection can move off-screen and Enter acts on an item the user cannot see, making approvals/error handling unreliable when Feed history grows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b535af8582
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case .once, .always, .all, .bypass, .manual, .autoAccept, .ultraplan, .number(_): | ||
| if let item = feedTUIItem(in: items, at: selectedIndex) { | ||
| statusLine = try resolveFeedTUIItem(item, key: key, client: client, rawMode: &rawMode) |
There was a problem hiding this comment.
This branch routes .once/.always/.all/.bypass/.manual/.autoAccept/.ultraplan/.number to resolveFeedTUIItem for every pending card type, but the per-kind resolvers treat unknown keys as defaults (for example, permission falls back to once). In practice, pressing keys like 1, m, or u while a permission row is selected can silently approve it instead of being ignored, which is a dangerous accidental action path.
Useful? React with 👍 / 👎.
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 `@cmuxUITests/FeedSidebarUITests.swift`:
- Around line 213-219: The early-return currently checks app.state against
.runningForeground and .runningBackground together; change this to handle them
separately in the test in FeedSidebarUITests.swift: if app.state ==
.runningForeground return immediately; else if app.state == .runningBackground
call app.activate(), then wait for app.wait(for: .runningForeground, timeout:
<shortTimeout>) before proceeding (so subsequent typeKey() calls can send
keyboard input reliably); otherwise (not running) continue to wait as before
with XCTAssertTrue(app.wait(for: .runningForeground, timeout: 15), ...). Use the
existing app variable and state checks (app.state, app.activate(),
app.wait(for:timeout:)) to implement this flow.
🪄 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
Run ID: 38397fef-7ebe-4a5c-8a7c-eec23d3b7b82
📒 Files selected for processing (1)
cmuxUITests/FeedSidebarUITests.swift
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 `@CLI/cmux.swift`:
- Around line 17871-17880: In readFeedTUIFeedbackPrompt, re-creating
TerminalRawMode() can fail silently and leave rawMode nil; update the function
to verify the new TerminalRawMode was created successfully (check the result of
TerminalRawMode() before assigning to rawMode), and if it fails log an error or
throw/propagate an error so the caller can handle it; specifically add a
conditional around the TerminalRawMode() construction in
readFeedTUIFeedbackPrompt to detect failure and either restore previous state,
write a clear processLogger/error message, or return an error indicator so
readFeedTUIKey won't run without a valid raw mode.
🪄 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
Run ID: 4cdf9fa8-6232-4aef-ab7a-66c0a1cf204f
📒 Files selected for processing (3)
CLI/cmux.swiftSources/DockPanelView.swiftdocs/dock.md
✅ Files skipped from review due to trivial changes (1)
- docs/dock.md
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/DockPanelView.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 148f4b2b00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case .number(let index): | ||
| option = feedTUIOption(in: item.questionOptions, at: index - 1) | ||
| default: | ||
| option = item.questionOptions.first |
There was a problem hiding this comment.
When a question item is selected, pressing d is routed to resolveFeedTUIItem with key .deny, but this switch falls through to the default branch and submits questionOptions.first as the answer. In practice, a user trying to deny a question can unintentionally send the first option back to the agent, which is a dangerous accidental action on pending prompts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf281139f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| environment["CMUX_DOCK_CONTROL_TITLE"] = definition.title | ||
|
|
||
| return TerminalPanel( | ||
| workspaceId: UUID(), |
There was a problem hiding this comment.
Creating each Dock control with workspaceId: UUID() gives the child terminal a synthetic CMUX_WORKSPACE_ID that does not exist in TabManager. TerminalSurface propagates that ID into the shell environment, and many cmux CLI commands default to CMUX_WORKSPACE_ID when no --workspace is passed, so running cmux commands inside Dock controls can fail with workspace-not-found or target the wrong context. Dock surfaces should inherit the active workspace ID (or avoid exporting a fake one) so commands behave like normal in-app terminals.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
cmuxUITests/FeedSidebarUITests.swift (1)
302-315:⚠️ Potential issue | 🟠 MajorActivate the app when launch leaves it backgrounded.
Returning at Lines 309-310 for
.runningBackgroundstill leaves the latertypeKey()steps dependent on a non-foreground app.⌨️ Suggested change
- if app.state == .runningForeground || app.state == .runningBackground { + if app.state == .runningBackground { + app.activate() + _ = app.wait(for: .runningForeground, timeout: 2) + return + } + if app.state == .runningForeground { return } XCTAssertTrue( app.wait(for: .runningForeground, timeout: 15), "cmux failed to launch for Feed UI test. state=\(app.state.rawValue)"In macOS XCTest UI tests, are `XCUIApplication.typeKey(...)` / `XCUIElement.typeKey(...)` reliable when the application is in `.runningBackground`, or should the test call `activate()` and wait for `.runningForeground` first?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxUITests/FeedSidebarUITests.swift` around lines 302 - 315, The helper launchAndEnsureUsable currently returns early if app.state is .runningBackground, which can leave subsequent typeKey/type interactions failing; change the logic in launchAndEnsureUsable (the function calling app.launch()) to call app.activate() and then wait for .runningForeground (using app.wait(for: .runningForeground, timeout: ...)) when the app is .runningBackground instead of returning immediately, ensuring the app is foregrounded before proceeding with UI interactions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxUITests/FeedSidebarUITests.swift`:
- Around line 257-267: The predicate in waitForDockPortalToLeaveVisibleSidebar
currently treats missing or non-numeric portal stats as zero via
integerValue(in:key:), causing false passes; change the logic to treat
missing/non-numeric values as failures by either modifying integerValue(in:key:)
to return an optional (or adding integerValueStrict(in:key:)) and use that to
ensure the keys "visible_invalid_anchor_entry_count" and
"visible_orphan_terminal_subview_count" exist and parse as integers before
comparing to 0; apply the same fix to the similar check at lines 292-299 so the
XCTNSPredicateExpectation fails if a key is absent or not numeric instead of
defaulting to 0.
In `@Sources/DockPanelView.swift`:
- Around line 46-58: Replace the hard-coded debugDescription strings used in the
DecodingError.dataCorruptedError for normalizedID and normalizedCommand with
localized strings (use String(localized: "dockcontrol.id.blank", defaultValue:
"Dock control id must not be blank") and String(localized:
"dockcontrol.command.blank", defaultValue: "Dock control command must not be
blank")); update the two calls that throw in DockPanelView (the guards
referencing normalizedID, normalizedCommand, container, and
DecodingError.dataCorruptedError) to pass these localized strings so
error.localizedDescription in DockErrorView is localizable, and add the matching
keys to Resources/Localizable.xcstrings for English and Japanese.
- Around line 532-537: The rows created by ForEach currently pass
DockControlRuntime instances into DockControlSectionView and DockTerminalView
causing those child views to observe an ObservableObject inside the row subtree;
instead, capture an immutable snapshot of the per-row state and a bundle of
action closures and pass only those into the row. Concretely, replace passing
`runtime` (DockControlRuntime) into DockControlSectionView/DockTerminalView with
a value struct (e.g. DockControlRowSnapshot containing id, title, status, etc.,
and terminalHeight from heights[index]) plus an actions struct or individual
closures (open/close/toggle/...); update DockControlSectionView and
DockTerminalView to accept the snapshot and closures (removing any
`@ObservedObject/`@Bindable usage) and invoke closures for events. Apply the same
refactor for the other occurrences that currently pass DockControlRuntime into
row views so all ForEach rows receive immutable snapshots + action closures
only.
---
Duplicate comments:
In `@cmuxUITests/FeedSidebarUITests.swift`:
- Around line 302-315: The helper launchAndEnsureUsable currently returns early
if app.state is .runningBackground, which can leave subsequent typeKey/type
interactions failing; change the logic in launchAndEnsureUsable (the function
calling app.launch()) to call app.activate() and then wait for
.runningForeground (using app.wait(for: .runningForeground, timeout: ...)) when
the app is .runningBackground instead of returning immediately, ensuring the app
is foregrounded before proceeding with UI interactions.
🪄 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
Run ID: 2830cdb2-514b-4569-86eb-bb6a667aca8d
📒 Files selected for processing (4)
Sources/DockPanelView.swiftSources/RightSidebarPanelView.swiftSources/TerminalWindowPortal.swiftcmuxUITests/FeedSidebarUITests.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c85cd8c464
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6d9be9d7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!option) { | ||
| this.statusMessage = "No option for selected question."; | ||
| break; |
There was a problem hiding this comment.
When a pending question has no question_options, this branch just reports "No option for selected question" and exits without sending any feed.question.reply. Feed supports question cards that do not provide predefined options, so those requests become unresolvable from the default OpenTUI Dock flow and can block agent workflows waiting on a user answer.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f719ecec73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3feb21f4b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| throw DecodingError.dataCorruptedError( | ||
| forKey: .id, | ||
| in: container, | ||
| debugDescription: "Dock control id must not be blank" |
There was a problem hiding this comment.
DockControlDefinition.init(from:) throws DecodingError messages with hardcoded English ("Dock control id must not be blank" / command equivalent), and DockControlsStore.reload surfaces those directly via error.localizedDescription in DockErrorView, so invalid dock.json entries show untranslated UI text; this violates the explicit localization rule in /workspace/cmux/AGENTS.md for user-facing strings.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab827b6a51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case 27: | ||
| var sequence = [UInt8](repeating: 0, count: 2) | ||
| let count = Darwin.read(STDIN_FILENO, &sequence, 2) | ||
| guard count == 2, sequence[0] == 91 else { return .ignored } |
There was a problem hiding this comment.
The legacy feed tui input parser does a blocking read(..., 2) immediately after seeing byte 27 (Escape). If the user presses Esc by itself (or an incomplete escape sequence arrives), the loop blocks waiting for two extra bytes and the TUI appears frozen until more input is typed. This impacts fallback mode users whenever OpenTUI is unavailable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddf163b7d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # .github/swift-file-length-budget.tsv # Sources/MainWindowFocusController.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1b3228163
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let mode = RightSidebarMode.modeShortcut(for: event), let window { | ||
| _ = AppDelegate.shared?.focusRightSidebarInActiveMainWindow(mode: mode, focusFirstItem: true, preferredWindow: window) | ||
| return |
There was a problem hiding this comment.
This unconditional branch captures every switchRightSidebarTo* key while any terminal surface is focused, so combos like the default Ctrl+1..5 are swallowed in normal workspace terminals and never reach terminal apps. Even though AppDelegate now context-gates these shortcuts, this new GhosttyNSView.keyDown path bypasses that guard and still forces focusRightSidebarInActiveMainWindow, causing unintended sidebar mode switches and input loss outside sidebar-focused responders.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bb4ac1aff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (primaryOptions.length === 0) { | ||
| return []; |
There was a problem hiding this comment.
For question cards with no options, this always returns [] regardless of which action key triggered the reply. Combined with question action routing, pressing numeric option keys (1-0) on optionless questions will submit an empty feed.question.reply instead of being ignored, creating an accidental-resolution path for pending prompts.
Useful? React with 👍 / 👎.
This branch was successfully deployed
1 participant
Summary:
Verification:
Summary by cubic
Adds a new Dock in the right sidebar for persistent TUI controls and a keyboard-first Feed TUI (
cmux feed tui) built on@opentui/core. Feed and Dock are separate modes; Dock controls run in login‑shell PTYs, persist across mode changes, forward shortcuts, and never steal focus.New Features
.cmux/dock.jsonor~/.config/cmux/dock.json(trust flow; per-controlcwd,env,height). Built‑in Feed uses bundledcmux. Docs:docs/dock.md,docs/feed.md.cmux feed tui [--opentui|--legacy]andcmux feed clear. OpenTUI Feed with keyboard nav and q/Ctrl‑C to quit.switchRightSidebarToDock). Labels, schema, localizations, docs, and tests updated.Bug Fixes
request_id, options,default_mode, with strict text limits.Written for commit 1ff5e2f. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
cmux feed tuiterminal TUI: keyboard-first navigation, accept/deny/replan/refresh actions.UI/UX
Documentation
Tests
Localization