Diff viewer: untracked files in unstaged, persisted viewer prefs, soft refresh, hunk/file navigation#6010
Diff viewer: untracked files in unstaged, persisted viewer prefs, soft refresh, hunk/file navigation#6010lawrencecchen wants to merge 5 commits into
Conversation
…nav shortcuts - cmux diff --unstaged must include untracked files (agents constantly create new files; plain git diff silently hides them) - persisted viewer preferences (CMUX_DIFF_VIEWER_PREFS_PATH) must seed payload layout + viewerOptions, with --layout still winning (#5284) - diff viewer payload must carry diffViewerNextHunk/PrevHunk/NextFile/PrevFile shortcut defaults (n / p / ] / [) Tests only; the fixes land in the next commit so CI proves they catch the behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…efresh, hunk/file navigation Four diff viewer improvements from issue triage: - Unstaged source now includes untracked files. `cmux diff --unstaged` appends an added-file patch per untracked path (gitignored files excluded), so files an agent just created are no longer invisible. - Viewer preferences persist globally (#5284). New DiffViewerPreferencesStore persists layout + options-menu toggles to ~/Library/Application Support/cmux/diff-viewer/preferences.json. The webview saves through new viewerPrefs.get/set methods on the existing cmuxDiffComments bridge and re-syncs at boot; the CLI reads the same file when generating a page so new diff panels open with the last-used layout (explicit --layout still wins). localStorage stays as fallback for pages opened outside cmux, with legacy-key migration. - Refresh is now a soft refresh. Instead of window.location.reload(), the options-menu Refresh re-streams the patch in place, preserving the split/unified layout and all viewer options (the bug report in #5284). Status-only pages keep the full reload to pick up replacements. - Keyboard navigation between hunks and files (#2526): n / p jump to the next / previous hunk, ] / [ to the next / previous file. Wired through KeyboardShortcutSettings, the CmuxSettings ShortcutAction enum, the CLI payload, cmux.json schema, the shortcuts docs data, and the Settings UI; rebindable like the existing diff viewer shortcuts. Localization: new shortcut labels added to Localizable.xcstrings for all locales (en + ja translated, others fall back to English per existing convention); docs entries carry en + ja. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When the selected git source is empty, the deferred fallback loop tried each other source and rethrew any non-empty error. A clean repo opened outside a cmux terminal (no workspace/surface context) dead-ended on the raw "cmux diff --last-turn requires a workspace and surface context" error instead of the friendly empty state, reproducing the #5246 raw CLI error empty state through a different path. Reproduced identically on a main baseline build, so this was pre-existing, not introduced by this branch. Fallback candidates that cannot be read are now skipped; only the originally selected source's own failure surfaces. This also makes the existing testDiffCommandShowsFriendlyEmptyStateWhenEveryGitSourceIsEmpty regression test pass as written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds keyboard navigation shortcuts for diff viewer hunks and files, introduces persistent viewer preferences (layout and display toggles) saved to the app's Application Support directory, includes untracked files in unstaged diffs, and makes diff streaming generation-aware to preserve UI state across refresh operations. ChangesDiff Viewer Navigation, Preferences, and Rendering
Sequence Diagram(s)The PR involves multiple independent flows rather than a single coordinated sequence. Stream rendering safety, preference persistence, and navigation are three distinct patterns that benefit from independent review but do not interact sequentially in a meaningful way for visualization. A high-level sequence of preference loading at app start would be too simple (three steps: init → bridge/localStorage fetch → apply to state), and navigation or streaming flows lack multi-component interaction that would clarify the diagram beyond the code. Diagrams are omitted as the architectural changes are clearer from code inspection. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple areas (native preferences, web rendering safety, keyboard navigation, payload structure) with moderate logic density in preference sanitization and stream generation tracking. Heterogeneous file spread and new module definitions add review scope, but patterns are consistent and test coverage is comprehensive. Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (7 errors, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds four improvements to the diff viewer: untracked files now appear in
Confidence Score: 4/5Safe to merge with one targeted fix in the fallback loop error handling The fallback loop's catch was widened from catch is EmptyDiffSourceError to a bare catch, which silences any git or write error thrown by a fallback candidate. A user with staged changes whose git index errors during fallback evaluation would see a friendly empty state rather than the git error, hiding real diff content. The rest of the PR — prefs persistence, soft refresh, keyboard nav, and untracked-file inclusion — is well-structured and thoroughly tested. CLI/cmux_open.swift around the fallback candidate error catch in completeDeferredDiffViewerSource Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as cmux CLI
participant FS as preferences.json
participant WV as Webview (App.tsx)
participant Bridge as DiffCommentsBridge
participant Store as DiffViewerPreferencesStore
CLI->>FS: read (persistedDiffViewerPreferences)
FS-->>CLI: layout + toggles
CLI->>WV: generate page (payload.layout, payload.viewerOptions)
WV->>WV: initialAppState (seeded from payload)
WV->>Bridge: viewerPrefs.get
Bridge->>Store: preferences()
Store->>FS: read (cached)
Store-->>Bridge: "{layout, wordWrap, ...}"
Bridge-->>WV: "{ok, value.preferences}"
WV->>WV: apply-persisted-options (override initial state)
WV->>Bridge: viewerPrefs.set (on user change)
Bridge->>Store: merge(updates)
Store->>FS: write atomically
Reviews (2): Last reviewed commit: "Retrigger CI (no run scheduled for previ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func merge(_ updates: [String: Any]) -> [String: Any] { | ||
| let sanitizedUpdates = Self.sanitize(updates) | ||
| lock.lock() | ||
| defer { lock.unlock() } | ||
| var merged = loadLocked() | ||
| for (key, value) in sanitizedUpdates { | ||
| merged[key] = value | ||
| } | ||
| cached = merged | ||
| persistLocked(merged) | ||
| return merged | ||
| } |
There was a problem hiding this comment.
merge() acquires NSLock and calls persistLocked, which executes FileManager.createDirectory, JSONSerialization.data, and data.write(to:options:.atomic) while holding the lock — all synchronously on the @MainActor bridge thread. DiffCommentsBridge is @MainActor, so every viewerPrefs.set message from the webview runs this I/O on the main thread. The preferences file is small so latency is negligible today, but the @unchecked Sendable + NSLock pattern bypasses actor isolation — the Swift concurrency rule for this codebase flags this where actor isolation or an async off-main path should own coordination.
| private func persistedDiffViewerPreferences() -> [String: Any] { | ||
| let fileURL: URL | ||
| if let override = ProcessInfo.processInfo.environment["CMUX_DIFF_VIEWER_PREFS_PATH"], | ||
| !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| fileURL = URL(fileURLWithPath: override, isDirectory: false) | ||
| } else if let appSupport = FileManager.default.urls( | ||
| for: .applicationSupportDirectory, | ||
| in: .userDomainMask | ||
| ).first { | ||
| fileURL = appSupport | ||
| .appendingPathComponent("cmux", isDirectory: true) | ||
| .appendingPathComponent("diff-viewer", isDirectory: true) | ||
| .appendingPathComponent("preferences.json", isDirectory: false) | ||
| } else { | ||
| return [:] | ||
| } | ||
| guard let data = try? Data(contentsOf: fileURL), | ||
| let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any] else { | ||
| return [:] | ||
| } | ||
| var sanitized: [String: Any] = [:] | ||
| if let layout = object["layout"] as? String, layout == "split" || layout == "unified" { | ||
| sanitized["layout"] = layout | ||
| } | ||
| if let indicators = object["diffIndicators"] as? String, | ||
| indicators == "bars" || indicators == "classic" || indicators == "none" { | ||
| sanitized["diffIndicators"] = indicators | ||
| } | ||
| for key in ["wordWrap", "wordDiffs", "lineNumbers", "showBackgrounds", "expandUnchanged"] { | ||
| if let value = object[key] as? Bool { | ||
| sanitized[key] = value | ||
| } | ||
| } | ||
| return sanitized | ||
| } |
There was a problem hiding this comment.
DiffViewerPreferencesStore.sanitize()
persistedDiffViewerPreferences() re-implements the same validation (layout enum, diffIndicators enum, five boolean keys) that DiffViewerPreferencesStore.sanitize() already encodes. The CLI cannot import the app target so the duplication is unavoidable at the binary level, but the two tables will drift if a new preference key is added to one side and missed on the other. A shared comment or schema reference would make it obvious when they need to be kept in sync.
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!
| patch = try joinedGitDiffPatches( | ||
| [gitStdout(gitDiffPatchArguments(["--"]), in: repoRoot)] | ||
| + gitUntrackedPaths(in: repoRoot).map { path in | ||
| try gitAddedUntrackedPatch(path: path, in: repoRoot) | ||
| } | ||
| ) |
There was a problem hiding this comment.
gitUntrackedPaths failure aborts the entire --unstaged diff
gitUntrackedPaths is now throws. If git ls-files --others --exclude-standard fails for any reason, the thrown error propagates out and the caller receives no diff at all — not even the regular git diff output already computed in [gitStdout(gitDiffPatchArguments(["--"]), in: repoRoot)]. Previously --unstaged never called ls-files, so this is a new failure mode. A non-throwing helper that returns [] on error would degrade gracefully: users see the staged/modified changes even when ls-files has a problem.
CMUXOpenCommandTests.swift +106 (regression tests), cmux_open.swift +77 (untracked-in-unstaged, persisted prefs, new shortcut actions), KeyboardShortcutSettings.swift +24 (four new diff viewer actions). Regenerated with scripts/swift_file_length_budget.py --write-budget, which also ratchets down entries that shrank on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9fa2556. Configure here.
| + gitUntrackedPaths(in: repoRoot).map { path in | ||
| try gitAddedUntrackedPatch(path: path, in: repoRoot) | ||
| } | ||
| ) |
There was a problem hiding this comment.
One bad untracked fails unstaged
Medium Severity
Building the unstaged patch runs git diff and then generates a patch for every untracked path in one throwing chain. If any single untracked path cannot be diffed, the whole unstaged read fails, so tracked unstaged edits and other untracked files never appear in the viewer.
Reviewed by Cursor Bugbot for commit 9fa2556. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
webviews/src/App.tsx (1)
1257-1329: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSoft refresh starts overlapping diff streams without cancellation.
The generation guard prevents stale callbacks from mutating state, but previous
streamPatchruns still continue after refresh. Repeated refreshes can stack concurrent parse/fetch work and degrade responsiveness. Add effect cleanup cancellation (AbortSignal or explicit stream cancel handle) so superseded generations stop work, not just ignore callbacks.
🤖 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 `@CLI/cmux_open.swift`:
- Around line 1497-1505: The current unstaged-patch generation code iterates
gitUntrackedPaths and calls gitAddedUntrackedPatch for each path, spawning an
unbounded subprocess per file (see gitUntrackedPaths, gitAddedUntrackedPatch,
joinedGitDiffPatches, gitStdout, gitDiffPatchArguments); change this to a
bounded strategy by introducing a configurable batch size constant (e.g.
UNTRACKED_BATCH_SIZE) and produce patches in batches (group N paths, call
gitAddedUntrackedPatch or a batched variant for that group) and if there are
more files than the cap append a single synthetic "TRUNCATED_UNTRACKED_FILES"
patch or metadata entry to signal truncation so callers know results are partial
and avoid per-file process explosion.
- Around line 1282-1288: Update the CLI help/usage text that describes the
--layout flag to state that when --layout is omitted the app will prefer the
user's persisted viewer preference (persistedDiffViewerPreferences()["layout"])
over the settings-file default (diffViewerDefaultLayoutSetting()), rather than
always falling back to the settings default; locate the string that documents
the --layout option in the CLI usage/help output and change its wording to
reflect this new precedence (persisted viewer prefs win, then settings default,
then "unified" fallback).
In `@Sources/DiffViewerPreferencesStore.swift`:
- Around line 21-23: preferences() currently trusts the in-memory cached
dictionary and never re-reads preferences.json, and merge() updates cached
before the file write succeeds; change behavior so the on-disk file is
authoritative: acquire lock, read and decode fileURL (preferences.json) on each
call to preferences() to refresh stale/cold state before returning; in merge(),
merge into a temporary copy, write the updated JSON to disk first (handling
write errors), and only after a successful write update the in-memory cached
property; ensure all file read/write and cached accesses are protected by the
existing lock (NSLock) and keep fileURL, cached, preferences(), and merge() as
the key symbols to modify.
In `@webviews/src/App.tsx`:
- Around line 284-287: navigateHunk is rebuilding
buildHunkAnchors(current.items) on every keystroke causing repeated O(n) scans;
instead compute and cache the anchors when the source collection changes (e.g.,
when latestState.current.items updates) and have navigateHunk read from that
cached value (store in a ref or on latestState) so nextHunkIndex and
hunkNavIndex.current use the precomputed anchors; apply the same caching for the
other hot-path call sites noted (around the 321-326 block) to avoid repeated
full scans.
🪄 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: df35aa03-24e5-436a-a8c9-b47f236afa37
📒 Files selected for processing (21)
CLI/cmux_open.swiftPackages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftPackages/CmuxSettings/Tests/CmuxSettingsTests/ShortcutActionNumberedDigitTests.swiftResources/Localizable.xcstringsResources/markdown-viewer/webviews-app/chunks/diffSurface.mjsSources/DiffViewerPreferencesStore.swiftSources/KeyboardShortcutContext.swiftSources/KeyboardShortcutSettings.swiftSources/Panels/DiffCommentsBridge.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CMUXOpenCommandTests.swiftweb/data/cmux-shortcuts.tsweb/data/cmux.schema.jsonwebviews/src/App.tsxwebviews/src/types.tswebviews/src/viewer-nav.tswebviews/src/viewer-prefs.tswebviews/test/app.test.tsxwebviews/test/viewer-nav.test.tswebviews/test/viewer-prefs.test.ts
| // The user's last in-viewer layout choice (persisted by the app's | ||
| // viewerPrefs bridge) wins over the settings-file default, so new diff | ||
| // panels open the way the user last left one (#5284). | ||
| if let persisted = persistedDiffViewerPreferences()["layout"] as? String { | ||
| return (persisted, "default") | ||
| } | ||
| return (diffViewerDefaultLayoutSetting() ?? "unified", "default") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update CLI usage text for the new layout precedence.
Layout now prefers persisted viewer prefs when --layout is omitted, but Line 6191 still says the default is unified/settings-based. That help output is now misleading.
Suggested help-text update
- --layout <split|unified> Diff layout (default: unified; configurable via diffViewer.defaultLayout in cmux.json)
+ --layout <split|unified> Diff layout (default: last persisted viewer layout; otherwise diffViewer.defaultLayout or unified)🤖 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 `@CLI/cmux_open.swift` around lines 1282 - 1288, Update the CLI help/usage text
that describes the --layout flag to state that when --layout is omitted the app
will prefer the user's persisted viewer preference
(persistedDiffViewerPreferences()["layout"]) over the settings-file default
(diffViewerDefaultLayoutSetting()), rather than always falling back to the
settings default; locate the string that documents the --layout option in the
CLI usage/help output and change its wording to reflect this new precedence
(persisted viewer prefs win, then settings default, then "unified" fallback).
| // Untracked files are part of the unstaged working-tree state but | ||
| // plain `git diff` omits them, which silently hides files agents | ||
| // just created. Append an added-file patch per untracked path. | ||
| patch = try joinedGitDiffPatches( | ||
| [gitStdout(gitDiffPatchArguments(["--"]), in: repoRoot)] | ||
| + gitUntrackedPaths(in: repoRoot).map { path in | ||
| try gitAddedUntrackedPatch(path: path, in: repoRoot) | ||
| } | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound untracked patch fan-out in --unstaged generation.
Line 1502 spawns one git diff --no-index subprocess per untracked path. This is an unbounded per-target batch scan and can become very slow in large repos with many untracked files. Add a bounded/benchmarked strategy (e.g., explicit cap + truncation signaling, or a batched generation path) before merge.
As per coding guidelines, production scalable paths must flag per-target rescans and unbenchmarked algorithm choices.
🤖 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 `@CLI/cmux_open.swift` around lines 1497 - 1505, The current unstaged-patch
generation code iterates gitUntrackedPaths and calls gitAddedUntrackedPatch for
each path, spawning an unbounded subprocess per file (see gitUntrackedPaths,
gitAddedUntrackedPatch, joinedGitDiffPatches, gitStdout, gitDiffPatchArguments);
change this to a bounded strategy by introducing a configurable batch size
constant (e.g. UNTRACKED_BATCH_SIZE) and produce patches in batches (group N
paths, call gitAddedUntrackedPatch or a batched variant for that group) and if
there are more files than the cap append a single synthetic
"TRUNCATED_UNTRACKED_FILES" patch or metadata entry to signal truncation so
callers know results are partial and avoid per-file process explosion.
Source: Coding guidelines
| private let lock = NSLock() | ||
| private let fileURL: URL? | ||
| private var cached: [String: Any]? |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the file authoritative over the in-memory cache.
After the first load, preferences() never revalidates preferences.json, and merge() updates cached before the write succeeds. Any external update or local write failure can leave the bridge serving a different preference set than the persisted file, which breaks this persistence path’s source-of-truth contract.
As per coding guidelines, cached substitutions in persistence paths must handle cold and stale caches explicitly.
Also applies to: 54-64, 83-113
🧰 Tools
🪛 SwiftLint (0.63.3)
[Warning] 23-23: Prefer empty collection over optional collection
(discouraged_optional_collection)
🤖 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/DiffViewerPreferencesStore.swift` around lines 21 - 23, preferences()
currently trusts the in-memory cached dictionary and never re-reads
preferences.json, and merge() updates cached before the file write succeeds;
change behavior so the on-disk file is authoritative: acquire lock, read and
decode fileURL (preferences.json) on each call to preferences() to refresh
stale/cold state before returning; in merge(), merge into a temporary copy,
write the updated JSON to disk first (handling write errors), and only after a
successful write update the in-memory cached property; ensure all file
read/write and cached accesses are protected by the existing lock (NSLock) and
keep fileURL, cached, preferences(), and merge() as the key symbols to modify.
Source: Coding guidelines
| const navigateHunk = (direction: 1 | -1) => { | ||
| const current = latestState.current; | ||
| const anchors = buildHunkAnchors(current.items); | ||
| const index = nextHunkIndex(anchors, hunkNavIndex.current, current.activeItemId, direction); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid rebuilding all hunk anchors on every next/prev-hunk keystroke.
navigateHunk currently calls buildHunkAnchors(current.items) per keypress, which does a full scan each time. On large diffs, this creates avoidable repeated O(n) work on an interaction hot path.
♻️ Proposed fix
-import { useCallback, useEffect, useReducer, useRef, useState } from "react";
+import { useCallback, useEffect, useMemo, useReducer, useRef, useState } from "react";
@@
- const hunkNavIndex = useRef(-1);
+ const hunkNavIndex = useRef(-1);
+ const hunkAnchors = useMemo(() => buildHunkAnchors(state.items), [state.items]);
@@
- const anchors = buildHunkAnchors(current.items);
+ const anchors = hunkAnchors;As per coding guidelines, production code over scalable user data should flag repeated full-collection scans in hot paths.
Also applies to: 321-326
🤖 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 `@webviews/src/App.tsx` around lines 284 - 287, navigateHunk is rebuilding
buildHunkAnchors(current.items) on every keystroke causing repeated O(n) scans;
instead compute and cache the anchors when the source collection changes (e.g.,
when latestState.current.items updates) and have navigateHunk read from that
cached value (store in a ref or on latestState) so nextHunkIndex and
hunkNavIndex.current use the precomputed anchors; apply the same caching for the
other hot-path call sites noted (around the 321-326 block) to avoid repeated
full scans.
Source: Coding guidelines
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| } catch { | ||
| // A fallback candidate that cannot be read (empty, or e.g. | ||
| // last-turn without a workspace/surface context) is skipped; | ||
| // only the originally selected source's own failure may | ||
| // surface. Otherwise `cmux diff --unstaged` in a clean repo | ||
| // outside a cmux terminal dead-ends on a raw last-turn | ||
| // context error instead of the friendly empty state (#5246). | ||
| continue |
There was a problem hiding this comment.
The outer catch block (changed from catch is EmptyDiffSourceError) now swallows every error thrown by writeDeferredDiffViewerSource for every fallback candidate — including git failures (e.g., corrupted index, git binary missing) and disk-write errors that indicate real problems.
Concrete failure: the user's --unstaged view is empty, the staged fallback is tried, git returns an error while reading the staged index (corrupted pack or misconfigured git config). The old code would surface that error; the new code silently falls through to the friendly empty state. The user now sees "no changes" when there are actually staged diffs that failed to load.
The intended fix was specifically for the case where last-turn context throws a non-EmptyDiffSourceError because there is no workspace/surface context. Catching that specific error class (or matching on it) would avoid swallowing genuine git or write failures from other fallback sources.
1 participant
Round 1 of diff viewer improvements from issue triage (#5284, #2526, #5246, #609).
Untracked files in the unstaged source.
cmux diff --unstagedwas plaingit diff, so files an agent just created were invisible in the default review view. The unstaged patch now appends an added-file patch per untracked path (gitignored files excluded), reusing the samegit diff --no-indexhelper the last-turn source already uses.Viewer preferences persist globally (fixes #5284). Layout and the options-menu toggles previously lived in page-local
localStoragekeyed to generated viewer origins, which does not reliably persist. A newDiffViewerPreferencesStorepersists them to~/Library/Application Support/cmux/diff-viewer/preferences.json; the webview saves through newviewerPrefs.get/viewerPrefs.setmethods on the existingcmuxDiffCommentsbridge and re-syncs at boot, and the CLI reads the same file at generation time so new diff panels open with the last-used layout. Explicit--layoutstill wins.localStorageremains a fallback for pages opened outside cmux, with legacy-key migration.Refresh preserves viewer state (the bug in #5284). The options-menu Refresh was
window.location.reload(), resetting layout and options. It now soft-refreshes: re-streams the patch in place behind a render-generation guard, keeping layout and all toggles. Status-only pages keep the full reload so pending replacements still resolve. Content regeneration on refresh (re-running git) is unchanged from before and is the round-2 live-refresh work.Keyboard navigation between hunks and files (from #2526).
n/pjump to the next / previous hunk,]/[to the next / previous file, following the existing vim-style in-viewer shortcuts (j/kscroll). Wired throughKeyboardShortcutSettings, theCmuxSettingsShortcutActionenum, the CLI payload,cmux.jsonschema, shortcut docs data, and Settings; all rebindable.Deferred empty-state fallback no longer dead-ends (remaining #5246 path). When the selected source was empty and a fallback candidate failed for a non-empty reason (last-turn without workspace/surface context), the raw error rendered as the page. Reproduced identically on a main baseline build (pre-existing). Unusable fallback candidates are now skipped, so the friendly empty state renders. This also makes the existing
testDiffCommandShowsFriendlyEmptyStateWhenEveryGitSourceIsEmptypass as written.Commit 1 adds the regression tests only (CI red), the fixes follow (CI green). New bun tests cover the prefs sanitizer/bridge fallback and hunk/file navigation helpers; new XCTests cover untracked-in-unstaged, persisted-prefs payload seeding, and the new shortcut defaults.
Localization: new shortcut labels in
Localizable.xcstringsfor all 20 locales (en + ja translated, others fall back to English per existing convention); docs entries carry en + ja.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches git patch assembly and diff-source fallback logic in the CLI plus global shortcut routing; behavior changes are user-visible but localized to the diff viewer path.
Overview
Unstaged diffs now include untracked (non-ignored) files by appending per-file added patches alongside plain
git diff, so agent-created files show up in the default unstaged view.Viewer preferences are read from persisted
diff-viewer/preferences.json(with optionalCMUX_DIFF_VIEWER_PREFS_PATH): default layout follows the user’s last in-viewer choice before settings defaults, and generated pages seedviewerOptions(wrap, line numbers, etc.) for first paint.Empty / fallback sources: when the selected diff source is empty, all failed fallback candidates are skipped—not only explicitly empty ones—so a clean repo or missing last-turn context still gets the friendly empty state instead of a raw error.
Keyboard navigation adds rebindable diff-viewer actions:
n/pnext/previous hunk,]/[next/previous file, wired throughcmux_openshortcut payloads,CmuxSettingsShortcutAction, defaults, tests, and localized labels.Reviewed by Cursor Bugbot for commit fce83b0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Improves the diff viewer with untracked files in Unstaged, globally persisted viewer preferences (with CLI seeding and soft refresh), and vim‑style hunk/file navigation. Also restores the friendly empty state for clean repos.
New Features
cmux diff --unstaged; gitignored files are excluded.DiffViewerPreferencesStoreandviewerPrefs.get/setbridge; the CLI reads the same file to seed layout and display options at first paint (viewerOptions), with--layoutstill overriding. SupportsCMUX_DIFF_VIEWER_PREFS_PATH;localStorageremains a fallback outsidecmux.n/pjump hunks,]/[jump files. Rebindable via Settings and reflected incmux.jsonschema and docs.Bug Fixes
Written for commit fce83b0. Summary will update on new commits.
Summary by CodeRabbit
n), previous hunk (p), next file (]), previous file ([).