From a438ec67328aabd840d8c58b7aab978435e50056 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 07:26:23 -0500
Subject: [PATCH 01/11] =?UTF-8?q?feat(machine):=20Settings=20tab=20?=
 =?UTF-8?q?=E2=80=94=20name/description,=20access=20toggles,=20delete?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace the ComingSoonTab placeholder in the Machine page's Settings tab
with a full-width form over the machine-settings route. Mirrors
TerminalAccessCard's optimistic persist() pattern (local update → PATCH →
rollback + toast on error); no React Hook Form, no SWR.

- Name + description text fields (persist on blur)
- visibleToGlobalAssistant / allowPageAgents access switches (persist on toggle)
- Destructive Delete Machine gated behind AlertDialog; on success navigates
  back to the drive off the now-trashed page
- Colocated tests: optimistic toggle + rollback-on-error, delete-confirm gate

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 136 ++++++++
 .../page-views/terminal/tabs/SettingsTab.tsx  | 292 +++++++++++++++++-
 2 files changed, 422 insertions(+), 6 deletions(-)
 create mode 100644 apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
new file mode 100644
index 0000000000..19fe912f05
--- /dev/null
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -0,0 +1,136 @@
+import { describe, test, beforeEach, vi } from 'vitest';
+import { render, screen, waitFor, within } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { assert } from '@/stores/__tests__/riteway';
+import type { MachineSettings } from '@pagespace/lib/services/machines/machine-settings';
+
+const push = vi.fn();
+vi.mock('next/navigation', () => ({
+  useRouter: () => ({ push }),
+  useParams: () => ({ driveId: 'drive-1' }),
+}));
+
+vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
+
+const fetchWithAuth = vi.fn();
+const apiPatch = vi.fn();
+const apiDelete = vi.fn();
+vi.mock('@/lib/auth/auth-fetch', () => ({
+  fetchWithAuth: (...args: unknown[]) => fetchWithAuth(...args),
+  patch: (...args: unknown[]) => apiPatch(...args),
+  del: (...args: unknown[]) => apiDelete(...args),
+}));
+
+import SettingsTab from './SettingsTab';
+import { toast } from 'sonner';
+
+const baseSettings: MachineSettings = {
+  name: 'My Machine',
+  description: 'does things',
+  visibleToGlobalAssistant: false,
+  allowPageAgents: false,
+};
+
+function mockLoad(settings: MachineSettings = baseSettings) {
+  fetchWithAuth.mockResolvedValue({ ok: true, json: async () => ({ settings }) });
+}
+
+describe('SettingsTab', () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  test('loads the machine settings into the form', async () => {
+    mockLoad();
+    render(<SettingsTab machineId="m1" />);
+
+    const nameInput = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    const globalToggle = screen.getByRole('switch', { name: /visible to global assistant/i });
+
+    assert({
+      given: 'a Machine whose settings load successfully',
+      should: 'populate the name field and reflect the access toggle state',
+      actual: { name: nameInput.value, visible: globalToggle.getAttribute('aria-checked') },
+      expected: { name: 'My Machine', visible: 'false' },
+    });
+  });
+
+  test('toggling an access switch optimistically PATCHes and reconciles with the server', async () => {
+    mockLoad();
+    apiPatch.mockResolvedValue({ settings: { ...baseSettings, visibleToGlobalAssistant: true } });
+    render(<SettingsTab machineId="m1" />);
+
+    const toggle = await screen.findByRole('switch', { name: /visible to global assistant/i });
+    await userEvent.click(toggle);
+
+    await waitFor(() => assert({
+      given: 'the visible-to-global-assistant switch clicked',
+      should: 'PATCH the settings route with the machineId and the flipped flag',
+      actual: apiPatch.mock.calls[0],
+      expected: ['/api/machines/settings', { machineId: 'm1', visibleToGlobalAssistant: true }],
+    }));
+
+    await waitFor(() => assert({
+      given: 'a successful PATCH response',
+      should: 'leave the switch on and never toast an error',
+      actual: { checked: toggle.getAttribute('aria-checked'), errored: (toast.error as ReturnType<typeof vi.fn>).mock.calls.length },
+      expected: { checked: 'true', errored: 0 },
+    }));
+  });
+
+  test('a failed PATCH rolls the toggle back and surfaces an error toast', async () => {
+    mockLoad();
+    apiPatch.mockRejectedValue(new Error('nope'));
+    render(<SettingsTab machineId="m1" />);
+
+    const toggle = await screen.findByRole('switch', { name: /allow page agents/i });
+    await userEvent.click(toggle);
+
+    await waitFor(() => assert({
+      given: 'the PATCH rejected',
+      should: 'show an error toast',
+      actual: (toast.error as ReturnType<typeof vi.fn>).mock.calls.length,
+      expected: 1,
+    }));
+
+    await waitFor(() => assert({
+      given: 'the optimistic update rolled back',
+      should: 'restore the switch to its pre-click (off) state',
+      actual: toggle.getAttribute('aria-checked'),
+      expected: 'false',
+    }));
+  });
+
+  test('Delete Machine is gated behind the confirm dialog', async () => {
+    mockLoad();
+    apiDelete.mockResolvedValue({ success: true, spriteTornDown: true });
+    render(<SettingsTab machineId="m1" />);
+
+    // Wait for load, then open the confirm dialog via the trigger.
+    await screen.findByLabelText('Name');
+    const trigger = screen.getByRole('button', { name: /delete machine/i });
+    await userEvent.click(trigger);
+
+    assert({
+      given: 'the delete trigger clicked (dialog opened, not confirmed)',
+      should: 'NOT call the DELETE route yet',
+      actual: apiDelete.mock.calls.length,
+      expected: 0,
+    });
+
+    const dialog = await screen.findByRole('alertdialog');
+    await userEvent.click(within(dialog).getByRole('button', { name: /delete machine/i }));
+
+    await waitFor(() => assert({
+      given: 'the confirm action clicked',
+      should: 'DELETE the machine by id',
+      actual: apiDelete.mock.calls[0],
+      expected: ['/api/machines/settings?machineId=m1'],
+    }));
+
+    await waitFor(() => assert({
+      given: 'a successful delete',
+      should: 'navigate back to the drive, off the trashed page',
+      actual: push.mock.calls[0],
+      expected: ['/dashboard/drive-1'],
+    }));
+  });
+});
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 13c7e6871e..62cc46b919 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -1,13 +1,293 @@
 "use client";
 
-import ComingSoonTab from './ComingSoonTab';
+import { useEffect, useState } from 'react';
+import { useParams, useRouter } from 'next/navigation';
+import { toast } from 'sonner';
+import { Loader2, Trash2, TriangleAlert } from 'lucide-react';
+import { Button } from '@/components/ui/button';
+import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card';
+import { Input } from '@/components/ui/input';
+import { Label } from '@/components/ui/label';
+import { Switch } from '@/components/ui/switch';
+import { Textarea } from '@/components/ui/textarea';
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogDescription,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+  AlertDialogTrigger,
+} from '@/components/ui/alert-dialog';
+import { fetchWithAuth, patch as apiPatch, del as apiDelete } from '@/lib/auth/auth-fetch';
+import type {
+  MachineSettings,
+  MachineSettingsPatch,
+} from '@pagespace/lib/services/machines/machine-settings';
 
 /**
- * PLACEHOLDER — the real Settings tab (machine config, env, resource controls)
- * is built by a sibling follow-up PR (spec `o2yv9twc782guwfrr2pawcif`). The
- * `{ machineId }` prop signature and file location are locked in here so that
- * agent only fills in the body (replacing the shared ComingSoonTab placeholder).
+ * The Machine page's Settings tab: a full-width form over the machine-settings
+ * route (`/api/machines/settings`). It mirrors `TerminalAccessCard`'s shape —
+ * a plain `useState`/`useEffect` fetch plus an optimistic `persist()` helper
+ * (local update → PATCH → rollback + toast on error). No React Hook Form, no SWR.
+ *
+ * `machineId` IS the backing Machine page's id. The two access toggles
+ * (`visibleToGlobalAssistant` / `allowPageAgents`) are persisted here but not yet
+ * enforced anywhere — the whole surface sits behind `CODE_EXECUTION_ENABLED`.
+ *
+ * Delete is gated behind an `AlertDialog` because it is irreversible from the
+ * user's point of view (the route soft-trashes the page and tears down the
+ * Sprite); on success we navigate back to the drive, off the now-trashed page.
  */
 export default function SettingsTab({ machineId }: { machineId: string }) {
-  return <ComingSoonTab machineId={machineId} label="Settings" />;
+  const router = useRouter();
+  const params = useParams();
+  const driveId = typeof params?.driveId === 'string' ? params.driveId : undefined;
+
+  const [settings, setSettings] = useState<MachineSettings | null>(null);
+  const [loading, setLoading] = useState(true);
+  const [loadError, setLoadError] = useState(false);
+  const [saving, setSaving] = useState(false);
+  const [deleting, setDeleting] = useState(false);
+  const [reloadToken, setReloadToken] = useState(0);
+
+  // Local drafts for the text fields — inputs stay editable per-keystroke and
+  // only persist on blur (the toggles persist immediately). Kept in sync with
+  // the server truth whenever `settings` is (re)loaded or reconciled.
+  const [nameDraft, setNameDraft] = useState('');
+  const [descriptionDraft, setDescriptionDraft] = useState('');
+
+  useEffect(() => {
+    let cancelled = false;
+    setLoading(true);
+    setLoadError(false);
+    (async () => {
+      try {
+        const response = await fetchWithAuth(
+          `/api/machines/settings?machineId=${encodeURIComponent(machineId)}`,
+        );
+        if (!response.ok) throw new Error('Failed to load machine settings');
+        const json = (await response.json()) as { settings: MachineSettings };
+        if (!cancelled) {
+          setSettings(json.settings);
+          setNameDraft(json.settings.name);
+          setDescriptionDraft(json.settings.description ?? '');
+        }
+      } catch (error) {
+        console.error('Failed to load machine settings:', error);
+        if (!cancelled) {
+          setLoadError(true);
+          toast.error('Failed to load machine settings');
+        }
+      } finally {
+        if (!cancelled) setLoading(false);
+      }
+    })();
+    return () => {
+      cancelled = true;
+    };
+  }, [machineId, reloadToken]);
+
+  /**
+   * Optimistically apply `patch` to local state, PATCH it, and reconcile with the
+   * server's authoritative response — rolling back (and toasting) on failure.
+   */
+  async function persist(patch: MachineSettingsPatch) {
+    if (!settings) return;
+    const previous = settings;
+    const optimistic: MachineSettings = { ...settings, ...patch };
+    setSettings(optimistic);
+    setSaving(true);
+    try {
+      const { settings: updated } = await apiPatch<{ settings: MachineSettings }>(
+        '/api/machines/settings',
+        { machineId, ...patch },
+      );
+      setSettings(updated);
+      setNameDraft(updated.name);
+      setDescriptionDraft(updated.description ?? '');
+    } catch (error) {
+      setSettings(previous);
+      setNameDraft(previous.name);
+      setDescriptionDraft(previous.description ?? '');
+      toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
+    } finally {
+      setSaving(false);
+    }
+  }
+
+  function commitName() {
+    if (!settings) return;
+    const trimmed = nameDraft.trim();
+    // An empty name is rejected server-side (name is the page title). Revert the
+    // draft rather than firing a doomed PATCH.
+    if (trimmed.length === 0) {
+      setNameDraft(settings.name);
+      return;
+    }
+    if (trimmed === settings.name) return;
+    persist({ name: trimmed });
+  }
+
+  function commitDescription() {
+    if (!settings) return;
+    const trimmed = descriptionDraft.trim();
+    const current = settings.description ?? '';
+    if (trimmed === current) return;
+    // Whitespace-only clears the description (round-trips to null server-side).
+    persist({ description: trimmed.length === 0 ? null : trimmed });
+  }
+
+  async function handleDelete() {
+    setDeleting(true);
+    try {
+      await apiDelete(`/api/machines/settings?machineId=${encodeURIComponent(machineId)}`);
+      toast.success('Machine deleted');
+      router.push(driveId ? `/dashboard/${driveId}` : '/dashboard');
+    } catch (error) {
+      toast.error(error instanceof Error ? error.message : 'Failed to delete machine');
+      setDeleting(false);
+    }
+  }
+
+  if (loading) {
+    return (
+      <div className="flex h-full items-center justify-center">
+        <Loader2 className="h-5 w-5 animate-spin text-muted-foreground" />
+      </div>
+    );
+  }
+
+  if (loadError || !settings) {
+    return (
+      <div className="flex h-full flex-col items-center justify-center gap-3">
+        <p className="text-sm text-muted-foreground">Could not load machine settings.</p>
+        <Button type="button" variant="outline" size="sm" onClick={() => setReloadToken((t) => t + 1)}>
+          Retry
+        </Button>
+      </div>
+    );
+  }
+
+  return (
+    <div className="h-full overflow-y-auto">
+      <div className="mx-auto flex max-w-2xl flex-col gap-6 p-6">
+        <Card>
+          <CardHeader>
+            <CardTitle className="text-lg">General</CardTitle>
+            <CardDescription>The Machine&apos;s name and description.</CardDescription>
+          </CardHeader>
+          <CardContent className="space-y-4">
+            <div className="space-y-2">
+              <Label htmlFor="machine-name">Name</Label>
+              <Input
+                id="machine-name"
+                value={nameDraft}
+                disabled={saving}
+                onChange={(e) => setNameDraft(e.target.value)}
+                onBlur={commitName}
+              />
+            </div>
+            <div className="space-y-2">
+              <Label htmlFor="machine-description">Description</Label>
+              <Textarea
+                id="machine-description"
+                value={descriptionDraft}
+                disabled={saving}
+                placeholder="What is this Machine for?"
+                onChange={(e) => setDescriptionDraft(e.target.value)}
+                onBlur={commitDescription}
+              />
+            </div>
+          </CardContent>
+        </Card>
+
+        <Card>
+          <CardHeader>
+            <CardTitle className="text-lg">Assistant access</CardTitle>
+            <CardDescription>Control which agents can see and use this Machine.</CardDescription>
+          </CardHeader>
+          <CardContent className="space-y-4">
+            <div className="flex items-center justify-between gap-4">
+              <div className="space-y-0.5">
+                <Label htmlFor="machine-visible-global">Visible to global assistant</Label>
+                <p className="text-xs text-muted-foreground">
+                  Let the global assistant discover and switch to this Machine.
+                </p>
+              </div>
+              <Switch
+                id="machine-visible-global"
+                checked={settings.visibleToGlobalAssistant}
+                disabled={saving}
+                onCheckedChange={(checked) => persist({ visibleToGlobalAssistant: checked })}
+              />
+            </div>
+            <div className="flex items-center justify-between gap-4">
+              <div className="space-y-0.5">
+                <Label htmlFor="machine-allow-page-agents">Allow page agents</Label>
+                <p className="text-xs text-muted-foreground">
+                  Let page-scoped agents run commands on this Machine.
+                </p>
+              </div>
+              <Switch
+                id="machine-allow-page-agents"
+                checked={settings.allowPageAgents}
+                disabled={saving}
+                onCheckedChange={(checked) => persist({ allowPageAgents: checked })}
+              />
+            </div>
+          </CardContent>
+        </Card>
+
+        <Card className="border-destructive/50">
+          <CardHeader>
+            <div className="flex items-center gap-2">
+              <TriangleAlert className="h-5 w-5 text-destructive" />
+              <div>
+                <CardTitle className="text-lg">Delete Machine</CardTitle>
+                <CardDescription>
+                  Trashes this Machine page and tears down its running compute.
+                </CardDescription>
+              </div>
+            </div>
+          </CardHeader>
+          <CardContent>
+            <AlertDialog>
+              <AlertDialogTrigger asChild>
+                <Button type="button" variant="destructive" disabled={deleting}>
+                  {deleting ? (
+                    <Loader2 className="h-4 w-4 animate-spin" />
+                  ) : (
+                    <Trash2 className="h-4 w-4" />
+                  )}
+                  Delete Machine
+                </Button>
+              </AlertDialogTrigger>
+              <AlertDialogContent>
+                <AlertDialogHeader>
+                  <AlertDialogTitle>Delete this Machine?</AlertDialogTitle>
+                  <AlertDialogDescription>
+                    This trashes the Machine page and tears down its Sprite and any branch compute.
+                    You can restore the page from Trash, but its running sessions will be gone.
+                  </AlertDialogDescription>
+                </AlertDialogHeader>
+                <AlertDialogFooter>
+                  <AlertDialogCancel disabled={deleting}>Cancel</AlertDialogCancel>
+                  <AlertDialogAction
+                    onClick={handleDelete}
+                    disabled={deleting}
+                    className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
+                  >
+                    Delete Machine
+                  </AlertDialogAction>
+                </AlertDialogFooter>
+              </AlertDialogContent>
+            </AlertDialog>
+          </CardContent>
+        </Card>
+      </div>
+    </div>
+  );
 }

From d3f529e25b8a2d0c9c2c75d6ee8a026dcb312cd4 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 07:32:07 -0500
Subject: [PATCH 02/11] fix(machine): don't let an in-flight save swallow the
 click that triggered it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Disabling the form controls while a PATCH was in flight silently dropped
user input: clicking a Switch blurs a text field, whose blur-commit fires
persist() -> setSaving(true) -> re-render -> the Switch is disabled before
the click's mouseup lands, so onCheckedChange never fires. The toggle was
lost with no feedback. Surface the in-flight save as a 'Saving…' indicator
instead of enforcing it.

Also stop reconciling local state from the PATCH response. The route echoes
the whole MachineSettings, so applying it would clobber a field the user is
concurrently editing with a value already stale when the request left. We
normalize every field client-side exactly as the route does, so the
optimistic value never diverges. Rollback now reverts only the patched keys,
so a failed toggle can't revert an unrelated edit that landed mid-flight.
This is also closer to TerminalAccessCard, which never reads the response.

Adds regression tests for both (swallowed-toggle, name rollback).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 42 +++++++++++-
 .../page-views/terminal/tabs/SettingsTab.tsx  | 67 ++++++++++++-------
 2 files changed, 82 insertions(+), 27 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index 19fe912f05..764f0758aa 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -53,7 +53,7 @@ describe('SettingsTab', () => {
     });
   });
 
-  test('toggling an access switch optimistically PATCHes and reconciles with the server', async () => {
+  test('toggling an access switch optimistically PATCHes only the flipped flag', async () => {
     mockLoad();
     apiPatch.mockResolvedValue({ settings: { ...baseSettings, visibleToGlobalAssistant: true } });
     render(<SettingsTab machineId="m1" />);
@@ -63,7 +63,7 @@ describe('SettingsTab', () => {
 
     await waitFor(() => assert({
       given: 'the visible-to-global-assistant switch clicked',
-      should: 'PATCH the settings route with the machineId and the flipped flag',
+      should: 'PATCH the settings route with the machineId and the flipped flag only',
       actual: apiPatch.mock.calls[0],
       expected: ['/api/machines/settings', { machineId: 'm1', visibleToGlobalAssistant: true }],
     }));
@@ -76,6 +76,44 @@ describe('SettingsTab', () => {
     }));
   });
 
+  test('committing a text field by clicking a toggle does not swallow the toggle', async () => {
+    mockLoad({ ...baseSettings, description: null });
+    // A save that never settles: if an in-flight save disabled the controls, the
+    // switch would already be disabled by the time the click's mouseup landed.
+    apiPatch.mockImplementation(() => new Promise(() => {}));
+    render(<SettingsTab machineId="m1" />);
+
+    const name = await screen.findByLabelText('Name');
+    await userEvent.type(name, '!');
+    // Clicking the switch blurs the input, which commits the name mid-click.
+    await userEvent.click(screen.getByRole('switch', { name: /visible to global assistant/i }));
+
+    await waitFor(() => assert({
+      given: 'a name edit committed on blur by the very click that hits a switch',
+      should: 'still PATCH the toggle — an in-flight save must not disable the control being clicked',
+      actual: apiPatch.mock.calls.map((call) => Object.keys(call[1] as object).filter((k) => k !== 'machineId')).flat(),
+      expected: ['name', 'visibleToGlobalAssistant'],
+    }));
+  });
+
+  test('a failed name save rolls the input back to the server value', async () => {
+    mockLoad();
+    apiPatch.mockRejectedValue(new Error('nope'));
+    render(<SettingsTab machineId="m1" />);
+
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Renamed');
+    await userEvent.tab();
+
+    await waitFor(() => assert({
+      given: 'a rejected name PATCH',
+      should: 'restore the input to the last-known-good server name',
+      actual: name.value,
+      expected: 'My Machine',
+    }));
+  });
+
   test('a failed PATCH rolls the toggle back and surfaces an error toast', async () => {
     mockLoad();
     apiPatch.mockRejectedValue(new Error('nope'));
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 62cc46b919..ec6e309116 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -54,8 +54,9 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   const [reloadToken, setReloadToken] = useState(0);
 
   // Local drafts for the text fields — inputs stay editable per-keystroke and
-  // only persist on blur (the toggles persist immediately). Kept in sync with
-  // the server truth whenever `settings` is (re)loaded or reconciled.
+  // only persist on blur (the toggles persist immediately). Seeded from the
+  // server on load, and otherwise only ever written back by a failed save's
+  // rollback, so an in-flight request can never clobber what the user is typing.
   const [nameDraft, setNameDraft] = useState('');
   const [descriptionDraft, setDescriptionDraft] = useState('');
 
@@ -91,27 +92,32 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   }, [machineId, reloadToken]);
 
   /**
-   * Optimistically apply `patch` to local state, PATCH it, and reconcile with the
-   * server's authoritative response — rolling back (and toasting) on failure.
+   * Optimistically apply `patch`, PATCH it, and roll back ONLY the patched keys
+   * on failure (toasting the error).
+   *
+   * We deliberately do NOT reconcile from the server's response body. The route
+   * echoes the whole `MachineSettings`, so applying it would clobber a field the
+   * user is concurrently editing with a value that was already stale when the
+   * request left. Every field we send is normalized client-side exactly as the
+   * route normalizes it (name trimmed; blank description → null), so the
+   * optimistic value and the persisted value never diverge. Rolling back only the
+   * patched keys — rather than restoring a whole snapshot — keeps a failed toggle
+   * from reverting an unrelated edit that landed while it was in flight.
    */
   async function persist(patch: MachineSettingsPatch) {
     if (!settings) return;
-    const previous = settings;
-    const optimistic: MachineSettings = { ...settings, ...patch };
-    setSettings(optimistic);
+    const rollback: MachineSettingsPatch = {};
+    for (const key of Object.keys(patch) as (keyof MachineSettingsPatch)[]) {
+      Object.assign(rollback, { [key]: settings[key] });
+    }
+    setSettings((current) => (current ? { ...current, ...patch } : current));
     setSaving(true);
     try {
-      const { settings: updated } = await apiPatch<{ settings: MachineSettings }>(
-        '/api/machines/settings',
-        { machineId, ...patch },
-      );
-      setSettings(updated);
-      setNameDraft(updated.name);
-      setDescriptionDraft(updated.description ?? '');
+      await apiPatch('/api/machines/settings', { machineId, ...patch });
     } catch (error) {
-      setSettings(previous);
-      setNameDraft(previous.name);
-      setDescriptionDraft(previous.description ?? '');
+      setSettings((current) => (current ? { ...current, ...rollback } : current));
+      if (rollback.name !== undefined) setNameDraft(rollback.name);
+      if (rollback.description !== undefined) setDescriptionDraft(rollback.description ?? '');
       toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
     } finally {
       setSaving(false);
@@ -127,6 +133,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
       setNameDraft(settings.name);
       return;
     }
+    setNameDraft(trimmed);
     if (trimmed === settings.name) return;
     persist({ name: trimmed });
   }
@@ -134,8 +141,8 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   function commitDescription() {
     if (!settings) return;
     const trimmed = descriptionDraft.trim();
-    const current = settings.description ?? '';
-    if (trimmed === current) return;
+    setDescriptionDraft(trimmed);
+    if (trimmed === (settings.description ?? '')) return;
     // Whitespace-only clears the description (round-trips to null server-side).
     persist({ description: trimmed.length === 0 ? null : trimmed });
   }
@@ -176,8 +183,22 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
       <div className="mx-auto flex max-w-2xl flex-col gap-6 p-6">
         <Card>
           <CardHeader>
-            <CardTitle className="text-lg">General</CardTitle>
-            <CardDescription>The Machine&apos;s name and description.</CardDescription>
+            <div className="flex items-center justify-between gap-2">
+              <div>
+                <CardTitle className="text-lg">General</CardTitle>
+                <CardDescription>The Machine&apos;s name and description.</CardDescription>
+              </div>
+              {/* An in-flight save is surfaced, never enforced: disabling the
+                  controls here would swallow the very click that triggered the
+                  save (a field's blur-commit fires before the mouseup that lands
+                  on a switch, so the switch would already be disabled). */}
+              {saving && (
+                <span className="flex items-center gap-1.5 text-xs text-muted-foreground">
+                  <Loader2 className="h-3.5 w-3.5 animate-spin" />
+                  Saving…
+                </span>
+              )}
+            </div>
           </CardHeader>
           <CardContent className="space-y-4">
             <div className="space-y-2">
@@ -185,7 +206,6 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Input
                 id="machine-name"
                 value={nameDraft}
-                disabled={saving}
                 onChange={(e) => setNameDraft(e.target.value)}
                 onBlur={commitName}
               />
@@ -195,7 +215,6 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Textarea
                 id="machine-description"
                 value={descriptionDraft}
-                disabled={saving}
                 placeholder="What is this Machine for?"
                 onChange={(e) => setDescriptionDraft(e.target.value)}
                 onBlur={commitDescription}
@@ -220,7 +239,6 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Switch
                 id="machine-visible-global"
                 checked={settings.visibleToGlobalAssistant}
-                disabled={saving}
                 onCheckedChange={(checked) => persist({ visibleToGlobalAssistant: checked })}
               />
             </div>
@@ -234,7 +252,6 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Switch
                 id="machine-allow-page-agents"
                 checked={settings.allowPageAgents}
-                disabled={saving}
                 onCheckedChange={(checked) => persist({ allowPageAgents: checked })}
               />
             </div>

From 041a8edf7e8f661514048ee874620e545ecbd3e3 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 07:41:45 -0500
Subject: [PATCH 03/11] fix(machine): resync from server on failed save; hold
 delete dialog open
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review pass on the Settings tab surfaced three real defects:

1. (major) A failed PATCH rolled back to a CLIENT-SIDE guess and never
   revalidated. A request that committed server-side and then timed out left
   the tab showing the old value forever — there is no SWR/socket here to
   revalidate it. persist() now reverts optimistically (no flicker) and then
   refetches, which is authoritative and also settles out-of-order saves.
2. A failed background resync tore the whole form down to the error screen even
   though it had perfectly good settings in hand. Dropped the loadError flag:
   `settings === null` after the first load IS the error state, and only the
   FIRST load blocks on a spinner (later refetches revalidate in place).
3. AlertDialogAction composes DialogClose, so the dialog unmounted on the same
   click that started the DELETE: the in-progress state could never render and a
   FAILED delete toasted against a dialog the user had to reopen. preventDefault
   holds it open until the request settles.

Also: 'saving' is now a COUNT, not a flag — a blur-commit and a switch click can
be in flight at once, and a boolean reported 'done' when the first settled.

Tests: 6 -> 10. Adds the commit-then-timeout resync, the load-error retry path,
the empty-name guard (must not fire a PATCH the route would 400), description
clearing to null, and pins the GET query-param shape.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        |  89 ++++++++++++++-
 .../page-views/terminal/tabs/SettingsTab.tsx  | 101 ++++++++++--------
 2 files changed, 143 insertions(+), 47 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index 764f0758aa..81cedbe730 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -47,12 +47,55 @@ describe('SettingsTab', () => {
 
     assert({
       given: 'a Machine whose settings load successfully',
-      should: 'populate the name field and reflect the access toggle state',
-      actual: { name: nameInput.value, visible: globalToggle.getAttribute('aria-checked') },
-      expected: { name: 'My Machine', visible: 'false' },
+      should: 'GET the settings by machineId and populate the form from the response',
+      actual: {
+        url: fetchWithAuth.mock.calls[0][0],
+        name: nameInput.value,
+        visible: globalToggle.getAttribute('aria-checked'),
+      },
+      expected: {
+        // The route reads machineId from the QUERY on GET (body only on PATCH) —
+        // pin it, since that mismatch would 400 in prod but pass a lax mock.
+        url: '/api/machines/settings?machineId=m1',
+        name: 'My Machine',
+        visible: 'false',
+      },
     });
   });
 
+  test('blanking the name reverts instead of firing a PATCH the route would reject', async () => {
+    mockLoad();
+    render(<SettingsTab machineId="m1" />);
+
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    await userEvent.clear(name);
+    await userEvent.tab();
+
+    assert({
+      given: 'the name field blanked and blurred',
+      should: 'revert to the server name and send no PATCH (an empty name is a 400)',
+      actual: { value: name.value, patches: apiPatch.mock.calls.length },
+      expected: { value: 'My Machine', patches: 0 },
+    });
+  });
+
+  test('clearing the description persists null, not an empty string', async () => {
+    mockLoad();
+    apiPatch.mockResolvedValue({ settings: { ...baseSettings, description: null } });
+    render(<SettingsTab machineId="m1" />);
+
+    const description = await screen.findByLabelText('Description');
+    await userEvent.clear(description);
+    await userEvent.tab();
+
+    await waitFor(() => assert({
+      given: 'the description emptied and blurred',
+      should: 'PATCH description:null — the route round-trips blank to null',
+      actual: apiPatch.mock.calls[0]?.[1],
+      expected: { machineId: 'm1', description: null },
+    }));
+  });
+
   test('toggling an access switch optimistically PATCHes only the flipped flag', async () => {
     mockLoad();
     apiPatch.mockResolvedValue({ settings: { ...baseSettings, visibleToGlobalAssistant: true } });
@@ -137,6 +180,46 @@ describe('SettingsTab', () => {
     }));
   });
 
+  test('a failed save resyncs from the server rather than trusting the local rollback', async () => {
+    // The PATCH fails, but the server had ALREADY committed it (commit-then-timeout).
+    // A pure client-side rollback would strand the tab showing the stale value with
+    // nothing to revalidate it, so persist() must refetch.
+    const committed: MachineSettings = { ...baseSettings, allowPageAgents: true };
+    fetchWithAuth
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) })
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: committed }) });
+    apiPatch.mockRejectedValue(new Error('timeout'));
+    render(<SettingsTab machineId="m1" />);
+
+    const toggle = await screen.findByRole('switch', { name: /allow page agents/i });
+    await userEvent.click(toggle);
+
+    await waitFor(() => assert({
+      given: 'a PATCH that failed after the server committed it',
+      should: 'refetch and converge on the true server state, not the guessed rollback',
+      actual: { fetches: fetchWithAuth.mock.calls.length, checked: toggle.getAttribute('aria-checked') },
+      expected: { fetches: 2, checked: 'true' },
+    }));
+  });
+
+  test('a first-load failure shows the error state and Retry refetches', async () => {
+    fetchWithAuth
+      .mockResolvedValueOnce({ ok: false, json: async () => ({}) })
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) });
+    render(<SettingsTab machineId="m1" />);
+
+    const retry = await screen.findByRole('button', { name: /retry/i });
+    await userEvent.click(retry);
+
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    assert({
+      given: 'the initial GET failing and then Retry clicked',
+      should: 'refetch and render the form once the settings load',
+      actual: name.value,
+      expected: 'My Machine',
+    });
+  });
+
   test('Delete Machine is gated behind the confirm dialog', async () => {
     mockLoad();
     apiDelete.mockResolvedValue({ success: true, spriteTornDown: true });
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index ec6e309116..65f3673eba 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -1,6 +1,6 @@
 "use client";
 
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 import { useParams, useRouter } from 'next/navigation';
 import { toast } from 'sonner';
 import { Loader2, Trash2, TriangleAlert } from 'lucide-react';
@@ -47,23 +47,29 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   const driveId = typeof params?.driveId === 'string' ? params.driveId : undefined;
 
   const [settings, setSettings] = useState<MachineSettings | null>(null);
-  const [loading, setLoading] = useState(true);
-  const [loadError, setLoadError] = useState(false);
-  const [saving, setSaving] = useState(false);
   const [deleting, setDeleting] = useState(false);
   const [reloadToken, setReloadToken] = useState(0);
 
+  // In-flight save COUNT, not a flag: a text field's blur-commit and a switch
+  // click can be airborne at once, and a boolean would report "done" as soon as
+  // the first of them settled.
+  const [pendingSaves, setPendingSaves] = useState(0);
+
   // Local drafts for the text fields — inputs stay editable per-keystroke and
   // only persist on blur (the toggles persist immediately). Seeded from the
-  // server on load, and otherwise only ever written back by a failed save's
-  // rollback, so an in-flight request can never clobber what the user is typing.
+  // server whenever settings (re)load.
   const [nameDraft, setNameDraft] = useState('');
   const [descriptionDraft, setDescriptionDraft] = useState('');
 
+  // Only the FIRST load blocks on a spinner. `reloadToken` refetches (retry after
+  // a load error, resync after a failed save) revalidate in place, so a failed
+  // toggle doesn't flash the whole form away and back.
+  const hasLoaded = useRef(false);
+  const [loading, setLoading] = useState(true);
+
   useEffect(() => {
     let cancelled = false;
-    setLoading(true);
-    setLoadError(false);
+    if (!hasLoaded.current) setLoading(true);
     (async () => {
       try {
         const response = await fetchWithAuth(
@@ -72,16 +78,16 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
         if (!response.ok) throw new Error('Failed to load machine settings');
         const json = (await response.json()) as { settings: MachineSettings };
         if (!cancelled) {
+          hasLoaded.current = true;
           setSettings(json.settings);
           setNameDraft(json.settings.name);
           setDescriptionDraft(json.settings.description ?? '');
         }
       } catch (error) {
         console.error('Failed to load machine settings:', error);
-        if (!cancelled) {
-          setLoadError(true);
-          toast.error('Failed to load machine settings');
-        }
+        // A failed RESYNC keeps the form (and its already-good settings) on screen
+        // — only a failed FIRST load falls through to the error state below.
+        if (!cancelled) toast.error('Failed to load machine settings');
       } finally {
         if (!cancelled) setLoading(false);
       }
@@ -92,35 +98,37 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   }, [machineId, reloadToken]);
 
   /**
-   * Optimistically apply `patch`, PATCH it, and roll back ONLY the patched keys
-   * on failure (toasting the error).
+   * Optimistically apply `patch`, PATCH it, and on failure revert + resync.
    *
-   * We deliberately do NOT reconcile from the server's response body. The route
-   * echoes the whole `MachineSettings`, so applying it would clobber a field the
-   * user is concurrently editing with a value that was already stale when the
-   * request left. Every field we send is normalized client-side exactly as the
-   * route normalizes it (name trimmed; blank description → null), so the
-   * optimistic value and the persisted value never diverge. Rolling back only the
-   * patched keys — rather than restoring a whole snapshot — keeps a failed toggle
-   * from reverting an unrelated edit that landed while it was in flight.
+   * We do NOT reconcile from the PATCH response body. The route echoes the whole
+   * `MachineSettings`, so applying it would clobber a field the user is
+   * concurrently editing with a value that was already stale when the request
+   * left. Every field we send is normalized client-side exactly as the route
+   * normalizes it (name trimmed; blank description → null), so on success the
+   * optimistic value already equals the persisted one.
+   *
+   * On failure the local snapshot is only a GUESS at server truth — the request
+   * may have committed and then timed out, and two saves in flight can land out
+   * of order. So we revert optimistically (instant, no flicker) and then refetch,
+   * which is authoritative and settles both cases. Without that refetch a
+   * commit-then-timeout would strand the tab showing the old value forever, with
+   * nothing to revalidate it (no SWR, no socket subscription here).
    */
   async function persist(patch: MachineSettingsPatch) {
     if (!settings) return;
-    const rollback: MachineSettingsPatch = {};
-    for (const key of Object.keys(patch) as (keyof MachineSettingsPatch)[]) {
-      Object.assign(rollback, { [key]: settings[key] });
-    }
+    const previous = settings;
     setSettings((current) => (current ? { ...current, ...patch } : current));
-    setSaving(true);
+    setPendingSaves((n) => n + 1);
     try {
       await apiPatch('/api/machines/settings', { machineId, ...patch });
     } catch (error) {
-      setSettings((current) => (current ? { ...current, ...rollback } : current));
-      if (rollback.name !== undefined) setNameDraft(rollback.name);
-      if (rollback.description !== undefined) setDescriptionDraft(rollback.description ?? '');
+      setSettings(previous);
+      setNameDraft(previous.name);
+      setDescriptionDraft(previous.description ?? '');
+      setReloadToken((t) => t + 1);
       toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
     } finally {
-      setSaving(false);
+      setPendingSaves((n) => n - 1);
     }
   }
 
@@ -167,7 +175,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     );
   }
 
-  if (loadError || !settings) {
+  if (!settings) {
     return (
       <div className="flex h-full flex-col items-center justify-center gap-3">
         <p className="text-sm text-muted-foreground">Could not load machine settings.</p>
@@ -189,10 +197,10 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
                 <CardDescription>The Machine&apos;s name and description.</CardDescription>
               </div>
               {/* An in-flight save is surfaced, never enforced: disabling the
-                  controls here would swallow the very click that triggered the
-                  save (a field's blur-commit fires before the mouseup that lands
-                  on a switch, so the switch would already be disabled). */}
-              {saving && (
+                  controls would swallow the very click that triggered the save
+                  (a field's blur-commit fires before the mouseup lands on a
+                  switch, so the switch would already be disabled). */}
+              {pendingSaves > 0 && (
                 <span className="flex items-center gap-1.5 text-xs text-muted-foreground">
                   <Loader2 className="h-3.5 w-3.5 animate-spin" />
                   Saving…
@@ -273,12 +281,8 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
           <CardContent>
             <AlertDialog>
               <AlertDialogTrigger asChild>
-                <Button type="button" variant="destructive" disabled={deleting}>
-                  {deleting ? (
-                    <Loader2 className="h-4 w-4 animate-spin" />
-                  ) : (
-                    <Trash2 className="h-4 w-4" />
-                  )}
+                <Button type="button" variant="destructive">
+                  <Trash2 className="h-4 w-4" />
                   Delete Machine
                 </Button>
               </AlertDialogTrigger>
@@ -292,12 +296,21 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
                 </AlertDialogHeader>
                 <AlertDialogFooter>
                   <AlertDialogCancel disabled={deleting}>Cancel</AlertDialogCancel>
+                  {/* `AlertDialogAction` composes `DialogClose`, so without
+                      preventDefault the dialog unmounts on the very click that
+                      starts the request — the in-progress state could never
+                      render, and a FAILED delete would toast against a dialog the
+                      user then had to reopen. Hold it open until we know. */}
                   <AlertDialogAction
-                    onClick={handleDelete}
+                    onClick={(e) => {
+                      e.preventDefault();
+                      handleDelete();
+                    }}
                     disabled={deleting}
                     className="bg-destructive text-destructive-foreground hover:bg-destructive/90"
                   >
-                    Delete Machine
+                    {deleting && <Loader2 className="h-4 w-4 animate-spin" />}
+                    {deleting ? 'Deleting…' : 'Delete Machine'}
                   </AlertDialogAction>
                 </AlertDialogFooter>
               </AlertDialogContent>

From c830e24efd356f91815e23388ed1a2f38395e0af Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 07:43:46 -0500
Subject: [PATCH 04/11] fix(machine): clear settings when machineId changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The 'only the first load blocks on a spinner' optimization was keyed to a bare
boolean, so a machineId change skipped the spinner AND kept the previous
machine's settings in state — a failed load for the new machine would render the
old machine's name and toggles under the new machine's identity. Key the ref to
the machineId that actually loaded, and drop stale settings on a switch.

Test pins it: switch to a machine whose GET fails -> error state, not a stale form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 22 +++++++++++++++++++
 .../page-views/terminal/tabs/SettingsTab.tsx  | 20 ++++++++++++-----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index 81cedbe730..8f4df82c9e 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -202,6 +202,28 @@ describe('SettingsTab', () => {
     }));
   });
 
+  test('switching machineId never renders the previous machine\'s settings', async () => {
+    fetchWithAuth
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) })
+      // The second machine fails to load — the first machine's settings must NOT
+      // linger on screen under the new machine's identity.
+      .mockResolvedValueOnce({ ok: false, json: async () => ({}) });
+    const { rerender } = render(<SettingsTab machineId="m1" />);
+    await screen.findByLabelText('Name');
+
+    rerender(<SettingsTab machineId="m2" />);
+
+    await waitFor(() => assert({
+      given: 'a switch to a machine whose settings fail to load',
+      should: 'show the error state, not the previous machine\'s stale form',
+      actual: {
+        staleForm: screen.queryByLabelText('Name') !== null,
+        errored: screen.queryByRole('button', { name: /retry/i }) !== null,
+      },
+      expected: { staleForm: false, errored: true },
+    }));
+  });
+
   test('a first-load failure shows the error state and Retry refetches', async () => {
     fetchWithAuth
       .mockResolvedValueOnce({ ok: false, json: async () => ({}) })
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 65f3673eba..1a96c7f064 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -61,15 +61,23 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   const [nameDraft, setNameDraft] = useState('');
   const [descriptionDraft, setDescriptionDraft] = useState('');
 
-  // Only the FIRST load blocks on a spinner. `reloadToken` refetches (retry after
-  // a load error, resync after a failed save) revalidate in place, so a failed
-  // toggle doesn't flash the whole form away and back.
-  const hasLoaded = useRef(false);
+  // Which machine's settings we currently hold. A `reloadToken` refetch of the
+  // SAME machine (retry after a load error, resync after a failed save)
+  // revalidates in place, so a failed toggle doesn't flash the whole form away
+  // and back — but a different `machineId` must block on the spinner rather than
+  // render the previous machine's settings as if they were this one's.
+  const loadedFor = useRef<string | null>(null);
   const [loading, setLoading] = useState(true);
 
   useEffect(() => {
     let cancelled = false;
-    if (!hasLoaded.current) setLoading(true);
+    if (loadedFor.current !== machineId) {
+      // Drop the previous machine's settings before loading a different one —
+      // otherwise a FAILED load would fall through to rendering the old machine's
+      // name/toggles under the new machine's identity.
+      setSettings(null);
+      setLoading(true);
+    }
     (async () => {
       try {
         const response = await fetchWithAuth(
@@ -78,7 +86,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
         if (!response.ok) throw new Error('Failed to load machine settings');
         const json = (await response.json()) as { settings: MachineSettings };
         if (!cancelled) {
-          hasLoaded.current = true;
+          loadedFor.current = machineId;
           setSettings(json.settings);
           setNameDraft(json.settings.name);
           setDescriptionDraft(json.settings.description ?? '');

From a2ffd43e16f93d7629f10247bb1442b2dd34395e Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 07:52:50 -0500
Subject: [PATCH 05/11] fix(machine): make the failed-save path safe under
 concurrent saves
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second review pass caught two bugs the resync fix itself introduced. A text
field's blur-commit and a switch click can be in flight together, and the
failure path handled neither:

1. (major) The resync GET was fired eagerly from the failing save's catch, while
   another PATCH was still airborne. The GET reads pre-PATCH state and is then
   overwritten by the PATCH landing after it — stranding exactly the staleness
   the resync exists to fix. Now deferred until every save drains (pendingSaves
   hits 0), tracked via a ref because finally must read the CURRENT count.
2. (major) Failure reverted a whole snapshot, rolling back an unrelated edit that
   had succeeded while the failing save was in flight. Now reverts only the keys
   that save owned.

Also: the resync refetch reseeded both text drafts unconditionally, deleting
text mid-keystroke. It now adopts the server value only for drafts the user
hasn't touched since we last saw the server (tracked via a serverSettings ref).

Delete dialog is now controlled: closing it always clears 'deleting', so an
Escape during a hung request can't strand a permanently disabled 'Deleting…'
button. A failed delete keeps the dialog open so the user can retry.

Tests 11 -> 14, and beforeEach now resetAllMocks (clearAllMocks left unconsumed
mockResolvedValueOnce queues, order-coupling the suite). Both new concurrency
tests are mutation-verified: reverting either fix makes them fail. Fixed one
vacuous assertion — a bare waitFor on 'fetch count is still 1' passes on the
first poll before an eager refetch fires, so it's gated on the error toast.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 117 +++++++++++++++++-
 .../page-views/terminal/tabs/SettingsTab.tsx  | 100 +++++++++++----
 2 files changed, 192 insertions(+), 25 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index 8f4df82c9e..c01ceb54b9 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -36,7 +36,10 @@ function mockLoad(settings: MachineSettings = baseSettings) {
 }
 
 describe('SettingsTab', () => {
-  beforeEach(() => vi.clearAllMocks());
+  // resetAllMocks, NOT clearAllMocks: clear only wipes call history, leaving any
+  // unconsumed `mockResolvedValueOnce` queued for the NEXT test. That order-couples
+  // the suite and turns one real failure into a misleading cascade.
+  beforeEach(() => vi.resetAllMocks());
 
   test('loads the machine settings into the form', async () => {
     mockLoad();
@@ -202,6 +205,118 @@ describe('SettingsTab', () => {
     }));
   });
 
+  test('a failed save does not revert an unrelated edit that landed while it was in flight', async () => {
+    mockLoad();
+    // The toggle's PATCH hangs and then fails; the name's PATCH succeeds meanwhile.
+    // Reverting a whole snapshot (rather than just the failed save's own keys)
+    // would roll the successful rename back out of the UI.
+    let failToggle: (e: Error) => void = () => {};
+    apiPatch.mockImplementation((_url: string, body: Record<string, unknown>) => {
+      if ('visibleToGlobalAssistant' in body) {
+        return new Promise((_resolve, reject) => { failToggle = reject; });
+      }
+      return Promise.resolve({ settings: baseSettings });
+    });
+    render(<SettingsTab machineId="m1" />);
+
+    await userEvent.click(await screen.findByRole('switch', { name: /visible to global assistant/i }));
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Renamed');
+    await userEvent.tab();
+    await waitFor(() => assert({
+      given: 'the rename PATCH sent while the toggle PATCH is still in flight',
+      should: 'have sent both',
+      actual: apiPatch.mock.calls.length,
+      expected: 2,
+    }));
+
+    failToggle(new Error('toggle failed'));
+
+    await waitFor(() => assert({
+      given: 'the toggle save failing after an unrelated rename succeeded',
+      should: 'revert only the toggle, leaving the successful rename intact',
+      actual: {
+        toggle: screen.getByRole('switch', { name: /visible to global assistant/i }).getAttribute('aria-checked'),
+        name: (screen.getByLabelText('Name') as HTMLInputElement).value,
+      },
+      expected: { toggle: 'false', name: 'Renamed' },
+    }));
+  });
+
+  test('the resync GET waits for every in-flight save to drain', async () => {
+    mockLoad();
+    let failToggle: (e: Error) => void = () => {};
+    let resolveName: (v: unknown) => void = () => {};
+    apiPatch.mockImplementation((_url: string, body: Record<string, unknown>) => {
+      if ('visibleToGlobalAssistant' in body) {
+        return new Promise((_resolve, reject) => { failToggle = reject; });
+      }
+      return new Promise((resolve) => { resolveName = resolve; });
+    });
+    render(<SettingsTab machineId="m1" />);
+
+    await userEvent.click(await screen.findByRole('switch', { name: /visible to global assistant/i }));
+    const name = await screen.findByLabelText('Name');
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Renamed');
+    await userEvent.tab();
+    await waitFor(() => assert({ given: 'two saves', should: 'both be sent', actual: apiPatch.mock.calls.length, expected: 2 }));
+
+    // The toggle fails while the rename is STILL in flight. A GET fired now would
+    // read pre-rename state and then be overwritten by the rename that lands after
+    // it — stranding exactly the staleness the resync exists to fix.
+    failToggle(new Error('toggle failed'));
+
+    // Gate on the error toast, which proves the catch block ran and its effects
+    // flushed. Asserting the fetch count under a bare waitFor would be vacuous:
+    // waitFor polls until the assertion PASSES, so "still 1" would succeed on the
+    // first poll — before an eager refetch had a chance to fire.
+    await waitFor(() => assert({
+      given: 'the toggle save rejecting',
+      should: 'have surfaced the failure (its catch block has run)',
+      actual: (toast.error as ReturnType<typeof vi.fn>).mock.calls.length,
+      expected: 1,
+    }));
+    assert({
+      given: 'a save that failed while another save is still airborne',
+      should: 'NOT refetch yet — only the initial GET has been made',
+      actual: fetchWithAuth.mock.calls.length,
+      expected: 1,
+    });
+
+    resolveName({ settings: baseSettings });
+
+    await waitFor(() => assert({
+      given: 'the last in-flight save draining',
+      should: 'now fire the deferred resync GET',
+      actual: fetchWithAuth.mock.calls.length,
+      expected: 2,
+    }));
+  });
+
+  test('a failed delete keeps the dialog open and re-enables the confirm button', async () => {
+    mockLoad();
+    apiDelete.mockRejectedValue(new Error('sprite teardown exploded'));
+    render(<SettingsTab machineId="m1" />);
+
+    await screen.findByLabelText('Name');
+    await userEvent.click(screen.getByRole('button', { name: /delete machine/i }));
+    const dialog = await screen.findByRole('alertdialog');
+    await userEvent.click(within(dialog).getByRole('button', { name: /delete machine/i }));
+
+    await waitFor(() => assert({
+      given: 'a DELETE that failed',
+      should: 'keep the dialog open with a re-enabled confirm button, and not navigate',
+      actual: {
+        open: screen.queryByRole('alertdialog') !== null,
+        disabled: (within(screen.getByRole('alertdialog')).getByRole('button', { name: /delete machine/i }) as HTMLButtonElement).disabled,
+        navigated: push.mock.calls.length,
+      },
+      expected: { open: true, disabled: false, navigated: 0 },
+    }));
+  });
+
   test('switching machineId never renders the previous machine\'s settings', async () => {
     fetchWithAuth
       .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) })
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 1a96c7f064..bacc0bc158 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -48,19 +48,27 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
 
   const [settings, setSettings] = useState<MachineSettings | null>(null);
   const [deleting, setDeleting] = useState(false);
+  const [confirmOpen, setConfirmOpen] = useState(false);
   const [reloadToken, setReloadToken] = useState(0);
 
   // In-flight save COUNT, not a flag: a text field's blur-commit and a switch
   // click can be airborne at once, and a boolean would report "done" as soon as
-  // the first of them settled.
+  // the first of them settled. Mirrored into a ref because `persist`'s `finally`
+  // has to read the CURRENT count, not the one closed over when it started.
   const [pendingSaves, setPendingSaves] = useState(0);
+  const pendingSavesRef = useRef(0);
+  const resyncWhenIdle = useRef(false);
 
   // Local drafts for the text fields — inputs stay editable per-keystroke and
-  // only persist on blur (the toggles persist immediately). Seeded from the
-  // server whenever settings (re)load.
+  // only persist on blur (the toggles persist immediately).
   const [nameDraft, setNameDraft] = useState('');
   const [descriptionDraft, setDescriptionDraft] = useState('');
 
+  // Last known SERVER value, used to tell a clean draft from a dirty one so a
+  // refetch can adopt server truth for fields the user isn't editing without
+  // stomping the one they are.
+  const serverSettings = useRef<MachineSettings | null>(null);
+
   // Which machine's settings we currently hold. A `reloadToken` refetch of the
   // SAME machine (retry after a load error, resync after a failed save)
   // revalidates in place, so a failed toggle doesn't flash the whole form away
@@ -76,8 +84,16 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
       // otherwise a FAILED load would fall through to rendering the old machine's
       // name/toggles under the new machine's identity.
       setSettings(null);
-      setLoading(true);
+      serverSettings.current = null;
     }
+    // Spinner whenever we have nothing to show for THIS machine — a different
+    // machine, or a retry after the error state. Keying on `settings` (not just
+    // the machine) also stops a return-trip to a machine whose load failed from
+    // rendering the error state while its GET is still in flight.
+    setSettings((current) => {
+      if (loadedFor.current !== machineId || current === null) setLoading(true);
+      return current;
+    });
     (async () => {
       try {
         const response = await fetchWithAuth(
@@ -85,12 +101,21 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
         );
         if (!response.ok) throw new Error('Failed to load machine settings');
         const json = (await response.json()) as { settings: MachineSettings };
-        if (!cancelled) {
-          loadedFor.current = machineId;
-          setSettings(json.settings);
-          setNameDraft(json.settings.name);
-          setDescriptionDraft(json.settings.description ?? '');
-        }
+        if (cancelled) return;
+        const next = json.settings;
+        const previousServer = serverSettings.current;
+        loadedFor.current = machineId;
+        serverSettings.current = next;
+        setSettings(next);
+        // Adopt the server value only for drafts the user has NOT touched since
+        // we last saw the server. A resync fires while the tab is open, so blindly
+        // reseeding would delete text mid-keystroke.
+        setNameDraft((draft) => (previousServer === null || draft === previousServer.name ? next.name : draft));
+        setDescriptionDraft((draft) =>
+          previousServer === null || draft === (previousServer.description ?? '')
+            ? next.description ?? ''
+            : draft,
+        );
       } catch (error) {
         console.error('Failed to load machine settings:', error);
         // A failed RESYNC keeps the form (and its already-good settings) on screen
@@ -115,28 +140,41 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
    * normalizes it (name trimmed; blank description → null), so on success the
    * optimistic value already equals the persisted one.
    *
-   * On failure the local snapshot is only a GUESS at server truth — the request
-   * may have committed and then timed out, and two saves in flight can land out
-   * of order. So we revert optimistically (instant, no flicker) and then refetch,
-   * which is authoritative and settles both cases. Without that refetch a
-   * commit-then-timeout would strand the tab showing the old value forever, with
-   * nothing to revalidate it (no SWR, no socket subscription here).
+   * Two rules make the failure path safe under CONCURRENT saves (a field's
+   * blur-commit and a switch click can be in flight together):
+   *
+   *  1. Revert ONLY the keys this save owned. Reverting a whole snapshot would
+   *     roll back an unrelated edit that landed while this one was in flight.
+   *  2. Defer the resync until every save has drained. The local snapshot is only
+   *     a GUESS at server truth (the request may have committed and then timed
+   *     out), so a refetch is the authority — but a GET fired while another PATCH
+   *     is still airborne reads pre-PATCH state and is then overwritten by it,
+   *     stranding the very staleness the resync exists to fix.
    */
   async function persist(patch: MachineSettingsPatch) {
     if (!settings) return;
-    const previous = settings;
+    const keys = Object.keys(patch) as (keyof MachineSettingsPatch)[];
+    const revert: MachineSettingsPatch = {};
+    for (const key of keys) Object.assign(revert, { [key]: settings[key] });
+
     setSettings((current) => (current ? { ...current, ...patch } : current));
-    setPendingSaves((n) => n + 1);
+    pendingSavesRef.current += 1;
+    setPendingSaves(pendingSavesRef.current);
     try {
       await apiPatch('/api/machines/settings', { machineId, ...patch });
     } catch (error) {
-      setSettings(previous);
-      setNameDraft(previous.name);
-      setDescriptionDraft(previous.description ?? '');
-      setReloadToken((t) => t + 1);
+      setSettings((current) => (current ? { ...current, ...revert } : current));
+      if (revert.name !== undefined) setNameDraft(revert.name);
+      if (revert.description !== undefined) setDescriptionDraft(revert.description ?? '');
+      resyncWhenIdle.current = true;
       toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
     } finally {
-      setPendingSaves((n) => n - 1);
+      pendingSavesRef.current -= 1;
+      setPendingSaves(pendingSavesRef.current);
+      if (pendingSavesRef.current === 0 && resyncWhenIdle.current) {
+        resyncWhenIdle.current = false;
+        setReloadToken((t) => t + 1);
+      }
     }
   }
 
@@ -168,13 +206,27 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     try {
       await apiDelete(`/api/machines/settings?machineId=${encodeURIComponent(machineId)}`);
       toast.success('Machine deleted');
+      setConfirmOpen(false);
       router.push(driveId ? `/dashboard/${driveId}` : '/dashboard');
     } catch (error) {
+      // Leave the dialog OPEN on failure so the user can simply retry.
       toast.error(error instanceof Error ? error.message : 'Failed to delete machine');
+    } finally {
       setDeleting(false);
     }
   }
 
+  /**
+   * Closing the confirm dialog always clears the in-progress state. The DELETE is
+   * held open behind `preventDefault` (see below), so without this an Escape
+   * during a hung request would strand `deleting: true` — reopening would then
+   * show a permanently disabled "Deleting…" button with no way back.
+   */
+  function onConfirmOpenChange(open: boolean) {
+    setConfirmOpen(open);
+    if (!open) setDeleting(false);
+  }
+
   if (loading) {
     return (
       <div className="flex h-full items-center justify-center">
@@ -287,7 +339,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
             </div>
           </CardHeader>
           <CardContent>
-            <AlertDialog>
+            <AlertDialog open={confirmOpen} onOpenChange={onConfirmOpenChange}>
               <AlertDialogTrigger asChild>
                 <Button type="button" variant="destructive">
                   <Trash2 className="h-4 w-4" />

From c10b3942e7eefc8b444fa26f7c35c509df73931e Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 08:15:26 -0500
Subject: [PATCH 06/11] fix(machine): impure state updater hung the Settings
 tab under StrictMode

Third review pass found a bug the previous fix introduced, and it broke the tab
in dev outright.

Calling setLoading(true) from INSIDE a setSettings updater is an impure updater.
Next 15 enables reactStrictMode by default, so React replays queued updaters at
render time -- the replay ran after the load's finally had already cleared
loading, stranding loading:true with nothing left to clear it. Result: the
Settings tab sat on a spinner forever in dev. It only survived a single mount by
luck (the eager-bailout path never replays the updater).

serverSettings moves in exact lockstep with settings, so the ref answers "do we
have anything to show for this machine?" with no state read at all.

Also guards persist() against a save that settles AFTER the tab moved to another
machine: without it, machine A's failed rename reverted A's old name into
machine B's input -- where the next blur would have renamed B. Unreachable today
(TerminalKeepAliveHost keys each TerminalView by id) but the component's own
tests assert in-place switching, so contract and code now agree.

Tests 14 -> 16, both mutation-verified. The machine-switch test deliberately
does NOT use waitFor: when the guard works the correct outcome is that NOTHING
happens, so a polling assertion passes on its first tick and asserts nothing --
it drains the rejection with act() and then asserts once. (Same vacuity trap
caught earlier in the resync test.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 72 ++++++++++++++++++-
 .../page-views/terminal/tabs/SettingsTab.tsx  | 32 ++++++---
 2 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index c01ceb54b9..fdc3c03962 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -1,5 +1,6 @@
+import { StrictMode } from 'react';
 import { describe, test, beforeEach, vi } from 'vitest';
-import { render, screen, waitFor, within } from '@testing-library/react';
+import { act, render, screen, waitFor, within } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { assert } from '@/stores/__tests__/riteway';
 import type { MachineSettings } from '@pagespace/lib/services/machines/machine-settings';
@@ -339,6 +340,75 @@ describe('SettingsTab', () => {
     }));
   });
 
+  test('renders the form under StrictMode (no impure updater stranding the spinner)', async () => {
+    // Next 15 enables reactStrictMode by default, so `bun run dev` always double-
+    // invokes effects and REPLAYS state updaters at render time. An updater with a
+    // side effect inside (e.g. calling setLoading from within a setSettings
+    // updater) gets replayed after the load's `finally` already cleared loading —
+    // stranding the tab on a spinner forever. This is the dev-only path, so only a
+    // StrictMode render catches it.
+    mockLoad();
+    render(
+      <StrictMode>
+        <SettingsTab machineId="m1" />
+      </StrictMode>,
+    );
+
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    assert({
+      given: 'the tab mounted under StrictMode with a successful load',
+      should: 'render the loaded form, not hang on the spinner',
+      actual: name.value,
+      expected: 'My Machine',
+    });
+  });
+
+  test('a save that settles after a machine switch never writes onto the new machine', async () => {
+    const other: MachineSettings = { ...baseSettings, name: 'Other Machine', description: 'other' };
+    fetchWithAuth.mockImplementation((url: string) =>
+      Promise.resolve({
+        ok: true,
+        json: async () => ({ settings: url.includes('m2') ? other : baseSettings }),
+      }),
+    );
+    let failRename: (e: Error) => void = () => {};
+    apiPatch.mockImplementation(() => new Promise((_r, reject) => { failRename = reject; }));
+
+    const { rerender } = render(<SettingsTab machineId="m1" />);
+    const name = await screen.findByLabelText('Name');
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Renamed');
+    await userEvent.tab(); // m1's rename PATCH is now in flight
+
+    rerender(<SettingsTab machineId="m2" />);
+    await waitFor(() => assert({
+      given: 'a switch to m2',
+      should: 'show m2 settings',
+      actual: (screen.getByLabelText('Name') as HTMLInputElement).value,
+      expected: 'Other Machine',
+    }));
+
+    // m1's rename now fails. Its revert must NOT write m1's old name into m2's
+    // form — where the next blur would rename m2.
+    failRename(new Error('too late'));
+
+    // Flush the rejection and React's resulting work. This CANNOT be a `waitFor`:
+    // when the guard works, the correct outcome is that nothing happens, so a
+    // polling assertion would pass on its first tick before the rejection even
+    // propagated — asserting nothing. Drain, then assert once.
+    await act(async () => { await Promise.resolve(); });
+
+    assert({
+      given: "a stale machine's save failing after the tab moved to another machine",
+      should: "leave the new machine's form untouched and stay silent",
+      actual: {
+        name: (screen.getByLabelText('Name') as HTMLInputElement).value,
+        errored: (toast.error as ReturnType<typeof vi.fn>).mock.calls.length,
+      },
+      expected: { name: 'Other Machine', errored: 0 },
+    });
+  });
+
   test('a first-load failure shows the error state and Retry refetches', async () => {
     fetchWithAuth
       .mockResolvedValueOnce({ ok: false, json: async () => ({}) })
diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index bacc0bc158..77477a6a47 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -79,21 +79,22 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
 
   useEffect(() => {
     let cancelled = false;
-    if (loadedFor.current !== machineId) {
+    const isNewMachine = loadedFor.current !== machineId;
+    if (isNewMachine) {
       // Drop the previous machine's settings before loading a different one —
       // otherwise a FAILED load would fall through to rendering the old machine's
       // name/toggles under the new machine's identity.
       setSettings(null);
       serverSettings.current = null;
     }
-    // Spinner whenever we have nothing to show for THIS machine — a different
-    // machine, or a retry after the error state. Keying on `settings` (not just
-    // the machine) also stops a return-trip to a machine whose load failed from
-    // rendering the error state while its GET is still in flight.
-    setSettings((current) => {
-      if (loadedFor.current !== machineId || current === null) setLoading(true);
-      return current;
-    });
+    // Spinner whenever we have nothing to show for THIS machine: a different
+    // machine, or a retry after the error state. `serverSettings` moves in exact
+    // lockstep with `settings` (both nulled above, both set on load, and `persist`
+    // touches neither), so the ref answers "do we have anything to show?" without
+    // reading state — reading it via a `setSettings` updater instead would be an
+    // impure updater, which StrictMode replays at render time and which strands
+    // `loading: true` after the load's `finally` has already cleared it.
+    if (isNewMachine || serverSettings.current === null) setLoading(true);
     (async () => {
       try {
         const response = await fetchWithAuth(
@@ -157,12 +158,21 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     const revert: MachineSettingsPatch = {};
     for (const key of keys) Object.assign(revert, { [key]: settings[key] });
 
+    // The machine this save belongs to. If the tab is showing a DIFFERENT machine
+    // by the time the request settles, every write below would land on that
+    // machine's form — reverting machine A's name into machine B's input, where
+    // the next blur would rename B. Late results for a machine we've left are
+    // dropped (the resync too: it would refetch the wrong machine).
+    const forMachine = machineId;
+    const isStale = () => loadedFor.current !== forMachine;
+
     setSettings((current) => (current ? { ...current, ...patch } : current));
     pendingSavesRef.current += 1;
     setPendingSaves(pendingSavesRef.current);
     try {
-      await apiPatch('/api/machines/settings', { machineId, ...patch });
+      await apiPatch('/api/machines/settings', { machineId: forMachine, ...patch });
     } catch (error) {
+      if (isStale()) return;
       setSettings((current) => (current ? { ...current, ...revert } : current));
       if (revert.name !== undefined) setNameDraft(revert.name);
       if (revert.description !== undefined) setDescriptionDraft(revert.description ?? '');
@@ -171,7 +181,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     } finally {
       pendingSavesRef.current -= 1;
       setPendingSaves(pendingSavesRef.current);
-      if (pendingSavesRef.current === 0 && resyncWhenIdle.current) {
+      if (!isStale() && pendingSavesRef.current === 0 && resyncWhenIdle.current) {
         resyncWhenIdle.current = false;
         setReloadToken((t) => t + 1);
       }

From 7376b4a983b7e9e1878c6a4dd66c40805e91ebcd Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 08:41:16 -0500
Subject: [PATCH 07/11] fix(machine): drop a pending resync when leaving a
 machine

resyncWhenIdle was never cleared on a machineId switch, so a resync owed to the
machine we left could fire a spurious refetch once an unrelated save on the NEW
machine drained. Harmless (an extra GET) but sloppy; clear it with the rest of
the outgoing machine's state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../middle-content/page-views/terminal/tabs/SettingsTab.tsx  | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 77477a6a47..3d1f04c2bc 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -83,9 +83,12 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     if (isNewMachine) {
       // Drop the previous machine's settings before loading a different one —
       // otherwise a FAILED load would fall through to rendering the old machine's
-      // name/toggles under the new machine's identity.
+      // name/toggles under the new machine's identity. A resync owed to the machine
+      // we're leaving is dropped with it, so it can't fire a spurious refetch once
+      // an unrelated save on the NEW machine drains.
       setSettings(null);
       serverSettings.current = null;
+      resyncWhenIdle.current = false;
     }
     // Spinner whenever we have nothing to show for THIS machine: a different
     // machine, or a retry after the error state. `serverSettings` moves in exact

From c8790147906da621a6f3d3ce797a02bed3c43592 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 08:52:39 -0500
Subject: [PATCH 08/11] polish(machine): move the Saving indicator to form
 level

It sat in the General card's header but fires for the Assistant-access toggles in
the card below it too, mislabelling which section was saving. Any control on the
form can be the one in flight, so the indicator belongs to the form.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../page-views/terminal/tabs/SettingsTab.tsx  | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 3d1f04c2bc..2893c6588b 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -262,24 +262,24 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
   return (
     <div className="h-full overflow-y-auto">
       <div className="mx-auto flex max-w-2xl flex-col gap-6 p-6">
+        {/* Form-level, not per-card: any field or toggle below can be the one
+            saving. An in-flight save is SURFACED, never ENFORCED — disabling the
+            controls would swallow the very click that triggered the save (a
+            field's blur-commit fires before the mouseup lands on a switch, so the
+            switch would already be disabled by the time the click resolved). */}
+        <div className="flex h-4 items-center justify-end">
+          {pendingSaves > 0 && (
+            <span className="flex items-center gap-1.5 text-xs text-muted-foreground">
+              <Loader2 className="h-3.5 w-3.5 animate-spin" />
+              Saving…
+            </span>
+          )}
+        </div>
+
         <Card>
           <CardHeader>
-            <div className="flex items-center justify-between gap-2">
-              <div>
-                <CardTitle className="text-lg">General</CardTitle>
-                <CardDescription>The Machine&apos;s name and description.</CardDescription>
-              </div>
-              {/* An in-flight save is surfaced, never enforced: disabling the
-                  controls would swallow the very click that triggered the save
-                  (a field's blur-commit fires before the mouseup lands on a
-                  switch, so the switch would already be disabled). */}
-              {pendingSaves > 0 && (
-                <span className="flex items-center gap-1.5 text-xs text-muted-foreground">
-                  <Loader2 className="h-3.5 w-3.5 animate-spin" />
-                  Saving…
-                </span>
-              )}
-            </div>
+            <CardTitle className="text-lg">General</CardTitle>
+            <CardDescription>The Machine&apos;s name and description.</CardDescription>
           </CardHeader>
           <CardContent className="space-y-4">
             <div className="space-y-2">

From 4458827b1c644a675c15b01e1ef5e9afe3e1f271 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 09:00:33 -0500
Subject: [PATCH 09/11] test(machine): cover four behaviours whose mutants were
 surviving

A dedicated pass over the test file found no vacuity or flakiness in the existing
16, but four pieces of real component logic were entirely unguarded -- breaking
each left the suite green:

- Escape during a hung DELETE must clear `deleting`. The whole reason the dialog
  is controlled; without it, reopening shows a dead disabled "Deleting..." button.
- A resync must not clobber a draft the user is mid-edit in, while still adopting
  the server value for a field they haven't touched. This is the subtlest logic in
  the component and had zero coverage.
- Blurring an unchanged field must send no PATCH.
- A name is trimmed before it is persisted AND before it is displayed. This is the
  premise of "the optimistic value already equals the persisted one", which is what
  licenses not reconciling from the PATCH response -- so it deserves to be pinned.

All four are mutation-verified: breaking each behaviour fails exactly its own test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index fdc3c03962..dc4effe7f6 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -83,6 +83,105 @@ describe('SettingsTab', () => {
     });
   });
 
+  test('blurring a field the user never edited sends no PATCH', async () => {
+    mockLoad();
+    render(<SettingsTab machineId="m1" />);
+
+    const name = await screen.findByLabelText('Name');
+    await userEvent.click(name);
+    await userEvent.tab(); // focus + blur, no edit
+    await userEvent.click(screen.getByLabelText('Description'));
+    await userEvent.tab();
+
+    assert({
+      given: 'both text fields focused and blurred without an edit',
+      should: 'send no PATCH — an unchanged value is not a save',
+      actual: apiPatch.mock.calls.length,
+      expected: 0,
+    });
+  });
+
+  test('a name is trimmed before it is persisted and before it is displayed', async () => {
+    mockLoad();
+    apiPatch.mockResolvedValue({ settings: baseSettings });
+    render(<SettingsTab machineId="m1" />);
+
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    await userEvent.clear(name);
+    await userEvent.type(name, '  Padded  ');
+    await userEvent.tab();
+
+    await waitFor(() => assert({
+      given: 'a name typed with surrounding whitespace',
+      should: 'PATCH the trimmed name and settle the input on it — the client must normalize exactly as the route does, which is what licenses not reconciling from the response',
+      actual: { patched: apiPatch.mock.calls[0]?.[1], displayed: name.value },
+      expected: { patched: { machineId: 'm1', name: 'Padded' }, displayed: 'Padded' },
+    }));
+  });
+
+  test('a resync adopts server values for untouched fields but never clobbers a dirty draft', async () => {
+    // The failed toggle triggers a resync GET. That GET must not delete text the
+    // user is in the middle of typing — but it SHOULD pick up the server's value
+    // for a field they haven't touched.
+    const serverMoved: MachineSettings = { ...baseSettings, description: 'changed elsewhere' };
+    fetchWithAuth
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) })
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: serverMoved }) });
+    let failToggle: (e: Error) => void = () => {};
+    apiPatch.mockImplementation(() => new Promise((_r, reject) => { failToggle = reject; }));
+    render(<SettingsTab machineId="m1" />);
+
+    await userEvent.click(await screen.findByRole('switch', { name: /allow page agents/i }));
+
+    // Type into Name WITHOUT blurring — this draft is dirty and uncommitted.
+    const name = (await screen.findByLabelText('Name')) as HTMLInputElement;
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Half-typed');
+
+    failToggle(new Error('boom')); // -> revert + resync GET
+
+    await waitFor(() => assert({
+      given: 'a resync landing while the user is mid-edit in Name',
+      should: "keep the dirty Name draft and adopt the server's Description (untouched)",
+      actual: {
+        name: (screen.getByLabelText('Name') as HTMLInputElement).value,
+        description: (screen.getByLabelText('Description') as HTMLTextAreaElement).value,
+      },
+      expected: { name: 'Half-typed', description: 'changed elsewhere' },
+    }));
+  });
+
+  test('escaping a hung delete leaves a usable dialog, not a stuck "Deleting…" button', async () => {
+    mockLoad();
+    apiDelete.mockImplementation(() => new Promise(() => {})); // never settles
+    render(<SettingsTab machineId="m1" />);
+
+    await screen.findByLabelText('Name');
+    await userEvent.click(screen.getByRole('button', { name: /delete machine/i }));
+    await userEvent.click(within(await screen.findByRole('alertdialog')).getByRole('button', { name: /delete machine/i }));
+    await screen.findByRole('button', { name: /deleting/i }); // request is in flight
+
+    // The DELETE is held open behind preventDefault, so Escape is the only exit.
+    // Closing must clear `deleting` or reopening shows a dead, disabled button.
+    await userEvent.keyboard('{Escape}');
+    await waitFor(() => assert({
+      given: 'Escape pressed during a hung delete',
+      should: 'close the dialog',
+      actual: screen.queryByRole('alertdialog') === null,
+      expected: true,
+    }));
+
+    await userEvent.click(screen.getByRole('button', { name: /delete machine/i }));
+    const reopened = within(await screen.findByRole('alertdialog')).getByRole('button', { name: /delete machine/i }) as HTMLButtonElement;
+
+    assert({
+      given: 'the confirm dialog reopened after escaping a hung delete',
+      should: 'offer an enabled "Delete Machine" button, not a stuck "Deleting…" one',
+      actual: { label: reopened.textContent, disabled: reopened.disabled },
+      expected: { label: 'Delete Machine', disabled: false },
+    });
+  });
+
   test('clearing the description persists null, not an empty string', async () => {
     mockLoad();
     apiPatch.mockResolvedValue({ settings: { ...baseSettings, description: null } });

From b3ab4ab08448fbf77f3c4a9ebf2a66ea7a2b11c3 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 09:04:58 -0500
Subject: [PATCH 10/11] refactor(machine): extract the settings state machine
 into useMachineSettings

SettingsTab had grown to 396 lines carrying 8 pieces of state and 4 refs, and
none of that machinery is about rendering -- it is the load/save/resync state
machine, where every subtlety lives (concurrent saves, per-key revert, deferred
resync, dirty-draft preservation, stale-machine results). Tangled with the form
JSX it reads as incidental complexity; on its own it reads as what it is.

SettingsTab is now 240 lines: the form, plus the delete action. The hook is 216
and documents its own invariants. It lives in apps/web/src/hooks/ beside its
useMachineProjects / useMachineBranches siblings -- deliberately NOT on SWR like
they are, because the spec pins this surface to TerminalAccessCard's plain
useState/optimistic-persist shape. (Revalidation is precisely what SWR would have
given for free, and precisely why most of the hook exists; better spelled out
than implicit.)

Behaviour-preserving: the 20 tests are UNCHANGED and still pass, and the four
mutation checks still kill their mutants against the extracted hook (StrictMode
impure-updater hang, eager resync racing an in-flight PATCH, dirty-draft clobber,
stale-machine write).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../page-views/terminal/tabs/SettingsTab.tsx  | 210 +++--------------
 apps/web/src/hooks/useMachineSettings.ts      | 216 ++++++++++++++++++
 2 files changed, 243 insertions(+), 183 deletions(-)
 create mode 100644 apps/web/src/hooks/useMachineSettings.ts

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
index 2893c6588b..3ac29f521f 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.tsx
@@ -1,6 +1,6 @@
 "use client";
 
-import { useEffect, useRef, useState } from 'react';
+import { useState } from 'react';
 import { useParams, useRouter } from 'next/navigation';
 import { toast } from 'sonner';
 import { Loader2, Trash2, TriangleAlert } from 'lucide-react';
@@ -21,198 +21,42 @@ import {
   AlertDialogTitle,
   AlertDialogTrigger,
 } from '@/components/ui/alert-dialog';
-import { fetchWithAuth, patch as apiPatch, del as apiDelete } from '@/lib/auth/auth-fetch';
-import type {
-  MachineSettings,
-  MachineSettingsPatch,
-} from '@pagespace/lib/services/machines/machine-settings';
+import { del as apiDelete } from '@/lib/auth/auth-fetch';
+import { useMachineSettings } from '@/hooks/useMachineSettings';
 
 /**
  * The Machine page's Settings tab: a full-width form over the machine-settings
- * route (`/api/machines/settings`). It mirrors `TerminalAccessCard`'s shape —
- * a plain `useState`/`useEffect` fetch plus an optimistic `persist()` helper
- * (local update → PATCH → rollback + toast on error). No React Hook Form, no SWR.
+ * route. `machineId` IS the backing Machine page's id.
  *
- * `machineId` IS the backing Machine page's id. The two access toggles
- * (`visibleToGlobalAssistant` / `allowPageAgents`) are persisted here but not yet
- * enforced anywhere — the whole surface sits behind `CODE_EXECUTION_ENABLED`.
+ * All the load/save/resync state lives in {@link useMachineSettings} — the
+ * failure path under concurrent saves is genuinely subtle and none of it is about
+ * rendering. This component is the form and the delete action.
  *
- * Delete is gated behind an `AlertDialog` because it is irreversible from the
- * user's point of view (the route soft-trashes the page and tears down the
- * Sprite); on success we navigate back to the drive, off the now-trashed page.
+ * The two access toggles (`visibleToGlobalAssistant` / `allowPageAgents`) are
+ * persisted here but not yet enforced anywhere — the whole surface sits behind
+ * `CODE_EXECUTION_ENABLED`.
  */
 export default function SettingsTab({ machineId }: { machineId: string }) {
   const router = useRouter();
   const params = useParams();
   const driveId = typeof params?.driveId === 'string' ? params.driveId : undefined;
 
-  const [settings, setSettings] = useState<MachineSettings | null>(null);
+  const {
+    settings,
+    loading,
+    pendingSaves,
+    nameDraft,
+    setNameDraft,
+    descriptionDraft,
+    setDescriptionDraft,
+    commitName,
+    commitDescription,
+    setAccess,
+    reload,
+  } = useMachineSettings(machineId);
+
   const [deleting, setDeleting] = useState(false);
   const [confirmOpen, setConfirmOpen] = useState(false);
-  const [reloadToken, setReloadToken] = useState(0);
-
-  // In-flight save COUNT, not a flag: a text field's blur-commit and a switch
-  // click can be airborne at once, and a boolean would report "done" as soon as
-  // the first of them settled. Mirrored into a ref because `persist`'s `finally`
-  // has to read the CURRENT count, not the one closed over when it started.
-  const [pendingSaves, setPendingSaves] = useState(0);
-  const pendingSavesRef = useRef(0);
-  const resyncWhenIdle = useRef(false);
-
-  // Local drafts for the text fields — inputs stay editable per-keystroke and
-  // only persist on blur (the toggles persist immediately).
-  const [nameDraft, setNameDraft] = useState('');
-  const [descriptionDraft, setDescriptionDraft] = useState('');
-
-  // Last known SERVER value, used to tell a clean draft from a dirty one so a
-  // refetch can adopt server truth for fields the user isn't editing without
-  // stomping the one they are.
-  const serverSettings = useRef<MachineSettings | null>(null);
-
-  // Which machine's settings we currently hold. A `reloadToken` refetch of the
-  // SAME machine (retry after a load error, resync after a failed save)
-  // revalidates in place, so a failed toggle doesn't flash the whole form away
-  // and back — but a different `machineId` must block on the spinner rather than
-  // render the previous machine's settings as if they were this one's.
-  const loadedFor = useRef<string | null>(null);
-  const [loading, setLoading] = useState(true);
-
-  useEffect(() => {
-    let cancelled = false;
-    const isNewMachine = loadedFor.current !== machineId;
-    if (isNewMachine) {
-      // Drop the previous machine's settings before loading a different one —
-      // otherwise a FAILED load would fall through to rendering the old machine's
-      // name/toggles under the new machine's identity. A resync owed to the machine
-      // we're leaving is dropped with it, so it can't fire a spurious refetch once
-      // an unrelated save on the NEW machine drains.
-      setSettings(null);
-      serverSettings.current = null;
-      resyncWhenIdle.current = false;
-    }
-    // Spinner whenever we have nothing to show for THIS machine: a different
-    // machine, or a retry after the error state. `serverSettings` moves in exact
-    // lockstep with `settings` (both nulled above, both set on load, and `persist`
-    // touches neither), so the ref answers "do we have anything to show?" without
-    // reading state — reading it via a `setSettings` updater instead would be an
-    // impure updater, which StrictMode replays at render time and which strands
-    // `loading: true` after the load's `finally` has already cleared it.
-    if (isNewMachine || serverSettings.current === null) setLoading(true);
-    (async () => {
-      try {
-        const response = await fetchWithAuth(
-          `/api/machines/settings?machineId=${encodeURIComponent(machineId)}`,
-        );
-        if (!response.ok) throw new Error('Failed to load machine settings');
-        const json = (await response.json()) as { settings: MachineSettings };
-        if (cancelled) return;
-        const next = json.settings;
-        const previousServer = serverSettings.current;
-        loadedFor.current = machineId;
-        serverSettings.current = next;
-        setSettings(next);
-        // Adopt the server value only for drafts the user has NOT touched since
-        // we last saw the server. A resync fires while the tab is open, so blindly
-        // reseeding would delete text mid-keystroke.
-        setNameDraft((draft) => (previousServer === null || draft === previousServer.name ? next.name : draft));
-        setDescriptionDraft((draft) =>
-          previousServer === null || draft === (previousServer.description ?? '')
-            ? next.description ?? ''
-            : draft,
-        );
-      } catch (error) {
-        console.error('Failed to load machine settings:', error);
-        // A failed RESYNC keeps the form (and its already-good settings) on screen
-        // — only a failed FIRST load falls through to the error state below.
-        if (!cancelled) toast.error('Failed to load machine settings');
-      } finally {
-        if (!cancelled) setLoading(false);
-      }
-    })();
-    return () => {
-      cancelled = true;
-    };
-  }, [machineId, reloadToken]);
-
-  /**
-   * Optimistically apply `patch`, PATCH it, and on failure revert + resync.
-   *
-   * We do NOT reconcile from the PATCH response body. The route echoes the whole
-   * `MachineSettings`, so applying it would clobber a field the user is
-   * concurrently editing with a value that was already stale when the request
-   * left. Every field we send is normalized client-side exactly as the route
-   * normalizes it (name trimmed; blank description → null), so on success the
-   * optimistic value already equals the persisted one.
-   *
-   * Two rules make the failure path safe under CONCURRENT saves (a field's
-   * blur-commit and a switch click can be in flight together):
-   *
-   *  1. Revert ONLY the keys this save owned. Reverting a whole snapshot would
-   *     roll back an unrelated edit that landed while this one was in flight.
-   *  2. Defer the resync until every save has drained. The local snapshot is only
-   *     a GUESS at server truth (the request may have committed and then timed
-   *     out), so a refetch is the authority — but a GET fired while another PATCH
-   *     is still airborne reads pre-PATCH state and is then overwritten by it,
-   *     stranding the very staleness the resync exists to fix.
-   */
-  async function persist(patch: MachineSettingsPatch) {
-    if (!settings) return;
-    const keys = Object.keys(patch) as (keyof MachineSettingsPatch)[];
-    const revert: MachineSettingsPatch = {};
-    for (const key of keys) Object.assign(revert, { [key]: settings[key] });
-
-    // The machine this save belongs to. If the tab is showing a DIFFERENT machine
-    // by the time the request settles, every write below would land on that
-    // machine's form — reverting machine A's name into machine B's input, where
-    // the next blur would rename B. Late results for a machine we've left are
-    // dropped (the resync too: it would refetch the wrong machine).
-    const forMachine = machineId;
-    const isStale = () => loadedFor.current !== forMachine;
-
-    setSettings((current) => (current ? { ...current, ...patch } : current));
-    pendingSavesRef.current += 1;
-    setPendingSaves(pendingSavesRef.current);
-    try {
-      await apiPatch('/api/machines/settings', { machineId: forMachine, ...patch });
-    } catch (error) {
-      if (isStale()) return;
-      setSettings((current) => (current ? { ...current, ...revert } : current));
-      if (revert.name !== undefined) setNameDraft(revert.name);
-      if (revert.description !== undefined) setDescriptionDraft(revert.description ?? '');
-      resyncWhenIdle.current = true;
-      toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
-    } finally {
-      pendingSavesRef.current -= 1;
-      setPendingSaves(pendingSavesRef.current);
-      if (!isStale() && pendingSavesRef.current === 0 && resyncWhenIdle.current) {
-        resyncWhenIdle.current = false;
-        setReloadToken((t) => t + 1);
-      }
-    }
-  }
-
-  function commitName() {
-    if (!settings) return;
-    const trimmed = nameDraft.trim();
-    // An empty name is rejected server-side (name is the page title). Revert the
-    // draft rather than firing a doomed PATCH.
-    if (trimmed.length === 0) {
-      setNameDraft(settings.name);
-      return;
-    }
-    setNameDraft(trimmed);
-    if (trimmed === settings.name) return;
-    persist({ name: trimmed });
-  }
-
-  function commitDescription() {
-    if (!settings) return;
-    const trimmed = descriptionDraft.trim();
-    setDescriptionDraft(trimmed);
-    if (trimmed === (settings.description ?? '')) return;
-    // Whitespace-only clears the description (round-trips to null server-side).
-    persist({ description: trimmed.length === 0 ? null : trimmed });
-  }
 
   async function handleDelete() {
     setDeleting(true);
@@ -252,7 +96,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
     return (
       <div className="flex h-full flex-col items-center justify-center gap-3">
         <p className="text-sm text-muted-foreground">Could not load machine settings.</p>
-        <Button type="button" variant="outline" size="sm" onClick={() => setReloadToken((t) => t + 1)}>
+        <Button type="button" variant="outline" size="sm" onClick={reload}>
           Retry
         </Button>
       </div>
@@ -320,7 +164,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Switch
                 id="machine-visible-global"
                 checked={settings.visibleToGlobalAssistant}
-                onCheckedChange={(checked) => persist({ visibleToGlobalAssistant: checked })}
+                onCheckedChange={(checked) => setAccess({ visibleToGlobalAssistant: checked })}
               />
             </div>
             <div className="flex items-center justify-between gap-4">
@@ -333,7 +177,7 @@ export default function SettingsTab({ machineId }: { machineId: string }) {
               <Switch
                 id="machine-allow-page-agents"
                 checked={settings.allowPageAgents}
-                onCheckedChange={(checked) => persist({ allowPageAgents: checked })}
+                onCheckedChange={(checked) => setAccess({ allowPageAgents: checked })}
               />
             </div>
           </CardContent>
diff --git a/apps/web/src/hooks/useMachineSettings.ts b/apps/web/src/hooks/useMachineSettings.ts
new file mode 100644
index 0000000000..496f460226
--- /dev/null
+++ b/apps/web/src/hooks/useMachineSettings.ts
@@ -0,0 +1,216 @@
+'use client';
+
+import { useEffect, useRef, useState } from 'react';
+import { toast } from 'sonner';
+import { fetchWithAuth, patch as apiPatch } from '@/lib/auth/auth-fetch';
+import type {
+  MachineSettings,
+  MachineSettingsPatch,
+} from '@pagespace/lib/services/machines/machine-settings';
+
+/**
+ * The Machine Settings state machine, behind `/api/machines/settings`.
+ *
+ * Deliberately NOT SWR (unlike its `useMachineProjects` / `useMachineBranches`
+ * siblings): the Settings tab is specified to follow `TerminalAccessCard`'s
+ * plain `useState`/`useEffect` + optimistic-`persist` shape. What SWR would have
+ * given us for free — revalidation — is the reason most of the machinery below
+ * exists, so it is spelled out explicitly here rather than left implicit in the
+ * view.
+ *
+ * The whole point of extracting this is that the failure path is where all the
+ * subtlety lives, and none of it is about rendering:
+ *
+ *  - Text fields are DRAFTS, committed on blur; toggles persist immediately. Both
+ *    can therefore be in flight AT ONCE, so every rule below is written for
+ *    concurrent saves, not a single one.
+ *  - A failed save reverts ONLY the keys it owned. Reverting a whole snapshot
+ *    would roll back an unrelated edit that succeeded while it was in flight.
+ *  - A failed save then RESYNCS, because the local snapshot is only a guess at
+ *    server truth (a request can commit and then time out, leaving the tab
+ *    permanently stale with nothing to revalidate it). The resync is deferred
+ *    until every save has drained: a GET fired while another PATCH is airborne
+ *    reads pre-PATCH state and is then overwritten by it — recreating exactly the
+ *    staleness it exists to fix.
+ *  - A resync adopts server values only for drafts the user has NOT touched,
+ *    so it can't delete text mid-keystroke.
+ *  - Results for a machine we have since navigated away from are dropped, rather
+ *    than written onto the machine now on screen.
+ */
+export interface UseMachineSettings {
+  settings: MachineSettings | null;
+  /** True only while we have nothing to show for this machine (first load / retry). */
+  loading: boolean;
+  /** Count, not a flag — a blur-commit and a toggle can be saving together. */
+  pendingSaves: number;
+  nameDraft: string;
+  setNameDraft: (value: string) => void;
+  descriptionDraft: string;
+  setDescriptionDraft: (value: string) => void;
+  /** Blur handlers: normalize, skip a no-op, and persist. */
+  commitName: () => void;
+  commitDescription: () => void;
+  /** Persist an access toggle immediately. */
+  setAccess: (patch: MachineSettingsPatch) => void;
+  reload: () => void;
+}
+
+export function useMachineSettings(machineId: string): UseMachineSettings {
+  const [settings, setSettings] = useState<MachineSettings | null>(null);
+  const [loading, setLoading] = useState(true);
+  const [reloadToken, setReloadToken] = useState(0);
+
+  const [pendingSaves, setPendingSaves] = useState(0);
+  // Mirrored into a ref because `persist`'s `finally` must read the CURRENT count,
+  // not the one closed over when it started.
+  const pendingSavesRef = useRef(0);
+  const resyncWhenIdle = useRef(false);
+
+  const [nameDraft, setNameDraft] = useState('');
+  const [descriptionDraft, setDescriptionDraft] = useState('');
+
+  // Last known SERVER value — lets a resync tell a clean draft from a dirty one.
+  const serverSettings = useRef<MachineSettings | null>(null);
+  // Which machine we currently hold settings for.
+  const loadedFor = useRef<string | null>(null);
+
+  useEffect(() => {
+    let cancelled = false;
+    const isNewMachine = loadedFor.current !== machineId;
+    if (isNewMachine) {
+      // Drop the previous machine's settings before loading a different one —
+      // otherwise a FAILED load would fall through to rendering the old machine's
+      // name/toggles under the new machine's identity. A resync owed to the machine
+      // we're leaving is dropped with it, so it can't fire a spurious refetch once
+      // an unrelated save on the NEW machine drains.
+      setSettings(null);
+      serverSettings.current = null;
+      resyncWhenIdle.current = false;
+    }
+    // Spinner whenever we have nothing to show for THIS machine. `serverSettings`
+    // moves in exact lockstep with `settings`, so the ref answers that without
+    // reading state — doing it via a `setSettings` updater instead would be an
+    // IMPURE UPDATER, which StrictMode replays at render time (after the load's
+    // `finally` already cleared `loading`), stranding the tab on a spinner forever.
+    if (isNewMachine || serverSettings.current === null) setLoading(true);
+
+    (async () => {
+      try {
+        const response = await fetchWithAuth(
+          `/api/machines/settings?machineId=${encodeURIComponent(machineId)}`,
+        );
+        if (!response.ok) throw new Error('Failed to load machine settings');
+        const json = (await response.json()) as { settings: MachineSettings };
+        if (cancelled) return;
+        const next = json.settings;
+        const previousServer = serverSettings.current;
+        loadedFor.current = machineId;
+        serverSettings.current = next;
+        setSettings(next);
+        // Adopt the server value only for drafts the user has NOT touched since we
+        // last saw the server: a resync fires while the tab is open and in use.
+        setNameDraft((draft) =>
+          previousServer === null || draft === previousServer.name ? next.name : draft,
+        );
+        setDescriptionDraft((draft) =>
+          previousServer === null || draft === (previousServer.description ?? '')
+            ? next.description ?? ''
+            : draft,
+        );
+      } catch (error) {
+        console.error('Failed to load machine settings:', error);
+        // A failed RESYNC keeps the form (and its already-good settings) on screen —
+        // only a failed FIRST load leaves `settings` null for the error state.
+        if (!cancelled) toast.error('Failed to load machine settings');
+      } finally {
+        if (!cancelled) setLoading(false);
+      }
+    })();
+
+    return () => {
+      cancelled = true;
+    };
+  }, [machineId, reloadToken]);
+
+  /**
+   * Optimistically apply `patch`, PATCH it, and on failure revert + resync.
+   *
+   * We do NOT reconcile from the PATCH response body. The route echoes the whole
+   * `MachineSettings`, so applying it would clobber a field the user is
+   * concurrently editing with a value that was already stale when the request
+   * left. Every field we send is normalized client-side exactly as the route
+   * normalizes it (name trimmed; blank description → null), so on success the
+   * optimistic value already equals the persisted one.
+   */
+  async function persist(patch: MachineSettingsPatch) {
+    if (!settings) return;
+    const revert: MachineSettingsPatch = {};
+    for (const key of Object.keys(patch) as (keyof MachineSettingsPatch)[]) {
+      Object.assign(revert, { [key]: settings[key] });
+    }
+
+    // If the tab is showing a DIFFERENT machine by the time this settles, every
+    // write below would land on that machine's form — reverting machine A's name
+    // into machine B's input, where the next blur would rename B.
+    const forMachine = machineId;
+    const isStale = () => loadedFor.current !== forMachine;
+
+    setSettings((current) => (current ? { ...current, ...patch } : current));
+    pendingSavesRef.current += 1;
+    setPendingSaves(pendingSavesRef.current);
+    try {
+      await apiPatch('/api/machines/settings', { machineId: forMachine, ...patch });
+    } catch (error) {
+      if (isStale()) return;
+      setSettings((current) => (current ? { ...current, ...revert } : current));
+      if (revert.name !== undefined) setNameDraft(revert.name);
+      if (revert.description !== undefined) setDescriptionDraft(revert.description ?? '');
+      resyncWhenIdle.current = true;
+      toast.error(error instanceof Error ? error.message : 'Failed to update machine settings');
+    } finally {
+      pendingSavesRef.current -= 1;
+      setPendingSaves(pendingSavesRef.current);
+      if (!isStale() && pendingSavesRef.current === 0 && resyncWhenIdle.current) {
+        resyncWhenIdle.current = false;
+        setReloadToken((t) => t + 1);
+      }
+    }
+  }
+
+  function commitName() {
+    if (!settings) return;
+    const trimmed = nameDraft.trim();
+    // An empty name is rejected server-side (it IS the page title). Revert the
+    // draft rather than firing a doomed PATCH.
+    if (trimmed.length === 0) {
+      setNameDraft(settings.name);
+      return;
+    }
+    setNameDraft(trimmed);
+    if (trimmed === settings.name) return;
+    persist({ name: trimmed });
+  }
+
+  function commitDescription() {
+    if (!settings) return;
+    const trimmed = descriptionDraft.trim();
+    setDescriptionDraft(trimmed);
+    if (trimmed === (settings.description ?? '')) return;
+    // Whitespace-only clears the description (round-trips to null server-side).
+    persist({ description: trimmed.length === 0 ? null : trimmed });
+  }
+
+  return {
+    settings,
+    loading,
+    pendingSaves,
+    nameDraft,
+    setNameDraft,
+    descriptionDraft,
+    setDescriptionDraft,
+    commitName,
+    commitDescription,
+    setAccess: persist,
+    reload: () => setReloadToken((t) => t + 1),
+  };
+}

From 1287fa4cfe4df189b770e60c7d502128e63ccfd2 Mon Sep 17 00:00:00 2001
From: 2witstudios <2witstudios@gmail.com>
Date: Sat, 11 Jul 2026 09:10:22 -0500
Subject: [PATCH 11/11] fix(machine): claim the machine on switch, not on load;
 narrow setAccess

Two findings from a review of the freshly-extracted hook.

1. The stale-save guard keyed off the machine last LOADED, so between switching
   machines and the new machine's GET resolving, a save from the machine we LEFT
   still looked current: its failure toasted an error for a machine the user was
   no longer on and wrote its old name into the new machine's draft. It self-healed
   (the new load overwrites), but the window was real. The ref now means "the
   machine the tab is showing" and is claimed the moment machineId changes;
   "do we have data yet?" was always a separate question, answered by
   serverSettings. Renamed loadedFor -> activeMachine to say so.

2. setAccess was typed with the full MachineSettingsPatch, so a caller could
   setAccess({ name }) and silently skip commitName's trim + empty-name guard.
   Narrowed to MachineAccessPatch (the two toggles), making the bypass
   unrepresentable rather than merely discouraged.

New test covers the window in (1) and is mutation-verified: restoring the
claim-on-load behaviour fails it. 21 tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwV8emSn4yaietjD9BCwb6
---
 .../terminal/tabs/SettingsTab.test.tsx        | 41 +++++++++++++++++++
 apps/web/src/hooks/useMachineSettings.ts      | 30 ++++++++++----
 2 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
index dc4effe7f6..7356197933 100644
--- a/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
+++ b/apps/web/src/components/layout/middle-content/page-views/terminal/tabs/SettingsTab.test.tsx
@@ -508,6 +508,47 @@ describe('SettingsTab', () => {
     });
   });
 
+  test('a stale save is dropped even while the new machine is still loading', async () => {
+    // The window between switching machines and the new machine's GET resolving.
+    // The tab has already MOVED — a save from the machine we left must be dropped
+    // here too, not just after the new one has landed. (It previously wasn't: the
+    // guard keyed off the last machine *loaded* rather than the one being shown,
+    // so a failure in this window toasted and wrote its name into the new form.)
+    let resolveM2: (v: unknown) => void = () => {};
+    fetchWithAuth
+      .mockResolvedValueOnce({ ok: true, json: async () => ({ settings: baseSettings }) })
+      .mockImplementationOnce(() => new Promise((resolve) => { resolveM2 = resolve; }));
+    let failRename: (e: Error) => void = () => {};
+    apiPatch.mockImplementation(() => new Promise((_r, reject) => { failRename = reject; }));
+
+    const { rerender } = render(<SettingsTab machineId="m1" />);
+    const name = await screen.findByLabelText('Name');
+    await userEvent.clear(name);
+    await userEvent.type(name, 'Renamed');
+    await userEvent.tab(); // m1's rename is in flight
+
+    rerender(<SettingsTab machineId="m2" />); // m2's GET is now hanging
+
+    failRename(new Error('too late'));
+    await act(async () => { await Promise.resolve(); });
+
+    assert({
+      given: "a save from the machine we left failing before the new machine has loaded",
+      should: 'be dropped silently — no toast for a machine the user is no longer on',
+      actual: (toast.error as ReturnType<typeof vi.fn>).mock.calls.length,
+      expected: 0,
+    });
+
+    // And m2 still loads cleanly on top of it.
+    resolveM2({ ok: true, json: async () => ({ settings: { ...baseSettings, name: 'Other Machine' } }) });
+    await waitFor(() => assert({
+      given: "the new machine's load resolving after the stale failure",
+      should: 'render the new machine, uncontaminated',
+      actual: (screen.getByLabelText('Name') as HTMLInputElement).value,
+      expected: 'Other Machine',
+    }));
+  });
+
   test('a first-load failure shows the error state and Retry refetches', async () => {
     fetchWithAuth
       .mockResolvedValueOnce({ ok: false, json: async () => ({}) })
diff --git a/apps/web/src/hooks/useMachineSettings.ts b/apps/web/src/hooks/useMachineSettings.ts
index 496f460226..02639ccdc3 100644
--- a/apps/web/src/hooks/useMachineSettings.ts
+++ b/apps/web/src/hooks/useMachineSettings.ts
@@ -37,6 +37,12 @@ import type {
  *  - Results for a machine we have since navigated away from are dropped, rather
  *    than written onto the machine now on screen.
  */
+/** The subset of the patch the access toggles are allowed to write. */
+export type MachineAccessPatch = Pick<
+  MachineSettingsPatch,
+  'visibleToGlobalAssistant' | 'allowPageAgents'
+>;
+
 export interface UseMachineSettings {
   settings: MachineSettings | null;
   /** True only while we have nothing to show for this machine (first load / retry). */
@@ -50,8 +56,13 @@ export interface UseMachineSettings {
   /** Blur handlers: normalize, skip a no-op, and persist. */
   commitName: () => void;
   commitDescription: () => void;
-  /** Persist an access toggle immediately. */
-  setAccess: (patch: MachineSettingsPatch) => void;
+  /**
+   * Persist an access toggle immediately. Deliberately NARROWER than the full
+   * patch: routing `name`/`description` through here would skip `commitName`'s
+   * trim + empty-name guard, so that bypass is made unrepresentable rather than
+   * merely discouraged.
+   */
+  setAccess: (patch: MachineAccessPatch) => void;
   reload: () => void;
 }
 
@@ -71,12 +82,17 @@ export function useMachineSettings(machineId: string): UseMachineSettings {
 
   // Last known SERVER value — lets a resync tell a clean draft from a dirty one.
   const serverSettings = useRef<MachineSettings | null>(null);
-  // Which machine we currently hold settings for.
-  const loadedFor = useRef<string | null>(null);
+  // The machine the tab is CURRENTLY showing — claimed the moment `machineId`
+  // changes, not when its load resolves. ("Do we have data yet?" is a separate
+  // question, answered by `serverSettings`.) If this only advanced on a successful
+  // load, a save from the machine we just left would still look current while the
+  // new one was in flight — toasting its error and writing its name into the new
+  // machine's draft.
+  const activeMachine = useRef<string | null>(null);
 
   useEffect(() => {
     let cancelled = false;
-    const isNewMachine = loadedFor.current !== machineId;
+    const isNewMachine = activeMachine.current !== machineId;
     if (isNewMachine) {
       // Drop the previous machine's settings before loading a different one —
       // otherwise a FAILED load would fall through to rendering the old machine's
@@ -86,6 +102,7 @@ export function useMachineSettings(machineId: string): UseMachineSettings {
       setSettings(null);
       serverSettings.current = null;
       resyncWhenIdle.current = false;
+      activeMachine.current = machineId;
     }
     // Spinner whenever we have nothing to show for THIS machine. `serverSettings`
     // moves in exact lockstep with `settings`, so the ref answers that without
@@ -104,7 +121,6 @@ export function useMachineSettings(machineId: string): UseMachineSettings {
         if (cancelled) return;
         const next = json.settings;
         const previousServer = serverSettings.current;
-        loadedFor.current = machineId;
         serverSettings.current = next;
         setSettings(next);
         // Adopt the server value only for drafts the user has NOT touched since we
@@ -153,7 +169,7 @@ export function useMachineSettings(machineId: string): UseMachineSettings {
     // write below would land on that machine's form — reverting machine A's name
     // into machine B's input, where the next blur would rename B.
     const forMachine = machineId;
-    const isStale = () => loadedFor.current !== forMachine;
+    const isStale = () => activeMachine.current !== forMachine;
 
     setSettings((current) => (current ? { ...current, ...patch } : current));
     pendingSavesRef.current += 1;