Repository navigation
iOS: data-driven terminal toolbar with custom actions + customize button - #5510
Merged
Merged
Conversation
Make the iOS keyboard accessory bar fully data-driven: built-in shortcuts and user-defined custom actions are ordered, shown/hidden, and reordered together. Adds a "customize" button at the end of the bar that opens the shortcuts editor directly, and an add/edit/delete flow for custom actions that type literal text (e.g. a Claude/Codex launcher), with an optional "Run after typing" that appends Return. New pure CmuxMobileTerminalKit types (ToolbarItemID, ToolbarActionPayload, CustomToolbarAction, ToolbarLayoutMigration) plus a generic TerminalAccessoryLayoutReducer<ID> own the logic and migration, unit-tested via swift test. TerminalAccessoryConfiguration persists the unified ToolbarItemID layout; a one-time v1->v2 migration preserves every existing user's order and hidden set. Button identity moves to AccessoryActionButton so custom UUIDs never collide with built-in enum raw values; the modifier/zoom/armed machinery is unchanged. The editor lives in CmuxMobileShellUI and is presented from the surface Coordinator to keep the package layering correct. en+ja localized. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Complex PR? Review this PR in Change Stack to move by importance, not file order. 📝 WalkthroughWalkthroughThis PR introduces user-configurable custom terminal toolbar actions by adding a unified toolbar item identification system, refactoring the accessory configuration from enum-based to ID-based storage with backward-compatible migration, integrating action editing into SwiftUI, and wiring the settings UI into existing UIKit terminal views. ChangesCustom Toolbar Actions Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR spans three packages and multiple architectural layers with significant logic density: generalized reducer, v1→v2 migration, configuration refactoring, UIKit/SwiftUI integration, and new model types. Changes are heterogeneous across model design, storage logic, type-safe button dispatch, and UI workflows, requiring separate reasoning for each layer despite consistent patterns. Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (17 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR makes the iOS terminal keyboard toolbar fully data-driven by introducing a unified
Confidence Score: 5/5Safe to merge. The data-driven toolbar refactor is well-isolated, the v1→v2 migration is lossless, and the new persistence/mutation paths are consistent and covered by 68 passing tests. All changed paths — migration, add/edit/delete custom actions, toolbar rebuild, modifier-key arming, and the customize-button presentation flow — behave correctly. The one open item (silent JSON-encode failure in persist()) was raised in a previous review thread and is unchanged here. No new logic defects were introduced. No files require special attention. TerminalAccessoryConfiguration.swift carries the pre-existing try? risk already flagged in an earlier review. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TerminalAccessoryConfiguration.shared] -->|displayItems / enabledItems| B[ResolvedToolbarItem]
B --> B1[.builtin TerminalInputAccessoryAction]
B --> B2[.custom CustomToolbarAction]
A -->|addCustomAction / updateCustomAction / removeCustomAction / resetToDefaults| C[TerminalAccessoryLayoutReducer<ToolbarItemID>]
C -->|.load / .setEnabled / .move / .defaultLayout| D[Layout order + enabled]
D --> A
A -->|persist| E[(UserDefaults v2 order / enabled / custom JSON)]
A -->|didChangeNotification| F[TerminalInputTextView]
F -->|enabledItems| G[AccessoryActionButton carries ResolvedToolbarItem]
G -->|tap .builtin| H[handleAccessoryAction]
G -->|tap .custom| I[handleCustomAction → onEscapeSequence]
F -->|tap customize button| J[onOpenToolbarSettings]
J --> K[GhosttySurfaceView delegate]
K --> L[GhosttySurfaceRepresentable.Coordinator]
L -->|UIHostingController| M[TerminalShortcutsSettingsView]
M -->|sheet| N[CustomToolbarActionEditorView]
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+115
to
+119
| @Test("empty text payload produces no output") | ||
| func emptyText() { | ||
| #expect(CustomToolbarAction(title: "x", payload: .text("")).output == nil) | ||
| #expect(CustomToolbarAction(title: "x", payload: .text("\n")).output == Data("\r".utf8)) | ||
| } |
Contributor
There was a problem hiding this comment.
" " does produce output (Data(" ".utf8)). A lone newline is not empty once it normalises to ; the test name and second expectation are in direct contradiction, which makes it easy to misread the intended contract for the " " case. Splitting into two focused tests clarifies intent.
Suggested change
| @Test("empty text payload produces no output") | |
| func emptyText() { | |
| #expect(CustomToolbarAction(title: "x", payload: .text("")).output == nil) | |
| #expect(CustomToolbarAction(title: "x", payload: .text("\n")).output == Data("\r".utf8)) | |
| } | |
| @Test("empty string payload produces no output") | |
| func emptyStringProducesNil() { | |
| #expect(CustomToolbarAction(title: "x", payload: .text("")).output == nil) | |
| } | |
| @Test("lone newline payload normalises to carriage return") | |
| func loneNewlineNormalisesToCR() { | |
| #expect(CustomToolbarAction(title: "x", payload: .text("\n")).output == Data("\r".utf8)) | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift`:
- Around line 95-159: Public API symbols in TerminalAccessoryConfiguration are
missing Swift-DocC triple-slash comments; add concise DocC comments for each
public symbol: displayItems, enabledItems, isEnabled(_:), setEnabled(_:_:),
moveItems(from:to:), addCustomAction(_:), updateCustomAction(_:),
removeCustomAction(id:), and resetToDefaults(). For each, add a one-sentence
summary using ///, and where applicable include /// - Parameters: and /// -
Returns: entries (e.g., isEnabled returns Bool,
setEnabled/moveItems/add/update/remove describe parameters,
displayItems/enabledItems describe contents); follow the existing file’s comment
style and wording pattern for consistency.
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction`+ItemID.swift:
- Line 6: TerminalInputAccessoryAction currently uses implicit Int raw values
which are persisted via ToolbarItemID.builtin(rawValue) -> "builtin.<rawValue>"
and will break when cases are reordered/inserted; make
TerminalInputAccessoryAction an Int-backed enum with explicit, stable rawValue
assignments for every case (and avoid reusing values), so itemID (the var
itemID: ToolbarItemID { .builtin(rawValue) }) continues to return the same
storageKey across versions; add a short comment on each case noting that the
numeric value is stable and reserved to prevent accidental reordering or reuse.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cbcf423f-0d2f-4a01-a495-a68b8ce90540
📒 Files selected for processing (19)
Packages/CmuxMobileShellUI/Package.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CustomToolbarActionEditorView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/AccessoryActionButton.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/ResolvedToolbarItem.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction+ItemID.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/CustomToolbarAction.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalKeyModifier+Codable.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalSpecialKey.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarActionPayload.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarItemID.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarLayoutMigration.swiftPackages/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/ToolbarCustomizationTests.swiftios/cmux/Resources/Localizable.xcstrings
Comment on lines
+95
to
159
| public var displayItems: [ResolvedToolbarItem] { | ||
| displayOrder.compactMap(resolve) | ||
| } | ||
|
|
||
| /// Snapshot of the live state in the reducer's raw-identifier vocabulary. | ||
| private var currentLayout: TerminalAccessoryLayoutReducer.Layout { | ||
| TerminalAccessoryLayoutReducer.Layout( | ||
| order: displayOrder.map(\.rawValue), | ||
| enabled: Set(enabledSet.map(\.rawValue)) | ||
| ) | ||
| /// The shown items in display order — exactly what the toolbar's configurable | ||
| /// region renders, after the pinned modifier/zoom buttons. | ||
| public var enabledItems: [ResolvedToolbarItem] { | ||
| displayOrder.filter { enabledSet.contains($0) }.compactMap(resolve) | ||
| } | ||
|
|
||
| /// Project a reducer layout back onto the `@Observable` stored properties. | ||
| private func apply(_ layout: TerminalAccessoryLayoutReducer.Layout) { | ||
| displayOrder = layout.order.compactMap(TerminalInputAccessoryAction.init(rawValue:)) | ||
| enabledSet = Set(layout.enabled.compactMap(TerminalInputAccessoryAction.init(rawValue:))) | ||
| /// Whether `id` is currently shown on the bar. | ||
| public func isEnabled(_ id: ToolbarItemID) -> Bool { | ||
| enabledSet.contains(id) | ||
| } | ||
|
|
||
| /// Show or hide `action`. No-op for non-configurable actions. | ||
| public func setEnabled(_ action: TerminalInputAccessoryAction, _ isEnabled: Bool) { | ||
| guard action.isUserConfigurable else { return } | ||
| apply(reducer.setEnabled(action.rawValue, isEnabled, in: currentLayout)) | ||
| // MARK: - Mutations | ||
|
|
||
| /// Show or hide the item identified by `id`. | ||
| public func setEnabled(_ id: ToolbarItemID, _ isEnabled: Bool) { | ||
| apply(reducer.setEnabled(id, isEnabled, in: currentLayout)) | ||
| persistAndNotify() | ||
| } | ||
|
|
||
| /// Reorder the configurable actions. `offsets`/`destination` are indices | ||
| /// into ``displayOrder`` (the SwiftUI `onMove` contract). | ||
| public func moveActions(from offsets: IndexSet, to destination: Int) { | ||
| /// Reorder the configurable items. `offsets`/`destination` are indices into | ||
| /// ``displayOrder`` (the SwiftUI `onMove` contract). | ||
| public func moveItems(from offsets: IndexSet, to destination: Int) { | ||
| apply(reducer.move(from: offsets, to: destination, in: currentLayout)) | ||
| persistAndNotify() | ||
| } | ||
|
|
||
| /// Restore the default order (enum order) with every shortcut shown. | ||
| /// Append a new custom action, shown at the end of the configurable region. | ||
| public func addCustomAction(_ action: CustomToolbarAction) { | ||
| customActions.append(action) | ||
| reducer = Self.makeReducer(customActions: customActions) | ||
| apply(reducer.load( | ||
| savedOrder: displayOrder, | ||
| savedEnabled: Array(enabledSet) + [action.itemID] | ||
| )) | ||
| persistAndNotify() | ||
| } | ||
|
|
||
| /// Replace an existing custom action in place (matched by ``CustomToolbarAction/id``). | ||
| /// Its position and shown/hidden state are preserved. | ||
| public func updateCustomAction(_ action: CustomToolbarAction) { | ||
| guard let index = customActions.firstIndex(where: { $0.id == action.id }) else { return } | ||
| customActions[index] = action | ||
| reducer = Self.makeReducer(customActions: customActions) | ||
| persistAndNotify() | ||
| } | ||
|
|
||
| /// Remove a custom action by id. It drops from the order and shown set. | ||
| public func removeCustomAction(id: UUID) { | ||
| guard customActions.contains(where: { $0.id == id }) else { return } | ||
| customActions.removeAll { $0.id == id } | ||
| reducer = Self.makeReducer(customActions: customActions) | ||
| apply(reducer.load(savedOrder: displayOrder, savedEnabled: Array(enabledSet))) | ||
| persistAndNotify() | ||
| } | ||
|
|
||
| /// Restore the default arrangement (canonical order, every item shown). | ||
| /// Custom actions are kept (appended after the built-ins), not deleted. | ||
| public func resetToDefaults() { | ||
| apply(reducer.defaultLayout()) | ||
| persistAndNotify() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Public method documentation is incomplete.
Several public methods lack Swift-DocC triple-slash comments:
displayItems(line 95)enabledItems(line 101)isEnabled(_:)(line 106)setEnabled(_:_:)(line 113)moveItems(from:to:)(line 120)addCustomAction(_:)(line 126)updateCustomAction(_:)(line 136)removeCustomAction(id:)(line 145)resetToDefaults()(line 156)
Based on coding guidelines, every public symbol in packages under Packages/ must be documented with a Swift-DocC triple-slash comment at the time of writing. Add doc comments for each public property and method following the established pattern (one-sentence summary, parameter/returns callouts where applicable).
📝 Proposed documentation template
+ /// Every configurable item in display order (regardless of shown/hidden),
+ /// resolved to its built-in action or custom action. This is what the
+ /// settings editor lists.
public var displayItems: [ResolvedToolbarItem] {
+ /// The shown items in display order — exactly what the toolbar's configurable
+ /// region renders, after the pinned modifier/zoom buttons.
public var enabledItems: [ResolvedToolbarItem] {
+ /// Whether `id` is currently shown on the bar.
+ /// - Parameter id: The toolbar item identifier.
+ /// - Returns: `true` if the item is shown, `false` otherwise.
public func isEnabled(_ id: ToolbarItemID) -> Bool {
+ /// Show or hide the item identified by `id`.
+ /// - Parameters:
+ /// - id: The toolbar item identifier.
+ /// - isEnabled: `true` to show, `false` to hide.
public func setEnabled(_ id: ToolbarItemID, _ isEnabled: Bool) {
+ /// Reorder the configurable items.
+ /// - Parameters:
+ /// - offsets: The indices being moved (from ``displayOrder``).
+ /// - destination: The insertion index.
public func moveItems(from offsets: IndexSet, to destination: Int) {
+ /// Append a new custom action, shown at the end of the configurable region.
+ /// - Parameter action: The custom action to add.
public func addCustomAction(_ action: CustomToolbarAction) {
+ /// Replace an existing custom action in place (matched by id).
+ /// - Parameter action: The updated action. Its position and shown/hidden state are preserved.
public func updateCustomAction(_ action: CustomToolbarAction) {
+ /// Remove a custom action by id. It drops from the order and shown set.
+ /// - Parameter id: The UUID of the custom action to remove.
public func removeCustomAction(id: UUID) {
+ /// Restore the default arrangement (canonical order, every item shown).
+ /// Custom actions are kept (appended after the built-ins), not deleted.
public func resetToDefaults() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift`
around lines 95 - 159, Public API symbols in TerminalAccessoryConfiguration are
missing Swift-DocC triple-slash comments; add concise DocC comments for each
public symbol: displayItems, enabledItems, isEnabled(_:), setEnabled(_:_:),
moveItems(from:to:), addCustomAction(_:), updateCustomAction(_:),
removeCustomAction(id:), and resetToDefaults(). For each, add a one-sentence
summary using ///, and where applicable include /// - Parameters: and /// -
Returns: entries (e.g., isEnabled returns Bool,
setEnabled/moveItems/add/update/remove describe parameters,
displayItems/enabledItems describe contents); follow the existing file’s comment
style and wording pattern for consistency.
Source: Coding guidelines
| public extension TerminalInputAccessoryAction { | ||
| /// This built-in action's unified identifier in the configurable region, | ||
| /// pairing it with custom actions under one ``ToolbarItemID`` space. | ||
| var itemID: ToolbarItemID { .builtin(rawValue) } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "itemID persistence mapping:"
rg -n 'var itemID: ToolbarItemID|\.builtin\(rawValue\)' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction+ItemID.swift
echo
echo "Enum declaration and case list:"
rg -n '^public enum TerminalInputAccessoryAction: Int' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
rg -n '^[[:space:]]*case[[:space:]]+[A-Za-z0-9_]+' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift | sed -n '1,40p'
echo
echo "Explicit case raw-value assignments (expected after hardening):"
rg -n '^[[:space:]]*case[[:space:]]+[A-Za-z0-9_]+[[:space:]]*=' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftRepository: manaflow-ai/cmux
Length of output: 1494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "TerminalInputAccessoryAction+ItemID.swift (itemID mapping):"
sed -n '1,120p' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction+ItemID.swift
echo
echo "TerminalInputAccessoryAction enum block (full case list / raw values):"
# Show around the enum declaration in GhosttySurfaceView.swift
sed -n '220,320p' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftRepository: manaflow-ai/cmux
Length of output: 3621
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '240,310p' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftRepository: manaflow-ai/cmux
Length of output: 1869
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find where ToolbarItemID/builtins are persisted or decoded:"
rg -n --hidden \
'ToolbarItemID|terminal\.input_accessory|TerminalInputAccessoryAction|\.builtin\(' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 200
echo
echo "Find any defaults keys or Codable payloads that store built-in toolbar item raw values:"
rg -n --hidden \
'quickTerminal|toolbar|enabled|layout|Codable|UserDefaults|`@AppStorage`|defaults' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 200Repository: manaflow-ai/cmux
Length of output: 25072
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "TerminalAccessoryConfiguration persistence keys + how ToolbarItemID/storageKey maps to builtin(rawValue):"
sed -n '1,240p' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift
echo
echo "ResolvedToolbarItem + ToolbarItemID storageKey/builtin format: (if in separate file, print both quickly)"
rg -n 'struct ToolbarItemID|enum ToolbarItemID|typealias ToolbarItemID|init\(storageKey' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 50
# Show the ToolbarItemID definition file by following likely path
# (Use fd to find the file by type name)
FILE=$(fd -a -e swift "ToolbarItemID" Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal --type f | head -n 1 || true)
if [ -n "${FILE}" ]; then
echo
echo "ToolbarItemID definition in: $FILE"
sed -n '1,220p' "$FILE"
else
echo
echo "Could not locate ToolbarItemID definition by filename; listing candidate matches:"
rg -n 'ToolbarItemID' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 80
fiRepository: manaflow-ai/cmux
Length of output: 13163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden \
'ToolbarItemID\.init\(storageKey:|init\(storageKey:|var storageKey:|case let \.builtin|enum ToolbarItemID|struct ToolbarItemID|indirect enum ToolbarItemID' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 200Repository: manaflow-ai/cmux
Length of output: 1369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for the ToolbarItemID type definition and storageKey formatting:"
rg -n --hidden \
'enum ToolbarItemID|struct ToolbarItemID|typealias ToolbarItemID|ToolbarItemID\s*\{|storageKey' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 200
echo
echo "Search specifically for .builtin(storageKey:) / storageKey constructors:"
rg -n --hidden \
'\.builtin\(|builtin\(|init\(storageKey:|storageKey:\s*String|var storageKey' \
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal | head -n 200Repository: manaflow-ai/cmux
Length of output: 3062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ToolbarItemID definition locations:"
rg -n --hidden --glob='*.swift' \
'enum ToolbarItemID|struct ToolbarItemID|typealias ToolbarItemID' . | head -n 200
echo
echo "Look for ToolbarItemID.init(storageKey:) / storageKey property across repo:"
rg -n --hidden --glob='*.swift' \
'init\(storageKey:|storageKey\s*:' . | rg 'ToolbarItemID' | head -n 200
echo
echo "Look for builtin case constructor that takes rawValue:"
rg -n --hidden --glob='*.swift' \
'\.builtin\(rawValue\)|case builtin|builtin\(' . | rg 'ToolbarItemID' | head -n 200Repository: manaflow-ai/cmux
Length of output: 3905
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' Packages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarItemID.swift
echo
echo "--- ToolbarLayoutMigration.swift ---"
sed -n '1,120p' Packages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/ToolbarLayoutMigration.swift
echo
echo "--- ResolvedToolbarItem.swift ---"
sed -n '1,120p' Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/ResolvedToolbarItem.swiftRepository: manaflow-ai/cmux
Length of output: 5179
Freeze TerminalInputAccessoryAction raw values used for persisted toolbar IDs.
TerminalInputAccessoryAction.itemID builds ToolbarItemID.builtin(rawValue), whose storageKey persists as "builtin.<rawValue>" in UserDefaults (v2 order/enabled). Because TerminalInputAccessoryAction currently relies on implicit Int raw values, reordering/inserting cases would remap existing "builtin.*" entries on upgrade.
Suggested hardening (explicit stable raw values on the enum)
--- a/Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
+++ b/Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
@@
public enum TerminalInputAccessoryAction: Int, CaseIterable, Sendable {
- case control
- case alternate
- case command
- case shift
- case zoomOut
- case zoomIn
- case escape
- case tab
- case upArrow
- case downArrow
- case leftArrow
- case rightArrow
- case claude
- case codex
- case tilde
- case pipe
- case dollar
- case slash
- case atSign
- case ctrlC
- case ctrlD
- case ctrlZ
- case ctrlL
- case home
- case end
- case pageUp
- case pageDown
+ case control = 0
+ case alternate = 1
+ case command = 2
+ case shift = 3
+ case zoomOut = 4
+ case zoomIn = 5
+ case escape = 6
+ case tab = 7
+ case upArrow = 8
+ case downArrow = 9
+ case leftArrow = 10
+ case rightArrow = 11
+ case claude = 12
+ case codex = 13
+ case tilde = 14
+ case pipe = 15
+ case dollar = 16
+ case slash = 17
+ case atSign = 18
+ case ctrlC = 19
+ case ctrlD = 20
+ case ctrlZ = 21
+ case ctrlL = 22
+ case home = 23
+ case end = 24
+ case pageUp = 25
+ case pageDown = 26
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputAccessoryAction`+ItemID.swift
at line 6, TerminalInputAccessoryAction currently uses implicit Int raw values
which are persisted via ToolbarItemID.builtin(rawValue) -> "builtin.<rawValue>"
and will break when cases are reordered/inserted; make
TerminalInputAccessoryAction an Int-backed enum with explicit, stable rawValue
assignments for every case (and avoid reusing values), so itemID (the var
itemID: ToolbarItemID { .builtin(rawValue) }) continues to return the same
storageKey across versions; add a short comment on each case noting that the
numeric value is stable and reserved to prevent accidental reordering or reuse.
This was referenced Jun 6, 2026
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
June 6, 2026 11:31
Dismissed: CodeRabbit now posts non-blocking comment reviews (request_changes_workflow=false, #5538).
…om-actions # Conflicts: # Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift # Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift # Packages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift # ios/cmux/Resources/Localizable.xcstrings
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 77cc9c4. Configure here.
| // view controller. | ||
| guard let presenter = presentingController(for: surfaceView) else { return } | ||
| let editor = UIHostingController(rootView: TerminalShortcutsSettingsView()) | ||
| presenter.present(editor, animated: true) |
There was a problem hiding this comment.
Customize stacks settings modals
Medium Severity
Each tap on the toolbar customize control calls present on the top view controller without checking whether TerminalShortcutsSettingsView is already presented. Repeated taps stack multiple identical settings screens; Done only dismisses the topmost one.
Reviewed by Cursor Bugbot for commit 77cc9c4. Configure here.
| title: trimmedTitle, | ||
| symbolName: nil, | ||
| payload: .text(text) | ||
| ) |
There was a problem hiding this comment.
Edit overwrites keyCombo payloads
Low Severity
Saving from the custom-action editor always writes a text payload and clears symbolName, even when the action being edited used keyCombo. Swipe-to-edit in shortcuts settings does not block those actions.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 77cc9c4. Configure here.
lawrencecchen
added a commit
that referenced
this pull request
Jun 7, 2026
- Defer composer field focus one runloop after appear so it reliably takes first responder from the terminal input while the keyboard is up (inline onAppear focus was unreliable; it now hands the keyboard over in place). - Give the field pill and the round send/dismiss buttons a shared 40pt control height so the single-line composer lines up; the field still grows multi-line. The accessory-bar glass restyle from the original commit is dropped on rebase: it targeted the pre-#5532 button.tag toolbar API, which #5510/#5532 replaced with AccessoryActionButton + .item. The composer button rides main's toolbar. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 11, 2026
Merged
This was referenced Jun 21, 2026
This branch was successfully 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.
Summary
claude --dangerously-skip-permissions), with an optional "Run after typing" that appends Return. The editor supports add / edit / delete; the model also supports key-combo actions (encoded via the existingTerminalKeyEncoder) for a follow-up editor surface.[Int]schema to a unifiedToolbarItemIDschema, preserving every existing user's order and hidden set.Design
CmuxMobileTerminalKit:ToolbarItemID(.builtin/.custom),ToolbarActionPayload,CustomToolbarAction(with byteoutput),ToolbarLayoutMigration, and a genericTerminalAccessoryLayoutReducer<ID>(the existing[Int]reducer tests still pass unchanged).TerminalAccessoryConfiguration(CmuxMobileTerminal) now persistsorder/enabledasToolbarItemIDstorage keys + custom actions as JSON, and projectsResolvedToolbarItems for the bar builder and the editor.Inttag to anAccessoryActionButtonthat carries its resolved item, so custom actions never collide with built-in enum raw values. The built-in modifier/zoom/armed machinery is unchanged.CmuxMobileShellUIand is presented from the surface'sCoordinator(the terminal package fires a delegate callback; the UI package owns the editor), keeping the package layering correct.Testing
swift test --package-path Packages/CmuxMobileTerminalKit— 68 tests pass, including new coverage forToolbarItemIDround-trip, the generic reducer over mixed built-in/custom ids, v1→v2 migration preserving order/enabled, andCustomToolbarAction.output(text normalization + key-combo encoding).ios/scripts/reload.sh --tag tbar).ios/cmux/Resources/Localizable.xcstringsfor every new string.Issues
Merge order
First of three stacked iOS PRs. The only cross-PR overlap is
ios/cmux/Resources/Localizable.xcstrings(each adds keys). Merge #5510 → #5512 → #5513; the later PRs rebase on main.Note
Medium Risk
Touches the live terminal input path (bytes sent on tap) and UserDefaults migration; mistakes could alter toolbar layout or inject unexpected input, but scope is localized and covered by kit tests.
Overview
The iOS terminal keyboard toolbar becomes data-driven: built-in shortcuts and new user-defined custom actions share one order, visibility, and reorder model via
ToolbarItemIDand a genericTerminalAccessoryLayoutReducer. Custom actions send literal text (optional auto-Return) or key combos; persistence moves to a v2UserDefaultsschema with a one-time v1→v2 migration that preserves existing order and hidden state.UI: A trailing Customize control on the bar fires a delegate callback;
GhosttySurfaceRepresentablepresentsTerminalShortcutsSettingsView, which now supports add/edit/delete custom actions (CustomToolbarActionEditorView). Toolbar buttons useAccessoryActionButtonwithResolvedToolbarIteminstead ofInttags. Zoom controls move to the trailing pinned region; modifier/armed behavior is unchanged.Reviewed by Cursor Bugbot for commit 77cc9c4. Bugbot is set up for automated code reviews on this repo. Configure here.