Skip to content

Default new workspace placement to end - #3293

Closed
austinywang wants to merge 9 commits into
mainfrom
issue-3291-default-placement-end
Closed

austinywang wants to merge 9 commits into
mainfrom
issue-3291-default-placement-end

Conversation

@austinywang

@austinywang austinywang commented Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #3291.

Changes the default new workspace placement from After Current to End so new Cmd+N workspaces append to the end when the user has not explicitly configured newWorkspacePlacement.

What changed:

  • WorkspacePlacementSettings.defaultPlacement now defaults to .end.
  • The settings-file template continues to read from WorkspacePlacementSettings.defaultPlacement, keeping the generated config default in sync.
  • The public config schema default for app.newWorkspacePlacement is now end.
  • Unit expectations now assert unset and invalid stored values fall back to .end, while explicit stored values for all placement modes still win.

Verification

  • git diff --check
  • Local tests not run per task instruction.

Summary by CodeRabbit

  • Chores

    • Changed the default placement for newly created workspaces from "after current" to "end", adjusting insertion behavior.
  • Tests

    • Expanded test coverage for workspace placement to validate all placement options, ensuring consistent behavior across every placement choice.
  • Documentation

    • Updated the configuration docs example to reflect the new default placement value ("end").

Note

Low Risk
Low risk: a small default-behavior tweak with synchronized schema/docs and updated unit coverage; only affects users who haven’t explicitly set newWorkspacePlacement.

Overview
Defaults newWorkspacePlacement to end (instead of afterCurrent) so newly created workspaces append to the bottom when the setting is unset or invalid.

Updates unit tests to expect the new fallback behavior and to validate all NewWorkspacePlacement stored values, and syncs the web config schema and docs example to show end as the default.

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

The new workspace placement default is changing for Cmd+N behavior, so this captures the intended default and the append insertion contract before the implementation changes.

Constraint: Local tests were explicitly disallowed for this task

Confidence: high

Scope-risk: narrow

Tested: Not run; test-only commit is intentionally red against current main because defaultPlacement is still .afterCurrent

Not-tested: Local test execution per user instruction
Cmd+N should match the expected append behavior from issue #3291 and the related #3238 discussion. The workspace placement default now resolves to end, while explicit user settings continue to win through UserDefaults and the managed settings template stays synced through WorkspacePlacementSettings.defaultPlacement.

Constraint: Preserve explicit stored newWorkspacePlacement values

Rejected: Duplicate an "end" literal in the settings-file template | using WorkspacePlacementSettings.defaultPlacement keeps managed config defaults synced with the app default

Confidence: high

Scope-risk: narrow

Tested: git diff --check

Not-tested: Local tests per user instruction; app build deferred to required final reload command
@vercel

vercel Bot commented Apr 29, 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 May 12, 2026 8:55pm
cmux-staging Building Building Preview, Comment May 12, 2026 8:55pm

@coderabbitai

coderabbitai Bot commented Apr 29, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Default new-workspace placement fallback changed from NewWorkspacePlacement.afterCurrent to NewWorkspacePlacement.end. The workspace-placement unit test was expanded to iterate all NewWorkspacePlacement cases stored in user defaults and validate resolution for each. The documentation example comment was updated to show "end".

Changes

Default placement behavior

Layer / File(s) Summary
Default placement constant
Sources/TabManager.swift
Changed WorkspacePlacementSettings.defaultPlacement from NewWorkspacePlacement.afterCurrent to NewWorkspacePlacement.end (fallback used when user-defaults value is missing/invalid).

Test coverage for placement decoding

Layer / File(s) Summary
Stored-valid-value tests for all placements
cmuxTests/WorkspaceUnitTests.swift
Updated WorkspacePlacementSettingsTests to iterate NewWorkspacePlacement.allCases, write each placement.rawValue to WorkspacePlacementSettings.placementKey in defaults, and assert WorkspacePlacementSettings.current(defaults:) equals the corresponding enum case before asserting the invalid-value fallback.

Docs example update

Layer / File(s) Summary
Config example JSON
web/app/[locale]/docs/configuration/page.tsx
Updated the commented example value for app.newWorkspacePlacement in settingsFileExample from "afterCurrent" to "end".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 I hopped through defaults to find a new end,
Each placement I tasted, then hopped to the next friend.
Tests scattered carrots in every small trail,
I nibbled and nodded — the changes set sail.
🥕

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Default new workspace placement to end' clearly and concisely describes the main change in the pull request.
Description check ✅ Passed The PR description covers the Summary and Verification sections from the template, clearly stating what changed and how it was verified.
Linked Issues check ✅ Passed All objectives from issue #3291 are met: defaultPlacement changed from .afterCurrent to .end, schema and docs updated, and unit tests validate the new default behavior.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue objective to change default workspace placement; no unrelated modifications present.
Cmux Swift Actor Isolation ✅ Passed The PR only changes a static default value in WorkspacePlacementSettings from .afterCurrent to .end. No actor isolation issues: no implicit MainActor, no mutable Sendable types, pure enum values only.
Cmux Swift Blocking Runtime ✅ Passed No blocking synchronization introduced. Production change is enum default value reassignment with no semaphores, waits, sleeps, or locks.
Cmux No Hacky Sleeps ✅ Passed No hacky sleeps introduced. Changes are Swift code and documentation/schema updates. No sleep, setTimeout, setInterval, polling, or timing constructs added to runtime scripts.
Cmux Swift Concurrency ✅ Passed PR contains no Swift concurrency patterns. Changes are configuration value updates and test assertions. No Dispatch, Combine, completion handlers, or fire-and-forget Tasks introduced.
Cmux Swift @Concurrent ✅ Passed No concurrent annotation issues. PR is simple config changes with no async work, @concurrent additions, or isolation violations.
Cmux Swift File And Package Boundaries ✅ Passed Minimal focused change: 1 line in TabManager.swift (enum default), net -4 lines in test file, 1 doc line. Fits allowed case: "focused bug fixes with small code additions to large files."
Cmux Swift Logging ✅ Passed The PR contains no Swift logging violations. Changes include test updates, a default value change, and documentation alignment - none introduce print, debugPrint, dump, NSLog, or improper logging.
Cmux Swiftui State Layout ✅ Passed PR updates only a static constant in a utility enum and does not introduce SwiftUI state management patterns that violate the rules.
Cmux Architecture Rethink ✅ Passed Simple correctness fix with clear owner and invariant. No timing, blocking, mutable state, observers, side channels, or lifecycle ownership issues introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR changes are limited to workspace placement default value, unit tests, and docs. No NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup code is added or materially changed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-3291-default-placement-end

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.

@greptile-apps

greptile-apps Bot commented Apr 29, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes the default new workspace placement from afterCurrent to end, so Cmd+N workspaces append to the end of the sidebar when no explicit preference is stored. All four touch points are updated atomically: the Swift fallback constant, unit tests, the JSON schema default, and the documentation example.

  • Sources/TabManager.swift: WorkspacePlacementSettings.defaultPlacement changed from .afterCurrent to .end; fallback path in current(defaults:) picks this up automatically.
  • cmuxTests/WorkspaceUnitTests.swift: Tests now assert .end for unset/invalid values; the stored-value test is widened to iterate all CaseIterable cases.
  • web/data/cmux.schema.json / page.tsx: Public schema default and settings-file template comment updated to "end" to stay in sync.

Confidence Score: 5/5

Safe to merge — the change is a single enum constant swap with no logic modifications, and all related layers are updated consistently.

The diff touches only the fallback default value; the insertion logic itself is untouched. The Swift constant, JSON schema, doc template, and tests all agree on end. The previously noted revert concern is resolved: TabManager.swift line 211 reads .end on the current HEAD.

No files require special attention.

Important Files Changed

Filename Overview
Sources/TabManager.swift Single-line change: defaultPlacement switched from .afterCurrent to .end; no logic changed beyond the fallback value.
cmuxTests/WorkspaceUnitTests.swift Tests updated to assert .end fallback; testCurrentPlacementReadsStoredValidValueAndFallsBackForInvalid now iterates all CaseIterable cases instead of testing a single value, improving coverage.
web/data/cmux.schema.json Schema default for newWorkspacePlacement updated from "afterCurrent" to "end", matching the Swift-side change.
web/app/[locale]/docs/configuration/page.tsx Inline settings-file example comment updated from "afterCurrent" to "end" to stay in sync with the new default.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User presses Cmd+N] --> B{newWorkspacePlacement\nstored in UserDefaults?}
    B -- "Yes (valid raw value)" --> C[Use stored placement\ntop / afterCurrent / end]
    B -- "No or invalid" --> D[Use defaultPlacement\n.end  ← changed from .afterCurrent]
    C --> E[insertionIndex resolves position]
    D --> E
    E --> F[Workspace appended / inserted]
Loading

Reviews (6): Last reviewed commit: "docs: align workspace placement config e..." | Re-trigger Greptile

Comment thread cmuxTests/WorkspaceUnitTests.swift Outdated
The previous default was afterCurrent. User feedback confirmed the three placement modes are useful, but the default itself should remain unchanged from the existing behavior.

Constraint: Latest user instruction overrides the earlier issue request to default to end

Rejected: Keep the end default from #3291 | user explicitly asked not to change the previous default

Confidence: high

Scope-risk: narrow

Tested: git diff --check

Not-tested: Local tests per prior task instruction; app rebuild/launch still required
@austinywang

Copy link
Copy Markdown
Contributor Author

Closing after follow-up feedback: the previous default was .afterCurrent, and we should not change it to .end. The branch now restores the prior default.

The placement selector already supports Top, End, and After Current. This strengthens the settings reader regression coverage so all stored placement values round-trip while preserving the original afterCurrent default.

Constraint: Do not change the existing default placement

Rejected: Keep the earlier end-default regression | user confirmed the default should not change

Confidence: high

Scope-risk: narrow

Tested: git diff --check

Not-tested: Local tests per earlier task instruction
@austinywang austinywang reopened this Apr 30, 2026
@austinywang austinywang changed the title Default new workspace placement to End (closes #3291) Verify workspace placement settings keep existing default Apr 30, 2026
The branch is synced with current main before addressing PR #3293. The workspace placement tests now use XCTest's throwing unwrap helper for isolated UserDefaults setup, preserving allCases coverage while removing setup boilerplate that pushed WorkspaceUnitTests over the file-length guard on the old CI merge base.

Constraint: PR intent is to keep the afterCurrent default while testing all stored placements

Constraint: iterate-pr requires syncing the latest base before pushing fixes

Rejected: Refresh the Swift file-length budget | this PR should not accept new test-file debt for setup boilerplate

Confidence: high

Scope-risk: narrow

Tested: git diff --check; ./tests/test_ci_swift_file_length_budget.sh

Not-tested: Local XCTest/app tests per repo policy
@austinywang austinywang changed the title Verify workspace placement settings keep existing default Default new workspace placement to end May 12, 2026

@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 7e32f52. Configure here.

Comment thread Sources/TabManager.swift
coderabbitai[bot]
coderabbitai Bot previously requested changes May 12, 2026

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

Actionable comments posted: 1

🤖 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 `@Sources/TabManager.swift`:
- Line 211: The fallback default for new workspace placement was changed
improperly: restore the static constant defaultPlacement in TabManager (symbol:
static let defaultPlacement) back to .afterCurrent so unset or invalid
newWorkspacePlacement continues to behave as before; update the value in
TabManager.swift from .end to .afterCurrent and run tests/quick manual check of
workspace opening to confirm behavior.
🪄 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: cea3a0eb-4a71-4746-8174-a6c8535c5339

📥 Commits

Reviewing files that changed from the base of the PR and between 37a4550 and 7e32f52.

📒 Files selected for processing (1)
  • Sources/TabManager.swift

Comment thread Sources/TabManager.swift
@lawrencecchen
lawrencecchen dismissed coderabbitai[bot]’s stale review May 12, 2026 04:33

Dismissed as a verified false positive: issue #3291 explicitly requests changing the default fallback to .end, and the inline CodeRabbit thread was answered and resolved.

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

♻️ Duplicate comments (1)
Sources/TabManager.swift (1)

211-211: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Restore the fallback default to .afterCurrent to match the branch’s final objective.

Line 211 currently keeps .end, so unset/invalid placement values still resolve to end instead of the restored legacy default.

Suggested fix
-    static let defaultPlacement: NewWorkspacePlacement = .end
+    static let defaultPlacement: NewWorkspacePlacement = .afterCurrent
🤖 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 `@Sources/TabManager.swift` at line 211, The stored default for new workspace
placement is wrong: change the constant static let defaultPlacement:
NewWorkspacePlacement = .end to use the legacy fallback .afterCurrent so that
unset or invalid placement values resolve to .afterCurrent; update the
declaration of defaultPlacement in TabManager (or wherever static let
defaultPlacement is defined) to use .afterCurrent and run tests/compile to
ensure callers of defaultPlacement pick up the restored fallback.
🤖 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.

Duplicate comments:
In `@Sources/TabManager.swift`:
- Line 211: The stored default for new workspace placement is wrong: change the
constant static let defaultPlacement: NewWorkspacePlacement = .end to use the
legacy fallback .afterCurrent so that unset or invalid placement values resolve
to .afterCurrent; update the declaration of defaultPlacement in TabManager (or
wherever static let defaultPlacement is defined) to use .afterCurrent and run
tests/compile to ensure callers of defaultPlacement pick up the restored
fallback.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0e2418c3-ad32-4c89-8e33-2df325d0ab35

📥 Commits

Reviewing files that changed from the base of the PR and between 7e32f52 and 27d12fe.

📒 Files selected for processing (2)
  • Sources/TabManager.swift
  • web/app/[locale]/docs/configuration/page.tsx

@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 — 27d12fe6 Deployed May 12, 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.

Change default new workspace placement from 'After Current' to 'End'

3 participants