From e052ff3f96bbd6f2ad3ab3a3cdbfaafc9080c4b4 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 07:02:13 -0700 Subject: [PATCH 01/16] Keep release recovery reachable and diagnose stalled storage Signed-off-by: Jeremy lewi --- app/src/components/ReleaseControls.tsx | 99 ++-------- .../ReleaseRecoveryControls.test.tsx | 100 ++++++++++ .../components/ReleaseRecoveryControls.tsx | 183 ++++++++++++++++++ .../components/StorageRecoveryBanner.test.tsx | 11 ++ app/src/components/StorageRecoveryBanner.tsx | 49 +++++ app/src/releases/controlChunks.ts | 8 +- app/src/releases/launcher.ts | 52 +++-- app/src/releases/releaseCatalog.test.ts | 34 ++++ app/src/releases/releaseCatalog.ts | 45 +++++ app/src/releases/releaseDatabase.test.ts | 62 ++++++ app/src/releases/releaseDatabase.ts | 87 +++++++-- app/src/releases/releaseDiagnostics.ts | 97 ++++++++++ app/src/releases/runtime.ts | 4 + app/src/releases/startupRecovery.test.ts | 13 ++ app/src/releases/startupRecovery.ts | 18 +- app/src/storage/storageOwner.test.ts | 88 ++++++++- app/src/storage/storageOwnerClient.ts | 5 + app/src/storage/storageOwnerDiagnostics.ts | 17 +- app/src/storage/storageOwnerHost.ts | 85 ++++++-- app/src/storage/storageRecovery.test.ts | 18 ++ app/src/storage/storageRecovery.ts | 19 +- docs-dev/CUJs/controlled-app-releases.md | 35 ++++ 22 files changed, 1000 insertions(+), 129 deletions(-) create mode 100644 app/src/components/ReleaseRecoveryControls.test.tsx create mode 100644 app/src/components/ReleaseRecoveryControls.tsx create mode 100644 app/src/releases/releaseCatalog.test.ts create mode 100644 app/src/releases/releaseCatalog.ts create mode 100644 app/src/releases/releaseDiagnostics.ts diff --git a/app/src/components/ReleaseControls.tsx b/app/src/components/ReleaseControls.tsx index 2ad81557..b9c7e326 100644 --- a/app/src/components/ReleaseControls.tsx +++ b/app/src/components/ReleaseControls.tsx @@ -1,3 +1,4 @@ +import { ReleaseRecoveryControls } from './ReleaseRecoveryControls' import { Button } from '@radix-ui/themes' import { useCallback, useEffect, useState } from 'react' @@ -6,7 +7,6 @@ import { type Control, type Manifest, type ReleaseProgress, - releaseIdValid, } from '../releases/control' import '../releases/runtime' import type { OfflineDownloadStatus } from '../releases/runtime' @@ -17,16 +17,25 @@ import type { OfflineDownloadStatus } from '../releases/runtime' * the common commands and can use the independent recovery page for rollback. */ export function ReleaseControls() { + return ( + <> + + + + ) +} + +/** Installed-state controls may load independently of recovery controls. */ +function InstalledReleaseControls() { const api = window.runmeVersions const [state, setState] = useState() const [message, setMessage] = useState('') const [progress, setProgress] = useState() const [offline, setOffline] = useState() const [error, setError] = useState('') + const [readError, setReadError] = useState('') const [busy, setBusy] = useState(false) - const [exact, setExact] = useState('') const [cached, setCached] = useState([]) - const [published, setPublished] = useState([]) // Polling also observes other tabs and older loaders without change events. // Ignore late responses after unmount so a closed workspace tab stays closed. @@ -41,6 +50,7 @@ export function ReleaseControls() { const next = await api.get() if (!disposed) { setState(next) + setReadError('') setProgress(api.progress?.()) setOffline( next.active && api.offline?.(next.active.manifest.releaseId) @@ -53,7 +63,7 @@ export function ReleaseControls() { ) } } catch (e) { - if (!disposed) setError(String(e)) + if (!disposed) setReadError(String(e)) } finally { reading = false } @@ -107,13 +117,16 @@ export function ReleaseControls() { if (!api) return

Release channels are available in published builds.

if (!state) - return

{error || 'Loading release information…'}

+ return ( +

+ {readError || error || 'Loading release information…'} +

+ ) const active = state.active?.manifest const candidate = state.candidate const hasUpdate = candidate && candidate.releaseId !== active?.releaseId const selection = state.selection.kind === 'channel' ? state.selection.name : 'Pinned release' - const recoveryUrl = '/recovery.html' + location.search + location.hash return (
)} - {error && ( + {(readError || error) && (

- {error} + {readError || error}

)}

@@ -271,68 +284,6 @@ export function ReleaseControls() { id="release-other-actions" className="mt-3 flex flex-col items-start gap-3" > - - - - {published.map((id) => ( - - ))} {api.activate && cached .filter((m) => m.releaseId !== active?.releaseId) @@ -351,14 +302,6 @@ export function ReleaseControls() { > Remove unused downloads - - Open recovery in another tab -

diff --git a/app/src/components/ReleaseRecoveryControls.test.tsx b/app/src/components/ReleaseRecoveryControls.test.tsx new file mode 100644 index 00000000..c379497d --- /dev/null +++ b/app/src/components/ReleaseRecoveryControls.test.tsx @@ -0,0 +1,100 @@ +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import { ReleaseControls } from './ReleaseControls' +import type { Control, Manifest } from '../releases/control' +const id = `${'a'.repeat(40)}-${'b'.repeat(64)}` +afterEach(() => { + cleanup() + delete window.runmeVersions + vi.unstubAllGlobals() +}) +/** A failed or forever-pending installed-state read must not gate rescue actions. */ +it.each(['failed', 'pending', 'missing'])( + 'keeps recovery reachable when settings are %s', + async (mode) => { + const prepareRecovery = vi.fn(async () => ({ releaseId: id }) as Manifest) + const recover = vi.fn(async () => {}) + if (mode !== 'missing') + window.runmeVersions = { + get: + mode === 'failed' + ? () => Promise.reject(new Error('read timeout (get)')) + : () => new Promise(() => {}), + check: vi.fn(), + upgrade: vi.fn(), + retry: vi.fn(), + cancel: vi.fn(), + prune: vi.fn(), + prepareRecovery, + recover, + } + render() + expect( + screen + .getByRole('link', { name: 'Open recovery in another tab' }) + .getAttribute('target') + ).toBe('_blank') + fireEvent.change( + screen.getByRole('textbox', { name: 'Commit SHA or release ID' }), + { target: { value: id } } + ) + fireEvent.click( + screen.getByRole('button', { name: 'Prepare selected release' }) + ) + if (mode === 'missing') { + await screen.findByText(/This launcher requires/) + expect(prepareRecovery).not.toHaveBeenCalled() + } else { + await screen.findByText(/downloaded and verified/) + expect(prepareRecovery).toHaveBeenCalledWith({ + kind: 'pinned', + releaseId: id, + }) + expect(recover).not.toHaveBeenCalled() + fireEvent.click( + screen.getByRole('button', { name: 'Activate selected release' }) + ) + await waitFor(() => + expect(recover).toHaveBeenCalledWith({ kind: 'pinned', releaseId: id }) + ) + } + expect( + screen + .getByRole('link', { name: 'Open recovery in another tab' }) + .getAttribute('href') + ).toContain(`release=${id}`) + } +) +it('keeps recovery navigation available during a hung download', async () => { + window.runmeVersions = { + get: () => new Promise(() => {}), + check: vi.fn(), + upgrade: vi.fn(), + retry: vi.fn(), + cancel: vi.fn(), + prune: vi.fn(), + prepareRecovery: () => new Promise(() => {}), + } + render() + fireEvent.change(screen.getByRole('textbox'), { target: { value: id } }) + fireEvent.click( + screen.getByRole('button', { name: 'Prepare selected release' }) + ) + await waitFor(() => + expect( + screen + .getByRole('link', { name: 'Open recovery in another tab' }) + .getAttribute('href') + ).toContain(id) + ) + fireEvent.click( + screen.getByRole('button', { name: 'Show release settings diagnostics' }) + ) + expect(screen.getByLabelText('Release settings diagnostics')).toBeTruthy() +}) diff --git a/app/src/components/ReleaseRecoveryControls.tsx b/app/src/components/ReleaseRecoveryControls.tsx new file mode 100644 index 00000000..4f1a6e28 --- /dev/null +++ b/app/src/components/ReleaseRecoveryControls.tsx @@ -0,0 +1,183 @@ +import { useState } from 'react' +import type { Manifest } from '../releases/control' +import { + publishedReleaseIds, + resolvePublishedRelease, +} from '../releases/releaseCatalog' +import { releaseDatabaseDiagnostics } from '../releases/releaseDiagnostics' +import '../releases/runtime' + +/** Recovery navigation and release discovery never wait for settings reads. + * Downloading is independent; activation still requires safe storage ownership. + */ +export function ReleaseRecoveryControls() { + const api = window.runmeVersions + const [input, setInput] = useState('') + const [releases, setReleases] = useState([]) + const [prepared, setPrepared] = useState() + const [selected, setSelected] = useState('') + const [busy, setBusy] = useState(false) + const [message, setMessage] = useState('') + const [diagnostics, setDiagnostics] = useState('') + const recoveryUrl = new URL('/recovery.html', window.location.href) + recoveryUrl.search = window.location.search + recoveryUrl.hash = window.location.hash + if (selected) { + recoveryUrl.searchParams.delete('channel') + recoveryUrl.searchParams.set('release', selected) + } + /** Keep failures local and leave navigation available during pending commands. */ + const run = async (action: () => Promise) => { + setBusy(true) + setMessage('') + try { + await action() + } catch (error) { + setMessage(String(error)) + } finally { + setBusy(false) + } + } + return ( +
+

Recovery and rollback

+

+ Choose a published commit or release, even when installed release + settings cannot load. Activation waits for tabs to preserve edits and + storage to finish pending writes. +

+ +
+ + + {prepared && api?.recover && ( + + )} + + Open recovery in another tab + + +
+ {releases.length > 0 && ( + + )} + {message &&

{message}

} + {diagnostics && ( + 
+          {diagnostics}
+
+ )} +
+ ) +} diff --git a/app/src/components/StorageRecoveryBanner.test.tsx b/app/src/components/StorageRecoveryBanner.test.tsx index add7f903..eca2e6ab 100644 --- a/app/src/components/StorageRecoveryBanner.test.tsx +++ b/app/src/components/StorageRecoveryBanner.test.tsx @@ -84,3 +84,14 @@ it('offers current editor downloads while storage is unavailable and restart is vi.unstubAllGlobals() vi.restoreAllMocks() }) + +it('allows diagnostics while restart is waiting and reports an unresponsive worker', async () => { + storageRecovery.set({ phase: 'preparing', message: 'Waiting for tabs' }) + const dispose = storageRecovery.registerDiagnostics(async () => { + throw new Error('No diagnostic reply') + }) + render() + fireEvent.click(screen.getByRole('button', { name: 'Check storage worker' })) + expect(await screen.findByText('Error: No diagnostic reply')).toBeTruthy() + dispose() +}) diff --git a/app/src/components/StorageRecoveryBanner.tsx b/app/src/components/StorageRecoveryBanner.tsx index 9183e787..e15a7d56 100644 --- a/app/src/components/StorageRecoveryBanner.tsx +++ b/app/src/components/StorageRecoveryBanner.tsx @@ -1,3 +1,4 @@ +import { appLogger } from '../lib/logging/runtime' import { useEffect, useState, useSyncExternalStore } from 'react' import { storageRecovery } from '../storage/storageRecovery' import { @@ -33,6 +34,8 @@ export default function StorageRecoveryBanner({ const [drafts, setDrafts] = useState([]) const [currentDrafts, setCurrentDrafts] = useState([]) const [captureMessage, setCaptureMessage] = useState('') + const [diagnostics, setDiagnostics] = useState('') + const [diagnosing, setDiagnosing] = useState(false) useEffect(() => { try { setDrafts(readRecoveryDrafts()) @@ -89,6 +92,52 @@ export default function StorageRecoveryBanner({ )} + {state.phase !== 'healthy' && ( +
+ + {diagnostics && ( + 
+              {diagnostics}
+
+ )} +

+ Diagnostics do not save or retry notebook operations. A reply + confirms the worker can process diagnostic messages, not that + storage writes are working. +

+ {state.phase === 'blocked' && ( +

+ If restart cannot finish, download current editor copies in every + Runme tab first. Then close all Runme tabs for this site and + reopen Runme. If the worker remains stuck, restart the browser + after saving the downloads. Do not clear site data. +

+ )} +
+ )} {captureMessage &&

{captureMessage}

} {currentDrafts.map((draft, index) => ( + +

+ Force restart skips draining pending work. Editor saves can retry; + other operations may have an uncertain outcome. +

)} {state.phase !== 'healthy' && ( diff --git a/app/src/components/StorageWorkerControls.tsx b/app/src/components/StorageWorkerControls.tsx new file mode 100644 index 00000000..896d419a --- /dev/null +++ b/app/src/components/StorageWorkerControls.tsx @@ -0,0 +1,117 @@ +import { useEffect, useRef, useState } from 'react' +import { storageRecovery } from '../storage/storageRecovery' +import { + StorageWorkerControl, + type WorkerStatus, +} from '../storage/storageWorkerControl' + +/** Recovery stays independent of the release-settings and notebook databases. */ +export function StorageWorkerControls() { + const [workers, setWorkers] = useState>(new Map()) + const management = useRef(undefined) + useEffect(() => { + const control = new StorageWorkerControl((event) => { + setWorkers((previous) => + new Map(previous).set(event.status.instanceId, event.status) + ) + }) + management.current = control + control.discover() + return () => { + management.current = undefined + control.close() + } + }, []) + return ( +
+

Storage workers

+

+ These are responding Runme workers for this site, not all browser + workers. Refresh to request current status. No reply means unknown + status. +

+ + + +

+ Restart keeps editors open. Force restart skips waiting for pending + work; retained editor saves retry after reconnecting. Other operations + may have an uncertain outcome. +

+ {[...workers.values()].map((worker) => ( +
+

+ {worker.releaseId ?? 'Development worker'} · epoch {worker.epoch} ·{' '} + {worker.phase} ({Math.round(worker.phaseElapsedMs / 1000)}s at last + reply) +

+

Instance: {worker.instanceId}

+ + +
+ Worker diagnostics + 
+              {JSON.stringify(worker, null, 2)}
+
+
+
+ ))} +

+ A force request still needs a responsive worker message loop. For a + completely unresponsive worker, use the browser’s worker inspection + tools (Chrome: chrome://inspect/#workers). Keep tabs open to retain + unsaved edits; download recovery copies before closing them. Do not + clear site data. +

+
+ ) +} diff --git a/app/src/lib/storageRecoveryDrafts.test.ts b/app/src/lib/storageRecoveryDrafts.test.ts index 6e03a76c..9931b78b 100644 --- a/app/src/lib/storageRecoveryDrafts.test.ts +++ b/app/src/lib/storageRecoveryDrafts.test.ts @@ -96,29 +96,20 @@ it('also blocks restart for running cells in read-only notebooks', async () => { expect(m.setReleasePending).not.toHaveBeenCalled() }) -it('cancels a hung flush promptly and does not unlock a later recovery round', async () => { +it('backs up without flushing and abort does not unlock a later recovery round', async () => { const m = model() - let finish!: () => void - m.flushPendingPersist.mockImplementationOnce( - () => - new Promise((resolve) => { - finish = resolve - }) - ) + m.flushPendingPersist.mockImplementation(() => new Promise(() => {})) const controller = new AbortController() - const preparation = prepareNotebookStorageRestart(controller.signal) - const rejected = expect(preparation).rejects.toThrow('Could not preserve') + const releaseOld = await prepareNotebookStorageRestart(controller.signal) expect(readRecoveryDrafts()).toHaveLength(1) + expect(m.flushPendingPersist).not.toHaveBeenCalled() controller.abort() expect(m.setReleasePending).toHaveBeenLastCalledWith(false) - await rejected const releaseNew = await prepareNotebookStorageRestart() expect(m.setReleasePending).toHaveBeenLastCalledWith(true) - const callsBeforeOldFlush = m.setReleasePending.mock.calls.length - finish() - await Promise.resolve() - await Promise.resolve() - expect(m.setReleasePending).toHaveBeenCalledTimes(callsBeforeOldFlush) + const calls = m.setReleasePending.mock.calls.length + releaseOld() + expect(m.setReleasePending).toHaveBeenCalledTimes(calls) releaseNew() expect(m.setReleasePending).toHaveBeenLastCalledWith(false) }) @@ -132,7 +123,7 @@ it.each(['[null]', '[{"uri":"local://file/a"}]', '{}'])( } ) -it('backs up every notebook before waiting for a hung first save', async () => { +it('backs up every notebook independently of a hung first save', async () => { const first = model() const second = model() second.getSnapshot = () => ({ @@ -149,14 +140,14 @@ it('backs up every notebook before waiting for a hung first save', async () => { ) first.flushPendingPersist.mockImplementation(() => new Promise(() => {})) const controller = new AbortController() - const preparing = prepareNotebookStorageRestart(controller.signal) - const rejected = expect(preparing).rejects.toThrow('Could not preserve') + const release = await prepareNotebookStorageRestart(controller.signal) + expect(first.flushPendingPersist).not.toHaveBeenCalled() expect(readRecoveryDrafts().map((draft) => draft.name)).toEqual([ 'draft.runme', 'second.runme', ]) controller.abort() - await rejected + release() expect(second.setReleasePending).toHaveBeenLastCalledWith(false) }) diff --git a/app/src/lib/storageRecoveryDrafts.ts b/app/src/lib/storageRecoveryDrafts.ts index 01663788..479f3646 100644 --- a/app/src/lib/storageRecoveryDrafts.ts +++ b/app/src/lib/storageRecoveryDrafts.ts @@ -99,16 +99,9 @@ export async function prepareNotebookStorageRestart( try { // Copy all notebooks before a stalled first flush can block the rest. backupNotebookRecoveryDrafts() - for (const model of models) { - signal.throwIfAborted() - if (model!.isReadOnly()) continue - try { - await waitForPersist(model!.flushPendingPersist(), signal) - } catch { - /* The recovery copy remains available after reload. */ - } - signal.throwIfAborted() - } + // Recovery must not wait on the persistence service being restarted. The + // worker drains admitted writes in graceful mode; snapshots cover unsaved edits. + signal.throwIfAborted() return release } catch (error) { release() @@ -117,21 +110,3 @@ export async function prepareNotebookStorageRestart( ) } } - -/** Cancel waiting, not the underlying commit. The owner still drains active writes. */ -async function waitForPersist( - persist: Promise, - signal: AbortSignal -): Promise { - let onAbort!: () => void - const aborted = new Promise((_resolve, reject) => { - onAbort = () => reject(signal.reason) - signal.addEventListener('abort', onAbort, { once: true }) - if (signal.aborted) onAbort() - }) - try { - await Promise.race([persist, aborted]) - } finally { - signal.removeEventListener('abort', onAbort) - } -} diff --git a/app/src/storage/editorSaveSession.test.ts b/app/src/storage/editorSaveSession.test.ts new file mode 100644 index 00000000..d8da22bc --- /dev/null +++ b/app/src/storage/editorSaveSession.test.ts @@ -0,0 +1,54 @@ +// @vitest-environment node +import { create } from '@bufbuild/protobuf' +import { expect, it, vi } from 'vitest' + +import { createInitialNotebookFile } from '../lib/notebookFormat' +import { parser_pb } from '../runme/client' +import { EditorSaveSession } from './editorSaveSession' + +it('captures the submitted snapshot while typing continues and retries identical operations after failure', async () => { + let acknowledge!: () => void + const commit = vi + .fn() + .mockImplementationOnce( + () => + new Promise((resolve) => { + acknowledge = resolve + }) + ) + .mockRejectedValueOnce(new Error('Worker exited')) + .mockResolvedValue(undefined) + const session = new EditorSaveSession( + 'local://file/test', + createInitialNotebookFile('test.runme'), + commit + ) + const editor = session.initialNotebook + editor.cells.push( + create(parser_pb.CellSchema, { + refId: 'one', + kind: parser_pb.CellKind.MARKUP, + value: 'submitted', + }) + ) + const first = session.save('local://file/test', editor) + await vi.waitFor(() => expect(commit).toHaveBeenCalledOnce()) + editor.cells[0].value = 'typed while waiting' + acknowledge() + await first + await expect(session.save('local://file/test', editor)).rejects.toThrow( + 'Worker exited' + ) + expect(session.hasPending).toBe(true) + const failed = structuredClone(commit.mock.calls[1]) + expect( + failed[2].some( + (op: any) => + op.kind === 'cell.update' && + op.payload.cell.value === 'typed while waiting' + ) + ).toBe(true) + await session.resume() + expect(commit.mock.calls[2]).toEqual(failed) + expect(session.hasPending).toBe(false) +}) diff --git a/app/src/storage/editorSaveSession.ts b/app/src/storage/editorSaveSession.ts new file mode 100644 index 00000000..268fc3ad --- /dev/null +++ b/app/src/storage/editorSaveSession.ts @@ -0,0 +1,113 @@ +import { + type RunmeOperation, + buildOperationLogDiff, + cloneNotebook, + highestActorSequence, + materializeOperationLog, + materializedLogToNotebook, + parseOperationLog, +} from '../lib/operationLog' +import { + captureCommittedRevision, + snapshotHeads, +} from '../lib/operationLog/versions' +import type { parser_pb } from '../runme/client' + +/** Page-owned save state survives replacing the storage connection. The worker + * only commits immutable batches; it never owns this editor's save baseline. + */ +export class EditorSaveSession { + readonly initialNotebook: parser_pb.Notebook + private readonly notebookId: string + private observed: RunmeOperation[] + private baseline: parser_pb.Notebook + private pending?: { + operations: RunmeOperation[] + snapshot: parser_pb.Notebook + } + private queue = Promise.resolve() + private saving = 0 + + /** Pending work blocks an app reload, but never prevents stopping the worker. */ + get hasPending(): boolean { + return Boolean(this.pending) || this.saving > 0 + } + + constructor( + private readonly uri: string, + document: string, + private readonly commit: ( + uri: string, + notebookId: string, + operations: RunmeOperation[] + ) => Promise, + private readonly actorId = `editor:${crypto.randomUUID()}` + ) { + const parsed = parseOperationLog(document) + this.notebookId = parsed.header.notebook_id + this.observed = parsed.operations + this.baseline = materializedLogToNotebook( + materializeOperationLog(this.observed) + ) + this.initialNotebook = cloneNotebook(this.baseline) + } + + /** Only acknowledged history participates in release handoff verification. */ + getObservedOperationHeads(): string[] { + return snapshotHeads(this.observed, captureCommittedRevision(this.observed)) + } + + /** Retry only the retained immutable batch, never regenerate an uncertain save. */ + private async commitPending(): Promise { + const pending = this.pending + if (!pending) return + await this.commit(this.uri, this.notebookId, pending.operations) + this.observed = [...this.observed, ...pending.operations] + this.baseline = pending.snapshot + this.pending = undefined + } + + /** Reconnect can finish a pending save without replacing the mounted model. */ + resume(): Promise { + const operation = this.queue.then(() => this.commitPending()) + this.queue = operation.catch(() => undefined) + return operation + } + + /** Capture at call time: keystrokes during an acknowledgement wait belong to + * the next save. Serializing also prevents overlapping actor sequence ranges. + */ + save(saveUri: string, notebook: parser_pb.Notebook): Promise { + if (saveUri !== this.uri) + return Promise.reject( + new Error( + `Operation-log save URI changed from ${this.uri} to ${saveUri}` + ) + ) + const next = cloneNotebook(notebook) + this.saving += 1 + const operation = this.queue + .then(async () => { + await this.commitPending() + const operations = await buildOperationLogDiff({ + previous: this.baseline, + next, + observedOperations: this.observed, + actorId: this.actorId, + firstActorSequence: + highestActorSequence(this.observed, this.actorId) + 1, + }) + if (!operations.length) { + this.baseline = next + return + } + this.pending = { operations, snapshot: next } + await this.commitPending() + }) + .finally(() => { + this.saving -= 1 + }) + this.queue = operation.catch(() => undefined) + return operation + } +} diff --git a/app/src/storage/local.test.ts b/app/src/storage/local.test.ts index 84ee5274..f6b17f9a 100644 --- a/app/src/storage/local.test.ts +++ b/app/src/storage/local.test.ts @@ -42,6 +42,7 @@ import { ancestorClosure } from '../lib/operationLog/versions' import { createNotebookCommentsRuntimeApi } from '../lib/runtime/notebookCommentsRuntime' import { MimeType, RunmeMetadataKey, parser_pb } from '../runme/client' import { MemoryConflictDocStorage } from './conflictDocs' +import { EditorSaveSession } from './editorSaveSession' import { DriveNotebookStore } from './drive' import type { DriveSyncCoordinator } from './driveSyncCoordinator' import { @@ -1107,6 +1108,53 @@ describe('LocalNotebooks operation-log storage', () => { ).toHaveLength(2) }) + it('retries the exact pending editor save after owner loss without duplicating or deleting unseen edits', async () => { + const storage = new MemoryOperationLogStorage() + const store = createTestStore({}, { operationLogStorage: storage }) + await store.folders.put({ id: LOCAL_FOLDER_URI, name: 'Local Notebooks', remoteId: '', children: [], lastSynced: '' }) + const created = await store.create(LOCAL_FOLDER_URI, 'resume.runme') + const seed = await store.createOperationLogSaveStore(created.uri, {actorId: 'seed'}) + await seed.save(created.uri, create(parser_pb.NotebookSchema, {cells: [create(parser_pb.CellSchema, { + refId: 'original', kind: parser_pb.CellKind.MARKUP, value: 'Original', + })]})) + let loseReply = true + const commit = vi.fn(async (...args: Parameters) => { + await store.commitEditorOperations(...args) + if (loseReply) throw new Error('Worker stopped before reply') + }) + const first = new EditorSaveSession(created.uri, await store.loadContent(created.uri), commit) + const acknowledged = first.getObservedOperationHeads() + const second = await store.createOperationLogSaveStore(created.uri, {actorId: 'other-editor'}) + const editor = first.initialNotebook + editor.cells[0].value = 'Saved, acknowledgement lost' + await expect(first.save(created.uri, editor)).rejects.toThrow('Worker stopped') + expect(first.getObservedOperationHeads()).toEqual(acknowledged) + const other = second.initialNotebook + other.cells.push(create(parser_pb.CellSchema, {refId: 'unseen', kind: parser_pb.CellKind.MARKUP, value: 'Other tab added this'})) + await second.save(created.uri, other) + const before = await store.loadContent(created.uri) + loseReply = false + await first.resume() + expect(commit.mock.calls[1]).toEqual(commit.mock.calls[0]) + expect(await store.loadContent(created.uri)).toBe(before) + expect(first.getObservedOperationHeads()).not.toEqual(acknowledged) + editor.cells[0].value = 'Edited after reconnect' + await first.save(created.uri, editor) + const result = await store.loadOperationLogSnapshot(created.uri) + expect(result.cells.map(cell => cell.value)).toEqual(['Edited after reconnect', 'Other tab added this']) + const saved = await store.loadContent(created.uri) + const [uri, notebookId, operations] = commit.mock.calls[0] + await expect(store.commitEditorOperations(uri, 'different-notebook', operations)).rejects.toThrow('identity changed') + const conflicting = structuredClone(operations) + conflicting[0].created_at = '2000-01-01T00:00:00Z' + await expect(store.commitEditorOperations(uri, notebookId, conflicting)).rejects.toThrow('Conflicting operations') + const missingHistory = structuredClone(operations) + missingHistory[0].op_id += '-new' + missingHistory[0].deps = ['missing-history'] + await expect(store.commitEditorOperations(uri, notebookId, missingHistory)).rejects.toThrow() + expect(await store.loadContent(created.uri)).toBe(saved) + }) + it('verifies acknowledged history across later commits and rejects missing commits before owner activation', async () => { const actor = vi.spyOn(actorIdentity, 'getNotebookActorId').mockResolvedValue('release_writer') const store = createTestStore({}, { operationLogStorage: new MemoryOperationLogStorage() }) diff --git a/app/src/storage/local.ts b/app/src/storage/local.ts index a000e405..68244df7 100644 --- a/app/src/storage/local.ts +++ b/app/src/storage/local.ts @@ -1,4 +1,3 @@ -import { registerReleaseDatabase } from '../releases/databases' import { create, fromJsonString, toJsonString } from '@bufbuild/protobuf' import Dexie, { Table } from 'dexie' import md5 from 'md5' @@ -89,6 +88,7 @@ import { } from '../lib/outputReference' import { appState } from '../lib/runtime/AppState' import type { ExampleJob } from '../lib/trainingExamples/protocol' +import { registerReleaseDatabase } from '../releases/databases' import { RunmeMetadataKey, parser_pb } from '../runme/client' import { type ConflictDocStorage, @@ -1924,7 +1924,8 @@ export class LocalNotebooks extends Dexie { ? await this.updateFolder(item.uri, item.name) : await this.driveSyncCoordinator.runExclusive( this.driveFolderMembershipLockKey(item.uri), - async () => (await this.ensureFolderMirror(item.uri, item.name)).folderId + async () => + (await this.ensureFolderMirror(item.uri, item.name)).folderId ) childUris.push(localFolderUri) } @@ -1981,7 +1982,10 @@ export class LocalNotebooks extends Dexie { return folderId } - async sync(localUri: string, options: { recursive?: boolean } = {}): Promise { + async sync( + localUri: string, + options: { recursive?: boolean } = {} + ): Promise { if (localUri.startsWith('drive-create:')) { const id = localUri.slice('drive-create:'.length) await this.queueDriveWork( @@ -2296,7 +2300,10 @@ export class LocalNotebooks extends Dexie { /** Create a tab-local snapshot adapter that appends .runme operations. */ async createOperationLogSaveStore( uri: string, - options: { actorId?: string; initialDocument?: string } = {} + options: { + actorId?: string + initialDocument?: string + } = {} ): Promise<{ save(saveUri: string, notebook: parser_pb.Notebook): Promise getObservedOperationHeads(): string[] @@ -2395,6 +2402,63 @@ export class LocalNotebooks extends Dexie { } } + /** Commit a tab-prepared batch by identity, including after a lost reply. + * The tab retains the exact bytes and causal baseline. Matching operations + * are acknowledged without duplication; conflicting IDs or missing history + * fail before writing. No editor identity is used to infer what it saw. + */ + async commitEditorOperations( + uri: string, + notebookId: string, + operations: RunmeOperation[] + ): Promise { + const record = await this.files.get(uri) + if ( + !record?.operationLogRef || + detectNotebookFileFormat(record.name) !== 'runme-operation-log' + ) + throw new Error(`Operation-log reference missing for ${uri}`) + const stored = await this.operationLogStorage.appendTransaction( + record.operationLogRef, + (document) => { + const current = parseOperationLog(document) + if (current.header.notebook_id !== notebookId) + throw new Error(`Operation-log notebook identity changed for ${uri}`) + // mergeOperationSets rejects different contents under the same ID. + const merged = mergeOperationSets(current.operations, operations) + const observed = ancestorClosure( + merged, + operations.map((op) => op.op_id) + ) + snapshotHeads( + observed, + observed.map((op) => op.op_id) + ) + const existing = new Set(current.operations.map((op) => op.op_id)) + const missing = operations.filter((op) => !existing.has(op.op_id)) + return missing.length + ? `${missing.map(encodeOperation).join('\n')}\n` + : '' + }, + { + prepareDocument: upgradeOperationLogToV2, + validate: (document) => void parseOperationLog(document), + } + ) + // Repeat metadata repair even when the OPFS append already succeeded. A + // previous worker may have stopped between these two durable writes. + await this.updateFile(uri, { + doc: '', + md5Checksum: this.localWriteChecksum(stored), + operationLogRef: stored.ref, + }) + this.notifySync(uri) + if (!record.conflict) { + this.enqueueSync(uri) + this.enqueueMarkdownSync(uri) + } + } + operationLogSupportsConcurrentWriters(): boolean { return this.operationLogStorage.supportsConcurrentWriters() } @@ -3300,21 +3364,28 @@ export class LocalNotebooks extends Dexie { * authoritative local history only: a remote refetch must not hide lost commits. * Newer operations may supersede these heads without invalidating their history. */ - async verifyReleaseCommits(refs: { uri: string; heads: string[] }[]): Promise { + async verifyReleaseCommits( + refs: { uri: string; heads: string[] }[] + ): Promise { const byUri = new Map>() for (const ref of refs) { if (!ref.heads.length) continue const heads = byUri.get(ref.uri) ?? new Set() - ref.heads.forEach(head => heads.add(head)) + ref.heads.forEach((head) => heads.add(head)) byUri.set(ref.uri, heads) } for (const [uri, heads] of byUri) { const record = await this.files.get(uri) - if (!record?.operationLogRef) throw new Error(`Confirmed notebook history is unavailable for ${uri}`) + if (!record?.operationLogRef) + throw new Error(`Confirmed notebook history is unavailable for ${uri}`) const stored = await this.operationLogStorage.read(record.operationLogRef) - const operations = new Set(parseOperationLog(stored.document).operations.map(op => op.op_id)) - if ([...heads].some(head => !operations.has(head))) - throw new Error(`Confirmed notebook commits are missing for ${uri}; preserve the existing data and use recovery`) + const operations = new Set( + parseOperationLog(stored.document).operations.map((op) => op.op_id) + ) + if ([...heads].some((head) => !operations.has(head))) + throw new Error( + `Confirmed notebook commits are missing for ${uri}; preserve the existing data and use recovery` + ) } } diff --git a/app/src/storage/storageOwner.test.ts b/app/src/storage/storageOwner.test.ts index 096c5330..49416904 100644 --- a/app/src/storage/storageOwner.test.ts +++ b/app/src/storage/storageOwner.test.ts @@ -601,48 +601,3 @@ it.each([1, 2])( expect(store.save).toHaveBeenCalledOnce() } ) - -it('answers diagnostics while restart is stuck acquiring release settings', async () => { - const controlModule = await import('../releases/control') - const { recoveryMessage } = await import('./storageRecoveryProtocol') - let rejectRead!: (error: Error) => void - const control = vi.spyOn(controlModule, 'control').mockImplementation( - () => - new Promise((_resolve, reject) => { - rejectRead = reject - }) - ) - const { store } = setup() - const shutdown = vi.fn() - const host = new StorageOwnerHost( - store as unknown as LocalNotebooks, - shutdown, - undefined, - { releaseId: 'test', epoch: 1, capabilities: [] } - ) - const channel = new MessageChannel() - ports.push(channel.port1 as unknown as MessagePort) - host.attach(channel.port1 as unknown as MessagePort) - const client = new StorageOwnerClient( - channel.port2 as unknown as MessagePort, - async () => 'token' - ) - clients.push(client) - try { - channel.port2.postMessage(recoveryMessage('restart', 'restart-round')) - await vi.waitFor(() => expect(control).toHaveBeenCalledOnce()) - const snapshot = await client.diagnose() - expect(snapshot.restart).toMatchObject({ - stage: 'release-settings', - readyTabs: 0, - }) - expect(shutdown).not.toHaveBeenCalled() - rejectRead(new Error('Release settings read timeout (get)')) - await vi.waitFor(async () => - expect((await client.diagnose()).restart?.stage).toBe('failed') - ) - expect(shutdown).not.toHaveBeenCalled() - } finally { - control.mockRestore() - } -}) diff --git a/app/src/storage/storageOwner.worker.ts b/app/src/storage/storageOwner.worker.ts index 8b6e719f..2d68d2ca 100644 --- a/app/src/storage/storageOwner.worker.ts +++ b/app/src/storage/storageOwner.worker.ts @@ -1,6 +1,7 @@ import { appLogger } from '../lib/logging/runtime' import { OWNER_LOCK, control, readControl } from '../releases/control' import { retireUnreadableOwner } from './releaseOwnerFailure' +import { installStorageWorkerControl } from './storageWorkerControl' const scope = globalThis as unknown as { onconnect: (event: MessageEvent) => void @@ -9,9 +10,55 @@ const scope = globalThis as unknown as { const releaseId = import.meta.env.VITE_RUNME_RELEASE_ID as string | undefined // CacheStorage responses retain their original URL, which can drop a query. // The browser-assigned worker name is the authoritative instance identity. -const epoch = Number( - (globalThis as unknown as { name: string }).name.split('-').pop() -) +const epoch = releaseId + ? Number((globalThis as unknown as { name: string }).name.split('-').pop()) + : 0 +const instanceId = crypto.randomUUID() +const startedAt = Date.now() +let phase = 'waiting-for-lock' +let phaseStartedAt = startedAt +let runtime: + | Awaited< + ReturnType<(typeof import('./storageOwnerRuntime'))['startStorageOwner']> + > + | undefined +let stopping = false +/** Track the last reached boundary even while an awaited operation is stuck. */ +function setPhase(next: string) { + phase = next + phaseStartedAt = Date.now() + appLogger.info('Storage worker lifecycle', { + attrs: { scope: 'storage.owner', instanceId, phase, releaseId, epoch }, + }) +} +installStorageWorkerControl({ + status: () => ({ + instanceId, + releaseId, + epoch, + phase, + phaseElapsedMs: Date.now() - phaseStartedAt, + uptimeMs: Date.now() - startedAt, + diagnostics: runtime?.diagnostics(), + }), + phase: setPhase, + drain: async () => { + stopping = true + // Before a runtime is available there are no admitted notebook requests. + // Startup may nevertheless be opening stores: wait for it to settle before + // graceful close. Force-close remains available on this independent channel. + if (!runtime) await runtimeReady + await runtime?.drain() + }, + close: () => { + stopping = true + scope.close() + }, +}) +let finishRuntime!: () => void +const runtimeReady = new Promise((resolve) => { + finishRuntime = resolve +}) const ports: MessagePort[] = [] let attach: ((port: MessagePort) => void) | undefined scope.onconnect = (event) => { @@ -29,6 +76,7 @@ async function start(): Promise { if (!navigator.locks) throw new Error('Web Locks are required for storage ownership') await navigator.locks.request(OWNER_LOCK, async () => { + setPhase('reading-release-settings') if (releaseId) { const state = await readControl() const pending = state.transition @@ -46,14 +94,21 @@ async function start(): Promise { ) throw new Error('Storage candidate is no longer selected') } + setPhase('recording-startup') if (releaseId) await control((s) => { s.ownerStartup = { releaseId, epoch, phase: 'initializing' } }) + setPhase('importing-runtime') const { startStorageOwner } = await import('./storageOwnerRuntime') - const runtime = await startStorageOwner( + setPhase('initializing-storage') + runtime = await startStorageOwner( releaseId ? { releaseId, epoch } : undefined ) + finishRuntime() + if (stopping) await new Promise(() => {}) + const activeRuntime = runtime + setPhase('publishing-ready') try { if (releaseId) await control((state) => { @@ -79,11 +134,12 @@ async function start(): Promise { }) } catch (error) { // If startup lost admission, finish maintenance before releasing ownership. - await runtime.drain() - runtime.close() + await activeRuntime.drain() + activeRuntime.close() throw error } - attach = runtime.attach + setPhase('ready') + attach = activeRuntime.attach for (const port of ports.splice(0)) attach(port) await new Promise((resolve) => { let checking = false @@ -96,9 +152,9 @@ async function start(): Promise { state.transition?.phase === 'handoff' || state.active?.epoch !== epoch ) { - await runtime.drain() + await activeRuntime.drain() clearInterval(timer) - runtime.close() + activeRuntime.close() resolve() } } catch (error) { @@ -106,7 +162,7 @@ async function start(): Promise { // lock and retry reconciliation; never release it on a timeout/error. try { if ( - await retireUnreadableOwner(error, runtime, () => { + await retireUnreadableOwner(error, activeRuntime, () => { clearInterval(timer) resolve() }) @@ -135,6 +191,8 @@ async function start(): Promise { scope.close() } void start().catch(async (error) => { + finishRuntime() + setPhase('failed') await control((state) => { if ( releaseId && diff --git a/app/src/storage/storageOwnerClient.ts b/app/src/storage/storageOwnerClient.ts index 16a8c186..983419a0 100644 --- a/app/src/storage/storageOwnerClient.ts +++ b/app/src/storage/storageOwnerClient.ts @@ -1,9 +1,14 @@ import { getWorkloadIdentityConfig } from '../auth/workloadIdentity' import { getGoogleDriveBaseUrl } from '../lib/googleDriveRuntime' import { appLogger } from '../lib/logging/runtime' -import { RELEASE_CAPABILITIES, releasePrefix } from '../releases/control' +import { + OWNER_LOCK, + RELEASE_CAPABILITIES, + releasePrefix, +} from '../releases/control' import { registerUpgradeBlocker } from '../releases/runtime' import { DriveCreateNotCommittedError, type DriveNotebookStore } from './drive' +import { EditorSaveSession } from './editorSaveSession' import { FilesystemEntryAlreadyExistsError } from './fs' import { readLegacyCreationJournal } from './legacyCreationJournal' import LocalNotebooks, { @@ -14,9 +19,14 @@ import type { StorageOwnerDiagnostics } from './storageOwnerDiagnostics' import { STORAGE_METHODS, STORAGE_OWNER_VERSION } from './storageOwnerProtocol' import { requiresStorageReload, storageRecovery } from './storageRecovery' import { StorageRecoveryClient } from './storageRecoveryClient' -import type { RecoveryMessage } from './storageRecoveryProtocol' +import { + type RecoveryMessage, + recoveryMessage, +} from './storageRecoveryProtocol' +import { StorageWorkerControl, type WorkerStatus } from './storageWorkerControl' type Pending = { + method: string resolve: (value: unknown) => void reject: (error: Error) => void timer: ReturnType @@ -50,6 +60,14 @@ export class StorageOwnerClient { private readonly getCredentialBoundary: () => string = () => 'default' ) { port.onmessage = (event) => { + if (event.data?.type === 'release-owner-error') { + this.close( + new Error( + `Notebook storage worker failed: ${String(event.data.error)}` + ) + ) + return + } if (event.data?.type === 'storage-owner-diagnostics') { const diagnostic = this.diagnostic if (diagnostic && diagnostic.id === event.data.id) { @@ -68,7 +86,7 @@ export class StorageOwnerClient { port.onmessageerror = () => this.close( new Error( - 'Storage worker message failed. Local saves may have committed; reload to recover.' + 'Storage worker message failed. Local saves may have committed; reconnect storage to recover.' ) ) port.start() @@ -107,10 +125,17 @@ export class StorageOwnerClient { return promise } + /** A failed transport can be replaced without discarding tab-owned saves. */ + get disconnected(): boolean { + return this.closed + } + request(method: string, args: unknown[] = []): Promise { if ( (this.uncertain.size || this.unresolvedCommit) && - !['disconnect', 'heartbeat', 'hello'].includes(method) + !['disconnect', 'heartbeat', 'hello', 'commitEditorOperations'].includes( + method + ) ) return Promise.reject( new Error( @@ -120,7 +145,7 @@ export class StorageOwnerClient { if (this.closed) return Promise.reject( new Error( - 'Storage worker disconnected. Reload to recover saved content.' + 'Storage worker disconnected. Reconnect to resume pending editor saves.' ) ) const id = crypto.randomUUID() @@ -136,6 +161,7 @@ export class StorageOwnerClient { 'heartbeat', 'disconnect', 'getDriveQueueMetrics', + 'commitEditorOperations', ].includes(method) ) this.uncertain.add(id) @@ -143,7 +169,9 @@ export class StorageOwnerClient { const error = new Error( method === 'getDriveQueueMetrics' ? 'Storage worker did not respond to queue diagnostics.' - : 'Storage worker did not acknowledge the request. Its outcome is uncertain; do not repeat creation blindly.' + : method === 'commitEditorOperations' + ? 'Storage worker did not acknowledge an editor save. Pending operations remain in this tab; reconnect storage to retry them.' + : 'Storage worker did not acknowledge the request. Its outcome is uncertain; do not repeat creation blindly.' ) appLogger.warn('Storage owner request timed out', { attrs: { @@ -177,7 +205,7 @@ export class StorageOwnerClient { ? 10_000 : 300_000 ) - this.pending.set(id, { resolve, reject, timer }) + this.pending.set(id, { method, resolve, reject, timer }) try { this.port.postMessage({ type: 'request', @@ -287,7 +315,19 @@ export class StorageOwnerClient { // Losing the transport does not settle an outstanding effect. Keep the // upgrade blocker until document reload/recovery; never turn disconnect // into an acknowledgement of an uncertain save or create. - if (this.pending.size || this.uncertain.size || this.unresolvedCommit) + if ( + [...this.pending.values()].some( + (p) => + ![ + 'hello', + 'heartbeat', + 'disconnect', + 'commitEditorOperations', + ].includes(p.method) + ) || + this.uncertain.size || + this.unresolvedCommit + ) this.unresolvedCommit = true else this.unregisterReleaseBlocker() if (this.diagnostic) { @@ -313,99 +353,282 @@ export function createSharedNotebookStore( drive: DriveNotebookStore ): LocalNotebooks { const release = window.runmeRelease?.active - const worker = release - ? new SharedWorker( - `${releasePrefix(release.manifest.releaseId)}${release.manifest.owner.entry}`, - { + const listeners = new Map void>>() + /** Preserve subscriptions when a new transport replaces a stopped worker. */ + function notifyChange(uri: string) { + for (const callback of listeners.get(uri) ?? []) callback() + window.dispatchEvent( + new CustomEvent('local-notebook-sync-updated', { detail: { uri } }) + ) + window.dispatchEvent( + new CustomEvent('local-notebook-updated', { detail: { uri } }) + ) + } + /** A fresh handshake is mandatory for every replacement owner. */ + function connect() { + let ownerInstance: string | undefined + const worker = release + ? new SharedWorker( + `${releasePrefix(release.manifest.releaseId)}${release.manifest.owner.entry}`, + { + type: 'module', + name: `runme-storage-owner-${release.manifest.releaseId}-${release.epoch}`, + } + ) + : new SharedWorker(new URL('./storageOwner.worker.ts', import.meta.url), { type: 'module', - name: `runme-storage-owner-${release.manifest.releaseId}-${release.epoch}`, - } + name: 'runme-storage-owner', + }) + const client = new StorageOwnerClient( + worker.port, + (options) => drive.getAccessToken(options), + credentialBoundary + ) + client.onFailure = (error) => { + if (requiresStorageReload(error)) storageRecovery.report(error) + } + worker.port.postMessage(recoveryMessage('register', 'editor-client')) + // Migration is part of the handshake: do not let a recovery scan generate a + // second remote identity while an old reserved ID only exists in localStorage. + const ready = Promise.resolve().then(async () => { + let attempts: ReturnType = [] + let creationMigrationError: string | undefined + try { + attempts = readLegacyCreationJournal(localStorage) + } catch (error) { + creationMigrationError = String(error) + } + const hello = await client.request('hello', [ + { + driveBaseUrl: getGoogleDriveBaseUrl(), + credentialBoundary: credentialBoundary(), + profileCredentialBoundary: localStorage.getItem( + 'runme/storageCredentialBoundary' + ), + attempts, + creationMigrationError, + }, + ]) + const handshake = hello as { + editorCommitVersion?: number + version: number + instanceId?: string + release?: { releaseId: string; epoch: number; capabilities: string[] } + } + if ( + release && + (handshake.version !== STORAGE_OWNER_VERSION || + !handshake.instanceId || + handshake.release?.releaseId !== release.manifest.releaseId || + handshake.release.epoch !== release.epoch || + RELEASE_CAPABILITIES.some( + (c) => !handshake.release?.capabilities?.includes(c) + )) ) - : new SharedWorker(new URL('./storageOwner.worker.ts', import.meta.url), { - type: 'module', - name: 'runme-storage-owner', + throw new Error( + 'Storage owner identity or lifecycle capabilities do not match this tab' + ) + for (const record of attempts) { + // Cleanup is optional after durable import. Failure cannot disconnect an + // otherwise healthy editor or discard its notebook persistence channel. + try { + if ( + localStorage.getItem(record.id) === JSON.stringify(record.attempt) + ) + localStorage.removeItem(record.id) + } catch { + /* A later handshake can safely repeat the import. */ + } + } + ownerInstance = handshake.instanceId + return handshake + }) + void ready.catch((error) => + appLogger.error('Notebook storage worker unavailable', { + attrs: { scope: 'storage.owner', error: String(error) }, }) - const client = new StorageOwnerClient( - worker.port, - (options) => drive.getAccessToken(options), - credentialBoundary - ) - client.onFailure = (error) => { - if (requiresStorageReload(error)) storageRecovery.report(error) - } - client.recovery = new StorageRecoveryClient( - (message) => worker.port.postMessage(message), - () => { - // This is an acknowledged shutdown, not worker failure. Keep editors frozen - // until navigation completes so no last keystroke escapes the recovery copy. - client.onFailure = undefined - client.recovery = undefined - client.close() - window.location.reload() - } - ) - storageRecovery.registerRestart(() => client.recovery!.restart()) - // Migration is part of the handshake: do not let a recovery scan generate a - // second remote identity while an old reserved ID only exists in localStorage. - const ready = Promise.resolve().then(async () => { - let attempts: ReturnType = [] - let creationMigrationError: string | undefined - try { - attempts = readLegacyCreationJournal(localStorage) - } catch (error) { - creationMigrationError = String(error) + ) + client.onChange = notifyChange + worker.onerror = (event) => { + if (connection.client === client) needsConnection = true + client.close( + new Error( + `Notebook storage worker failed: ${event.message || 'startup error'} (${event.filename}:${event.lineno}). Reconnect storage to resume saving.` + ) + ) } - const hello = await client.request('hello', [ - { - driveBaseUrl: getGoogleDriveBaseUrl(), - credentialBoundary: credentialBoundary(), - profileCredentialBoundary: localStorage.getItem( - 'runme/storageCredentialBoundary' - ), - attempts, - creationMigrationError, + return { + worker, + client, + ready, + get instanceId() { + return ownerInstance }, - ]) - const handshake = hello as { - version: number - instanceId?: string - release?: { releaseId: string; epoch: number; capabilities: string[] } } + } + let connection = connect() + let needsConnection = false + let connecting: Promise | undefined + let retrying: Promise | undefined + let restartRequested = false + let driveAvailable = false + let currentWorker: WorkerStatus | undefined + let lastManagementReply = 0 + const sessions = new Map< + EditorSaveSession, + { disposed: boolean; unregister: () => void } + >() + const management = new StorageWorkerControl((event) => { if ( - release && - (handshake.version !== STORAGE_OWNER_VERSION || - !handshake.instanceId || - handshake.release?.releaseId !== release.manifest.releaseId || - handshake.release.epoch !== release.epoch || - RELEASE_CAPABILITIES.some( - (c) => !handshake.release?.capabilities?.includes(c) - )) + event.status.releaseId !== release?.manifest.releaseId || + event.status.epoch !== (release?.epoch ?? 0) ) - throw new Error( - 'Storage owner identity or lifecycle capabilities do not match this tab' + return + currentWorker = event.status + lastManagementReply = Date.now() + // Status replies are observations only. No tab tells another tab to connect. + if (event.type === 'stopping') + storageRecovery.set({ + phase: 'restarting', + message: + 'Storage is stopping. Editors and pending saves remain in this tab.', + }) + if (event.type === 'stopped') { + connection.client.onFailure = undefined + connection.client.close( + new Error('Storage worker stopped; pending editor saves retained') ) - for (const record of attempts) { - // Cleanup is optional after durable import. Failure cannot disconnect an - // otherwise healthy editor or discard its notebook persistence channel. + needsConnection = true + storageRecovery.set({ + phase: 'required', + message: + 'Storage stopped. Retry a save or restart storage to reconnect; editors remain open.', + }) + if ( + event.restart && + (restartRequested || + [...sessions.keys()].some((session) => session.hasPending)) + ) + void retryPending() + restartRequested = false + } + }) + /** A tab reconnects lazily on its own next request. The browser joins the same + * named SharedWorker; the worker's lifetime lock remains the only writer gate. + */ + async function ensureConnected(): Promise { + if (connection.client.disconnected) needsConnection = true + if (connecting) return connecting + if (!needsConnection) { + await connection.ready + return + } + const operation = (async () => { + connection = connect() + const handshake = await connection.ready + if (handshake.editorCommitVersion !== 1) + throw new Error( + 'This worker cannot resume pending editor saves. Keep this tab open and upgrade Runme.' + ) + needsConnection = false + if (driveAvailable) + await connection.client.request('availability', [ + true, + credentialBoundary(), + ]) + })() + connecting = operation + try { + await operation + } catch (error) { + needsConnection = true + throw error + } finally { + connecting = undefined + } + } + /** Retry only this tab's retained idempotent batches, independently of peers. */ + async function retryPending(): Promise { + if (retrying) return retrying + const operation = (async () => { try { - if (localStorage.getItem(record.id) === JSON.stringify(record.attempt)) - localStorage.removeItem(record.id) - } catch { - /* A later handshake can safely repeat the import. */ + await ensureConnected() + const results = await Promise.allSettled( + [...sessions.keys()].map((session) => session.resume()) + ) + for (const [session, state] of sessions) { + if (state.disposed && !session.hasPending) { + state.unregister() + sessions.delete(session) + } + } + const failed = results.filter((result) => result.status === 'rejected') + storageRecovery.set( + failed.length + ? { + phase: 'blocked', + message: `${failed.length} notebook save(s) still need recovery. Editors and pending batches remain in this tab.`, + } + : { phase: 'healthy', message: '' } + ) + management.discover() + } catch (error) { + storageRecovery.set({ + phase: 'blocked', + message: `Storage reconnect failed: ${String(error)}. Editors and pending saves remain in this tab.`, + }) } + })() + retrying = operation + try { + await operation + } finally { + retrying = undefined } - }) - void ready.catch((error) => - appLogger.error('Notebook storage worker unavailable', { - attrs: { scope: 'storage.owner', error: String(error) }, - }) - ) - worker.onerror = (event) => - client.close( - new Error( - `Notebook storage worker failed: ${event.message || 'startup error'} (${event.filename}:${event.lineno}). Reload to recover durable edits.` + } + storageRecovery.registerRestart((force = false) => { + const requestedAt = Date.now() + const sendStop = async () => { + if ( + currentWorker && + lastManagementReply >= requestedAt && + currentWorker.phase !== 'closing' + ) { + restartRequested = true + management.stop(currentWorker.instanceId, { force, restart: true }) + return + } + // Silence alone is not death. A free ownership lock permits reconnecting + // after a crash without a stopped event; never steal an occupied lock. + const absent = await navigator.locks.request( + OWNER_LOCK, + { mode: 'shared', ifAvailable: true }, + (lock) => Boolean(lock) ) + if (absent) { + connection.client.onFailure = undefined + connection.client.close() + needsConnection = true + void retryPending() + } else + storageRecovery.set({ + phase: 'blocked', + message: + 'No management reply and storage ownership is still held. Keep editors open and use browser worker inspection.', + }) + } + management.discover() + setTimeout( + () => + void sendStop().catch((error) => + storageRecovery.report( + error instanceof Error ? error : new Error(String(error)) + ) + ), + 250 ) + }) + management.discover() const local = new LocalNotebooks( drive, undefined, @@ -416,43 +639,36 @@ export function createSharedNotebookStore( undefined, { client: true } ) - const listeners = new Map void>>() - client.onChange = (uri) => { - for (const callback of listeners.get(uri) ?? []) callback() - window.dispatchEvent( - new CustomEvent('local-notebook-sync-updated', { detail: { uri } }) - ) - window.dispatchEvent( - new CustomEvent('local-notebook-updated', { detail: { uri } }) - ) + const call = async (method: string, args: unknown[]): Promise => { + const reconnect = needsConnection + await ensureConnected() + if (reconnect) void retryPending() + return connection.client.request(method, args) } - const call = async (method: string, args: unknown[]) => { - await ready - return client.request(method, args) - } - const heartbeat = setInterval( - () => void call('heartbeat', []).catch(() => {}), - 15_000 - ) + const heartbeat = setInterval(() => { + if (!needsConnection) void call('heartbeat', []).catch(() => {}) + }, 15_000) window.addEventListener( 'pagehide', (event) => { if (event.persisted) return clearInterval(heartbeat) - void client.request('disconnect').catch(() => {}) - client.close() + void connection.client.request('disconnect').catch(() => {}) + connection.client.close() + management.close() } // A BFCache round trip must not remove the eventual disconnect handler. ) const methods = new Set(STORAGE_METHODS) return new Proxy(local, { get(target, key) { - if (key === 'ready') return ready + if (key === 'ready') return connection.ready if (key === 'setFilesystemStore' || key === 'stopSyncQueue') return () => {} if (key === 'operationLogSupportsConcurrentWriters') return () => true if (key === 'setDriveSyncAvailable') return (available: boolean) => { + driveAvailable = available const boundary = credentialBoundary() const saved = localStorage.getItem('runme/storageCredentialBoundary') // Existing queued edits belong to this profile's identity. A different @@ -484,7 +700,52 @@ export function createSharedNotebookStore( } } if (key === 'createOperationLogSaveStore') - return async (uri: string, options: unknown) => { + return async ( + uri: string, + options: Parameters< + LocalNotebooks['createOperationLogSaveStore'] + >[1] = {} + ) => { + const handshake = await connection.ready + if (handshake.editorCommitVersion === 1) { + const document = + options.initialDocument ?? + ((await call('loadContent', [uri])) as string) + const session = new EditorSaveSession( + uri, + document, + async (...args) => { + await call('commitEditorOperations', args) + }, + options.actorId + ) + const state = { + disposed: false, + unregister: registerUpgradeBlocker( + () => + session.hasPending + ? 'A notebook has pending editor saves; reconnect storage before upgrading' + : undefined, + true + ), + } + sessions.set(session, state) + return { + initialNotebook: session.initialNotebook, + getObservedOperationHeads: () => + session.getObservedOperationHeads(), + save: session.save.bind(session), + dispose: () => { + state.disposed = true + if (!session.hasPending) { + state.unregister() + sessions.delete(session) + } + }, + } + } + // Older releases still support their original view protocol. Never + // assume they understand immutable editor commits merely from version 8. const view = (await call('createView', [uri, options])) as { id: string heads: string[] diff --git a/app/src/storage/storageOwnerDiagnostics.ts b/app/src/storage/storageOwnerDiagnostics.ts index 700eb201..693ce1b6 100644 --- a/app/src/storage/storageOwnerDiagnostics.ts +++ b/app/src/storage/storageOwnerDiagnostics.ts @@ -7,6 +7,7 @@ export type StorageOwnerDiagnostics = { instanceId: string uptimeMs: number connectedTabs: number + connectedPorts?: number initialization: 'pending' | 'ready' | 'failed' pendingCredentials: number scanning: boolean diff --git a/app/src/storage/storageOwnerHost.ts b/app/src/storage/storageOwnerHost.ts index e2d8d491..a1973128 100644 --- a/app/src/storage/storageOwnerHost.ts +++ b/app/src/storage/storageOwnerHost.ts @@ -1,6 +1,5 @@ import { setGoogleDriveBaseUrl } from '../lib/googleDriveRuntime' import { appLogger } from '../lib/logging/runtime' -import { control } from '../releases/control' import { releaseDatabaseDiagnostics } from '../releases/releaseDiagnostics' import type { parser_pb } from '../runme/client' import { @@ -25,7 +24,12 @@ import { SyncDeferred } from './syncWorkQueue' export class StorageOwnerHost { private ports = new Map< MessagePort, - { available: boolean; lastSeen: number; credentialBoundary: string } + { + available: boolean + lastSeen: number + credentialBoundary: string + editor: boolean + } >() private tokenRequests = new Map< string, @@ -99,7 +103,7 @@ export class StorageOwnerHost { private releaseDraining = false /** A bounded memory-only snapshot still answers while IndexedDB or an RPC is stalled. */ - private diagnostics(): StorageOwnerDiagnostics { + diagnostics(): StorageOwnerDiagnostics { // Stop iterating at the response limit: a stalled owner may have many // outstanding requests, and diagnosis must not copy the whole backlog. const oldestRequests: StorageOwnerDiagnostics['oldestRequests'] = [] @@ -117,7 +121,8 @@ export class StorageOwnerHost { release: this.release, instanceId: this.instanceId, uptimeMs: Date.now() - this.startedAt, - connectedTabs: this.ports.size, + connectedTabs: this.editorPorts().length, + connectedPorts: this.ports.size, initialization: this.initialization, pendingCredentials: this.tokenRequests.size, scanning: Boolean(this.scanning), @@ -133,7 +138,7 @@ export class StorageOwnerHost { stage: this.recoveryStage, elapsedMs: Date.now() - this.recoveryStageStartedAt, readyTabs: this.recovery?.ready.size ?? 0, - connectedTabs: this.ports.size, + connectedTabs: this.editorPorts().length, inFlight: this.inFlight.size, }, } @@ -144,6 +149,7 @@ export class StorageOwnerHost { available: false, lastSeen: Date.now(), credentialBoundary: 'default', + editor: false, }) this.views.set(port, new Map()) port.onmessage = (event) => { @@ -179,6 +185,19 @@ export class StorageOwnerHost { } port.onmessageerror = () => this.detach(port) port.start() + } + + /** Only application connections own editors; passive launcher ports do not vote. */ + private editorPorts(): MessagePort[] { + return [...this.ports] + .filter(([, state]) => state.editor) + .map(([port]) => port) + } + + private registerEditor(port: MessagePort): void { + const state = this.ports.get(port) + if (!state || state.editor) return + state.editor = true if (this.recovery) { port.postMessage(recoveryMessage('prepare', this.recovery.id)) this.armRecoveryTimeout() @@ -192,19 +211,12 @@ export class StorageOwnerHost { ): Promise { if (this.releaseDraining) return if (!this.ports.has(port) || message.protocol !== 1) return + if (message.action === 'register') { + this.registerEditor(port) + return + } if (message.action === 'restart') { - this.markRecovery('release-settings') - if (this.release) - await control((s) => { - if (s.transition || s.active?.epoch !== this.release!.epoch) - throw new Error( - 'A release transition is pending; use Versions recovery' - ) - s.emergencyRestart = { - epoch: this.release!.epoch, - id: this.recovery?.id ?? message.id, - } - }) + this.registerEditor(port) if (this.recovery && !this.recovery.draining) { port.postMessage(recoveryMessage('prepare', this.recovery.id)) return @@ -228,7 +240,7 @@ export class StorageOwnerHost { this.recovery = round this.markRecovery('waiting-for-tab-backups') this.armRecoveryTimeout() - for (const client of this.ports.keys()) + for (const client of this.editorPorts()) client.postMessage(recoveryMessage('prepare', round.id)) } else if (message.action === 'ready' && message.id === this.recovery?.id) { if (message.error) { @@ -267,26 +279,18 @@ export class StorageOwnerHost { // gather new editor acknowledgements after a late tab fixes its backup error. if (!round.draining) { this.recovery = undefined - if (this.release) - void control((s) => { - if ( - s.emergencyRestart?.epoch === this.release!.epoch && - s.emergencyRestart.id === round.id - ) - delete s.emergencyRestart - }).catch(() => {}) } else round.ready.clear() - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage(recoveryMessage('blocked', round.id, error)) } /** No timer may force termination while a storage or network commit is still active. */ private async finishRecovery(): Promise { const round = this.recovery - if (!round || [...this.ports.keys()].some((port) => !round.ready.has(port))) + if (!round || this.editorPorts().some((port) => !round.ready.has(port))) return clearTimeout(round.timer) - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage(recoveryMessage('draining', round.id)) if (!round.draining) { round.draining = true @@ -307,17 +311,17 @@ export class StorageOwnerHost { // A tab attached during draining must preserve its editor too. if ( this.recovery !== round || - [...this.ports.keys()].some((port) => !round.ready.has(port)) + this.editorPorts().some((port) => !round.ready.has(port)) ) return this.recovery = undefined - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage(recoveryMessage('reload', round.id)) this.markRecovery('shutdown') this.shutdown?.() } catch { // Remain quiescent rather than start a second owner after an uncertain drain. - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage( recoveryMessage( 'blocked', @@ -414,12 +418,12 @@ export class StorageOwnerHost { } event(type: string, detail: unknown): void { - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage({ type: 'event', eventType: type, detail }) } notify(uri: string): void { - for (const port of this.ports.keys()) + for (const port of this.editorPorts()) port.postMessage({ type: 'changed', uri }) } @@ -445,6 +449,7 @@ export class StorageOwnerHost { return } if (request.type !== 'request' || typeof request.id !== 'string') return + if (request.method !== 'disconnect') this.registerEditor(port) // Closing a stale tab must release its port even when its data protocol differs. if (request.method === 'disconnect') { this.detach(port) @@ -569,6 +574,7 @@ export class StorageOwnerHost { setGoogleDriveBaseUrl(baseUrl) value = { version: STORAGE_OWNER_VERSION, + editorCommitVersion: 1, instanceId: this.instanceId, release: this.release, } diff --git a/app/src/storage/storageOwnerProtocol.ts b/app/src/storage/storageOwnerProtocol.ts index c702daea..f6b5da47 100644 --- a/app/src/storage/storageOwnerProtocol.ts +++ b/app/src/storage/storageOwnerProtocol.ts @@ -20,6 +20,7 @@ export const STORAGE_METHODS = [ 'listFileSyncStatusPage', 'getMetadata', 'save', + 'commitEditorOperations', 'isOperationLogNotebook', 'listOperationLogComments', 'addOperationLogComment', diff --git a/app/src/storage/storageOwnerRuntime.ts b/app/src/storage/storageOwnerRuntime.ts index 65623b82..56d4d54b 100644 --- a/app/src/storage/storageOwnerRuntime.ts +++ b/app/src/storage/storageOwnerRuntime.ts @@ -97,6 +97,7 @@ export async function startStorageOwner(identity?: { } return { attach: (port: MessagePort) => host.attach(port), + diagnostics: () => host.diagnostics(), drain: () => { clearInterval(timer) return host.drainForRelease() diff --git a/app/src/storage/storageRecovery.test.ts b/app/src/storage/storageRecovery.test.ts index 1a2dc766..37409810 100644 --- a/app/src/storage/storageRecovery.test.ts +++ b/app/src/storage/storageRecovery.test.ts @@ -53,6 +53,7 @@ function connect(host: StorageOwnerHost, prepare = vi.fn(async () => vi.fn())) { reload, prepare ) + pagePort.postMessage(recoveryMessage('register', 'editor-client')) clients.push(client) return { client, reload, prepare, pagePort } } @@ -145,6 +146,16 @@ describe('coordinated storage restart', () => { channel.port2 as unknown as MessagePort ) host.attach(channel.port1 as unknown as MessagePort) + channel.port2.postMessage({ + type: 'request', + id: 'legacy-hello', + method: 'hello', + version: 0, + args: [], + }) + await vi.waitFor(async () => + expect((await first.client.diagnose()).connectedTabs).toBe(2) + ) vi.useFakeTimers() first.client.recovery!.restart() await vi.waitFor(() => expect(first.prepare).toHaveBeenCalled()) @@ -437,3 +448,48 @@ it('offers recovery for a release-settings failure before the RPC deadline', asy ) ).toBe(false) }) + +it('does not count the launcher control connection as an editor tab', async () => { + const { host, shutdown } = setup() + const editor = connect(host) + const launcher = new MessageChannel() + ports.push( + launcher.port1 as unknown as MessagePort, + launcher.port2 as unknown as MessagePort + ) + host.attach(launcher.port1 as unknown as MessagePort) + const messages: unknown[] = [] + launcher.port2.on('message', (message) => messages.push(message)) + await vi.waitFor(async () => + expect((await editor.client.diagnose()).connectedTabs).toBe(1) + ) + expect((await editor.client.diagnose()).connectedPorts).toBe(2) + editor.client.recovery!.restart() + await vi.waitFor(() => expect(editor.reload).toHaveBeenCalledOnce()) + expect(shutdown).toHaveBeenCalledOnce() + expect(messages).toEqual([]) +}) + +it('backs up and restarts an admitted owner without consulting release settings', async () => { + const { store, shutdown } = setup() + const module = await import('../releases/control') + const control = vi + .spyOn(module, 'control') + .mockImplementation(() => new Promise(() => {})) + try { + const host = new StorageOwnerHost( + store as unknown as LocalNotebooks, + shutdown, + undefined, + { releaseId: 'test', epoch: 1, capabilities: [] } + ) + const editor = connect(host) + editor.client.recovery!.restart() + await vi.waitFor(() => expect(editor.reload).toHaveBeenCalledOnce()) + expect(editor.prepare).toHaveBeenCalledOnce() + expect(control).not.toHaveBeenCalled() + expect(shutdown).toHaveBeenCalledOnce() + } finally { + control.mockRestore() + } +}) diff --git a/app/src/storage/storageRecovery.ts b/app/src/storage/storageRecovery.ts index 59e77b32..e60917ae 100644 --- a/app/src/storage/storageRecovery.ts +++ b/app/src/storage/storageRecovery.ts @@ -7,7 +7,7 @@ export type StorageRecoveryState = { } let state: StorageRecoveryState = { phase: 'healthy', message: '' } const listeners = new Set<() => void>() -let restart: (() => void) | undefined +let restart: ((force?: boolean) => void) | undefined let diagnose: (() => Promise) | undefined let backup: (() => number) | undefined let prepare: ((signal: AbortSignal) => Promise<() => void>) | undefined @@ -48,7 +48,7 @@ export const storageRecovery = { new Error('Storage worker diagnostics are not connected.') ) }, - registerRestart(callback: () => void) { + registerRestart(callback: (force?: boolean) => void) { restart = callback }, registerPreparation( @@ -78,9 +78,9 @@ export const storageRecovery = { ) return prepare(signal) }, - restart() { + restart(force = false) { if (!restart) return - restart() + restart(force) }, } diff --git a/app/src/storage/storageRecoveryProtocol.ts b/app/src/storage/storageRecoveryProtocol.ts index f41a349b..c91ce092 100644 --- a/app/src/storage/storageRecoveryProtocol.ts +++ b/app/src/storage/storageRecoveryProtocol.ts @@ -3,7 +3,14 @@ export const STORAGE_RECOVERY_PROTOCOL = 1 export type RecoveryMessage = { type: 'storage-recovery' protocol: 1 - action: 'restart' | 'prepare' | 'ready' | 'blocked' | 'draining' | 'reload' + action: + | 'register' + | 'restart' + | 'prepare' + | 'ready' + | 'blocked' + | 'draining' + | 'reload' id: string error?: string } diff --git a/app/src/storage/storageWorkerControl.test.ts b/app/src/storage/storageWorkerControl.test.ts new file mode 100644 index 00000000..ce85711d --- /dev/null +++ b/app/src/storage/storageWorkerControl.test.ts @@ -0,0 +1,58 @@ +// @vitest-environment node +import { expect, it, vi } from 'vitest' + +import { + StorageWorkerControl, + type WorkerControlEvent, + installStorageWorkerControl, +} from './storageWorkerControl' + +it('answers diagnostics during a stalled drain and allows force close without tab acknowledgements', async () => { + let phase = 'initializing-storage' + const close = vi.fn() + const drain = vi.fn(() => new Promise(() => {})) + const dispose = installStorageWorkerControl({ + status: () => ({ + instanceId: 'stalled', + epoch: 0, + phase, + phaseElapsedMs: 100, + uptimeMs: 200, + }), + phase: (value) => { + phase = value + }, + drain, + close, + }) + const events: WorkerControlEvent[] = [] + const client = new StorageWorkerControl((event) => { + events.push(event) + }) + try { + client.discover() + await vi.waitFor(() => + expect(events.at(-1)?.status.phase).toBe('initializing-storage') + ) + client.stop('unrelated-worker', { force: true, restart: true }) + client.stop('stalled', { force: false, restart: true }) + await vi.waitFor(() => expect(drain).toHaveBeenCalledOnce()) + expect(close).not.toHaveBeenCalled() + client.discover() + await vi.waitFor(() => + expect(events.at(-1)).toMatchObject({ + type: 'status', + status: { phase: 'draining' }, + }) + ) + client.stop('stalled', { force: true, restart: true }) + await vi.waitFor(() => + expect(events.at(-1)).toMatchObject({ type: 'stopped', restart: true }) + ) + expect(close).toHaveBeenCalledOnce() + expect(drain).toHaveBeenCalledOnce() + } finally { + client.close() + dispose() + } +}) diff --git a/app/src/storage/storageWorkerControl.ts b/app/src/storage/storageWorkerControl.ts new file mode 100644 index 00000000..5f4808f1 --- /dev/null +++ b/app/src/storage/storageWorkerControl.ts @@ -0,0 +1,105 @@ +import type { StorageOwnerDiagnostics } from './storageOwnerDiagnostics' + +export const STORAGE_WORKER_CONTROL = 'runme-storage-worker-control-v1' +export type WorkerStatus = { + instanceId: string + releaseId?: string + epoch: number + phase: string + phaseElapsedMs: number + uptimeMs: number + diagnostics?: StorageOwnerDiagnostics + error?: string +} +export type WorkerControlEvent = + | { type: 'status'; status: WorkerStatus } + | { type: 'stopping' | 'stopped'; status: WorkerStatus; restart: boolean } +type WorkerCommand = + | { type: 'discover' } + | { type: 'stop'; instanceId: string; force: boolean; restart: boolean } + +/** A memory-only management connection, independent of storage RPC and IndexedDB. + * Discovery includes only responding Runme workers implementing this protocol. + */ +export class StorageWorkerControl { + private readonly channel = new BroadcastChannel(STORAGE_WORKER_CONTROL) + constructor(onEvent: (event: WorkerControlEvent) => void) { + this.channel.onmessage = ({ data }) => { + if (['status', 'stopping', 'stopped'].includes(data?.type) && data.status) + onEvent(data as WorkerControlEvent) + } + } + discover(): void { + this.channel.postMessage({ type: 'discover' } satisfies WorkerCommand) + } + stop( + instanceId: string, + options: { force: boolean; restart: boolean } + ): void { + this.channel.postMessage({ + type: 'stop', + instanceId, + ...options, + } satisfies WorkerCommand) + } + close(): void { + this.channel.close() + } +} + +/** Install before lock acquisition/imports so a stalled startup remains visible. + * Force-close is cooperative: the worker event loop must still process messages. + * No tab acknowledgements are needed because this never reloads a page. + */ +export function installStorageWorkerControl(options: { + status: () => WorkerStatus + drain: () => Promise + close: () => void + phase: (phase: string) => void +}): () => void { + const channel = new BroadcastChannel(STORAGE_WORKER_CONTROL) + let stopping = false + let closed = false + const publish = (type: 'stopping' | 'stopped', restart: boolean) => + channel.postMessage({ + type, + status: options.status(), + restart, + } satisfies WorkerControlEvent) + channel.onmessage = async ({ data }: MessageEvent) => { + if (data?.type === 'discover') { + channel.postMessage({ + type: 'status', + status: options.status(), + } satisfies WorkerControlEvent) + return + } + if ( + data?.type !== 'stop' || + data.instanceId !== options.status().instanceId || + closed + ) + return + // A force request can interrupt a graceful drain stuck in storage/network IO. + if (stopping && !data.force) return + stopping = true + options.phase(data.force ? 'force-closing' : 'draining') + publish('stopping', data.restart) + try { + if (!data.force) await options.drain() + if (closed) return + closed = true + options.phase('closing') + publish('stopped', data.restart) + options.close() + } catch (error) { + stopping = false + options.phase('drain-failed') + channel.postMessage({ + type: 'status', + status: { ...options.status(), error: String(error) }, + } satisfies WorkerControlEvent) + } + } + return () => channel.close() +} diff --git a/app/test/browser/storage-worker-reconnect.ts b/app/test/browser/storage-worker-reconnect.ts new file mode 100644 index 00000000..7f622f20 --- /dev/null +++ b/app/test/browser/storage-worker-reconnect.ts @@ -0,0 +1,309 @@ +import assert from 'node:assert/strict' +import { mkdir, writeFile } from 'node:fs/promises' +import { type Page, chromium } from 'playwright-core' + +/** Await the actual asynchronous probe result, rather than a truthy Promise. */ +async function waitForOwner(page: Page, previous: string) { + const deadline = Date.now() + 30000 + while (Date.now() < deadline) { + const ready = await page.evaluate(async (old) => { + const w = window as any + if (w.recovery.getSnapshot().phase === 'required') + await w.store.getMetadata(w.uri) + if (w.recovery.getSnapshot().phase !== 'healthy') return false + try { + return (await w.recovery.diagnose()).instanceId !== old + } catch { + return false + } + }, previous) + if (ready) return + await new Promise((resolve) => setTimeout(resolve, 100)) + } + throw new Error( + `Owner did not reconnect: ${JSON.stringify(await page.evaluate(() => (window as any).recovery.getSnapshot()))}` + ) +} + +/** Exercise the actual tab store, SharedWorker and OPFS in a disposable profile. + * Run against Vite development serving this checkout; no live credentials/data. + */ +async function main() { + const browser = await chromium.launch({ + executablePath: process.env.CHROMIUM_PATH, + headless: true, + }) + const output = 'test/browser/test-output/storage-worker-reconnect' + await mkdir(output, { recursive: true }) + const logs: string[] = [] + try { + const context = await browser.newContext() + const base = process.env.RUNME_TEST_URL ?? 'http://127.0.0.1:5194' + await context.route('**/reconnect-test', (route) => + route.fulfill({ + contentType: 'text/html', + body: 'Storage reconnect

Disposable storage lifecycle test

', + }) + ) + const pages = [await context.newPage(), await context.newPage()] + for (const page of pages) { + page.on('console', (message) => logs.push(message.text())) + page.on('pageerror', (error) => logs.push(String(error))) + await page.goto(`${base}/reconnect-test`) + await page.evaluate(async () => { + const w = window as any + w.pageIdentity = crypto.randomUUID() + // Drop only a successful editor-commit reply, after the actual OPFS write. + // The tab must keep the same batch and deduplicate it on the new owner. + const NativeWorker = SharedWorker + w.SharedWorker = class extends NativeWorker { + constructor(...args: ConstructorParameters) { + super(...args) + w.lastWorkerArgs = args + const ids = new Set() + const send = this.port.postMessage.bind(this.port) + this.port.postMessage = (message: any) => { + if (message.method === 'commitEditorOperations') + ids.add(message.id) + send(message) + } + const descriptor = Object.getOwnPropertyDescriptor( + MessagePort.prototype, + 'onmessage' + )! + Object.defineProperty(this.port, 'onmessage', { + set(handler) { + descriptor.set!.call(this, (event: MessageEvent) => { + if ( + w.dropReply && + event.data.type === 'result' && + !event.data.error && + ids.has(event.data.id) + ) { + w.lostReply = true + return + } + handler?.(event) + }) + }, + }) + } + } + const clientPath = '/src/storage/storageOwnerClient.ts' + const drivePath = '/src/storage/drive.ts' + const protoPath = '/src/runme/client.ts' + const controlPath = '/src/storage/storageRecovery.ts' + const protobufPath = '/.vite/deps/@bufbuild_protobuf.js' + w.createProto = (await import(protobufPath)).create + const { createSharedNotebookStore } = await import(clientPath) + const { DriveNotebookStore } = await import(drivePath) + w.proto = (await import(protoPath)).parser_pb + w.recovery = (await import(controlPath)).storageRecovery + w.recovery.subscribe(() => + console.log( + 'RECOVERY', + w.pageIdentity, + JSON.stringify(w.recovery.getSnapshot()) + ) + ) + const watch = new BroadcastChannel('runme-storage-worker-control-v1') + watch.onmessage = ({ data }) => + console.log('MANAGEMENT', w.pageIdentity, JSON.stringify(data)) + w.store = createSharedNotebookStore( + new DriveNotebookStore(async () => { + throw new Error('No test credentials') + }) + ) + await w.store.ready + w.passive = new NativeWorker( + ...(w.lastWorkerArgs as ConstructorParameters) + ) + w.passive.port.start() + }) + } + const identities = await Promise.all( + pages.map((page) => page.evaluate(() => (window as any).pageIdentity)) + ) + const original = await pages[0].evaluate(async () => { + const w = window as any + const file = await w.store.create('local://folder/local', 'restart.runme') + w.uri = file.uri + w.view = await w.store.createOperationLogSaveStore(file.uri) + w.editor = w.view.initialNotebook + w.editor.cells.push( + w.createProto(w.proto.CellSchema, { + refId: 'first', + kind: w.proto.CellKind.MARKUP, + value: 'Original', + }) + ) + await w.view.save(file.uri, w.editor) + return { + uri: file.uri, + instance: (await w.recovery.diagnose()).instanceId, + } + }) + await pages[1].evaluate(async (uri) => { + const w = window as any + w.uri = uri + w.view = await w.store.createOperationLogSaveStore(uri) + w.editor = w.view.initialNotebook + }, original.uri) + await pages[0].evaluate(() => { + const w = window as any + w.dropReply = true + w.editor.cells[0].value = 'Committed before worker stop' + w.saveResult = w.view.save(w.uri, w.editor).catch((error: Error) => { + w.saveError = String(error) + }) + }) + await pages[0].waitForFunction(() => (window as any).lostReply, undefined, { + timeout: 20000, + }) + await pages[1].evaluate(async () => { + const w = window as any + w.editor.cells.push( + w.createProto(w.proto.CellSchema, { + refId: 'other', + kind: w.proto.CellKind.MARKUP, + value: 'Other tab content', + }) + ) + await w.view.save(w.uri, w.editor) + }) + const before = await pages[1].evaluate(() => + (window as any).store.loadContent((window as any).uri) + ) + await pages[0].evaluate(() => { + const w = window as any + w.dropReply = false + w.recovery.restart(true) + }) + for (const page of pages) await waitForOwner(page, original.instance) + assert.deepEqual( + await Promise.all( + pages.map((page) => page.evaluate(() => (window as any).pageIdentity)) + ), + identities, + 'Neither page may reload' + ) + assert.equal( + await pages[0].evaluate(() => + (window as any).store.loadContent((window as any).uri) + ), + before, + 'Retry must not duplicate the committed batch' + ) + await pages[0].evaluate(async () => { + const w = window as any + w.editor.cells[0].value = 'Edited after reconnect' + await w.view.save(w.uri, w.editor) + }) + const cells = await pages[1].evaluate(async () => + ( + await (window as any).store.loadOperationLogSnapshot( + (window as any).uri + ) + ).cells.map((cell: any) => cell.value) + ) + console.log('Force restart completed') + assert.deepEqual(cells, ['Edited after reconnect', 'Other tab content']) + const forceInstance = await pages[0].evaluate( + async () => (await (window as any).recovery.diagnose()).instanceId + ) + await pages[0].evaluate(() => (window as any).recovery.restart()) + for (const page of pages) await waitForOwner(page, forceInstance) + assert.deepEqual( + await Promise.all( + pages.map((page) => page.evaluate(() => (window as any).pageIdentity)) + ), + identities + ) + console.log('Graceful restart completed') + // Idle tabs do not need immediate reconnection. Each can join on its next + // request without waiting for, or sending readiness messages to, another tab. + const beforeStop = await pages[0].evaluate( + async () => (await (window as any).recovery.diagnose()).instanceId + ) + await pages[0].evaluate(async () => { + const path = '/src/storage/storageWorkerControl.ts' + const { StorageWorkerControl } = await import(path) + await new Promise((resolve) => { + const control = new StorageWorkerControl((event: any) => { + if (event.type !== 'status') return + control.stop(event.status.instanceId, { force: true, restart: false }) + control.close() + resolve() + }) + control.discover() + }) + }) + for (const page of pages) + await page.waitForFunction( + () => (window as any).recovery.getSnapshot().phase === 'required' + ) + await pages[1].evaluate(async () => { + const w = window as any + w.editor.cells[1].value = 'Saved by idle tab after stop' + await w.view.save(w.uri, w.editor) + }) + for (const page of pages) await waitForOwner(page, beforeStop) + assert.deepEqual( + await Promise.all( + pages.map((page) => page.evaluate(() => (window as any).pageIdentity)) + ), + identities + ) + const finalCells = await pages[0].evaluate(async () => + ( + await (window as any).store.loadOperationLogSnapshot( + (window as any).uri + ) + ).cells.map((cell: any) => cell.value) + ) + assert.deepEqual(finalCells, [ + 'Edited after reconnect', + 'Saved by idle tab after stop', + ]) + const ownerIds = await Promise.all( + pages.map((page) => + page.evaluate( + async () => (await (window as any).recovery.diagnose()).instanceId + ) + ) + ) + assert.equal( + ownerIds[0], + ownerIds[1], + 'Independent retries join the same worker' + ) + await writeFile( + `${output}/result.json`, + JSON.stringify( + { + passed: true, + pagesPreserved: identities, + cells, + lostReplyDeduplicated: true, + forceRestart: true, + gracefulRestart: true, + independentRetryAfterStop: true, + finalCells, + ownerIds, + }, + null, + 2 + ) + ) + console.log( + 'PASS: force and graceful restarts preserve both pages; lost save reply is deduplicated; concurrent edits survive' + ) + } finally { + await writeFile(`${output}/browser.log`, logs.join('\n')) + await browser.close() + } +} +main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/docs-dev/CUJs/drive-sync-recovery.md b/docs-dev/CUJs/drive-sync-recovery.md index dd4e8d1c..d5951566 100644 --- a/docs-dev/CUJs/drive-sync-recovery.md +++ b/docs-dev/CUJs/drive-sync-recovery.md @@ -351,26 +351,29 @@ all notebooks copied before a hung first flush, cancellation after worker loss, downloads with unavailable session storage, and bounded diagnostic probes outside storage initialization. A timeout never retries the uncertain operation. -On an isolated test origin, reproduce a storage protocol mismatch and verify -**Restart Runme** appears above the notebook and inside Google Drive Sync Status. -With two tabs open, make an edit, click the button, and verify both tabs reload -without closing the browser, notebook storage works again, and recovery JSON -contains the editor content. Block one tab's acknowledgement: neither tab should -reload. Block a storage/network write: restart must wait for the write to settle. -Repeat against an older worker without the recovery protocol: show the actionable -fallback and retain editor content rather than silently reloading in a loop. -Verify the initiating tab has session copies even without a worker response. Fill -session storage, click **Get recovery copies**, and download the current editors; -verify exact cell text in the JSON. The banner must distinguish earlier snapshots -from current captures, and explain that session copies do not survive tab closure. -Perform these destructive failure injections only in an isolated test profile. - -The emitted-worker/browser check is `test/browser/storage-owner-recovery.ts`. -Against a production preview on port 5194, compile it with the same `tsc` flags -above and run `node test/browser/.generated/storage-owner-recovery.js`. It asserts -that two pages reload, the old SharedWorker target terminates, recovery copies -survive, durable notebook bytes are unchanged, and a new owner accepts requests. -A frontend worker fixture then verifies the actual notebook and status-page -buttons and legacy fallback. Screenshots and JSON results are written under -`test/browser/test-output/storage-*recovery*`. Use an isolated Chromium for Testing -binary via `CHROMIUM_PATH`; the test never uses an existing browser profile. +On an isolated test origin, verify **Restart storage** and **Force restart +storage** appear above the notebook when storage fails. Open the Version +Information page and verify worker discovery and stop controls remain available +while release settings fail. Discovery must label nonresponders as unknown. + +With two tabs open, save a notebook and discard its successful worker reply. +Let the other tab save an additional cell. Force restart storage: both pages and +editor objects must remain open, the worker instance must change, the retained +batch must be acknowledged without changing persisted bytes, and a subsequent +edit must preserve the other tab's cell. Repeat with graceful restart. Neither +path may call page reload. Block a drain and verify diagnostics and force restart +still work. Block release settings during startup and inspect the last reached +phase. App upgrade/reload continues to require its separate preservation protocol. + +The browser regression is `test/browser/storage-worker-reconnect.ts`, running +against Vite on localhost:5194 with disposable Chromium storage. Compile it with +`tsc --target es2022 --module nodenext --moduleResolution nodenext +--esModuleInterop --skipLibCheck --outDir test/browser/.generated + test/browser/storage-worker-reconnect.ts`, then run the generated script with +`CHROMIUM_PATH` pointing to an isolated Chromium for Testing binary. Evidence goes +to `test/browser/test-output/storage-worker-reconnect`. + +`storage-owner-recovery.ts` retains coverage of the older coordinated reload wire +protocol for compatibility. It is not evidence for the new in-place restart path. +Current editor downloads remain available independently of session storage and +worker health. Never inject failures into a live user's browser profile. diff --git a/docs-dev/design/20261027_storage_lifecycle.md b/docs-dev/design/20261027_storage_lifecycle.md new file mode 100644 index 00000000..d3a59f76 --- /dev/null +++ b/docs-dev/design/20261027_storage_lifecycle.md @@ -0,0 +1,100 @@ +# Storage lifecycle and recovery + +## Decision + +Keep the lifetime Web Lock as the single storage-ownership authority. Keep +release selection durable, but do not require a release-settings transaction to +back up editors or stop the existing worker. Recovery controls and diagnostic +probes must remain available before notebook storage initializes. + +A storage restart does not reload pages. Tabs retain their editor models and +pending save batches. The old worker closes; replacements reacquire the same +lifetime lock, validate release admission, and accept the retained batches. +Stopping the worker requires no tab acknowledgement or session-storage write. +Each tab reconnects independently on its next request; idle tabs need not connect +immediately, and there are no tab readiness or reconnect broadcasts. +Only app upgrade/reload requires coordinated editor preservation. + +The tab owns the save baseline: the snapshot last loaded into the editor or +successfully acknowledged after a save. It prepares immutable operations before +sending them to storage, retaining both the operations and the submitted snapshot +until acknowledgement. Edits typed while waiting belong to the next snapshot. +The worker validates notebook identity and causal dependencies, rejects conflicting +contents under an existing operation ID, and appends only missing operations. +Retrying the identical batch repairs a lost acknowledgement without duplication. +No editor identity search reconstructs a baseline, and no latest merged notebook +is substituted for the tab's actual observed version. + +Graceful stop drains admitted work. Force stop requests `self.close()` immediately, +without awaiting storage or network IO. A force request can interrupt a stalled +graceful drain, provided the JavaScript message loop still responds. Other +operations, such as remote notebook creation, retain uncertain-outcome blockers +and are never blindly retried. Editor batches alone have this idempotent contract. + +A BroadcastChannel management endpoint starts before lock acquisition and runtime +imports. It reports the current startup phase and elapsed time without reading +IndexedDB. Once initialized, it includes host request diagnostics. Version UI +always exposes discovery, stop, force stop, restart and force restart controls. +Discovery describes responding Runme workers, not all workers in the browser. + +## Failure analysis + +Recent releases added editor snapshots and diagnostics (#417), immutable app +releases and multi-tab handoff (#418), version UI and partial offline readiness +(#419), loading diagnostics/UI handoff (#421), bounded release-settings access +(#423), unreadable-record quarantine (#425), and an independently updated launcher +(#428). #430 fixed the publication runner. These changes each address a real +failure, but recovery became dependent on the release database it was recovering. + +The previous restart handler writes `emergencyRestart` before requesting editor +snapshots. If the release database stalls, no other tab is asked to preserve its +editors. This is a shared failure dependency, not proof of an IndexedDB lock cycle. +The editor preparation routine also awaits persistence through the failing worker. +That defeats the purpose of its independent session-storage snapshot. + +A second bug treats MessagePorts as tabs. The launcher holds one control port and +the notebook app holds another editor port to the same worker. The launcher cannot +acknowledge editor preparation. Counting its control port as a participant can +block every restart. Diagnostics must report connections and editor clients +separately. A control connection is not evidence of unsaved editors. + +## Invariants + +- Only a worker holding `runme-storage-owner-lifetime-v1` opens notebook stores. +- Editor clients explicitly register for recovery; existing clients that send + notebook RPCs are also participants. Passive launcher/control ports are excluded. +- Storage stop never reloads a tab, discards an editor, or waits for tab votes. +- App reload remains a separate operation with preparation, preservation and + blockers for uncertain outcomes. Session copies do not survive tab closure. +- Graceful stop drains admitted work. Force stop skips drain and retains unknown + outcomes. Neither path replays generic mutations or overwrites history with an + editor snapshot. Only immutable editor batches may be retried automatically. +- A tab advances its baseline only after acknowledgement of the exact submitted + snapshot. Its pending batch and later editor contents survive transport loss. +- Duplicate operation IDs require identical contents; missing causal history or a + changed notebook identity rejects the batch without altering OPFS bytes. +- The replacement must reacquire the lifetime lock. Release admission still occurs + at startup; a restart is not permission to activate a different release. +- Diagnostics read memory, not the database under investigation. + +## Browser boundary + +Web pages cannot enumerate all browser SharedWorkers or externally terminate them. +Runme can list workers that implement its own discovery protocol, and request a +cooperative stop or force-close from those workers. A nonresponse is unknown status, +not proof of death. Browser DevTools can enumerate and terminate workers with +privileges unavailable to the page. The UI must state this limitation explicitly. + +A dedicated storage worker with a supervisor could support external termination, +but introduces supervisor election, port forwarding and a second lifecycle. Chrome +also differs from the current HTML specification on Worker construction inside a +SharedWorker. Do not add this architecture as an untested recovery patch. + +## Verification required + +Real-browser tests must cover launcher plus editor ports, multiple editor tabs, +pending release-settings reads, pending persistence, new clients during recovery, +failed snapshots, worker initialization failure, and replacement identity under +the lifetime lock. Verify OPFS notebook contents after restart, and recovery copies +when an edit had not committed. Existing upgrade/offline/rollback journeys must +still pass. Unit tests alone do not establish lifecycle correctness. From 4c7c946e8020ce48a98add4d0ecda8c70b09250b Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 07:50:01 -0700 Subject: [PATCH 03/16] Exercise independent reconnect and observable storage startup stalls Signed-off-by: Jeremy lewi --- .github/workflows/test.yaml | 12 +- app/package.json | 3 +- .../components/StorageWorkerControls.test.tsx | 70 +++++ app/src/storage/storageOwner.worker.ts | 2 + app/src/storage/storageWorkerControl.ts | 1 + app/test/browser/controlled-releases.ts | 15 ++ app/test/browser/reconnect-fixture.ts | 2 + app/test/browser/run-controlled-releases.ts | 18 ++ .../browser/run-storage-worker-reconnect.ts | 57 ++++ app/test/browser/storage-worker-reconnect.ts | 4 +- app/test/browser/storage-worker-startup.ts | 249 ++++++++++++++++++ docs-dev/CUJs/drive-sync-recovery.md | 20 +- docs-dev/design/20261027_storage_lifecycle.md | 3 +- 13 files changed, 444 insertions(+), 12 deletions(-) create mode 100644 app/src/components/StorageWorkerControls.test.tsx create mode 100644 app/test/browser/reconnect-fixture.ts create mode 100644 app/test/browser/run-storage-worker-reconnect.ts create mode 100644 app/test/browser/storage-worker-startup.ts diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 802e0a0a..bdaa5c15 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -72,7 +72,8 @@ jobs: - name: Test storage restart and editor recovery run: >- pnpm -C app exec vitest run - src/storage/storageRecovery.test.ts src/lib/storageRecoveryDrafts.test.ts + src/storage/storageRecovery.test.ts src/storage/editorSaveSession.test.ts + src/storage/storageWorkerControl.test.ts src/lib/storageRecoveryDrafts.test.ts src/components/StorageRecoveryBanner.test.tsx --maxWorkers=2 --minWorkers=1 - name: Test explorer folder refresh @@ -123,7 +124,11 @@ jobs: src/releases/releaseDatabase.test.ts src/releases/startupRecovery.test.ts src/releases/recoverControl.test.ts src/releases/controlChunks.test.ts src/releases/recoveryOwnerLock.test.ts src/storage/releaseOwnerFailure.test.ts src/releases/startupDiagnostics.test.ts src/pwa/loader-worker.test.ts src/components/ReleaseControls.test.tsx + src/components/ReleaseRecoveryControls.test.tsx src/components/StorageWorkerControls.test.tsx src/releases/releaseCatalog.test.ts - run: pnpm -C app exec playwright-core install --with-deps chromium + - name: Test independent storage reconnect + if: matrix.baseline == 'current' + run: pnpm -C app run cuj:storage - run: pnpm -C app run cuj:releases env: RUNME_RELEASE_BASELINE_COMMIT: ${{ matrix.baseline != 'current' && matrix.baseline || '' }} @@ -131,4 +136,7 @@ jobs: if: always() with: name: controlled-release-evidence-${{ matrix.baseline }} - path: app/test/browser/test-output/controlled-releases/ + path: | + app/test/browser/test-output/controlled-releases/ + app/test/browser/test-output/storage-worker-startup/ + app/test/browser/test-output/storage-worker-reconnect/ diff --git a/app/package.json b/app/package.json index cec924f0..3f2c9353 100644 --- a/app/package.json +++ b/app/package.json @@ -16,7 +16,8 @@ "format": "prettier --ignore-path .prettierignore --write .", "preview": "vite preview", "test": "pnpm run check:documentation && vitest", - "cuj:releases": "tsc --target es2020 --module nodenext --moduleResolution nodenext --esModuleInterop --skipLibCheck --outDir test/browser/.generated test/browser/run-controlled-releases.ts test/browser/controlled-releases.ts && node test/browser/.generated/run-controlled-releases.js" + "cuj:storage": "tsc --target es2022 --module nodenext --moduleResolution nodenext --esModuleInterop --skipLibCheck --outDir test/browser/.generated test/browser/run-storage-worker-reconnect.ts test/browser/storage-worker-reconnect.ts && node test/browser/.generated/run-storage-worker-reconnect.js", + "cuj:releases": "tsc --target es2020 --module nodenext --moduleResolution nodenext --esModuleInterop --skipLibCheck --outDir test/browser/.generated test/browser/run-controlled-releases.ts test/browser/controlled-releases.ts test/browser/storage-worker-startup.ts && node test/browser/.generated/run-controlled-releases.js" }, "dependencies": { "@buf/googleapis_googleapis.bufbuild_es": "2.6.0-20250411203938-61b203b9a916.1", diff --git a/app/src/components/StorageWorkerControls.test.tsx b/app/src/components/StorageWorkerControls.test.tsx new file mode 100644 index 00000000..ed426e55 --- /dev/null +++ b/app/src/components/StorageWorkerControls.test.tsx @@ -0,0 +1,70 @@ +import { + cleanup, + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import { STORAGE_WORKER_CONTROL } from '../storage/storageWorkerControl' +import { ReleaseControls } from './ReleaseControls' + +afterEach(() => { + cleanup() + delete window.runmeVersions +}) + +it('keeps targeted worker controls available when settings fail and a worker is draining', async () => { + window.runmeVersions = { + get: vi.fn(async () => { + throw new Error('Release settings read timeout (get)') + }), + } as unknown as NonNullable + const peer = new BroadcastChannel(STORAGE_WORKER_CONTROL) + const commands: unknown[] = [] + peer.onmessage = ({ data }) => { + commands.push(data) + if (data.type === 'discover') + for (const instanceId of ['worker-one', 'worker-two']) + peer.postMessage({ + type: 'status', + status: { + instanceId, + epoch: 1, + phase: 'draining', + phaseElapsedMs: 5000, + uptimeMs: 10000, + }, + }) + } + try { + render() + expect(await screen.findByText('Instance: worker-two')).toBeTruthy() + expect( + await screen.findByText(/Release settings read timeout/) + ).toBeTruthy() + expect( + screen.getByRole('button', { name: 'Browse published releases' }) + ).toBeTruthy() + expect(screen.getByLabelText('Commit SHA or release ID')).toBeTruthy() + const row = screen.getByText('Instance: worker-two').closest('div')! + const force = within(row).getByRole('button', { name: 'Force stop worker' }) + expect(force.hasAttribute('disabled')).toBe(false) + fireEvent.click(force) + await waitFor(() => + expect(commands).toContainEqual({ + type: 'stop', + instanceId: 'worker-two', + force: true, + restart: false, + }) + ) + expect(commands).not.toContainEqual( + expect.objectContaining({ type: 'stop', instanceId: 'worker-one' }) + ) + expect(screen.getByText(/No reply means unknown/)).toBeTruthy() + } finally { + peer.close() + } +}) diff --git a/app/src/storage/storageOwner.worker.ts b/app/src/storage/storageOwner.worker.ts index 2d68d2ca..b9ad8a11 100644 --- a/app/src/storage/storageOwner.worker.ts +++ b/app/src/storage/storageOwner.worker.ts @@ -1,5 +1,6 @@ import { appLogger } from '../lib/logging/runtime' import { OWNER_LOCK, control, readControl } from '../releases/control' +import { releaseDatabaseDiagnostics } from '../releases/releaseDiagnostics' import { retireUnreadableOwner } from './releaseOwnerFailure' import { installStorageWorkerControl } from './storageWorkerControl' @@ -40,6 +41,7 @@ installStorageWorkerControl({ phaseElapsedMs: Date.now() - phaseStartedAt, uptimeMs: Date.now() - startedAt, diagnostics: runtime?.diagnostics(), + releaseDatabase: releaseDatabaseDiagnostics(), }), phase: setPhase, drain: async () => { diff --git a/app/src/storage/storageWorkerControl.ts b/app/src/storage/storageWorkerControl.ts index 5f4808f1..f9a875d1 100644 --- a/app/src/storage/storageWorkerControl.ts +++ b/app/src/storage/storageWorkerControl.ts @@ -9,6 +9,7 @@ export type WorkerStatus = { phaseElapsedMs: number uptimeMs: number diagnostics?: StorageOwnerDiagnostics + releaseDatabase?: StorageOwnerDiagnostics['releaseDatabase'] error?: string } export type WorkerControlEvent = diff --git a/app/test/browser/controlled-releases.ts b/app/test/browser/controlled-releases.ts index 4f5533f1..155eef46 100644 --- a/app/test/browser/controlled-releases.ts +++ b/app/test/browser/controlled-releases.ts @@ -883,6 +883,21 @@ async function main() { exact: true, }) .click() + // Clicking starts asynchronous cache verification and handoff. Opening a + // new app immediately can admit it to the still-active broken release and + // make it a participant before recovery intent has been committed. Observe + // completed activation, as shown by recovery status, before opening it. + const recoveredDeadline = Date.now() + 60000 + let recovered = false + while (Date.now() < recoveredDeadline) { + recovered = await recovery.evaluate(async (id) => { + const state = await (window as any).runmeVersions.get() + return !state.transition && state.active?.manifest.releaseId === id + }, a) + if (recovered) break + await new Promise((resolve) => setTimeout(resolve, 100)) + } + assert.ok(recovered, 'Recovery must activate A before opening a new app') const restored = await context.newPage() pages.splice(0, 3, restored) await restored.goto(base + '/?session=release-one') diff --git a/app/test/browser/reconnect-fixture.ts b/app/test/browser/reconnect-fixture.ts new file mode 100644 index 00000000..7ec48ce0 --- /dev/null +++ b/app/test/browser/reconnect-fixture.ts @@ -0,0 +1,2 @@ +// Let Vite resolve the dependency; do not rely on prebundled cache filenames. +export { create } from '@bufbuild/protobuf' diff --git a/app/test/browser/run-controlled-releases.ts b/app/test/browser/run-controlled-releases.ts index 17f15b2e..89854c83 100644 --- a/app/test/browser/run-controlled-releases.ts +++ b/app/test/browser/run-controlled-releases.ts @@ -130,6 +130,24 @@ async function main() { ) const code = await new Promise((resolve) => test.once('exit', resolve)) if (code !== 0) throw new Error(`Controlled release CUJ failed: ${code}`) + const startup = spawn( + process.execPath, + [ + join( + dirname(fileURLToPath(import.meta.url)), + 'storage-worker-startup.js' + ), + ], + { + env: { ...process.env, RUNME_TEST_URL: 'http://127.0.0.1:5198' }, + stdio: 'inherit', + } + ) + const startupCode = await new Promise((resolve) => + startup.once('exit', resolve) + ) + if (startupCode !== 0) + throw new Error(`Storage startup CUJ failed: ${startupCode}`) } finally { server.kill('SIGTERM') } diff --git a/app/test/browser/run-storage-worker-reconnect.ts b/app/test/browser/run-storage-worker-reconnect.ts new file mode 100644 index 00000000..1231058d --- /dev/null +++ b/app/test/browser/run-storage-worker-reconnect.ts @@ -0,0 +1,57 @@ +import { spawn } from 'node:child_process' +import { mkdir, writeFile } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' + +/** Run the real app's Vite server and browser test with isolated storage. */ +async function main() { + const output = 'test/browser/test-output/storage-worker-reconnect' + await mkdir(output, { recursive: true }) + const logs: string[] = [] + const server = spawn( + 'pnpm', + ['exec', 'vite', '--host', '127.0.0.1', '--port', '5194', '--strictPort'], + { + env: { ...process.env, CHOKIDAR_USEPOLLING: 'true' }, + stdio: ['ignore', 'pipe', 'pipe'], + } + ) + server.stdout.on('data', (chunk) => logs.push(String(chunk))) + server.stderr.on('data', (chunk) => logs.push(String(chunk))) + try { + for (let attempt = 0; ; attempt++) { + if (server.exitCode !== null) + throw new Error('Vite exited before the test') + if ( + await fetch('http://127.0.0.1:5194/') + .then((r) => r.ok) + .catch(() => false) + ) + break + if (attempt === 99) throw new Error('Vite did not become ready') + await new Promise((resolve) => setTimeout(resolve, 100)) + } + const test = spawn( + process.execPath, + [ + join( + dirname(fileURLToPath(import.meta.url)), + 'storage-worker-reconnect.js' + ), + ], + { + env: { ...process.env, RUNME_TEST_URL: 'http://127.0.0.1:5194' }, + stdio: 'inherit', + } + ) + const code = await new Promise((resolve) => test.once('exit', resolve)) + if (code !== 0) throw new Error(`Storage reconnect test failed: ${code}`) + } finally { + server.kill('SIGTERM') + await writeFile(`${output}/vite.log`, logs.join('')) + } +} +void main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/app/test/browser/storage-worker-reconnect.ts b/app/test/browser/storage-worker-reconnect.ts index 7f622f20..4a896373 100644 --- a/app/test/browser/storage-worker-reconnect.ts +++ b/app/test/browser/storage-worker-reconnect.ts @@ -35,6 +35,7 @@ async function main() { }) const output = 'test/browser/test-output/storage-worker-reconnect' await mkdir(output, { recursive: true }) + const watchdog = setTimeout(() => void browser.close(), 90000) const logs: string[] = [] try { const context = await browser.newContext() @@ -93,7 +94,7 @@ async function main() { const drivePath = '/src/storage/drive.ts' const protoPath = '/src/runme/client.ts' const controlPath = '/src/storage/storageRecovery.ts' - const protobufPath = '/.vite/deps/@bufbuild_protobuf.js' + const protobufPath = '/test/browser/reconnect-fixture.ts' w.createProto = (await import(protobufPath)).create const { createSharedNotebookStore } = await import(clientPath) const { DriveNotebookStore } = await import(drivePath) @@ -299,6 +300,7 @@ async function main() { 'PASS: force and graceful restarts preserve both pages; lost save reply is deduplicated; concurrent edits survive' ) } finally { + clearTimeout(watchdog) await writeFile(`${output}/browser.log`, logs.join('\n')) await browser.close() } diff --git a/app/test/browser/storage-worker-startup.ts b/app/test/browser/storage-worker-startup.ts new file mode 100644 index 00000000..a726404a --- /dev/null +++ b/app/test/browser/storage-worker-startup.ts @@ -0,0 +1,249 @@ +import assert from 'node:assert/strict' +import { mkdir, writeFile } from 'node:fs/promises' +import { type Page, chromium } from 'playwright-core' + +/** Poll an asynchronous browser observation with an actual awaited result. */ +async function until( + page: Page, + predicate: () => Promise, + label: string +) { + const deadline = Date.now() + 20000 + while (Date.now() < deadline) { + if (await page.evaluate(predicate)) return + await new Promise((resolve) => setTimeout(resolve, 100)) + } + throw new Error(`Timed out: ${label}`) +} + +/** Hold a real IndexedDB transaction ahead of the emitted worker's settings read. + * This exercises Chromium scheduling rather than mocking a request callback. + */ +async function main() { + const base = process.env.RUNME_TEST_URL ?? 'http://127.0.0.1:5198' + await fetch(`${base}/test/promote?release=b`, { method: 'POST' }) + const browser = await chromium.launch({ + executablePath: process.env.CHROMIUM_PATH, + headless: true, + }) + const output = 'test/browser/test-output/storage-worker-startup' + await mkdir(output, { recursive: true }) + const events: unknown[] = [] + const watchdog = setTimeout(() => void browser.close(), 90000) + try { + const context = await browser.newContext() + const page = await context.newPage() + page.on('console', (message) => events.push(message.text())) + await page.goto(`${base}/recovery.html`) + await until( + page, + async () => Boolean((await (window as any).runmeVersions?.get())?.active), + 'initial owner activation' + ) + const identity = await page.evaluate(async () => { + const w = window as any + w.active = (await w.runmeVersions.get()).active + w.pageIdentity = crypto.randomUUID() + w.events = [] + w.management = new BroadcastChannel('runme-storage-worker-control-v1') + w.management.onmessage = ({ data }: MessageEvent) => { + if (!data.status) return + w.events.push(data) + w.workerStatus = data.status + } + w.startOwner = () => { + const { manifest, epoch } = w.active + w.worker = new SharedWorker( + `/version/${manifest.releaseId}/${manifest.owner.entry}`, + { + type: 'module', + name: `runme-storage-owner-${manifest.releaseId}-${epoch}`, + } + ) + w.worker.port.start() + } + return w.pageIdentity + }) + await until( + page, + async () => { + const w = window as any + w.management.postMessage({ type: 'discover' }) + return w.workerStatus?.phase === 'ready' + }, + 'ready owner diagnostics' + ) + await page.evaluate(() => { + const w = window as any + w.management.postMessage({ + type: 'stop', + instanceId: w.workerStatus.instanceId, + force: true, + restart: false, + }) + }) + await until( + page, + async () => + navigator.locks.request( + 'runme-storage-owner-lifetime-v1', + { ifAvailable: true }, + (lock) => Boolean(lock) + ), + 'old owner exit' + ) + await page.evaluate(async () => { + const w = window as any + const db = await new Promise((resolve, reject) => { + const request = indexedDB.open('runme-release-control') + request.onsuccess = () => resolve(request.result) + request.onerror = () => reject(request.error) + }) + const tx = db.transaction('state', 'readwrite') + const table = tx.objectStore('state') + // Keep this transaction active without changing any settings or notebook. + const keepAlive = () => { + table.get('control').onsuccess = keepAlive + } + keepAlive() + w.releaseDatabase = () => { + tx.abort() + db.close() + } + w.workerStatus = undefined + w.startOwner() + }) + await until( + page, + async () => { + const w = window as any + w.management.postMessage({ type: 'discover' }) + return ( + w.workerStatus?.phase === 'reading-release-settings' && + w.workerStatus.releaseDatabase?.active?.some( + (trace: any) => trace.phase === 'get-requested' + ) + ) + }, + 'queued settings get visible before runtime initialization' + ) + const queued = await page.evaluate(() => (window as any).workerStatus) + await page.evaluate(() => { + const w = window as any + w.management.postMessage({ + type: 'stop', + instanceId: w.workerStatus.instanceId, + force: false, + restart: false, + }) + }) + await until( + page, + async () => { + const w = window as any + w.management.postMessage({ type: 'discover' }) + return w.workerStatus?.phase === 'draining' + }, + 'graceful drain still observable' + ) + await page.evaluate(() => { + const w = window as any + w.management.postMessage({ + type: 'stop', + instanceId: w.workerStatus.instanceId, + force: true, + restart: false, + }) + }) + await until( + page, + async () => + navigator.locks.request( + 'runme-storage-owner-lifetime-v1', + { ifAvailable: true }, + (lock) => Boolean(lock) + ), + 'force close bypasses stalled initialization' + ) + await page.evaluate(() => { + const w = window as any + w.workerStatus = undefined + w.startOwner() + }) + await until( + page, + async () => { + const w = window as any + w.management.postMessage({ type: 'discover' }) + return ( + w.workerStatus?.phase === 'failed' && + w.workerStatus.releaseDatabase?.failures?.length > 0 + ) + }, + 'real queued get timeout reported' + ) + const timedOut = await page.evaluate(() => (window as any).workerStatus) + await page.evaluate(() => { + const w = window as any + w.management.postMessage({ + type: 'stop', + instanceId: w.workerStatus.instanceId, + force: true, + restart: false, + }) + }) + await until( + page, + async () => + navigator.locks.request( + 'runme-storage-owner-lifetime-v1', + { ifAvailable: true }, + (lock) => Boolean(lock) + ), + 'failed owner exit' + ) + await page.evaluate(() => { + const w = window as any + w.releaseDatabase() + w.workerStatus = undefined + w.startOwner() + }) + await until( + page, + async () => { + const w = window as any + w.management.postMessage({ type: 'discover' }) + return w.workerStatus?.phase === 'ready' + }, + 'replacement initializes after the blocking transaction ends' + ) + assert.equal( + await page.evaluate(() => (window as any).pageIdentity), + identity + ) + await writeFile( + `${output}/result.json`, + JSON.stringify( + { + passed: true, + queued, + timedOut, + replacement: await page.evaluate(() => (window as any).workerStatus), + }, + null, + 2 + ) + ) + console.log( + 'PASS: real IndexedDB get stall is observable; force close bypasses a stalled drain; replacement starts without page reload' + ) + } finally { + clearTimeout(watchdog) + await writeFile(`${output}/browser.log`, events.join('\n')) + await browser.close() + } +} +void main().catch((error) => { + console.error(error) + process.exitCode = 1 +}) diff --git a/docs-dev/CUJs/drive-sync-recovery.md b/docs-dev/CUJs/drive-sync-recovery.md index d5951566..cd4eec21 100644 --- a/docs-dev/CUJs/drive-sync-recovery.md +++ b/docs-dev/CUJs/drive-sync-recovery.md @@ -365,13 +365,19 @@ path may call page reload. Block a drain and verify diagnostics and force restar still work. Block release settings during startup and inspect the last reached phase. App upgrade/reload continues to require its separate preservation protocol. -The browser regression is `test/browser/storage-worker-reconnect.ts`, running -against Vite on localhost:5194 with disposable Chromium storage. Compile it with -`tsc --target es2022 --module nodenext --moduleResolution nodenext ---esModuleInterop --skipLibCheck --outDir test/browser/.generated - test/browser/storage-worker-reconnect.ts`, then run the generated script with -`CHROMIUM_PATH` pointing to an isolated Chromium for Testing binary. Evidence goes -to `test/browser/test-output/storage-worker-reconnect`. +Run `pnpm -C app run cuj:storage` for `storage-worker-reconnect.ts`. Its driver +starts the real Vite app on localhost:5194 and uses disposable Chromium storage. +Set `CHROMIUM_PATH` when using a Chromium for Testing binary instead of Playwright's +installed browser. Evidence goes to `test/browser/test-output/storage-worker-reconnect`. +The test also stops the worker without requesting a restart, then verifies that +each tab independently reconnects on its next save and joins the same replacement. + +`pnpm -C app run cuj:releases` also runs `storage-worker-startup.ts` against emitted +production assets and the Go release fixture server. It holds a real IndexedDB +transaction ahead of the worker's settings read. It verifies the pending +`get-requested` trace, force-close during a stalled graceful drain, a timeout trace, +and successful replacement after releasing the transaction without reloading the +page. Evidence goes to `test/browser/test-output/storage-worker-startup`. `storage-owner-recovery.ts` retains coverage of the older coordinated reload wire protocol for compatibility. It is not evidence for the new in-place restart path. diff --git a/docs-dev/design/20261027_storage_lifecycle.md b/docs-dev/design/20261027_storage_lifecycle.md index d3a59f76..1162e15a 100644 --- a/docs-dev/design/20261027_storage_lifecycle.md +++ b/docs-dev/design/20261027_storage_lifecycle.md @@ -33,7 +33,8 @@ and are never blindly retried. Editor batches alone have this idempotent contrac A BroadcastChannel management endpoint starts before lock acquisition and runtime imports. It reports the current startup phase and elapsed time without reading -IndexedDB. Once initialized, it includes host request diagnostics. Version UI +IndexedDB. It includes release-database traces before runtime initialization and +host request diagnostics afterward. Version UI always exposes discovery, stop, force stop, restart and force restart controls. Discovery describes responding Runme workers, not all workers in the browser. From 31b5472b7ab6069cc15724d2828fdeba124b948f Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 07:52:15 -0700 Subject: [PATCH 04/16] Reconnect editor retries even when worker stop notifications are missed Signed-off-by: Jeremy lewi --- app/src/storage/storageOwner.test.ts | 21 +++++ app/src/storage/storageOwnerClient.ts | 20 +++- app/test/browser/storage-worker-reconnect.ts | 91 +++++++++++++++++++ docs-dev/CUJs/drive-sync-recovery.md | 3 + docs-dev/design/20261027_storage_lifecycle.md | 2 + 5 files changed, 132 insertions(+), 5 deletions(-) diff --git a/app/src/storage/storageOwner.test.ts b/app/src/storage/storageOwner.test.ts index 49416904..8963b8af 100644 --- a/app/src/storage/storageOwner.test.ts +++ b/app/src/storage/storageOwner.test.ts @@ -531,6 +531,7 @@ it('blocks upgrade and mutation replay after a timeout until the original acknow ).rejects.toThrow('uncertain') await vi.advanceTimersByTimeAsync(300_000) await outcome + expect(client.needsReconnect).toBe(false) const { assertNoUpgradeBlockers } = await import('../releases/runtime') expect(assertNoUpgradeBlockers).toThrow('unconfirmed') await expect(client.request('save', ['retry', {}])).rejects.toThrow( @@ -549,6 +550,26 @@ it('blocks upgrade and mutation replay after a timeout until the original acknow expect(sent.filter((m) => m.method === 'save')).toHaveLength(1) }) +it('allows reconnect after an editor-save timeout without requiring a stop notification', async () => { + vi.useFakeTimers() + const channel = new MessageChannel() + ports.push(channel.port1 as unknown as MessagePort) + const client = new StorageOwnerClient( + channel.port2 as unknown as MessagePort, + async () => 'token' + ) + clients.push(client) + const outcome = expect( + client.request('commitEditorOperations', ['uri', 'notebook', []]) + ).rejects.toThrow('Pending operations remain') + await vi.advanceTimersByTimeAsync(300_000) + await outcome + expect(client.needsReconnect).toBe(true) + client.close() + const { assertNoUpgradeBlockers } = await import('../releases/runtime') + expect(assertNoUpgradeBlockers).not.toThrow() +}) + it('does not treat disconnect as acknowledgement of an outstanding write', async () => { const channel = new MessageChannel() ports.push(channel.port1 as unknown as MessagePort) diff --git a/app/src/storage/storageOwnerClient.ts b/app/src/storage/storageOwnerClient.ts index 983419a0..8027810c 100644 --- a/app/src/storage/storageOwnerClient.ts +++ b/app/src/storage/storageOwnerClient.ts @@ -38,6 +38,7 @@ export class StorageOwnerClient { private uncertain = new Set() private unresolvedCommit = false private closed = false + private retryConnection = false private unregisterDiagnostics?: () => void private unregisterReleaseBlocker = registerUpgradeBlocker(() => this.uncertain.size || this.unresolvedCommit @@ -125,9 +126,12 @@ export class StorageOwnerClient { return promise } - /** A failed transport can be replaced without discarding tab-owned saves. */ - get disconnected(): boolean { - return this.closed + /** Missing stop notifications must not strand a tab on a dead MessagePort. + * Safe request timeouts allow the next call to replace this connection. Keep + * generic uncertain mutations on their original correlation until recovery. + */ + get needsReconnect(): boolean { + return this.closed || this.retryConnection } request(method: string, args: unknown[] = []): Promise { @@ -153,6 +157,8 @@ export class StorageOwnerClient { return new Promise((resolve, reject) => { const timer = setTimeout( () => { + if (['hello', 'heartbeat', 'commitEditorOperations'].includes(method)) + this.retryConnection = true // Keep the original correlation until its late acknowledgement. An // upgrade retry must not turn a timed-out mutation into a new mutation. if ( @@ -517,13 +523,15 @@ export function createSharedNotebookStore( * named SharedWorker; the worker's lifetime lock remains the only writer gate. */ async function ensureConnected(): Promise { - if (connection.client.disconnected) needsConnection = true + if (connection.client.needsReconnect) needsConnection = true if (connecting) return connecting if (!needsConnection) { await connection.ready return } const operation = (async () => { + connection.client.onFailure = undefined + connection.client.close() connection = connect() const handshake = await connection.ready if (handshake.editorCommitVersion !== 1) @@ -662,7 +670,8 @@ export function createSharedNotebookStore( const methods = new Set(STORAGE_METHODS) return new Proxy(local, { get(target, key) { - if (key === 'ready') return connection.ready + if (key === 'ready') + return ensureConnected().then(() => connection.ready) if (key === 'setFilesystemStore' || key === 'stopSyncQueue') return () => {} if (key === 'operationLogSupportsConcurrentWriters') return () => true @@ -706,6 +715,7 @@ export function createSharedNotebookStore( LocalNotebooks['createOperationLogSaveStore'] >[1] = {} ) => { + await ensureConnected() const handshake = await connection.ready if (handshake.editorCommitVersion === 1) { const document = diff --git a/app/test/browser/storage-worker-reconnect.ts b/app/test/browser/storage-worker-reconnect.ts index 4a896373..4b37f395 100644 --- a/app/test/browser/storage-worker-reconnect.ts +++ b/app/test/browser/storage-worker-reconnect.ts @@ -54,6 +54,43 @@ async function main() { await page.evaluate(async () => { const w = window as any w.pageIdentity = crypto.randomUUID() + // Exercise a missed lifecycle notification without waiting five minutes + // for the real RPC timeout. All other timer behavior remains unchanged. + const nativeTimeout = window.setTimeout.bind(window) + w.setTimeout = ( + callback: TimerHandler, + delay: number, + ...args: any[] + ) => + nativeTimeout( + callback, + w.fastTimeout && delay === 300000 ? 1000 : delay, + ...args + ) + const NativeChannel = BroadcastChannel + w.BroadcastChannel = class extends NativeChannel { + constructor(name: string) { + super(name) + const descriptor = Object.getOwnPropertyDescriptor( + NativeChannel.prototype, + 'onmessage' + )! + Object.defineProperty(this, 'onmessage', { + set(handler) { + descriptor.set!.call(this, (event: MessageEvent) => { + if ( + w.dropManagement && + name === 'runme-storage-worker-control-v1' + ) { + w.droppedManagement = (w.droppedManagement ?? 0) + 1 + return + } + handler?.(event) + }) + }, + }) + } + } // Drop only a successful editor-commit reply, after the actual OPFS write. // The tab must keep the same batch and deduplicate it on the new owner. const NativeWorker = SharedWorker @@ -278,6 +315,59 @@ async function main() { ownerIds[1], 'Independent retries join the same worker' ) + await pages[1].evaluate(() => { + const w = window as any + w.dropManagement = true + w.fastTimeout = true + }) + await pages[0].evaluate(async () => { + const path = '/src/storage/storageWorkerControl.ts' + const { StorageWorkerControl } = await import(path) + await new Promise((resolve) => { + const control = new StorageWorkerControl((event: any) => { + if (event.type !== 'status') return + control.stop(event.status.instanceId, { force: true, restart: false }) + control.close() + resolve() + }) + control.discover() + }) + }) + await pages[0].waitForFunction( + () => (window as any).recovery.getSnapshot().phase === 'required' + ) + const missedNotification = await pages[1].evaluate(async () => { + const w = window as any + w.editor.cells[1].value = 'Saved after missing the stop notification' + let timedOut = false + try { + await w.view.save(w.uri, w.editor) + } catch (error) { + timedOut = String(error).includes('did not acknowledge an editor save') + } + if (!timedOut) throw new Error('Stale port should time out before retry') + await w.view.save(w.uri, w.editor) + return { + timedOut, + droppedManagement: w.droppedManagement, + instanceId: (await w.recovery.diagnose()).instanceId, + cells: (await w.store.loadOperationLogSnapshot(w.uri)).cells.map( + (cell: any) => cell.value + ), + } + }) + assert.ok(missedNotification.droppedManagement > 0) + assert.notEqual(missedNotification.instanceId, ownerIds[1]) + assert.deepEqual(missedNotification.cells, [ + 'Edited after reconnect', + 'Saved after missing the stop notification', + ]) + assert.deepEqual( + await Promise.all( + pages.map((page) => page.evaluate(() => (window as any).pageIdentity)) + ), + identities + ) await writeFile( `${output}/result.json`, JSON.stringify( @@ -291,6 +381,7 @@ async function main() { independentRetryAfterStop: true, finalCells, ownerIds, + missedNotification, }, null, 2 diff --git a/docs-dev/CUJs/drive-sync-recovery.md b/docs-dev/CUJs/drive-sync-recovery.md index cd4eec21..a7fafefb 100644 --- a/docs-dev/CUJs/drive-sync-recovery.md +++ b/docs-dev/CUJs/drive-sync-recovery.md @@ -371,6 +371,9 @@ Set `CHROMIUM_PATH` when using a Chromium for Testing binary instead of Playwrig installed browser. Evidence goes to `test/browser/test-output/storage-worker-reconnect`. The test also stops the worker without requesting a restart, then verifies that each tab independently reconnects on its next save and joins the same replacement. +It then drops management notifications for one tab, lets a save on its stale +port time out, and verifies that retrying replaces the connection and commits +the retained batch without losing either tab's edits. `pnpm -C app run cuj:releases` also runs `storage-worker-startup.ts` against emitted production assets and the Go release fixture server. It holds a real IndexedDB diff --git a/docs-dev/design/20261027_storage_lifecycle.md b/docs-dev/design/20261027_storage_lifecycle.md index 1162e15a..4bed39b6 100644 --- a/docs-dev/design/20261027_storage_lifecycle.md +++ b/docs-dev/design/20261027_storage_lifecycle.md @@ -13,6 +13,8 @@ lifetime lock, validate release admission, and accept the retained batches. Stopping the worker requires no tab acknowledgement or session-storage write. Each tab reconnects independently on its next request; idle tabs need not connect immediately, and there are no tab readiness or reconnect broadcasts. +An editor-save timeout also makes the next request replace the connection, so +recovery does not depend on receiving the old worker's stop notification. Only app upgrade/reload requires coordinated editor preservation. The tab owns the save baseline: the snapshot last loaded into the editor or From 1baaa8cd9d5a8de1cfcec30233e16015e8468890 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 07:56:41 -0700 Subject: [PATCH 05/16] Use reliable Ubuntu mirror for release browser checks Signed-off-by: Jeremy lewi --- .github/workflows/test.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bdaa5c15..9222217a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -125,6 +125,15 @@ jobs: src/releases/startupDiagnostics.test.ts src/pwa/loader-worker.test.ts src/components/ReleaseControls.test.tsx src/components/ReleaseRecoveryControls.test.tsx src/components/StorageWorkerControls.test.tsx src/releases/releaseCatalog.test.ts + - name: Use the primary Ubuntu package mirror + shell: bash + run: | + set -euo pipefail + # Match app-tests: the Azure mirror can stall browser dependencies for + # several minutes before apt falls back to the primary archive. + if [[ -f /etc/apt/apt-mirrors.txt ]]; then + sudo sed -i 's|http://azure.archive.ubuntu.com/ubuntu|https://archive.ubuntu.com/ubuntu|g' /etc/apt/apt-mirrors.txt + fi - run: pnpm -C app exec playwright-core install --with-deps chromium - name: Test independent storage reconnect if: matrix.baseline == 'current' From b56f2f412f1912519e04f257e3a44420511d2b50 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 10:41:06 -0700 Subject: [PATCH 06/16] Upgrade tabs independently with replayable local recovery Signed-off-by: Jeremy lewi --- .github/workflows/test.yaml | 2 + app/src/components/ReleaseControls.test.tsx | 8 +- app/src/components/ReleaseControls.tsx | 23 +- .../components/ReleaseRecoveryControls.tsx | 6 +- app/src/components/StorageRecoveryBanner.tsx | 16 + app/src/contexts/NotebookContext.tsx | 4 +- app/src/lib/appConsole/AppConsoleData.ts | 4 +- app/src/lib/notebookDataController.ts | 16 + app/src/releases/launcher.ts | 214 ++++++------- app/src/releases/prepare.ts | 4 +- app/src/releases/prepareTabReload.test.ts | 97 ++++++ app/src/releases/prepareTabReload.ts | 103 ++++++ app/src/releases/runtime.ts | 24 +- app/src/storage/editorSaveSession.test.ts | 86 +++++ app/src/storage/editorSaveSession.ts | 90 +++++- app/src/storage/storageOwner.test.ts | 35 +++ app/src/storage/storageOwnerClient.ts | 162 ++++++++-- app/src/storage/storageOwnerHost.ts | 22 +- app/src/storage/storageOwnerRuntime.ts | 1 - app/src/storage/tabRecoveryCheckpoint.test.ts | 56 ++++ app/src/storage/tabRecoveryCheckpoint.ts | 135 ++++++++ app/test/browser/controlled-releases.ts | 294 +++++++++++------- app/test/browser/storage-worker-reconnect.ts | 70 ++++- docs-dev/CUJs/controlled-app-releases.md | 17 +- docs-dev/design/20261027_storage_lifecycle.md | 105 ++++--- releaser/README.md | 6 +- 26 files changed, 1261 insertions(+), 339 deletions(-) create mode 100644 app/src/releases/prepareTabReload.test.ts create mode 100644 app/src/releases/prepareTabReload.ts create mode 100644 app/src/storage/tabRecoveryCheckpoint.test.ts create mode 100644 app/src/storage/tabRecoveryCheckpoint.ts diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9222217a..ba287a02 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -73,6 +73,8 @@ jobs: run: >- pnpm -C app exec vitest run src/storage/storageRecovery.test.ts src/storage/editorSaveSession.test.ts + src/storage/tabRecoveryCheckpoint.test.ts src/storage/storageOwner.test.ts + src/releases/prepareTabReload.test.ts src/storage/storageWorkerControl.test.ts src/lib/storageRecoveryDrafts.test.ts src/components/StorageRecoveryBanner.test.tsx --maxWorkers=2 --minWorkers=1 diff --git a/app/src/components/ReleaseControls.test.tsx b/app/src/components/ReleaseControls.test.tsx index bf8a2c6e..0c3e6e3e 100644 --- a/app/src/components/ReleaseControls.test.tsx +++ b/app/src/components/ReleaseControls.test.tsx @@ -97,12 +97,16 @@ describe('Information release controls', () => { expect( ( screen.getByRole('button', { - name: 'Upgrade all tabs', + name: 'Activate release and update this tab', }) as HTMLButtonElement ).disabled ).toBe(false) ) - fireEvent.click(screen.getByRole('button', { name: 'Upgrade all tabs' })) + fireEvent.click( + screen.getByRole('button', { + name: 'Activate release and update this tab', + }) + ) await waitFor(() => expect(api.upgrade).toHaveBeenCalledTimes(1)) }) it('keeps update errors visible in Information and permits retry', async () => { diff --git a/app/src/components/ReleaseControls.tsx b/app/src/components/ReleaseControls.tsx index 1dd38ac9..9a35ac61 100644 --- a/app/src/components/ReleaseControls.tsx +++ b/app/src/components/ReleaseControls.tsx @@ -160,9 +160,28 @@ function InstalledReleaseControls() { )} + {window.runmeRelease && + active && + window.runmeRelease.active.manifest.releaseId !== active.releaseId && ( +
+

+ This tab is running{' '} + {window.runmeRelease.active.manifest.webCommit.slice(0, 8)}. The + selected worker uses a different version; other tabs can update + independently. +

+ +
+ )}

Choose a channel to download its release in the background. Your - installed version stays active until you choose Upgrade all tabs. + selected worker version stays active until you activate the release. + Other tabs keep their current UI until you reload them individually.

)} {state.transition && ( diff --git a/app/src/components/ReleaseRecoveryControls.tsx b/app/src/components/ReleaseRecoveryControls.tsx index 4f1a6e28..7d2c97c4 100644 --- a/app/src/components/ReleaseRecoveryControls.tsx +++ b/app/src/components/ReleaseRecoveryControls.tsx @@ -47,8 +47,8 @@ export function ReleaseRecoveryControls() {

Recovery and rollback

Choose a published commit or release, even when installed release - settings cannot load. Activation waits for tabs to preserve edits and - storage to finish pending writes. + settings cannot load. Activation replaces the shared storage worker. + This tab preserves its edits before reloading; other tabs stay open.

+ + - ) } @@ -131,11 +147,7 @@ function InstalledReleaseControls() { state.selection.kind === 'channel' ? state.selection.name : 'Pinned release' return ( -
-

Application versions

+
Current channel
{selection}
@@ -206,20 +218,6 @@ function InstalledReleaseControls() { > Check for updates - {active && ( - - )}
{hasUpdate && (
+ ) } diff --git a/app/src/components/ReleaseRecoveryControls.test.tsx b/app/src/components/ReleaseRecoveryControls.test.tsx index c379497d..c4c98038 100644 --- a/app/src/components/ReleaseRecoveryControls.test.tsx +++ b/app/src/components/ReleaseRecoveryControls.test.tsx @@ -7,92 +7,194 @@ import { } from '@testing-library/react' import { afterEach, expect, it, vi } from 'vitest' import { ReleaseControls } from './ReleaseControls' +import { ReleaseRecoveryControls } from './ReleaseRecoveryControls' +import { showWorkspaceDocument } from '../lib/workspaceDocuments/workspaceDocumentController' import type { Control, Manifest } from '../releases/control' -const id = `${'a'.repeat(40)}-${'b'.repeat(64)}` + +vi.mock('../lib/workspaceDocuments/workspaceDocumentController', () => ({ + showWorkspaceDocument: vi.fn(), +})) +const installed = `${'a'.repeat(40)}-${'b'.repeat(64)}` +const id = `${'c'.repeat(40)}-${'d'.repeat(64)}` +function setup( + get = async () => + ({ active: { manifest: { releaseId: installed } } }) as Control +) { + const api = { + get, + check: vi.fn(async () => {}), + pin: vi.fn(async () => {}), + upgrade: vi.fn(), + retry: vi.fn(), + cancel: vi.fn(), + prune: vi.fn(), + prepareRecovery: vi.fn(async () => ({ releaseId: id }) as Manifest), + recover: vi.fn(async () => {}), + } + window.runmeVersions = api + return api +} afterEach(() => { cleanup() delete window.runmeVersions vi.unstubAllGlobals() + vi.clearAllMocks() }) -/** A failed or forever-pending installed-state read must not gate rescue actions. */ +async function selectCommit(value = id) { + fireEvent.click( + screen.getByRole('button', { name: /Choose another release/ }) + ) + fireEvent.click(screen.getByRole('button', { name: 'Enter a commit' })) + fireEvent.change( + screen.getByRole('textbox', { name: 'Commit SHA or release ID' }), + { target: { value } } + ) + fireEvent.click(screen.getByRole('button', { name: 'Review release' })) + await screen.findByRole('heading', { name: 'Pin this release?' }) +} + it.each(['failed', 'pending', 'missing'])( - 'keeps recovery reachable when settings are %s', + 'keeps Pin Release reachable when settings are %s', async (mode) => { - const prepareRecovery = vi.fn(async () => ({ releaseId: id }) as Manifest) - const recover = vi.fn(async () => {}) if (mode !== 'missing') - window.runmeVersions = { - get: - mode === 'failed' - ? () => Promise.reject(new Error('read timeout (get)')) - : () => new Promise(() => {}), - check: vi.fn(), - upgrade: vi.fn(), - retry: vi.fn(), - cancel: vi.fn(), - prune: vi.fn(), - prepareRecovery, - recover, - } - render() - expect( - screen - .getByRole('link', { name: 'Open recovery in another tab' }) - .getAttribute('target') - ).toBe('_blank') - fireEvent.change( - screen.getByRole('textbox', { name: 'Commit SHA or release ID' }), - { target: { value: id } } - ) - fireEvent.click( - screen.getByRole('button', { name: 'Prepare selected release' }) - ) - if (mode === 'missing') { - await screen.findByText(/This launcher requires/) - expect(prepareRecovery).not.toHaveBeenCalled() - } else { - await screen.findByText(/downloaded and verified/) - expect(prepareRecovery).toHaveBeenCalledWith({ - kind: 'pinned', - releaseId: id, - }) - expect(recover).not.toHaveBeenCalled() - fireEvent.click( - screen.getByRole('button', { name: 'Activate selected release' }) - ) - await waitFor(() => - expect(recover).toHaveBeenCalledWith({ kind: 'pinned', releaseId: id }) + setup( + mode === 'failed' + ? async () => { + throw new Error('read timeout (get)') + } + : () => new Promise(() => {}) ) - } - expect( - screen - .getByRole('link', { name: 'Open recovery in another tab' }) - .getAttribute('href') - ).toContain(`release=${id}`) + render() + fireEvent.click(screen.getByRole('button', { name: 'Pin Release' })) + expect(showWorkspaceDocument).toHaveBeenCalledWith('app://pin-release') + expect(screen.queryByText('Recovery and rollback')).toBeNull() + if (mode === 'failed') await screen.findByText(/read timeout/) } ) -it('keeps recovery navigation available during a hung download', async () => { - window.runmeVersions = { - get: () => new Promise(() => {}), - check: vi.fn(), - upgrade: vi.fn(), - retry: vi.fn(), - cancel: vi.fn(), - prune: vi.fn(), - prepareRecovery: () => new Promise(() => {}), - } - render() - fireEvent.change(screen.getByRole('textbox'), { target: { value: id } }) +it('pins the installed release only after review without downloading or activating', async () => { + const api = setup() + render() + const current = screen.getByRole('button', { + name: /Keep the installed release/, + }) + await waitFor(() => expect(current.hasAttribute('disabled')).toBe(false)) + fireEvent.click(current) + expect(api.pin).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Pin installed release' })) + await screen.findByRole('heading', { name: 'Release pinned' }) + expect(api.pin).toHaveBeenCalledOnce() + expect(api.check).not.toHaveBeenCalled() + expect(api.recover).not.toHaveBeenCalled() +}) +it('reviews an exact build, pins it, and activates only on a separate action', async () => { + const api = setup() + render() + await selectCommit() + expect(api.check).not.toHaveBeenCalled() + expect(api.prepareRecovery).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Pin and download' })) + await screen.findByRole('heading', { name: 'Release pinned' }) + expect(api.check).toHaveBeenCalledWith({ kind: 'pinned', releaseId: id }) + expect(api.recover).not.toHaveBeenCalled() + fireEvent.click( + screen.getByRole('button', { name: 'Activate release and update this tab' }) + ) + await waitFor(() => + expect(api.recover).toHaveBeenCalledWith({ kind: 'pinned', releaseId: id }) + ) +}) +it('browses published builds and reviews the exact selection without pinning yet', async () => { + const api = setup() + vi.stubGlobal( + 'fetch', + vi.fn( + async () => + new Response( + JSON.stringify({ format: 1, releases: [{ releaseId: id }] }) + ) + ) + ) + render() + fireEvent.click( + screen.getByRole('button', { name: /Choose another release/ }) + ) + fireEvent.click( + screen.getByRole('button', { name: 'Load published releases' }) + ) fireEvent.click( - screen.getByRole('button', { name: 'Prepare selected release' }) + await screen.findByRole('button', { name: /Commit cccccccc/ }) + ) + expect( + screen.getByRole('heading', { name: 'Pin this release?' }) + ).toBeTruthy() + expect(screen.getByText('d'.repeat(64))).toBeTruthy() + expect(api.check).not.toHaveBeenCalled() +}) +it('reports ambiguous commits without guessing a build', async () => { + const api = setup() + vi.stubGlobal( + 'fetch', + vi.fn( + async () => + new Response( + JSON.stringify({ + format: 1, + releases: [ + { releaseId: id }, + { releaseId: `${'c'.repeat(40)}-${'e'.repeat(64)}` }, + ], + }) + ) + ) + ) + render() + fireEvent.click( + screen.getByRole('button', { name: /Choose another release/ }) + ) + fireEvent.click(screen.getByRole('button', { name: 'Enter a commit' })) + fireEvent.change(screen.getByRole('textbox'), { + target: { value: 'ccccccc' }, + }) + fireEvent.click(screen.getByRole('button', { name: 'Review release' })) + expect((await screen.findByRole('alert')).textContent).toContain( + 'Multiple builds' + ) + expect(api.check).not.toHaveBeenCalled() +}) +it('retains a verified recovery download when settings cannot record the pin', async () => { + const api = setup(async () => { + throw new Error('read timeout (get)') + }) + api.check.mockRejectedValue(new Error('read timeout (get)')) + render() + await selectCommit() + fireEvent.click(screen.getByRole('button', { name: 'Pin and download' })) + await screen.findByRole('alert') + expect(api.prepareRecovery).toHaveBeenCalledWith({ + kind: 'pinned', + releaseId: id, + }) + expect(api.recover).not.toHaveBeenCalled() + expect(screen.queryByRole('heading', { name: 'Release pinned' })).toBeNull() + fireEvent.click( + screen.getByRole('button', { name: 'Recover with this release' }) ) await waitFor(() => - expect( - screen - .getByRole('link', { name: 'Open recovery in another tab' }) - .getAttribute('href') - ).toContain(id) + expect(api.recover).toHaveBeenCalledWith({ kind: 'pinned', releaseId: id }) ) +}) +it('keeps recovery navigation and diagnostics available during a hung download', async () => { + const api = setup(() => new Promise(() => {})) + api.prepareRecovery.mockImplementation(() => new Promise(() => {})) + render() + await selectCommit() + fireEvent.click(screen.getByRole('button', { name: 'Pin and download' })) + fireEvent.click(screen.getByText('Troubleshooting')) + expect( + screen + .getByRole('link', { name: /Open recovery in another tab/ }) + .getAttribute('href') + ).toContain(`release=${id}`) fireEvent.click( screen.getByRole('button', { name: 'Show release settings diagnostics' }) ) diff --git a/app/src/components/ReleaseRecoveryControls.tsx b/app/src/components/ReleaseRecoveryControls.tsx index 7d2c97c4..c3b86d9c 100644 --- a/app/src/components/ReleaseRecoveryControls.tsx +++ b/app/src/components/ReleaseRecoveryControls.tsx @@ -1,24 +1,55 @@ -import { useState } from 'react' +import { Button, ScrollArea } from '@radix-ui/themes' +import { useEffect, useState } from 'react' import type { Manifest } from '../releases/control' import { publishedReleaseIds, resolvePublishedRelease, } from '../releases/releaseCatalog' import { releaseDatabaseDiagnostics } from '../releases/releaseDiagnostics' +import { showWorkspaceDocument } from '../lib/workspaceDocuments/workspaceDocumentController' +import { VERSION_INFO_DOCUMENT_URI } from '../lib/workspaceDocuments/workspaceDocumentTypes' import '../releases/runtime' -/** Recovery navigation and release discovery never wait for settings reads. - * Downloading is independent; activation still requires safe storage ownership. +type Step = 'choose' | 'select' | 'review' | 'done' + +/** A tab-local wizard separates choosing, downloading and activating a release. + * Discovery and recovery navigation remain usable even if settings never reply. */ export function ReleaseRecoveryControls() { const api = window.runmeVersions + const [step, setStep] = useState('choose') + const [source, setSource] = useState<'browse' | 'commit'>('browse') + const [installed, setInstalled] = useState() + const [settingsError, setSettingsError] = useState('') const [input, setInput] = useState('') const [releases, setReleases] = useState([]) + const [listed, setListed] = useState(false) const [prepared, setPrepared] = useState() const [selected, setSelected] = useState('') const [busy, setBusy] = useState(false) const [message, setMessage] = useState('') + const [error, setError] = useState('') const [diagnostics, setDiagnostics] = useState('') + + useEffect(() => { + let disposed = false + if (api) + void api + .get() + .then((state) => { + if (!disposed) setInstalled(state.active?.manifest.releaseId) + }) + .catch(() => { + if (!disposed) + setSettingsError( + 'Installed release settings are unavailable. You can still choose another release.' + ) + }) + return () => { + disposed = true + } + }, [api]) + const recoveryUrl = new URL('/recovery.html', window.location.href) recoveryUrl.search = window.location.search recoveryUrl.hash = window.location.hash @@ -26,158 +57,408 @@ export function ReleaseRecoveryControls() { recoveryUrl.searchParams.delete('channel') recoveryUrl.searchParams.set('release', selected) } - /** Keep failures local and leave navigation available during pending commands. */ + /** Keep errors in this tab; navigation stays available during stalled work. */ const run = async (action: () => Promise) => { setBusy(true) + setError('') setMessage('') try { await action() - } catch (error) { - setMessage(String(error)) + } catch (failure) { + setError(failure instanceof Error ? failure.message : String(failure)) } finally { setBusy(false) } } + const back = () => { + setStep(step === 'review' && selected !== installed ? 'select' : 'choose') + setError('') + setMessage('') + setPrepared(undefined) + } + const review = (id: string) => { + setSelected(id) + setPrepared(undefined) + setStep('review') + setError('') + } + const choiceClass = + 'rounded-xl border border-nb-border bg-white p-5 text-left transition hover:border-violet-500 hover:bg-violet-50 focus-visible:outline focus-visible:outline-2 focus-visible:outline-violet-600 disabled:cursor-not-allowed disabled:opacity-50' + return ( -
-

Recovery and rollback

-

- Choose a published commit or release, even when installed release - settings cannot load. Activation replaces the shared storage worker. - This tab preserves its edits before reloading; other tabs stay open. -

- -
- - - {prepared && api?.recover && ( - + +
+ )} + {step === 'choose' && settingsError && ( +

{settingsError}

+ )} + + {step === 'select' && ( +
+
+ + +
+ {source === 'browse' ? ( + <> +

+ Choose a published build. Each commit can have more than one + build. +

+ + {listed && !releases.length && ( +

+ No published releases found. You can enter a full release ID + instead. +

+ )} +
+ {releases.map((id) => ( + + ))} +
+ + ) : ( + <> + +

+ Use at least 7 characters of a published commit, or a full + release ID. +

+ + + )} +
+ )} + + {(step === 'review' || step === 'done') && ( +
- Activate selected release - +

+ {step === 'done' + ? 'Release pinned' + : selected === installed + ? 'Keep this version?' + : 'Pin this release?'} +

+
+
Commit
+
+ {selected.split('-')[0]} +
+
Build
+
+ {selected.split('-')[1]} +
+
+

+ {selected === installed + ? 'Runme will stop following channel updates. Your running app stays unchanged.' + : 'Pinning downloads this build. Your running app stays unchanged until you activate it.'} +

+ {step === 'review' && ( + + )} + {prepared && error && ( +

+ The release was downloaded, but pinning did not finish. Retry or + use recovery below. +

+ )} + {((step === 'done' && selected !== installed) || + (prepared && error)) && + api?.recover && ( + <> +

+ Activating saves and reloads this tab. Other tabs stay open. +

+ + + )} +
)} - - Open recovery in another tab - - - - {releases.length > 0 && ( - - )} - {message &&

{message}

} - {diagnostics && ( - 
-          {diagnostics}
-
- )} -
+ {error} +

+ )} + {(busy || message) && ( +

+ {busy ? 'Working…' : message} +

+ )} +
+ {step !== 'choose' && step !== 'done' && ( + + )} + {step === 'done' && ( + + )} + +
+
+ + Troubleshooting + +
+

+ If this tab cannot finish, use the independent recovery page. +

+ +
+ +
+ {diagnostics && ( + 
+                {diagnostics}
+
+ )} +
+
+ + ) } diff --git a/app/src/components/StorageWorkerControls.test.tsx b/app/src/components/StorageWorkerControls.test.tsx index ed426e55..c1aa63b7 100644 --- a/app/src/components/StorageWorkerControls.test.tsx +++ b/app/src/components/StorageWorkerControls.test.tsx @@ -45,9 +45,8 @@ it('keeps targeted worker controls available when settings fail and a worker is await screen.findByText(/Release settings read timeout/) ).toBeTruthy() expect( - screen.getByRole('button', { name: 'Browse published releases' }) + screen.getByRole('button', { name: 'Pin Release' }) ).toBeTruthy() - expect(screen.getByLabelText('Commit SHA or release ID')).toBeTruthy() const row = screen.getByText('Instance: worker-two').closest('div')! const force = within(row).getByRole('button', { name: 'Force stop worker' }) expect(force.hasAttribute('disabled')).toBe(false) diff --git a/app/src/lib/workspaceDocuments/workspaceDocumentTypes.ts b/app/src/lib/workspaceDocuments/workspaceDocumentTypes.ts index 2ee27b80..306c4a6b 100644 --- a/app/src/lib/workspaceDocuments/workspaceDocumentTypes.ts +++ b/app/src/lib/workspaceDocuments/workspaceDocumentTypes.ts @@ -8,6 +8,7 @@ export const DRIVE_LINK_STATUS_DOCUMENT_URI = 'status://drive-link' export const DRIVE_SYNC_STATUS_DOCUMENT_URI = 'status://drive-sync' export const OIDC_STATUS_DOCUMENT_URI = 'status://oidc' export const VERSION_INFO_DOCUMENT_URI = 'app://version' +export const PIN_RELEASE_DOCUMENT_URI = 'app://pin-release' export const RUNNER_STATUS_DOCUMENT_URI = 'status://runners' const RUNNER_KERNELS_DOCUMENT_PREFIX = `${RUNNER_STATUS_DOCUMENT_URI}/` const RUNNER_KERNELS_DOCUMENT_SUFFIX = '/kernels' @@ -187,6 +188,7 @@ export function deriveWorkspaceDocumentTitle(uri: string): string { return 'Google Drive Sync Status' } if (isOidcStatusUri(documentUri)) return 'Runme authentication' + if (documentUri === PIN_RELEASE_DOCUMENT_URI) return 'Pin Release' if (isVersionInfoUri(documentUri)) { return 'Version Information' } diff --git a/app/test/browser/controlled-releases.ts b/app/test/browser/controlled-releases.ts index 094cdee2..68a9f079 100644 --- a/app/test/browser/controlled-releases.ts +++ b/app/test/browser/controlled-releases.ts @@ -628,6 +628,21 @@ async function main() { console.log('PASS: deploy B; refresh and new tabs retain A') await openVersions(one) await one.screenshot({ path: output + '/00-version-information.png' }) + const pinFlow = one.getByRole('button', { name: 'Pin Release', exact: true }) + const hasPinFlow = await pinFlow.count() > 0 + if (hasPinFlow) { + await pinFlow.click() + await one.getByRole('tab', { name: 'Pin Release', exact: true }).waitFor() + await one.screenshot({ path: output + '/00-pin-release-choices.png' }) + await one.getByRole('button', { name: /Choose another release/ }).click() + await one.getByRole('button', { name: 'Load published releases', exact: true }).click() + await one.getByRole('button', { name: new RegExp(`Commit ${expected.b.slice(0, 8)}`) }).click() + await one.screenshot({ path: output + '/00-pin-release-review.png' }) + assert.equal((await one.evaluate(() => (window as any).runmeVersions.get())).selection.kind, 'channel') + await one.getByRole('button', { name: 'Back', exact: true }).click() + await one.getByRole('button', { name: 'Back', exact: true }).click() + await one.getByRole('button', { name: /Keep the installed release/ }).click() + } await one .getByRole('button', { name: 'Pin installed release', exact: true }) .click() @@ -638,6 +653,7 @@ async function main() { s.selection.releaseId === a && !s.candidate ) + if (hasPinFlow) await one.getByRole('button', { name: 'Back to version information', exact: true }).click() await one .getByRole('button', { name: 'Check for updates', exact: true }) .click() diff --git a/docs-dev/CUJs/controlled-app-releases.md b/docs-dev/CUJs/controlled-app-releases.md index 0c9b202e..a03260f2 100644 --- a/docs-dev/CUJs/controlled-app-releases.md +++ b/docs-dev/CUJs/controlled-app-releases.md @@ -91,8 +91,12 @@ advances even when the stable application channel is pinned. ## Recovery while the app is already open - Inject a rejected or never-settling `runmeVersions.get()` after opening Version - Information. Recovery navigation, commit/release selection, published-release - discovery, and memory-only diagnostics must remain available. Also cover an + Information. The **Pin Release** button must remain available and open a + separate workspace tab. Users choose the installed release or select another + build by browsing or entering a commit. Reviewing a selection has no side + effects; pinning and activation are separate actions. Recovery navigation and + memory-only diagnostics remain reachable under **Troubleshooting**, even + while settings or a download are stalled. Also cover an older launcher without the new recovery API: the selected release is carried to the independent recovery page without navigating the editor tab away. - Preparing a build downloads and verifies it without accessing release settings. From c664cfa167f8e50c9198ae4fbc8919d00ea30c2e Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 17:17:45 -0700 Subject: [PATCH 14/16] Serve published release catalog in browser acceptance fixtures Signed-off-by: Jeremy lewi --- testing/release-server/main.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/testing/release-server/main.go b/testing/release-server/main.go index 8d62a53c..b0d351b9 100644 --- a/testing/release-server/main.go +++ b/testing/release-server/main.go @@ -99,6 +99,16 @@ func main() { mu.Unlock() w.WriteHeader(204) }) + // Release browsing exercises the same immutable IDs as the real publisher. + mux.HandleFunc("/releases/catalog.json", func(w http.ResponseWriter, r *http.Request) { + entries := make([]map[string]string, 0, len(releases)) + for _, rel := range releases { + parts := strings.SplitN(rel.id, "-", 2) + entries = append(entries, map[string]string{"releaseId": rel.id, "webCommit": parts[0], "buildId": parts[1]}) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{"format": 1, "releases": entries}) + }) mux.HandleFunc("/test/releases", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]string{"a": first.id, "b": second.id}) From 15bae3c3ac8882c59effa93beeac253c0acae0fa Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 18:36:53 -0700 Subject: [PATCH 15/16] Return from recovery and finish restored saves after reconnect Signed-off-by: Jeremy lewi --- app/src/releases/launcher.ts | 42 +++++++++++++-- app/src/storage/editorSaveSession.test.ts | 18 +++++-- app/src/storage/editorSaveSession.ts | 17 ++++-- app/src/storage/storageOwnerClient.ts | 4 +- app/test/browser/controlled-releases.ts | 64 +++++++++++++++++++---- docs-dev/CUJs/controlled-app-releases.md | 4 +- 6 files changed, 126 insertions(+), 23 deletions(-) diff --git a/app/src/releases/launcher.ts b/app/src/releases/launcher.ts index 490a9ea9..f637330e 100644 --- a/app/src/releases/launcher.ts +++ b/app/src/releases/launcher.ts @@ -29,11 +29,36 @@ import { } from './startupRecovery' const tabId = crypto.randomUUID() +const recovery = location.pathname === '/recovery.html' const panel = document.createElement('section') panel.id = 'runme-release-panel' panel.setAttribute('aria-label', 'Runme versions') panel.style.cssText = 'position:fixed;right:12px;bottom:12px;z-index:2147483647;background:white;color:#222;border:1px solid #aaa;border-radius:8px;padding:12px;max-width:480px;font:14px system-ui;max-height:80vh;overflow:auto' +// Recovery deliberately never imports the app. Keep its explanation and exit +// outside the dynamic controls so errors and collapsed details cannot hide them. +const recoveryIntro = document.createElement('header') +if (recovery) { + recoveryIntro.id = 'runme-recovery-intro' + const heading = document.createElement('h1') + heading.textContent = 'Runme recovery' + heading.style.cssText = 'font-size:20px;margin:0 0 8px' + const explanation = document.createElement('p') + explanation.textContent = + 'This page manages app versions. Your notebooks open in Runme when you return.' + const returnLink = document.createElement('a') + const appUrl = new URL(location.href) + appUrl.pathname = '/' + // Recovery requests have already been applied; returning must not replay them. + appUrl.searchParams.delete('release') + appUrl.searchParams.delete('channel') + returnLink.href = appUrl.pathname + appUrl.search + appUrl.hash + returnLink.textContent = 'Return to Runme' + returnLink.style.cssText = + 'display:inline-block;background:#1463d6;color:white;padding:10px 16px;border-radius:6px;font-weight:600;text-decoration:none' + recoveryIntro.append(heading, explanation, returnLink) + panel.append(recoveryIntro) +} const status = document.createElement('p') status.setAttribute('role', 'status') panel.append(status) @@ -61,8 +86,7 @@ let appReady = false let versionUiUsers = 0 let transitioning = false let tabUpdateAvailable = false -let details = location.pathname === '/recovery.html' -const recovery = details +let details = recovery let startupPrepared = false let recoveringStartup = false let deferredFonts = false @@ -635,7 +659,12 @@ async function render(): Promise { s.selection, ]) for (const node of [...panel.children]) - if (node !== status && node !== progressBar && node !== diagnostics.element) + if ( + node !== status && + node !== progressBar && + node !== diagnostics.element && + node !== recoveryIntro + ) node.remove() report( s.active @@ -850,7 +879,12 @@ function showStartupRecovery(error: unknown): void { stage: `release-settings-${error.stage}`, }) for (const node of [...panel.children]) - if (node !== status && node !== progressBar && node !== diagnostics.element) + if ( + node !== status && + node !== progressBar && + node !== diagnostics.element && + node !== recoveryIntro + ) node.remove() report( `${String(error instanceof Error ? error.message : error)} Retry loading stable, or choose another release below. Browser storage must respond before Runme can safely open notebooks.` diff --git a/app/src/storage/editorSaveSession.test.ts b/app/src/storage/editorSaveSession.test.ts index 543c7a9d..d85cf88f 100644 --- a/app/src/storage/editorSaveSession.test.ts +++ b/app/src/storage/editorSaveSession.test.ts @@ -99,15 +99,27 @@ it('restores a lost-ack batch and later typing without overwriting another tab await other.save(uri, other.initialNotebook) editor.cells[0].value = 'typed after submission' const checkpoint = JSON.parse(JSON.stringify(await first.checkpoint(editor))) + const recovered = vi.fn() const restored = new EditorSaveSession( uri, checkpoint.document, - commit, + vi + .fn() + .mockRejectedValueOnce(new Error('Worker unavailable during replay')) + .mockImplementation(commit), checkpoint.actorId, - checkpoint + checkpoint, + recovered ) expect(restored.initialNotebook.cells[0].value).toBe('typed after submission') - await restored.save(uri, restored.initialNotebook) + await expect(restored.save(uri, restored.initialNotebook)).rejects.toThrow( + 'Worker unavailable during replay' + ) + expect(recovered).not.toHaveBeenCalled() + // No new keystroke/save callback is required to persist the restored snapshot. + await restored.resume() + expect(recovered).toHaveBeenCalledOnce() + expect(restored.hasPending).toBe(false) expect(commit.mock.calls[2][2]).toEqual(originalBatch) const parsed = parseOperationLog(durable) expect(new Set(parsed.operations.map((op) => op.op_id)).size).toBe( diff --git a/app/src/storage/editorSaveSession.ts b/app/src/storage/editorSaveSession.ts index f0dafe66..70739009 100644 --- a/app/src/storage/editorSaveSession.ts +++ b/app/src/storage/editorSaveSession.ts @@ -60,7 +60,8 @@ export class EditorSaveSession { operations: RunmeOperation[] ) => Promise, private readonly actorId = `editor:${crypto.randomUUID()}`, - checkpoint?: EditorSaveCheckpoint + checkpoint?: EditorSaveCheckpoint, + private onRecovered?: () => void ) { const parsed = parseOperationLog(upgradeOperationLogToV2(document)) this.header = parsed.header @@ -148,11 +149,11 @@ export class EditorSaveSession { return this.uri } - /** Reconnect can finish a pending save without replacing the mounted model. */ + /** Replay the original batch first, then save later typing retained in the + * snapshot. A failed replay may have prevented that snapshot's diff entirely. + */ resume(): Promise { - const operation = this.queue.then(() => this.commitPending()) - this.queue = operation.catch(() => undefined) - return operation + return this.save(this.uri, this.latest) } /** Capture at call time: keystrokes during an acknowledgement wait belong to @@ -186,6 +187,12 @@ export class EditorSaveSession { this.pending = { operations, snapshot: next } await this.commitPending() }) + .then(() => { + // A checkpoint is consumed only after its full editor state is durable, + // including when the initial replay failed and reconnect completed it. + this.onRecovered?.() + this.onRecovered = undefined + }) .finally(() => { this.saving -= 1 }) diff --git a/app/src/storage/storageOwnerClient.ts b/app/src/storage/storageOwnerClient.ts index f4d2ba46..f8bca004 100644 --- a/app/src/storage/storageOwnerClient.ts +++ b/app/src/storage/storageOwnerClient.ts @@ -848,7 +848,10 @@ export function createSharedNotebookStore( await call('commitEditorOperations', args) }, checkpoint?.actorId ?? options.actorId, + checkpoint, checkpoint + ? () => acknowledgeTabCheckpoint(uri, checkpoint.actorId) + : undefined ) const state = { disposed: false, @@ -865,7 +868,6 @@ export function createSharedNotebookStore( if (checkpoint) { void session .save(uri, session.initialNotebook) - .then(() => acknowledgeTabCheckpoint(uri, checkpoint.actorId)) .catch((error) => storageRecovery.report( error instanceof Error ? error : new Error(String(error)) diff --git a/app/test/browser/controlled-releases.ts b/app/test/browser/controlled-releases.ts index 68a9f079..74b0e90e 100644 --- a/app/test/browser/controlled-releases.ts +++ b/app/test/browser/controlled-releases.ts @@ -628,20 +628,35 @@ async function main() { console.log('PASS: deploy B; refresh and new tabs retain A') await openVersions(one) await one.screenshot({ path: output + '/00-version-information.png' }) - const pinFlow = one.getByRole('button', { name: 'Pin Release', exact: true }) - const hasPinFlow = await pinFlow.count() > 0 + const pinFlow = one.getByRole('button', { + name: 'Pin Release', + exact: true, + }) + const hasPinFlow = (await pinFlow.count()) > 0 if (hasPinFlow) { await pinFlow.click() await one.getByRole('tab', { name: 'Pin Release', exact: true }).waitFor() await one.screenshot({ path: output + '/00-pin-release-choices.png' }) await one.getByRole('button', { name: /Choose another release/ }).click() - await one.getByRole('button', { name: 'Load published releases', exact: true }).click() - await one.getByRole('button', { name: new RegExp(`Commit ${expected.b.slice(0, 8)}`) }).click() + await one + .getByRole('button', { name: 'Load published releases', exact: true }) + .click() + await one + .getByRole('button', { + name: new RegExp(`Commit ${expected.b.slice(0, 8)}`), + }) + .click() await one.screenshot({ path: output + '/00-pin-release-review.png' }) - assert.equal((await one.evaluate(() => (window as any).runmeVersions.get())).selection.kind, 'channel') + assert.equal( + (await one.evaluate(() => (window as any).runmeVersions.get())) + .selection.kind, + 'channel' + ) await one.getByRole('button', { name: 'Back', exact: true }).click() await one.getByRole('button', { name: 'Back', exact: true }).click() - await one.getByRole('button', { name: /Keep the installed release/ }).click() + await one + .getByRole('button', { name: /Keep the installed release/ }) + .click() } await one .getByRole('button', { name: 'Pin installed release', exact: true }) @@ -653,7 +668,13 @@ async function main() { s.selection.releaseId === a && !s.candidate ) - if (hasPinFlow) await one.getByRole('button', { name: 'Back to version information', exact: true }).click() + if (hasPinFlow) + await one + .getByRole('button', { + name: 'Back to version information', + exact: true, + }) + .click() await one .getByRole('button', { name: 'Check for updates', exact: true }) .click() @@ -984,14 +1005,39 @@ async function main() { console.log('PASS: compatible offline rollback keeps edits made on B') const recovery = await context.newPage() pages.push(recovery) - await recovery.goto(base + '/recovery.html') + await recovery.goto( + base + '/recovery.html?session=recovery-return#notebooks' + ) await recovery .getByRole('button', { name: 'Check for updates', exact: true }) .waitFor({ timeout: 30000 }) assert.equal(await recovery.locator('#root').textContent(), '') await recovery.screenshot({ path: output + '/06-independent-recovery.png' }) + // Return explicitly enters the notebook app, preserving this tab's session. + // Leave the independent selector open afterward for broken-release recovery. + const returnLink = recovery.getByRole('link', { + name: 'Return to Runme', + exact: true, + }) + assert.equal( + await returnLink.getAttribute('href'), + '/?session=recovery-return#notebooks' + ) + await returnLink.click() + await ready(recovery) + assert.equal(new URL(recovery.url()).pathname, '/') + assert.equal( + new URL(recovery.url()).searchParams.get('session'), + 'recovery-return' + ) + assert.equal(new URL(recovery.url()).hash, '#notebooks') + await recovery.screenshot({ path: output + '/06-return-to-runme.png' }) + await recovery.goto(base + '/recovery.html') + await recovery + .getByRole('link', { name: 'Return to Runme', exact: true }) + .waitFor() console.log( - 'PASS: recovery selector works without importing the application' + 'PASS: recovery stays independent and explicitly returns to the app with the same session' ) if (process.env.RUNME_TEST_BROKEN) { await context.setOffline(false) diff --git a/docs-dev/CUJs/controlled-app-releases.md b/docs-dev/CUJs/controlled-app-releases.md index a03260f2..d6a3b8cf 100644 --- a/docs-dev/CUJs/controlled-app-releases.md +++ b/docs-dev/CUJs/controlled-app-releases.md @@ -63,7 +63,9 @@ Acceptance: - Historical rollback requires a durable flush of the initiating tab. Current peers refuse checkpoint reload into an app without `tabRecoveryFormat: 1`, retaining their mounted editors. - Historical launchers retain their original behavior. The frozen-baseline migration closes durable legacy peers before activation; the initiating historical app flushes locally before worker replacement. - After full offline readiness, offline restart uses B; offline rollback to A preserves edits made on B and leaves other tabs on their loaded UI versions. -- The recovery selector works independently of React/application imports. +- The recovery selector works independently of React/application imports, explains + that notebooks open in the app, and keeps **Return to Runme** visible. Returning + opens the app in the same tab with its session and hash preserved. - A checksum-valid but throwing app entry can be recovered through the selector after failed app tabs close, retaining current notebook content. From fb139dc4ea102473d292d9eb545f2a4da5d0d632 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Tue, 27 Oct 2026 18:48:17 -0700 Subject: [PATCH 16/16] Move version attention into the information icon and tab Signed-off-by: Jeremy lewi --- .github/workflows/test.yaml | 3 +- app/src/components/ReleaseControls.tsx | 11 +++ .../components/SidePanel/SidePanel.test.tsx | 25 +++++- app/src/components/SidePanel/SidePanel.tsx | 22 ++++- app/src/releases/launcher.ts | 17 +++- app/src/releases/releaseAttention.test.ts | 19 ++++ app/src/releases/releaseAttention.ts | 35 ++++++++ app/src/releases/runtime.test.ts | 2 +- app/src/releases/runtime.ts | 6 +- app/src/releases/useReleaseAttention.ts | 52 +++++++++++ app/test/browser/controlled-releases.ts | 90 +++++++++++++------ docs-dev/CUJs/controlled-app-releases.md | 7 +- 12 files changed, 251 insertions(+), 38 deletions(-) create mode 100644 app/src/releases/releaseAttention.test.ts create mode 100644 app/src/releases/releaseAttention.ts create mode 100644 app/src/releases/useReleaseAttention.ts diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index ba287a02..ed729686 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -121,7 +121,8 @@ jobs: - name: Test release controls and manifest discovery run: >- pnpm -C app exec vitest run - src/releases/control.test.ts src/releases/runtime.test.ts + src/releases/control.test.ts src/releases/runtime.test.ts src/releases/releaseAttention.test.ts + src/components/SidePanel/SidePanel.test.tsx src/releases/bootstrap.test.ts src/releases/launcherSelection.test.ts src/releases/loaderController.test.ts src/releases/releaseDatabase.test.ts src/releases/startupRecovery.test.ts src/releases/recoverControl.test.ts src/releases/controlChunks.test.ts src/releases/recoveryOwnerLock.test.ts src/storage/releaseOwnerFailure.test.ts src/releases/startupDiagnostics.test.ts diff --git a/app/src/components/ReleaseControls.tsx b/app/src/components/ReleaseControls.tsx index 724ad7e9..1f8cbfb8 100644 --- a/app/src/components/ReleaseControls.tsx +++ b/app/src/components/ReleaseControls.tsx @@ -1,3 +1,4 @@ +import { useReleaseAttention } from '../releases/useReleaseAttention' import { StorageWorkerControls } from './StorageWorkerControls' import { showWorkspaceDocument } from '../lib/workspaceDocuments/workspaceDocumentController' import { PIN_RELEASE_DOCUMENT_URI } from '../lib/workspaceDocuments/workspaceDocumentTypes' @@ -19,6 +20,7 @@ import type { OfflineDownloadStatus } from '../releases/runtime' * the common commands and can use the independent recovery page for rollback. */ export function ReleaseControls() { + const attention = useReleaseAttention() return ( <>
+ {attention && ( +

+ {attention.message}. Manage this tab’s version below. +

+ )}
diff --git a/app/src/components/SidePanel/SidePanel.test.tsx b/app/src/components/SidePanel/SidePanel.test.tsx index e42be4d7..a0e08ee9 100644 --- a/app/src/components/SidePanel/SidePanel.test.tsx +++ b/app/src/components/SidePanel/SidePanel.test.tsx @@ -1,12 +1,14 @@ // @vitest-environment jsdom import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ServiceAccountCredentialStatus } from '../../auth/googleServiceAccountImpersonation' const { showToastMock } = vi.hoisted(() => ({ showToastMock: vi.fn(), })) +afterEach(() => { delete window.runmeVersions }) + type PanelKey = | 'explorer' | 'documentation' @@ -529,6 +531,27 @@ describe('SidePanelToolbar drive status button', () => { expect(setCurrentDocMock).toHaveBeenCalledWith('app://version') }) + it('marks version attention on the information icon and clears it without reading storage', async () => { + let attention: import('../../releases/releaseAttention').ReleaseAttention | undefined = { + kind: 'update', message: 'An update is ready to activate', + } + const get = vi.fn() + window.runmeVersions = { get, attention: () => attention } as unknown as NonNullable + render() + const button = screen.getByRole('button', { name: 'Open Version Information' }) + await waitFor(() => expect(button.getAttribute('aria-description')).toBe('An update is ready to activate')) + expect(screen.getByTestId('version-attention-badge').getAttribute('data-kind')).toBe('update') + fireEvent.click(button) + expect(showDocumentMock).toHaveBeenCalledWith('app://version', { title: 'Version Information' }) + attention = {kind: 'reload', message: 'This tab is out of date'} + fireEvent(window, new Event('runme-release-change')) + await waitFor(() => expect(button.getAttribute('aria-description')).toBe('This tab is out of date')) + attention = undefined + fireEvent(window, new Event('runme-release-change')) + await waitFor(() => expect(screen.queryByTestId('version-attention-badge')).toBeNull()) + expect(get).not.toHaveBeenCalled() + }) + it('opens the runner status document from the toolbar', () => { runnersState = [ { diff --git a/app/src/components/SidePanel/SidePanel.tsx b/app/src/components/SidePanel/SidePanel.tsx index 6a9e3d6a..c2aba8c4 100644 --- a/app/src/components/SidePanel/SidePanel.tsx +++ b/app/src/components/SidePanel/SidePanel.tsx @@ -67,6 +67,8 @@ import AuthenticationSettingsPanel from '../AuthenticationSettings/Authenticatio import { SuggestionGraderSettings } from '../SuggestionGraderSettings/SuggestionGraderSettings' import { showToast } from '../../lib/toast' +import { useReleaseAttention } from '../../releases/useReleaseAttention' + const sideButtonBase = 'group side-btn' const sideButtonInactive = 'side-btn-inactive' @@ -238,6 +240,7 @@ function OpenDocumentsPanel() { } export function SidePanelToolbar() { + const versionAttention = useReleaseAttention() // Keep the indicator current even while the vault panel is closed. const vaultSnapshot = useSyncExternalStore(keyVault.subscribe, keyVault.getSnapshot) const vaultUnlocked = vaultSnapshot.status === 'unlocked' @@ -718,10 +721,25 @@ export function SidePanelToolbar() { }`} aria-pressed={versionInfoSelected} aria-label="Open Version Information" + aria-description={versionAttention?.message} onClick={handleVersionInfoClick} > - - Version Information + + + {versionAttention && ( + + )} + + + {versionAttention ? `Version Information — ${versionAttention.message}` : 'Version Information'} + diff --git a/app/src/releases/launcher.ts b/app/src/releases/launcher.ts index f637330e..d5ae5215 100644 --- a/app/src/releases/launcher.ts +++ b/app/src/releases/launcher.ts @@ -18,6 +18,7 @@ import { import { loaderController } from './loaderController' import { recoverUnreadableControl } from './recoverControl' import { withRecoveryOwner } from './recoveryOwnerLock' +import { type ReleaseAttention, releaseAttention } from './releaseAttention' import { resolvePublishedRelease } from './releaseCatalog' import { ReleaseControlError } from './releaseDatabase' import { releaseDatabaseDiagnostics } from './releaseDiagnostics' @@ -84,6 +85,8 @@ let ownerKey = '' let dismissedCandidate: string | undefined let appReady = false let versionUiUsers = 0 +let attentionUiUsers = 0 +let attention: ReleaseAttention | undefined let transitioning = false let tabUpdateAvailable = false let details = recovery @@ -131,6 +134,11 @@ function completeOffline(manifest: Manifest): Promise { /** Display diagnostics as text, never executable release metadata or user content. */ function report(value: unknown, nextProgress?: ReleaseProgress): void { + if (value instanceof Error) + attention = { + kind: 'error', + message: 'Version information needs attention', + } status.textContent = String(value instanceof Error ? value.message : value) progress = nextProgress progressBar.hidden = !progress @@ -173,8 +181,7 @@ function updatePanelVisibility(): void { appReady && versionUiUsers > 0 && !recovery && - !transitioning && - !tabUpdateAvailable + (attentionUiUsers > 0 || (!transitioning && !tabUpdateAvailable)) } /** A held tab lock is proof of liveness, including suspended tabs. No heartbeat @@ -642,6 +649,7 @@ async function render(): Promise { const generation = ++renderGeneration const s = await readControl() if (generation !== renderGeneration) return + attention = releaseAttention(s, runtime?.active.manifest.releaseId) transitioning = Boolean(s.transition) tabUpdateAvailable = Boolean( runtime && @@ -978,14 +986,17 @@ window.runmeVersions = { await tick() }, uiVersion: 1, - registerVersionUi: () => { + attention: () => attention, + registerVersionUi: (options) => { versionUiUsers++ + if (options?.attention) attentionUiUsers++ updatePanelVisibility() let released = false return () => { if (released) return released = true versionUiUsers-- + if (options?.attention) attentionUiUsers-- updatePanelVisibility() } }, diff --git a/app/src/releases/releaseAttention.test.ts b/app/src/releases/releaseAttention.test.ts new file mode 100644 index 00000000..184568dc --- /dev/null +++ b/app/src/releases/releaseAttention.test.ts @@ -0,0 +1,19 @@ +import { expect, it } from 'vitest' + +import type { Control } from './control' +import { releaseAttention } from './releaseAttention' + +it('distinguishes a downloaded update, an outdated tab, activation and a healthy pin', () => { + const state = { + active: { manifest: { releaseId: 'installed' } }, + selection: { kind: 'pinned', releaseId: 'installed' }, + } as Control + expect(releaseAttention(state, 'installed')).toBeUndefined() + state.candidate = { releaseId: 'new' } as Control['candidate'] + expect(releaseAttention(state, 'installed')?.kind).toBe('update') + expect(releaseAttention(state, 'old')?.kind).toBe('reload') + state.transition = { phase: 'handoff' } as Control['transition'] + expect(releaseAttention(state, 'old')?.kind).toBe('busy') + state.ownerError = { message: 'Failed' } as Control['ownerError'] + expect(releaseAttention(state, 'old')?.kind).toBe('error') +}) diff --git a/app/src/releases/releaseAttention.ts b/app/src/releases/releaseAttention.ts new file mode 100644 index 00000000..0c386b75 --- /dev/null +++ b/app/src/releases/releaseAttention.ts @@ -0,0 +1,35 @@ +import type { Control } from './control' + +export type ReleaseAttention = { + kind: 'update' | 'reload' | 'busy' | 'error' + message: string +} + +/** Describe actionable version state without fetching or activating a release. */ +export function releaseAttention( + state: Control, + runningReleaseId?: string +): ReleaseAttention | undefined { + if (state.ownerError) + return { kind: 'error', message: 'Version activation needs attention' } + if (state.transition) + return { + kind: 'busy', + message: + state.transition.phase === 'prepare' + ? 'An earlier upgrade needs attention' + : 'Version activation in progress', + } + if ( + runningReleaseId && + state.active && + runningReleaseId !== state.active.manifest.releaseId + ) + return { kind: 'reload', message: 'This tab is out of date' } + if ( + state.candidate && + state.candidate.releaseId !== state.active?.manifest.releaseId + ) + return { kind: 'update', message: 'An update is ready to activate' } + return undefined +} diff --git a/app/src/releases/runtime.test.ts b/app/src/releases/runtime.test.ts index cbc4abeb..c3e3078c 100644 --- a/app/src/releases/runtime.test.ts +++ b/app/src/releases/runtime.test.ts @@ -15,7 +15,7 @@ it('explicitly hands version controls to the app and releases them on cleanup', registerVersionUi, } as unknown as NonNullable const cleanup = adoptLegacyVersionUi() - expect(registerVersionUi).toHaveBeenCalledOnce() + expect(registerVersionUi).toHaveBeenCalledWith({ attention: true }) cleanup() expect(release).toHaveBeenCalledOnce() }) diff --git a/app/src/releases/runtime.ts b/app/src/releases/runtime.ts index 95fc237a..a442a68e 100644 --- a/app/src/releases/runtime.ts +++ b/app/src/releases/runtime.ts @@ -6,6 +6,7 @@ import type { ReleaseProgress, Selection, } from './control' +import type { ReleaseAttention } from './releaseAttention' import type { releaseDatabaseDiagnostics } from './releaseDiagnostics' /** The independent launcher owns worker activation; React registers this tab’s @@ -39,7 +40,8 @@ declare global { prepareRecovery?: (selection: Selection) => Promise recover?: (selection: Selection) => Promise uiVersion?: number - registerVersionUi?: () => () => void + registerVersionUi?: (options?: { attention?: boolean }) => () => void + attention?: () => ReleaseAttention | undefined status?: () => string progress?: () => ReleaseProgress | undefined offline?: (releaseId: string) => OfflineDownloadStatus | undefined @@ -57,7 +59,7 @@ declare global { */ export function adoptLegacyVersionUi(): () => void { if (window.runmeVersions?.registerVersionUi) - return window.runmeVersions.registerVersionUi() + return window.runmeVersions.registerVersionUi({ attention: true }) if (window.runmeVersions?.uiVersion) return () => {} const panel = document.getElementById('runme-release-panel') const root = document.getElementById('root') diff --git a/app/src/releases/useReleaseAttention.ts b/app/src/releases/useReleaseAttention.ts new file mode 100644 index 00000000..d2305545 --- /dev/null +++ b/app/src/releases/useReleaseAttention.ts @@ -0,0 +1,52 @@ +import { useEffect, useState } from 'react' + +import { type ReleaseAttention, releaseAttention } from './releaseAttention' +import './runtime' + +/** Current launchers publish a memory-only snapshot, including read failures. + * Older launchers fall back to bounded, non-overlapping settings reads. + */ +export function useReleaseAttention(): ReleaseAttention | undefined { + const api = window.runmeVersions + const [attention, setAttention] = useState() + useEffect(() => { + if (!api) return + let disposed = false + let reading = false + const refresh = async () => { + if (disposed || reading) return + if (api.attention) { + setAttention(api.attention()) + return + } + reading = true + try { + const state = await api.get() + if (!disposed) + setAttention( + releaseAttention( + state, + window.runmeRelease?.active.manifest.releaseId + ) + ) + } catch { + if (!disposed) + setAttention({ + kind: 'error', + message: 'Version information needs attention', + }) + } finally { + reading = false + } + } + void refresh() + const timer = window.setInterval(() => void refresh(), 5000) + window.addEventListener('runme-release-change', refresh) + return () => { + disposed = true + window.clearInterval(timer) + window.removeEventListener('runme-release-change', refresh) + } + }, [api]) + return attention +} diff --git a/app/test/browser/controlled-releases.ts b/app/test/browser/controlled-releases.ts index 74b0e90e..65162cc7 100644 --- a/app/test/browser/controlled-releases.ts +++ b/app/test/browser/controlled-releases.ts @@ -697,6 +697,17 @@ async function main() { s.selection.name === 'stable' && s.candidate?.releaseId === expected.b ) assert.equal(await active(one), a) + if (!process.env.RUNME_RELEASE_BASELINE) { + await one.getByTestId('version-attention-badge').waitFor() + assert.equal( + await one + .getByTestId('version-attention-badge') + .getAttribute('data-kind'), + 'update' + ) + await one.locator('#runme-release-panel').waitFor({ state: 'hidden' }) + await one.screenshot({ path: output + '/00-version-attention.png' }) + } console.log( 'PASS: Information view owns controls; exact pin and channel selection never activate implicitly' ) @@ -809,6 +820,25 @@ async function main() { (s) => !s.transition && s.active?.manifest.releaseId === b ) assert.equal(await active(two), a, 'Saving must not reload the older UI') + await two.waitForFunction( + () => + document + .querySelector('[data-testid="version-attention-badge"]') + ?.getAttribute('data-kind') === 'reload' + ) + await two.locator('#runme-release-panel').waitFor({ state: 'hidden' }) + await two + .getByRole('button', { name: 'Open Version Information', exact: true }) + .click() + await two + .getByRole('button', { + name: 'Preserve edits and reload this tab', + exact: true, + }) + .waitFor() + await two.screenshot({ + path: output + '/02-outdated-tab-information.png', + }) // A local backup failure must not reload this page or roll back the worker. const thirdId = await three.evaluate( () => (window as any).runmeRelease.tabId @@ -1013,32 +1043,40 @@ async function main() { .waitFor({ timeout: 30000 }) assert.equal(await recovery.locator('#root').textContent(), '') await recovery.screenshot({ path: output + '/06-independent-recovery.png' }) - // Return explicitly enters the notebook app, preserving this tab's session. - // Leave the independent selector open afterward for broken-release recovery. - const returnLink = recovery.getByRole('link', { - name: 'Return to Runme', - exact: true, - }) - assert.equal( - await returnLink.getAttribute('href'), - '/?session=recovery-return#notebooks' - ) - await returnLink.click() - await ready(recovery) - assert.equal(new URL(recovery.url()).pathname, '/') - assert.equal( - new URL(recovery.url()).searchParams.get('session'), - 'recovery-return' - ) - assert.equal(new URL(recovery.url()).hash, '#notebooks') - await recovery.screenshot({ path: output + '/06-return-to-runme.png' }) - await recovery.goto(base + '/recovery.html') - await recovery - .getByRole('link', { name: 'Return to Runme', exact: true }) - .waitFor() - console.log( - 'PASS: recovery stays independent and explicitly returns to the app with the same session' - ) + // The frozen pre-bootstrap service worker still serves its old root + // recovery page offline. Only the current launcher contains this new UI. + if (!historicalApp) { + // Return explicitly enters the notebook app, preserving this tab's session. + // Leave the independent selector open afterward for broken-release recovery. + const returnLink = recovery.getByRole('link', { + name: 'Return to Runme', + exact: true, + }) + assert.equal( + await returnLink.getAttribute('href'), + '/?session=recovery-return#notebooks' + ) + await returnLink.click() + await ready(recovery) + assert.equal(new URL(recovery.url()).pathname, '/') + assert.equal( + new URL(recovery.url()).searchParams.get('session'), + 'recovery-return' + ) + assert.equal(new URL(recovery.url()).hash, '#notebooks') + await recovery.screenshot({ path: output + '/06-return-to-runme.png' }) + await recovery.goto(base + '/recovery.html') + await recovery + .getByRole('link', { name: 'Return to Runme', exact: true }) + .waitFor() + console.log( + 'PASS: recovery stays independent and explicitly returns to the app with the same session' + ) + } else { + console.log( + 'PASS: frozen historical recovery remains independent of app imports' + ) + } if (process.env.RUNME_TEST_BROKEN) { await context.setOffline(false) await fetch(base + '/test/promote?release=broken', { method: 'POST' }) diff --git a/docs-dev/CUJs/controlled-app-releases.md b/docs-dev/CUJs/controlled-app-releases.md index d6a3b8cf..bdaffbfa 100644 --- a/docs-dev/CUJs/controlled-app-releases.md +++ b/docs-dev/CUJs/controlled-app-releases.md @@ -21,7 +21,10 @@ Acceptance: - The floating launcher disappears after startup only when the loaded app explicitly registers its version controls. Older clients retain the fallback controls. With a current app, the left Information icon opens version controls - inside the Version Information document. + inside the Version Information document. A badge and descriptive tooltip on the + Information icon signal a downloaded update, an outdated tab, or a version + problem. Apps supporting this affordance keep the overlay hidden during + updates and handoffs; actions and status remain in Version Information. - Startup diagnostics expose timestamped stage, duration, and waiting/failure events before the app starts. The trace is bounded, stays in memory, and does not include notebook data, credentials, or URL query parameters. @@ -63,7 +66,7 @@ Acceptance: - Historical rollback requires a durable flush of the initiating tab. Current peers refuse checkpoint reload into an app without `tabRecoveryFormat: 1`, retaining their mounted editors. - Historical launchers retain their original behavior. The frozen-baseline migration closes durable legacy peers before activation; the initiating historical app flushes locally before worker replacement. - After full offline readiness, offline restart uses B; offline rollback to A preserves edits made on B and leaves other tabs on their loaded UI versions. -- The recovery selector works independently of React/application imports, explains +- The current recovery selector works independently of React/application imports, explains that notebooks open in the app, and keeps **Return to Runme** visible. Returning opens the app in the same tab with its session and hash preserved. - A checksum-valid but throwing app entry can be recovered through the selector