Skip to content

File explorer sort options - #6986

Closed
austinywang wants to merge 28 commits into
mainfrom
issue-5737-file-explorer-add-sort-options-date-created-m
Closed

austinywang wants to merge 28 commits into
mainfrom
issue-5737-file-explorer-add-sort-options-date-created-m

Conversation

@austinywang

@austinywang austinywang commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5737

Summary

  • Add file explorer sort options for name, date created, and date modified with ascending/descending order.
  • Persist sort preferences through cmux.json (fileExplorer.sortBy, fileExplorer.sortOrder) and UserDefaults.
  • Fetch local file creation/modification dates and best-effort remote SSH timestamps.
  • Add AppKit header sort menu, localization, schema docs, and behavior tests.

Validation

  • python3 -m json.tool Resources/Localizable.xcstrings
  • python3 -m json.tool web/data/cmux.schema.json and web/messages/*.json
  • python3 scripts/swift_file_length_budget.py
  • ./scripts/lint-pbxproj-test-wiring.sh

No dev build or xcodebuild run per task instruction.


Summary by cubic

Adds sortable file explorer (name, date created, date modified) with ascending/descending and saved preferences. Also makes remote SSH listings shell‑agnostic and faster, preserves expanded folders on re‑sort/reload, and unifies FileExplorerStore init.

  • New Features

    • Sort button with menu and localized tooltip; “Sort Files” accessibility label.
    • Preferences persist via fileExplorer.sortBy and fileExplorer.sortOrder; schema/messages updated; template defaults; live re‑sort.
    • Sort policy: folders first for name; date sorts mix files/folders by created/modified; missing dates last; stable tie‑breakers.
  • Bug Fixes

    • Keeps expanded folders on sort, reload, and refresh; fewer redraws by tracking content/sort revisions.
    • Remote SSH listing: POSIX /bin/sh bootstrap; single batched stat via find … -exec … + (ARG_MAX‑safe); caches timestamps; stable mode format.
    • Correctness: dangling symlinks stay visible; dotfiles hidden unless requested; permission/command failures surface as errors.
    • Local listing prefetches .isDirectoryKey, dropping redundant per‑entry stat (local dangling symlinks stay visible).

Written for commit 721d8ae. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added file explorer sorting controls (sort by name/creation/modification and ascending/descending).
    • Sorting state is reflected in the header with localized titles, tooltip, and accessibility label.
    • Sorting preferences are persisted and restored across launches, including from remote listings.
  • Bug Fixes
    • Improved refresh behavior so the file explorer reliably re-sorts and reloads when sort settings change.
    • Better ordering for items with missing timestamps (date sorts place unknown dates last).
  • Documentation
    • Updated settings schema and localized help text for the new sorting options.

@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 9:25am
cmux-staging Building Building Preview, Comment Jul 5, 2026 9:25am

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds configurable file explorer sorting by name, date created, and date modified, with ascending/descending order, persisted settings, UI controls, schema support, and localized strings. It also updates local and SSH directory listing data to carry timestamps for sorting.

Changes

File Explorer Sorting Feature

Layer / File(s) Summary
Sort model and comparator
Sources/FileExplorerSortKey.swift, Sources/FileExplorerSortOrder.swift, Sources/FileExplorerSortOptions.swift, Sources/FileExplorerSortSettings.swift, Sources/FileExplorerNodeSorter.swift, cmux.xcodeproj/project.pbxproj
Adds sort key/order types, sort options, persisted sort settings, the node sorter, and project wiring for the new sources.
Store dates and sort state
Sources/FileExplorerStore.swift
Adds timestamp storage to entries/nodes, populates local and SSH listings with dates, and centralizes sort state, persistence, and re-sorting in the store.
Header sort control
Sources/FileExplorerHeaderView.swift
Adds the sort button, menu actions, and header state updates for sort key, sort order, tooltip, and accessibility text.
Outline reload and selection wiring
Sources/FileExplorerView.swift
Tracks sort revisions for reload decisions, switches the outline data source to node children, restores expansion safely, and wires header selections to the store.
Settings parsing and schema
Sources/CmuxSettingsJSONPathSupport.swift, Sources/KeyboardShortcutSettingsFileStore+Template.swift, Sources/KeyboardShortcutSettingsFileStore.swift, web/data/cmux.schema.json
Allows fileExplorer.sortBy/sortOrder, adds template defaults, parses and validates settings, posts change notifications, and defines schema properties.
Sorting and listing tests
cmuxTests/FileExplorerDoubleClickActionTests.swift, cmuxTests/FileExplorerStoreTests.swift
Adds coverage for sort settings, node sorting, SSH listing execution and parsing, and settings-file parsing.
Localized strings and docs
Resources/Localizable.xcstrings, web/messages/*.json
Adds localized sort labels/tooltips and file explorer sorting descriptions across the app and web docs.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant FileExplorerHeaderView
  participant FileExplorerView
  participant FileExplorerStore
  participant FileExplorerSortSettings
  participant FileExplorerNodeSorter

  User->>FileExplorerHeaderView: select sort key/order
  FileExplorerHeaderView->>FileExplorerView: onSelectSortKey/onSelectSortOrder
  FileExplorerView->>FileExplorerStore: setSortKey/setSortOrder
  FileExplorerStore->>FileExplorerSortSettings: setOptions(sortOptions)
  FileExplorerSortSettings-->>FileExplorerStore: didChangeNotification
  FileExplorerStore->>FileExplorerNodeSorter: sorted(rootNodes, loadedChildren)
  FileExplorerView->>FileExplorerView: reloadIfNeeded()
Loading

Related Issues: #5737

Suggested labels: area: file-explorer, area: localization, platform: macOS

Suggested reviewers: maintainers familiar with FileExplorerStore and FileExplorerView


Important

Pre-merge checks failed

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

❌ Failed checks (4 errors, 2 warnings)

Check name Status Explanation Resolution
Cmux Swift @Concurrent ❌ Error FileExplorerStore.loadChildren is @MainActor and directly awaits LocalFileExplorerProvider.listDirectory, which does synchronous directory/resourceValue I/O without a hop. Move the local directory scan off MainActor, e.g. annotate the listing helper @concurrent or wrap it in a detached/background task before awaiting it.
Cmux Swift File And Package Boundaries ❌ Error New pure sort core (FileExplorerSort*, FileExplorerNodeSorter) lives in app-target Sources/ instead of a small SwiftPM package, despite being independently testable. Extract those types into a small package target (e.g. CmuxFileExplorerCore) and have FileExplorerStore/UI import it; keep header/view code as app glue.
Cmux Architecture Rethink ❌ Error FileExplorerStore adds a custom NotificationCenter observer for sort settings, mirroring the same sort state through a side channel alongside direct setters. Make sort options single-owned: have the store own/publish sortOptions and update it directly from settings changes via one callback path, removing the custom notification mirror.
Cmux No Ambient Global State ❌ Error PR adds ProcessSSHFileExplorerTransport.shared and uses it as a default transport, introducing a new singleton ambient surface. Inject a transport instance from the app seam (or keep the factory private/file-scoped) instead of exposing a new shared singleton.
Out of Scope Changes check ⚠️ Warning The pbxproj also wires unrelated File Explorer search source files, which are outside the sorting scope. Move the unrelated search-file project wiring to a separate PR or explain why it belongs in this sorting change.
Description check ⚠️ Warning The description covers summary and validation, but it omits required template sections like Demo Video, Review Trigger, and Checklist. Add the missing template sections: Testing, Demo Video, Review Trigger, and Checklist, and include the requested verification details.
✅ Passed checks (19 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement the requested sort control, date fields, persistence, and sorting behavior required by #5737.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed New sort APIs stay on the existing main-thread UI store, and the only new shared reference type uses an NSLock with documented @unchecked Sendable safety.
Cmux Swift Blocking Runtime ✅ Passed No new blocking waits/sleeps/locks were added in touched production Swift files; existing NSLock/waitUntilExit in FileExplorerStore were already in HEAD^.
Cmux Browser Automation Off-Main ✅ Passed PASS: PR doesn't touch Sources/TerminalController.swift or ControlCommandExecutionPolicy.swift; browser-related diffs are CLI/help/telemetry churn, not wait/callback routing.
Cmux Expensive Synchronous Load ✅ Passed No agent-history/transcript/trajectory loaders were added; the diff is confined to file-explorer sorting and settings, with no new sync history load on a main-actor path.
Cmux Cache Substitution Correctness ✅ Passed Sort prefs are read from UserDefaults with change notifications and a nil/default fallback; no persistence/history/snapshot path now trusts a stale cache instead of a fresh read.
Cmux No Hacky Sleeps ✅ Passed No new fixed sleeps/timers/polling were introduced in changed TS/JS/shell files; the only requestAnimationFrame deferral predates this PR.
Cmux Algorithmic Complexity ✅ Passed No nested rescans or hot-path repeated sorts: remote listing is batched once, and resorting happens only on explicit sort changes with cached children.
Cmux Swift Concurrency ✅ Passed The file-explorer diff adds async/await and AppKit callbacks, but no new background DispatchQueue, completion-handler APIs, Combine app state, or fire-and-forget lifecycle Tasks.
Cmux Swiftpm Lockfiles ✅ Passed No cmux-owned .gitignore/Package.swift changes; cmux.xcodeproj diff only adds source files, with no SwiftPM package refs or lockfile edits.
Cmux Swift Logging ✅ Passed PASS: The PR diff adds no production logging; changed runtime hunks only adjust sort state, and logging-related code is unchanged or test-only.
Cmux User-Facing Error Privacy ✅ Passed Changed user-facing errors are generic and actionable; I found no leaked vendor names, secrets, raw upstream messages, or other prohibited internals.
Cmux Full Internationalization ✅ Passed New Swift UI text uses localized APIs with matching xcstrings entries for all 20 catalog locales, and all 20 web locales in routing.ts were updated with matching messages.
Cmux Swiftui State Layout ✅ Passed No banned SwiftUI patterns were introduced; the changes use root-level @Bindable/@State and AppKit bridges, with no GeometryReader or lazy list-row store refs.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR only adds a sort menu inside an existing FileExplorer NSView/sidebar UI; no new NSWindow/NSPanel/NSWindowController/WindowGroup or cmux.* identifier changes.
Cmux Source Artifacts ✅ Passed All changed paths are intentional source/docs/config/localization/test assets; no build, temp, cache, or artifact directories appear, and the TSV/checksum files are deliberate CI inputs.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Touched sort files add no test/debug seams; the only ForTesting hook in Sources/FileExplorerStore.swift is preexisting and unchanged.
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding file explorer sort options.
✨ 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-5737-file-explorer-add-sort-options-date-created-m

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.

…rer-add-sort-options-date-created-m

# Conflicts:
#	.github/swift-file-length-budget.tsv
@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds sortable file explorer (name, date created, date modified with ascending/descending) persisted via cmux.json and UserDefaults, a new AppKit sort button with localized menu, and a refactored SSH remote listing that batches stat calls through find … -exec … + and wraps the script in a POSIX /bin/sh bootstrap to support non-POSIX login shells.

  • Sort core: FileExplorerNodeSorter handles stable tie-breaking and correct nil-date ordering; FileExplorerSortSettings is a properly injectable value type; applySortOptions calls objectWillChange.send() before mutations and the coordinator's 50 ms debounce ensures sortRevision is read post-commit.
  • SSH listing overhaul: single batched stat via find … -exec … + replaces per-entry stat; birth-time minimum epoch (≈1973) filters Linux st_birthtime=0 sentinels; posixShellBootstrap base64-encodes the script to survive fish/csh login shells.
  • Expansion preservation: coordinator now tracks both contentRevision and sortRevision; restoreExpansionState iterates with a live row count so nested expanded folders are re-opened correctly after a sort change.

Confidence Score: 5/5

Safe to merge; the sort, persistence, and SSH listing changes are well-structured with no correctness regressions found.

The most consequential new paths — applySortOptions/resortLoadedNodes, SSH find+stat batching, and the coordinator's debounced reload — are all implemented correctly. The objectWillChange + 50ms debounce pattern ensures sortRevision is always read after mutations commit. The SSH bootstrap safely handles non-POSIX login shells and birth-time zero sentinels. The only findings are a maintenance mismatch in the JSON-path allowlist (hardcoded strings vs. constants) and a notifyDidChange visibility nit — neither affects runtime behaviour.

No files require special attention; the hardcoded key strings in CmuxSettingsJSONPathSupport.swift are worth cleaning up before a future key rename.

Important Files Changed

Filename Overview
Sources/FileExplorerStore.swift Major additions: date-stamped FileExplorerEntry/Node, SSH stat batching via find+exec, resortLoadedNodes, and sort settings observation. The applySortOptions/objectWillChange pattern and debounced coordinator reload are correct. Minor: setSortKey doesn't reset order when switching back to name.
Sources/FileExplorerNodeSorter.swift New sort struct with correct stable tie-breaking (name fallback), proper nil-date ordering (known dates first regardless of direction), and no ambient global state.
Sources/FileExplorerSortSettings.swift Injectable struct wrapping UserDefaults + NotificationCenter; correctly constructable and injected at the app seam. notifyDidChange() should be private — no external caller uses it.
Sources/CmuxSettingsJSONPathSupport.swift New sortBy/sortOrder keys added to the JSON-path allowlist as hardcoded strings instead of FileExplorerSortSettings constants; will silently diverge if keys are renamed.
Sources/FileExplorerHeaderView.swift Adds sort button with NSMenu, localized tooltip/accessibility label, and correct menu-state checkmarks. Layout constraints look correct; update guard prevents redundant redraws.
Sources/FileExplorerView.swift Coordinator now tracks sortRevision alongside contentRevision; 50ms debounce on objectWillChange ensures sortRevision is read after all synchronous mutations. restoreExpansionState correctly uses a live-count loop to handle newly expanded rows.
Sources/FileExplorerSortKey.swift Clean CaseIterable enum with safe rawValue resolution fallback and localized titles.
Sources/KeyboardShortcutSettingsFileStore.swift sortBy/sortOrder parsing mirrors the existing doubleClickAction pattern; posts didChangeNotification using the static Notification.Name constant correctly.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Header as FileExplorerHeaderView
    participant Store as FileExplorerStore
    participant Settings as FileExplorerSortSettings
    participant UD as UserDefaults
    participant NC as NotificationCenter
    participant Coord as Coordinator (debounced 50ms)

    Header->>Store: setSortKey(key)
    Store->>Store: applySortOptions(options, persist:true)
    Store->>Store: objectWillChange.send()
    Store->>Store: "sortOptions = options"
    Store->>Store: resortLoadedNodes()
    Store->>Store: "sortRevision &+= 1"
    Store->>Settings: setOptions(options)
    Settings->>UD: set sortKeyKey / sortOrderKey
    Settings->>NC: post(didChangeNotification)
    NC-->>Store: observer fires (queue:.main, next runloop)
    Store->>Store: "applySortOptionsFromDefaults() guard sortOptions != options return early"
    NC-->>Coord: objectWillChange debounced 50ms
    Coord->>Store: read sortRevision (now updated)
    Coord->>Coord: outlineView.reloadData() + restoreExpansionState
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 Header as FileExplorerHeaderView
    participant Store as FileExplorerStore
    participant Settings as FileExplorerSortSettings
    participant UD as UserDefaults
    participant NC as NotificationCenter
    participant Coord as Coordinator (debounced 50ms)

    Header->>Store: setSortKey(key)
    Store->>Store: applySortOptions(options, persist:true)
    Store->>Store: objectWillChange.send()
    Store->>Store: "sortOptions = options"
    Store->>Store: resortLoadedNodes()
    Store->>Store: "sortRevision &+= 1"
    Store->>Settings: setOptions(options)
    Settings->>UD: set sortKeyKey / sortOrderKey
    Settings->>NC: post(didChangeNotification)
    NC-->>Store: observer fires (queue:.main, next runloop)
    Store->>Store: "applySortOptionsFromDefaults() guard sortOptions != options return early"
    NC-->>Coord: objectWillChange debounced 50ms
    Coord->>Store: read sortRevision (now updated)
    Coord->>Coord: outlineView.reloadData() + restoreExpansionState
Loading

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

Comment thread Resources/Localizable.xcstrings
Comment thread web/messages/ar.json
Comment thread Sources/FileExplorerStore.swift Outdated
cmux added 4 commits June 28, 2026 20:21
austinywang and others added 6 commits June 29, 2026 16:45
…rer-add-sort-options-date-created-m

# Conflicts:
#	.github/swift-file-length-budget.tsv
The date-sort rewrite replaced the simple `ls` listing with a multi-line
POSIX script sent as the SSH command, which OpenSSH runs through the
account's login shell. fish/csh/tcsh cannot parse `for`/`if`/`case`, so
the remote file explorer could fail entirely for those users. The script
also ran two-plus `stat` processes per entry to gather timestamps, even
for the default name sort that needs none, turning one `ls` into O(entries)
remote process spawns and making large remote directories appear to hang.

- Run the listing under `/bin/sh` regardless of login shell by
  base64-encoding the script and bootstrapping via `/bin/sh -c` (only a
  base64 payload with no shell metacharacters reaches the login shell),
  mirroring the existing DockSplitStore remote-script pattern.
- Detect GNU vs BSD `stat` once, then run a single batched `stat` over all
  entries (O(1) remote processes). Timestamps are always collected so
  switching the sort key re-sorts the cached listing without a re-fetch.
- Decompose into testable `remoteListingScript`/`posixShellBootstrap`/
  `parseRemoteListing` helpers and add regression coverage for the
  POSIX-shell wrapping, batched stat, hidden/`.`/`..` filtering, and the
  birth-time threshold.

Verified the generated command empirically against BSD `stat` and across
/bin/sh, zsh, and bash. Surfaced by autoreview on PR #6986.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
autoreview flagged that the batched SSH listing's `cd ... || exit 0` plus
the unconditional trailing `exit 0` reported permission/command failures as
empty directories, losing the pre-rewrite ability to distinguish an
inaccessible directory from an empty one.

Exit non-zero (so `runSSHCommand` raises `sshCommandFailed`) when `cd`
fails, when the directory is not readable (`[ -r . ]`), or when no usable
`stat` is found. A readable-but-empty directory still reaches `exit 0` and
lists as empty even though its unmatched glob makes `stat` exit non-zero.

Verified on macOS that normal/empty dirs succeed while nonexistent and
unreadable (x-only) dirs exit 1; added regression coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
autoreview flagged that the batched listing used `stat -L`, which follows
symlinks. On GNU, a following stat of a dangling symlink emits no record and
exits non-zero for that operand, so with stderr suppressed the broken
symlink was silently dropped — users could no longer see, delete, rename, or
fix it. The pre-rewrite `ls -F` still listed dangling symlinks.

Drop `-L` so the listing uses lstat: every entry, including dangling and
directory symlinks, is reported with its own type ("symbolic link"),
matching the previous `ls -F` behavior (symlinks shown, non-expandable).

Added regression coverage that symlink rows stay listed and non-directory.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
autoreview flagged two more remote-listing regressions:

- The glob `* .[!.]* ..?*` expands in the remote shell before `stat`, so a
  large directory could exceed ARG_MAX; `stat` then fails to exec and, with
  stderr suppressed and the trailing `exit 0`, the directory looked empty.
  Replace the glob with `find . -mindepth 1 -maxdepth 1 ... -exec stat {} +`,
  which enumerates the directory and batches `stat` within ARG_MAX (still a
  small bounded number of processes, never per-entry).
- The bootstrap ran `eval "$(...base64...)"` without checking the decode, so a
  host lacking a compatible `base64` silently listed every directory as empty.
  Capture the decoded script and `exit 1` when it is empty.

`find .` reports entries as `./name`, so the parser now keeps only the final
path component. Dotfiles are excluded with `! -name '.*'` unless hidden entries
are requested. Verified on macOS across normal/empty/missing/unreadable dirs,
dangling symlinks, and a simulated missing-base64 host; extended the tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
autoreview flagged that the remote-listing tests mostly asserted substrings of
the generated shell script, which the repo's test-quality rule disallows and
which can pass even when the command is invalid under /bin/sh or behaves
differently at runtime.

Replace the implementation-text assertions with executable behavioral tests:
build the exact command the SSH transport sends and run it locally through a
real shell against temp-directory fixtures, then assert the parsed result and
exit status. Coverage now includes directory contents, hidden-file filtering,
dangling/directory symlink visibility, empty-directory success, missing and
unreadable directories surfacing as non-zero exits, single-quoted paths,
identical results across /bin/sh and /bin/zsh, and a missing-base64 host (via an
emptied PATH) erroring instead of listing empty. The synthetic parser unit
tests are kept to cover GNU stat output that the macOS test host cannot produce.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rer-add-sort-options-date-created-m

# Conflicts:
#	.github/swift-file-length-budget.tsv

@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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/KeyboardShortcutSettingsFileStore.swift (1)

1588-1601: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Batch the sort-settings notification instead of posting inline per key.

Unlike the sibling flags in this same loop (agentSessionAutoResumeDidChange, agentHibernationDidChange, rendererRealizationDidChange), which set a boolean and notify once after the loop, the new sort-settings check posts FileExplorerSortSettings.didChangeNotification directly inside the loop. If both sortBy and sortOrder change in the same settings reload (a common case — the user changes sort key and direction together, or cmux.json sets both keys at once), the notification fires twice, causing FileExplorerStore's observer to reapply sort options and re-sort the loaded nodes twice.

♻️ Proposed fix: batch like the other flags
             var agentSessionAutoResumeDidChange = false
             var agentHibernationDidChange = false
             var rendererRealizationDidChange = false
+            var fileExplorerSortSettingsDidChange = false
             for change in changes {
                 if change.defaultsKey == TerminalScrollBarSettings.showScrollBarKey {
                     TerminalScrollBarSettings.notifyDidChange(notificationCenter: notificationCenter)
                 }

                 if change.defaultsKey == TerminalCopyOnSelectSettings.copyOnSelectKey {
                     TerminalCopyOnSelectSettings.notifyDidChange(notificationCenter: notificationCenter)
                 }

                 if change.defaultsKey == FileExplorerSortSettings.sortKeyKey ||
                     change.defaultsKey == FileExplorerSortSettings.sortOrderKey {
-                    notificationCenter.post(name: FileExplorerSortSettings.didChangeNotification, object: nil)
+                    fileExplorerSortSettingsDidChange = true
                 }
...
             }
+            if fileExplorerSortSettingsDidChange {
+                notificationCenter.post(name: FileExplorerSortSettings.didChangeNotification, object: nil)
+            }
🤖 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/KeyboardShortcutSettingsFileStore.swift` around lines 1588 - 1601,
The FileExplorer sort-settings change handling is posting
FileExplorerSortSettings.didChangeNotification inside the changes loop, which
can fire twice when both sortKeyKey and sortOrderKey change together. Update
KeyboardShortcutSettingsFileStore’s settings reload logic to match the other
batched flags by setting a local boolean when either
FileExplorerSortSettings.sortKeyKey or sortOrderKey changes, then post the
notification once after the loop.
🤖 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 `@cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift`:
- Around line 537-587: The current test only covers valid fileExplorer sort
values, so it does not verify the settings-file parsing path rejects bad enum
raw values safely. Add a regression test alongside
testSettingsFileParsesFileExplorerSortOptions that writes an invalid
sortBy/sortOrder into cmux.json, then initializes
KeyboardShortcutSettingsFileStore and asserts FileExplorerSortSettings still
resolves to FileExplorerSortOptions.defaultValue without blocking the rest of
the file from being processed. Use the existing FileExplorerSortSettings,
KeyboardShortcutSettingsFileStore, and resolvedOptions() symbols to keep the
test aligned with the current parsing flow.

In `@Sources/FileExplorerHeaderView.swift`:
- Around line 104-114: The sort tooltip in FileExplorerHeaderView’s sort button
setup is building a localized format string with String(format:), which can
ignore locale-specific argument ordering. Update the tooltip construction in the
sortButton configuration to use String.localizedStringWithFormat with the
localized key/defaultValue string and the sortOptions.key.localizedTitle and
sortOptions.order.localizedTitle arguments, keeping the rest of the sortButton
accessibility setup unchanged.

In `@Sources/FileExplorerStore.swift`:
- Around line 213-381: The new sort-related types and comparator logic in
FileExplorerStore.swift add another large responsibility to an already oversized
mixed-purpose file. Move the self-contained sorting model and persistence pieces
out of FileExplorerStore.swift by extracting FileExplorerSortKey,
FileExplorerSortOrder, FileExplorerSortOptions, FileExplorerSortSettings, and
FileExplorerNodeSorter into a separate file so the store keeps only wiring and
usage. Keep the new file independent of AppKit/store concerns and update
FileExplorerStore and any callers to reference the extracted symbols.
- Around line 480-499: The local directory listing in FileExplorerStore is doing
a redundant filesystem stat for every entry via fileExists(atPath:isDirectory:)
after contentsOfDirectory already fetched metadata. Update the listing logic to
request .isDirectoryKey in contentsOfDirectory and read the directory flag from
resourceValues instead of probing each URL again, keeping the filtering and
FileExplorerEntry construction in the same flow.

---

Outside diff comments:
In `@Sources/KeyboardShortcutSettingsFileStore.swift`:
- Around line 1588-1601: The FileExplorer sort-settings change handling is
posting FileExplorerSortSettings.didChangeNotification inside the changes loop,
which can fire twice when both sortKeyKey and sortOrderKey change together.
Update KeyboardShortcutSettingsFileStore’s settings reload logic to match the
other batched flags by setting a local boolean when either
FileExplorerSortSettings.sortKeyKey or sortOrderKey changes, then post the
notification once after the loop.
🪄 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: 6f55b732-f4a1-472b-918b-6523316a4890

📥 Commits

Reviewing files that changed from the base of the PR and between 871aa71 and 21be975.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (31)
  • Resources/Localizable.xcstrings
  • Sources/CmuxSettingsJSONPathSupport.swift
  • Sources/FileExplorerHeaderView.swift
  • Sources/FileExplorerStore.swift
  • Sources/FileExplorerView.swift
  • Sources/KeyboardShortcutSettingsFileStore+Template.swift
  • Sources/KeyboardShortcutSettingsFileStore.swift
  • cmuxTests/FileExplorerDoubleClickActionTests.swift
  • cmuxTests/FileExplorerStoreTests.swift
  • cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift
  • web/data/cmux.schema.json
  • web/messages/ar.json
  • web/messages/bs.json
  • web/messages/da.json
  • web/messages/de.json
  • web/messages/en.json
  • web/messages/es.json
  • web/messages/fr.json
  • web/messages/it.json
  • web/messages/ja.json
  • web/messages/km.json
  • web/messages/ko.json
  • web/messages/no.json
  • web/messages/pl.json
  • web/messages/pt-BR.json
  • web/messages/ru.json
  • web/messages/th.json
  • web/messages/tr.json
  • web/messages/uk.json
  • web/messages/zh-CN.json
  • web/messages/zh-TW.json

Comment thread cmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swift Outdated
Comment thread Sources/FileExplorerHeaderView.swift
Comment thread Sources/FileExplorerStore.swift Outdated
Comment thread Sources/FileExplorerStore.swift
The remote listing script runs `find ... -exec stat ...` then reaches the
trailing `exit 0` unconditionally, so a wholesale `find`/`stat` failure
(e.g. a remote `find` lacking `-mindepth`/`-maxdepth`) is swallowed and the
directory appears empty instead of raising `sshCommandFailed`.

This behavioral test shadows `find` with a stub that exits non-zero and
asserts the generated command exits non-zero. It fails against the current
script (which masks the failure as exit 0); the fix follows in the next
commit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

2 issues found across 32 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cmuxTests/FileExplorerStoreTests.swift">

<violation number="1" location="cmuxTests/FileExplorerStoreTests.swift:1281">
P2: The new ProcessSSHFileExplorerListingTests suite creates Process objects with pipes to run shell commands and verify output/exit codes. Per team convention, tests that rely on a process/socket harness should use XCTest (XCTestCase) rather than Swift Testing, keeping pure unit tests in Swift Testing.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread Sources/FileExplorerStore.swift Outdated
Comment thread cmuxTests/FileExplorerStoreTests.swift
The listing script ran `find ... -exec stat ...` and then reached the
trailing `exit 0` unconditionally, with the `find` stderr suppressed. Any
wholesale failure of the listing command — a remote `find` that lacks
`-mindepth`/`-maxdepth`, a `stat` format the probe did not exercise, or a
per-entry error — was swallowed and reported as an empty directory,
regressing the pre-existing behavior where a failed `ls` raised
`sshCommandFailed`.

Propagate the failure with `|| exit 1` on both `find` branches so a failed
listing exits non-zero. A genuinely empty directory still makes `find` exit
zero with no output and reaches the trailing `exit 0`, so empty listings are
unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@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 `@cmuxTests/FileExplorerStoreTests.swift`:
- Line 1459: The stdout emptiness check in the FileExplorerStore test is
decoding `Data` to a string unnecessarily; update the assertion to use
`data.isEmpty` directly instead of `String(decoding:as:)`. Locate the test
around the `FileExplorerStoreTests` assertion and keep the check focused on the
`data` value itself.
🪄 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: 40d37e12-972b-4e0e-88f9-e2d948cc0d3b

📥 Commits

Reviewing files that changed from the base of the PR and between 21be975 and d5ce0eb.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (2)
  • Sources/FileExplorerStore.swift
  • cmuxTests/FileExplorerStoreTests.swift

Comment thread cmuxTests/FileExplorerStoreTests.swift Outdated
austinywang and others added 2 commits July 1, 2026 22:54
… stat

- Move FileExplorerSortKey/Order/Options/Settings/NodeSorter out of
  FileExplorerStore.swift into a dedicated Sources/FileExplorerSortOptions.swift
  (CodeRabbit Major + file-organization findings). Wire the new file into
  the Xcode project (UUID FE59960A) and refresh the file-length budget.
- Use String.localizedStringWithFormat for the sort tooltip so locales that
  reorder positional arguments render correctly (CodeRabbit).
- Prefetch .isDirectoryKey and read isDirectory from the cached resource
  values in the local listDirectory, dropping the redundant per-entry
  fileExists stat (CodeRabbit). Local dangling symlinks now stay visible,
  matching the remote listing behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Avoids the redundant Data->String decode (SwiftLint
optional_data_string_conversion) while asserting the same masked-empty-output
regression. (CodeRabbit)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-created-m

Resolve conflicts:
- Sources/FileExplorerStore.swift: keep the sorted(using:) Array extension
  (used at the local listing call site); drop the inline GitFileStatus /
  GitStatusProvider types, which origin/main extracted into their own files
  (Sources/GitFileStatus.swift, Sources/GitStatusProvider.swift).
- .github/swift-file-length-budget.tsv: regenerated via
  scripts/swift_file_length_budget.py --write-budget.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
austinywang and others added 3 commits July 2, 2026 02:06
The origin/main merge left two all-default initializers on FileExplorerStore:
the branch's sort-settings init(sortDefaults:notificationCenter:) and main's
init(gitStatusProvider:). Each left the other's stored properties
uninitialized, and FileExplorerStore() was ambiguous between them, so the
target could not compile. Merge them into a single designated initializer that
initializes notificationCenter, gitStatusProvider, sortSettings, sortOptions,
and sortSettingsObserver, exposing a superset of parameters (all with
defaults) so every existing call site keeps working.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rer-add-sort-options-date-created-m

# Conflicts:
#	.github/swift-file-length-budget.tsv
…rer-add-sort-options-date-created-m

# Conflicts:
#	.github/swift-file-length-budget.tsv
…rer-add-sort-options-date-created-m

Resolve .github/swift-file-length-budget.tsv via scripts/swift_file_length_budget.py --write-budget on the merged tree.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
namil-k added a commit to namil-k/cmux that referenced this pull request Sep 22, 2026
The right-sidebar file explorer always sorted folders first, then
case-insensitive by name. This adds a sort menu to the explorer header
(name, date created, date modified; ascending or descending), persists the
choice through fileExplorer.sortBy / fileExplorer.sortOrder in cmux.json
and UserDefaults, and reads file dates for local and SSH listings.

Rebased from manaflow-ai#6986 onto current main. Changes beyond the original PR:
- settings parsing moved to KeyboardShortcutSettingsFileStore+SectionParsers
  and the key registry to CmuxSettingsFileStore+SupportedPaths, following
  the upstream split
- sort-settings change notification is batched like the sibling flags
- the header sort button uses RenderableSystemSymbol.configuredAppKitImage
  instead of NSImage(systemSymbolName:).withSymbolConfiguration
- CmuxConfigSchema.generated.swift regenerated so the CLI validator accepts
  the new keys
- configuration docs example lists the new keys

Fixes manaflow-ai#5737
Supersedes manaflow-ai#6986

Co-authored-by: Austin Wang <austinwang115@gmail.com>
@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
namil-k added a commit to namil-k/cmux that referenced this pull request Sep 25, 2026
The right-sidebar file explorer always sorted folders first, then
case-insensitive by name. This adds a sort menu to the explorer header
(name, date created, date modified; ascending or descending), persists the
choice through fileExplorer.sortBy / fileExplorer.sortOrder in cmux.json
and UserDefaults, and reads file dates for local and SSH listings.

Rebased from manaflow-ai#6986 onto current main. Changes beyond the original PR:
- settings parsing moved to KeyboardShortcutSettingsFileStore+SectionParsers
  and the key registry to CmuxSettingsFileStore+SupportedPaths, following
  the upstream split
- sort-settings change notification is batched like the sibling flags
- the header sort button uses RenderableSystemSymbol.configuredAppKitImage
  instead of NSImage(systemSymbolName:).withSymbolConfiguration
- CmuxConfigSchema.generated.swift regenerated so the CLI validator accepts
  the new keys
- configuration docs example lists the new keys

Fixes manaflow-ai#5737
Supersedes manaflow-ai#6986

Co-authored-by: Austin Wang <austinwang115@gmail.com>
namil-k added a commit to namil-k/cmux that referenced this pull request Sep 29, 2026
The right-sidebar file explorer always sorted folders first, then
case-insensitive by name. This adds a sort menu to the explorer header
(name, date created, date modified; ascending or descending), persists the
choice through fileExplorer.sortBy / fileExplorer.sortOrder in cmux.json
and UserDefaults, and reads file dates for local and SSH listings.

Rebased from manaflow-ai#6986 onto current main. Changes beyond the original PR:
- settings parsing moved to KeyboardShortcutSettingsFileStore+SectionParsers
  and the key registry to CmuxSettingsFileStore+SupportedPaths, following
  the upstream split
- sort-settings change notification is batched like the sibling flags
- the header sort button uses RenderableSystemSymbol.configuredAppKitImage
  instead of NSImage(systemSymbolName:).withSymbolConfiguration
- CmuxConfigSchema.generated.swift regenerated so the CLI validator accepts
  the new keys
- configuration docs example lists the new keys

Fixes manaflow-ai#5737
Supersedes manaflow-ai#6986

Co-authored-by: Austin Wang <austinwang115@gmail.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — 721d8aee 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.

File explorer: add sort options (date created/modified)

3 participants