Skip to content

Configurable placement for new surfaces - #3898

Closed
austinywang wants to merge 16 commits into
mainfrom
issue-3760-new-surface-placement
Closed

austinywang wants to merge 16 commits into
mainfrom
issue-3760-new-surface-placement

Conversation

@austinywang

@austinywang austinywang commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add app.newSurfacePlacement with top, afterCurrent, and end support in schema, settings UI, cmux.json parsing, and docs example.
  • Route user-created terminal, browser, markdown, and file-preview surfaces through one Workspace placement decision.
  • Keep restore/config/internal repair flows on explicit append placement so saved ordering and scaffolding are not affected by the user preference.
  • Ensure socket tab actions use the same explicit target-index creation path as app context menus, avoiding create-then-reorder placement repairs.

Fixes #3760

Testing

  • git diff --check
  • jq empty Resources/Localizable.xcstrings
  • jq empty web/data/cmux.schema.json

Local tests/build were not run per task instruction; CI will run tests.


Note

Medium Risk
Changes core tab/surface creation and ordering logic across terminal/browser/markdown/file-preview surfaces, which could affect tab ordering and pinning behaviors if edge cases are missed, but is localized and covered by new unit tests.

Overview
Adds a new user/config-managed setting, app.newSurfacePlacement, to control where newly created surfaces (tabs) are inserted within a pane (top, afterCurrent, end), including UI picker, settings search aliases, schema/docs updates, and full localization strings.

Refactors surface creation so terminal/browser/markdown/file-preview surfaces apply a single placement decision at creation time (optionally via explicit targetIndex/placementOverride), ensuring restoration/internal repair flows always append and that socket/context-menu “to right” actions behave consistently. Adds unit tests covering placement parsing and insertion/index behavior, including pinned-tab interactions and browser profile inheritance for anchored actions.

Reviewed by Cursor Bugbot for commit eaef6e6. Bugbot is set up for automated code reviews on this repo. Configure here.

Surface creation had no cmux-owned insertion policy, so user-facing entry points inherited Bonsplit's implicit after-current behavior. Add app.newSurfacePlacement as the sibling to workspace placement and route terminal, browser, markdown, and file-preview surface creation through one Workspace placement decision while preserving explicit restore/config ordering.

Constraint: Do not run local tests or xcodebuild for this task; CI owns test execution.

Constraint: Existing Bonsplit createTab only exposes current/end behavior, so cmux applies start/end policies as a post-create reorder before focus reconciliation.

Rejected: Configure Bonsplit globally | it cannot express top/start and would affect restoration/configuration paths that need explicit ordering.

Confidence: medium

Scope-risk: moderate

Directive: Keep user-initiated surface creation on the shared Workspace placement path; pass explicit placement overrides for restore/config flows that must preserve stored order.

Tested: git diff --check; jq empty Resources/Localizable.xcstrings; jq empty web/data/cmux.schema.json

Not-tested: Local unit/UI tests and local app build per user instruction; CI pending.
@vercel

vercel Bot commented May 12, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Canceled Canceled May 19, 2026 8:09am
cmux-staging Building Building Preview, Comment May 19, 2026 8:09am

@coderabbitai

coderabbitai Bot commented May 12, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR implements app.newSurfacePlacement setting to control where new tabs are inserted within a pane (top/afterCurrent/end), extending surface-creation APIs with placement overrides and applying logic across keyboard shortcuts, context menus, and restoration paths.

Changes

Configurable new surface placement

Layer / File(s) Summary
Placement model and insertion logic
Sources/TabManager.swift
NewSurfacePlacement enum with localized metadata and SurfacePlacementSettings that persists the chosen placement and computes insertion indices based on pinned surfaces and selection state.
Settings schema, parsing, and defaults
Sources/CmuxSettingsJSONPathSupport.swift, Sources/KeyboardShortcutSettingsFileStore+Template.swift, Sources/KeyboardShortcutSettingsFileStore.swift, web/data/cmux.schema.json
New app.newSurfacePlacement key added to cmux.json parsing and template defaults; JSON schema field added for validation.
Localization and search discoverability
Resources/Localizable.xcstrings, Sources/SettingsNavigation.swift, Sources/SettingsSearchAliases.swift
Localization strings added for UI display and search; settings search index and navigation anchors extended to reference the new placement option.
Settings UI and reset
Sources/cmuxApp.swift
SettingsView adds @AppStorage binding and picker row for NewSurfacePlacement selection; reset-all resets to default.
Surface creation APIs with placement logic
Sources/Workspace.swift
Four surface-creation APIs extended with placementOverride parameter; pre-creation selection state captured and applyNewSurfacePlacement helper reorders newly created tabs based on setting and override.
Restoration paths and context menu refactoring
Sources/Workspace.swift
Session/custom layout restoration paths pass placementOverride: .end for deterministic insertion; context menu "new tab to right" actions refactored to honor current SurfacePlacementSettings via new createTerminalFromContextMenu and createBrowserFromContextMenu helpers.
Unit tests and documentation
cmuxTests/WorkspaceUnitTests.swift, web/app/[locale]/docs/configuration/page.tsx
SurfacePlacementSettingsTests validate persistence and insertion logic; WorkspaceSurfaceCreationPlacementTests verify ordering and focus behavior across placement modes; documentation example updated.

Sequence Diagram

sequenceDiagram
  participant User as User action
  participant Workspace as Workspace
  participant SelectionCapture as Selection<br/>capture
  participant CreateTab as CreateTab
  participant PlacementHelper as applyNew<br/>SurfacePlacement
  participant Reorder as reorderTab
  User->>Workspace: newTerminalSurface<br/>(placementOverride)
  Workspace->>SelectionCapture: capture<br/>selectionBeforeCreation
  Workspace->>CreateTab: createTab
  CreateTab->>PlacementHelper: call with<br/>placement state
  PlacementHelper->>PlacementHelper: compute insertionIndex<br/>via SurfacePlacementSettings
  PlacementHelper->>Reorder: reorder tab<br/>to computed index
  Reorder-->>User: tab positioned
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • manaflow-ai/cmux#3381: Both PRs modify TabManager and Workspace to change how new/detached surfaces are inserted and focused (adding insertion/placement logic and focus-intent plumbing).
  • manaflow-ai/cmux#3294: Both PRs extend the settings search/index and localization layers (SettingsSearchAliases, SettingsNavigation, and Resources/Localizable.xcstrings) with new setting entries and search aliases.
  • manaflow-ai/cmux#3430: Both PRs propagate insertion/placement overrides through Workspace and TabManager APIs to control where new surfaces are inserted.

🐰 A picky rabbit hops 'round the pane,
Arranging new tabs in a tidy lane,
afterCurrent? Or end? Or the top?
Now it's your choice—the placement won't flop! 🎯


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Cmux Architecture Rethink ❓ Inconclusive No result was produced after verification. Marking as INCONCLUSIVE. Re-run the check or adjust instructions to produce a final result.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding configurable placement options for new surfaces in panes.
Linked Issues check ✅ Passed All core requirements from issue #3760 are met: configurable app.newSurfacePlacement with top/afterCurrent/end values, applied across terminal/browser/markdown/file-preview creation, schema/UI/parsing implementation, and unified placement logic.
Out of Scope Changes check ✅ Passed All changes directly support the configurable placement feature: localization, schema, UI, parsing, workspace logic, tests, and documentation are all scoped to implementing issue #3760.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed Introduces pure value enums mirroring existing patterns. Used only in MainActor contexts. No reference types or background access violations introduced.
Cmux Swift Blocking Runtime ✅ Passed No blocking primitives detected in the PR. Feature adds configuration and deterministic tab reordering logic only.
Cmux No Hacky Sleeps ✅ Passed No sleep, timer, delay, polling, or retry patterns found. Non-Swift changes are static: TypeScript documentation example string (+1 line) and JSON schema definitions (+6 lines). No runtime code added.
Cmux Swift Concurrency ✅ Passed New code is fully synchronous with no concurrency issues. Adds placement enums, synchronous methods, and @AppStorage UI. No DispatchQueue.global, Combine @Published, or fire-and-forget Tasks.
Cmux Swift @Concurrent ✅ Passed All Swift changes in PR are purely synchronous. No async functions were added, no @concurrent annotations were used or required. No violations of Swift concurrent annotation rules detected.
Cmux Swift File And Package Boundaries ✅ Passed All Swift files pass boundary checks. No file exceeded 250-line growth cap. New code follows existing patterns. Pure logic isolated and independently tested. No inappropriate responsibility mixing.
Cmux Swift Logging ✅ Passed No violations found. No new print/debugPrint/dump/NSLog statements added. The logInvalid call follows pre-existing infrastructure patterns.
Cmux Swiftui State Layout ✅ Passed No SwiftUI state violations found. Uses @AppStorage for settings, simple enums, computed properties, and standard ForEach patterns without store reference issues.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR does not add or materially change any standalone cmux-owned windows. It only adds configuration enums, settings storage, and a UI picker row within the existing Settings view.
Description check ✅ Passed The pull request description comprehensively addresses all required template sections with clear context and testing methodology.
✨ 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-3760-new-surface-placement

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 and usage tips.

Merge origin/main after opening the PR so CI evaluates the surface-placement changes against the current base branch rather than stale app and IME test code.

Constraint: iterate-pr requires syncing the PR branch with the base branch before CI iteration.

Rejected: Rebase after publishing | preserving the opened PR branch history is less disruptive for review and CI.

Confidence: high

Scope-risk: moderate

Directive: Do not revert the merged base changes; they are upstream main updates required for current CI.

Tested: git merge origin/main completed without conflicts.

Not-tested: Local tests/build per user instruction.
@greptile-apps

greptile-apps Bot commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a configurable app.newSurfacePlacement setting (top, afterCurrent, end) and routes all user-initiated surface creation (terminal, browser, markdown, file preview) through a single applyNewSurfacePlacement placement decision, while restore, scaffold, and repair flows are pinned to explicit .end to preserve saved ordering.

  • Settings plumbing: new NewSurfacePlacement enum and SurfacePlacementSettings in TabManager.swift, JSON parsing in KeyboardShortcutSettingsFileStore, schema entry, template default, settings-search wiring, and a Settings UI picker.
  • Placement centralisation: all four surface-creation methods gain targetIndex/placementOverride params; context-menu "to right" actions use contextMenuSurfaceTargetIndex; socket explicit-anchor actions pass a pre-computed targetIndex directly.
  • Localisation: all eight new string-catalog keys carry translations for all 19 locales supported by the parallel newWorkspacePlacement strings.

Confidence Score: 5/5

Safe to merge — placement changes are well-scoped, restore/repair flows are explicitly pinned to append, and the new logic is covered by both unit and end-to-end ordering tests.

All four surface-creation paths now use a single applyNewSurfacePlacement function or an explicit targetIndex, eliminating the old create-then-reorder pattern. Restore and scaffold paths are guarded with explicit .end overrides so saved workspace ordering cannot be disrupted. Localization is complete across all 19 supported locales. The two flagged items are minor API-contract notes that do not affect current behavior.

Sources/Workspace.swift — the insertionIndexToRight access-level promotion and the silent targetIndex/placementOverride precedence in newBrowserSurface are worth a second look, though neither affects current behavior.

Important Files Changed

Filename Overview
Resources/Localizable.xcstrings Adds all 8 new surface-placement string keys with translations for all 19 supported locales — fully matching the coverage of the parallel workspace-placement strings.
Sources/TabManager.swift Adds NewSurfacePlacement enum (top/afterCurrent/end) with localized display names/descriptions, and SurfacePlacementSettings with insertionIndex pure function; handles pinned-tab offsets, nil selection, and boundary clamping correctly.
Sources/Workspace.swift Core placement logic: adds applyNewSurfacePlacement, surfaceSelectionBeforeCreation, contextMenuSurfaceTargetIndex; all four surface-creation methods gain targetIndex/placementOverride params; restore/repair paths explicitly use .end. insertionIndexToRight promoted from private to internal access for use by TerminalController.
Sources/TerminalController.swift Socket new_terminal_right / new_browser_right actions now call workspace.insertionIndexToRight and pass targetIndex directly, eliminating the post-create reorder. Placeholder creation uses explicit placementOverride: .end.
Sources/cmuxApp.swift Adds @AppStorage binding and SettingsPickerRow for app.newSurfacePlacement, wired identically to the existing workspace placement picker; reset path updated accordingly.
cmuxTests/WorkspaceUnitTests.swift Adds SurfacePlacementSettingsTests and WorkspaceSurfaceCreationPlacementTests covering insertionIndex logic and end-to-end ordering for all three placements, context-menu, and socket paths.
Sources/KeyboardShortcutSettingsFileStore.swift Parses app.newSurfacePlacement from settings JSON with validation, mapping to SurfacePlacementSettings.placementKey in the managed defaults snapshot.
web/data/cmux.schema.json Adds newSurfacePlacement property to the app schema with correct enum values and default.

Reviews (10): Last reviewed commit: "fix: localize surface placement strings" | Re-trigger Greptile

@austinywang

Copy link
Copy Markdown
Contributor Author

Addressed Greptile's double-reorder feedback in 9eb7a0c. Explicit to-right creation now passes a target index into the surface creation path, so terminal/browser/duplicate-browser creation uses either an explicit target placement or the configured global placement, not both; the top-placement test now uses the helper lifecycle directly.

Comment thread Sources/Workspace.swift Outdated

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit faff724. Configure here.

Comment thread Sources/Workspace.swift Outdated

@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 13 files

Re-trigger cubic

Comment thread Resources/Localizable.xcstrings

This branch was successfully deployed

1 active deployment
Preview – cmux — eaef6e60 Deployed May 19, 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.

Configurable placement for new surfaces (tabs) in a pane

3 participants