diff --git a/TESTING_AND_MODIFICATION_GUIDE.md b/TESTING_AND_MODIFICATION_GUIDE.md index f3b0a0beb..c4e38711c 100644 --- a/TESTING_AND_MODIFICATION_GUIDE.md +++ b/TESTING_AND_MODIFICATION_GUIDE.md @@ -196,7 +196,7 @@ the heading, and the rule gets a `(rationale)` marker. | Concern | Code | Spec | |---|---|---| | Strip look, tabs, menus, rename, indicators | `lib/src/components/WorkspaceStrip.tsx`, `workspace-strip-drag.ts`, stories in `lib/src/stories/` | `docs/specs/layout.md` → Workspaces; `standalone.md` → AppBar; `alert.md` → Workspace union | -| Shared Workspace kill confirmation and iframe move gate | `lib/src/components/WorkspaceKillConfirm.tsx` ("Confirm kill workspace"), `lib/src/components/WorkspaceStrip.tsx` (`pendingClose` and the iframe `pendingMove` gate), `standalone/src/WorkspaceTeardownModal.tsx` (window close / app quit), `lib/src/lib/workspace-ui-store.ts` | `layout.md` → Workspaces; `standalone.md` → Confirmation UI | +| Shared Workspace kill confirmation and iframe move gate | `lib/src/components/WorkspaceKillConfirm.tsx` ("Confirm kill workspace"), `lib/src/components/WorkspaceStrip.tsx` (`confirmation`), `standalone/src/WorkspaceTeardownModal.tsx` (window close / app quit), `lib/src/lib/workspace-ui-store.ts` | `layout.md` → Workspaces; `standalone.md` → Confirmation UI | | Command-mode keys (`c n p l 1-9 W & !` etc.) | `lib/src/components/wall/keyboard/handle-workspace-shortcuts.ts` | `layout.md` → Workspaces, `shortcuts.md` | | Composition, active/hidden Wall, input gating | `lib/src/components/WorkspaceWindow.tsx`, `Wall.tsx` (`WorkspaceActiveContext`) | `layout.md` → Workspaces | | Hidden-Workspace terminal minimize | `lib/src/components/TerminalPane.tsx` mount effect (gated on `workspaceActive`), `lib/src/lib/terminal-lifecycle.ts` (`mountElement`/`unmountElement`), `terminal-webgl.ts` | `layout.md` → Workspaces, Renderer; `layout.rationale.md` → Workspaces | diff --git a/docs/specs/layout.md b/docs/specs/layout.md index 12f57087a..1868527c1 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -254,7 +254,7 @@ Source of truth: `createWorkspaceMotion` in `lib/src/components/workspace-motion - **Must offer `+` and New workspace only when the source has more than one Surface**, counting Panes and Doors; create a receiving Wall with only the moved Surface. Disable the picker item and `+` drop otherwise, and refuse CLI `--new`. - **Must offer Move to workspace in terminal and Tool context** (placement: the Title row above), using the same coordinator as dragging. **Never add a browser context menu. Never add a command-mode move binding.** Browser Surfaces move by dragging or CLI. - **Must retain stable Surface identity and Session state while remounting in the destination Wall**: terminals keep their registry instance, browser automation reconnects, and a retained helper follows its source. Never close a departing Session. Pin a moved preview slot by removing its preview mark. -- **Must confirm plain iframe and serving iframe Tool moves before creating a destination or changing membership**, with a stable random character over the Window content area. Doors remain minimized while waiting. Show “moving this iframe will trigger a refresh and reopen at its saved URL, possibly losing page state or returning to an earlier page”; the prompted character confirms, anything else cancels. Saved URLs are last-known URLs, not necessarily the page's current location. CLI consent follows `docs/specs/dor-cli.md` → dor move. +- **Must confirm plain iframe and serving iframe Tool moves before creating a destination or changing membership**, with a stable random character over the Window content area ([one pending confirmation](#workspace-lifecycle)). Doors remain minimized while waiting. Show “moving this iframe will trigger a refresh and reopen at its saved URL, possibly losing page state or returning to an earlier page”; the prompted character confirms, anything else cancels. Saved URLs are last-known URLs, not necessarily the page's current location. CLI consent follows `docs/specs/dor-cli.md` → dor move. - **Must refuse dirty Tools, pending Tool approval, browser startup, closing Surfaces/Workspaces and helper promotion**, rechecking after consent and asynchronous preparation. Dirty/pending refusals cannot be bypassed by iframe consent. - **Must follow a GUI move into destination passthrough** (acknowledgement: `docs/specs/alert.md` → Workspace union); CLI focus policy follows `docs/specs/dor-cli.md` → dor move. Remove a source with no Panes or Doors; if Doors remain but no pane does, refill normally. - **Must prepare before departure and roll back failed adoption**, restoring layout, Doors, parked state, selection, zoom, metadata and refs. Ref allocation belongs to `docs/specs/dor-cli.md` → Handle Model; coordinated durable publication belongs to `docs/specs/transport.md` → Persisted session types; Activity follows `docs/specs/alert.md` → Workspace union. @@ -296,17 +296,22 @@ nothing (`iframeSurfaceRefs` on the Wall handle; `standalone/src/workspace-drag. - **Must drop the closing Workspace’s rename editor and pending confirmation, and no other’s** (`releases the rename lease when the tab being renamed is middle-clicked closed` in `lib/src/components/WorkspaceStrip.test.tsx`; `preserves another Workspace’s rename and close confirmation when closing a sibling` in `lib/src/components/wall/workspace-lifecycle.test.ts`). - **Every Workspace verb runs outside the strip**, which renders the rename editor and confirmation from a store, so tab gestures and `dor` commands take one path. -**Must use `WorkspaceKillConfirm` for Workspace close, the iframe move gate, +**Must use `WorkspaceKillConfirm` for Workspace close, the iframe move gates, and host termination confirmations**, titled “Confirm kill workspace” except the -move gate: **a bare matching letter confirms, another bare key cancels, and a +move gates: **a bare matching letter confirms, another bare key cancels, and a modifier or chord never answers**, so `Cmd+Q` still quits. **Must ignore -its confirmation key while that Workspace transfers.** A successful transfer -dismisses only the departing Workspace's pending close, move, and rename UI; -a failed transfer retains them. No pending kill follows a Workspace to its +the close confirmation's key while that Workspace transfers.** +**Must hold at most one pending Workspace confirmation** (close, cross-Window +move gate, Surface move iframe consent), **answered no when a newer one is +raised or any close, cross-Window move, or Surface move starts**, by gesture or +`dor`, even one that refuses. **Must abandon superseded preparation while awaiting a Wall, window probe or editor decision**, so an older verb cannot later act or replace the newer question. A move refusal waits behind it and rename +(`lib/src/components/WorkspaceWindow.test.tsx`, `lib/src/lib/workspace-ui-store.test.ts`). +A successful transfer dismisses only the departing Workspace's pending +confirmation and rename UI; a failed transfer retains them. No pending kill follows a Workspace to its destination. Pinned by `does not accept a pending kill during transfer and releases its keyboard lease on departure` in `lib/src/components/WorkspaceStrip.test.tsx` and `keeps the pending kill until commit, then dismisses only the departing Workspace` in `lib/src/components/wall/workspace-transfer.test.ts`. -Source of truth: `dismissWorkspaceUi` in `lib/src/lib/workspace-ui-store.ts`; +Source of truth: `requestConfirmation` / `cancelPendingConfirmation` / `dismissWorkspaceUi` in `lib/src/lib/workspace-ui-store.ts`; `prepareWorkspaceTransfer` in `lib/src/components/wall/workspace-transfer.ts`. The union projection and its indicators are owned by `docs/specs/alert.md` → Workspace union; the strip that renders them by `docs/specs/standalone.md` → AppBar. Persisted containers are owned by `docs/specs/transport.md`: standalone stores one `PersistedWindow` per window, so a relaunch restores every Workspace ([Session persistence](#session-persistence)). @@ -355,7 +360,7 @@ That order is load-bearing twice: a rename input suppresses the pane shortcuts b **Every open dialog holds its own reference-counted lease on that gate**, and command-mode dispatch resumes only once the last lease is released — so a dialog closing over another cannot lift the survivor's suppression (`createDialogKeyboardCoordinator` in `lib/src/components/wall/wall-context.tsx`). -**Must defer Workspace close and move confirmations while an inline Workspace rename editor is open**, leaving its keys to the input; the pending gate appears after rename ends. Pinned by `defers the %s gate while another Workspace is being renamed` in `lib/src/components/WorkspaceStrip.test.tsx`. +**Must defer the pending Workspace confirmation while an inline Workspace rename editor is open**, leaving its keys to the input; it appears after rename ends. Pinned by `defers the %s gate while another Workspace is being renamed` in `lib/src/components/WorkspaceStrip.test.tsx`. **Chrome outside every Wall takes the chrome keyboard lease instead**: the Workspace strip's rename editor and close confirmation live in the app bar, where `stopPropagation` cannot reach a capture-phase window listener. **The Workspace branch is inert on a Wall with no Workspace id**, which is what leaves those keys unbound on a bare Wall. Source of truth: `acquireChromeKeyboardLease` in `lib/src/components/wall/chrome-keyboard-lease.ts`; `handleWorkspaceShortcuts` in `lib/src/components/wall/keyboard/handle-workspace-shortcuts.ts`. diff --git a/docs/specs/transport.md b/docs/specs/transport.md index e270184ca..892b964a1 100644 --- a/docs/specs/transport.md +++ b/docs/specs/transport.md @@ -243,7 +243,7 @@ Source of truth: `ManagedVoicePort` in `lib/src/lib/platform/managed-voice-types **Each mounted Workspace publishes its `PersistedSession` to a Window collector**, which orders them by the Workspace store and writes the whole Window through one debounced writer the host installs at boot. **A Workspace with neither a published nor a boot-seeded session is dropped rather than written empty**, so a snapshot taken mid-boot cannot replace a restored Workspace with a blank one. **A Workspace's save compares against its own previous record** — seeded from disk until its Wall publishes — never the Window's active one, or a dead PTY's retained cwd and alert would come from the wrong Workspace. **Reordering, renaming, or switching the active Workspace writes too**: each changes the blob with no Session changing. A `PersistedWorkspace` is a `WorkspaceId`, a `name`, `nameIsAuto`, and that Workspace's `PersistedSession`. **Always write `nameIsAuto`**; lacking it, only a `Workspace ` name is auto. The top-level snapshot is a `PersistedWindow` (its own `version: 1`) wrapping v3 sessions: the ordered `PersistedWorkspace` list plus the active `WorkspaceId`. **VS Code does not use it** — each webview persists one bare `PersistedSession`, its single Workspace, through its own per-surface state API (`docs/specs/vscode.md`). -**Must publish both Workspace records together across a Surface move**, fencing saves collected before ownership changed and retaining a departed Session's previous cwd/alert in the destination. Window writes are held until both records publish or the move rolls back. Source of truth: `beginWorkspaceSessionBatch` / `invalidateWorkspaceSaves` / `moveRetainedSurfaceRecord` in `lib/src/lib/window-session-aggregator.ts`; `doSave` in `lib/src/components/wall/use-session-persistence.ts`. +**Must publish both Workspace records in one synchronous step with the Surface move's ownership change**, unprobed and behind one Window write (`pagehide` included), fencing saves collected before or during the ownership change and retaining a departed Session's previous cwd/alert in the destination. Source of truth: `publishWorkspaceSessions` / `invalidateWorkspaceSaves` / `moveRetainedSurfaceRecord` in `lib/src/lib/window-session-aggregator.ts`; `serializeNow` / `doSave` in `lib/src/components/wall/use-session-persistence.ts`; `assemblePersistedSession` in `lib/src/lib/session-save.ts`. **The Window wrapping lives at the standalone adapter boundary, never in the shared save/restore code.** `window-persistence.ts` owns the JSON and the storage slot over a `SessionKeyValueStore` synchronous slot (`docs/specs/standalone.md` → Persistence); the shared save/restore code still operates on a bare `PersistedSession`, which the boot hands it per Workspace. **A Window-persisting adapter answers through `getWindowState` / `saveWindowState`, and answers nothing on the bare-Session `getState` / `saveState` pair** — its blob is a Window and every shared reader of `getState` wants a Session. **A blob written before standalone persisted Windows is wrapped as the window's one Workspace**; that is the only migration. diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 469525ab5..9be8420bd 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -1768,7 +1768,8 @@ export function Wall({ // re-render never replaces a registered entry. // Capture before a synchronous commit. Rollback restores the complete Wall, // including a Door's held rect and the ref allocator, without killing Sessions. - const captureMoveRollback = (incomingId?: string) => { + // It runs only before `finishSurfaceMove`, so no refill has mounted a shell. + const captureMoveRollback = () => { const snapshot = lath.store.getSnapshot(); const savedDoors = doorsRef.current; const refs = new Map(dorSurfaceRefsRef.current); @@ -1780,10 +1781,6 @@ export function Wall({ const context = terminalContextRef.current; return () => { movingSurfaceRef.current = true; - // A temporary auto-refill may already have mounted its default shell. - for (const member of memberSurfaceIds()) { - if (member !== incomingId && !snapshot.leafMeta.has(member)) disposeSession(member); - } doorsRef.current = savedDoors; setDoors(savedDoors); dorSurfaceRefsRef.current = refs; nextDorSurfaceRefIndexRef.current = next; lath.store.restoreSnapshot(snapshot); @@ -1833,7 +1830,7 @@ export function Wall({ }, adoptSurfaceMove: (id, meta) => { if (ownsSurface(id) || closingWorkspaceRef.current) throw new Error('The destination cannot accept this Surface'); - const rollback = captureMoveRollback(id); + const rollback = captureMoveRollback(); movingSurfaceRef.current = true; const ref = livePaneId(); if (!lath.store.addLeaf(id, meta, ref ? { refId: ref, edge: lath.store.autoEdgeFor(ref) } : null).ok) throw new Error('Could not place the Surface'); @@ -1850,7 +1847,7 @@ export function Wall({ else enterTerminalMode(id); }, showMoveNotice: (id, text) => showShellSpawnNotice(id, text, 8000), - serializePersistence: persistence.serialize, + serializeNow: persistence.serializeNow, surfaceIds: memberSurfaceIds, ownsSurface, diff --git a/lib/src/components/WorkspaceStrip.test.tsx b/lib/src/components/WorkspaceStrip.test.tsx index 3b5076d61..49ec0b348 100644 --- a/lib/src/components/WorkspaceStrip.test.tsx +++ b/lib/src/components/WorkspaceStrip.test.tsx @@ -8,7 +8,7 @@ import { WorkspaceStrip } from './WorkspaceStrip'; import { chromeKeyboardHeld, resetChromeKeyboardLeases } from './wall/chrome-keyboard-lease'; import { registerWallHandle, resetWallHandles, stubWallHandle, type WallHandle } from './wall/wall-handles'; import { ensureResizeObserver } from './wall/wall-test-utils'; -import { requestWorkspaceClose, requestWorkspaceRename } from './wall/workspace-lifecycle'; +import { requestWorkspaceClose, requestWorkspaceRename, workspaceCloseConfirmation } from './wall/workspace-lifecycle'; import { resetWorkspaceSurfaces, setWorkspaceSurfaces } from '../lib/workspace-surfaces'; import { clearTerminalActivity, setTerminalActivity } from '../lib/terminal-registry'; import { createAlertEpisode } from '../lib/alert-episode'; @@ -18,8 +18,7 @@ import { getWorkspaceUiSnapshot, dismissWorkspaceUi, resetWorkspaceUi, - setPendingWorkspaceClose, - setPendingWorkspaceMove, + requestConfirmation, setWorkspaceMoveError, } from '../lib/workspace-ui-store'; import { @@ -525,8 +524,7 @@ describe('WorkspaceStrip', () => { await render(); await act(async () => { requestWorkspaceRename(first); }); await act(async () => { - if (kind === 'close') setPendingWorkspaceClose({ id: 'ws-2', char: 'q' }); - else setPendingWorkspaceMove({ id: 'ws-2', char: 'q', iframeCount: 1, proceed }); + requestConfirmation(kind === 'close' ? workspaceCloseConfirmation('ws-2', 'q') : { id: 'ws-2', char: 'q', answer: ok => { if (ok) void proceed(); } }); }); const input = container.querySelector(`[data-workspace-rename-for="${first}"]`)!; expect(document.body.querySelector('#kill-confirm-title')).toBeNull(); @@ -540,7 +538,7 @@ describe('WorkspaceStrip', () => { expect(proceed).toHaveBeenCalledOnce(); }); - it('keeps the move gate behind a close confirmation, and ignores modifiers and chords', async () => { + it('answers the pending confirmation no when a newer one is raised, and ignores modifiers and chords', async () => { const first = getWorkspacesSnapshot().workspaces[0].id; await act(async () => { createWorkspace({ id: 'ws-2' }); }); stubHandle(first); @@ -548,7 +546,8 @@ describe('WorkspaceStrip', () => { stubHandle('ws-2', { closeAll: closed }); await render(); const proceed = vi.fn(); - await act(async () => { setPendingWorkspaceMove({ id: first, char: 'k', iframeCount: 1, proceed }); }); + const move = vi.fn((ok: boolean) => { if (ok) proceed(); }); + await act(async () => { requestConfirmation({ id: first, char: 'k', title: 'Move and lose page state?', answer: move }); }); expect(document.body.querySelector('#kill-confirm-title')).not.toBeNull(); // A bare modifier or a chord is never an answer, even on the gate's letter. @@ -559,27 +558,22 @@ describe('WorkspaceStrip', () => { window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', ctrlKey: true, bubbles: true })); window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', metaKey: true, bubbles: true })); }); - expect(proceed).not.toHaveBeenCalled(); - expect(getWorkspaceUiSnapshot().pendingMove).not.toBeNull(); + expect(move).not.toHaveBeenCalled(); + expect(getWorkspaceUiSnapshot().confirmation).not.toBeNull(); - // A close raised over it shows its own letter; the same letter typed at it - // closes and must not also move, however the two were minted. - await act(async () => { setPendingWorkspaceClose({ id: 'ws-2', char: 'k' }); }); + // A close raised over it answers the move no; the same letter typed at the + // close closes and must not also move, however the two were minted. + await act(async () => { requestConfirmation(workspaceCloseConfirmation('ws-2', 'k')); }); + expect(move).toHaveBeenCalledExactlyOnceWith(false); + expect(document.body.querySelector('#kill-confirm-title')?.textContent).toBe('Confirm kill workspace'); await act(async () => { window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', bubbles: true })); }); await act(async () => { await Promise.resolve(); }); expect(closed).toHaveBeenCalledTimes(1); expect(proceed).not.toHaveBeenCalled(); - expect(getWorkspaceUiSnapshot().pendingClose).toBeNull(); - expect(getWorkspaceUiSnapshot().pendingMove).not.toBeNull(); - - // With the close resolved the gate is armed again, and its letter moves. - await act(async () => { - window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', bubbles: true })); - }); - expect(proceed).toHaveBeenCalledTimes(1); - expect(getWorkspaceUiSnapshot().pendingMove).toBeNull(); + expect(getWorkspaceUiSnapshot().confirmation).toBeNull(); + expect(document.body.querySelector('#kill-confirm-title')).toBeNull(); }); it('releases the rename lease when the tab being renamed is middle-clicked closed', async () => { @@ -637,13 +631,13 @@ describe('WorkspaceStrip', () => { const closeAll = vi.fn(async () => null); stubHandle('ws-2', { hasTouchedSurfaces: () => true, closeAll }); await render(); - await act(async () => { setPendingWorkspaceClose({ id: 'ws-2', char: 'q' }); }); + await act(async () => { requestConfirmation(workspaceCloseConfirmation('ws-2', 'q')); }); expect(document.body.querySelector('#kill-confirm-title')?.textContent).toBe('Confirm kill workspace'); expect(chromeKeyboardHeld()).toBe(true); setWorkspaceTransferPending('ws-2', true); await act(async () => { window.dispatchEvent(new KeyboardEvent('keydown', { key: 'q', bubbles: true })); }); expect(closeAll).not.toHaveBeenCalled(); - expect(getWorkspaceUiSnapshot().pendingClose?.id).toBe('ws-2'); + expect(getWorkspaceUiSnapshot().confirmation?.id).toBe('ws-2'); // A failed transfer leaves this prompt usable; successful commit dismisses it. setWorkspaceTransferPending('ws-2', false); await act(async () => { dismissWorkspaceUi('ws-2'); }); diff --git a/lib/src/components/WorkspaceStrip.tsx b/lib/src/components/WorkspaceStrip.tsx index 738004eae..74fdfc87b 100644 --- a/lib/src/components/WorkspaceStrip.tsx +++ b/lib/src/components/WorkspaceStrip.tsx @@ -20,16 +20,14 @@ import { createWorkspaceStripDrag, type StripDragHost } from './workspace-strip- import { acquireChromeKeyboardLease } from './wall/chrome-keyboard-lease'; import { getWallHandle } from './wall/wall-handles'; import { useDialogKeyboardOwner } from './wall/wall-context'; -import { closeWorkspaceWithSurfaces, enterWorkspace, requestWorkspaceClose, requestWorkspaceRename } from './wall/workspace-lifecycle'; +import { enterWorkspace, requestWorkspaceClose, requestWorkspaceRename } from './wall/workspace-lifecycle'; import { getActivitySnapshot, subscribeToActivity } from '../lib/terminal-registry'; import { getWorkspaceSurfacesSnapshot, subscribeToWorkspaceSurfaces } from '../lib/workspace-surfaces'; import { computeWorkspaceUnion, type WorkspaceUnion } from '../lib/workspace-union'; import { spotlightTodo } from '../lib/todo-spotlight'; -import { isWorkspaceTransferPending } from '../lib/window-session-aggregator'; import { getWorkspaceUiSnapshot, - setPendingWorkspaceClose, - setPendingWorkspaceMove, + settleConfirmation, setWorkspaceMoveError, setRenamingWorkspace, subscribeToWorkspaceUi, @@ -72,7 +70,7 @@ export function WorkspaceStrip({ const { workspaces, activeId } = useSyncExternalStore(subscribeToWorkspaces, getWorkspacesSnapshot); const membership = useSyncExternalStore(subscribeToWorkspaceSurfaces, getWorkspaceSurfacesSnapshot); const activity = useSyncExternalStore(subscribeToActivity, getActivitySnapshot); - const { renamingId, pendingClose, pendingMove, pendingSurfaceMove, moveError } = useSyncExternalStore(subscribeToWorkspaceUi, getWorkspaceUiSnapshot); + const { renamingId, confirmation, moveError } = useSyncExternalStore(subscribeToWorkspaceUi, getWorkspaceUiSnapshot); const [draggingId, setDraggingId] = useState(null); const stripRef = useRef(null); @@ -80,7 +78,7 @@ export function WorkspaceStrip({ // The editor and the confirmation both sit outside every Wall, so a // capture-phase command-mode shortcut would still fire behind them. - useDialogKeyboardOwner(renamingId !== null || pendingClose !== null || pendingMove !== null || pendingSurfaceMove !== null || moveError !== null, acquireChromeKeyboardLease); + useDialogKeyboardOwner(renamingId !== null || confirmation !== null || moveError !== null, acquireChromeKeyboardLease); const activate = useCallback((id: WorkspaceId) => { getWallHandle(id)?.enterCommandMode(); @@ -160,8 +158,8 @@ export function WorkspaceStrip({ // confirmation lands in the same place whichever Workspace it is about. No // Window (Storybook) leaves it viewport-centered. const confirmTarget = useMemo( - () => (pendingClose || pendingMove || pendingSurfaceMove || moveError ? document.querySelector('[data-workspace-content]') : null), - [pendingClose, pendingMove, pendingSurfaceMove, moveError], + () => (confirmation || moveError ? document.querySelector('[data-workspace-content]') : null), + [confirmation, moveError], ); // One union per tab, computed in the loop it is rendered in: every tab shows @@ -232,11 +230,10 @@ export function WorkspaceStrip({ >