From b504b8387dcf8e2cebe1dd28674e6f66d9eec115 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 03:27:34 +0000 Subject: [PATCH 1/9] fix(frontend): validate persisted undo history before applying state Implement the admitted FE-A05 subset on exact ca9edde. Decode the existing unversioned record from unknown, checking nested actions, opaque nonblank IDs, nonnegative safe millisecond timestamps, the existing 32-entry bound and valid cursor relationships. Keep the omitted-position legacy fallback. Reject invalid/oversized records as a whole, log UNDO_STORE_LOAD_FAILED, and preserve their bytes. Unavailable or rejected storage disables writes for that store instance, including later in-memory pushes and clear; skipped persistence reports UNDO_STORE_SAVE_FAILED. No schema migration or recovery writes. Move unchanged operation bodies into an injected store factory, with per-store callback ownership. Keep the app singleton and exported action/entry types. No timeline or Git cleanup implementation changes. Rejected entries never reach an aging callback; fresh user actions retain their existing in-memory behavior. Validation on pinned Node 24.12.0/npm 11.6.2: - 35 focused decoder/store tests and 582 aggregate frontend tests pass. - Full lint, TypeScript, production build, staged critical/a11y/traceability gates and diff whitespace pass. - Existing stale Browserslist data notice remains; no dependency changes. - Source comparison confirms all operation bodies preserved after logger and constant renaming. Tests use recording callbacks, never permanent Git deletion. The evidence report records exact policy and limits. This does not qualify GUI, native/model execution, cold browser reopen, other persistence owners or full FE-A05 schema/version migration. Separate scanner/fixture branches are preserved; write sets are disjoint. Independent review remains required before integration. --- .../2026-10-04-undo-persistence-validation.md | 53 ++++ src/stores/createUndoStore.ts | 220 ++++++++++++++++ src/stores/undoPersistence.test.ts | 244 ++++++++++++++++++ src/stores/undoPersistence.ts | 61 +++++ src/stores/undoStore.ts | 236 +---------------- 5 files changed, 584 insertions(+), 230 deletions(-) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-undo-persistence-validation.md create mode 100644 src/stores/createUndoStore.ts create mode 100644 src/stores/undoPersistence.test.ts create mode 100644 src/stores/undoPersistence.ts diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-undo-persistence-validation.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-undo-persistence-validation.md new file mode 100644 index 000000000..37d1450ce --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-undo-persistence-validation.md @@ -0,0 +1,53 @@ +# FE-A05 bounded undo persistence validation + +This slice addresses the existing browser-persistence obligation retained by M4 +and DA-05. It does not close FE-A05 for other storage owners, introduce a record +version, migrate data, or qualify desktop cold reopen. + +## Admission and behavior + +The parent admitted whole-record rejection for malformed/oversized undo data, +retaining stored bytes and invoking no recovery/deletion callbacks. The existing +`pantograph-unified-undo` key, unversioned valid shape and 32-entry limit remain. +Opaque entry IDs and commit hashes must be nonblank strings; no hash format or +new string-length restriction is imposed. Timestamps must be nonnegative safe +integers, as produced by the existing millisecond timestamp writer. Positions +must be integers from -1 through the last entry; an omitted legacy position +still defaults to the last entry. No wall-clock freshness or ordering policy is +added. Nested invalid entries reject the whole record instead of being filtered. + +Rejected or unreadable storage initializes an empty in-memory history and logs +`UNDO_STORE_LOAD_FAILED` through the existing application logger. Persistence +remains disabled for that store instance: later actions and clear operate in +memory and log `UNDO_STORE_SAVE_FAILED` instead of replacing the stored bytes. +There is no automatic recovery write, removal or schema migration. Valid or +absent records retain normal writes and error reporting. + +The store factory owns its callbacks and receives storage/logging dependencies; +the application retains its existing singleton and exported action/entry types. +The operation bodies are unchanged after logger/constant renaming. Valid full +histories still age their oldest action only on a new push; the timeline's Git +cleanup owner is not changed. Rejected entries never enter that callback path. +New user actions may form and age a fresh in-memory history under the existing +policy even when persistence remains blocked. + +## Evidence and limits + +The base `ca9edde6850cd58ade0b7e534bb4f58e704c2c64` restored a null nested action +that threw in undo, accepted cursor 99 while reporting canUndo, and loaded 33 +entries. The read-only probe used isolated storage and no timeline/Git callback. + +Focused decoder/store tests use recording callbacks only. They cover malformed +and mixed records, invalid JSON, bounds, opaque identifiers, legacy cursor +fallback, repeated restore, unavailable storage, byte preservation after new +pushes/clear, normal undo/redo and reread, redo truncation, valid capacity aging, +new-history aging after rejection, callback failure, save failure and isolated +stores. No permanent Git-object deletion or Tauri invocation occurs in tests. + +The 35 focused tests and all 582 discovered frontend tests pass on pinned Node +24.12.0/npm 11.6.2. TypeScript, full lint, production build and the staged +critical-lint/accessibility/traceability gate pass. The final scope is five paths +with zero mapped decision impacts; semantic review remains required. The production build emits the existing stale Browserslist-data notice. +No dependency manifests, audit gates, native downloads or review branches change. +This is deterministic frontend contract evidence, not GUI/native/model or full +persistence-migration acceptance. Independent review remains with the integrator. diff --git a/src/stores/createUndoStore.ts b/src/stores/createUndoStore.ts new file mode 100644 index 000000000..6848b97da --- /dev/null +++ b/src/stores/createUndoStore.ts @@ -0,0 +1,220 @@ +import { writable, get } from 'svelte/store'; +import { decodeUndoState, MAX_UNDO_HISTORY, UNDO_STORAGE_KEY } from './undoPersistence.ts'; +import type { UndoableAction, UndoEntry, UndoState } from './undoPersistence.ts'; + +interface UndoStoreOptions { + storage: () => Pick; + log: (type: string, payload: Record, severity?: 'info' | 'warn' | 'error') => void; +} + +// Generate simple unique ID +function generateId(): string { + return `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`; +} + +// Callback registry for handling aged-out actions and undo/redo +type AgedActionCallback = (action: UndoableAction) => Promise; +type UndoCallback = (action: UndoableAction) => void; +type RedoCallback = (action: UndoableAction) => void; + +export function createUndoStore({ storage, log }: UndoStoreOptions) { + // Rejected/unavailable bytes are never replaced by fallback or later edits. + let canPersist = false; + let initial: UndoState = { history: [], position: -1 }; + try { + const stored = storage().getItem(UNDO_STORAGE_KEY); + if (stored === null) { + canPersist = true; + } else { + const decoded = decodeUndoState(JSON.parse(stored)); + if (decoded.ok) { + initial = decoded.state; + canPersist = true; + } else { + log('UNDO_STORE_LOAD_FAILED', { error: decoded.reason }, 'warn'); + } + } + } catch (error) { + log('UNDO_STORE_LOAD_FAILED', { error: String(error) }, 'warn'); + } + + function saveToStorage(state: UndoState): void { + if (!canPersist) { + log('UNDO_STORE_SAVE_FAILED', { error: 'Undo storage was rejected or unavailable; existing bytes preserved.' }, 'warn'); + return; + } + try { + storage().setItem(UNDO_STORAGE_KEY, JSON.stringify(state)); + } catch (error) { + log('UNDO_STORE_SAVE_FAILED', { error: String(error) }, 'warn'); + } + } + + let agedActionCallback: AgedActionCallback | null = null; + const undoCallbacks = new Map(); + const redoCallbacks = new Map(); + const { subscribe, set } = writable(initial); + + const store = { + subscribe, + + /** + * Register callback for when actions age past 32 steps + */ + onAgedAction(callback: AgedActionCallback): void { + agedActionCallback = callback; + }, + + /** + * Register undo handler for specific action type + */ + onUndo(type: UndoableAction['type'], callback: UndoCallback): void { + undoCallbacks.set(type, callback); + }, + + /** + * Register redo handler for specific action type + */ + onRedo(type: UndoableAction['type'], callback: RedoCallback): void { + redoCallbacks.set(type, callback); + }, + + /** + * Push a new action to the undo history. + * If history exceeds MAX_UNDO_HISTORY, oldest action is permanently executed. + */ + async push(action: UndoableAction): Promise { + const state = get({ subscribe }); + + // If we're not at the end of history, truncate future actions + // (user did something new after undoing) + const newHistory = state.history.slice(0, state.position + 1); + + // Add new action + const entry: UndoEntry = { + id: generateId(), + action, + timestamp: Date.now(), + }; + newHistory.push(entry); + + // Check if we need to age out the oldest action + if (newHistory.length > MAX_UNDO_HISTORY) { + const aged = newHistory.shift(); + if (aged && agedActionCallback) { + try { + await agedActionCallback(aged.action); + log('UNDO_ACTION_AGED', { type: aged.action.type }); + } catch (e) { + log('UNDO_AGED_ACTION_FAILED', { error: String(e) }, 'error'); + } + } + } + + const newState: UndoState = { + history: newHistory, + position: newHistory.length - 1, + }; + + set(newState); + saveToStorage(newState); + log('UNDO_PUSH', { type: action.type, historySize: newHistory.length }); + }, + + /** + * Undo the most recent action + */ + undo(): boolean { + const state = get({ subscribe }); + + if (state.position < 0 || state.history.length === 0) { + return false; + } + + const entry = state.history[state.position]; + if (!entry) return false; + + // Call undo handler + const handler = undoCallbacks.get(entry.action.type); + if (handler) { + handler(entry.action); + } + + const newState: UndoState = { + ...state, + position: state.position - 1, + }; + + set(newState); + saveToStorage(newState); + log('UNDO_EXECUTED', { type: entry.action.type, position: newState.position }); + return true; + }, + + /** + * Redo a previously undone action + */ + redo(): boolean { + const state = get({ subscribe }); + + if (state.position >= state.history.length - 1) { + return false; + } + + const nextPosition = state.position + 1; + const entry = state.history[nextPosition]; + if (!entry) return false; + + // Call redo handler + const handler = redoCallbacks.get(entry.action.type); + if (handler) { + handler(entry.action); + } + + const newState: UndoState = { + ...state, + position: nextPosition, + }; + + set(newState); + saveToStorage(newState); + log('REDO_EXECUTED', { type: entry.action.type, position: newState.position }); + return true; + }, + + /** + * Check if undo is available + */ + canUndo(): boolean { + const state = get({ subscribe }); + return state.position >= 0 && state.history.length > 0; + }, + + /** + * Check if redo is available + */ + canRedo(): boolean { + const state = get({ subscribe }); + return state.position < state.history.length - 1; + }, + + /** + * Clear all history + */ + clear(): void { + const newState: UndoState = { history: [], position: -1 }; + set(newState); + saveToStorage(newState); + log('UNDO_HISTORY_CLEARED', {}); + }, + + /** + * Get current state for debugging + */ + getState(): UndoState { + return get({ subscribe }); + }, + }; + + return store; +} diff --git a/src/stores/undoPersistence.test.ts b/src/stores/undoPersistence.test.ts new file mode 100644 index 000000000..cbb235daa --- /dev/null +++ b/src/stores/undoPersistence.test.ts @@ -0,0 +1,244 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createUndoStore } from './createUndoStore.ts'; +import { decodeUndoState, MAX_UNDO_HISTORY, UNDO_STORAGE_KEY } from './undoPersistence.ts'; +import type { UndoEntry, UndoableAction } from './undoPersistence.ts'; + +const entry = (id = 'opaque-entry', hash = 'opaque-commit'): UndoEntry => ({ + id, action: { type: 'COMMIT_SOFT_DELETE', hash }, timestamp: 0, +}); +const state = (history = [entry()], position = history.length - 1) => ({ history, position }); + +function fixture(initial: string | null) { + let bytes = initial; + const writes: string[] = []; + const logs: { type: string; payload: Record; severity?: string }[] = []; + const callbacks: { operation: string; hash: string }[] = []; + const storage = { + getItem(key: string) { assert.equal(key, UNDO_STORAGE_KEY); return bytes; }, + setItem(key: string, value: string) { + assert.equal(key, UNDO_STORAGE_KEY); + writes.push(value); + bytes = value; + }, + }; + const create = () => { + const store = createUndoStore({ + storage: () => storage, + log: (type, payload, severity) => logs.push({ type, payload, severity }), + }); + // These are recorders, never the timeline's destructive Git implementation. + store.onAgedAction(async (action) => { callbacks.push({ operation: 'age', hash: action.hash }); }); + store.onUndo('COMMIT_SOFT_DELETE', (action) => { callbacks.push({ operation: 'undo', hash: action.hash }); }); + store.onRedo('COMMIT_SOFT_DELETE', (action) => { callbacks.push({ operation: 'redo', hash: action.hash }); }); + return store; + }; + return { create, writes, logs, callbacks, bytes: () => bytes }; +} + +const malformed: [string, unknown][] = [ + ['null record', null], + ['array record', []], + ['missing history', {}], + ['non-array history', { history: {} }], + ['null entry', state([null as unknown as UndoEntry])], + ['mixed valid and invalid entries', state([entry(), null as unknown as UndoEntry])], + ['null action', state([{ ...entry(), action: null as unknown as UndoableAction }])], + ['unknown action', { history: [{ ...entry(), action: { type: 'DELETE_ALL', hash: 'commit' } }], position: 0 }], + ['missing identifier', { history: [{ action: entry().action, timestamp: 0 }], position: 0 }], + ['blank identifier', state([entry(' ')])], + ['non-string hash', { history: [{ ...entry(), action: { type: 'COMMIT_SOFT_DELETE', hash: 3 } }], position: 0 }], + ['blank hash', state([entry('entry', ' \t ')])], + ['missing timestamp', { history: [{ id: 'entry', action: entry().action }], position: 0 }], + ['negative timestamp', state([{ ...entry(), timestamp: -1 }])], + ['fractional timestamp', state([{ ...entry(), timestamp: 0.5 }])], + ['unsafe timestamp', state([{ ...entry(), timestamp: Number.MAX_SAFE_INTEGER + 1 }])], + ['negative cursor', state([entry()], -2)], + ['fractional cursor', state([entry()], -0.5)], + ['out-of-range cursor', state([entry()], 99)], + ['null cursor', { history: [entry()], position: null }], + ['string cursor', { history: [entry()], position: '0' }], + ['empty history cursor', state([], 0)], + ['oversize history', state(Array.from({ length: MAX_UNDO_HISTORY + 1 }, (_, i) => entry(`old-${i}`, `old-${i}`)))], +]; + +for (const [name, value] of malformed) { + test(`rejects the whole persisted record without recovery effects: ${name}`, async () => { + assert.equal(decodeUndoState(value).ok, false); + const original = JSON.stringify(value); + const f = fixture(original); + const store = f.create(); + assert.deepEqual(store.getState(), state([], -1)); + assert.equal(store.canUndo(), false); + assert.equal(store.canRedo(), false); + assert.equal(store.undo(), false); + assert.equal(store.redo(), false); + assert.deepEqual(f.callbacks, []); + assert.equal(f.logs[0].type, 'UNDO_STORE_LOAD_FAILED'); + assert.equal(f.logs[0].severity, 'warn'); + // No persisted entry can be aged into the destructive callback on a new push. + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new-only' }); + assert.deepEqual(f.callbacks, []); + store.clear(); + assert.deepEqual(f.writes, []); + assert.equal(f.bytes(), original); + assert.deepEqual(f.create().getState(), state([], -1)); + assert.equal(f.bytes(), original); + }); +} + +test('rejects invalid JSON and empty bytes without overwriting either', async () => { + for (const original of ['{broken', '']) { + const f = fixture(original); + const store = f.create(); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'fresh' }); + store.clear(); + assert.deepEqual(f.callbacks, []); + assert.deepEqual(f.writes, []); + assert.equal(f.bytes(), original); + assert.equal(f.logs[0].type, 'UNDO_STORE_LOAD_FAILED'); + } +}); + +test('rejects non-finite numeric values at the unknown-value boundary', () => { + for (const number of [NaN, Infinity, -Infinity]) { + assert.equal(decodeUndoState(state([{ ...entry(), timestamp: number }])).ok, false); + assert.equal(decodeUndoState(state([entry()], number)).ok, false); + } +}); + +test('accepts valid legacy records and omitted-position fallback without restore writes', () => { + const history = [entry('first', 'short'), entry('second', 'opaque/reference')]; + for (const record of [{ history }, ...[-1, 0, 1].map((position) => ({ history, position }))]) { + const f = fixture(JSON.stringify(record)); + const store = f.create(); + assert.deepEqual(store.getState(), { history, position: 'position' in record ? record.position : 1 }); + assert.deepEqual(f.writes, []); + assert.deepEqual(f.callbacks, []); + } + assert.deepEqual(decodeUndoState({ history: [] }), { ok: true, state: state([], -1) }); + assert.equal(decodeUndoState(state([{ ...entry(), timestamp: Number.MAX_SAFE_INTEGER }])).ok, true); +}); + +test('valid undo and redo invoke only their corresponding actions and survive reread', () => { + const f = fixture(JSON.stringify(state([entry('first', 'a'), entry('second', 'b')]))); + const store = f.create(); + assert.equal(store.undo(), true); + assert.equal(store.undo(), true); + assert.equal(store.undo(), false); + assert.equal(store.canUndo(), false); + assert.equal(store.canRedo(), true); + assert.equal(store.redo(), true); + assert.equal(store.redo(), true); + assert.equal(store.redo(), false); + assert.deepEqual(f.callbacks, [ + { operation: 'undo', hash: 'b' }, { operation: 'undo', hash: 'a' }, + { operation: 'redo', hash: 'a' }, { operation: 'redo', hash: 'b' }, + ]); + assert.deepEqual(f.create().getState(), store.getState()); +}); + +test('a new action after undo truncates redo without aging discarded actions', async () => { + const f = fixture(JSON.stringify(state([entry('first', 'a'), entry('second', 'b')], 0))); + const store = f.create(); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new' }); + assert.deepEqual(store.getState().history.map((item) => item.action.hash), ['a', 'new']); + assert.equal(store.canRedo(), false); + assert.deepEqual(f.callbacks, []); +}); + +test('valid full history ages exactly the oldest entry on a new action', async () => { + const history = Array.from({ length: MAX_UNDO_HISTORY }, (_, i) => entry(`entry-${i}`, `commit-${i}`)); + const f = fixture(JSON.stringify(state(history))); + const store = f.create(); + assert.deepEqual(f.callbacks, []); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new' }); + assert.deepEqual(f.callbacks, [{ operation: 'age', hash: 'commit-0' }]); + assert.equal(store.getState().history.length, MAX_UNDO_HISTORY); + assert.equal(store.getState().position, MAX_UNDO_HISTORY - 1); + assert.equal(store.getState().history[0].action.hash, 'commit-1'); + assert.deepEqual(f.create().getState(), store.getState()); +}); + +test('rejected history never reaches aging even after a full new in-memory history', async () => { + const original = JSON.stringify(malformed.at(-1)![1]); + const f = fixture(original); + const store = f.create(); + for (let i = 0; i <= MAX_UNDO_HISTORY; i++) { + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: `new-${i}` }); + } + assert.deepEqual(f.callbacks, [{ operation: 'age', hash: 'new-0' }]); + assert.deepEqual(f.writes, []); + assert.equal(f.bytes(), original); +}); + +test('missing storage supports ordinary persistence and explicit clear', async () => { + const f = fixture(null); + const store = f.create(); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new' }); + assert.equal(f.writes.length, 1); + assert.deepEqual(f.create().getState(), store.getState()); + store.clear(); + assert.deepEqual(JSON.parse(f.bytes()!), state([], -1)); + assert.deepEqual(f.callbacks, []); +}); + +test('unavailable storage reports failure and never attempts recovery writes', async () => { + for (const failAcquisition of [true, false]) { + const logs: string[] = []; + let writes = 0; + const store = createUndoStore({ + storage: () => { + if (failAcquisition) throw new Error('storage denied'); + return { getItem: () => { throw new Error('read denied'); }, setItem: () => { writes++; } }; + }, + log: (type) => { logs.push(type); }, + }); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'fresh' }); + store.clear(); + assert.equal(logs[0], 'UNDO_STORE_LOAD_FAILED'); + assert.ok(logs.includes('UNDO_STORE_SAVE_FAILED')); + assert.equal(writes, 0); + } +}); + +test('store instances keep storage and action callbacks isolated', async () => { + const a = fixture(JSON.stringify(state([entry('a', 'a')]))); + const b = fixture(JSON.stringify(state([entry('b', 'b')]))); + const first = a.create(); + const second = b.create(); + first.undo(); + assert.deepEqual(a.callbacks, [{ operation: 'undo', hash: 'a' }]); + assert.deepEqual(b.callbacks, []); + assert.equal(second.getState().position, 0); + await second.push({ type: 'COMMIT_SOFT_DELETE', hash: 'b-next' }); + assert.equal(first.getState().history.length, 1); + second.undo(); + assert.deepEqual(b.callbacks, [{ operation: 'undo', hash: 'b-next' }]); +}); + + +test('ordinary save failure is reported while valid in-memory actions remain usable', async () => { + const logs: string[] = []; + const store = createUndoStore({ + storage: () => ({ getItem: () => null, setItem: () => { throw new Error('quota'); } }), + log: (type) => { logs.push(type); }, + }); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new' }); + assert.equal(store.canUndo(), true); + assert.equal(store.undo(), true); + assert.equal(store.redo(), true); + assert.equal(logs.filter((type) => type === 'UNDO_STORE_SAVE_FAILED').length, 3); +}); + +test('valid-history aging callback failure retains existing error and state behavior', async () => { + const f = fixture(JSON.stringify(state(Array.from({ length: MAX_UNDO_HISTORY }, (_, i) => entry(`id-${i}`))))); + const store = f.create(); + let calls = 0; + store.onAgedAction(async () => { calls++; throw new Error('controlled cleanup failure'); }); + await store.push({ type: 'COMMIT_SOFT_DELETE', hash: 'new' }); + assert.equal(calls, 1); + assert.ok(f.logs.some(({ type, severity }) => type === 'UNDO_AGED_ACTION_FAILED' && severity === 'error')); + assert.equal(store.getState().history.length, MAX_UNDO_HISTORY); + assert.equal(store.getState().history.at(-1)?.action.hash, 'new'); +}); diff --git a/src/stores/undoPersistence.ts b/src/stores/undoPersistence.ts new file mode 100644 index 000000000..fdb1a215d --- /dev/null +++ b/src/stores/undoPersistence.ts @@ -0,0 +1,61 @@ +// Existing unversioned browser record; this decoder does not migrate or write it. +export type UndoableAction = { type: 'COMMIT_SOFT_DELETE'; hash: string }; + +export interface UndoEntry { + id: string; + action: UndoableAction; + timestamp: number; +} + +export interface UndoState { + history: UndoEntry[]; + position: number; // Index of next action to undo (-1 means nothing to undo) +} + +export const MAX_UNDO_HISTORY = 32; +export const UNDO_STORAGE_KEY = 'pantograph-unified-undo'; + +type UndoDecodeResult = + | { ok: true; state: UndoState } + | { ok: false; reason: string }; + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value); +} + +function isIdentifier(value: unknown): value is string { + // IDs are opaque: do not impose a new hash format or length limit. + return typeof value === 'string' && value.trim().length > 0; +} + +export function decodeUndoState(value: unknown): UndoDecodeResult { + if (!isRecord(value) || !Array.isArray(value.history)) { + return { ok: false, reason: 'Undo history must be an array in a record.' }; + } + if (value.history.length > MAX_UNDO_HISTORY) { + return { ok: false, reason: 'Undo history exceeds its 32-entry limit.' }; + } + + const history: UndoEntry[] = []; + for (const entry of value.history) { + if (!isRecord(entry) || !isIdentifier(entry.id) + || typeof entry.timestamp !== 'number' || !Number.isSafeInteger(entry.timestamp) + || entry.timestamp < 0 || !isRecord(entry.action) + || entry.action.type !== 'COMMIT_SOFT_DELETE' || !isIdentifier(entry.action.hash)) { + return { ok: false, reason: 'Undo history contains an invalid entry or action.' }; + } + history.push({ + id: entry.id, + timestamp: entry.timestamp, + action: { type: 'COMMIT_SOFT_DELETE', hash: entry.action.hash }, + }); + } + + // Preserve the existing omitted-position fallback for recognized legacy records. + const position = Object.hasOwn(value, 'position') ? value.position : history.length - 1; + if (typeof position !== 'number' || !Number.isInteger(position) + || position < -1 || position >= history.length) { + return { ok: false, reason: 'Undo position must be an integer within the history.' }; + } + return { ok: true, state: { history, position } }; +} diff --git a/src/stores/undoStore.ts b/src/stores/undoStore.ts index bcd33a653..644b69364 100644 --- a/src/stores/undoStore.ts +++ b/src/stores/undoStore.ts @@ -1,233 +1,9 @@ -import { writable, get } from 'svelte/store'; import { Logger } from '../services/Logger'; +import { createUndoStore } from './createUndoStore.ts'; -// Action types that can be undone -export type UndoableAction = - | { type: 'COMMIT_SOFT_DELETE'; hash: string } - // Add more action types as needed +export type { UndoableAction, UndoEntry } from './undoPersistence.ts'; -export interface UndoEntry { - id: string; - action: UndoableAction; - timestamp: number; -} - -interface UndoState { - history: UndoEntry[]; - position: number; // Index of next action to undo (-1 means nothing to undo) -} - -const MAX_HISTORY = 32; -const STORAGE_KEY = 'pantograph-unified-undo'; - -// Generate simple unique ID -function generateId(): string { - return `${Date.now()}-${Math.random().toString(36).slice(2, 9)}`; -} - -// Load state from localStorage -function loadFromStorage(): UndoState { - try { - const stored = localStorage.getItem(STORAGE_KEY); - if (stored) { - const parsed = JSON.parse(stored); - if (parsed && Array.isArray(parsed.history)) { - return { - history: parsed.history, - position: typeof parsed.position === 'number' ? parsed.position : parsed.history.length - 1, - }; - } - } - } catch (e) { - Logger.log('UNDO_STORE_LOAD_FAILED', { error: String(e) }, 'warn'); - } - return { history: [], position: -1 }; -} - -// Save state to localStorage -function saveToStorage(state: UndoState): void { - try { - localStorage.setItem(STORAGE_KEY, JSON.stringify(state)); - } catch (e) { - Logger.log('UNDO_STORE_SAVE_FAILED', { error: String(e) }, 'warn'); - } -} - -// Callback registry for handling aged-out actions and undo/redo -type AgedActionCallback = (action: UndoableAction) => Promise; -type UndoCallback = (action: UndoableAction) => void; -type RedoCallback = (action: UndoableAction) => void; - -let agedActionCallback: AgedActionCallback | null = null; -const undoCallbacks: Map = new Map(); -const redoCallbacks: Map = new Map(); - -function createUndoStore() { - const initial = loadFromStorage(); - const { subscribe, set } = writable(initial); - - const store = { - subscribe, - - /** - * Register callback for when actions age past 32 steps - */ - onAgedAction(callback: AgedActionCallback): void { - agedActionCallback = callback; - }, - - /** - * Register undo handler for specific action type - */ - onUndo(type: UndoableAction['type'], callback: UndoCallback): void { - undoCallbacks.set(type, callback); - }, - - /** - * Register redo handler for specific action type - */ - onRedo(type: UndoableAction['type'], callback: RedoCallback): void { - redoCallbacks.set(type, callback); - }, - - /** - * Push a new action to the undo history. - * If history exceeds MAX_HISTORY, oldest action is permanently executed. - */ - async push(action: UndoableAction): Promise { - const state = get({ subscribe }); - - // If we're not at the end of history, truncate future actions - // (user did something new after undoing) - const newHistory = state.history.slice(0, state.position + 1); - - // Add new action - const entry: UndoEntry = { - id: generateId(), - action, - timestamp: Date.now(), - }; - newHistory.push(entry); - - // Check if we need to age out the oldest action - if (newHistory.length > MAX_HISTORY) { - const aged = newHistory.shift(); - if (aged && agedActionCallback) { - try { - await agedActionCallback(aged.action); - Logger.log('UNDO_ACTION_AGED', { type: aged.action.type }); - } catch (e) { - Logger.log('UNDO_AGED_ACTION_FAILED', { error: String(e) }, 'error'); - } - } - } - - const newState: UndoState = { - history: newHistory, - position: newHistory.length - 1, - }; - - set(newState); - saveToStorage(newState); - Logger.log('UNDO_PUSH', { type: action.type, historySize: newHistory.length }); - }, - - /** - * Undo the most recent action - */ - undo(): boolean { - const state = get({ subscribe }); - - if (state.position < 0 || state.history.length === 0) { - return false; - } - - const entry = state.history[state.position]; - if (!entry) return false; - - // Call undo handler - const handler = undoCallbacks.get(entry.action.type); - if (handler) { - handler(entry.action); - } - - const newState: UndoState = { - ...state, - position: state.position - 1, - }; - - set(newState); - saveToStorage(newState); - Logger.log('UNDO_EXECUTED', { type: entry.action.type, position: newState.position }); - return true; - }, - - /** - * Redo a previously undone action - */ - redo(): boolean { - const state = get({ subscribe }); - - if (state.position >= state.history.length - 1) { - return false; - } - - const nextPosition = state.position + 1; - const entry = state.history[nextPosition]; - if (!entry) return false; - - // Call redo handler - const handler = redoCallbacks.get(entry.action.type); - if (handler) { - handler(entry.action); - } - - const newState: UndoState = { - ...state, - position: nextPosition, - }; - - set(newState); - saveToStorage(newState); - Logger.log('REDO_EXECUTED', { type: entry.action.type, position: newState.position }); - return true; - }, - - /** - * Check if undo is available - */ - canUndo(): boolean { - const state = get({ subscribe }); - return state.position >= 0 && state.history.length > 0; - }, - - /** - * Check if redo is available - */ - canRedo(): boolean { - const state = get({ subscribe }); - return state.position < state.history.length - 1; - }, - - /** - * Clear all history - */ - clear(): void { - const newState: UndoState = { history: [], position: -1 }; - set(newState); - saveToStorage(newState); - Logger.log('UNDO_HISTORY_CLEARED', {}); - }, - - /** - * Get current state for debugging - */ - getState(): UndoState { - return get({ subscribe }); - }, - }; - - return store; -} - -export const undoStore = createUndoStore(); +export const undoStore = createUndoStore({ + storage: () => localStorage, + log: (type, payload, severity) => Logger.log(type, payload, severity), +}); From 934147092e33b1adfb0d0c17424dd12e22608efa Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 03:52:11 +0000 Subject: [PATCH 2/9] fix(graph): own navigation and persistence timer lifetimes Resolve FE-A02 stale animation completions by cancelling and settling superseded navigation, reset, and immediate breadcrumb timers. Share persistence subscriptions and debounce timers across independently released enablePersistence owners. Validation: 15 focused timer tests; 562 aggregate frontend tests; TypeScript, full lint, production build, staged critical/a11y/traceability, whitespace pass. Regression fails against ca9 before repair. GUI/native/model checks not executed. Base: ca9edde6850cd58ade0b7e534bb4f58e704c2c64 (full ancestry retained). --- .../2026-10-04-view-store-lifecycle.md | 40 +++ packages/svelte-graph/README.md | 16 ++ .../src/stores/createViewStores.test.ts | 232 ++++++++++++++++++ .../src/stores/createViewStores.ts | 119 ++++++--- 4 files changed, 367 insertions(+), 40 deletions(-) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-store-lifecycle.md create mode 100644 packages/svelte-graph/src/stores/createViewStores.test.ts diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-store-lifecycle.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-store-lifecycle.md new file mode 100644 index 000000000..1adc3f191 --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-store-lifecycle.md @@ -0,0 +1,40 @@ +# FE-A02 view-store timer ownership + +The active M4 coverage retains the async-owner obligation from FE-A02. Source +inspection confirmed the two gaps in the existing `createViewStores` owner: +old animation timers clear newer navigation state, and repeated automatic +persistence starts create duplicate subscriptions and writes. + +A base-ca9 probe started a 10 ms group animation followed by a 1000 ms animation. +After the first caller completed, the second remained pending but isAnimating +was false and zoomTarget null. A second isolated probe started persistence twice +and observed two pending timers and two writes for one snapshot. Neither probe +used Tauri, models or production storage. + +The repair gives each store one animation timer. A new animated navigation +cancels the old timer and settles its caller; reset and immediate breadcrumb +navigation do the same. Only the current timer clears transient state. Graph +navigation state still changes immediately and no-op navigation preserves the +current animation. The existing Promise signatures are retained. + +Automatic persistence shares subscriptions and its existing 500 ms debounce +while any enable handle remains owned. Handles release idempotently; final +release unsubscribes and cancels the pending write. Re-enabling starts a fresh +scope. Storage shape, restoration and migration policy are outside this slice. +The package guide records these lifecycle semantics. + +Fifteen deterministic Node timer tests cover single/overlapping navigation, +reset, each animation route, immediate breadcrumbs, no-op behavior, isolated +stores, duplicate persistence starts, independent ownership, final cleanup, +restart and disabled persistence. The supersession regression fails against the +base implementation (only its runtime TypeScript import spelling is normalized +for Node). All 562 discovered frontend tests, full lint, TypeScript and production +build pass on Node 24.12.0/npm 11.6.2. The build retains the stale Browserslist-data +notice. Staged critical/a11y/traceability checks pass (nine accessibility +checks, four traced paths, zero impacted requirements), as does whitespace +validation. Exact results are recorded in the commit message. + +This is controlled frontend lifecycle evidence. It does not qualify browser +interaction, GUI/native/model execution, persisted view decoding, or all FE-A02 +owners. The branch is directly atop ca9 and disjoint from the frozen +fixture/scanner/undo repairs; the integrator owns independent review. diff --git a/packages/svelte-graph/README.md b/packages/svelte-graph/README.md index 6873377eb..3dfc59ed3 100644 --- a/packages/svelte-graph/README.md +++ b/packages/svelte-graph/README.md @@ -33,3 +33,19 @@ become an implicit dependency of a package-local command. Implementation lives under `packages/svelte-graph/src/`. Repository-level development and verification guidance is in [`docs/development.md`](../../docs/development.md). + +## Navigation Animation Lifetime + +Each view-store instance owns one navigation animation timer. New animated +navigation supersedes the prior animation; reset and immediate breadcrumb +navigation cancel it. Superseded/cancelled callers settle their existing +`Promise` without clearing a newer target or animation state. Navigation +state still changes immediately, and the current animation clears its transient +target only when its configured duration finishes. No-op navigation preserves +the current animation. Store instances keep independent timer ownership. + +Automatic view persistence also has one shared subscription/debounce scope per +store. Each `enablePersistence()` call returns an independent, idempotent release +handle. Persistence remains active until the last handle is released; final +release unsubscribes and cancels the pending write. Re-enabling starts a fresh +scope. The existing storage shape and 500 ms debounce are unchanged. diff --git a/packages/svelte-graph/src/stores/createViewStores.test.ts b/packages/svelte-graph/src/stores/createViewStores.test.ts new file mode 100644 index 000000000..4963dbc12 --- /dev/null +++ b/packages/svelte-graph/src/stores/createViewStores.test.ts @@ -0,0 +1,232 @@ +import assert from 'node:assert/strict'; +import test, { type TestContext } from 'node:test'; +import { get } from 'svelte/store'; +import { createViewStores } from './createViewStores.ts'; + +function useClock(t: TestContext) { + t.mock.timers.enable({ apis: ['setTimeout'] }); + const view = createViewStores(); + view.setAnimationConfig({ duration: 100 }); + return view; +} + +test('single group navigation keeps target until its configured completion', async (t) => { + const view = useClock(t); + const completion = view.tabIntoGroup('first'); + assert.equal(get(view.isAnimating), true); + assert.equal(get(view.zoomTarget)?.nodeId, 'first'); + assert.deepEqual(get(view.groupStack), ['first']); + t.mock.timers.tick(99); + assert.equal(get(view.isAnimating), true); + t.mock.timers.tick(1); + await completion; + assert.equal(get(view.isAnimating), false); + assert.equal(get(view.zoomTarget), null); +}); + +test('superseded navigation settles without clearing the newest animation', async (t) => { + const view = useClock(t); + let firstSettled = false; + const first = view.tabIntoGroup('first').then(() => { firstSettled = true; }); + t.mock.timers.tick(20); + view.setAnimationConfig({ duration: 200 }); + const second = view.tabIntoGroup('second'); + await Promise.resolve(); + await Promise.resolve(); + assert.equal(firstSettled, true, 'supersession settles the previous caller'); + await first; + t.mock.timers.tick(80); + assert.equal(get(view.isAnimating), true); + assert.equal(get(view.zoomTarget)?.nodeId, 'second'); + assert.deepEqual(get(view.groupStack), ['first', 'second']); + t.mock.timers.tick(120); + await second; + assert.equal(get(view.isAnimating), false); + assert.equal(get(view.zoomTarget), null); +}); + +test('reset cancels owned timer and settles the pending navigation', async (t) => { + const view = useClock(t); + const first = view.tabIntoGroup('first'); + view.resetViewState(); + await first; + assert.deepEqual(get(view.groupStack), []); + assert.equal(get(view.viewLevel), 'data-graph'); + assert.equal(get(view.isAnimating), false); + assert.equal(get(view.zoomTarget), null); + view.setAnimationConfig({ duration: 200 }); + const second = view.tabIntoGroup('after-reset'); + t.mock.timers.tick(100); + assert.equal(get(view.isAnimating), true); + assert.equal(get(view.zoomTarget)?.nodeId, 'after-reset'); + t.mock.timers.tick(100); + await second; +}); + +test('orchestration navigation supersedes group animation without retaining its target', async (t) => { + const view = useClock(t); + const group = view.tabIntoGroup('group'); + const orchestration = view.zoomToOrchestration(); + await group; + assert.equal(get(view.zoomTarget), null); + assert.equal(get(view.isAnimating), true); + assert.equal(get(view.viewLevel), 'orchestration'); + assert.deepEqual(get(view.groupStack), []); + t.mock.timers.tick(100); + await orchestration; +}); + +test('data-graph navigation supersedes orchestration and retains its own target', async (t) => { + const view = useClock(t); + const orchestration = view.zoomToOrchestration('old'); + const graph = view.zoomToDataGraph('new', 'graph-id'); + await orchestration; + assert.equal(get(view.zoomTarget)?.nodeId, 'new'); + assert.equal(get(view.currentDataGraphId), 'graph-id'); + assert.equal(get(view.viewLevel), 'data-graph'); + t.mock.timers.tick(100); + await graph; +}); + +test('tab out supersedes tab in and preserves stack and target semantics', async (t) => { + const view = useClock(t); + const enter = view.tabIntoGroup('group'); + const leave = view.tabOutOfGroup(); + await enter; + assert.equal(get(view.viewLevel), 'data-graph'); + assert.deepEqual(get(view.groupStack), []); + assert.equal(get(view.zoomTarget)?.nodeId, 'group'); + assert.equal(get(view.isAnimating), true); + t.mock.timers.tick(100); + await leave; +}); + +for (const level of ['data-graph', 'group'] as const) { + test(`breadcrumb navigation cancels obsolete animation: ${level}`, async (t) => { + const view = useClock(t); + const first = view.tabIntoGroup('first'); + const second = view.tabIntoGroup('second'); + await first; + await view.navigateToBreadcrumb({ id: 'first', name: 'First', level }); + await second; + assert.equal(get(view.isAnimating), false); + assert.equal(get(view.zoomTarget), null); + assert.deepEqual(get(view.groupStack), level === 'group' ? ['first'] : []); + t.mock.timers.tick(100); + assert.equal(get(view.isAnimating), false); + }); +} + +test('no-op navigation leaves the current animation and completion intact', async (t) => { + const view = useClock(t); + const current = view.zoomToOrchestration('current'); + await view.zoomToOrchestration('ignored'); + await view.tabOutOfGroup(); + assert.equal(get(view.zoomTarget)?.nodeId, 'current'); + assert.equal(get(view.isAnimating), true); + t.mock.timers.tick(100); + await current; +}); + +test('independent view stores do not supersede each other', async (t) => { + const first = useClock(t); + const second = createViewStores(); + second.setAnimationConfig({ duration: 200 }); + const a = first.tabIntoGroup('a'); + const b = second.tabIntoGroup('b'); + first.resetViewState(); + await a; + t.mock.timers.tick(100); + assert.equal(get(second.isAnimating), true); + assert.equal(get(second.zoomTarget)?.nodeId, 'b'); + t.mock.timers.tick(100); + await b; +}); + + +function useStorage(t: TestContext) { + const previous = Object.getOwnPropertyDescriptor(globalThis, 'localStorage'); + const writes: { key: string; value: string }[] = []; + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + value: { setItem: (key: string, value: string) => { writes.push({ key, value }); } }, + }); + t.after(() => { + if (previous) Object.defineProperty(globalThis, 'localStorage', previous); + else Reflect.deleteProperty(globalThis, 'localStorage'); + }); + t.mock.timers.enable({ apis: ['setTimeout'] }); + return writes; +} + +test('duplicate persistence starts share one debounced write', (t) => { + const writes = useStorage(t); + const view = createViewStores({ storageKey: 'view' }); + const releaseA = view.enablePersistence(); + const releaseB = view.enablePersistence(); + view.groupStack.set(['latest']); + t.mock.timers.tick(499); + assert.equal(writes.length, 0); + t.mock.timers.tick(1); + assert.equal(writes.length, 1); + assert.deepEqual(JSON.parse(writes[0].value).groupStack, ['latest']); + releaseA(); + releaseB(); +}); + +test('releasing one persistence owner preserves the remaining owner', (t) => { + const writes = useStorage(t); + const view = createViewStores({ storageKey: 'view' }); + const releaseA = view.enablePersistence(); + const releaseB = view.enablePersistence(); + releaseA(); + releaseA(); // Repeated release cannot consume another caller's ownership. + view.viewLevel.set('orchestration'); + t.mock.timers.tick(500); + assert.equal(writes.length, 1); + releaseB(); + view.viewLevel.set('group'); + t.mock.timers.tick(500); + assert.equal(writes.length, 1); +}); + +test('final persistence release cancels a pending write and supports a fresh scope', (t) => { + const writes = useStorage(t); + const view = createViewStores({ storageKey: 'view' }); + const release = view.enablePersistence(); + t.mock.timers.tick(499); + release(); + t.mock.timers.tick(1); + assert.equal(writes.length, 0); + view.groupStack.set(['new-state']); + t.mock.timers.tick(500); + assert.equal(writes.length, 0); + const releaseAgain = view.enablePersistence(); + t.mock.timers.tick(500); + assert.equal(writes.length, 1); + assert.deepEqual(JSON.parse(writes[0].value).groupStack, ['new-state']); + releaseAgain(); +}); + +test('independent persistence scopes retain their own storage keys and cleanup', (t) => { + const writes = useStorage(t); + const first = createViewStores({ storageKey: 'first' }); + const second = createViewStores({ storageKey: 'second' }); + const releaseFirst = first.enablePersistence(); + const releaseSecond = second.enablePersistence(); + releaseFirst(); + t.mock.timers.tick(500); + assert.deepEqual(writes.map(({ key }) => key), ['second']); + releaseSecond(); +}); + +test('disabled persistence schedules no writes', (t) => { + const writes = useStorage(t); + const view = createViewStores(); + const release = view.enablePersistence(); + view.groupStack.set(['group']); + t.mock.timers.tick(500); + release(); + release(); + assert.deepEqual(writes, []); +}); diff --git a/packages/svelte-graph/src/stores/createViewStores.ts b/packages/svelte-graph/src/stores/createViewStores.ts index e6f2c8a02..2df533e72 100644 --- a/packages/svelte-graph/src/stores/createViewStores.ts +++ b/packages/svelte-graph/src/stores/createViewStores.ts @@ -8,7 +8,7 @@ */ import { writable, derived, get } from 'svelte/store'; import type { ViewLevel, BreadcrumbItem, ViewportState, ZoomTarget, AnimationConfig } from '../types/view.js'; -import { DEFAULT_ANIMATION } from '../types/view.js'; +import { DEFAULT_ANIMATION } from '../types/view.ts'; export interface ViewStoreOptions { /** localStorage key for auto-persistence (omit to disable) */ @@ -100,8 +100,37 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { ); // --- Utility --- - function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); + let activeAnimation: { + timer: ReturnType; + resolve: () => void; + } | null = null; + + function cancelAnimation(): void { + if (!activeAnimation) return; + const previous = activeAnimation; + activeAnimation = null; + clearTimeout(previous.timer); + previous.resolve(); + } + + function beginAnimation(target: ZoomTarget | null): Promise { + cancelAnimation(); + const completion = new Promise((resolve) => { + const animation = { + resolve, + timer: setTimeout(() => { + if (activeAnimation !== animation) return; + activeAnimation = null; + zoomTarget.set(null); + isAnimating.set(false); + resolve(); + }, get(animationConfig).duration), + }; + activeAnimation = animation; + }); + isAnimating.set(true); + zoomTarget.set(target); + return completion; } // --- Actions --- @@ -109,64 +138,48 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { async function zoomToOrchestration(targetNodeId?: string): Promise { if (get(viewLevel) === 'orchestration') return; - isAnimating.set(true); - if (targetNodeId) { - zoomTarget.set({ nodeId: targetNodeId, position: { x: 0, y: 0 } }); - } + const completion = beginAnimation(targetNodeId + ? { nodeId: targetNodeId, position: { x: 0, y: 0 } } + : null); viewLevel.set('orchestration'); groupStack.set([]); - const config = get(animationConfig); - await sleep(config.duration); - zoomTarget.set(null); - isAnimating.set(false); + await completion; } async function zoomToDataGraph(orchestrationNodeId: string, dataGraphId: string): Promise { const currentLevel = get(viewLevel); if (currentLevel === 'data-graph' || currentLevel === 'group') return; - isAnimating.set(true); - zoomTarget.set({ nodeId: orchestrationNodeId, position: { x: 0, y: 0 } }); + const completion = beginAnimation({ nodeId: orchestrationNodeId, position: { x: 0, y: 0 } }); currentDataGraphId.set(dataGraphId); viewLevel.set('data-graph'); - const config = get(animationConfig); - await sleep(config.duration); - zoomTarget.set(null); - isAnimating.set(false); + await completion; } async function tabIntoGroup(groupId: string): Promise { - isAnimating.set(true); - zoomTarget.set({ nodeId: groupId, position: { x: 0, y: 0 } }); + const completion = beginAnimation({ nodeId: groupId, position: { x: 0, y: 0 } }); groupStack.update((stack) => [...stack, groupId]); viewLevel.set('group'); - const config = get(animationConfig); - await sleep(config.duration); - zoomTarget.set(null); - isAnimating.set(false); + await completion; } async function tabOutOfGroup(): Promise { const stack = get(groupStack); if (stack.length === 0) return; - isAnimating.set(true); const poppedGroupId = stack[stack.length - 1]; + const completion = beginAnimation({ nodeId: poppedGroupId, position: { x: 0, y: 0 } }); groupStack.update((s) => s.slice(0, -1)); - zoomTarget.set({ nodeId: poppedGroupId, position: { x: 0, y: 0 } }); if (stack.length === 1) { viewLevel.set('data-graph'); } - const config = get(animationConfig); - await sleep(config.duration); - zoomTarget.set(null); - isAnimating.set(false); + await completion; } async function navigateBack(): Promise { @@ -189,12 +202,18 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { await zoomToOrchestration(); } else if (item.level === 'data-graph') { if (currentLevel === 'group') { + cancelAnimation(); + zoomTarget.set(null); + isAnimating.set(false); groupStack.set([]); viewLevel.set('data-graph'); } } else if (item.level === 'group') { const targetIndex = stack.indexOf(item.id); if (targetIndex >= 0 && targetIndex < stack.length - 1) { + cancelAnimation(); + zoomTarget.set(null); + isAnimating.set(false); groupStack.set(stack.slice(0, targetIndex + 1)); } } @@ -217,6 +236,7 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { } function resetViewState(): void { + cancelAnimation(); viewLevel.set('data-graph'); currentOrchestrationId.set(null); currentDataGraphId.set(null); @@ -262,23 +282,42 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { return false; } - /** Enable auto-persistence on state changes. Returns a cleanup function. */ + let persistenceOwners = 0; + let cleanupPersistence: (() => void) | null = null; + + /** Share auto-persistence while any caller owns an enable handle. */ function enablePersistence(): () => void { if (!storageKey) return () => {}; - let persistTimeout: ReturnType | null = null; - const debouncedPersist = () => { - if (persistTimeout) clearTimeout(persistTimeout); - persistTimeout = setTimeout(persistViewState, 500); - }; + persistenceOwners += 1; + if (persistenceOwners === 1) { + let persistTimeout: ReturnType | null = null; + const debouncedPersist = () => { + if (persistTimeout) clearTimeout(persistTimeout); + persistTimeout = setTimeout(() => { + persistTimeout = null; + persistViewState(); + }, 500); + }; - const unsub1 = viewLevel.subscribe(debouncedPersist); - const unsub2 = groupStack.subscribe(debouncedPersist); + const unsub1 = viewLevel.subscribe(debouncedPersist); + const unsub2 = groupStack.subscribe(debouncedPersist); + cleanupPersistence = () => { + unsub1(); + unsub2(); + if (persistTimeout) clearTimeout(persistTimeout); + }; + } + let released = false; return () => { - unsub1(); - unsub2(); - if (persistTimeout) clearTimeout(persistTimeout); + if (released) return; + released = true; + persistenceOwners -= 1; + if (persistenceOwners === 0) { + cleanupPersistence?.(); + cleanupPersistence = null; + } }; } From 6a3ef2cf8a42c0ef0c0823c3d5af5384774a0a44 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 03:56:53 +0000 Subject: [PATCH 3/9] fix(frontend): own DeviceConfig initialization through teardown Prevent deferred device initialization from creating refresh intervals or applying read results after component destruction. Share initialization across duplicate starts and observe active/late failures with a mount-owned lifecycle helper. Keep backend requests settling and preserve existing configuration policies. Validation: 10 focused lifecycle tests, actual-script before/after deferred probe, 557 aggregate frontend tests, full lint, TypeScript, production build, staged critical/a11y/traceability and whitespace pass. GUI/native/model checks not run. Base: ca9edde6850cd58ade0b7e534bb4f58e704c2c64 (full ancestry retained). --- .../2026-10-04-device-config-lifecycle.md | 45 +++++ src/components/DeviceConfig.svelte | 62 ++++--- src/components/deviceConfigLifecycle.test.ts | 170 ++++++++++++++++++ src/components/deviceConfigLifecycle.ts | 55 ++++++ 4 files changed, 311 insertions(+), 21 deletions(-) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-device-config-lifecycle.md create mode 100644 src/components/deviceConfigLifecycle.test.ts create mode 100644 src/components/deviceConfigLifecycle.ts diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-device-config-lifecycle.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-device-config-lifecycle.md new file mode 100644 index 000000000..e112c61f5 --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-device-config-lifecycle.md @@ -0,0 +1,45 @@ +# DeviceConfig initialization lifetime qualification + +Source base: `ca9edde6850cd58ade0b7e534bb4f58e704c2c64`, retaining the complete +PR44 stacked ancestry. Isolated branch: `repair/device-config-lifecycle`. + +This is a bounded FE-A02 repair. The active repository review retains FE-A02 +async terminal observation and cleanup; the frontend audit explicitly names +DeviceConfig's destruction race. The component's asynchronous mount callback +awaited the initial device request before constructing its refresh scope. A +mount destroyed during that request had already run cleanup when the continuation +created a new interval and applied device and embedding-mode state. + +A controlled script probe runs the actual component script after TypeScript +transformation, injecting deferred services, lifecycle callbacks, identity runes, +and a recording interval API. On the base, destroying before the initial device +response then resolving it releases all three subscriptions but leaves one active +interval, starts a mode read, and applies both late values. On this repair, the +same sequence leaves zero intervals, starts zero mode reads, and retains the +initial device/mode values. The mount callback returns synchronously. This is +script-level lifecycle evidence, not Svelte DOM or native IPC execution. + +The mount now owns an initialization scope before asynchronous work begins. +Duplicate starts share one promise. Destruction permanently closes that scope, +stops its refresh owner, and blocks later mode application or refresh creation. +Device reads capture the same owner and observe successful and rejected late +responses without mutating unmounted component state. Live initialization +failures are observed through the component's existing console error channel. +Backend requests are allowed to settle; this slice introduces no backend +cancellation, mutation, credential, network, or configuration policy. + +Ten deterministic helper tests cover duplicate starts, immediate destruction, +destruction during both reads, live and late failures, independent remounts, +idempotent cleanup, and reentrant destruction during refresh creation. The +component probe also verifies its wiring and late device-state suppression. +All 557 discovered frontend tests, full lint, TypeScript, production build, +staged critical/a11y/traceability (four paths, zero mapped impacts), and whitespace +checks pass on Node 24.12.0/npm 11.6.2. The build retains its existing stale +Browserslist-data notice. Exact results are recorded in the milestone commit +message. + +GUI, model inference, native IPC, and browser interaction were not executed. +Save-command lifetime and foreground/background device refresh ordering are +outside this initialization/teardown slice; this report does not close all +FE-A02 owners. No frozen fixture/scanner/undo/view source is changed. The branch +is directly atop ca9; the integrator owns independent review and integration. diff --git a/src/components/DeviceConfig.svelte b/src/components/DeviceConfig.svelte index 420b172c4..e547800bf 100644 --- a/src/components/DeviceConfig.svelte +++ b/src/components/DeviceConfig.svelte @@ -5,6 +5,7 @@ import { RagService } from '../services/RagService'; import { expandedSection, toggleSection } from '../stores/accordionStore'; import { createScopedDeviceRefresh, type DeviceRefreshScope } from './deviceConfigRefreshScope'; + import { createDeviceConfigLifecycle, type DeviceConfigLifecycle } from './deviceConfigLifecycle'; import { buildBackendConfirmedDeviceOptions, formatDeviceDisplayName, @@ -24,6 +25,7 @@ let oomFlashTimer: ReturnType | null = null; let lastOomAt = 0; let deviceRefreshScope: DeviceRefreshScope | null = null; + let deviceLifecycle: DeviceConfigLifecycle | null = null; // Available devices confirmed by the backend. let availableDevices: DeviceInfo[] = $state([]); @@ -34,7 +36,7 @@ let embeddingMemoryMode: EmbeddingMemoryMode = $state('cpu_parallel'); let initialEmbeddingMode: EmbeddingMemoryMode = $state('cpu_parallel'); - onMount(async () => { + onMount(() => { unsubscribe = ConfigService.subscribe((nextState) => { state = nextState; // Sync local form state @@ -51,28 +53,39 @@ triggerOomFlash(nextState.error); }); - // Load available devices - await loadDevices(); - deviceRefreshScope = createScopedDeviceRefresh( - () => { - void loadDevices({ silent: true }); + deviceLifecycle = createDeviceConfigLifecycle({ + loadDevices, + startRefresh: () => { + const scope = createScopedDeviceRefresh( + () => { + void loadDevices({ silent: true }); + }, + window, + ); + deviceRefreshScope = scope; + scope.update($expandedSection === 'device'); + return () => { + scope.stop(); + deviceRefreshScope = null; + }; }, - window, - ); - deviceRefreshScope.update($expandedSection === 'device'); - - // Load embedding memory mode - const mode = await ConfigService.getEmbeddingMemoryMode(); - embeddingMemoryMode = mode; - initialEmbeddingMode = mode; + loadEmbeddingMemoryMode: () => ConfigService.getEmbeddingMemoryMode(), + applyEmbeddingMemoryMode: (mode) => { + embeddingMemoryMode = mode; + initialEmbeddingMode = mode; + }, + onFailure: (error) => { + console.error('Failed to initialize device config:', error); + }, + }); + void deviceLifecycle.start(); }); onDestroy(() => { + deviceLifecycle?.stop(); unsubscribe?.(); unsubscribeLLM?.(); unsubscribeRag?.(); - deviceRefreshScope?.stop(); - deviceRefreshScope = null; if (oomFlashTimer) { clearTimeout(oomFlashTimer); oomFlashTimer = null; @@ -108,6 +121,8 @@ }; const loadDevices = async (options: { silent?: boolean } = {}) => { + const lifecycle = deviceLifecycle; + if (!lifecycle?.isActive()) return; const silent = options.silent ?? false; if (silent) { if (isRefreshingDevices) return; @@ -118,21 +133,26 @@ deviceLoadError = null; } try { - availableDevices = await ConfigService.listDevices(); + const devices = await ConfigService.listDevices(); + if (!lifecycle.isActive()) return; + availableDevices = devices; if (deviceLoadError && silent) { deviceLoadError = null; } } catch (error) { + if (!lifecycle.isActive()) return; if (!silent) { deviceLoadError = String(error); console.error('Failed to load devices:', error); availableDevices = []; } } finally { - if (silent) { - isRefreshingDevices = false; - } else { - isLoadingDevices = false; + if (lifecycle.isActive()) { + if (silent) { + isRefreshingDevices = false; + } else { + isLoadingDevices = false; + } } } }; diff --git a/src/components/deviceConfigLifecycle.test.ts b/src/components/deviceConfigLifecycle.test.ts new file mode 100644 index 000000000..c4d08a49c --- /dev/null +++ b/src/components/deviceConfigLifecycle.test.ts @@ -0,0 +1,170 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createDeviceConfigLifecycle } from './deviceConfigLifecycle.ts'; +import type { EmbeddingMemoryMode } from '../services/ConfigService'; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((accept, fail) => { + resolve = accept; + reject = fail; + }); + return { promise, resolve, reject }; +} + +function fixture() { + const devices = deferred(); + const mode = deferred(); + const applied: EmbeddingMemoryMode[] = []; + const failures: unknown[] = []; + let deviceReads = 0; + let modeReads = 0; + let refreshStarts = 0; + let refreshStops = 0; + const lifecycle = createDeviceConfigLifecycle({ + loadDevices: () => { deviceReads++; return devices.promise; }, + startRefresh: () => { + refreshStarts++; + return () => { refreshStops++; }; + }, + loadEmbeddingMemoryMode: () => { modeReads++; return mode.promise; }, + applyEmbeddingMemoryMode: value => { applied.push(value); }, + onFailure: error => { failures.push(error); }, + }); + return { + lifecycle, devices, mode, applied, failures, + counts: () => ({ deviceReads, modeReads, refreshStarts, refreshStops }), + }; +} + +async function reachMode(f: ReturnType) { + f.devices.resolve(); + await Promise.resolve(); + await Promise.resolve(); +} + +test('device initialization is shared by duplicate starts and owns one refresh cleanup', async () => { + const f = fixture(); + const first = f.lifecycle.start(); + assert.equal(f.lifecycle.start(), first); + await reachMode(f); + f.mode.resolve('gpu_parallel'); + await first; + assert.deepEqual(f.applied, ['gpu_parallel']); + assert.deepEqual(f.counts(), { deviceReads: 1, modeReads: 1, refreshStarts: 1, refreshStops: 0 }); + f.lifecycle.stop(); + f.lifecycle.stop(); + assert.equal(f.lifecycle.isActive(), false); + assert.equal(f.counts().refreshStops, 1); +}); + +test('destroying before initialization begins starts no backend read', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + f.lifecycle.stop(); + await pending; + await f.lifecycle.start(); + assert.deepEqual(f.counts(), { deviceReads: 0, modeReads: 0, refreshStarts: 0, refreshStops: 0 }); +}); + +test('destroying during the initial device read cannot create a late refresh', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + await Promise.resolve(); + assert.equal(f.counts().deviceReads, 1); + f.lifecycle.stop(); + f.devices.resolve(); + await pending; + assert.deepEqual(f.counts(), { deviceReads: 1, modeReads: 0, refreshStarts: 0, refreshStops: 0 }); + assert.deepEqual(f.applied, []); +}); + +test('destroying during mode loading stops refresh and ignores the late mode', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + await reachMode(f); + assert.equal(f.counts().modeReads, 1); + f.lifecycle.stop(); + f.mode.resolve('sequential'); + await pending; + assert.deepEqual(f.applied, []); + assert.equal(f.counts().refreshStops, 1); +}); + +test('live device-read failure is observed and skips later initialization', async () => { + const f = fixture(); + const failure = new Error('devices unavailable'); + const pending = f.lifecycle.start(); + f.devices.reject(failure); + await pending; + assert.deepEqual(f.failures, [failure]); + assert.equal(f.counts().modeReads, 0); + assert.equal(f.counts().refreshStarts, 0); +}); + +test('live mode-read failure is observed and refresh remains owned until cleanup', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + await reachMode(f); + const failure = new Error('mode unavailable'); + f.mode.reject(failure); + await pending; + assert.deepEqual(f.failures, [failure]); + assert.deepEqual(f.applied, []); + f.lifecycle.stop(); + assert.equal(f.counts().refreshStops, 1); +}); + +test('late device rejection settles without an unmounted error callback', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + await Promise.resolve(); + f.lifecycle.stop(); + f.devices.reject(new Error('late device failure')); + await pending; + assert.deepEqual(f.failures, []); +}); + +test('late mode rejection settles without an unmounted error callback', async () => { + const f = fixture(); + const pending = f.lifecycle.start(); + await reachMode(f); + f.lifecycle.stop(); + f.mode.reject(new Error('late mode failure')); + await pending; + assert.deepEqual(f.failures, []); +}); + +test('a stopped initialization cannot restart while a new mount is independent', async () => { + const oldMount = fixture(); + oldMount.lifecycle.stop(); + await oldMount.lifecycle.start(); + const newMount = fixture(); + const pending = newMount.lifecycle.start(); + await reachMode(newMount); + newMount.mode.resolve('cpu_parallel'); + await pending; + assert.deepEqual(oldMount.applied, []); + assert.deepEqual(newMount.applied, ['cpu_parallel']); + assert.equal(newMount.counts().refreshStarts, 1); + newMount.lifecycle.stop(); +}); + +test('cleanup returned by refresh creation is released if creation destroys the mount', async () => { + let stops = 0; + let modeReads = 0; + const lifecycle = createDeviceConfigLifecycle({ + loadDevices: async () => {}, + startRefresh: () => { + lifecycle.stop(); + return () => { stops++; }; + }, + loadEmbeddingMemoryMode: async () => { modeReads++; return 'cpu_parallel'; }, + applyEmbeddingMemoryMode: () => { assert.fail('destroyed mount applied mode'); }, + onFailure: error => { assert.fail(String(error)); }, + }); + await lifecycle.start(); + assert.equal(stops, 1); + assert.equal(modeReads, 0); +}); diff --git a/src/components/deviceConfigLifecycle.ts b/src/components/deviceConfigLifecycle.ts new file mode 100644 index 000000000..6ee7b5435 --- /dev/null +++ b/src/components/deviceConfigLifecycle.ts @@ -0,0 +1,55 @@ +import type { EmbeddingMemoryMode } from '../services/ConfigService'; + +export interface DeviceConfigLifecycleOptions { + loadDevices(): Promise; + startRefresh(): () => void; + loadEmbeddingMemoryMode(): Promise; + applyEmbeddingMemoryMode(mode: EmbeddingMemoryMode): void; + onFailure(error: unknown): void; +} + +/** One mount owns initialization, late results, and the refresh cleanup. */ +export function createDeviceConfigLifecycle(options: DeviceConfigLifecycleOptions) { + let active = true; + let initialization: Promise | null = null; + let stopRefresh: (() => void) | null = null; + + const initialize = async () => { + if (!active) return; + try { + await options.loadDevices(); + if (!active) return; + + const cleanup = options.startRefresh(); + if (!active) { + cleanup(); + return; + } + stopRefresh = cleanup; + + const mode = await options.loadEmbeddingMemoryMode(); + if (active) options.applyEmbeddingMemoryMode(mode); + } catch (error) { + if (active) options.onFailure(error); + } + }; + + return { + start(): Promise { + if (!active) return initialization ?? Promise.resolve(); + // Assign ownership before initialization can invoke any callbacks. + initialization ??= Promise.resolve().then(initialize); + return initialization; + }, + stop(): void { + active = false; + stopRefresh?.(); + stopRefresh = null; + }, + isActive(): boolean { + return active; + }, + }; +} + +export type DeviceConfigLifecycle = ReturnType; From 9d2fe06f48002b9d5d09fdacc24f3047efa4692e Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 04:05:22 +0000 Subject: [PATCH 4/9] fix(graph): register mount teardown before loading definitions Return synchronous cleanup from application and package graph mounts. Preserve six listener identities/options and existing registrar exports. The shared graph mount owner guards late definition application and observes live/late rejection. Validation: 12 tests execute actual component mount callbacks plus the existing listener identity test; both synchronous-cleanup regressions fail against ca9. All 559 frontend tests, full lint, TypeScript, production build, staged critical/a11y/traceability and whitespace pass. GUI/native/model checks not run. Base: ca9edde6850cd58ade0b7e534bb4f58e704c2c64 (full ancestry retained). --- ...26-10-04-workflow-graph-mount-lifecycle.md | 42 ++++ .../src/components/WorkflowGraph.svelte | 17 +- packages/svelte-graph/src/index.ts | 3 +- .../src/workflowGraphWindowListeners.ts | 34 +++ src/components/WorkflowGraph.svelte | 17 +- .../frontendLifecycleContract.test.ts | 208 ++++++++++++++++++ 6 files changed, 306 insertions(+), 15 deletions(-) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-workflow-graph-mount-lifecycle.md create mode 100644 src/components/frontendLifecycleContract.test.ts diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-workflow-graph-mount-lifecycle.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-workflow-graph-mount-lifecycle.md new file mode 100644 index 000000000..58fdb82b9 --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-workflow-graph-mount-lifecycle.md @@ -0,0 +1,42 @@ +# WorkflowGraph synchronous teardown qualification + +Source base: `ca9edde6850cd58ade0b7e534bb4f58e704c2c64`, retaining the complete +PR44 stacked ancestry. Isolated branch: `repair/workflow-graph-mount-lifecycle`. + +This bounded FE-A02 slice addresses the two canonical graph mounts identified +by the frontend audit and retained in the active repository-review requirements. +Both components registered six window listeners, awaited node definitions in an +asynchronous mount callback, and only then returned the listener remover. Svelte +requires synchronous mount cleanup; the returned promise did not register the +remover. A late definition response also applied state after unmount, and a +rejected request escaped the mount callback without a terminal error observer. + +Both mounts now return cleanup synchronously. The shared graph-specific owner +registers the existing listeners with their existing identities and capture +options, owns one deferred definition read, and closes permanently at teardown. +Cleanup is idempotent; live definitions apply once, late responses settle without +application, and read failures are observed through the existing console error +channel while live. No backend request cancellation or connection/keyboard +policy changes are introduced. The existing listener registrar remains exported +for compatibility; the new mount helper and its options are additive exports. + +Twelve controlled contract tests parse each actual Svelte mount callback and +execute that callback with deferred services, recording stores/listener targets, +and the real production mount owner. They assert synchronous cleanup before +resolution, live application, exact handler cleanup, no keyboard callback after +cleanup, live/late rejection observation, immediate destruction, and independent +remount authority. Both synchronous-cleanup assertions fail against the exact +base component sources. The existing six-listener identity/options test also +passes. This tests the actual callback wiring, but does not execute Svelte DOM +mounting or native IPC. + +All 559 discovered frontend tests, full lint, TypeScript, production build, +staged critical/a11y/traceability (six paths, zero mapped impacts), and whitespace +checks pass; exact results are recorded in the milestone commit. The build retains its +existing stale Browserslist-data notice. Validation uses Node 24.12.0/npm 11.6.2. + +GUI, native/model execution, async connection intents and other graph operation +lifetimes are outside this mount/read slice. It does not close every FE-A02 +owner or claim native qualification. No frozen fixture/scanner/undo/view/device +source is changed. The branch is directly atop ca9; the integrator owns +independent review and integration. diff --git a/packages/svelte-graph/src/components/WorkflowGraph.svelte b/packages/svelte-graph/src/components/WorkflowGraph.svelte index fb9b334d5..ec13de0c3 100644 --- a/packages/svelte-graph/src/components/WorkflowGraph.svelte +++ b/packages/svelte-graph/src/components/WorkflowGraph.svelte @@ -68,7 +68,7 @@ import { resolveWorkflowReconnectResultDecision, resolveWorkflowReconnectStartDecision } from '../workflowGraphReconnect.js'; import { resolveWorkflowDragCursorUpdate } from '../workflowDragCursor.js'; import { resolveWorkflowGraphInteractionState } from '../workflowGraphInteraction.js'; - import { registerWorkflowGraphWindowListeners } from '../workflowGraphWindowListeners.js'; + import { createWorkflowGraphMount } from '../workflowGraphWindowListeners.js'; import { resolveWorkflowHorseshoeBlockedReasonLog } from '../workflowHorseshoeTrace.js'; import { resolveWorkflowGroupZoomTarget, @@ -211,17 +211,20 @@ } }); - onMount(async () => { - const removeWindowListeners = registerWorkflowGraphWindowListeners(window, { + onMount(() => { + const scope = createWorkflowGraphMount(window, { onKeyDown: handleWindowKeyDown, onPaletteDragEnd: handleWorkflowPaletteDragEnd, onPaletteDragStart: handleWorkflowPaletteDragStart, + }, { + loadNodeDefinitions: () => backend.getNodeDefinitions(), + applyNodeDefinitions: (definitions) => nodeDefsStore.set(definitions), + onFailure: (error) => { + console.error('[WorkflowGraph] Failed to load node definitions:', error); + }, }); - const definitions = await backend.getNodeDefinitions(); - nodeDefsStore.set(definitions); - - return removeWindowListeners; + return scope.stop; }); onDestroy(() => { diff --git a/packages/svelte-graph/src/index.ts b/packages/svelte-graph/src/index.ts index 43fa76938..b6b329db7 100644 --- a/packages/svelte-graph/src/index.ts +++ b/packages/svelte-graph/src/index.ts @@ -327,8 +327,9 @@ export { WORKFLOW_GRAPH_MIN_ZOOM, WORKFLOW_GRAPH_PAN_ACTIVATION_KEY, } from './workflowGraphViewport.js'; -export { registerWorkflowGraphWindowListeners } from './workflowGraphWindowListeners.js'; +export { createWorkflowGraphMount, registerWorkflowGraphWindowListeners } from './workflowGraphWindowListeners.js'; export type { + WorkflowGraphMountOptions, WorkflowGraphWindowListenerHandlers, WorkflowGraphWindowListenerTarget, } from './workflowGraphWindowListeners.js'; diff --git a/packages/svelte-graph/src/workflowGraphWindowListeners.ts b/packages/svelte-graph/src/workflowGraphWindowListeners.ts index 5d99bc8b1..795dc2186 100644 --- a/packages/svelte-graph/src/workflowGraphWindowListeners.ts +++ b/packages/svelte-graph/src/workflowGraphWindowListeners.ts @@ -44,3 +44,37 @@ export function registerWorkflowGraphWindowListeners( target.removeEventListener('blur', onPaletteDragEnd); }; } + +export interface WorkflowGraphMountOptions { + loadNodeDefinitions(): Promise; + applyNodeDefinitions(definitions: Definitions): void; + onFailure(error: unknown): void; +} + +/** Register synchronously; the same mount owns the deferred definitions read. */ +export function createWorkflowGraphMount( + target: WorkflowGraphWindowListenerTarget, + handlers: WorkflowGraphWindowListenerHandlers, + options: WorkflowGraphMountOptions, +): { stop(): void; ready: Promise } { + let active = true; + const removeListeners = registerWorkflowGraphWindowListeners(target, handlers); + const ready = Promise.resolve().then(async () => { + if (!active) return; + try { + const definitions = await options.loadNodeDefinitions(); + if (active) options.applyNodeDefinitions(definitions); + } catch (error) { + if (active) options.onFailure(error); + } + }); + + return { + ready, + stop() { + if (!active) return; + active = false; + removeListeners(); + }, + }; +} diff --git a/src/components/WorkflowGraph.svelte b/src/components/WorkflowGraph.svelte index 7db301f18..94e8c8bc5 100644 --- a/src/components/WorkflowGraph.svelte +++ b/src/components/WorkflowGraph.svelte @@ -8,7 +8,7 @@ createConnectionDragState, createHorseshoeDragSessionState, createHorseshoeInsertFeedbackState, isPortTypeCompatible, markConnectionDragFinalizing, normalizeWorkflowHorseshoeSelectedIndex, - registerWorkflowGraphWindowListeners, rejectHorseshoeInsertFeedback, + createWorkflowGraphMount, rejectHorseshoeInsertFeedback, requestWorkflowHorseshoeOpen, resolveWorkflowDragCursorUpdate, resolveWorkflowDeleteSelectionRequest, resolveWorkflowGraphInteractionState, resolveWorkflowGroupZoomTarget, @@ -304,17 +304,20 @@ } }); - onMount(async () => { - const removeWindowListeners = registerWorkflowGraphWindowListeners(window, { + onMount(() => { + const scope = createWorkflowGraphMount(window, { onKeyDown: handleWindowKeyDown, onPaletteDragEnd: handleWorkflowPaletteDragEnd, onPaletteDragStart: handleWorkflowPaletteDragStart, + }, { + loadNodeDefinitions: () => workflowService.getNodeDefinitions(), + applyNodeDefinitions: (definitions) => nodeDefinitions.set(definitions), + onFailure: (error) => { + console.error('[WorkflowGraph] Failed to load node definitions:', error); + }, }); - const definitions = await workflowService.getNodeDefinitions(); - nodeDefinitions.set(definitions); - - return removeWindowListeners; + return scope.stop; }); onDestroy(() => { diff --git a/src/components/frontendLifecycleContract.test.ts b/src/components/frontendLifecycleContract.test.ts new file mode 100644 index 000000000..6d9cadb00 --- /dev/null +++ b/src/components/frontendLifecycleContract.test.ts @@ -0,0 +1,208 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import vm from 'node:vm'; +import { parse } from 'svelte/compiler'; +import { + createWorkflowGraphMount, + registerWorkflowGraphWindowListeners, + type WorkflowGraphWindowListenerTarget, +} from '../../packages/svelte-graph/src/workflowGraphWindowListeners.ts'; + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (error: unknown) => void; + const promise = new Promise((accept, fail) => { + resolve = accept; + reject = fail; + }); + return { promise, resolve, reject }; +} + +interface Listener { + type: string; + callback: EventListenerOrEventListenerObject; + options?: boolean | AddEventListenerOptions | EventListenerOptions; +} + +function mountFixture(path: URL) { + const source = readFileSync(path, 'utf8'); + const script = parse(source, { modern: true }).instance; + assert.ok(script); + const statement = script.content.body.find(node => + node.type === 'ExpressionStatement' && + node.expression.type === 'CallExpression' && + node.expression.callee.type === 'Identifier' && + node.expression.callee.name === 'onMount', + ); + assert.ok(statement && statement.type === 'ExpressionStatement'); + assert.equal(statement.expression.type, 'CallExpression'); + const expression = statement.expression; + assert.ok(expression.type === 'CallExpression'); + const callback = expression.arguments[0]; + assert.ok(callback && callback.type === 'ArrowFunctionExpression'); + assert.ok('start' in callback && typeof callback.start === 'number'); + assert.ok('end' in callback && typeof callback.end === 'number'); + const added: Listener[] = []; + const removed: Listener[] = []; + const active = new Set(); + const target: WorkflowGraphWindowListenerTarget = { + addEventListener(type, listener, options) { + const record = { type, callback: listener, options }; + added.push(record); + active.add(record); + }, + removeEventListener(type, listener, options) { + for (const record of active) { + if (record.type === type && record.callback === listener && record.options === options) { + active.delete(record); + removed.push(record); + break; + } + } + }, + }; + const requests: ReturnType>[] = []; + const scopes: { ready: Promise }[] = []; + const applied: unknown[] = []; + const failures: unknown[][] = []; + let keyboardActions = 0; + const service = { + getNodeDefinitions: () => { + const request = deferred(); + requests.push(request); + return request.promise; + }, + }; + const store = { set: (value: unknown) => { applied.push(value); } }; + const mount = vm.runInNewContext(`(${source.slice(callback.start, callback.end)})`, { + window: target, + registerWorkflowGraphWindowListeners, + createWorkflowGraphMount: (...args: Parameters) => { + const scope = createWorkflowGraphMount(...args); + scopes.push(scope); + return scope; + }, + workflowService: service, + backend: service, + nodeDefinitions: store, + nodeDefsStore: store, + handleWindowKeyDown: () => { keyboardActions++; }, + handleWorkflowPaletteDragEnd: () => {}, + handleWorkflowPaletteDragStart: () => {}, + console: { error: (...args: unknown[]) => { failures.push(args); } }, + }) as () => unknown; + + return { + mount, active, added, removed, requests, scopes, applied, failures, + keyboardActions: () => keyboardActions, + keyDown: () => { + for (const record of active) { + if (record.type === 'keydown' && typeof record.callback === 'function') { + record.callback(new Event('keydown')); + } + } + }, + }; +} + +for (const [name, path] of [ + ['application', new URL('./WorkflowGraph.svelte', import.meta.url)], + ['package', new URL('../../packages/svelte-graph/src/components/WorkflowGraph.svelte', import.meta.url)], +] as const) { + test(`${name} graph mount returns synchronous cleanup before definitions resolve`, async () => { + const f = mountFixture(path); + const result = f.mount(); + try { + assert.equal(typeof result, 'function'); + assert.equal(f.active.size, 6); + await Promise.resolve(); + assert.equal(f.requests.length, 1); + (result as () => void)(); + f.requests[0].resolve(['late']); + await f.scopes[0].ready; + assert.equal(f.active.size, 0); + assert.deepEqual(f.removed, f.added); + assert.deepEqual(f.applied, []); + } finally { + // Also settle the old asynchronous mount in the baseline regression run. + f.requests[0]?.resolve(['late']); + if (typeof result === 'function') result(); + else { + const cleanup = await result; + if (typeof cleanup === 'function') cleanup(); + } + } + }); + + test(`${name} graph applies live definitions and removes each handler once`, async () => { + const f = mountFixture(path); + const stop = f.mount() as () => void; + await Promise.resolve(); + f.requests[0].resolve(['live']); + await f.scopes[0].ready; + assert.deepEqual(f.applied, [['live']]); + f.keyDown(); + assert.equal(f.keyboardActions(), 1); + stop(); + stop(); + f.keyDown(); + assert.equal(f.keyboardActions(), 1); + assert.equal(f.active.size, 0); + assert.equal(f.removed.length, 6); + }); + + test(`${name} graph observes live definition failure and retains cleanup`, async () => { + const f = mountFixture(path); + const stop = f.mount() as () => void; + await Promise.resolve(); + const failure = new Error('definitions unavailable'); + f.requests[0].reject(failure); + await f.scopes[0].ready; + assert.equal(f.failures.length, 1); + assert.equal(f.failures[0][1], failure); + assert.deepEqual(f.applied, []); + stop(); + assert.equal(f.active.size, 0); + }); + + test(`${name} graph observes late rejection without an unmounted error callback`, async () => { + const f = mountFixture(path); + const stop = f.mount() as () => void; + await Promise.resolve(); + stop(); + f.requests[0].reject(new Error('late failure')); + await f.scopes[0].ready; + assert.deepEqual(f.failures, []); + assert.equal(f.active.size, 0); + }); + + test(`${name} graph destroyed immediately starts no definitions request`, async () => { + const f = mountFixture(path); + const stop = f.mount() as () => void; + stop(); + await f.scopes[0].ready; + assert.equal(f.requests.length, 0); + assert.equal(f.active.size, 0); + }); + + test(`${name} graph remount owns six listeners and rejects the prior mount result`, async () => { + const f = mountFixture(path); + const stopFirst = f.mount() as () => void; + await Promise.resolve(); + stopFirst(); + const stopSecond = f.mount() as () => void; + await Promise.resolve(); + assert.equal(f.active.size, 6); + f.requests[1].resolve(['current']); + await f.scopes[1].ready; + f.requests[0].resolve(['old']); + await f.scopes[0].ready; + assert.deepEqual(f.applied, [['current']]); + f.keyDown(); + assert.equal(f.keyboardActions(), 1); + stopSecond(); + assert.equal(f.active.size, 0); + assert.equal(f.removed.length, 12); + }); +} From bb45b81f72b258f71121a794184eef812769632d Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 04:28:18 +0000 Subject: [PATCH 5/9] docs: record combined accepted frontend qualification Record exact ancestry-preserving composition of accepted undo, view, device and graph mount repairs. Combined focused 73 and aggregate 619 frontend tests pass, as do full lint, TypeScript, build, explicit ca9-to-composition critical/a11y/ traceability and whitespace checks. Preserve native/visual/IPC/model limits. Post-base fixture/scanner branches are excluded; no PR44/47 ref changes. Record a controlled malformed-view-record probe as the next bounded candidate, without implementing or accepting additional persistence policy. --- ...026-10-04-frontend-accepted-composition.md | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-frontend-accepted-composition.md diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-frontend-accepted-composition.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-frontend-accepted-composition.md new file mode 100644 index 000000000..416bdc03c --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-frontend-accepted-composition.md @@ -0,0 +1,89 @@ +# Accepted frontend composition qualification + +Isolated branch: `qualification/frontend-accepted-composition`. +Source base: `ca9edde6850cd58ade0b7e534bb4f58e704c2c64`. +Composition commit: `81ced09473d932a47aa55161421abdfa43bfc7f5`. +Qualified source tree: `746fd66ed1062565ab03eca39f1845b26754c427`. + +## Exact admitted source and isolation + +The parent reported independent acceptance of these four exact repairs: + +| Repair | Exact accepted commit | +| --- | --- | +| Undo persistence validation | `b504b8387dcf8e2cebe1dd28674e6f66d9eec115` | +| View animation and persistence timer lifetime | `934147092e33b1adfb0d0c17424dd12e22608efa` | +| DeviceConfig initialization lifetime | `6a3ef2cf8a42c0ef0c0823c3d5af5384774a0a44` | +| Application/package graph mount lifetime | `9d2fe06f48002b9d5d09fdacc24f3047efa4692e` | + +A single ancestry-preserving composition commit has ca9 and all four accepted +commits as parents. The 19 changed source/documentation paths are exactly the +union of those disjoint repairs. Each changed blob matches its accepted source +commit. Graph consumers, helper implementation and public exports are composed +together. Remote source heads were read and matched before composition. + +Post-base native fixture/report branch `827b1b264a5918e2339c503f447aeb01008ac9b4` +and scanner branch `9996ddfd8b4c15c11de4d7141e20e0165dd0f12c` are absent from this +ancestry and write set. No scripts, workflows, Rust/native source, credentials, +network settings, protected PR44/47 refs or external review metadata were changed. +This frontend milestone is independent of the separately blocked native-qualified +publication. The final report successor changes only this Markdown evidence file. + +## Actual combined checks + +Environment: published Debian cloud checkout, Node 24.12.0/npm 11.6.2 matching +repository pins. Existing installed dependencies passed `npm ls --depth=0`. +No dependencies or toolchain pins were changed. + +| Command | Actual result | +| --- | --- | +| `node --experimental-strip-types --test src/stores/undoPersistence.test.ts packages/svelte-graph/src/stores/createViewStores.test.ts src/components/deviceConfigLifecycle.test.ts src/components/frontendLifecycleContract.test.ts packages/svelte-graph/src/workflowGraphWindowListeners.test.ts` | 73 passed | +| `npm run test:frontend` | 619 passed; zero failed/skipped/cancelled | +| `npm run lint:full` | passed | +| `npm run typecheck` | passed | +| `npm run build` | passed; existing stale Browserslist-data notice | +| `TRACEABILITY_MODE=range TRACEABILITY_BASE_REF=ca9edde6850cd58ade0b7e534bb4f58e704c2c64 TRACEABILITY_HEAD_REF=81ced09473d932a47aa55161421abdfa43bfc7f5 npm run lint:no-new` | critical, nine accessibility scanner tests, accessibility scan and traceability passed; 19 paths, zero mapped impacts | +| `git diff --check ca9edde6850cd58ade0b7e534bb4f58e704c2c64 HEAD` | passed | + +The final documentation successor also receives explicit ca9-to-head traceability +and whitespace checks. Local detailed logs use the prefix +`/workspace/pantograph-cache/frontend-combined-`; these cache files are not +assumed to transfer to a fresh environment. The tracked results above preserve +the meaningful evidence. No fresh hosted workflow or external review is claimed. + +This is combined frontend qualification. Controlled timers, deferred services, +recording storage and execution of actual mount callbacks do not execute Svelte +DOM interactions, desktop IPC, package/native loading or model inference. Those +checks were not executed. Visual, keyboard/focus behavior in WebKit/Tauri, +real cold reopen, release artifacts, all FE-A02 owners and whole FE-A05 acceptance +remain unqualified. The parent owns final all-source integration and review. + +## Next bounded source candidate: view record validation + +The active repository review retains FE-A05 browser persistence bounds, versions, +migration and typed outcomes before application. Its historical owner-specific +population includes `createViewStores.ts`; the successor transfers those +obligations rather than accepting them by supersession. + +On this combined source, `restoreViewState()` still trusts parsed fields before +applying them. A controlled in-memory storage probe supplies +`{"viewLevel":"group","orchestrationId":42,"dataGraphId":"graph","groupStack":{"length":1}}`. +The actual store reports `true`, applies the numeric identity and object stack, +and breadcrumb evaluation throws `TypeError: $groupStack is not iterable`. +An explicit persistence call writes that malformed state back. No real browser +storage is read or written by this probe. Its source and output are retained as +`/workspace/pantograph-cache/view-persistence-gap-probe.mjs` and `.log`. + +A proportionate next goal is an owner-local decoder for the existing unversioned +view record, validating its enum, optional identities and bounded string stack +before any application, with controlled malformed/nested/unavailable/repeated/ +isolated-store evidence and preservation of rejected bytes. Preserve valid record +serialization and the accepted navigation/timer lifetime. Inject a small storage +Interface where needed for evidence; do not create a universal registry or alter +backend domain authority. Future-version/migration and public outcome changes +need an explicitly admitted compatibility scope; this milestone does not choose +those policies or implement this candidate. + +Other inspected persistence owners, including package/application last-graph +restore, retain raw parsed-record trust. They are separate candidates, not part +of this view-record proposal or the accepted composition write set. From fd1ace640ddb51ef906991a6d658707d5ef034f5 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 05:02:11 +0000 Subject: [PATCH 6/9] fix(graph): validate view records before restore and writeback Decode the existing unversioned view record before applying any owned field. Retain malformed/unavailable bytes and close instance writeback on rejection. Initialize before the first persistence scope; validate later enables without replacing local edits. Suppress synchronous restore-time writes and recheck storage/outgoing values before each write. Preserve shared timer ownership, partial legacy records, serialization and existing public action return types. No schema migration, recovery writes or new product limits are introduced. Validation: 55 focused cases, including all 15 accepted view lifecycle tests; 659 aggregate frontend tests; full lint, TypeScript, build, critical/a11y/ traceability (7 paths, 0 mapped impacts), whitespace pass. Two defect regressions fail against exact bb45; two restart compatibility controls pass on base/repair. GUI, actual native IPC, model and real cold-reopen checks were not executed. Base: bb45b81f72b258f71121a794184eef812769632d; accepted source ancestry retained. --- .../2026-10-04-view-record-validation.md | 81 ++++ packages/svelte-graph/README.md | 20 + .../createViewStores.persistence.test.ts | 426 ++++++++++++++++++ .../src/stores/createViewStores.test.ts | 5 +- .../src/stores/createViewStores.ts | 115 +++-- .../src/stores/viewStatePersistence.test.ts | 42 ++ .../src/stores/viewStatePersistence.ts | 38 ++ 7 files changed, 699 insertions(+), 28 deletions(-) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-record-validation.md create mode 100644 packages/svelte-graph/src/stores/createViewStores.persistence.test.ts create mode 100644 packages/svelte-graph/src/stores/viewStatePersistence.test.ts create mode 100644 packages/svelte-graph/src/stores/viewStatePersistence.ts diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-record-validation.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-record-validation.md new file mode 100644 index 000000000..e66de2be2 --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-view-record-validation.md @@ -0,0 +1,81 @@ +# View record validation qualification + +Isolated successor branch: `repair/view-record-validation`. +Exact base: `bb45b81f72b258f71121a794184eef812769632d`, tree +`e1d1e723cd8818d1b3f5760feeec64b2faa52a46`. The accepted frontend composition +and all four reviewed source commits remain unchanged in ancestry. Post-base +native fixture/report `827b1b` and scanner `9996dd` branches remain excluded. + +## Reproduced FE-A05 defect and bounded repair + +The active repository review retains browser record validation before application. +On the exact base, a record with a numeric orchestration identity and an object +instead of a string stack changes state, reports success without subscribers, +and breaks breadcrumb iteration. With an active breadcrumb subscriber, restore +reports failure but still leaves the malformed fields applied. Enabling +persistence also installs writeback without reading the malformed stored bytes, +allowing its initial debounce to overwrite them with default/local state. + +Two focused regressions fail against the exact base component store source: +no state application on rejection, and no subscription installation for malformed +storage. The original script probe also fails before this repair. After repair, +it returns false, retains default valid state, evaluates breadcrumbs without an +error and records zero writes. All probes use controlled storage rather than a +real user's browser data. + +The 38-line owner-local decoder validates every present owned field of the +existing unversioned record: the three view-level literals, nullable string +identities, and an array containing only strings. Numbers, including nonfinite +values produced by JSON exponent overflow, are invalid in these owned fields. +No numeric viewport or animation field belongs to this persisted record. +Missing legacy fields retain current state; explicit null identities clear their +prior values. Empty strings, arbitrary existing identity lengths/group counts, +partial/empty records and ignored extension fields remain compatible. No new +schema version, migration, domain inference or product bound is introduced. + +Initial persistence startup validates and restores before subscription callbacks +can schedule defaults. An explicit restore or previous persistence scope marks +initialization complete; re-enabling validates storage without replacing later +local edits. Duplicate owners still share one debounce and final release cancels +its timer. Synchronous subscriber writes are suppressed during restoration; +existing pending work snapshots the complete restored state. Before every write, +the owner rechecks storage and validates the outgoing writable-store snapshot. + +Invalid/unavailable storage, invalid outgoing data or a failed write closes +writeback for that instance and cancels its subscriptions/pending timer. Original +bytes are retained; local store operations remain available. An explicit later +restore may apply externally corrected valid data to memory, but never resumes +writes. There are no automatic recovery writes. A fresh instance validates again. +The additive optional `ViewStoreOptions.storage` uses the existing browser +getItem/setItem Interface; default browser localStorage and public action return +types remain. The existing timer fixture now supplies getItem as well as setItem. + +## Actual evidence and limits + +Validation uses repository-pinned Node 24.12.0/npm 11.6.2. No dependencies, +permissions, credentials or network settings changed. + +- 55 focused cases pass: 40 new decoder/storage cases plus all 15 accepted view + lifecycle cases. They cover breadcrumb iteration, partial/null/repeated + restores, malformed JSON/root/nested types, nonfinite owned fields, valid + roundtrip serialization, subscriber/writeback ordering, unavailable storage, + failed writes, isolated storage/instances and retained-byte behavior. +- Two compatibility regressions exposed a repeated-start reset in the initial + repair draft; the initialization phase fixes it. Both compatibility cases pass + on the exact baseline and final repair, protecting edits between scopes or + after an explicit restore. +- All 659 discovered frontend tests pass, with zero failures/skips/cancellations. +- Full lint, TypeScript and production build pass. The build retains its existing + stale Browserslist-data notice. +- Staged critical checks, nine accessibility scanner tests, accessibility scan, + traceability (seven paths, zero mapped impacts), and whitespace checks pass. + Exact final source identities are recorded in the milestone commit and handoff; + semantic contract review remains parent-owned. + +Detailed local evidence uses `/workspace/pantograph-cache/view-record-*`; these +cache files are not assumed to transfer to a fresh environment. This tracked +report preserves the actual results and scope. Native IPC, GUI/WebKit interaction, +real browser cold reopen, model loading/inference and release acceptance were not +executed. Whole FE-A05 acceptance, including other record owners and future-version +policy, remains open. Parent owns independent review and final all-source +integration. No PR44/47 refs or external review requests are changed here. diff --git a/packages/svelte-graph/README.md b/packages/svelte-graph/README.md index 3dfc59ed3..38df2d16c 100644 --- a/packages/svelte-graph/README.md +++ b/packages/svelte-graph/README.md @@ -49,3 +49,23 @@ store. Each `enablePersistence()` call returns an independent, idempotent releas handle. Persistence remains active until the last handle is released; final release unsubscribes and cancels the pending write. Re-enabling starts a fresh scope. The existing storage shape and 500 ms debounce are unchanged. + + +View persistence reads the existing unversioned record's `viewLevel`, nullable +string `orchestrationId`/`dataGraphId`, and string-array `groupStack`. It validates +all present owned fields before applying any of them. Missing legacy fields leave +current state intact; explicit null identities clear that identity. Empty strings, +partial records and ignored extension fields remain compatible. No new length or +group-count limit or schema migration is introduced. + +`enablePersistence()` initializes valid stored state before installing the first +shared writeback scope. After an explicit restore or earlier scope, enabling +validates storage without replacing current edits. Each write rechecks existing +storage and validates its outgoing snapshot. Restoration suppresses synchronous +subscriber write attempts; an already pending debounce observes the complete +restored snapshot. Invalid JSON/records or unavailable storage stop writeback +for that instance and retain +the stored bytes. Later reads may update memory from externally corrected data, +but they do not restart writes. There are no recovery writes. A fresh instance +must validate storage again. `ViewStoreOptions.storage` can supply a per-instance +`getItem`/`setItem` boundary; it otherwise uses browser `localStorage`. diff --git a/packages/svelte-graph/src/stores/createViewStores.persistence.test.ts b/packages/svelte-graph/src/stores/createViewStores.persistence.test.ts new file mode 100644 index 000000000..674b37d76 --- /dev/null +++ b/packages/svelte-graph/src/stores/createViewStores.persistence.test.ts @@ -0,0 +1,426 @@ +import assert from 'node:assert/strict'; +import test, { type TestContext } from 'node:test'; +import { get } from 'svelte/store'; +import { createViewStores } from './createViewStores.ts'; + +function storageFixture(initial: Record = {}) { + const values = new Map(Object.entries(initial)); + const writes: { key: string; value: string }[] = []; + let reads = 0; + const storage = { + getItem(key: string): string | null { + reads++; + return values.get(key) ?? null; + }, + setItem(key: string, value: string): void { + writes.push({ key, value }); + values.set(key, value); + }, + }; + return { storage, values, writes, reads: () => reads }; +} + +function useStorage(t: TestContext, storage: Pick) { + const previous = Object.getOwnPropertyDescriptor(globalThis, 'localStorage'); + Object.defineProperty(globalThis, 'localStorage', { configurable: true, value: storage }); + t.after(() => { + if (previous) Object.defineProperty(globalThis, 'localStorage', previous); + else Reflect.deleteProperty(globalThis, 'localStorage'); + }); + t.mock.timers.enable({ apis: ['setTimeout'] }); +} + +function snapshot(view: ReturnType) { + return { + viewLevel: get(view.viewLevel), + orchestrationId: get(view.currentOrchestrationId), + dataGraphId: get(view.currentDataGraphId), + groupStack: get(view.groupStack), + }; +} + +const malformed = '{"viewLevel":"group","orchestrationId":42,"dataGraphId":"graph","groupStack":{"length":1}}'; + +test('rejects malformed record without state application or broken breadcrumb iteration', (t) => { + const f = storageFixture({ view: malformed }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + view.currentOrchestrationId.set('existing'); + const before = snapshot(view); + const breadcrumbs: unknown[] = []; + const release = view.breadcrumb.subscribe(value => { breadcrumbs.push(value); }); + t.after(release); + assert.equal(view.restoreViewState(), false); + assert.deepEqual(snapshot(view), before); + assert.equal(breadcrumbs.length, 1); + assert.deepEqual(get(view.breadcrumb), [{ id: 'existing', name: 'existing', level: 'orchestration' }]); + view.persistViewState(); + assert.equal(f.values.get('view'), malformed); + assert.equal(f.writes.length, 0); +}); + +test('enable persistence validates before installing writeback', (t) => { + const f = storageFixture({ view: malformed }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + let subscriptions = 0; + const subscribe = view.viewLevel.subscribe; + view.viewLevel.subscribe = (...args) => { subscriptions++; return subscribe(...args); }; + const releaseA = view.enablePersistence(); + const releaseB = view.enablePersistence(); + assert.equal(subscriptions, 0); + view.groupStack.set(['local']); + t.mock.timers.tick(1000); + assert.equal(f.writes.length, 0); + assert.equal(f.values.get('view'), malformed); + releaseA(); + releaseA(); + releaseB(); +}); + +for (const [name, record] of [ + ['malformed JSON', '{'], + ['empty bytes', ''], + ['null root', 'null'], + ['array root', '[]'], + ['string root', '"view"'], + ['numeric root', '42'], + ['boolean root', 'false'], + ['unknown level', '{"viewLevel":"other"}'], + ['null level', '{"viewLevel":null}'], + ['nonfinite identity', '{"dataGraphId":1e309}'], + ['boolean identity', '{"orchestrationId":false}'], + ['object identity', '{"dataGraphId":{"id":"graph"}}'], + ['null stack', '{"groupStack":null}'], + ['string stack', '{"groupStack":"group"}'], + ['mixed stack', '{"groupStack":["group",42]}'], + ['nested stack', '{"groupStack":[["group"]]}'], +] as const) { + test(`invalid stored ${name} is retained across restore, explicit writes and enable`, (t) => { + const f = storageFixture({ view: record }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const before = snapshot(view); + assert.equal(view.restoreViewState(), false); + assert.equal(view.restoreViewState(), false); + assert.deepEqual(snapshot(view), before); + view.persistViewState(); + const release = view.enablePersistence(); + t.mock.timers.tick(1000); + release(); + assert.equal(f.values.get('view'), record); + assert.equal(f.writes.length, 0); + }); +} + +test('valid complete record roundtrips with iterable breadcrumbs and unchanged serialization', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + const first = createViewStores({ storageKey: 'view', storage: f.storage }); + first.viewLevel.set('group'); + first.currentOrchestrationId.set('orchestration'); + first.currentDataGraphId.set('graph'); + first.groupStack.set(['outer', 'inner']); + first.persistViewState(); + const expected = snapshot(first); + assert.equal(f.values.get('view'), JSON.stringify(expected)); + const second = createViewStores({ storageKey: 'view', storage: f.storage }); + assert.equal(second.restoreViewState(), true); + assert.deepEqual(snapshot(second), expected); + assert.deepEqual(get(second.breadcrumb), [ + { id: 'orchestration', name: 'orchestration', level: 'orchestration' }, + { id: 'graph', name: 'graph', level: 'data-graph' }, + { id: 'outer', name: 'outer', level: 'group' }, + { id: 'inner', name: 'inner', level: 'group' }, + ]); +}); + +test('partial and empty legacy records retain missing fields and tolerate ignored extensions', (t) => { + const f = storageFixture({ view: '{"viewLevel":"group","legacyExtension":{"enabled":true}}' }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + view.currentDataGraphId.set('existing'); + view.groupStack.set(['existing-group']); + assert.equal(view.restoreViewState(), true); + assert.deepEqual(snapshot(view), { + viewLevel: 'group', orchestrationId: null, dataGraphId: 'existing', groupStack: ['existing-group'], + }); + f.values.set('view', '{}'); + const before = snapshot(view); + assert.equal(view.restoreViewState(), true); + assert.deepEqual(snapshot(view), before); + assert.equal(f.writes.length, 0); +}); + +test('repeated complete restoration clears explicit null identities without writing', (t) => { + const f = storageFixture({ view: '{"viewLevel":"group","orchestrationId":"old","dataGraphId":"old-graph","groupStack":["old-group"]}' }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + assert.equal(view.restoreViewState(), true); + const cleared = '{"viewLevel":"data-graph","orchestrationId":null,"dataGraphId":null,"groupStack":[]}'; + f.values.set('view', cleared); + assert.equal(view.restoreViewState(), true); + assert.equal(view.restoreViewState(), true); + assert.deepEqual(snapshot(view), { viewLevel: 'data-graph', orchestrationId: null, dataGraphId: null, groupStack: [] }); + assert.deepEqual(get(view.breadcrumb), []); + assert.equal(f.values.get('view'), cleared); + assert.equal(f.writes.length, 0); +}); + +test('enable restores valid data before initial subscription writeback can replace it', (t) => { + const record = { viewLevel: 'group', orchestrationId: 'o', dataGraphId: 'g', groupStack: ['nested'] }; + const f = storageFixture({ view: JSON.stringify(record) }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const releaseA = view.enablePersistence(); + assert.deepEqual(snapshot(view), record); + view.groupStack.set(['local']); + const releaseB = view.enablePersistence(); + assert.deepEqual(get(view.groupStack), ['local'], 'duplicate enable cannot restore over newer state'); + t.mock.timers.tick(499); + assert.equal(f.writes.length, 0); + t.mock.timers.tick(1); + assert.equal(f.writes.length, 1); + assert.deepEqual(JSON.parse(f.writes[0].value), { ...record, groupStack: ['local'] }); + releaseA(); + releaseB(); +}); + +test('restore suppresses synchronous subscriber writes before the pending snapshot', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const releasePersistence = view.enablePersistence(); + let observingRestore = false; + const releaseObserver = view.groupStack.subscribe(() => { + if (observingRestore) view.persistViewState(); + }); + const record = { viewLevel: 'group', orchestrationId: 'o', dataGraphId: 'g', groupStack: ['restored'] }; + const bytes = JSON.stringify(record); + f.values.set('view', bytes); + observingRestore = true; + assert.equal(view.restoreViewState(), true); + assert.deepEqual(snapshot(view), record); + assert.equal(f.writes.length, 0, 'no partial record can be written during restore'); + t.mock.timers.tick(500); + assert.equal(f.values.get('view'), bytes); + assert.equal(f.writes.length, 1); + assert.deepEqual(JSON.parse(f.writes[0].value), record); + observingRestore = false; + view.groupStack.set(['later']); + t.mock.timers.tick(500); + assert.deepEqual(JSON.parse(f.writes[1].value), { ...record, groupStack: ['later'] }); + releaseObserver(); + releasePersistence(); +}); + +test('malformed storage appearing before a pending write is rechecked and retained', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const release = view.enablePersistence(); + f.values.set('view', malformed); + t.mock.timers.tick(500); + view.groupStack.set(['later']); + view.persistViewState(); + t.mock.timers.tick(1000); + assert.equal(f.values.get('view'), malformed); + assert.equal(f.writes.length, 0); + release(); + release(); +}); + +test('invalid repeated restore cancels an active scope and cannot restart implicit writes', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const releaseA = view.enablePersistence(); + const releaseB = view.enablePersistence(); + view.groupStack.set(['local']); + f.values.set('view', malformed); + const before = snapshot(view); + assert.equal(view.restoreViewState(), false); + assert.deepEqual(snapshot(view), before); + t.mock.timers.tick(1000); + const releaseC = view.enablePersistence(); + releaseA(); + releaseB(); + releaseC(); + assert.equal(f.writes.length, 0); + assert.equal(f.values.get('view'), malformed); +}); + +test('valid explicit restore after rejection can update memory but cannot recover writeback', (t) => { + const f = storageFixture({ view: malformed }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + assert.equal(view.restoreViewState(), false); + const repairedBytes = '{"viewLevel":"group","groupStack":["external-repair"]}'; + f.values.set('view', repairedBytes); + assert.equal(view.restoreViewState(), true); + assert.deepEqual(get(view.groupStack), ['external-repair']); + view.persistViewState(); + const release = view.enablePersistence(); + t.mock.timers.tick(500); + release(); + assert.equal(f.values.get('view'), repairedBytes); + assert.equal(f.writes.length, 0); +}); + +test('unavailable storage leaves state intact and no later call starts recovery writes', (t) => { + const f = storageFixture(); + let unavailable = true; + const storage = { + getItem: (key: string) => { + if (unavailable) throw new Error('storage denied'); + return f.storage.getItem(key); + }, + setItem: f.storage.setItem, + }; + useStorage(t, storage); + const view = createViewStores({ storageKey: 'view', storage }); + view.currentDataGraphId.set('local'); + const before = snapshot(view); + assert.equal(view.restoreViewState(), false); + assert.deepEqual(snapshot(view), before); + unavailable = false; + view.persistViewState(); + const release = view.enablePersistence(); + t.mock.timers.tick(500); + release(); + assert.equal(f.writes.length, 0); +}); + +test('ambient storage accessor failure is observed without attempting writeback', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + Object.defineProperty(globalThis, 'localStorage', { + configurable: true, + get: () => { throw new Error('storage accessor denied'); }, + }); + const view = createViewStores({ storageKey: 'view' }); + assert.equal(view.restoreViewState(), false); + const release = view.enablePersistence(); + view.persistViewState(); + t.mock.timers.tick(500); + release(); + assert.equal(f.writes.length, 0); +}); + +test('setItem failure closes the scope and prevents automatic retry writes', (t) => { + const f = storageFixture(); + let attempts = 0; + const storage = { + getItem: f.storage.getItem, + setItem: () => { attempts++; throw new Error('storage full'); }, + }; + useStorage(t, storage); + const view = createViewStores({ storageKey: 'view', storage }); + const release = view.enablePersistence(); + t.mock.timers.tick(500); + assert.equal(attempts, 1); + view.groupStack.set(['later']); + view.persistViewState(); + t.mock.timers.tick(1000); + assert.equal(attempts, 1); + assert.equal(f.writes.length, 0); + release(); +}); + +test('isolated views with the same key keep injected storage and rejection authority separate', (t) => { + const bad = storageFixture({ view: malformed }); + const good = storageFixture({ view: '{"viewLevel":"group","groupStack":["good"]}' }); + useStorage(t, bad.storage); + const first = createViewStores({ storageKey: 'view', storage: bad.storage }); + const second = createViewStores({ storageKey: 'view', storage: good.storage }); + const releaseFirst = first.enablePersistence(); + const releaseSecond = second.enablePersistence(); + assert.deepEqual(get(second.groupStack), ['good']); + t.mock.timers.tick(500); + assert.deepEqual(bad.writes, []); + assert.equal(bad.values.get('view'), malformed); + assert.equal(good.writes.length, 1); + releaseFirst(); + releaseSecond(); +}); + +test('public writable values are validated before they can corrupt a valid record', (t) => { + const bytes = '{"viewLevel":"data-graph","orchestrationId":null,"dataGraphId":null,"groupStack":[]}'; + const f = storageFixture({ view: bytes }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + // JavaScript consumers can bypass TypeScript at the public writable boundary. + Reflect.apply(view.currentDataGraphId.set, undefined, [Infinity]); + view.persistViewState(); + assert.equal(f.values.get('view'), bytes); + assert.equal(f.writes.length, 0); +}); + +test('missing record and disabled key preserve normal in-memory behavior', (t) => { + const f = storageFixture(); + useStorage(t, f.storage); + const missing = createViewStores({ storageKey: 'view', storage: f.storage }); + assert.equal(missing.restoreViewState(), false); + missing.persistViewState(); + assert.equal(f.writes.length, 1); + const reads = f.reads(); + const disabled = createViewStores({ storage: f.storage }); + disabled.groupStack.set(['local']); + assert.equal(disabled.restoreViewState(), false); + disabled.persistViewState(); + const release = disabled.enablePersistence(); + t.mock.timers.tick(500); + release(); + assert.equal(f.reads(), reads); + assert.equal(f.writes.length, 1); +}); + +test('storage becoming unavailable before a pending write closes only its active scope', (t) => { + const f = storageFixture(); + let unavailable = false; + const storage = { + getItem: (key: string) => { + if (unavailable) throw new Error('storage denied'); + return f.storage.getItem(key); + }, + setItem: f.storage.setItem, + }; + useStorage(t, storage); + const view = createViewStores({ storageKey: 'view', storage }); + const release = view.enablePersistence(); + unavailable = true; + t.mock.timers.tick(500); + unavailable = false; + view.groupStack.set(['later']); + view.persistViewState(); + t.mock.timers.tick(1000); + assert.equal(f.writes.length, 0); + release(); +}); + +test('re-enabling validates storage without replacing edits made while writeback was off', (t) => { + const f = storageFixture({ view: '{"viewLevel":"group","groupStack":["stored"]}' }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + const release = view.enablePersistence(); + release(); + view.groupStack.set(['local']); + const releaseAgain = view.enablePersistence(); + assert.deepEqual(get(view.groupStack), ['local']); + t.mock.timers.tick(500); + assert.deepEqual(JSON.parse(f.writes[0].value).groupStack, ['local']); + releaseAgain(); +}); + +test('explicit restore before first enable preserves later local edits', (t) => { + const f = storageFixture({ view: '{"viewLevel":"group","groupStack":["stored"]}' }); + useStorage(t, f.storage); + const view = createViewStores({ storageKey: 'view', storage: f.storage }); + assert.equal(view.restoreViewState(), true); + view.groupStack.set(['local']); + const release = view.enablePersistence(); + assert.deepEqual(get(view.groupStack), ['local']); + t.mock.timers.tick(500); + assert.deepEqual(JSON.parse(f.writes[0].value).groupStack, ['local']); + release(); +}); diff --git a/packages/svelte-graph/src/stores/createViewStores.test.ts b/packages/svelte-graph/src/stores/createViewStores.test.ts index 4963dbc12..01f058818 100644 --- a/packages/svelte-graph/src/stores/createViewStores.test.ts +++ b/packages/svelte-graph/src/stores/createViewStores.test.ts @@ -149,7 +149,10 @@ function useStorage(t: TestContext) { const writes: { key: string; value: string }[] = []; Object.defineProperty(globalThis, 'localStorage', { configurable: true, - value: { setItem: (key: string, value: string) => { writes.push({ key, value }); } }, + value: { + getItem: () => null, + setItem: (key: string, value: string) => { writes.push({ key, value }); }, + }, }); t.after(() => { if (previous) Object.defineProperty(globalThis, 'localStorage', previous); diff --git a/packages/svelte-graph/src/stores/createViewStores.ts b/packages/svelte-graph/src/stores/createViewStores.ts index 2df533e72..070fb0199 100644 --- a/packages/svelte-graph/src/stores/createViewStores.ts +++ b/packages/svelte-graph/src/stores/createViewStores.ts @@ -9,10 +9,13 @@ import { writable, derived, get } from 'svelte/store'; import type { ViewLevel, BreadcrumbItem, ViewportState, ZoomTarget, AnimationConfig } from '../types/view.js'; import { DEFAULT_ANIMATION } from '../types/view.ts'; +import { decodeViewStateRecord, type ViewStateRecord } from './viewStatePersistence.ts'; export interface ViewStoreOptions { /** localStorage key for auto-persistence (omit to disable) */ storageKey?: string; + /** Per-instance storage boundary; defaults to browser localStorage. */ + storage?: Pick; } export interface ViewStores { @@ -249,51 +252,109 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { animationConfig.update((current) => ({ ...current, ...config })); } + let persistenceOwners = 0; + let cleanupPersistence: (() => void) | null = null; + let restoringViewState = false; + let writebackBlocked = false; + let storageInitialized = false; + + function blockWriteback(): void { + // Retain rejected/unavailable bytes for this instance, without recovery writes. + writebackBlocked = true; + cleanupPersistence?.(); + cleanupPersistence = null; + } + + function readViewState(): + | { status: 'valid'; record: ViewStateRecord } + | { status: 'missing' | 'invalid' | 'unavailable' } { + if (!storageKey) return { status: 'missing' }; + let stored: string | null; + try { + stored = (options?.storage ?? localStorage).getItem(storageKey); + } catch { + blockWriteback(); + return { status: 'unavailable' }; + } + if (stored === null) return { status: 'missing' }; + try { + const value: unknown = JSON.parse(stored); + const record = decodeViewStateRecord(value); + if (record) return { status: 'valid', record }; + } catch { + // Invalid JSON has the same non-applying disposition as an invalid record. + } + blockWriteback(); + return { status: 'invalid' }; + } + function persistViewState(): void { - if (!storageKey) return; + if (!storageKey || writebackBlocked || restoringViewState) return; + // Recheck before each write, including changes since subscriptions started. + const existing = readViewState(); + if (existing.status === 'invalid' || existing.status === 'unavailable') return; + const state = decodeViewStateRecord({ + viewLevel: get(viewLevel), + orchestrationId: get(currentOrchestrationId), + dataGraphId: get(currentDataGraphId), + groupStack: get(groupStack), + }); + if (!state) { + blockWriteback(); + return; + } try { - const state = { - viewLevel: get(viewLevel), - orchestrationId: get(currentOrchestrationId), - dataGraphId: get(currentDataGraphId), - groupStack: get(groupStack), - }; - localStorage.setItem(storageKey, JSON.stringify(state)); + (options?.storage ?? localStorage).setItem(storageKey, JSON.stringify(state)); + storageInitialized = true; } catch { - // localStorage might not be available + blockWriteback(); } } function restoreViewState(): boolean { - if (!storageKey) return false; + if (restoringViewState) return false; + const stored = readViewState(); + if (stored.status !== 'valid') { + if (stored.status === 'missing') storageInitialized = true; + return false; + } + restoringViewState = true; try { - const stored = localStorage.getItem(storageKey); - if (stored) { - const state = JSON.parse(stored); - if (state.viewLevel) viewLevel.set(state.viewLevel); - if (state.orchestrationId) currentOrchestrationId.set(state.orchestrationId); - if (state.dataGraphId) currentDataGraphId.set(state.dataGraphId); - if (state.groupStack) groupStack.set(state.groupStack); - return true; - } + const state = stored.record; + if (state.groupStack !== undefined) groupStack.set(state.groupStack); + if (state.orchestrationId !== undefined) currentOrchestrationId.set(state.orchestrationId); + if (state.dataGraphId !== undefined) currentDataGraphId.set(state.dataGraphId); + if (state.viewLevel !== undefined) viewLevel.set(state.viewLevel); + storageInitialized = true; + return true; } catch { - // localStorage might not be available or corrupted + blockWriteback(); + return false; + } finally { + restoringViewState = false; } - return false; } - let persistenceOwners = 0; - let cleanupPersistence: (() => void) | null = null; - /** Share auto-persistence while any caller owns an enable handle. */ function enablePersistence(): () => void { - if (!storageKey) return () => {}; + if (!storageKey || writebackBlocked || restoringViewState) return () => {}; + if (persistenceOwners === 0) { + // Initialize before subscriptions schedule defaults; restarts retain local edits. + if (storageInitialized) readViewState(); + else restoreViewState(); + if (writebackBlocked) return () => {}; + } persistenceOwners += 1; if (persistenceOwners === 1) { let persistTimeout: ReturnType | null = null; - const debouncedPersist = () => { + const cancelPending = () => { if (persistTimeout) clearTimeout(persistTimeout); + persistTimeout = null; + }; + const debouncedPersist = () => { + if (restoringViewState || writebackBlocked) return; + cancelPending(); persistTimeout = setTimeout(() => { persistTimeout = null; persistViewState(); @@ -305,7 +366,7 @@ export function createViewStores(options?: ViewStoreOptions): ViewStores { cleanupPersistence = () => { unsub1(); unsub2(); - if (persistTimeout) clearTimeout(persistTimeout); + cancelPending(); }; } diff --git a/packages/svelte-graph/src/stores/viewStatePersistence.test.ts b/packages/svelte-graph/src/stores/viewStatePersistence.test.ts new file mode 100644 index 000000000..3760828de --- /dev/null +++ b/packages/svelte-graph/src/stores/viewStatePersistence.test.ts @@ -0,0 +1,42 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { decodeViewStateRecord } from './viewStatePersistence.ts'; + +test('decoder accepts each current view level, nullable identities and string stacks', () => { + for (const viewLevel of ['orchestration', 'data-graph', 'group']) { + const record = { viewLevel, orchestrationId: null, dataGraphId: '', groupStack: ['', 'opaque/id'] }; + assert.deepEqual(decodeViewStateRecord(record), record); + } +}); + +test('decoder keeps partial legacy fields and ignores unowned extensions', () => { + assert.deepEqual(decodeViewStateRecord({}), {}); + assert.deepEqual(decodeViewStateRecord({ groupStack: [], extension: { value: true } }), { groupStack: [] }); +}); + +test('decoder adds no new identity-length or nesting-count limit', () => { + const record = { dataGraphId: 'x'.repeat(10000), groupStack: Array.from({ length: 512 }, (_, i) => String(i)) }; + assert.deepEqual(decodeViewStateRecord(record), record); +}); + +test('decoder validates the whole record and detaches the returned stack', () => { + const groupStack = ['group']; + const decoded = decodeViewStateRecord({ viewLevel: 'group', groupStack }); + groupStack.push('mutated'); + assert.deepEqual(decoded, { viewLevel: 'group', groupStack: ['group'] }); + assert.equal(decodeViewStateRecord({ viewLevel: 'group', groupStack: ['group', {}] }), null); +}); + +test('decoder rejects malformed roots and every invalid owned field including nonfinite values', () => { + const invalid: unknown[] = [ + null, [], 'view', 42, true, + { viewLevel: 'other' }, { viewLevel: null }, { viewLevel: undefined }, + { orchestrationId: 0 }, { orchestrationId: NaN }, { orchestrationId: {} }, + { dataGraphId: Infinity }, { dataGraphId: -Infinity }, { dataGraphId: false }, + { groupStack: null }, { groupStack: 'group' }, { groupStack: {} }, + { groupStack: ['group', null] }, { groupStack: ['group', undefined] }, + { groupStack: ['group', NaN] }, { groupStack: [['group']] }, + { groupStack: Array(1) }, + ]; + for (const record of invalid) assert.equal(decodeViewStateRecord(record), null); +}); diff --git a/packages/svelte-graph/src/stores/viewStatePersistence.ts b/packages/svelte-graph/src/stores/viewStatePersistence.ts new file mode 100644 index 000000000..42c1eeb58 --- /dev/null +++ b/packages/svelte-graph/src/stores/viewStatePersistence.ts @@ -0,0 +1,38 @@ +import type { ViewLevel } from '../types/view.js'; + +/** Existing unversioned record. Missing legacy fields leave current state intact. */ +export interface ViewStateRecord { + viewLevel?: ViewLevel; + orchestrationId?: string | null; + dataGraphId?: string | null; + groupStack?: string[]; +} + +export function decodeViewStateRecord(value: unknown): ViewStateRecord | null { + if (typeof value !== 'object' || value === null || Array.isArray(value)) return null; + const record: ViewStateRecord = {}; + + if ('viewLevel' in value) { + const level = value.viewLevel; + if (level !== 'orchestration' && level !== 'data-graph' && level !== 'group') return null; + record.viewLevel = level; + } + if ('orchestrationId' in value) { + if (value.orchestrationId !== null && typeof value.orchestrationId !== 'string') return null; + record.orchestrationId = value.orchestrationId; + } + if ('dataGraphId' in value) { + if (value.dataGraphId !== null && typeof value.dataGraphId !== 'string') return null; + record.dataGraphId = value.dataGraphId; + } + if ('groupStack' in value) { + if (!Array.isArray(value.groupStack)) return null; + const groups: string[] = []; + for (const group of value.groupStack) { + if (typeof group !== 'string') return null; + groups.push(group); + } + record.groupStack = groups; + } + return record; +} From ea0378defadebb8665bc964c352166f0fbb15bb7 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Sun, 4 Oct 2026 05:53:56 +0000 Subject: [PATCH 7/9] docs: record accepted all-source candidate qualification Qualify the exact fd1ace frontend lineage composed with native/scanner integration 41701d68. Preserve both accepted parents and every admitted blob; no path overlaps or conflict resolution. Preserve exact scanner 5590b4a and unchanged Svelte markup. Combined 659 frontend tests and 63 tooling tests pass; full lint, TypeScript, build, Rust formatting, critical/a11y (27 scanner tests), whitespace and explicit traceability pass. Common ca9 range: 34 paths/0 impacts; native417 range: 24/0; historical main4938 range: 251/1 with declared legacy-map adoption. Qualified source: 5eb2f087e36d1dec1377f78bed5b7afd067c282e, tree 2826057b536babcca9aeae7260fd25d46b50a9a3. This successor adds evidence only. No native/WebKit/model/release execution or PR44/47 updates/bot requests. --- ...0-04-all-source-candidate-qualification.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-all-source-candidate-qualification.md diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-all-source-candidate-qualification.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-all-source-candidate-qualification.md new file mode 100644 index 000000000..c806ee2ea --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-all-source-candidate-qualification.md @@ -0,0 +1,79 @@ +# Accepted all-source candidate qualification + +Separate branch: `qualification/all-source-accepted-candidate`. +Qualified composition commit: `5eb2f087e36d1dec1377f78bed5b7afd067c282e`. +Qualified source tree: `2826057b536babcca9aeae7260fd25d46b50a9a3`. + +## Exact admitted lineage and intersections + +The composition has exactly these two parents, without rewriting either lineage: + +| Accepted source | Exact head | Exact tree | +| --- | --- | --- | +| Frontend composition and view-record validation | `fd1ace640ddb51ef906991a6d658707d5ef034f5` | `fbf1a0d15e345024b5c3403d9c55a5aba09059b2` | +| Native fixture/scanner integration | `41701d68fb3748f0b422b247427e4ca6d51ccbd5` | `879b59cbd7ff61e7c88c9542f577724c93fb3fdb` | + +Both supplied remote heads and trees were verified before composition. Their +single merge base is `ca9edde6850cd58ade0b7e534bb4f58e704c2c64`. Relative to it, +the frontend lineage changes 24 tracked paths and the native/scanner lineage +changes 10. Their path intersection is empty. The resulting 34-path diff is +exactly their union, and every changed blob equals its accepted parent blob. +No conflict resolution or additional source repair was necessary. + +All accepted ancestry is retained: the four undo/view/device/graph-mount repairs, +frontend checkpoint bb45, view-record fd1ace, native fixture/report 827b1b and +scanner 9996dd. The native integration's exact parents are 827b1b and 9996dd. +The new composition changes ten paths relative to fd1ace, and 24 relative to +41701d68. The final evidence successor adds only this Markdown report. + +The scanner blob is exactly `5590b4aeab50a859b8bc7e7678426d746f033be4`, equal to +both 41701d68 and accepted 9996dd. Its tests, scan wrapper, baseline, workflow +range gate and aggregate inference CI step are preserved. As a semantic +intersection check, all three frontend-changed Svelte components have byte-equal +markup/style after their script relative to ca9. Their changes concern lifecycle +scripts, while the accepted scanner conservatively evaluates rendered name +evidence. Combined compiler, scan and contract evidence pass. The native Python +lock addition remains cfg(test); its accepted source is unchanged. + +## Actual combined checks + +Environment: published cloud checkout; Node 24.12.0/npm 11.6.2 and Rust 1.92.0, +matching repository pins. No dependencies, credentials, permissions, network or +paid/speed settings were changed. + +| Command | Actual result | +| --- | --- | +| `npm run test:frontend` | 659 passed; zero failed/skipped/cancelled | +| `node --test scripts/*.test.mjs` | 63 passed; zero failed/skipped/cancelled | +| `npm run lint:full` | passed | +| `npm run typecheck` | passed | +| `npm run build` | passed; existing stale Browserslist-data notice | +| `cargo fmt --all -- --check` | passed; source formatting only | +| `TRACEABILITY_MODE=range TRACEABILITY_BASE_REF=ca9edde6850cd58ade0b7e534bb4f58e704c2c64 TRACEABILITY_HEAD_REF=5eb2f087e36d1dec1377f78bed5b7afd067c282e npm run lint:no-new` | critical checks, 27 exact scanner tests, accessibility scan and traceability passed; 34 paths, zero mapped impacts | +| Explicit 41701d68-to-composition `npm run traceability` | 24 paths, zero mapped impacts; passed | +| Explicit historical main 4938e405-to-composition `npm run traceability` | 251 paths, one mapped impact; passed with declared legacy-map adoption | +| `git diff --check ca9edde6850cd58ade0b7e534bb4f58e704c2c64 HEAD` | passed | + +The final report successor also receives explicit native-integration/common-base/ +main-base traceability and whitespace checks; source blobs remain exactly the +qualified composition's. Semantic contract review remains parent-owned. +Detailed local evidence uses `/workspace/pantograph-cache/all-source-*`; these +cache files are not assumed to transfer to a fresh environment. The tracked +results and identities above preserve the meaningful evidence. + +## Qualification limits and coordinated handoff + +No concrete blocker was found in the selected combined checks. This is one +coherent accepted-source candidate with fresh frontend/tooling/static gates, +not a new native/WebKit or release acceptance claim. The tooling suite includes +controlled GUI-wrapper fixtures; those fixtures do not launch a real desktop, +browser or model. Native IPC, native package/linkage/loading, C#/BEAM hosts, +GUI/WebKit interaction, real browser cold reopen, model loading/inference and +packaged release execution were not performed here. Rust unit/native builds +were not rerun in this milestone; the parent owns refreshed native qualification. +The reported CDN restriction remains; no denied artifact retry or bypass was +attempted. Existing source reports retain their scoped evidence and limits. + +This task does not update PR44/47 or request external/bot review. Parent owns +those refs, review coordination and final integration. No new source goal, +schema policy, gate relaxation, redesign or unrelated repair is introduced. From 150efee067ed5bfb6773d0dc9cff9bbdc63fb4c6 Mon Sep 17 00:00:00 2001 From: Puma Date: Sun, 4 Oct 2026 00:57:11 -0700 Subject: [PATCH 8/9] test(runtime): prepare shared Pumas registry before parallel owners --- crates/pantograph-embedded-runtime/src/lib.rs | 2 + .../src/lib_tests.rs | 2 +- .../lib_tests/edit_session_execution_tests.rs | 8 +- .../lib_tests/workflow_run_execution_tests.rs | 2 +- .../src/pumas_dispatch_package_facts.rs | 6 +- .../src/pumas_test_support.rs | 83 +++++++++++++++++++ .../src/runtime_host_execution_port.rs | 2 +- .../src/task_executor_tests/puma_lib.rs | 10 +-- .../src/technical_fit.rs | 4 +- .../src/workflow_service_composition.rs | 20 ++--- 10 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 crates/pantograph-embedded-runtime/src/pumas_test_support.rs diff --git a/crates/pantograph-embedded-runtime/src/lib.rs b/crates/pantograph-embedded-runtime/src/lib.rs index 4873a50b9..0e07ff8dd 100644 --- a/crates/pantograph-embedded-runtime/src/lib.rs +++ b/crates/pantograph-embedded-runtime/src/lib.rs @@ -61,6 +61,8 @@ mod node_io_artifacts; pub mod package_readiness_provider; #[allow(dead_code)] mod pumas_dispatch_package_facts; +#[cfg(test)] +mod pumas_test_support; pub mod python_package_readiness_probe; pub mod python_runtime; mod python_runtime_env_resolution; diff --git a/crates/pantograph-embedded-runtime/src/lib_tests.rs b/crates/pantograph-embedded-runtime/src/lib_tests.rs index c758cdc7a..5ddd192ad 100644 --- a/crates/pantograph-embedded-runtime/src/lib_tests.rs +++ b/crates/pantograph-embedded-runtime/src/lib_tests.rs @@ -121,7 +121,7 @@ async fn runtime_extensions_apply_workflow_service_for_stream_artifacts() { async fn runtime_extensions_apply_pumas_selector_access() { let temp_dir = TempDir::new().expect("temporary Pumas root should be created"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"), diff --git a/crates/pantograph-embedded-runtime/src/lib_tests/edit_session_execution_tests.rs b/crates/pantograph-embedded-runtime/src/lib_tests/edit_session_execution_tests.rs index dd1f8d216..b9b2c811c 100644 --- a/crates/pantograph-embedded-runtime/src/lib_tests/edit_session_execution_tests.rs +++ b/crates/pantograph-embedded-runtime/src/lib_tests/edit_session_execution_tests.rs @@ -7,7 +7,7 @@ async fn execute_edit_session_graph_reconciles_registry_after_restore() { let (model_id, embedding_model_path) = write_imported_embedding_model(temp.path()); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp.path()) + crate::pumas_test_support::builder(temp.path()) .build() .await .expect("build pumas api"), @@ -123,7 +123,7 @@ async fn execute_edit_session_graph_restore_keeps_scheduler_runtime_registry_dia let (model_id, embedding_model_path) = write_imported_embedding_model(temp.path()); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp.path()) + crate::pumas_test_support::builder(temp.path()) .build() .await .expect("build pumas api"), @@ -278,7 +278,7 @@ async fn execute_edit_session_graph_reconciles_registry_after_embedding_prepare( let (model_id, embedding_model_path) = write_imported_embedding_model(temp.path()); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp.path()) + crate::pumas_test_support::builder(temp.path()) .build() .await .expect("build pumas api"), @@ -404,7 +404,7 @@ async fn execute_edit_session_graph_reconciles_registry_after_failed_restore() { let (model_id, embedding_model_path) = write_imported_embedding_model(temp.path()); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp.path()) + crate::pumas_test_support::builder(temp.path()) .build() .await .expect("build pumas api"), diff --git a/crates/pantograph-embedded-runtime/src/lib_tests/workflow_run_execution_tests.rs b/crates/pantograph-embedded-runtime/src/lib_tests/workflow_run_execution_tests.rs index 89336a945..340a43779 100644 --- a/crates/pantograph-embedded-runtime/src/lib_tests/workflow_run_execution_tests.rs +++ b/crates/pantograph-embedded-runtime/src/lib_tests/workflow_run_execution_tests.rs @@ -121,7 +121,7 @@ async fn workflow_execution_session_dispatches_through_production_embedded_image let pumas_root = temp.path().join("pumas"); std::fs::create_dir_all(&pumas_root).expect("pumas root"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(pumas_root) + crate::pumas_test_support::builder(pumas_root) .with_hf_client(false) .with_process_manager(false) .build() diff --git a/crates/pantograph-embedded-runtime/src/pumas_dispatch_package_facts.rs b/crates/pantograph-embedded-runtime/src/pumas_dispatch_package_facts.rs index 16366135d..4818f5595 100644 --- a/crates/pantograph-embedded-runtime/src/pumas_dispatch_package_facts.rs +++ b/crates/pantograph-embedded-runtime/src/pumas_dispatch_package_facts.rs @@ -332,7 +332,7 @@ mod tests { write_test_diffusers_bundle(&model_dir); write_imported_diffusion_metadata(&model_dir, model_id, &model_dir); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -400,7 +400,7 @@ mod tests { write_test_diffusers_bundle(&model_dir); write_imported_diffusion_metadata(&model_dir, model_id, &model_dir); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -490,7 +490,7 @@ mod tests { #[tokio::test] async fn read_only_selector_access_does_not_promote_summaries() { let temp_dir = create_test_env(); - let api = pumas_library::PumasApi::builder(temp_dir.path()) + let api = crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() diff --git a/crates/pantograph-embedded-runtime/src/pumas_test_support.rs b/crates/pantograph-embedded-runtime/src/pumas_test_support.rs new file mode 100644 index 000000000..37a63fe3b --- /dev/null +++ b/crates/pantograph-embedded-runtime/src/pumas_test_support.rs @@ -0,0 +1,83 @@ +//! Shared startup preparation for Pumas owner API fixtures. + +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use pumas_library::registry::LibraryRegistry; + +pub(crate) fn builder(launcher_root: impl Into) -> pumas_library::PumasApiBuilder { + static REGISTRY: OnceLock = OnceLock::new(); + + // Fixture model directories are independent, but every owner API opens the + // same platform registry. Finish its first WAL/schema initialization once + // before parallel builders open additional connections, and keep it alive + // for the test process. Initialization errors still fail the test. + let db_path = pumas_library::platform::registry_db_path().expect("Pumas test registry path"); + prepare_registry(®ISTRY, &db_path); + pumas_library::PumasApi::builder(launcher_root) +} + +fn prepare_registry(registry: &OnceLock, db_path: &Path) { + registry + .get_or_init(|| LibraryRegistry::open_at(db_path).expect("Pumas test registry startup")); +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Barrier; + + #[test] + fn concurrent_owner_registry_connections_follow_completed_cold_startup() { + const OWNERS: usize = 32; + + for _ in 0..8 { + let temp = tempfile::tempdir().expect("isolated registry directory"); + let db_path = temp.path().join("registry.db"); + let registry = OnceLock::new(); + let barrier = Barrier::new(OWNERS); + + std::thread::scope(|scope| { + let mut owners = Vec::new(); + for owner in 0..OWNERS { + let library_path = temp.path().join(format!("owner-{owner}")); + std::fs::create_dir(&library_path).expect("owner library directory"); + let db_path = &db_path; + let registry = ®istry; + let barrier = &barrier; + owners.push(scope.spawn(move || { + barrier.wait(); + prepare_registry(registry, db_path); + let connection = + LibraryRegistry::open_at(db_path).expect("owner registry connection"); + connection + .register(&library_path, "test-owner") + .expect("owner registration"); + assert!(matches!( + connection + .try_claim_instance(&library_path, std::process::id()) + .expect("owner instance claim"), + pumas_library::registry::InstanceClaimResult::Claimed(_) + )); + connection + .unregister_instance(&library_path) + .expect("owner instance cleanup"); + })); + } + for owner in owners { + owner.join().expect("owner startup should succeed"); + } + }); + + assert_eq!( + registry + .get() + .expect("prepared registry") + .list() + .expect("registered owners") + .len(), + OWNERS + ); + } + } +} diff --git a/crates/pantograph-embedded-runtime/src/runtime_host_execution_port.rs b/crates/pantograph-embedded-runtime/src/runtime_host_execution_port.rs index 306fb79a8..9a43acac9 100644 --- a/crates/pantograph-embedded-runtime/src/runtime_host_execution_port.rs +++ b/crates/pantograph-embedded-runtime/src/runtime_host_execution_port.rs @@ -2045,7 +2045,7 @@ mod tests { let pumas_root = temp.path().join("pumas"); std::fs::create_dir_all(&pumas_root).expect("pumas launcher root"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(pumas_root) + crate::pumas_test_support::builder(pumas_root) .with_hf_client(false) .with_process_manager(false) .build() diff --git a/crates/pantograph-embedded-runtime/src/task_executor_tests/puma_lib.rs b/crates/pantograph-embedded-runtime/src/task_executor_tests/puma_lib.rs index 479ee3fa7..b30887b1e 100644 --- a/crates/pantograph-embedded-runtime/src/task_executor_tests/puma_lib.rs +++ b/crates/pantograph-embedded-runtime/src/task_executor_tests/puma_lib.rs @@ -18,7 +18,7 @@ async fn puma_lib_execution_hydrates_model_ref_from_model_id_without_path_output write_imported_diffusion_metadata(&model_dir, "diffusion/imported/test-bundle", &bundle_root); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"), @@ -157,7 +157,7 @@ async fn puma_lib_execution_does_not_rebind_model_id_from_raw_pumas_api() { write_imported_diffusion_metadata(&model_dir, model_id, &bundle_root); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"), @@ -227,7 +227,7 @@ async fn puma_lib_execution_hydrates_model_ref_from_selector_access_without_puma .join(model_id); write_imported_diffusion_metadata(&model_dir, model_id, &bundle_root); - let api = pumas_library::PumasApi::builder(temp_dir.path()) + let api = crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"); @@ -310,7 +310,7 @@ async fn puma_lib_execution_does_not_emit_inference_settings_from_saved_or_selec write_imported_diffusion_metadata(&model_dir, model_id, &bundle_root); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"), @@ -373,7 +373,7 @@ async fn puma_lib_execution_does_not_resolve_saved_model_name_without_model_id() write_imported_diffusion_metadata(&model_dir, model_id, &bundle_root); let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .build() .await .expect("pumas api should initialize"), diff --git a/crates/pantograph-embedded-runtime/src/technical_fit.rs b/crates/pantograph-embedded-runtime/src/technical_fit.rs index 5cf66eef6..9d82825e8 100644 --- a/crates/pantograph-embedded-runtime/src/technical_fit.rs +++ b/crates/pantograph-embedded-runtime/src/technical_fit.rs @@ -2884,7 +2884,7 @@ mod tests { let temp_dir = tempfile::tempdir().expect("temp dir"); let model_id = "llm/test/live-technical-fit-facts"; let api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -2960,7 +2960,7 @@ mod tests { let temp_dir = tempfile::tempdir().expect("temp dir"); std::fs::create_dir_all(temp_dir.path().join("shared-resources/models")) .expect("models dir"); - let api = pumas_library::PumasApi::builder(temp_dir.path()) + let api = crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() diff --git a/crates/pantograph-embedded-runtime/src/workflow_service_composition.rs b/crates/pantograph-embedded-runtime/src/workflow_service_composition.rs index 189dc9c09..5ef5927d1 100644 --- a/crates/pantograph-embedded-runtime/src/workflow_service_composition.rs +++ b/crates/pantograph-embedded-runtime/src/workflow_service_composition.rs @@ -772,7 +772,7 @@ mod tests { async fn builds_resource_backed_hosted_workflow_service_before_sharing() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -808,7 +808,7 @@ mod tests { write_test_diffusers_bundle(&model_dir); write_imported_diffusion_metadata(&model_dir, model_id, &model_dir); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -909,7 +909,7 @@ mod tests { write_test_diffusers_bundle(&model_dir); write_imported_diffusion_metadata(&model_dir, model_id, &model_dir); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1084,7 +1084,7 @@ mod tests { async fn hosted_resource_backed_factory_requires_artifact_writer_before_sharing() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1117,7 +1117,7 @@ mod tests { #[tokio::test] async fn hosted_resource_backed_factory_rejects_non_owner_pumas_access() { let temp_dir = tempfile::tempdir().expect("tempdir"); - let pumas_api = pumas_library::PumasApi::builder(temp_dir.path()) + let pumas_api = crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1157,7 +1157,7 @@ mod tests { async fn resource_backed_hosted_bundle_returns_service_and_lifecycle_handle() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1194,7 +1194,7 @@ mod tests { async fn resource_backed_hosted_bundle_rejects_invalid_producer_config_before_sharing() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1283,7 +1283,7 @@ mod tests { async fn hosted_startup_composition_returns_service_extensions_and_lifecycle_handle() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1394,7 +1394,7 @@ mod tests { #[tokio::test] async fn hosted_startup_composition_rejects_non_owner_pumas_selector_access() { let temp_dir = tempfile::tempdir().expect("tempdir"); - let pumas_api = pumas_library::PumasApi::builder(temp_dir.path()) + let pumas_api = crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() @@ -1440,7 +1440,7 @@ mod tests { async fn hosted_startup_composition_rejects_service_error_before_starting_sidecar() { let temp_dir = tempfile::tempdir().expect("tempdir"); let pumas_api = Arc::new( - pumas_library::PumasApi::builder(temp_dir.path()) + crate::pumas_test_support::builder(temp_dir.path()) .with_hf_client(false) .with_process_manager(false) .build() From ee91e5d094112414c9a2c6de6480be0c4ab57d66 Mon Sep 17 00:00:00 2001 From: Puma Date: Sun, 4 Oct 2026 00:57:41 -0700 Subject: [PATCH 9/9] docs: record PR48 registry contention evidence and limits --- .../2026-10-04-pr48-pumas-registry-startup.md | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-pr48-pumas-registry-startup.md diff --git a/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-pr48-pumas-registry-startup.md b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-pr48-pumas-registry-startup.md new file mode 100644 index 000000000..9f62bbf5e --- /dev/null +++ b/docs/plans/domain-architecture-and-multimodal/reports/2026-10-04-pr48-pumas-registry-startup.md @@ -0,0 +1,119 @@ +# PR48 Pumas registry startup diagnosis and test repair + +Separate branch: `repair/pr48-pumas-registry-test-startup`. +Source repair: `78d35f9ec3c4d24d092ff92eb7c26a503774b3d6`. +Source tree: `dfe5deaa446530866a0ea2075442ff6f1e1545f1`. +Its direct parent is accepted PR48 head +`ea0378defadebb8665bc964c352166f0fbb15bb7`, tree +`f03ad681e6952266f48a1907e07cb39b05e00328`. All stacked ancestry is retained. +Independent composition review accepted that parent; this repair still needs +parent review and full embedded-runtime qualification. + +## Hosted evidence and source identity + +[Quality run 37181984567, job 111376229170](https://github.com/MrScripty/Pantograph/actions/runs/37181984567/job/111376229170) +ran the PR merge snapshot `2e9829e8944c9750357c0443a7800aa6e1443a40` of +ea0378de into main `b84d7ec49773ab306432b393b735e4aae2e027b1`. The run API +identifies ea0378de as its PR head and attempt 1. Embedded-runtime finished with +454 passing tests and one failure. At package-facts source line 340, the test +`owner_api_projects_diffusers_package_facts_without_paths` panicked on +`PumasApi::build()` returning `DatabaseBusy`, extended SQLite code 5, +"database is locked". This occurs before explicit index rebuilding and before +Pantograph package-facts projection. The hosted log has no native backtrace or +SQL-statement context. + +Cargo.toml, Cargo.lock, the Quality workflow, and package-facts source have no +diff between ea0378de and b84d7ec. The parent reports all four postmerge main +workflows passed. The sibling checkout uses workflow ref 66c0c11, but the Rust +dependency is separately locked to Pumas-Library +`f87c3da8276a914a54c6f4f36d617bef9d9f424e`; the cached checkout HEAD was verified +at that exact commit. Its registry source blob is +`97ff4eb5c50881e4c07f81006df387f44314006a`. + +## Reproduced contention + +Each owner fixture has an independent temporary model directory. Nevertheless, +the pinned builder unconditionally calls `LibraryRegistry::open()` at the shared +platform database before initializing the model library. Registry startup opens +a new connection, configures WAL/timeout/synchronous/temp-store pragmas, then +ensures the schema. The process-local connection mutex does not coordinate +different builder connections to that shared database. + +An offline Rust harness compiled the exact, unchanged pinned registry source +with bundled rusqlite 0.32.1. Threads synchronized at a barrier, each opening +the same fresh registry and registering/claiming/cleaning up a different library +path. All SQLite operations used the real pinned code and SQLite engine. + +| Diagnostic run | Concurrent cold opens | Initialized-database control | Serial cold control | +| --- | --- | --- | --- | +| 100 batches, 128 threads | 2 failures / 12,800 operations | 0 / 12,800 | 0 / 100 | +| 200 batches, 128 threads, captured backtraces | 7 failures / 25,600 operations | 0 / 25,600 | 0 / 200 | + +Every reproduced failure returned the same DatabaseBusy/code-5 error while +opening the registry. Captured backtraces locate it in +`LibraryRegistry::configure_connection`, before schema completion. This +demonstrates a real cold registry configuration race exposed by parallel owner +fixtures. It is the best-supported explanation for the hosted failure; the +hosted log alone cannot establish that its failing SQL statement is identical. +No Pantograph production lifecycle regression was demonstrated. + +## Repair and exact source write set + +One cfg(test) helper prepares the platform registry through OnceLock, retains +that connection for the test process, then returns the unchanged Pumas builder. +All 27 existing owner-fixture builder sites use it. Only initial shared registry +preparation is coordinated; API builds and test bodies still run concurrently. +Errors still fail tests. No retry, error suppression, timeout change, dependency +change, environment mutation, or assertion weakening was introduced. + +The source commit changes exactly these ten paths under +`crates/pantograph-embedded-runtime/src/`: + +- `lib.rs`: cfg(test) module declaration. +- `pumas_test_support.rs`: new shared preparation and real-registry regression. +- `lib_tests.rs`: one builder reference. +- `lib_tests/edit_session_execution_tests.rs`: four builder references. +- `lib_tests/workflow_run_execution_tests.rs`: one builder reference. +- `pumas_dispatch_package_facts.rs`: three builder references. +- `runtime_host_execution_port.rs`: one builder reference. +- `task_executor_tests/puma_lib.rs`: five builder references. +- `technical_fit.rs`: two builder references. +- `workflow_service_composition.rs`: ten builder references. + +After reversing only those references and the module declaration, all nine +existing files are byte-identical to ea0378de. Every edited call is in existing +test-only code. Production behavior and all existing projection, ownership, +read-only-access, lifecycle, and cleanup assertions are preserved. This report +is the sole additional path in the documentation successor. + +## Executed checks and limits + +Tools: Node 24.12.0, npm 11.6.2 and Rust 1.92.0 via the existing activation file. +No speed/credit, credential, permission or network setting was changed. + +- The checked-in regression passed in the isolated Rust harness: one test, + eight fresh databases and 32 concurrent owners per database (256 operations). + It executes the checked-in preparation helper and the real pinned registry, + verifies successful instance claims/cleanup and all registered owner rows. +- `node --test scripts/*.test.mjs`: 63 passed, zero failures/skips/cancellations. +- `cargo fmt --all -- --check` and whitespace checks passed. +- Explicit ea0378de-to-source `npm run lint:no-new` passed: critical checks, + all 27 scanner cases, accessibility scan, and traceability (10 paths, + zero mapped impacts). Audit/traceability gates and workflow files are intact. + +The isolated harness supplies error/platform support and compile-only API +builder signatures; no API builder behavior is executed there. It verifies +real registry/SQLite behavior, not the full Pumas API or Pantograph test binary. +Full `cargo test -p pantograph-embedded-runtime --lib` was not executed locally. +The existing ORT/CDN restriction remains; no download retry or bypass was made. +Native ORT qualification remains parent-owned. Genuine GUI/WebKit, native IPC, +native package loading, C#/BEAM hosts, model loading and inference were not run. +Earlier frontend qualification remains prior evidence, not a new run here. + +Local supporting evidence is in `/workspace/pantograph-cache/`: +`pr48-registry-stress.log`, `pr48-registry-backtrace.log`, +`pr48-registry-repair-regression.log`, `pr48-registry-tooling.log`, +`pr48-registry-fmt.log`, `pr48-registry-range-gates.log`, and +`pr48-hosted-failure-excerpt.log`. The harness and its locked dependencies are +in `pr48-registry-repro/`; `diagnostic-main.rs` preserves the failing harness. +PR48 metadata, its branch, bot requests and merging remain untouched.