Merge pull request #565: feat(settings): add an Apply button that saves without closing

.changeset/settings-apply-button.md

@@ -0,0 +1,5 @@
+---
+'aicodeman': minor
+---
+
+Settings has an Apply button next to Save. It saves the same way but keeps Settings open, so switching on MCP server sync makes its Preview and Sync usable straight away, and CLI management's add, enable and disable work without a close-and-reopen. The MCP sync hint now says to Apply or Save first.

docs/architecture-invariants.md

@@ -728,7 +728,7 @@ The guard is a one-shot flag on each trap rather than a browser test or a readin
 
 **One visual language, three modals.** `#appSettingsModal`, `#sessionOptionsModal` and `#createCaseModal` share the `set-*` surface (left rail, sections of grouped row cards, label + description on the left, control pinned right) through a single `:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal)` scope in `styles.css`. An `:is()` list takes the specificity of its **most specific argument**, and all three arguments are ids, so every rule kept exactly the weight it had when the block was `#appSettingsModal`-only: nothing downstream shifted in the cascade. That property is what let the surface absorb Session Options and then Add Case in two separate commits without a cascade audit each time.
 
-Anatomy: `.set-shell` → `.set-shell-head` (title + `.set-head-actions`) + `.set-body` (grid of `.set-rail` and `.set-doc`). The rail holds `.set-search`, `.set-rail-items` and `.set-rail-foot`; the document holds `.set-section`s, each with `.set-section-head` + `.set-section-blurb` + `.set-group`s of `.set-row`s (`.set-row-text` = `.set-row-label` + `.set-row-desc`; controls sit in `.set-row-actions`). ⚠️ Close is deliberately **first in the DOM** inside `.set-head-actions` so the focus trap lands on it rather than on Save; `row-reverse` is what puts Save to its left visually on phones.
+Anatomy: `.set-shell` → `.set-shell-head` (title + `.set-head-actions`) + `.set-body` (grid of `.set-rail` and `.set-doc`). The rail holds `.set-search`, `.set-rail-items` and `.set-rail-foot`; the document holds `.set-section`s, each with `.set-section-head` + `.set-section-blurb` + `.set-group`s of `.set-row`s (`.set-row-text` = `.set-row-label` + `.set-row-desc`; controls sit in `.set-row-actions`). ⚠️ Close is deliberately **first in the DOM** inside `.set-head-actions` so the focus trap lands on it rather than on Save; `row-reverse` is what puts Save to its left visually on phones. Apply (`applyAppSettings()`) sits beside Save in the same actions and saves without closing; it only sets a one-shot flag that `saveAppSettings()` consumes before its first await, so a Save clicked mid-Apply still closes the sheet.
 
 **The rail means two different things, and that is deliberate:**
 

docs/wiki/Settings-Reference.md

@@ -152,7 +152,7 @@ instead of its native cloud backend. See [Custom Model Endpoints](Custom-Model-E
 | Default Codex reasoning effort   | Reasoning level for new local Codex sessions; empty uses Codex's own config.                   |
 | Bypass approvals and sandbox     | Starts new Codex sessions with `--dangerously-bypass-approvals-and-sandbox`. Read [Agent CLIs](Agent-CLIs) before enabling. |
 | Animated status effects          | Cosmetic.                                                                                      |
-| MCP server sync                  | Copies the MCP servers each installed, enabled CLI (Claude, Codex, Gemini, OpenCode, Antigravity) has into the others' own config files. Synced, off by default, admin only in multi-user mode. Turn it on and save, then **Preview** shows what would change and **Sync now** applies it. It only adds missing servers, keeps the previous file as `.codeman-bak`, and leaves a file that receives env values or headers readable by you only. A config dir moved by `CODEX_HOME`, `CLAUDE_CONFIG_DIR`, `XDG_CONFIG_HOME` or `GEMINI_CLI_HOME` in Codeman's own environment is followed. |
+| MCP server sync                  | Copies the MCP servers each installed, enabled CLI (Claude, Codex, Gemini, OpenCode, Antigravity) has into the others' own config files. Synced, off by default, admin only in multi-user mode. Turn it on and **Apply** or **Save** (Apply keeps Settings open), then **Preview** shows what would change and **Sync now** applies it. It only adds missing servers, keeps the previous file as `.codeman-bak`, and leaves a file that receives env values or headers readable by you only. A config dir moved by `CODEX_HOME`, `CLAUDE_CONFIG_DIR`, `XDG_CONFIG_HOME` or `GEMINI_CLI_HOME` in Codeman's own environment is followed. |
 
 ### Notifications
 

src/web/public/i18n.js

@@ -721,6 +721,8 @@
 
     // Dynamic common status / toasts
     'Settings saved': '设置已保存',
+    'Settings applied': '设置已应用',
+    'Save and keep Settings open': '保存并保持设置打开',
     'Settings saved locally': '设置已保存到本机',
     'Tunnel active': '隧道已启用',
     'Tunnel starting — QR code will appear when ready...': '隧道正在启动，准备好后将显示二维码…',

src/web/public/index.html

@@ -1635,6 +1635,7 @@
                Save; row-reverse keeps Save to the left of it on phones. -->
           <div class="set-head-actions">
             <button class="modal-close" onclick="app.closeAppSettings()" aria-label="Close app settings">&times;</button>
+            <button class="set-head-save set-head-apply" onclick="app.applyAppSettings()" title="Save and keep Settings open">Apply</button>
             <button class="set-head-save" onclick="app.saveAppSettings()">Save</button>
           </div>
         </div>
@@ -3117,6 +3118,7 @@
         </div>
         <div class="form-actions set-foot">
           <button class="btn-toolbar" onclick="app.closeAppSettings()">Cancel</button>
+          <button class="btn-toolbar" onclick="app.applyAppSettings()" title="Save and keep Settings open">Apply</button>
           <button class="btn-toolbar btn-primary" onclick="app.saveAppSettings()">Save</button>
         </div>
       </div>

src/web/public/mobile.css

@@ -3552,7 +3552,7 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat
   }
 
   /* Save + Close are the two ways out of the sheet (save-and-close vs
-     discard-and-close), hit in the same corner with the same thumb, so here —
+     discard-and-close; Apply saves but keeps the sheet open), hit in the same corner with the same thumb, so here —
      and only here, since Save is header-only below 860px — they share a
      recessed tray and matching pill geometry instead of reading as a fat
      accent pill parked beside a stray × glyph. Tray colors come from skin

src/web/public/settings-ui.js

@@ -1227,12 +1227,12 @@ Object.assign(CodemanApp.prototype, {
   /** Preview (apply=false) or run (apply=true) the MCP server sync across enabled CLIs. */
   async mcpSync(apply) {
     const out = this.$('mcpSyncResult');
-    const show = (html) => {
-      if (out) { out.style.display = 'block'; out.innerHTML = html; }
+    const show = (html, hint = '') => {
+      if (out) { out.style.display = 'block'; out.innerHTML = html; out.dataset.hint = hint; }
     };
     // Switched on in this modal but not saved yet: the routes would only answer "disabled".
     if (!this._mcpSyncSavedOn) {
-      show('Save settings to turn MCP sync on first, then reopen Settings to preview or sync.');
+      show('Apply or Save settings to turn MCP sync on first, then preview or sync.', 'save-first');
       return;
     }
     if (apply && !confirm('Add missing MCP servers to every installed, enabled CLI\'s config file? Env values and headers on those servers are copied too.')) return;
@@ -2513,7 +2513,31 @@ Object.assign(CodemanApp.prototype, {
     claudeEl.className = 'voice-provider-status' + (status?.available ? ' active' : '');
   },
 
+  /**
+   * Apply button: the same save as Save, but the modal stays open and the MCP sync group (its
+   * Preview and Sync need the saved flag) and the CLI management writes are refreshed in place, so
+   * turning either on needs no close-and-reopen. It is a wrapper rather than an option on
+   * saveAppSettings() so that function's signature (which tests locate by text) stays as it was.
+   *
+   * `_keepSettingsOpenOnce` is the one-shot intent and `_applyInFlight` the double-click guard:
+   * saveAppSettings() consumes the intent before its first await, so a Save clicked while an Apply
+   * is still in flight is an ordinary Save and closes the modal.
+   */
+  async applyAppSettings() {
+    if (this._applyInFlight) return;
+    this._applyInFlight = true;
+    this._keepSettingsOpenOnce = true;
+    try {
+      await this.saveAppSettings();
+    } finally {
+      this._applyInFlight = false;
+      this._keepSettingsOpenOnce = false;
+    }
+  },
+
   async saveAppSettings() {
+    const keepOpen = this._keepSettingsOpenOnce === true;
+    this._keepSettingsOpenOnce = false;
     // Gesture overlay is injected at page render (server-side), so a change to it
     // only takes effect on reload — remember the prior value to decide below.
     const _prev = this.loadAppSettingsFromStorage();
@@ -2859,6 +2883,7 @@ Object.assign(CodemanApp.prototype, {
       ...serverSettings
     } = settings;
     let webhookError = '';
+    let serverSaved = false;
     try {
       const res = await this._apiPut('/api/settings', {
         ...serverSettings,
@@ -2876,10 +2901,14 @@ Object.assign(CodemanApp.prototype, {
         this.saveAppSettingsToStorage(settings);
         const cb = document.getElementById('appSettingsTunnelEnabled');
         if (cb) cb.checked = false;
-        this.closeAppSettings();
+        if (!keepOpen) this.closeAppSettings();
         return;
       }
 
+      // `_apiPut` answers null or a non-ok response instead of throwing, so this is the only
+      // evidence the server kept the flags the Apply refresh below reads.
+      serverSaved = !!res?.ok;
+
       // Save model configuration separately
       await this.saveModelConfigFromSettings();
 
@@ -2891,7 +2920,7 @@ Object.assign(CodemanApp.prototype, {
       if (webhookError) {
         this.showToast(`Settings saved, but not the webhook: ${webhookError}`, 'warning');
       } else {
-        this.showToast('Settings saved', 'success');
+        this.showToast(keepOpen ? 'Settings applied' : 'Settings saved', 'success');
       }
 
       // Show tunnel-specific feedback if toggled on
@@ -2903,9 +2932,13 @@ Object.assign(CodemanApp.prototype, {
       this.showToast('Settings saved locally', 'warning');
     }
 
+    // Only when the settings PUT landed: after a 400 or a dropped connection the server still has the
+    // old flags, and a webhook-only failure still saved the rest, so this runs ahead of that branch.
+    if (keepOpen && serverSaved) this._refreshSettingsAfterApply(settings);
+
     if (webhookError) {
       document.getElementById('webhookGroup')?.scrollIntoView({ block: 'center' });
-    } else {
+    } else if (!keepOpen) {
       this.closeAppSettings();
     }
 
@@ -2926,6 +2959,25 @@ Object.assign(CodemanApp.prototype, {
     }
   },
 
+  /**
+   * After Apply: bring the groups whose contents depend on a SAVED value up to date without
+   * reopening the modal. openAppSettings does the same on open; this is the part of it that
+   * a save can change, without touching what the user is editing or the scroll position.
+   */
+  _refreshSettingsAfterApply(settings) {
+    // The MCP routes read the saved flag, so switching it on is only usable from now.
+    this._mcpSyncSavedOn = settings.mcpSyncEnabled === true;
+    const out = this.$('mcpSyncResult');
+    if (this._mcpSyncSavedOn && out && out.dataset?.hint === 'save-first') {
+      out.style.display = 'none';
+      out.innerHTML = '';
+    }
+    this.applyMcpSyncVisibility();
+    this.applyCustomModelEndpointsVisibility();
+    this.applyCliManagementVisibility();
+    this._applyDoctorAdminGate();
+  },
+
   // Load model configuration from server for the settings modal
   async loadModelConfigForSettings() {
     try {

src/web/public/styles.css

@@ -17005,6 +17005,12 @@ html[data-tab-orientation='vertical'] .home-sessions {
     transform 0.1s ease;
 }
 
+:is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save.set-head-apply {
+  background: transparent;
+  color: var(--text);
+  box-shadow: inset 0 0 0 1px var(--border);
+}
+
 :is(#appSettingsModal, #sessionOptionsModal, #createCaseModal) .set-head-save:hover {
   filter: brightness(1.08);
 }

test/settings-apply-button.test.ts

@@ -0,0 +1,204 @@
+/**
+ * @fileoverview Settings "Apply": saves like Save but keeps the modal open and refreshes the
+ * groups that depend on a saved value (MCP sync, custom model endpoints, CLI management).
+ */
+import { readFileSync } from 'node:fs';
+import { resolve } from 'node:path';
+import vm from 'node:vm';
+import { describe, expect, it, vi } from 'vitest';
+
+const publicDir = resolve(import.meta.dirname, '../src/web/public');
+const html = readFileSync(resolve(publicDir, 'index.html'), 'utf8');
+const settingsUi = readFileSync(resolve(publicDir, 'settings-ui.js'), 'utf8');
+
+function loadApp() {
+  const CodemanApp = function CodemanApp() {} as unknown as { prototype: Record<string, unknown> };
+  const context = vm.createContext({
+    CodemanApp,
+    localStorage: { getItem: () => null, setItem: () => {} },
+    document: { getElementById: () => null, querySelectorAll: () => [], addEventListener: () => {} },
+    window: {},
+    console,
+  });
+  vm.runInContext(settingsUi, context, { filename: 'settings-ui.js' });
+  return new (CodemanApp as unknown as new () => Record<string, any>)();
+}
+
+describe('Settings Apply button', () => {
+  it('sits next to Save in the footer and the phone header, wired to applyAppSettings', () => {
+    const start = html.indexOf('<div class="modal" id="appSettingsModal">');
+    const modal = html.slice(start, html.indexOf('<!-- Shortcut Overlay Modal -->', start));
+    expect(modal.match(/onclick="app\.applyAppSettings\(\)"/g)).toHaveLength(2);
+    expect(modal.match(/onclick="app\.saveAppSettings\(\)"/g)).toHaveLength(2);
+  });
+
+  it('raises the keep-open intent for the save it wraps and clears it afterwards', async () => {
+    const app = loadApp();
+    const seen: boolean[] = [];
+    app.saveAppSettings = vi.fn(async () => {
+      seen.push(app._keepSettingsOpenOnce);
+    });
+    await app.applyAppSettings();
+    expect(seen).toEqual([true]);
+    expect(app._keepSettingsOpenOnce).toBe(false);
+    expect(app._applyInFlight).toBe(false);
+  });
+
+  it('clears the flag even when the save throws, and ignores a second click mid-save', async () => {
+    const app = loadApp();
+    let release!: () => void;
+    app.saveAppSettings = vi.fn(() => new Promise<void>((r) => (release = r)));
+    const first = app.applyAppSettings();
+    await app.applyAppSettings();
+    expect(app.saveAppSettings).toHaveBeenCalledTimes(1);
+    release();
+    await first;
+
+    app.saveAppSettings = vi.fn(async () => {
+      throw new Error('boom');
+    });
+    await expect(app.applyAppSettings()).rejects.toThrow('boom');
+    expect(app._applyInFlight).toBe(false);
+    expect(app._keepSettingsOpenOnce).toBe(false);
+  });
+
+  it('refreshes the dependent groups from the saved values', () => {
+    const app = loadApp();
+    const out = {
+      textContent: 'Anything: the hint is found by its marker, not its (translatable) text',
+      dataset: { hint: 'save-first' },
+      style: { display: 'block' },
+      innerHTML: 'x',
+    };
+    app.$ = (id: string) => (id === 'mcpSyncResult' ? out : null);
+    app.applyMcpSyncVisibility = vi.fn();
+    app.applyCustomModelEndpointsVisibility = vi.fn();
+    app.applyCliManagementVisibility = vi.fn();
+    app._applyDoctorAdminGate = vi.fn();
+    app._mcpSyncSavedOn = false;
+
+    app._refreshSettingsAfterApply({ mcpSyncEnabled: true });
+
+    expect(app._mcpSyncSavedOn).toBe(true);
+    expect(out.style.display).toBe('none');
+    expect(app.applyMcpSyncVisibility).toHaveBeenCalled();
+    expect(app.applyCustomModelEndpointsVisibility).toHaveBeenCalled();
+    expect(app.applyCliManagementVisibility).toHaveBeenCalled();
+  });
+
+  describe('the real saveAppSettings()', () => {
+    /** An element that answers any property read, call or write: enough for a DOM-heavy function to run. */
+    function genericElement(): any {
+      const el: any = new Proxy(function () {}, {
+        get: (_t, prop) => {
+          if (prop === 'value') return '';
+          if (prop === 'checked') return false;
+          if (prop === Symbol.toPrimitive) return () => '';
+          if (prop === 'then') return undefined;
+          return el;
+        },
+        set: () => true,
+        apply: () => el,
+      });
+      return el;
+    }
+
+    function loadRealApp(put: () => Promise<unknown>) {
+      const CodemanApp = function CodemanApp() {} as unknown as { prototype: Record<string, unknown> };
+      const el = genericElement();
+      const context = vm.createContext({
+        CodemanApp,
+        localStorage: { getItem: () => null, setItem: () => {} },
+        document: new Proxy(
+          {
+            getElementById: () => el,
+            querySelectorAll: () => [],
+            querySelector: () => el,
+            addEventListener: () => {},
+            body: el,
+            documentElement: el,
+          },
+          { get: (t: any, prop: string) => (prop in t ? t[prop] : el) }
+        ),
+        window: {},
+        location: { reload: vi.fn() },
+        VoiceInput: new Proxy({}, { get: () => vi.fn(() => ({})) }),
+        KeyboardAccessoryBar: new Proxy({}, { get: () => vi.fn(() => ({})) }),
+        MobileDetection: new Proxy({}, { get: () => vi.fn(() => false) }),
+        setTimeout,
+        console,
+      });
+      vm.runInContext(settingsUi, context, { filename: 'settings-ui.js' });
+      const target = new (CodemanApp as unknown as new () => Record<string, any>)();
+      // saveAppSettings() leans on helpers from the other settings mixins; anything it asks for
+      // that is not defined is a harmless stub, while the state the Apply logic owns stays real.
+      const state = new Set(['_keepSettingsOpenOnce', '_applyInFlight', '_mcpSyncSavedOn']);
+      const app: Record<string, any> = new Proxy(target, {
+        get: (t, prop) =>
+          prop in t || typeof prop !== 'string' || state.has(prop) || prop === 'then'
+            ? t[prop as string]
+            : vi.fn(() => 14),
+      });
+      app.loadAppSettingsFromStorage = () => ({});
+      app.saveAppSettingsToStorage = vi.fn();
+      app.notificationManager = new Proxy(
+        { preferences: {}, normalizePreferences: (p: unknown) => p, getToastDurationMs: () => 3000 },
+        { get: (t: any, prop: string) => (prop in t ? t[prop] : vi.fn()) }
+      );
+      app._apiPut = vi.fn(put);
+      app._handleTunnelEnableRefusal = vi.fn(async () => false);
+      app.saveModelConfigFromSettings = vi.fn(async () => {});
+      app._webhookPending = () => false;
+      app.showToast = vi.fn();
+      app.closeAppSettings = vi.fn();
+      app._refreshSettingsAfterApply = vi.fn();
+      // The apply*() helpers re-skin and re-lay-out the page from real DOM geometry; that is not what
+      // these tests are about, so they are stubbed. applyAppSettings() itself stays real.
+      for (const name of Object.keys(CodemanApp.prototype)) {
+        if (/^apply[A-Z]/.test(name) && name !== 'applyAppSettings') app[name] = vi.fn();
+      }
+      return app;
+    }
+
+    it('a Save clicked while an Apply is in flight closes the modal, and the Apply keeps it open', async () => {
+      let resolvePut!: (v: unknown) => void;
+      const first = new Promise((r) => (resolvePut = r));
+      const app = loadRealApp(() => first);
+      const apply = app.applyAppSettings();
+      // The Apply's PUT is still pending; the user clicks Save now.
+      const save = app.saveAppSettings();
+      resolvePut({ ok: true });
+      await Promise.all([apply, save]);
+      expect(app.closeAppSettings).toHaveBeenCalledTimes(1);
+      expect(app._refreshSettingsAfterApply).toHaveBeenCalledTimes(1);
+      const toasts = app.showToast.mock.calls.map((c: unknown[]) => c[0]);
+      expect(toasts).toContain('Settings applied');
+      expect(toasts).toContain('Settings saved');
+    });
+
+    it('does not refresh the dependent groups when the settings PUT failed', async () => {
+      for (const failure of [{ ok: false }, null]) {
+        const app = loadRealApp(async () => failure);
+        await app.applyAppSettings();
+        expect(app._refreshSettingsAfterApply, JSON.stringify(failure)).not.toHaveBeenCalled();
+        expect(app.closeAppSettings).not.toHaveBeenCalled();
+      }
+    });
+
+    it('still refreshes when only the webhook failed (the rest was saved)', async () => {
+      const app = loadRealApp(async () => ({ ok: true }));
+      app._webhookPending = () => true;
+      app.saveWebhook = vi.fn(async () => 'bad URL');
+      await app.applyAppSettings();
+      expect(app._refreshSettingsAfterApply).toHaveBeenCalledTimes(1);
+      expect(app.closeAppSettings).not.toHaveBeenCalled();
+    });
+
+    it('a plain Save closes the modal and never refreshes in place', async () => {
+      const app = loadRealApp(async () => ({ ok: true }));
+      await app.saveAppSettings();
+      expect(app.closeAppSettings).toHaveBeenCalledTimes(1);
+      expect(app._refreshSettingsAfterApply).not.toHaveBeenCalled();
+    });
+  });
+});