From 1068dfeb2ba118a8f1ad4061b7934d19d58289a2 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 12:17:28 -0700 Subject: [PATCH 1/2] Diff viewer: Group Files by Kind is a setting, so the toggle sticks The toolbar's "Group files by kind" reset itself on every reopen. The page persists a display option by sending `diff.` to the host, the host refuses a `diff.*` key it has no schema row for (DiffPrefKey.accepts), and saveViewerPrefs swallows that refusal on purpose -- so the one option with no row silently never persisted. It was kept session-local and off by default for exactly that reason (2026-10-20 review of #22951). This gives it the row: `diff.groupByKind`, a toggle in Settings > Appearance > Diff Viewer, agent-settable with the rest of the diff page's looks (it is in DiffViewerSettingsSchema, which SettingsSchema+AgentPolicy unions whole), in DiffPrefKey.booleans so the host accepts the write, and titled in all 21 locales. On the page, `groupByKind` moves into `defaultDiffViewerOptions` with the other schema-backed defaults, is sanitized and persisted like them, and `setOption` no longer skips it. It stays off by default. The familiar order is the order the diff arrived in, and now that the toggle lasts past the tab, turning it on is the reader's to make; whether it becomes the default is a separate, reviewable change. Red then green: `bun test test/viewer-prefs-host-keys.test.ts` fails its new case against the old host list ("expect(accepted.has(\"groupByKind\")).toBe(true)" receives false) and passes with it. `test/diff-option-defaults.test.ts` holds every key of `defaultDiffViewerOptions` to the exported schema, so it stays red until the Mac settings export regenerates schemas/settings/settings-schema.json for the new row; that commit follows. Co-Authored-By: Claude Opus 5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red --- .../Sources/CmuxNextDiff/DiffPrefs.swift | 3 +- .../CmuxNextSettings/Localizable.xcstrings | 131 ++++++++++++++++++ .../Schema/DiffViewerSettingsSchema.swift | 5 + webviews/src/App.tsx | 4 +- webviews/src/diff-viewer/state.ts | 22 ++- webviews/src/viewer-prefs.ts | 23 +-- webviews/test/viewer-prefs-host-keys.test.ts | 15 +- 7 files changed, 175 insertions(+), 28 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDiff/DiffPrefs.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDiff/DiffPrefs.swift index 52bf311f26f5..36d2cd107bc0 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextDiff/DiffPrefs.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDiff/DiffPrefs.swift @@ -9,7 +9,8 @@ import Foundation /// the page keeps (webviews/src/viewer-prefs.ts `sanitizeViewerPrefs`). public nonisolated enum DiffPrefKey { public static let section = "diff" - static let booleans: Set = ["wordWrap", "wordDiffs", "lineNumbers", "showBackgrounds", "expandUnchanged"] + static let booleans: Set = ["wordWrap", "wordDiffs", "lineNumbers", "showBackgrounds", "expandUnchanged", + "groupByKind"] static let choices: [String: Set] = [ "layout": ["split", "unified"], "diffIndicators": ["bars", "classic", "none"], diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings b/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings index 889e5ebc49f3..54469408af71 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings @@ -62889,6 +62889,137 @@ } } }, + "settings.diff.groupByKind": { + "extractionState": "manual", + "localizations": { + "en": { + "stringUnit": { + "state": "translated", + "value": "Group Files by Kind" + } + }, + "ja": { + "stringUnit": { + "state": "needs_review", + "value": "ファイルを種類別にグループ化" + } + }, + "ar": { + "stringUnit": { + "state": "needs_review", + "value": "تجميع الملفات حسب النوع" + } + }, + "bs": { + "stringUnit": { + "state": "needs_review", + "value": "Grupiši datoteke po vrsti" + } + }, + "da": { + "stringUnit": { + "state": "needs_review", + "value": "Gruppér filer efter type" + } + }, + "de": { + "stringUnit": { + "state": "needs_review", + "value": "Dateien nach Art gruppieren" + } + }, + "es": { + "stringUnit": { + "state": "needs_review", + "value": "Agrupar archivos por tipo" + } + }, + "fr": { + "stringUnit": { + "state": "needs_review", + "value": "Grouper les fichiers par type" + } + }, + "it": { + "stringUnit": { + "state": "needs_review", + "value": "Raggruppa i file per tipo" + } + }, + "km": { + "stringUnit": { + "state": "needs_review", + "value": "ដាក់ជាក្រុមឯកសារតាមប្រភេទ" + } + }, + "ko": { + "stringUnit": { + "state": "needs_review", + "value": "파일을 종류별로 그룹화" + } + }, + "nb": { + "stringUnit": { + "state": "needs_review", + "value": "Grupper filer etter type" + } + }, + "pl": { + "stringUnit": { + "state": "needs_review", + "value": "Grupuj pliki według rodzaju" + } + }, + "pt-BR": { + "stringUnit": { + "state": "needs_review", + "value": "Agrupar arquivos por tipo" + } + }, + "ru": { + "stringUnit": { + "state": "needs_review", + "value": "Группировать файлы по типу" + } + }, + "th": { + "stringUnit": { + "state": "needs_review", + "value": "จัดกลุ่มไฟล์ตามประเภท" + } + }, + "tr": { + "stringUnit": { + "state": "needs_review", + "value": "Dosyaları türe göre grupla" + } + }, + "uk": { + "stringUnit": { + "state": "needs_review", + "value": "Групувати файли за типом" + } + }, + "vi": { + "stringUnit": { + "state": "needs_review", + "value": "Nhóm tệp theo loại" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "needs_review", + "value": "按类型分组文件" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "needs_review", + "value": "按類型分組檔案" + } + } + } + }, "settings.diff.layout": { "extractionState": "manual", "localizations": { diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift index db2f0e7eb344..b954c54f414c 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift @@ -50,6 +50,11 @@ nonisolated enum DiffViewerSettingsSchema { toggle("showBackgrounds", SettingsText.keyed("settings.diff.showBackgrounds", "Change Backgrounds"), true, ["background", "color"]), toggle("expandUnchanged", SettingsText.keyed("settings.diff.expandUnchanged", "Expand Unchanged Lines"), false, ["unchanged", "context", "expand"]), + // Off by default: the familiar order is the order the diff arrived in, and a reorder + // nobody can turn off has no business being the default. The toolbar's toggle writes + // this row, so turning it on now lasts past the tab (webviews/src/viewer-prefs.ts). + toggle("groupByKind", SettingsText.keyed("settings.diff.groupByKind", "Group Files by Kind"), false, + ["group", "kind", "tests", "generated", "order", "sort"]), ] } } diff --git a/webviews/src/App.tsx b/webviews/src/App.tsx index 000fe06c1c4f..df8cbf895b3f 100644 --- a/webviews/src/App.tsx +++ b/webviews/src/App.tsx @@ -611,10 +611,10 @@ export function App({ config, initialStatus }: ConfigProps) { dispatch({ type: "set-option", key: "layout", value: layout }); }; // Dispatches an options change and persists it globally when the key is a - // persisted preference (`collapsed` and `groupByKind` stay session-local; see ViewerPrefs). + // persisted preference (`collapsed` stays session-local; see ViewerPrefs). const setOption = (key: keyof DiffViewerOptions, value: any) => { dispatch({ type: "set-option", key, value }); - if (key !== "collapsed" && key !== "groupByKind") { + if (key !== "collapsed") { saveViewerPrefs({ [key]: value }, writes); } }; diff --git a/webviews/src/diff-viewer/state.ts b/webviews/src/diff-viewer/state.ts index b7df2f011743..b939db7d9935 100644 --- a/webviews/src/diff-viewer/state.ts +++ b/webviews/src/diff-viewer/state.ts @@ -132,12 +132,6 @@ export function initialAppState(config: DiffViewerConfig, initialStatus: DiffVie options: { collapsed: false, ...defaultDiffViewerOptions(payload), - // Outside `defaultDiffViewerOptions`, which diff-option-defaults.test.ts holds to the - // exported `diff.*` schema: this one has no schema row yet. The delineated diff is opt-in - // from the toolbar while it is session-local, because a reorder of the file column that a - // reader cannot durably turn off is not a default (familiar first). It moves in there, and - // can become the default, once `diff.groupByKind` has its row and the toggle sticks. - groupByKind: false, ...seededOptions, layout: initialDiffViewerLayout(payload), } as DiffViewerOptions, @@ -156,18 +150,20 @@ export function initialAppState(config: DiffViewerConfig, initialStatus: DiffVie /** * The display options before any stored pref. In cmux NEXT (the page host; its config's transport * is `page`) they are the defaults of the `diff.*` settings (DiffViewerSettingsSchema.swift), and - * diff-option-defaults.test.ts holds every key here to the exported schema — which is why - * `groupByKind`, the one display option with no schema row yet, is defaulted by the caller and - * named in the type rather than left to drift in here. Word changes are on there (Leo, - * 2026-10-20). The classic viewer and the dev server embed their config and keep word changes off. + * diff-option-defaults.test.ts holds every key here to the exported schema. Word changes are on + * there (Leo, 2026-10-20). The classic viewer and the dev server embed their config and keep word + * changes off. + * + * `groupByKind` is off here: the familiar order is the order the diff arrived in, and the grouping + * is turned on from the toolbar. Now that `diff.groupByKind` has a schema row, turning it on lasts + * past the tab, which is what a default would have to be worth before it becomes one. */ -export function defaultDiffViewerOptions( - payload: DiffViewerPayload, -): Omit { +export function defaultDiffViewerOptions(payload: DiffViewerPayload): Omit { return { colors: "theme", diffIndicators: "bars", expandUnchanged: false, + groupByKind: false, lineNumbers: true, showBackgrounds: true, wordDiffs: payload.transport?.kind === "page", diff --git a/webviews/src/viewer-prefs.ts b/webviews/src/viewer-prefs.ts index b755942adfff..cceba9e4d35e 100644 --- a/webviews/src/viewer-prefs.ts +++ b/webviews/src/viewer-prefs.ts @@ -20,15 +20,13 @@ import type { DiffViewerOptions } from "./pierre-options"; * `cmux.diff.prefs.set {key, value}` (diff/pageStore.ts), and web storage is never touched (the * page host pool clears it). The host seeds first paint through `payload.viewerOptions`. * - * `groupByKind` is session-local for now, like `collapsed`: the host only accepts the `diff.*` - * keys it has schema rows for (`DiffPrefKey.booleans`), so sending it would be refused and - * dropped without a word. Giving it a row is the new-setting checklist (schema row, agent policy, - * 21 locales, regenerated settings exports) and lands on its own, once the defaults snapshot is - * green on base; `test/viewer-prefs-host-keys.test.ts` fails if the two lists drift apart again. - * Until then the grouping is off by default (state.ts) and turned on from the toolbar for the - * session: a reorder nobody can durably turn off has no business being the default. + * `groupByKind` is persisted like the rest since it got its `diff.groupByKind` schema row: it + * shipped as a toolbar toggle that reset itself on every reopen, because the host refuses a + * `diff.*` key it has no row for (`DiffPrefKey.booleans`) and `saveViewerPrefs` swallows that + * refusal. `test/viewer-prefs-host-keys.test.ts` fails if the two lists drift apart again. It + * stays off by default (state.ts): a reorder of the file column is the reader's to ask for. */ -export type ViewerPrefs = Partial> & { +export type ViewerPrefs = Partial> & { collapsedFiles?: string[]; }; @@ -51,7 +49,14 @@ export function sanitizeViewerPrefs(raw: unknown): ViewerPrefs { if (source.colors === "theme" || source.colors === "muted" || source.colors === "standard") { prefs.colors = source.colors; } - for (const key of ["wordWrap", "wordDiffs", "lineNumbers", "showBackgrounds", "expandUnchanged"] as const) { + for (const key of [ + "wordWrap", + "wordDiffs", + "lineNumbers", + "showBackgrounds", + "expandUnchanged", + "groupByKind", + ] as const) { if (typeof source[key] === "boolean") { prefs[key] = source[key]; } diff --git a/webviews/test/viewer-prefs-host-keys.test.ts b/webviews/test/viewer-prefs-host-keys.test.ts index 147f6855e2c7..c3a5db929a7e 100644 --- a/webviews/test/viewer-prefs-host-keys.test.ts +++ b/webviews/test/viewer-prefs-host-keys.test.ts @@ -56,8 +56,17 @@ test("every diff pref the page persists is a key the host accepts", async () => test("a session-local option is never persisted", async () => { const persisted = Object.keys(sanitizeViewerPrefs(EVERY_OPTION)); - // `collapsed` is session-local by design; `groupByKind` is session-local until it has a - // `diff.groupByKind` schema row, and the test above is what lets it become persisted safely. + // The collapse-all toggle is session-local by design: it is a way of reading this diff now, not + // a preference about every diff. expect(persisted).not.toContain("collapsed"); - expect(persisted).not.toContain("groupByKind"); +}); + +test("the file grouping is kept, because the host now has a row for it", async () => { + // It shipped as a toolbar toggle that reset itself on every reopen: the page sent + // `diff.groupByKind`, the host had no schema row for it, and `saveViewerPrefs` swallowed the + // refusal. Both halves are asserted here, because either one alone is the silent bug again. + const accepted = await hostKeys(); + expect(accepted.has("groupByKind")).toBe(true); + expect(sanitizeViewerPrefs(EVERY_OPTION).groupByKind).toBe(true); + expect(sanitizeViewerPrefs({ groupByKind: "yes" })).toEqual({}); }); From 7723b8f3cd94d8537f5268ed8c69f04e4cbb0220 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 12:58:29 -0700 Subject: [PATCH 2/2] Diff viewer: the settings exports for Group Files by Kind A new setting is two things: the descriptor in `DiffViewerSettingsSchema`, and the exports every other consumer reads from it -- the MDM manifests, the portable `settings-schema.json`, the defaults snapshot, and the source stamp that proves the exports match the sources that produced them. Only the descriptor was in this branch, so `cmux-next generated files` and the defaults guard were red on a tree whose exports did not name `diff.groupByKind`. These six files come from the sanctioned producer, not from an editor: the PR autofix run of this head (39660516674, `pr-autofix-patch`), which runs `scripts/cmux-next/regenerate-settings-exports.sh` on a Mac and then rewrites the defaults snapshot from the exports it produced. That workflow cannot push to a pull request branch -- its push job is gated `github.event_name != 'pull_request'` (cmux-next-pr-autofix.yml:559) while its only trigger is `pull_request` -- so the artifact is the whole of its output and the lane commits it. The base merge in this branch touches no `CmuxNextSettings` source, so the stamp the Mac computed still matches the tree it lands on. The diff is the one key and nothing else: the row in the Diff Viewer group, its plist and JSON manifest entries, its line in the managed-preferences table, its `false` in the defaults snapshot, and the two source hashes that changed (`DiffViewerSettingsSchema.swift` and `CmuxNextSettings/Localizable.xcstrings`). Co-Authored-By: Claude Opus 5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red --- docs/mdm/com.manaflow.cmux.json | 6 +++ docs/mdm/com.manaflow.cmux.plist | 10 ++++ docs/mdm/managed-preferences.md | 1 + schemas/defaults/cmux-next.json | 1 + .../settings-schema.sources.sha256 | 4 +- schemas/settings/settings-schema.json | 50 ++++++++++++++++++- 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/docs/mdm/com.manaflow.cmux.json b/docs/mdm/com.manaflow.cmux.json index 4ead3d2dbfe2..97d3030e9f89 100644 --- a/docs/mdm/com.manaflow.cmux.json +++ b/docs/mdm/com.manaflow.cmux.json @@ -1639,6 +1639,12 @@ "title" : "Expand Unchanged Lines", "type" : "boolean" }, + "diff.groupByKind" : { + "default" : false, + "description" : "diff.groupByKind", + "title" : "Group Files by Kind", + "type" : "boolean" + }, "diff.layout" : { "default" : "unified", "description" : "diff.layout", diff --git a/docs/mdm/com.manaflow.cmux.plist b/docs/mdm/com.manaflow.cmux.plist index 17015768d4da..4daad8a162af 100644 --- a/docs/mdm/com.manaflow.cmux.plist +++ b/docs/mdm/com.manaflow.cmux.plist @@ -3154,6 +3154,16 @@ pfm_type boolean + + pfm_default + + pfm_name + diff.groupByKind + pfm_title + Group Files by Kind + pfm_type + boolean + pfm_default diff --git a/docs/mdm/managed-preferences.md b/docs/mdm/managed-preferences.md index 80c77d5e7ad9..bea5b75bc50e 100644 --- a/docs/mdm/managed-preferences.md +++ b/docs/mdm/managed-preferences.md @@ -213,6 +213,7 @@ Files: `com.manaflow.cmux.plist` (ProfileManifests: iMazing Profile Editor, Prof | `diff.lineNumbers` | boolean | `true` | | Line Numbers | | `diff.showBackgrounds` | boolean | `true` | | Change Backgrounds | | `diff.expandUnchanged` | boolean | `false` | | Expand Unchanged Lines | +| `diff.groupByKind` | boolean | `false` | | Group Files by Kind | | `markdown.remoteImages` | boolean | `false` | | Load Web Images Automatically. Web image sites can see your address and when you open a document. When off, each document asks before loading its web images. | | `markdown.layout.width` | real | `72` | 30 to 1000 | Text Width. The widest a line of text gets, in characters (30 to 1000). On a wider window the text stays centered with margins. Wide tables and code scroll sideways in their own box. | | `markdown.layout.fullWidth` | boolean | `false` | | Full Width. Text fills the whole pane instead of the text width. | diff --git a/schemas/defaults/cmux-next.json b/schemas/defaults/cmux-next.json index 4c8b934dc8df..1088187d346c 100644 --- a/schemas/defaults/cmux-next.json +++ b/schemas/defaults/cmux-next.json @@ -400,6 +400,7 @@ "setting.diff.colors": "theme", "setting.diff.diffIndicators": "bars", "setting.diff.expandUnchanged": false, + "setting.diff.groupByKind": false, "setting.diff.layout": "unified", "setting.diff.lineNumbers": true, "setting.diff.showBackgrounds": true, diff --git a/schemas/settings-export/settings-schema.sources.sha256 b/schemas/settings-export/settings-schema.sources.sha256 index 38f135d266ac..2275317c0d68 100644 --- a/schemas/settings-export/settings-schema.sources.sha256 +++ b/schemas/settings-export/settings-schema.sources.sha256 @@ -206,7 +206,7 @@ f6190cfeca4fa8b6e7a8fc495646e4f1febdbf801e79cac5030ae811aa5e28c1 Packages/macOS 69d7e7fe1d80c27c5ca44664c5b145d3151b9f3374f7c1885d8d66168c2ffdf4 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/LinkRules/BrowserExternalURLWildcardToken.swift -445674dd450a4be3157cba291e8a345c289f570312d46de47e15841e75ab8ce0 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings +4f16449fb662357304182eeca1942d39598fec1057a5c63234458995b38a0487 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Localizable.xcstrings e9de7365e47f67dee0d803eb974aa58404888670588b0064be42b523dc463ffa Packages/macOS/CmuxNext/Sources/CmuxNextSettings/MainPaneSetting.swift @@ -322,7 +322,7 @@ b2c7a42ded8d358de1d247e1a95761c6924f48f9b55ef2b50b55897f515a67c1 Packages/macOS a9aa2826a915c5886057e7f782451f9d9c3d8d4f5cbcd4bed953040944b15566 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DeterministicJSON.swift -f8b2429d8bd0ab7336241ffc38b147582440a6829d44e5a30b0795cf21d42bc0 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift +a5b9b7a60596f31e8e1e5af4b215665dc822d664d1da568d5d7bdf073f69f2f3 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/DiffViewerSettingsSchema.swift e36a5c327fb10a60497bdfebd32c71ca655869150d9d6adf0fd2142822d84a97 Packages/macOS/CmuxNext/Sources/CmuxNextSettings/Schema/EditorSettingsSchema.swift diff --git a/schemas/settings/settings-schema.json b/schemas/settings/settings-schema.json index 37640d5410e4..8b0ba4ec0b2f 100644 --- a/schemas/settings/settings-schema.json +++ b/schemas/settings/settings-schema.json @@ -131,7 +131,8 @@ "diff.wordDiffs", "diff.lineNumbers", "diff.showBackgrounds", - "diff.expandUnchanged" + "diff.expandUnchanged", + "diff.groupByKind" ], "title" : { "key" : "settings.group.diffViewer", @@ -16252,6 +16253,53 @@ }, "validation" : "portable" }, + { + "accepts" : [ + true, + false + ], + "agent_refusal" : null, + "agent_settable" : true, + "consumers" : [ + "cmux-next" + ], + "default" : false, + "default_label" : null, + "group" : { + "key" : "settings.group.diffViewer", + "text" : "Diff Viewer" + }, + "help" : null, + "kept_on_reset_all" : false, + "key" : "diff.groupByKind", + "keywords" : [ + "diff", + "group", + "kind", + "tests", + "generated", + "order", + "sort" + ], + "kind" : "toggle", + "page_hidden" : false, + "path" : [ + "diff", + "groupByKind" + ], + "refuses" : [ + "true", + 1, + null + ], + "section" : "appearance", + "summary" : null, + "title" : { + "key" : "settings.diff.groupByKind", + "text" : "Group Files by Kind" + }, + "validation" : "portable" + }, { "accepts" : [ true,