fix(split-pane): stop breaking every settings save, finish the desktop gate

Blocker from Ark0N's second PR #453 pass: moving showSplitButton into
settings-ui.js's per-device displayKeys set was only half of making it
per-device. saveAppSettings() still put it in the object PUT to
/api/settings, SettingsUpdateSchema (.strict()) does not declare it,
the server answered 400 INVALID_INPUT, and because the call site never
checked res.ok the UI still reported "Settings saved" while NOTHING
persisted — workspaceHooksEnabled, agentSkillEnabled, tunnelEnabled,
claudeModel, every toggle, on every save, on every device. Strip it
out via the same destructure every other per-device key goes through
(`showSplitButton: _ssp,`), drop the stray mention from a schemas.ts
comment (a mention there reads as "this is a real field" to the next
grep), and add a static guard test mirroring
test/terminal-auto-copy.test.ts's three-way rule.

Also finishes the desktop gate the first pass only did in CSS at
599px: SPLIT_PANE_MIN_WIDTH (1180, matching HOME_SESSIONS_MIN_WIDTH)
now backs an actual JS width check in _applySplitButtonVisibility,
with a matchMedia listener so a live window resize hides/shows the
button without a reload — the CSS backstop in styles.css is the
reverse-direction guarantee for when JS hasn't run.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
timkjr
2026-09-20 13:10:32 -05:00
co-authored by Claude Sonnet 5
parent 6c8bd6c606
commit 165cfb52d6
3 changed files with 61 additions and 9 deletions
+9 -5
View File
@@ -2352,6 +2352,10 @@ Object.assign(CodemanApp.prototype, {
showPlanUsageLimits: _pul, showPlanUsageLimits: _pul,
showAttachmentsButton: _ahb, showAttachmentsButton: _ahb,
showFileViewerButton: _fvb, showFileViewerButton: _fvb,
// Desktop-only header button, per-device, and absent from
// SettingsUpdateSchema (.strict()) — sending it 400s the whole PUT
// (moving it into displayKeys alone is not the strip; this is).
showSplitButton: _ssp,
webglRendererEnabled: _wgl, webglRendererEnabled: _wgl,
terminalWheelLocalScrollback: _twls, terminalWheelLocalScrollback: _twls,
// Copy-on-select. Per-device (clipboard access differs by device and by // Copy-on-select. Per-device (clipboard access differs by device and by
@@ -2950,12 +2954,12 @@ Object.assign(CodemanApp.prototype, {
multiMonitorBtn.classList.toggle('btn-multimonitor--hidden', !showMultiMonitorButton); multiMonitorBtn.classList.toggle('btn-multimonitor--hidden', !showMultiMonitorButton);
} }
// Split button — hidden by default // Split button — hidden by default, and hard-gated to desktop widths
// regardless of the setting (window.CodemanSplitPane.SPLIT_PANE_MIN_WIDTH,
// matching HOME_SESSIONS_MIN_WIDTH's JS-check + media-query-backstop
// pattern — the CSS in styles.css is the backstop, this is the check).
const showSplitButton = settings.showSplitButton ?? defaults.showSplitButton ?? false; const showSplitButton = settings.showSplitButton ?? defaults.showSplitButton ?? false;
const splitBtn = document.querySelector('.btn-split'); this._applySplitButtonVisibility?.(showSplitButton);
if (splitBtn) {
splitBtn.classList.toggle('btn-split--hidden', !showSplitButton);
}
// Ultracode/Workflow agents launcher — hidden by default; reveal when enabled. // Ultracode/Workflow agents launcher — hidden by default; reveal when enabled.
// Marker class only (base is display:inline-flex !important) so it's auto-excluded // Marker class only (base is display:inline-flex !important) so it's auto-excluded
-4
View File
@@ -1376,10 +1376,6 @@ export const SettingsUpdateSchema = z
showFileBrowser: z.boolean().optional(), showFileBrowser: z.boolean().optional(),
showSubagents: z.boolean().optional(), showSubagents: z.boolean().optional(),
showMultiMonitorButton: z.boolean().optional(), showMultiMonitorButton: z.boolean().optional(),
// showSplitButton is per-device (displayKeys in settings-ui.js) and
// deliberately NOT declared here, matching showFileViewerButton/skin/etc:
// a desktop opt-in must never sync onto a phone that never asked for it,
// and the phone header hard-hides .btn-split regardless (mobile.css).
// Doubles as the plan-usage telemetry COLLECTION switch, read fresh from // Doubles as the plan-usage telemetry COLLECTION switch, read fresh from
// disk by readPlanUsageTelemetryEnabled() (hooks-config.ts) at every claude // disk by readPlanUsageTelemetryEnabled() (hooks-config.ts) at every claude
// session create/respawn — not just the chip's DISPLAY preference. See that // session create/respawn — not just the chip's DISPLAY preference. See that
@@ -0,0 +1,52 @@
// test/split-pane-per-device-setting.test.ts
// Port: none (pure static analysis — runs in CI, no browser/server).
//
// Regression guard for the review finding that landed the blocker: moving
// showSplitButton into settings-ui.js's per-device `displayKeys` set is only
// HALF of making a setting per-device. The other half is stripping it out of
// the object `saveAppSettings()` PUTs to `/api/settings` — displayKeys is a
// client-side merge policy, not a wire filter. Without the strip, every save
// sent `showSplitButton` in the body, `SettingsUpdateSchema` (.strict()) does
// not declare it, the server answered 400 INVALID_INPUT, and because the
// call site never checked `res.ok` the UI still reported "Settings saved"
// while NOTHING persisted — workspaceHooksEnabled, agentSkillEnabled,
// tunnelEnabled, claudeModel, every toggle, on every save, on every device.
//
// Mirrors test/terminal-auto-copy.test.ts's "keeps the toggle per-device"
// guard for autoCopySelection — same three-way rule, same shape of test.
import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { join } from 'node:path';
const HERE = fileURLToPath(new URL('.', import.meta.url));
const PUBLIC = join(HERE, '../src/web/public');
function read(file: string): string {
return readFileSync(join(PUBLIC, file), 'utf8');
}
describe('showSplitButton stays per-device: display key, stripped from the PUT, absent from the schema', () => {
const settingsUi = read('settings-ui.js');
const schemas = readFileSync(join(HERE, '../src/web/schemas.ts'), 'utf8');
it('is in the client-side displayKeys merge policy', () => {
const displayKeys = settingsUi.slice(
settingsUi.indexOf('const displayKeys = new Set(['),
settingsUi.indexOf('])', settingsUi.indexOf('const displayKeys = new Set(['))
);
expect(displayKeys).toContain("'showSplitButton'");
});
it('is stripped out of the object saveAppSettings() PUTs to the server', () => {
// The strip is a destructure: `showSplitButton: _ssp,` pulls the key out
// of `settings` so it never reaches `...serverSettings` in the PUT body.
expect(settingsUi).toContain('showSplitButton: _ssp,');
});
it('is never declared in the .strict() SettingsUpdateSchema', () => {
// Not even in a comment — a mention there reads as "this is a real
// field" to the next person grepping schemas.ts for it.
expect(schemas).not.toContain('showSplitButton');
});
});