Skip to content

iOS: multiple configurable toolbar rows - #6920

Closed
austinywang wants to merge 29 commits into
mainfrom
issue-6090-ios-multiple-configurable-toolbar-rows
Closed

austinywang wants to merge 29 commits into
mainfrom
issue-6090-ios-multiple-configurable-toolbar-rows

Conversation

@austinywang

@austinywang austinywang commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6090

Summary

  • Persist terminal toolbar layouts as configurable rows while migrating existing flat layouts into one row.
  • Render multiple horizontal shortcut rows above the keyboard and reserve matching terminal grid height.
  • Add row-count and row-assignment controls to iOS shortcut settings.

Validation

  • swift test (Packages/iOS/CmuxMobileTerminalKit)
  • ios/cmux/Resources/Localizable.xcstrings parsed with python3 -m json.tool
  • swift test (Packages/iOS/CmuxMobileShellUI) blocked locally by GhosttyKit.xcframework artifact missing a binary artifact

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Adds 1–3 configurable iOS toolbar rows with per‑row horizontal scroll and a row‑count–aware dock height; the terminal and dock reflow instantly on any row or item change. Migrates to a v4 row schema that preserves existing setups and centralizes height/spacing in TerminalAccessoryDockMetrics.

  • New Features

    • Settings: rows stepper (1–3), per‑item “Move to Row” picker, and per‑row sections with drag reordering; increases append empty rows, decreases merge overflow into the last row; localized labels.
    • Toolbar: vertically stacked, per‑row horizontal lists; bottom row still carries HIDE and customize.
    • Layout/Persistence: v4 row‑based layout persisted as rows (with migration from v1–v3), preserves empty rows, forward‑compat appends new actions to the last row; GhosttySurfaceView observes TerminalAccessoryConfiguration.didChangeNotification and updates visibility/height immediately.
  • Bug Fixes

    • Selecting an item’s current row is a no‑op to avoid churn.
    • Flat scoped lists reorder only the scoped items while preserving non‑scoped positions and row lengths; full cross‑row moves use the row picker.
    • Stabilized the shell integration PATH test to avoid env‑dependent failures.

Written for commit f900bc8. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Terminal shortcut buttons can now be arranged across multiple rows in settings.
    • Added support for moving shortcuts between rows and adjusting the number of rows.
    • The terminal accessory area now adapts its layout dynamically to the selected row count.
  • Bug Fixes

    • Improved shortcut reordering so it stays consistent within each row.
    • Preserved row layout and hidden/visible states more reliably when settings are reloaded.
    • Added localized labels for the new row controls.

@vercel

vercel Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jul 5, 2026 2:19am
cmux-staging Building Building Preview, Comment Jul 5, 2026 2:19am

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR replaces the iOS terminal accessory toolbar's flat single-row model with a configurable multi-row model. TerminalAccessoryLayoutReducer stores a row matrix, TerminalAccessoryConfiguration migrates to v4 row-based persistence, TerminalInputTextView builds stacked per-row scroll views, GhosttySurfaceView computes dock height from the live row count, and the settings view exposes row-count and move-to-row controls.

Changes

iOS terminal toolbar rows

Layer / File(s) Summary
Row layout model
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift
Layout switches from a stored flat order to a stored rows matrix with init(rows:enabled:), computed order/visibleOrder/visibleRows, and a new row-aware load(savedRows:savedEnabled:rowCount:) plus legacy migration overload.
Row operations
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift
setEnabled, scoped reorder, row-local move, cross-row move, and setRowCount all return Layout(rows:enabled:) and preserve empty rows during count changes.
Reducer tests
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalAccessoryLayoutReducerTests.swift
New tests cover saved-row loading, empty-row preservation, row-count decrease/increase, row-local moves, scoped reorders, and cross-row moves.
Accessory configuration
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift, ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalAccessoryConfigurationTests.swift
displayRows replaces the stored displayOrder; initialization reads v4 rows and migrates legacy v3/v2 data; new mutation APIs (moveItems(inRow:), moveItem(toRow:), setRowCount, reorderItems(limitedTo:)) added; persist() writes v4 rows key; tests updated for v4 migration and no-op move.
Shortcut row settings
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutRowSection.swift, ...TerminalShortcutsSettingsView.swift, ios/cmux/Resources/Localizable.xcstrings
New TerminalShortcutRowSection model; settings body splits by scope into row sections with a Stepper and per-row move controls; row(for:rowIndex:) adds a "Move to Row" Picker; row helpers and localized strings added.
Accessory toolbar layout
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift
Single scroll/stack replaced by a vertical rowsStack containing per-row horizontally-scrolling pairs; row sizing via dockedRowsHeight(rowCount:)/dockedButtonRowHeight(rowCount:); modifier state and button styles iterate across all accessoryActionStackViews.
Dock geometry
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift
persistentToolbarHeight becomes a computed property from rowCount; an observer for didChangeNotification triggers dock and viewport relayout; deinit removes the observer.

Sequence Diagram(s)

sequenceDiagram
  participant TerminalShortcutsSettingsView
  participant TerminalAccessoryConfiguration
  participant NotificationCenter
  participant GhosttySurfaceView
  participant TerminalInputTextView

  TerminalShortcutsSettingsView->>TerminalAccessoryConfiguration: setRowCount / moveItem(toRow:) / reorderItems(limitedTo:)
  TerminalAccessoryConfiguration->>TerminalAccessoryConfiguration: persist displayRows under v4 key
  TerminalAccessoryConfiguration->>NotificationCenter: post didChangeNotification
  NotificationCenter-->>GhosttySurfaceView: handleAccessoryConfigurationChanged()
  GhosttySurfaceView->>TerminalInputTextView: dockedButtonRowHeight(rowCount:)
  GhosttySurfaceView->>GhosttySurfaceView: relayout bottom dock and terminal viewport
  NotificationCenter-->>TerminalInputTextView: handleAccessoryConfigurationChanged()
  TerminalInputTextView->>TerminalInputTextView: populateAccessoryActions() per enabledItemRows
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • manaflow-ai/cmux#5510: Establishes the ToolbarItemID/custom-action foundation and TerminalShortcutsSettingsView that this PR's v4 row-based persistence and row editing build directly on.
  • manaflow-ai/cmux#5532: Modifies TerminalAccessoryConfiguration and TerminalAccessoryLayoutReducer default-order loading, directly related to this PR's row-based redesign of those same load paths.
  • manaflow-ai/cmux#6101: Overlaps in TerminalAccessoryConfiguration migration/folding logic for the Return shortcut, which this PR extends into the v4 migration path.

Poem

🐇 Hoppity hop, the rows now stack,
Each shortcut button neatly racked.
V4 rows persist with care,
Migration runs once — fair and square.
Multi-row toolbar, finally here! 🎉

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.12% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement multiple configurable toolbar rows and persist the layout, matching issue #6090.
Out of Scope Changes check ✅ Passed The reviewed changes stay focused on row-based iOS toolbar layout, persistence, UI controls, tests, and localization.
Cmux Swift Actor Isolation ✅ Passed No new off-main access or unsafe Sendable refs; the new reducer stays pure, and the UI stores/views that use configuration are already @MainActor.
Cmux Swift Blocking Runtime ✅ Passed The changed Swift code adds row/layout/observer logic only; no new waits/sleeps/syncs appear, and the only NSLock/DispatchSemaphore in GhosttySurfaceView are documented carve-outs.
Cmux Browser Automation Off-Main ✅ Passed PR only changes iOS toolbar/layout, reducer, localization, and tests; no browser.* socket commands, worker routing, or policy-test files were touched.
Cmux Expensive Synchronous Load ✅ Passed Touched Swift files only add toolbar-row UI/layout and UserDefaults row persistence; no RestorableAgentSessionIndex, history stores, JSONL scans, or Task.detached loads were introduced.
Cmux Cache Substitution Correctness ✅ Passed No fresh authoritative read was swapped for a stale cache; the new row-height reads are live and configuration changes trigger rebuilds via notifications.
Cmux No Hacky Sleeps ✅ Passed No changed TypeScript/JS/shell/build runtime scripts; the PR touches Swift, tests, and localization only, so the no-hacky-sleeps rule is out of scope.
Cmux Algorithmic Complexity ✅ Passed The new row-aware paths stay linear and row-bounded (1–3 rows); I found no new quadratic batch rescans or hot-path sort/filter regressions.
Cmux Swift Concurrency ✅ Passed Changed hunks are layout/state-only; keyword search found only pre-existing Task/DispatchQueue code in GhosttySurfaceView, not new async patterns.
Cmux Swift @Concurrent ✅ Passed PASS: PR only adds sync, MainActor/UI-bound layout code; no new nonisolated async work or @concurrent misuse, and existing async helpers are unchanged.
Cmux Swift File And Package Boundaries ✅ Passed Core row logic lives in the package reducer; the 433-line config type is a cohesive source-of-truth/persistence singleton, and oversized existing files only got small incidental edits.
Cmux Swiftpm Lockfiles ✅ Passed The only Xcode project diff is unrelated source-file wiring; no SwiftPM package-reference, .gitignore, workflow, or Package.resolved changes appear.
Cmux Swift Logging ✅ Passed No added print/debugPrint/dump/NSLog or ad hoc stdout/file logging found in the touched runtime code; the existing debug Logger use appears unchanged.
Cmux User-Facing Error Privacy ✅ Passed The added user-facing text is limited to generic row/toolbar labels; no alerts, error bodies, or recovery copy expose vendor/internal details.
Cmux Full Internationalization ✅ Passed New row UI strings use localized APIs, and the new catalog keys have both en and ja entries; no changed Swift UI text is hard-coded.
Cmux Swiftui State Layout ✅ Passed The SwiftUI changes use @Observable/@State, not ObservableObject/@published, and the List rows are snapshot-based with closures; no GeometryReader or render-time state writes found.
Cmux Architecture Rethink ✅ Passed Configuration stays the source of truth; the new notification is a documented UIKit bridge, with no sleeps/polling/extra owner introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed No NSWindow/WindowGroup/NSWindowController code was added; the diff only changes views, sheets, and terminal-pane layout/configuration.
Cmux Source Artifacts ✅ Passed No changed path matches the rule’s artifact/scratch patterns; the diff is source, tests, localization, docs/config, or allowed lock files.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: the new production APIs are row/layout features used by UIKit/SwiftUI callers; no new debug/*ForTesting seam was added in Sources.
Cmux No Ambient Global State ✅ Passed No new ambient global surface: changes add only static let constants and instance methods on owning types; the existing shared config/logger namespace types are incidental.
Title check ✅ Passed The title is concise and accurately summarizes the main change: configurable iOS toolbar rows.
Description check ✅ Passed Summary and validation are filled in, but the demo video, review trigger block, and checklist sections from the template are missing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-6090-ios-multiple-configurable-toolbar-rows

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@austinywang I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 26, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 5 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@austinywang

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 26, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 5 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@austinywang I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@austinywang

Copy link
Copy Markdown
Contributor Author

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@cubic-dev-ai

cubic-dev-ai Bot commented Jun 26, 2026

Copy link
Copy Markdown

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 241,260 of the 240,000 allowed lines of code this month. Reviews resume on 1 July 2026 (in 5 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews.

To help optimise your usage, you can tune cubic to get the most out of your usage limits:

Learn more →

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

@austinywang I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds multi-row iOS terminal toolbar support: configurable 1–3 rows persisted in a new v4 rows UserDefaults schema, migration from the existing v3 flat layout (single-row), per-row horizontal scroll in the accessory bar, and row-count/row-assignment controls in the shortcuts settings UI.

  • Persistence: displayOrder: [ToolbarItemID] is replaced by displayRows: [[ToolbarItemID]] with a v4 key; the reducer's load(savedRows:savedOrder:savedEnabled:rowCount:) orchestrates migration from v1–v3 flat keys and forward-appends unknown new actions to the last row.
  • Layout: TerminalAccessoryDockMetrics centralises the height maths (nubSize × rowCount + rowSpacing × (rowCount − 1) + bottomPadding); GhosttySurfaceView subscribes to TerminalAccessoryConfiguration.didChangeNotification and re-runs updateDockedToolbarVisibility (now @discardableResult) so layout + geometry sync only fire when the reserved height actually changes.
  • Settings UI: A Stepper for row count (1–3), per-item "Move to Row" pickers, and per-row drag-reorder sections; reorderAcrossRows (new reducer primitive) powers the flat agentChat-scope reorder without scrambling terminal row assignments.

Confidence Score: 5/5

Safe to merge — all layout, persistence, and migration paths look correct and are backed by new tests.

The migration from v3 flat order to v4 rows is correct and idempotent: v4 keys take precedence on every subsequent launch, and the reducer's load path normalises any edge cases (empty rows, overflow, forward-compat append). Height maths are centralised in TerminalAccessoryDockMetrics and shared between GhosttySurfaceView and TerminalInputTextView via the same computed path. The notification guard in GhosttySurfaceView correctly short-circuits layout work when only content (not height) changes. All new reducer behaviour is pinned by targeted unit tests. Concerns flagged in prior review rounds (dead guard branch, integer-index ForEach identity, cross-row agentChat reorder) are pre-existing design trade-offs that the author is aware of.

No files require special attention.

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Core reducer extended with row-aware Layout, load(savedRows:), reorder/reorderAcrossRows, move(inRow:), setRowCount, and a private split helper; all well-covered by the new reducer tests.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift Migrates persistence to v4 rows schema, wires new public API (setRowCount, moveItem, reorderItemsAcrossRows), and delegates cleanly to the reducer; migration path from v1–v3 is correct and runs at most once.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift Subscribes to didChangeNotification and guards layout calls behind an actual height change; persistentToolbarHeight converted to a computed property backed by TerminalAccessoryDockMetrics; deinit now removes all NSNotificationCenter observers.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Replaces single horizontal scroll view with a vertical UIStackView of per-row scroll views; accessoryRowsHeightConstraint drives row-stack height; populateAccessoryActions clears and rebuilds correctly.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryDockMetrics.swift New struct centralising height constants (nubSize, bottomPadding, rowSpacing) and computing rowsHeight / buttonRowHeight; static constants aliased in TerminalInputTextView for backward compat.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Adds Stepper, per-row sections with drag reorder, and per-item row pickers; the dead guard scope != .terminal branch remains from previous review threads.
ios/cmux/Resources/Localizable.xcstrings Five new keys for toolbar-rows UI; all carry both en and ja translations, matching the catalog's existing locale set.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User changes row count / reorders / toggles] --> B[TerminalAccessoryConfiguration.persistAndNotify]
    B --> C[NotificationCenter post didChangeNotification]
    C --> D[GhosttySurfaceView.handleAccessoryConfigurationChanged]
    C --> E[TerminalInputTextView.handleAccessoryConfigurationChanged]

    D --> F{updateDockedToolbarVisibility\nheight or visibility changed?}
    F -- No change --> G[return — no layout work]
    F -- Changed --> H[layoutRenderedTerminalForCurrentViewport\nsetNeedsGeometrySync\nsetNeedsLayout]
    H --> I[layoutBottomDock\nreposition toolbar frame]
    I --> J[layoutZoomOverlay]

    E --> K[populateAccessoryActions\nrebuild per-row scroll views]
    K --> L[updateAccessoryRowsHeight\nupdate constraint constant]
    K --> M[applyModifierPresentation]
    M --> N[setNeedsLayout / layoutIfNeeded]

    subgraph Persistence
        P1[cmux.terminal.toolbar.rows.v4]
        P2[cmux.terminal.toolbar.enabled.v4]
    end

    B --> Persistence

    subgraph Migration
        V3[v3 order key] --> MIG[reducer.load savedRows=nil]
        V2[v2 key] --> MIG
        V1[v1 int keys] --> MIG
        MIG --> WRITE[persist under v4 keys]
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[User changes row count / reorders / toggles] --> B[TerminalAccessoryConfiguration.persistAndNotify]
    B --> C[NotificationCenter post didChangeNotification]
    C --> D[GhosttySurfaceView.handleAccessoryConfigurationChanged]
    C --> E[TerminalInputTextView.handleAccessoryConfigurationChanged]

    D --> F{updateDockedToolbarVisibility\nheight or visibility changed?}
    F -- No change --> G[return — no layout work]
    F -- Changed --> H[layoutRenderedTerminalForCurrentViewport\nsetNeedsGeometrySync\nsetNeedsLayout]
    H --> I[layoutBottomDock\nreposition toolbar frame]
    I --> J[layoutZoomOverlay]

    E --> K[populateAccessoryActions\nrebuild per-row scroll views]
    K --> L[updateAccessoryRowsHeight\nupdate constraint constant]
    K --> M[applyModifierPresentation]
    M --> N[setNeedsLayout / layoutIfNeeded]

    subgraph Persistence
        P1[cmux.terminal.toolbar.rows.v4]
        P2[cmux.terminal.toolbar.enabled.v4]
    end

    B --> Persistence

    subgraph Migration
        V3[v3 order key] --> MIG[reducer.load savedRows=nil]
        V2[v2 key] --> MIG
        V1[v1 int keys] --> MIG
        MIG --> WRITE[persist under v4 keys]
    end
Loading

Reviews (27): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the iOS terminal toolbar from a single configurable row to 1–3 user-configurable rows, each scrolling independently. It introduces a v4 UserDefaults schema (rows + enabled arrays) with clean migration from v3/v2/v1 flat layouts, a row-aware pure reducer in CmuxMobileTerminalKit, and settings UI with a row-count Stepper and per-item "Move to Row" picker.

  • TerminalAccessoryLayoutReducer: New load(savedRows:savedEnabled:rowCount:), move(_:toRow:in:), move(from:to:inRow:in:), and setRowCount(_:in:) operations replace/augment the flat-order API; backward-compat overload retained.
  • TerminalInputTextView: Single UIScrollView + UIStackView replaced by a vertical UIStackView of per-row scroll views; height constraint captured and updated live on TerminalAccessoryConfiguration.didChangeNotification.
  • GhosttySurfaceView: Subscribes to didChangeNotification to re-run persistentToolbarHeight (now a computed property keyed to rowCount) and relayout the terminal grid; observer removed in deinit.

Confidence Score: 4/5

The change is safe to merge; the migration path is one-way and well-guarded, the notification chain is @MainActor-posted so all UIKit layout calls stay on the main thread, and the reducer is exercised by six new deterministic unit tests.

The overall implementation is solid: the v4 persistence keys are disjoint from v3, the reducer is pure and tested, and the notification-driven relayout in both TerminalInputTextView and GhosttySurfaceView follows the existing pattern correctly. The two flagged items — integer-index ForEach identity for row sections and duplicated split logic — are non-blocking quality concerns rather than correctness bugs.

TerminalShortcutsSettingsView.swift (ForEach index identity) and TerminalAccessoryLayoutReducer.swift (duplicated split helper).

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Adds row-count Stepper, per-section row headers, and per-item row-picker; index-keyed ForEach for the sections is a minor SwiftUI identity concern.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift Adds a notification observer for configuration changes that relays to existing layout methods; observer correctly removed in deinit, notification is posted on main thread by the @mainactor configuration.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift Introduces v4 row-based persistence with clean v3/v2/v1 migration; duplicates the split helper that also lives in the reducer.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Replaces the single-row scroll view with a vertical stack of per-row scroll views; height constraint is properly captured and updated on configuration-change notifications.
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Extends the pure reducer with row-count changes, in-row moves, and cross-row moves; duplicates the split helper that is also present in TerminalAccessoryConfiguration.
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalAccessoryLayoutReducerTests.swift Adds 6 new tests covering forward-compat append, empty row persistence, row count reduction/expansion, in-row move, and cross-row move; all look correct and complete.
ios/cmux/Resources/Localizable.xcstrings Adds 5 new strings (rows header, label, footer, move-picker label, row title format) with English and Japanese translations; both are the only locales in this catalog, so coverage is complete.
.github/swift-file-length-budget.tsv Bumps or trims line budgets for files touched by this PR; all deltas are small and directionally correct.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Settings as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant Defaults as UserDefaults
    participant NC as NotificationCenter
    participant TITV as TerminalInputTextView
    participant GSV as GhosttySurfaceView

    Settings->>Config: setRowCount(_:) / moveItem(_:toRow:) / moveItems(from:to:inRow:)
    Config->>Reducer: setRowCount / move(inRow:) / move(toRow:)
    Reducer-->>Config: Layout(rows:enabled:)
    Config->>Defaults: persist rows.v4 + enabled.v4
    Config->>NC: post didChangeNotification
    NC-->>TITV: handleAccessoryConfigurationChanged()
    TITV->>TITV: populateAccessoryActions() → updateAccessoryRowsHeight()
    NC-->>GSV: handleAccessoryConfigurationChanged()
    GSV->>GSV: updateDockedToolbarVisibility() + layoutBottomDock() + setNeedsGeometrySync()
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Settings as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant Defaults as UserDefaults
    participant NC as NotificationCenter
    participant TITV as TerminalInputTextView
    participant GSV as GhosttySurfaceView

    Settings->>Config: setRowCount(_:) / moveItem(_:toRow:) / moveItems(from:to:inRow:)
    Config->>Reducer: setRowCount / move(inRow:) / move(toRow:)
    Reducer-->>Config: Layout(rows:enabled:)
    Config->>Defaults: persist rows.v4 + enabled.v4
    Config->>NC: post didChangeNotification
    NC-->>TITV: handleAccessoryConfigurationChanged()
    TITV->>TITV: populateAccessoryActions() → updateAccessoryRowsHeight()
    NC-->>GSV: handleAccessoryConfigurationChanged()
    GSV->>GSV: updateDockedToolbarVisibility() + layoutBottomDock() + setNeedsGeometrySync()
Loading

Comments Outside Diff (2)

  1. Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift, line 203-218 (link)

    P2 Integer-index ForEach identity can cause stale section state

    ForEach(displayedItemRows.indices, id: \.self) keys sections by position rather than by stable content identity. When the user changes rowCount via the Stepper, SwiftUI recycles section views by index: the section view at index 0 stays bound to index 0, etc. Any implicit SwiftUI section state (e.g. internal UICollectionView scroll offsets that List preserves per-identity) will silently transfer to the wrong section when rows are added or removed. A stable identifier — for example a typed wrapper carrying the row index that is treated as opaque by SwiftUI — avoids this category of reuse mismatch entirely.

    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!

  2. Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift, line 1364-1377 (link)

    P2 Duplicated split helper between reducer and configuration layer

    TerminalAccessoryLayoutReducer.split(_:matchingRowLengthsOf:) and TerminalAccessoryConfiguration.split(_:matchingRowLengthsOf:) implement the same slicing logic. The reducer is a public Sendable value type in CmuxMobileTerminalKit while the configuration lives in CmuxMobileTerminal, so cross-package reuse requires making the helper at least internal on the reducer — but any divergence in the two copies risks subtle off-by-one disagreements on the "append overflow to the last row" edge case. Surfacing the helper as a public static on the reducer (or as a shared utility) gives the configuration layer a single authoritative implementation.

Reviews (2): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extends the iOS terminal toolbar to support 1–3 configurable shortcut rows, persisting them as a new v4 schema alongside a v1/v2/v3 migration path. A TerminalAccessoryLayoutReducer gains row-aware operations (row count change, per-row move, per-row reorder), and both the UIKit toolbar and the SwiftUI settings view are updated to render and manage the multi-row layout.

  • Adds TerminalAccessoryLayoutReducer operations: load(savedRows:savedEnabled:rowCount:), move(_:toRow:in:), move(from:to:inRow:in:), and setRowCount(_:in:), all covered by new tests.
  • TerminalInputTextView replaces the single UIScrollView/UIStackView pair with a vertical UIStackView of per-row scroll views, and updates height constraints dynamically when TerminalAccessoryConfiguration.didChangeNotification fires.
  • GhosttySurfaceView subscribes to didChangeNotification so the terminal grid reservation and layout respond immediately to row-count changes.

Confidence Score: 4/5

The change is safe to merge. The core reducer logic is pure and testable, the migration paths cover v1–v3 correctly, and all new user-facing strings are translated in both supported locales.

One helper (updateAccessoryRowsHeight) updates both a layout constraint constant and the toolbar's frame size directly — the direct mutation is redundant once Auto Layout runs and could leave a brief stale frame in certain layout orderings, though it is unlikely to be user-visible in practice. The dead guard scope != .terminal branch in the flat-move path is misleading but harmless.

TerminalInputTextView.swift — the updateAccessoryRowsHeight direct frame mutation alongside the constraint update is worth a second look.

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Core reducer gains row-aware load/move/setRowCount operations with correct edge-case handling (empty rows preserved, overflow merges, forward-compat appends to last row). New tests cover the key paths.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift v4 persistence schema correctly added; v1/v2/v3 migration paths each set savedRows=nil and let the reducer fall back to the single-row path. reorderItems now uses split+load to preserve multi-row structure, an improvement over the old single-row collapse.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Single scroll-row replaced by a vertical stack of per-row scroll views; height constraint and direct frame pre-set updated on config change. The accessoryLayoutDiagnostics debug property correctly updated to inspect the first row's scroll view.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift Subscribes to didChangeNotification to re-sync grid reservation when row count changes; persistentToolbarHeight now computed from shared.rowCount at call time. deinit now removes all selector observers, cleaning up both the new and pre-existing keyboard observers.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Row count Stepper and per-item row Picker added for terminal scope; non-terminal scope keeps the existing flat list. One dead guard branch remains but is harmless.
ios/cmux/Resources/Localizable.xcstrings Five new keys added with both en and ja translations, matching every locale already in the catalog.
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalAccessoryLayoutReducerTests.swift Six new tests cover savedRows forward-compat, empty-row persistence, row count reduce/expand, row-local move, and cross-row move with enabled-state preservation.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as User (Settings)
    participant TV as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant ND as NotificationCenter
    participant GSV as GhosttySurfaceView
    participant TIV as TerminalInputTextView

    User->>TV: Stepper: increment rowCount
    TV->>Config: setRowCount(newCount)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows: [...], enabled: ...)
    Config->>Config: persist() → UserDefaults v4 keys
    Config->>ND: post(didChangeNotification)
    ND-->>GSV: handleAccessoryConfigurationChanged()
    GSV->>GSV: updateDockedToolbarVisibility()
    GSV->>GSV: layoutBottomDock() [reads persistentToolbarHeight → rowCount]
    GSV->>GSV: layoutRenderedTerminalForCurrentViewport()
    GSV->>GSV: setNeedsGeometrySync() + setNeedsLayout()
    ND-->>TIV: handleAccessoryConfigurationChanged()
    TIV->>TIV: populateAccessoryActions() [rebuilds row stack views]
    TIV->>TIV: updateAccessoryRowsHeight(rowCount)
    TIV->>TIV: applyModifierPresentation()
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User as User (Settings)
    participant TV as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant ND as NotificationCenter
    participant GSV as GhosttySurfaceView
    participant TIV as TerminalInputTextView

    User->>TV: Stepper: increment rowCount
    TV->>Config: setRowCount(newCount)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows: [...], enabled: ...)
    Config->>Config: persist() → UserDefaults v4 keys
    Config->>ND: post(didChangeNotification)
    ND-->>GSV: handleAccessoryConfigurationChanged()
    GSV->>GSV: updateDockedToolbarVisibility()
    GSV->>GSV: layoutBottomDock() [reads persistentToolbarHeight → rowCount]
    GSV->>GSV: layoutRenderedTerminalForCurrentViewport()
    GSV->>GSV: setNeedsGeometrySync() + setNeedsLayout()
    ND-->>TIV: handleAccessoryConfigurationChanged()
    TIV->>TIV: populateAccessoryActions() [rebuilds row stack views]
    TIV->>TIV: updateAccessoryRowsHeight(rowCount)
    TIV->>TIV: applyModifierPresentation()
Loading

Comments Outside Diff (1)

  1. Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift, line 213-217 (link)

    P2 Stale dead branch — this guard can never fire. In the terminal scope, .onMove is now wired to moveDisplayedItems(from:to:inRow:) via the inline closure, so moveDisplayedItems(from:to:) is only ever called from the non-terminal scope's .onMove(perform:). The guard scope != .terminal condition is always true at that call site, making the else branch unreachable. Leaving it in place looks like an active safeguard but obscures that the flat configuration.moveItems(from:to:) call below it is the code that actually runs for non-terminal scope. Consider removing the guard so the intent is clear, or adding a comment explaining why the defensive check is kept.

    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!

Reviews (2): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds support for 1–3 configurable horizontal toolbar rows on iOS, stacking them above the keyboard so more shortcuts stay visible without horizontal scrolling. It introduces a v4 persistence schema (rows + enabled), auto-migrates all legacy flat layouts into a single row, and wires live layout updates through NotificationCenter so the terminal grid height adjusts immediately when the row count changes.

  • Reducer (TerminalAccessoryLayoutReducer): extended to row-aware load, per-row move, cross-row move, and setRowCount; all new paths covered by six new tests in CmuxMobileTerminalKit.
  • Configuration (TerminalAccessoryConfiguration): new v4 keys (rows.v4, enabled.v4) replace the old v3 order key; migration runs at most once per device, correctly clamping stored row counts and forward-compat-appending unknown items to the last row.
  • UIKit rendering (TerminalInputTextView, GhosttySurfaceView): persistentToolbarHeight promoted from a static constant to a live computed property keyed on shared.rowCount; the single scroll view + stack replaced with a vertical stack of per-row scroll views; deinit now removes all observers.
  • Settings UI (TerminalShortcutsSettingsView): row-count stepper and per-item row picker added; new user-facing strings localized in both en and ja.

Confidence Score: 4/5

Safe to merge; the multi-row feature, migration path, and live-layout wiring are well-implemented and tested.

The reducer logic is pure and fully tested, the v4 migration correctly handles all legacy schema versions, and the UIKit layout updates route through the same notification channel that already drives toolbar rebuilds. The two findings are both in the settings view: a now-unreachable terminal-scope branch in the flat-move helper, and positional integer IDs on the row-section ForEach that cause full section reconstruction on row-count changes. Neither affects correctness of the feature.

TerminalShortcutsSettingsView.swift — the dead terminal-scope branch in moveDisplayedItems(from:to:) and the integer-index row section identity warrant a second look.

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Pure reducer correctly extended to row-aware layout: load/merge/forward-compat, per-row move, cross-row move, and setRowCount are all well-specified and verified by new tests.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift v4 persistence schema cleanly introduced alongside renamed legacy keys; migration path (v3/v2/v1 → v4) runs at most once per device. clampedRowCount correctly guards out-of-range stored row counts.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Single-row scroll stack replaced with a vertical UIStackView of per-row UIScrollView+UIStackView pairs; row height constraint updated live via updateAccessoryRowsHeight on configuration-change notification.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift persistentToolbarHeight promoted from a static constant to a computed property reading shared.rowCount; new didChangeNotification observer triggers layout refresh. deinit now removes all observers correctly.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Row-count stepper and per-item row picker added for the terminal scope. Two minor issues: unreachable terminal-scope branch in the flat moveDisplayedItems overload, and positional integer IDs in the row section ForEach.
ios/cmux/Resources/Localizable.xcstrings Five new string keys added with both en and ja translations matching the existing locale support for this catalog.
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalAccessoryLayoutReducerTests.swift Six new tests cover forward-compat append, empty-row persistence, row-count reduction/increase, row-local move, and cross-row move with enabled-state preservation.
.github/swift-file-length-budget.tsv Budget counts updated to reflect file size changes; TerminalInputTextView.swift correctly increased by ~48 lines for the new row-stack implementation.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User as User (Settings)
    participant View as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant Defaults as UserDefaults
    participant NC as NotificationCenter
    participant TV as TerminalInputTextView
    participant SV as GhosttySurfaceView

    User->>View: tap stepper (+1 row)
    View->>Config: setRowCount(newCount)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows: [...], enabled: ...)
    Config->>Config: apply(layout) — update displayRows
    Config->>Defaults: persist rows.v4 + enabled.v4
    Config->>NC: post(didChangeNotification)
    NC->>TV: handleAccessoryConfigurationChanged()
    TV->>TV: populateAccessoryActions() — rebuild row stack
    TV->>TV: updateAccessoryRowsHeight(rowCount:)
    NC->>SV: handleAccessoryConfigurationChanged()
    SV->>SV: updateDockedToolbarVisibility()
    Note over SV: persistentToolbarHeight reads shared.rowCount
    SV->>SV: layoutBottomDock() + layoutRenderedTerminalForCurrentViewport()
    SV->>SV: setNeedsGeometrySync() + setNeedsLayout()
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User as User (Settings)
    participant View as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant Defaults as UserDefaults
    participant NC as NotificationCenter
    participant TV as TerminalInputTextView
    participant SV as GhosttySurfaceView

    User->>View: tap stepper (+1 row)
    View->>Config: setRowCount(newCount)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows: [...], enabled: ...)
    Config->>Config: apply(layout) — update displayRows
    Config->>Defaults: persist rows.v4 + enabled.v4
    Config->>NC: post(didChangeNotification)
    NC->>TV: handleAccessoryConfigurationChanged()
    TV->>TV: populateAccessoryActions() — rebuild row stack
    TV->>TV: updateAccessoryRowsHeight(rowCount:)
    NC->>SV: handleAccessoryConfigurationChanged()
    SV->>SV: updateDockedToolbarVisibility()
    Note over SV: persistentToolbarHeight reads shared.rowCount
    SV->>SV: layoutBottomDock() + layoutRenderedTerminalForCurrentViewport()
    SV->>SV: setNeedsGeometrySync() + setNeedsLayout()
Loading

Reviews (4): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds support for 1–3 configurable horizontal toolbar rows on iOS, migrating existing flat layouts (v3/v2/v1) into a single row under the new v4 rows+enabled persistence schema and updating the terminal grid to reserve the exact stacked height dynamically.

  • TerminalAccessoryLayoutReducer gains row-aware load, setRowCount, per-row move, and cross-row move operations as pure value-type transformations, with full test coverage.
  • TerminalAccessoryConfiguration adopts the row schema; persistentToolbarHeight in GhosttySurfaceView is promoted from a static let to a computed var, and a new notification observer triggers a live layout refresh when settings change.
  • TerminalShortcutsSettingsView adds a row-count stepper and per-item row picker for the terminal scope; all new UI strings are localized into both supported locales (en, ja).

Confidence Score: 4/5

The change is well-scoped to the iOS toolbar path and carries correct migration, height propagation, and live-update wiring. The single finding is a latent maintenance trap rather than a current defect.

The reducer, migration, and notification-driven layout refresh are all correctly implemented and tested. The only concern is a dead guard scope != .terminal branch in moveDisplayedItems(from:to:) whose unreachable body calls the flat row-merging moveItems(from:to:). It cannot be triggered by any current code path, but if a future change accidentally connects this overload to a terminal onMove it would silently flatten multi-row layouts.

Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift — dead terminal guard in moveDisplayedItems(from:to:) should be removed or documented.

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Core row-aware reducer; pure value-type transformations. New load/setRowCount/move operations are correct, forward-compat append lands on last row, overflow merge preserves all IDs, tests cover every new code path.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift v4 migration correctly chains from v3/v2/v1 flat layouts into one row; persist writes v4 keys so migration runs once; loadRows handles empty-array edge case via defaultOrder fallback.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Single scrollable row replaced with a vertical UIStackView of per-row UIScrollViews; height constraint updated via handleAccessoryConfigurationChanged; accessoryRowsHeightConstraint is correctly stored and updated.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift persistentToolbarHeight promoted from static let to computed var reading shared rowCount; new notification observer triggers full layout refresh on config change; deinit removes all observers.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Adds per-row sections, row-count stepper, and row-assignment picker for terminal scope; non-terminal scope unchanged. Dead guard in moveDisplayedItems(from:to:) is a maintenance trap.
ios/cmux/Resources/Localizable.xcstrings Five new string keys (rows label/header/footer/movePicker/rowTitleFormat) with both en and ja translations; covers all locales the catalog supports.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant SettingsView as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant NC as NotificationCenter
    participant TextInput as TerminalInputTextView
    participant Surface as GhosttySurfaceView

    User->>SettingsView: Adjust row stepper
    SettingsView->>Config: setRowCount(n)
    Config->>Reducer: setRowCount(n, in: layout)
    Reducer-->>Config: Layout(rows: merged/expanded)
    Config->>Config: apply + persist (v4 keys)
    Config->>NC: post didChangeNotification

    NC->>TextInput: handleAccessoryConfigurationChanged
    TextInput->>TextInput: populateAccessoryActions()
    TextInput->>TextInput: updateAccessoryRowsHeight(rowCount)
    TextInput->>TextInput: setNeedsLayout

    NC->>Surface: handleAccessoryConfigurationChanged
    Surface->>Surface: updateDockedToolbarVisibility()
    Note right of Surface: persistentToolbarHeight now reads new rowCount
    Surface->>Surface: layoutBottomDock()
    Surface->>Surface: layoutRenderedTerminalForCurrentViewport()
    Surface->>Surface: setNeedsGeometrySync + setNeedsLayout
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant SettingsView as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant NC as NotificationCenter
    participant TextInput as TerminalInputTextView
    participant Surface as GhosttySurfaceView

    User->>SettingsView: Adjust row stepper
    SettingsView->>Config: setRowCount(n)
    Config->>Reducer: setRowCount(n, in: layout)
    Reducer-->>Config: Layout(rows: merged/expanded)
    Config->>Config: apply + persist (v4 keys)
    Config->>NC: post didChangeNotification

    NC->>TextInput: handleAccessoryConfigurationChanged
    TextInput->>TextInput: populateAccessoryActions()
    TextInput->>TextInput: updateAccessoryRowsHeight(rowCount)
    TextInput->>TextInput: setNeedsLayout

    NC->>Surface: handleAccessoryConfigurationChanged
    Surface->>Surface: updateDockedToolbarVisibility()
    Note right of Surface: persistentToolbarHeight now reads new rowCount
    Surface->>Surface: layoutBottomDock()
    Surface->>Surface: layoutRenderedTerminalForCurrentViewport()
    Surface->>Surface: setNeedsGeometrySync + setNeedsLayout
Loading

Comments Outside Diff (1)

  1. Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift, line 213-217 (link)

    Dead terminal-scope guard that merges rows on accidental activation

    The guard scope != .terminal else { configuration.moveItems(from:to:); return } branch is now unreachable: moveDisplayedItems(from:to:) is only wired to the non-terminal .onMove handler, while all terminal reordering goes through the new moveDisplayedItems(from:to:inRow:) overload. If a future change accidentally connects this overload to a terminal ForEach, the dead branch calls the flat configuration.moveItems(from:to:), which operates on the entire flattened order and re-splits by current row lengths — silently collapsing the user's multi-row arrangement without any warning.

Reviews (4): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Migrates the iOS terminal toolbar from a single flat configurable row to 1–3 user-configurable stacked rows, persisted under a new v4 schema, with automatic migration of v1–v3 flat layouts into a single row. The terminal grid and dock geometry are updated live whenever row count changes via a NotificationCenter observer (also fixing a pre-existing dangling-observer gap in GhosttySurfaceView.deinit).

  • TerminalAccessoryLayoutReducer gains pure load(savedRows:savedEnabled:rowCount:), move(inRow:), move(toRow:), and setRowCount operations; the existing flat-order API is preserved as a backward-compatible shim, with six new tests covering all new operations.
  • TerminalInputTextView replaces the single horizontal UIScrollView+UIStackView with a vertical UIStackView of per-row scroll views; the buttonRows layout guide height constraint is updated in place whenever the row count changes.
  • TerminalShortcutsSettingsView adds a row-count Stepper and per-item row Picker for the terminal scope; the agentChat scope continues to use a flat list with its existing index-translating reorder path, though this path now calls reorderItems which distributes the flat order back into rows via positional split.

Confidence Score: 4/5

Safe to merge; the change is well-contained, migration logic is correct, and UIKit layout paths are updated consistently.

The core row-aware reducer, v4 migration, and UIKit layout updates are all correct and well-tested. The one behavioral concern is that the agentChat scope flat-reorder path calls reorderItems, which now uses a positional split to reassign rows — this can silently shift terminal-only items between rows when a user with a multi-row terminal config reorders items in the agentChat settings view. This only matters for users who have configured more than one toolbar row, which is a new feature not yet in production, so the real-world risk is low for the initial release.

Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift — specifically the reorderItems method and its interaction with the agentChat flat-reorder path

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Core row-aware reducer: new load, setRowCount, move(inRow:), and move(toRow:) operations are well-structured pure functions; backward-compatible overload bridges v1-v3 flat-order migration correctly; split helper correctly distributes flat orders by row lengths
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift v4 schema with rows + enabled keys; v3/v2/v1 migration paths are correct; reorderItems now uses split to maintain row lengths, but this can silently shift items between rows when called from the agentChat flat-reorder path on a multi-row config
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Refactors single scrollable row into a vertical UIStackView of per-row scrollable action rows; constraint management, height updates, and rebuild logic are correct; accessoryActionStackViews array replaces single weak ref properly
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift Converts persistentToolbarHeight from a static let to a live computed property reading rowCount; adds NotificationCenter observer for configuration changes with matching removeObserver in deinit (fixing a pre-existing dangling-observer gap)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Adds row-count Stepper and per-row sections with per-item row Picker for terminal scope; agentChat scope continues using flat list with correct index-translation reorder; rowBinding fallback is safe because terminal scope.includes returns true for all items
Packages/iOS/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalAccessoryLayoutReducerTests.swift Six new tests cover saved-row forward compat, empty row preservation, row count reduction/expansion, row-local move, and move-to-row; good coverage of the new reducer operations
ios/cmux/Resources/Localizable.xcstrings Five new string keys (rows.footer, rows.header, rows.label, rows.movePicker, rows.rowTitleFormat) each translated into all catalog locales (en + ja); format strings match between locales

Comments Outside Diff (1)

  1. Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift, line 582-590 (link)

    P2 agentChat scope reorder silently shifts items between terminal rows

    reorderItems is called only from the agentChat flat-list path. It now reconstructs rows via Self.split(orderedIDs, matchingRowLengthsOf: displayRows), which distributes the new flat order by current row lengths. Because the agentChat filter excludes most terminal-only items (modifiers, zoom, arrows), moving a visible item like Paste from one position to another can change where in the flat order adjacent terminal-only items land, pushing them across a row boundary. For example: with displayRows = [[Ctrl, Paste, Esc], [Tab, Up, Down]], moving Paste to after Tab produces flat order [Ctrl, Esc, Tab, Paste, Up, Down], which splits into [[Ctrl, Esc, Tab], [Paste, Up, Down]] — Tab silently moved from row 1 to row 0. This only affects users who have configured rowCount > 1 alongside the agentChat shortcut settings, but the row layout change is invisible to the user making the edit.

Reviews (4): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR migrates the iOS terminal toolbar from a single flat row to 1–3 user-configurable horizontal rows, each scrolling independently. The terminal grid height reservation is updated live when row count changes, and existing flat layouts are migrated to a single-row v4 schema on first launch.

  • Reducer extended with load(savedRows:savedEnabled:rowCount:), move(from:to:inRow:), move(_:toRow:), and setRowCount operations; six new tests cover edge cases.
  • Persistence bumped to v4 schema (rows.v4 + enabled.v4); v3/v2/v1 keys read once for migration then left untouched.
  • UIKit toolbar replaces the single scroll view with a vertical stack of per-row scroll views; GhosttySurfaceView subscribes to didChangeNotification to relayout the grid live.

Confidence Score: 4/5

Safe to merge. Migration logic is sound, tests cover key reducer operations, localization is complete for both supported locales, and UIKit layout changes correctly update both the constraint constant and hosting frame.

The v4 schema migration is carefully gated and runs at most once. The row-aware reducer is pure and well-tested. Two minor issues worth follow-up: the new GhosttySurfaceView notification handler fires full geometry syncs on every configuration change (not only row-count changes), and the onMove offset forwarding relies on an undocumented invariant that terminal scope includes all items.

GhosttySurfaceView.swift (unconditional geometry sync in the new notification handler) and TerminalShortcutsSettingsView.swift (onMove offset forwarding assumption).

Important Files Changed

Filename Overview
Packages/iOS/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalAccessoryLayoutReducer.swift Pure reducer extended with row-aware layout operations; well-tested with six new cases covering edge cases.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalAccessoryConfiguration.swift V4 schema migration correct; legacy keys read-only after migration; new row APIs consistent.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift Single scroll view replaced with vertical stack of per-row scroll views; height constraint and placeholder frame updated on configuration change.
Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift New notification observer triggers unconditional geometry sync on all config changes; deinit correctly removes all observers.
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift Row Stepper and per-item Picker added; onMove offset forwarding relies on undocumented terminal-scope invariant.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant Settings as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant InputTV as TerminalInputTextView
    participant Surface as GhosttySurfaceView

    User->>Settings: change row count (Stepper)
    Settings->>Config: setRowCount(_:)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows:enabled:)
    Config->>Config: apply + persist (v4 keys)
    Config->>Config: post didChangeNotification

    Config-->>InputTV: handleAccessoryConfigurationChanged
    InputTV->>InputTV: populateAccessoryActions()
    InputTV->>InputTV: updateAccessoryRowsHeight(rowCount:)

    Config-->>Surface: handleAccessoryConfigurationChanged
    Surface->>Surface: updateDockedToolbarVisibility()
    Surface->>Surface: layoutBottomDock()
    Surface->>Surface: layoutRenderedTerminalForCurrentViewport()
    Surface->>Surface: setNeedsGeometrySync()
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant Settings as TerminalShortcutsSettingsView
    participant Config as TerminalAccessoryConfiguration
    participant Reducer as TerminalAccessoryLayoutReducer
    participant InputTV as TerminalInputTextView
    participant Surface as GhosttySurfaceView

    User->>Settings: change row count (Stepper)
    Settings->>Config: setRowCount(_:)
    Config->>Reducer: setRowCount(_:in:)
    Reducer-->>Config: Layout(rows:enabled:)
    Config->>Config: apply + persist (v4 keys)
    Config->>Config: post didChangeNotification

    Config-->>InputTV: handleAccessoryConfigurationChanged
    InputTV->>InputTV: populateAccessoryActions()
    InputTV->>InputTV: updateAccessoryRowsHeight(rowCount:)

    Config-->>Surface: handleAccessoryConfigurationChanged
    Surface->>Surface: updateDockedToolbarVisibility()
    Surface->>Surface: layoutBottomDock()
    Surface->>Surface: layoutRenderedTerminalForCurrentViewport()
    Surface->>Surface: setNeedsGeometrySync()
Loading

Comments Outside Diff (2)

  1. Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift, line 1373-1380 (link)

    P2 Unconditional geometry sync on every configuration change

    handleAccessoryConfigurationChanged calls layoutRenderedTerminalForCurrentViewport(), layoutZoomOverlay(), and setNeedsGeometrySync() for every configuration change — including button enable/disable toggles and per-row reorders that leave persistentToolbarHeight unchanged. The updateDockedToolbarVisibility() guard correctly early-returns when the dock height hasn't changed, but the three subsequent calls happen unconditionally. When the user toggles a button in settings, this fires a geometry sync and terminal viewport relayout even though the grid dimensions are identical. Scoping those calls behind an actual height-change check would avoid spurious syncs during non-row-count edits.

    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!

  2. Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swift, line 318-320 (link)

    P2 onMove offset mismatch if scope filtering ever applies to rows

    moveDisplayedItems(from:to:inRow:) forwards offsets and destination from SwiftUI's onMove directly to configuration.moveItems(from:to:inRow:), which operates on the raw displayRows[rowIndex] (unfiltered). If scope.includes ever returns false for any item on the .terminal scope, the displayed-array indices no longer match the backing-array indices, causing items to move to the wrong positions. Today .terminal.includes always returns true, so this is safe — but the function lacks a guard or comment documenting that invariant.

Reviews (7): Last reviewed commit: "Merge origin/main" | Re-trigger Greptile

austinywang and others added 2 commits June 29, 2026 16:30
…le-configurable-toolbar-rows

# Conflicts:
#	.github/swift-file-length-budget.tsv
The agent-chat Shared Shortcuts flat list reorders scoped items strictly within
their current terminal rows. A flat reorder cannot move one item across a
fixed-length row boundary without changing row lengths or cascading another item
into a different row (the silent row scramble previously fixed for this path), so
cross-row moves are intentionally routed through the per-item row picker. Add a
call-site comment so the deliberate snap-back behavior is clear to future readers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Conflicts resolved:
- Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swift:
  Adopt origin/main's TerminalViewportCoordinator refactor (bottomDockFrames now
  delegates to viewportSnapshot). Preserve the multiple-configurable-toolbar-rows
  feature by feeding the instance persistentToolbarHeight (driven by rowCount) into
  the snapshot's toolbarFrameHeight, replacing origin/main's deleted single-row
  static Self.persistentToolbarHeight. The coordinator's toolbar-frame math is a
  faithful generalization of the prior inline computation, so behavior is preserved.
- .github/swift-file-length-budget.tsv: regenerated via
  scripts/swift_file_length_budget.py --write-budget (not hand-edited).
Second sync with origin/main (6 commits, incl. #7116 iOS toolbar glass/lifecycle).
GhosttySurfaceView.swift auto-merged cleanly on top of the prior coordinator-based
resolution; multi-row invariants verified intact (instance persistentToolbarHeight
flows into the viewport snapshot's toolbarFrameHeight; no stale Self. static
reference). Only conflict was .github/swift-file-length-budget.tsv, regenerated via
scripts/swift_file_length_budget.py --write-budget (not hand-edited).
autoreview flagged that increasing the iOS toolbar row count appends empty
rows at the bottom, leaving existing shortcuts on the upper row. This is
intentional. Rows are numbered top-to-bottom ("Row 1"..."Row N") in both
the settings UI (Row sections + per-item "Move to Row" picker) and the
toolbar (Row 1 at top, Row N nearest the keyboard with the fixed
HIDE/customize controls). Growth appends new empty rows AFTER the existing
ones so each existing row keeps its number and position rather than being
renumbered on every count change; users populate the new rows explicitly
via the row picker and within-row drag.

Prepending / bottom-anchoring instead would push the user's existing
shortcuts to a higher-numbered row on every increase, contradicting the
stable "Row 1...N" numbering, and would break the behavior pinned by
TerminalAccessoryLayoutReducerTests (setRowCount grow -> [[...],[],[]]).

Doc-only change: expands the setRowCount doc comment and adds an inline
note at the append site. No behavior, API, or test change.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 10 files

Re-trigger cubic

…le-configurable-toolbar-rows

# Conflicts:
#	.github/swift-file-length-budget.tsv
…le-configurable-toolbar-rows

# Conflicts:
#	.github/swift-file-length-budget.tsv
@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview – cmux — f900bc8e Deployed Jul 5, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS: multiple configurable toolbar rows

3 participants