Remove unused advanced zone field; add explicit Identity Save button

Two changes:

1. Remove 'Internal zone name (advanced)' from Settings. The field
   edited _identity.domain (the internal .cell TLD) which no user
   should ever change post-install — changing it breaks all internal
   service DNS. Removed the Advanced collapse section and the
   showAdvancedZone state. The LAN-mode 'Local Domain' field is kept
   since that mode genuinely needs a user-editable domain value.

2. Add an explicit Save button to the Cell Identity section. The
   previous auto-save fix (no auto-save for pic_ngo cell name changes)
   accidentally removed the only way to save those changes. The Save
   button appears whenever the section is dirty and is disabled when:
   - there are validation errors, or
   - domainMode is pic_ngo, cell name changed, and the availability
     check hasn't confirmed the name is free yet.

Adds 8 Vitest regression tests covering Save button visibility,
disabled states, that auto-save is blocked for pic_ngo cell name
changes, and that it still fires for ip_range-only changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

webui/src/__tests__/Settings.IdentitySave.test.jsx

@@ -0,0 +1,208 @@
+/**
+ * Regression tests for Cell Identity save behaviour in Settings.jsx.
+ *
+ * Covers:
+ * - Save button appears only when identity is dirty
+ * - Save button disabled while availability check in progress (pic_ngo, name changed)
+ * - Save button disabled when name is taken (pic_ngo)
+ * - Save button enabled once availability confirmed (pic_ngo, name changed)
+ * - Auto-save does NOT fire for pic_ngo cell_name changes (requires explicit Save)
+ * - Auto-save DOES fire for ip_range-only changes in pic_ngo mode
+ */
+import { render, screen, fireEvent, waitFor, act } from '@testing-library/react';
+import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
+
+// ── API mocks ─────────────────────────────────────────────────────────────────
+
+const mockGetConfig = vi.fn();
+const mockUpdateConfig = vi.fn();
+const mockListBackups = vi.fn();
+const mockGetStatus = vi.fn();
+const mockCheckName = vi.fn();
+const mockGetCertStatus = vi.fn();
+
+vi.mock('../services/api', () => ({
+  cellAPI: {
+    getConfig: (...a) => mockGetConfig(...a),
+    updateConfig: (...a) => mockUpdateConfig(...a),
+    listBackups: (...a) => mockListBackups(...a),
+  },
+  ddnsAPI: {
+    getStatus: (...a) => mockGetStatus(...a),
+    checkName: (...a) => mockCheckName(...a),
+  },
+  caddyAPI: {
+    getCertStatus: (...a) => mockGetCertStatus(...a),
+  },
+}));
+
+vi.mock('../contexts/ConfigContext', () => ({
+  useConfig: () => ({ domain: 'cell', cell_name: 'pic1', refresh: vi.fn() }),
+}));
+
+vi.mock('../contexts/DraftConfigContext', () => ({
+  useDraftConfig: () => ({
+    registerFlusher: vi.fn(() => vi.fn()),
+    setDirty: vi.fn(),
+    hasDirty: vi.fn(() => false),
+    flushAll: vi.fn(),
+  }),
+}));
+
+vi.mock('../utils/serviceConfig', () => ({
+  PORT_CONFLICT_FIELDS: {},
+  detectPortConflicts: vi.fn(() => ({})),
+  validateServiceConfig: vi.fn(() => ({})),
+  SERVICE_DEFS: [],
+}));
+
+// ── helpers ───────────────────────────────────────────────────────────────────
+
+function makeCfg(overrides = {}) {
+  return {
+    cell_name: 'pic1',
+    domain: 'cell',
+    ip_range: '172.20.0.0/16',
+    domain_mode: 'pic_ngo',
+    domain_name: 'pic1.pic.ngo',
+    effective_domain: 'pic1.pic.ngo',
+    ddns: { has_token: true },
+    service_configs: {},
+    installed_services: [],
+    ...overrides,
+  };
+}
+
+async function renderSettings() {
+  const { default: Settings } = await import('../pages/Settings.jsx');
+  let result;
+  await act(async () => {
+    result = render(<Settings />);
+    // Flush the loadAll async chain
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+  // Wait until the cell_name input appears (confirms loadAll completed)
+  await waitFor(() => screen.getByDisplayValue('pic1'));
+  return result;
+}
+
+// ── shared beforeEach ─────────────────────────────────────────────────────────
+
+function defaultMocks() {
+  mockGetConfig.mockResolvedValue({ data: makeCfg() });
+  mockUpdateConfig.mockResolvedValue({ data: { warnings: [] } });
+  mockListBackups.mockResolvedValue({ data: [] });
+  mockGetStatus.mockResolvedValue({ data: { registered: true, public_ip: '1.2.3.4', last_ip: '1.2.3.4' } });
+  mockGetCertStatus.mockResolvedValue({ data: null });
+  mockCheckName.mockResolvedValue({ data: { available: true } });
+}
+
+// ── tests ─────────────────────────────────────────────────────────────────────
+
+describe('Cell Identity — Save button visibility', () => {
+  beforeEach(() => { vi.useFakeTimers({ shouldAdvanceTime: true }); vi.clearAllMocks(); defaultMocks(); });
+  afterEach(async () => { await act(async () => { vi.runAllTimers(); }); vi.useRealTimers(); vi.resetModules(); });
+
+  it('Save button is absent when identity is clean', async () => {
+    await renderSettings();
+    expect(screen.queryByRole('button', { name: 'Save' })).not.toBeInTheDocument();
+  });
+
+  it('Save button appears after editing ip_range', async () => {
+    await renderSettings();
+    fireEvent.change(screen.getByDisplayValue('172.20.0.0/16'), { target: { value: '10.0.0.0/8' } });
+    expect(screen.getByRole('button', { name: 'Save' })).toBeInTheDocument();
+  });
+});
+
+describe('Cell Identity — pic_ngo cell name change gating', () => {
+  beforeEach(() => { vi.useFakeTimers({ shouldAdvanceTime: true }); vi.clearAllMocks(); defaultMocks(); });
+  afterEach(async () => { await act(async () => { vi.runAllTimers(); }); vi.useRealTimers(); vi.resetModules(); });
+
+  it('Save button is disabled while availability check is in progress', async () => {
+    mockCheckName.mockReturnValue(new Promise(() => {})); // never resolves → stays 'checking'
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('pic1'), { target: { value: 'pic2' } });
+    await act(async () => { vi.advanceTimersByTime(950); }); // past 900 ms debounce
+
+    expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled();
+  });
+
+  it('Save button is disabled when name is taken', async () => {
+    mockCheckName.mockResolvedValue({ data: { available: false } });
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('pic1'), { target: { value: 'taken-name' } });
+    await act(async () => { vi.advanceTimersByTime(950); });
+    await act(async () => { await Promise.resolve(); });
+
+    expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled();
+  });
+
+  it('Save button is enabled once name is confirmed available', async () => {
+    mockCheckName.mockResolvedValue({ data: { available: true } });
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('pic1'), { target: { value: 'pic2' } });
+    await act(async () => { vi.advanceTimersByTime(950); });
+    await act(async () => { await Promise.resolve(); });
+
+    expect(screen.getByRole('button', { name: 'Save' })).not.toBeDisabled();
+  });
+
+  it('auto-save does NOT fire for pic_ngo cell_name changes', async () => {
+    mockCheckName.mockResolvedValue({ data: { available: true } });
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('pic1'), { target: { value: 'pic2' } });
+
+    // Advance well past both debounces (availability check: 900 ms, auto-save: 800 ms)
+    await act(async () => { vi.advanceTimersByTime(2500); });
+    await act(async () => { await Promise.resolve(); });
+
+    expect(mockUpdateConfig).not.toHaveBeenCalled();
+  });
+
+  it('clicking Save calls updateConfig with new cell_name', async () => {
+    mockCheckName.mockResolvedValue({ data: { available: true } });
+    mockGetConfig
+      .mockResolvedValueOnce({ data: makeCfg() })
+      .mockResolvedValue({ data: makeCfg({ cell_name: 'pic2', domain_name: 'pic2.pic.ngo' }) });
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('pic1'), { target: { value: 'pic2' } });
+    await act(async () => { vi.advanceTimersByTime(950); });
+    await act(async () => { await Promise.resolve(); });
+
+    await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Save' })); });
+    await act(async () => { await Promise.resolve(); });
+
+    expect(mockUpdateConfig).toHaveBeenCalledOnce();
+    expect(mockUpdateConfig.mock.calls[0][0]).toMatchObject({ cell_name: 'pic2' });
+  });
+});
+
+describe('Cell Identity — ip_range auto-save (name unchanged in pic_ngo mode)', () => {
+  beforeEach(() => { vi.useFakeTimers({ shouldAdvanceTime: true }); vi.clearAllMocks(); defaultMocks(); });
+  afterEach(async () => { await act(async () => { vi.runAllTimers(); }); vi.useRealTimers(); vi.resetModules(); });
+
+  it('auto-save fires after 800 ms when only ip_range changes', async () => {
+    mockGetConfig
+      .mockResolvedValueOnce({ data: makeCfg() })
+      .mockResolvedValue({ data: makeCfg({ ip_range: '10.0.0.0/8' }) });
+    await renderSettings();
+
+    fireEvent.change(screen.getByDisplayValue('172.20.0.0/16'), { target: { value: '10.0.0.0/8' } });
+
+    await act(async () => { vi.advanceTimersByTime(500); });
+    expect(mockUpdateConfig).not.toHaveBeenCalled(); // not yet
+
+    await act(async () => { vi.advanceTimersByTime(400); }); // total 900 ms
+    await act(async () => { await Promise.resolve(); });
+
+    expect(mockUpdateConfig).toHaveBeenCalledOnce();
+    expect(mockUpdateConfig.mock.calls[0][0]).toMatchObject({ ip_range: '10.0.0.0/8' });
+  });
+});

webui/src/pages/Settings.jsx

@@ -340,7 +340,6 @@ function Settings() {
   const [identityDirty, setIdentityDirty] = useState(false);
   const [loadedCellName, setLoadedCellName] = useState('');
   const [effectiveDomain, setEffectiveDomain] = useState('');
-  const [showAdvancedZone, setShowAdvancedZone] = useState(false);
 
   // DDNS
   const [domainMode, setDomainMode] = useState('lan');
@@ -842,28 +841,6 @@ function Settings() {
                 </div>
                 <span className="text-xs text-gray-400">managed by DDNS</span>
               </div>
-              <div className="mt-1">
-                <button
-                  type="button"
-                  onClick={() => setShowAdvancedZone((v) => !v)}
-                  className="text-xs text-gray-400 hover:text-gray-600 flex items-center gap-1"
-                >
-                  {showAdvancedZone ? <ChevronDown className="h-3 w-3" /> : <ChevronRight className="h-3 w-3" />}
-                  Advanced
-                </button>
-                {showAdvancedZone && (
-                  <div className="mt-2 pl-1 border-l-2 border-gray-100">
-                    <Field label="Internal zone name (advanced)" error={identity.domain && identity.domain.length > 255 ? 'Domain must be 255 characters or fewer' : null} hint="Used for LAN DNS — most users should not change this">
-                      <TextInput
-                        value={identity.domain}
-                        onChange={(v) => { setIdentity((i) => ({ ...i, domain: v })); setIdentityDirty(true); draftConfig?.setDirty('identity', true); }}
-                        placeholder="cell"
-                        maxLength={255}
-                      />
-                    </Field>
-                  </div>
-                )}
-              </div>
             </Field>
           )}
           <Field label="IP Range" hint="Docker bridge subnet" error={ipRangeError}>
@@ -873,6 +850,20 @@ function Settings() {
               placeholder="172.20.0.0/16"
             />
           </Field>
+          {identityDirty && (
+            <div className="flex justify-end pt-1">
+              <button
+                onClick={saveIdentity}
+                disabled={
+                  !!cellNameError || !!ipRangeError || !!domainError
+                  || (domainMode === 'pic_ngo' && identity.cell_name !== loadedCellName && picAvail !== 'available')
+                }
+                className="btn-primary text-sm disabled:opacity-50"
+              >
+                Save
+              </button>
+            </div>
+          )}
         </div>
       </Section>