feat: client-side SPA navigation + session registry refactor#72
Merged
Conversation
Introduce web/src/session/session-modals.svelte.js, a $state-backed store that owns the session viewer's modal open-state (shortcuts, model-usage, fork, cat-settings, label). SessionPage binds the modal components to it and every opener — the command menu, keyboard globals, content runtime, and cat-gatekeeper — imports the store helpers instead of reaching through window.__piOpen*. This removes the five global openers and the onMount ordering hazard they created. The store is live-only and never pulled into the static export (TestExportBundleIsSelfContained stays green). Phase 5 step 1.
…stry Add web/src/session/session-runtime.js, a plain singleton registry whose slots (annotations, artifacts, rightSidebar, layout) hold the imperative control surfaces child components used to publish on window.__pi*. Each component assigns its slot on mount and clears it on destroy; consumers (SessionTree, LiveReload, session-ui-runner, session-globals, SessionPage) read sessionRuntime.<slot>?.method() instead of reaching through window. __piCatGatekeeper was a window round-trip inside CatGatekeeper.svelte; it now uses a hoisted local controller. SessionPage resets the registry on unmount. The registry is dependency-free and stays out of live-only code, so the export bundle (rebuilt) no longer references any of these globals and TestExportBundleIsSelfContained stays green. Phase 5 step 2.
Replace the last window globals in the session UI layer with the sessionRuntime registry. setupSessionUi now registers the toggle controller as sessionRuntime.toggleState instead of writing window.sessionToggleState + window.applyToggleStateToNode; the message-pane afterRender hook (live content-runtime and the static export) reads sessionRuntime.toggleState ?.applyToNode(). Drops the dead window.sessionToggleState (no readers) and export's __piFilterState (written, never read). The imperative setupSessionUi runner itself stays: it is shared by the live app and the server-less export to wire the static session.html DOM, so it is legitimate shared infrastructure, not migration residue. Folding that wiring into components would require migrating export's static sidebar chrome to a component mount (a separate, E2E-gated change). Phase 5 step 3.
Make App.svelte's router reactive: it now listens for popstate (back/ forward) and wraps history.pushState/replaceState to swap the active page when the pathname changes, instead of reading the path once at mount. Route the index/settings navigation through a shared navigate() helper so links no longer trigger full page reloads: session cards, the command palette, new-session creation, the settings menu link, the Cmd+, shortcut, and the settings back link. External target=_blank links and modified clicks (Cmd/Ctrl/Shift/Alt, middle-click) keep their default browser behaviour.
Key <SessionPage> on the ?id= query param so navigating between sessions (same pathname, different id) tears down and remounts it instead of falling through as a no-op — SessionPage reads ?id= only at mount. App.svelte now tracks search alongside pathname to drive the key. Route fork/clone/new-session navigation (CommandMenu, SessionHeader, the entry fork in session-content-runtime) through navigate() so they no longer trigger full page reloads. Within-session navigation never mutates the URL, so the key stays stable while reading a session. Also remove the pi-session-reload listener in SessionPage's onMount cleanup, which a remount would otherwise leak.
The dd7d654 registry refactor dropped window.__piCatGatekeeper, which the cat e2e test uses to force a break (skipToBreak) without waiting out the 25-min focus timer. The controller has no in-app consumers, so re-expose it as a plain window seam set on mount / cleared on destroy rather than a sessionRuntime registry slot. Also pin the test's bedtime == wakeup (zero-width sleep window): the default 23:00–07:00 window made the gatekeeper enter bedtime mode when the suite ran overnight, showing the sleep overlay instead of the break and failing across all browser projects.
The per-window expect.poll (banner gone OR button re-enabled) was the flake source: prepending the merged conversation triggers a synchronous full re-render that, under the parallel matrix, briefly jams the main thread and can blow the fixed 15s poll budget even though the load succeeds (the failure snapshot showed the banner already gone and the earliest message rendered). Drive the clicks with click()'s built-in enabled-auto-wait plus a best-effort settle, and let only the final auto-retrying assertions (banner count 0 + earliest message rendered) be authoritative, so a transient render jam no longer fails the test.
AppearanceSettings.fontSelectValue() wrote $state (customVisible,
customValues) while computing the font <select> value inside the {#each}
font-kinds block. An {#each} is a Svelte block effect, so that write
throws state_unsafe_mutation — but only when the component mounts during
a client-side route swap (the new pushState navigation) rather than a
fresh page load, and only when a custom (non-builtin) font is configured
so the branch runs. The error aborted the swap, leaving the settings
page blank (or the source page stuck).
Make the template-called helpers pure: derive the select value and the
custom-field visibility from settings, and track in-progress edits in a
draft written only from event handlers. Add an e2e regression
(client-side nav to /settings with a custom font) that blanks without
this fix.
handleNewSession initializes the worker on a background goroutine, so the chatSender's methods (EnsureWorker/SetModel/GetState/...) run concurrently with test assertions that read the fake's recorded fields — a data race under -race (e.g. TestHandleNewSessionCopiesSourceModelAndThinking polling ensureWorkerSessionID). Guard the mutated fields with a sync.Mutex: lock the writes in each sender method (channel sends stay outside the lock) and read them in tests through accessor helpers. Config fields set once at construction stay lock-free. Test-only; no production behavior changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pushState/popstateinApp.svelte, and key<SessionPage>on the?id=param so session→session navigation tears down and remounts instead of no-op'ing. Fork/clone/new-session flows now go throughnavigate()instead of full page reloads.windowbridges with a reactive store (session-modals.svelte.js) and asession-runtimeregistry.AppearanceSettingstemplate helpers pure so they no longer write$stateinside an{#each}block effect (state_unsafe_mutation).fakeSendershared fields with a mutex to fix a-racedata race; restore thewindow.__piCatGatekeepere2e seam dropped by the registry refactor and pin its sleep window; de-flake the load-earlier e2e by relying on auto-retrying assertions.Testing