diff --git a/docs/superpowers/plans/2026-03-26-auto-clean-runtime-handlers.md b/docs/superpowers/plans/2026-03-26-auto-clean-runtime-handlers.md new file mode 100644 index 0000000..f0ec16c --- /dev/null +++ b/docs/superpowers/plans/2026-03-26-auto-clean-runtime-handlers.md @@ -0,0 +1,508 @@ +# Auto-clean Runtime Story.on() Handlers — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Automatically unsubscribe event handlers registered via `Story.on()` during the runtime phase when `Story.restart()` is called, preventing handler duplication across restarts. + +**Architecture:** A module-level boolean `inRuntimePhase` and array `runtimeUnsubs` in `store.ts` track which unsub functions were registered after the startup phase. On restart, after `fireBeforeRestart()`, all tracked unsubs are called and the list is cleared. The flag is set before `executeStoryInit()` in both the boot path (`index.tsx`) and the restart path (`store.ts`). + +**Tech Stack:** TypeScript, Vitest, Zustand + +**Spec:** `docs/superpowers/specs/2026-03-26-auto-clean-runtime-handlers-design.md` + +--- + +### Task 1: Add runtime phase tracking to store.ts + +**Files:** + +- Modify: `src/store.ts:178` (after `resetModuleState`) +- Test: `test/unit/store.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add a new `describe` block at the end of `test/unit/store.test.ts`: + +```typescript +import { + useStoryStore, + onBeforeRestart, + onStoryInit, + trackRuntimeUnsub, + enterRuntimePhase, + _resetRuntimePhase, +} from '../../src/store'; + +// ... existing imports and helpers ... + +describe('runtime handler cleanup', () => { + beforeEach(() => { + _resetRuntimePhase(); + useStoryStore.setState({ + storyData: null, + currentPassage: '', + variables: {}, + variableDefaults: {}, + temporary: {}, + history: [], + historyIndex: -1, + visitCounts: {}, + renderCounts: {}, + transitionConfig: null, + nextTransition: null, + renderDeferred: false, + }); + }); + + it('trackRuntimeUnsub is a no-op before enterRuntimePhase', () => { + const unsub = vi.fn(); + trackRuntimeUnsub(unsub); + + // Trigger restart — unsub should NOT have been called + const story = makeStoryData([makePassage(1, 'Start')]); + useStoryStore.getState().init(story); + useStoryStore.getState().restart(); + + expect(unsub).not.toHaveBeenCalled(); + }); + + it('calls tracked unsubs on restart after enterRuntimePhase', () => { + const story = makeStoryData([makePassage(1, 'Start')]); + useStoryStore.getState().init(story); + + enterRuntimePhase(); + + const unsub = vi.fn(); + trackRuntimeUnsub(unsub); + + useStoryStore.getState().restart(); + + expect(unsub).toHaveBeenCalledOnce(); + }); + + it('does not call startup-phase unsubs on restart', () => { + const startupUnsub = vi.fn(); + trackRuntimeUnsub(startupUnsub); // before enterRuntimePhase — should be ignored + + const story = makeStoryData([makePassage(1, 'Start')]); + useStoryStore.getState().init(story); + + enterRuntimePhase(); + + const runtimeUnsub = vi.fn(); + trackRuntimeUnsub(runtimeUnsub); + + useStoryStore.getState().restart(); + + expect(startupUnsub).not.toHaveBeenCalled(); + expect(runtimeUnsub).toHaveBeenCalledOnce(); + }); + + it('clears tracked unsubs after restart so they are not called again', () => { + const story = makeStoryData([makePassage(1, 'Start')]); + useStoryStore.getState().init(story); + + enterRuntimePhase(); + const unsub = vi.fn(); + trackRuntimeUnsub(unsub); + + useStoryStore.getState().restart(); + expect(unsub).toHaveBeenCalledOnce(); + + // Second restart should not call it again + useStoryStore.getState().restart(); + expect(unsub).toHaveBeenCalledOnce(); + }); +}); +``` + +Note: You'll need to add `vi` to the existing vitest imports at the top of the file: + +```typescript +import { describe, it, expect, beforeEach, vi } from 'vitest'; +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/unit/store.test.ts` +Expected: FAIL — `trackRuntimeUnsub` and `enterRuntimePhase` are not exported from `store.ts` + +- [ ] **Step 3: Implement runtime phase tracking in store.ts** + +Add the following after the `resetModuleState` function (around line 178) and before the storyinit callbacks section (line 180): + +```typescript +// --------------------------------------------------------------------------- +// Runtime handler cleanup (auto-unsub on restart) +// --------------------------------------------------------------------------- + +let runtimeUnsubs: Array<() => void> = []; +let inRuntimePhase = false; + +/** + * Track an unsubscribe function for automatic cleanup on restart. + * No-op if called during the startup phase (before enterRuntimePhase). + */ +export function trackRuntimeUnsub(unsub: () => void): void { + if (inRuntimePhase) { + runtimeUnsubs.push(unsub); + } +} + +/** Mark the start of the runtime phase. Called before executeStoryInit(). */ +export function enterRuntimePhase(): void { + inRuntimePhase = true; +} + +/** Call all tracked unsubs and reset the runtime phase. */ +function cleanupRuntimeHandlers(): void { + for (const unsub of runtimeUnsubs) unsub(); + runtimeUnsubs = []; + inRuntimePhase = false; +} + +/** Test-only: reset runtime phase state between tests. */ +export function _resetRuntimePhase(): void { + runtimeUnsubs = []; + inRuntimePhase = false; +} +``` + +Then modify the `restart()` method. Insert `cleanupRuntimeHandlers()` after `fireBeforeRestart()` / `const keepDeferred = get().renderDeferred;` and insert `enterRuntimePhase()` just before `executeStoryInit()`: + +```typescript + restart: () => { + const { storyData, variableDefaults } = get(); + if (!storyData) return; + + const startPassage = storyData.passagesById.get(storyData.startNode); + if (!startPassage) return; + + set((state) => { + state.renderDeferred = false; + }); + + fireBeforeRestart(); + + const keepDeferred = get().renderDeferred; + + // Clean up all runtime-phase handlers (after beforerestart has fired) + cleanupRuntimeHandlers(); + + resetPRNG(); + resetTriggers(); + const initialVars = deepClone(variableDefaults); + resetModuleState(deepClone(initialVars)); + + set((state) => { + state.currentPassage = startPassage.name; + state.variables = initialVars; + state.temporary = {}; + state.history = [ + { + passage: startPassage.name, + timestamp: Date.now(), + }, + ]; + state.historyIndex = 0; + state.visitCounts = { [startPassage.name]: 1 }; + state.renderCounts = { [startPassage.name]: 1 }; + if (!keepDeferred) { + state.renderDeferred = false; + } + }); + + lastNavigationVars = get().variables; + + // Re-enter runtime phase before StoryInit so new handlers are tracked + enterRuntimePhase(); + + executeStoryInit(); + clearSession(storyData.ifid); + fireStoryInit(); + + startNewPlaythrough(storyData.ifid) + .then((newId) => { + set((state) => { + state.playthroughId = newId; + }); + }) + .catch((err) => + console.error('spindle: failed to start new playthrough', err), + ); + }, +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/unit/store.test.ts` +Expected: PASS — all new tests and existing tests pass + +- [ ] **Step 5: Run full type check** + +Run: `npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 6: Commit** + +```bash +git add src/store.ts test/unit/store.test.ts +git commit -m "feat: add runtime phase tracking and cleanup on restart (#128)" +``` + +--- + +### Task 2: Wire Story.on() to trackRuntimeUnsub + +**Files:** + +- Modify: `src/story-api.ts:1,371-418` +- Test: `test/unit/story-api.test.ts` + +- [ ] **Step 1: Write the failing test** + +Add to the end of `test/unit/story-api.test.ts`, inside the top-level `describe('StoryAPI', ...)`: + +```typescript +import { enterRuntimePhase, _resetRuntimePhase } from '../../src/store'; + +// ... (these imports go at the top of the file alongside the existing imports) + +describe('runtime handler auto-cleanup', () => { + beforeEach(() => { + _resetRuntimePhase(); + }); + + it('navigate handler registered during runtime is cleaned on restart', () => { + enterRuntimePhase(); + + const cb = vi.fn(); + Story.on('navigate', cb); + + // Navigate to verify handler works + Story.goto('Room'); + expect(cb).toHaveBeenCalledWith('Room', 'Start'); + + cb.mockClear(); + + // Restart cleans the handler + Story.restart(); + + // Navigate again — handler should NOT fire + Story.goto('Room'); + expect(cb).not.toHaveBeenCalled(); + }); + + it('beforerestart handler fires during the restart that cleans it', () => { + enterRuntimePhase(); + + const cb = vi.fn(); + Story.on('beforerestart', cb); + + Story.restart(); + // Should have fired once during this restart + expect(cb).toHaveBeenCalledOnce(); + + cb.mockClear(); + + // Second restart — handler was cleaned, should NOT fire + Story.restart(); + expect(cb).not.toHaveBeenCalled(); + }); + + it('no duplicate handlers after multiple restart cycles', () => { + enterRuntimePhase(); + + const calls: string[] = []; + + // Simulate what a host boot() does: register on storyinit, + // and inside storyinit register a navigate handler + Story.on('storyinit', () => { + Story.on('navigate', () => { + calls.push('nav'); + }); + }); + + // First restart: storyinit fires, registers one navigate handler + Story.restart(); + Story.goto('Room'); + expect(calls).toEqual(['nav']); + + calls.length = 0; + + // Second restart: old navigate handler cleaned, storyinit registers a new one + Story.restart(); + Story.goto('Room'); + expect(calls).toEqual(['nav']); // still exactly one, not two + }); + + it('manual unsub still works and double-call is safe', () => { + enterRuntimePhase(); + + const cb = vi.fn(); + const unsub = Story.on('navigate', cb); + + // Manually unsub + unsub(); + + // Navigate — should not fire + Story.goto('Room'); + expect(cb).not.toHaveBeenCalled(); + + // Restart — the stale entry in runtimeUnsubs is a no-op + expect(() => Story.restart()).not.toThrow(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run test/unit/story-api.test.ts` +Expected: FAIL — navigate handler still fires after restart because `Story.on()` doesn't call `trackRuntimeUnsub` yet + +- [ ] **Step 3: Implement trackRuntimeUnsub in Story.on()** + +In `src/story-api.ts`, add the import: + +```typescript +import { + useStoryStore, + onStoryInit, + onBeforeRestart, + trackRuntimeUnsub, +} from './store'; +``` + +Then modify the `on()` method. Wrap each event branch so the unsub is tracked before being returned. The full updated method: + +```typescript + on(event: string, callback: (...args: any[]) => void): () => void { + if (event === 'navigate') { + let prev = useStoryStore.getState().currentPassage; + const unsub = useStoryStore.subscribe((state) => { + if (state.currentPassage !== prev) { + const from = prev; + prev = state.currentPassage; + (callback as NavigateCallback)(state.currentPassage, from); + } + }); + trackRuntimeUnsub(unsub); + return unsub; + } + + if (event === 'beforerestart') { + const unsub = onBeforeRestart(callback as BeforeRestartCallback); + trackRuntimeUnsub(unsub); + return unsub; + } + + if (event === 'storyinit') { + const unsub = onStoryInit(callback as StoryInitCallback); + trackRuntimeUnsub(unsub); + return unsub; + } + + if (event === 'actionsChanged') { + const unsub = onActionsChanged(callback as ActionsChangedCallback); + trackRuntimeUnsub(unsub); + return unsub; + } + + if (event === 'variableChanged') { + let prevVars = { ...useStoryStore.getState().variables }; + const unsub = useStoryStore.subscribe((state) => { + const changed: Record = {}; + let hasChanges = false; + const allKeys = new Set([ + ...Object.keys(prevVars), + ...Object.keys(state.variables), + ]); + for (const key of allKeys) { + if (state.variables[key] !== prevVars[key]) { + changed[key] = { from: prevVars[key], to: state.variables[key] }; + hasChanges = true; + } + } + prevVars = { ...state.variables }; + if (hasChanges) { + (callback as VariableChangedCallback)(changed); + } + }); + trackRuntimeUnsub(unsub); + return unsub; + } + + throw new Error(`spindle: Unknown event "${event}".`); + }, +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run test/unit/story-api.test.ts` +Expected: PASS — all new and existing tests pass + +- [ ] **Step 5: Commit** + +```bash +git add src/story-api.ts test/unit/story-api.test.ts +git commit -m "feat: wire Story.on() to trackRuntimeUnsub for auto-cleanup (#128)" +``` + +--- + +### Task 3: Call enterRuntimePhase() during boot + +**Files:** + +- Modify: `src/index.tsx:4,102-103` + +- [ ] **Step 1: Add import and call in index.tsx** + +Add `enterRuntimePhase` to the import from `./store`: + +```typescript +import { useStoryStore, fireStoryInit, enterRuntimePhase } from './store'; +``` + +Insert `enterRuntimePhase()` just before the `executeStoryInit()` call (around line 102): + +```typescript +useStoryStore.getState().init(storyData, defaults); + +// Enter runtime phase — handlers registered from here on are cleaned on restart +enterRuntimePhase(); + +// Execute StoryInit passage if it exists +executeStoryInit(); +``` + +- [ ] **Step 2: Run type check and full test suite** + +Run: `npx tsc --noEmit && npx vitest run` +Expected: No type errors, all tests pass + +- [ ] **Step 3: Commit** + +```bash +git add src/index.tsx +git commit -m "feat: enter runtime phase before executeStoryInit during boot (#128)" +``` + +--- + +### Task 4: Final verification + +- [ ] **Step 1: Run full test suite** + +Run: `npx vitest run` +Expected: All tests pass + +- [ ] **Step 2: Run type check** + +Run: `npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 3: Verify no regressions in existing store/story-api tests** + +Run: `npx vitest run test/unit/store.test.ts test/unit/story-api.test.ts` +Expected: All existing tests still pass alongside new tests diff --git a/docs/superpowers/specs/2026-03-26-auto-clean-runtime-handlers-design.md b/docs/superpowers/specs/2026-03-26-auto-clean-runtime-handlers-design.md new file mode 100644 index 0000000..a41f799 --- /dev/null +++ b/docs/superpowers/specs/2026-03-26-auto-clean-runtime-handlers-design.md @@ -0,0 +1,86 @@ +# Auto-clean Story.on() handlers registered after storyinit on restart + +**Issue:** rohal12/spindle#128 +**Date:** 2026-03-26 + +## Problem + +Host applications that register `Story.on('navigate', ...)` and other event handlers during `boot()` (called from `storyinit`) accumulate duplicate handlers across restarts. Each `Story.restart()` fires `storyinit` again, and the host re-registers handlers without the old ones being cleaned up. + +The same applies to handlers registered via `{do}` blocks in the StoryInit passage — `executeStoryInit()` creates a new hidden Preact container on each restart without unmounting the old one. + +## Solution: Phase flag + cleanup list + +### Lifecycle phases + +Spindle already distinguishes two lifecycle phases: + +- **Startup phase** — from page load through `:storystartup`. Handlers registered here (e.g., custom macro registrations, permanent subscriptions) survive restarts. +- **Runtime phase** — from `executeStoryInit()` onwards. Handlers registered here are automatically unsubscribed on restart. + +### New module-level state in `store.ts` + +```typescript +let runtimeUnsubs: Array<() => void> = []; +let inRuntimePhase = false; +``` + +### New exports from `store.ts` + +- `trackRuntimeUnsub(unsub: () => void)` — if in runtime phase, pushes onto the cleanup list; no-op otherwise +- `enterRuntimePhase()` — sets the flag to true + +Cleanup logic stays internal to `restart()`. + +### Story.on() change in `story-api.ts` + +After obtaining the unsub function for any event type (`navigate`, `beforerestart`, `storyinit`, `actionsChanged`, `variableChanged`), check the phase and track if runtime: + +```typescript +const unsub = /* existing registration */; +trackRuntimeUnsub(unsub); // no-op if still in startup phase +return unsub; +``` + +No API surface change — `Story.on()` signature and return type are unchanged. + +### Updated restart sequence in `store.ts` + +1. Reset `renderDeferred` to false +2. `fireBeforeRestart()` — all beforerestart handlers fire, including runtime ones +3. **Call all `runtimeUnsubs`, clear the list, set `inRuntimePhase = false`** +4. `resetPRNG()` +5. `resetTriggers()` +6. `resetModuleState()` +7. Reset store state atomically (passage, variables, history, etc.) +8. **`enterRuntimePhase()`** — set flag to true +9. `executeStoryInit()` — new handlers registered here are tracked as runtime +10. `clearSession()` +11. `fireStoryInit()` — `boot()` re-registers handlers, tracked as runtime +12. `startNewPlaythrough()` (async) + +### Boot sequence change in `index.tsx` + +Call `enterRuntimePhase()` just before `executeStoryInit()` — after `:storystartup` has fired and all startup-phase registration is complete. + +### Manual unsub remains safe + +If a host calls the unsub function returned by `Story.on()` manually, it removes the handler as usual. The stale entry in `runtimeUnsubs` becomes a no-op — calling an already-removed unsub is safe because: + +- `onBeforeRestart`/`onStoryInit` filter-based removal finds nothing to remove +- Zustand's `unsubscribe` is idempotent + +## Files to modify + +- `src/store.ts` — add `runtimeUnsubs`, `inRuntimePhase`, `trackRuntimeUnsub()`, `enterRuntimePhase()`, cleanup in `restart()` +- `src/story-api.ts` — wrap `Story.on()` return values with `trackRuntimeUnsub()` when in runtime phase +- `src/index.tsx` — call `enterRuntimePhase()` before `executeStoryInit()` during boot + +## Testing + +- Verify handlers registered during `:storystartup` survive restart +- Verify handlers registered during `storyinit` callback are cleaned on restart +- Verify handlers registered during gameplay are cleaned on restart +- Verify `beforerestart` handlers fire before being cleaned +- Verify manual unsub still works and double-unsub is safe +- Verify no handler duplication after multiple restart cycles diff --git a/src/components/macros/Set.tsx b/src/components/macros/Set.tsx index 27c33fd..fb169a7 100644 --- a/src/components/macros/Set.tsx +++ b/src/components/macros/Set.tsx @@ -1,9 +1,11 @@ import { defineMacro } from '../../define-macro'; +import { MacroError } from './MacroError'; defineMacro({ name: 'set', render({ rawArgs }, ctx) { const ran = ctx.hooks.useRef(false); + const error = ctx.hooks.useRef(null); if (!ran.current) { ran.current = true; @@ -11,14 +13,23 @@ defineMacro({ try { ctx.mutate(rawArgs); } catch (err) { + error.current = err; console.error( `spindle: Error in {set ${rawArgs}}${ctx.sourceLocation()}:`, err, ); - return null; } } + if (error.current) { + return ( + + ); + } + return null; }, }); diff --git a/src/markup/render.tsx b/src/markup/render.tsx index 85a27fd..7e51003 100644 --- a/src/markup/render.tsx +++ b/src/markup/render.tsx @@ -17,7 +17,11 @@ export interface LocalsUpdater { } const defaultUpdater: LocalsUpdater = { - update: () => {}, + update: (key: string) => { + throw new Error( + `Cannot set @${key} — local variables require a {for}, widget, {link}, or {button} scope`, + ); + }, getValues: () => ({}), }; diff --git a/test/dom/macros.test.tsx b/test/dom/macros.test.tsx index 4b7f67a..f27f910 100644 --- a/test/dom/macros.test.tsx +++ b/test/dom/macros.test.tsx @@ -51,6 +51,13 @@ describe('macro components', () => { expect(state.variables.a).toBe(1); expect(state.variables.b).toBe(2); }); + + it('shows error when setting @local outside a locals context', () => { + const el = renderPassage('{set @foo = 42}'); + const error = el.querySelector('.error'); + expect(error).not.toBeNull(); + expect(error!.textContent).toMatch(/@foo/); + }); }); describe('{$var} display', () => {