From 182c4d1f94ac0d4605c9485c89147523b2715302 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 15:49:50 +0000 Subject: [PATCH] fix(desktop): switching tabs no longer throws away the Fleet view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as "if i click away i lose all my data", and that is exactly right. Making Fleet a tab in v0.2.9 turned the body into a ternary: {tab === 'transfer' ? ( …panes… ) : } React unmounts the branch that is not rendered, so switching to Transfer destroyed everything in the Fleet view — a half-written script, the servers you had ticked, the output of a run still in flight — and switching back gave you a blank editor. Clicking a tab to check a path is not a reset, and this was a regression I introduced. Both views stay mounted now, and the inactive one is hidden. Verified by driving the built renderer: type a three-line script, tick two servers, set "at once" to 7, click Transfer, click Fleet — all three come back. The same complaint applies to quitting the app, so the Fleet editor now keeps an unsaved draft in the window's own storage: script, interpreter, sudo, pacing, timeout and the ticked servers. That is the safety net under "Save these settings", not a replacement for it — the saved command is still the deliberate act. Two things the draft deliberately does not do. It never stores a sudo password; it has no field for one, and there is a test asserting the written blob contains no such string. And every read falls back field by field, so last week's shape after a schema change, a half-written value, or storage that throws outright gives a blank editor rather than a broken window. A restored selection is filtered against the servers that actually exist, and missing ones are dropped quietly — this is a draft, not a saved list, and there is nothing for a run to get wrong yet. 508 tests plus 9 new ones for the draft. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UeSWg1Czsb2Lwxj8vHUnA4 --- apps/desktop/src/app/page.tsx | 19 +++- apps/desktop/src/components/fleet-view.tsx | 37 ++++++-- apps/desktop/src/lib/fleet-draft.test.ts | 103 +++++++++++++++++++++ apps/desktop/src/lib/fleet-draft.ts | 96 +++++++++++++++++++ docs/desktop.md | 11 +++ 5 files changed, 253 insertions(+), 13 deletions(-) create mode 100644 apps/desktop/src/lib/fleet-draft.test.ts create mode 100644 apps/desktop/src/lib/fleet-draft.ts diff --git a/apps/desktop/src/app/page.tsx b/apps/desktop/src/app/page.tsx index 6bc37fe..edeac6d 100644 --- a/apps/desktop/src/app/page.tsx +++ b/apps/desktop/src/app/page.tsx @@ -447,8 +447,16 @@ export default function Workspace() { footer and status band; Fleet brings its own, pinned so the action it exists for cannot scroll out of reach. */} - {tab === 'transfer' ? ( - <> + {/* + Both views stay MOUNTED, and the inactive one is hidden. + + This was a ternary, so switching tabs unmounted the other branch and + React threw its state away: a half-written script, the servers you had + ticked, the output of a run still going. Clicking Transfer to check a + path and coming back to a blank Fleet view is not a tab, it is a + reset. + */} +
- - ) : ( +
+ +
setShowConnection(true)} /> - )} +
setShowConnection(false)} onSaved={() => void refreshConnections()} /> diff --git a/apps/desktop/src/components/fleet-view.tsx b/apps/desktop/src/components/fleet-view.tsx index c32a4a4..02edb7b 100644 --- a/apps/desktop/src/components/fleet-view.tsx +++ b/apps/desktop/src/components/fleet-view.tsx @@ -25,6 +25,7 @@ import { Input } from '@/components/ui/input' import { ScrollArea } from '@/components/ui/scroll-area' import { Textarea } from '@/components/ui/textarea' import { blankHost, foldHosts, type HostView } from '@/lib/fleet-events' +import { readDraft, writeDraft } from '@/lib/fleet-draft' import { api, unwrap, @@ -73,19 +74,25 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) { const [newListName, setNewListName] = useState('') const [savingCommand, setSavingCommand] = useState(false) const [newCommandName, setNewCommandName] = useState('') - const [selected, setSelected] = useState>(new Set()) + const [selected, setSelected] = useState>(() => new Set(readDraft().selected)) const [tagFilter, setTagFilter] = useState(null) - const [script, setScript] = useState('') - const [label, setLabel] = useState('') + // Read once, synchronously, as the initial state. Restoring in an effect + // would paint an empty editor and then replace what you were looking at. + const [draft] = useState(readDraft) + + const [script, setScript] = useState(draft.script) + const [label, setLabel] = useState(draft.label) const [commandId, setCommandId] = useState(null) - const [interpreter, setInterpreter] = useState<'sh' | 'bash' | 'raw'>('raw') - const [sudo, setSudo] = useState(false) + const [interpreter, setInterpreter] = useState<'sh' | 'bash' | 'raw'>(draft.interpreter) + const [sudo, setSudo] = useState(draft.sudo) + // Never restored. A password is held for one run, and writing it anywhere it + // could be read back is exactly what the CLI refuses to do. const [sudoPassword, setSudoPassword] = useState('') const [askSudoPassword, setAskSudoPassword] = useState(false) - const [concurrency, setConcurrency] = useState(4) - const [timeoutSeconds, setTimeoutSeconds] = useState(900) - const [stopOnError, setStopOnError] = useState(false) + const [concurrency, setConcurrency] = useState(draft.concurrency) + const [timeoutSeconds, setTimeoutSeconds] = useState(draft.timeoutSeconds) + const [stopOnError, setStopOnError] = useState(draft.stopOnError) const [hazards, setHazards] = useState([]) const [runId, setRunId] = useState(null) @@ -108,6 +115,14 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) { setServers(serverList) setCommands(commandList) setLists(listList) + // A restored draft can name servers that have since been deleted. + // Dropping them quietly is right here — this is a draft, not a saved + // list, and there is nothing for the run to get wrong yet. + const live = new Set(serverList.map((server) => server.id)) + setSelected((current) => { + const kept = [...current].filter((id) => live.has(id)) + return kept.length === current.size ? current : new Set(kept) + }) } catch (caught) { setError(caught instanceof Error ? caught.message : String(caught)) } @@ -117,6 +132,12 @@ export function FleetView({ onAddServer }: { onAddServer: () => void }) { void refresh() }, [refresh]) + // Keep the draft current. Cheap, and it is the difference between closing + // the window and losing a fifty-line script. + useEffect(() => { + writeDraft({ script, interpreter, sudo, concurrency, timeoutSeconds, stopOnError, label, selected: [...selected] }) + }, [script, interpreter, sudo, concurrency, timeoutSeconds, stopOnError, label, selected]) + useEffect(() => { const bridge = api() if (!bridge) return diff --git a/apps/desktop/src/lib/fleet-draft.test.ts b/apps/desktop/src/lib/fleet-draft.test.ts new file mode 100644 index 0000000..dfc6342 --- /dev/null +++ b/apps/desktop/src/lib/fleet-draft.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { clearDraft, EMPTY_DRAFT, readDraft, writeDraft } from './fleet-draft' + +/** A stand-in for localStorage, since vitest runs this without a DOM. */ +function installStorage(impl?: Partial) { + const data = new Map() + const storage = { + getItem: (k: string) => data.get(k) ?? null, + setItem: (k: string, v: string) => void data.set(k, v), + removeItem: (k: string) => void data.delete(k), + ...impl, + } + vi.stubGlobal('window', { localStorage: storage }) + return data +} + +beforeEach(() => { + vi.unstubAllGlobals() +}) + +describe('readDraft', () => { + it('is empty when nothing was ever written', () => { + installStorage() + expect(readDraft()).toEqual(EMPTY_DRAFT) + }) + + it('round-trips what was written', () => { + installStorage() + const draft = { + script: 'systemctl restart app.service', + interpreter: 'bash' as const, + sudo: true, + concurrency: 7, + timeoutSeconds: 45, + stopOnError: true, + label: 'restart-app', + selected: ['id-web-01', 'id-web-03'], + } + writeDraft(draft) + expect(readDraft()).toEqual(draft) + }) + + it('returns a blank draft rather than throwing on nonsense', () => { + // Last week's shape after a schema change, or a half-written value. + const data = installStorage() + data.set('diskpush:fleet-draft:v1', '{not json') + expect(readDraft()).toEqual(EMPTY_DRAFT) + }) + + it('falls back field by field, so one bad value does not lose the script', () => { + const data = installStorage() + data.set( + 'diskpush:fleet-draft:v1', + JSON.stringify({ script: 'keep me', interpreter: 'python', concurrency: -4, selected: 'nope' }), + ) + const draft = readDraft() + expect(draft.script).toBe('keep me') + expect(draft.interpreter).toBe(EMPTY_DRAFT.interpreter) + expect(draft.concurrency).toBe(EMPTY_DRAFT.concurrency) + expect(draft.selected).toEqual([]) + }) + + it('survives storage that throws outright', () => { + // Private windows and blocked site data throw on access, not on read. + installStorage({ + getItem: () => { + throw new Error('access denied') + }, + }) + expect(readDraft()).toEqual(EMPTY_DRAFT) + }) + + it('is a blank draft, never a crash, with no window at all', () => { + vi.stubGlobal('window', undefined) + expect(readDraft()).toEqual(EMPTY_DRAFT) + }) +}) + +describe('writeDraft', () => { + it('never throws, because losing a draft must not break a run', () => { + installStorage({ + setItem: () => { + throw new Error('quota exceeded') + }, + }) + expect(() => writeDraft({ ...EMPTY_DRAFT, script: 'x' })).not.toThrow() + }) + + it('never stores a sudo password, because the draft has no field for one', () => { + const data = installStorage() + writeDraft({ ...EMPTY_DRAFT, script: 'id' }) + expect(data.get('diskpush:fleet-draft:v1')).not.toMatch(/password/i) + }) +}) + +describe('clearDraft', () => { + it('removes it', () => { + installStorage() + writeDraft({ ...EMPTY_DRAFT, script: 'x' }) + clearDraft() + expect(readDraft()).toEqual(EMPTY_DRAFT) + }) +}) diff --git a/apps/desktop/src/lib/fleet-draft.ts b/apps/desktop/src/lib/fleet-draft.ts new file mode 100644 index 0000000..2a6f6c2 --- /dev/null +++ b/apps/desktop/src/lib/fleet-draft.ts @@ -0,0 +1,96 @@ +'use client' + +/** + * What you had typed, kept across a restart. + * + * Keeping both tabs mounted stops a tab switch from wiping the Fleet view, but + * quitting the app still would, and a fifty-line script is not something to + * lose to a window close. This is the unsaved draft — the thing you have not + * decided to name yet. A *saved* command is the deliberate act; this is the + * safety net under it. + * + * `localStorage`, not the database: it is per-machine UI state, it must be + * readable synchronously during the first render (an async round trip would + * paint an empty editor and then replace what you were looking at), and it + * carries nothing anyone would want in a backup. + */ + +const KEY = 'diskpush:fleet-draft:v1' + +export type FleetDraft = { + script: string + interpreter: 'sh' | 'bash' | 'raw' + sudo: boolean + concurrency: number + timeoutSeconds: number + stopOnError: boolean + label: string + /** Connection ids. Ones that no longer exist are dropped on read. */ + selected: string[] +} + +export const EMPTY_DRAFT: FleetDraft = { + script: '', + interpreter: 'raw', + sudo: false, + concurrency: 4, + timeoutSeconds: 900, + stopOnError: false, + label: '', + selected: [], +} + +/** + * Reads the draft, and is not allowed to fail. + * + * Storage throws outright in some contexts, and anything in there is last + * week's shape after a schema change. A draft that cannot be restored is a + * blank editor, never a broken window. + */ +export function readDraft(): FleetDraft { + if (typeof window === 'undefined') return EMPTY_DRAFT + try { + const raw = window.localStorage.getItem(KEY) + if (!raw) return EMPTY_DRAFT + const parsed = JSON.parse(raw) as Partial + return { + script: typeof parsed.script === 'string' ? parsed.script : EMPTY_DRAFT.script, + interpreter: + parsed.interpreter === 'sh' || parsed.interpreter === 'bash' || parsed.interpreter === 'raw' + ? parsed.interpreter + : EMPTY_DRAFT.interpreter, + sudo: typeof parsed.sudo === 'boolean' ? parsed.sudo : EMPTY_DRAFT.sudo, + concurrency: positive(parsed.concurrency, EMPTY_DRAFT.concurrency), + timeoutSeconds: positive(parsed.timeoutSeconds, EMPTY_DRAFT.timeoutSeconds), + stopOnError: typeof parsed.stopOnError === 'boolean' ? parsed.stopOnError : EMPTY_DRAFT.stopOnError, + label: typeof parsed.label === 'string' ? parsed.label : EMPTY_DRAFT.label, + selected: Array.isArray(parsed.selected) ? parsed.selected.filter((id) => typeof id === 'string') : [], + } + } catch { + return EMPTY_DRAFT + } +} + +/** Writes the draft. Also cannot fail: losing a draft must not break a run. */ +export function writeDraft(draft: FleetDraft): void { + if (typeof window === 'undefined') return + try { + window.localStorage.setItem(KEY, JSON.stringify(draft)) + } catch { + // A private window, cleared site data, or storage disabled. The editor + // still works; it just will not be there next time. + } +} + +export function clearDraft(): void { + if (typeof window === 'undefined') return + try { + window.localStorage.removeItem(KEY) + } catch { + /* see writeDraft */ + } +} + +function positive(value: unknown, fallback: number): number { + return typeof value === 'number' && Number.isFinite(value) && value > 0 ? value : fallback +} diff --git a/docs/desktop.md b/docs/desktop.md index 274cc1f..7a99882 100644 --- a/docs/desktop.md +++ b/docs/desktop.md @@ -137,6 +137,17 @@ It names what it matched and how many servers it would reach. That check is repeated in the main process, so it is not something the window can be talked out of. +## Switching tabs keeps your work + +Both views stay mounted; the inactive one is hidden, not destroyed. Clicking +Transfer to check a path and coming back finds the Fleet view exactly as you +left it — the script, the ticked servers, the output of a run still going. + +The Fleet editor's contents also survive quitting the app. That is the +*unsaved draft*: the thing you have not decided to name yet, kept in the +window's own storage as a safety net under **Save these settings**. A sudo +password is never part of it. + ## Keyboard ```text