Restore the widget layer, and make session persistence hold - #484
Merged
AllTerrainDeveloper merged 1 commit intoAug 4, 2026
Merged
Conversation
Two unrelated regressions, both silent. The widget column stopped rendering. The rebrand renamed the shell element `desktop-mode-widgets` to `os-widgets` in the markup and left three TS lookups on the old id. The mount one decided whether `WidgetLayer` was ever constructed, so `getElementById` returned null, the guard took the "not present" branch, and the column rendered empty on every load — no error, no warning. The other two set the window-links z-order insertion point and the overview inert list. Closed windows came back on refresh, for three separate reasons: - A save triggered while another was on the wire was dropped. The debounce timer had already fired and cleared itself, so nothing retried and the mutation was lost. It now marks the session dirty and re-runs once the request settles. - `updated`, the server's write-ordering key, was stamped in seconds. The two writes that race hardest are the `keepalive` fetch still in flight and the `pagehide` beacon that supersedes it; at second resolution they tie, and the tie rule hands the win to whichever the server processes last — which can be the stale one. Now milliseconds on both sides. Sessions written before the switch carry a seconds value, ~1000x smaller, so the first write after an upgrade wins. - The 500ms debounce only collapsed changes closer together than its own window; closes a beat apart cleared it every time, and a settling save would hand straight over to its queued successor. A rate limit underneath it holds writes to one per 1500ms. Nothing is dropped to honour it — a save arriving too soon is delayed, and unload still flushes past everything. `createSessionSaver` had no coverage; it has nine tests now. The shell's five server-rendered element ids are pinned on both sides, so a rename can't silently orphan one again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AllTerrainDeveloper
deleted the
fix/widgets-layer-and-session-persistence
branch
August 4, 2026 10:45
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
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.
Two unrelated regressions, both silent — nothing threw, nothing warned.
The widget column stopped rendering
The rebrand (#475) renamed the shell element
desktop-mode-widgetstoos-widgetsinincludes/render/shell.phpand left three TypeScript lookups on the old id.The one that mattered is
src/desktop.ts— it decides whetherWidgetLayeris ever constructed:getElementByIdreturnednull, the guard took the "not present" branch, and the right-hand column rendered empty on every load. The other two stale lookups set the window-links z-order insertion point and the overview inert list.I swept the rest of the rebrand. Everything else lines up; the remaining
desktop-mode-*literals are the frozen ones (localStorage keys, native-window ids) and are correct perAGENTS.md.Closed windows came back on refresh
Three separate causes, same symptom.
Saves were silently dropped.
doSave()returned early when a request was in flight — and the debounce timer had already fired and cleared itself, so nothing retried. Close a second window while the first close is still on the wire and that close was lost for good. It now marks the session dirty and re-runs once the request settles.The write-ordering key was too coarse.
updatedwasMath.floor( Date.now() / 1000 ). The two writes that race hardest are thekeepalivefetch still in flight and thepagehidebeacon that supersedes it; at second resolution they tie, and the server's tie rule hands the win to whichever it processes last — which can be the stale one, reinstating a window the user just closed. Now epoch milliseconds on both sides, withopenstation_session_now_ms()matching the(int) round( microtime( true ) * 1000 )idiom already used in presence and content-changes. Sessions written before the switch carry a seconds value, ~1000× smaller, so the first write after an upgrade correctly wins.The debounce had no floor under it. The existing 500ms window only collapsed changes that arrived closer together than itself — closes a beat apart cleared it every time and posted three times, and a settling in-flight save would hand straight over to its queued successor.
SESSION_SAVE_MIN_INTERVAL_MS = 1500sits underneath the debounce and holds writes to one per interval however the changes are spaced. Nothing is dropped to honour it: a save arriving too soon is delayed, not discarded, andpagehidestill flushes past both viasendBeacon.Tests
createSessionSaverhad no coverage at all; it has nine tests now, covering burst collapsing, the in-flight re-send, the rate limit, failure recovery, and the beacon path.shell-element-ids.test.tspins the five server-rendered shell ids on both sides so a rename can't silently orphan one again.Each guard was verified to fail against the unfixed code — the two coalescing tests fail without the
dirtyflag, and the rate-limit test fails with the interval set to0.One thing worth calling out for review: the new PHPUnit
test_save_session_orders_writes_within_the_same_seconddocuments the ordering contract but is not a regression guard for the millisecond fix — the server comparison was already<and is unit-agnostic. The real guards for that one are the vitestsnapshot stamps updated in epoch millisecondsand the PHPUnit fallback-unit test.Green: build, typecheck, lint, PHPCS, 3667 vitest, 1954 PHPUnit.
Docs updated in
architecture.md(theupdatedordering contract) andjavascript-reference.md(saveSessionscheduling semantics).🤖 Generated with Claude Code