fix(web): lift route repair out of TreePane into the shell #36

Merged

dominik.polakovics merged 1 commit from afk/35 into main

2026-07-14 22:08:41 +02:00

dominik.polakovics commented

2026-07-14 21:31:52 +02:00

Owner

Route repair was implemented as an effect inside TreePane — a conditionally-mounted component — and navigated with a push. All three defects in #35 follow from that one placement, so they are fixed together by moving the rule to where it belongs.

What changed

New web/src/routerepair.ts. A pure repairRoute(route, facts) judges module, language and page against the module registry and the page tree, and returns the path the URL must be replaced with (or null when the route is sound). A shell-level effect applies it. It waits for the tree, because the site's language is the tree's to report — until it arrives there is no route to repair to.

Defect 1 — repair only ran while the tree was mounted. It now runs in the shell, so a collapsed tree column (persisted in sessionStorage) or a future module declaring tree: false no longer leaves a stale bookmark stranded on a permanent error pane. The module body is additionally held back until the route is worth rendering: a route awaiting repair can only fetch a 404 or a 400, so the doomed request is never issued and the error pane never flashes.
Defect 2 — the repair pushed. Every repair now replaces, so Back is no longer trapped bouncing off the bad URL. selectPage gains an explicit history intent: a tree click pushes (a user navigation Back must undo), while a follow of a server mutation replaces, because the URL it would otherwise leave behind names a file the site no longer has.
Defect 3 — an unknown language was never repaired. The language is now corrected to the one the tree reports, and the page is kept when the tree has it: on an English site /page/de/about.md lands on /page/en/about.md rather than dropping the page.

The mutation-follow seam. Rename, move and bundle conversion change the tree and the route together and cannot do it atomically — the fresh tree lands while the URL still names the old path, and Solid flushes effects synchronously on that write, so the repair fired in between and pruned the editor to the empty state. follow() suspends repair for the length of such a follow, stating the invariant honestly (the route and the tree are allowed to disagree while one is in flight) and re-judging the now-consistent pair on the way out, so a follow that left a bad route behind is still caught. This is what removes the spurious empty-state history entry every rename, move and conversion used to leave behind.

Also fixed — the two defects recorded under "Not blocking" in the issue:

saveAndContinue was re-entrant: a double-click on "Save & continue" posted the same page file twice, the exact read-modify-write race its sequential-save loop is documented to prevent. It now drops a re-entrant call (before taking the one-shot retry continuation, so the pending navigation survives) and the prompt shows its work and stops taking answers while saves are on the wire.
A pending preview-bridge selection was only ever cleared when its own page loaded, so navigating away first left it lying in wait to hijack that page's element selection on a later visit. It is now dropped once the editor leaves the page it names — armed only after the route has actually reached that page, because the router applies a navigation in a transition and clearing on any mismatch would destroy every bridge selection the instant it was stashed.

One inherited asymmetry, called out deliberately

An unknown language keeps a page the tree has (/page/de/about.md → /page/en/about.md), but an unknown module drops everything behind it (/bogus/en/about.md → /page/en). The latter is the behavior #33 shipped and its test pins it; changing it is not in scope for this issue, so it was left alone rather than quietly redefined. Worth a look if you disagree.

Verification

Written test-first: the three repros from the issue were encoded as tests against the broken code and observed to fail for the right reasons (URL never repairs / history.length grows / lang left broken) before any fix was written.

web/src/test/ShellRouting.test.tsx — 5 new tests: repair with the tree collapsed, repair costing no history entry, a rename leaving no empty-state entry behind, an unknown lang keeping its page, and an unknown lang plus a nonexistent page repaired in one step.
web/src/test/routerepair.test.ts — 16 table-driven unit tests for the pure judgment, including the tree-not-loaded case that must repair nothing.
Guard re-entrancy and the bridge-hijack scenario have unit and UI tests; both were proven by temporarily installing the wrong implementation and confirming the tests fail (the naive pending-select fix breaks 3 existing bridge tests — the transition-window trap).

Full gate green: tsc --noEmit clean, npm run build OK, 418 web tests pass (389 on base), go vet clean, all Go packages pass.

Closes #35

Route repair was implemented as an effect inside `TreePane` — a conditionally-mounted component — and navigated with a push. All three defects in #35 follow from that one placement, so they are fixed together by moving the rule to where it belongs. ## What changed **New `web/src/routerepair.ts`.** A pure `repairRoute(route, facts)` judges module, language and page against the module registry and the page tree, and returns the path the URL must be **replaced** with (or null when the route is sound). A shell-level effect applies it. It waits for the tree, because the site's language is the tree's to report — until it arrives there is no route to repair *to*. - **Defect 1 — repair only ran while the tree was mounted.** It now runs in the shell, so a collapsed tree column (persisted in `sessionStorage`) or a future module declaring `tree: false` no longer leaves a stale bookmark stranded on a permanent error pane. The module body is additionally held back until the route is worth rendering: a route awaiting repair can only fetch a 404 or a 400, so the doomed request is never issued and the error pane never flashes. - **Defect 2 — the repair pushed.** Every repair now replaces, so Back is no longer trapped bouncing off the bad URL. `selectPage` gains an explicit history intent: a tree click **pushes** (a user navigation Back must undo), while a follow of a server mutation **replaces**, because the URL it would otherwise leave behind names a file the site no longer has. - **Defect 3 — an unknown language was never repaired.** The language is now corrected to the one the tree reports, and the page is *kept* when the tree has it: on an English site `/page/de/about.md` lands on `/page/en/about.md` rather than dropping the page. **The mutation-follow seam.** Rename, move and bundle conversion change the tree and the route together and cannot do it atomically — the fresh tree lands while the URL still names the old path, and Solid flushes effects synchronously on that write, so the repair fired *in between* and pruned the editor to the empty state. `follow()` suspends repair for the length of such a follow, stating the invariant honestly (the route and the tree are allowed to disagree while one is in flight) and re-judging the now-consistent pair on the way out, so a follow that left a bad route behind is still caught. This is what removes the spurious empty-state history entry every rename, move and conversion used to leave behind. **Also fixed** — the two defects recorded under "Not blocking" in the issue: - `saveAndContinue` was re-entrant: a double-click on "Save & continue" posted the same page file twice, the exact read-modify-write race its sequential-save loop is documented to prevent. It now drops a re-entrant call (before taking the one-shot `retry` continuation, so the pending navigation survives) and the prompt shows its work and stops taking answers while saves are on the wire. - A pending preview-bridge selection was only ever cleared when its own page loaded, so navigating away first left it lying in wait to hijack that page's element selection on a later visit. It is now dropped once the editor leaves the page it names — armed only after the route has actually *reached* that page, because the router applies a navigation in a transition and clearing on any mismatch would destroy every bridge selection the instant it was stashed. ## One inherited asymmetry, called out deliberately An unknown **language** keeps a page the tree has (`/page/de/about.md` → `/page/en/about.md`), but an unknown **module** drops everything behind it (`/bogus/en/about.md` → `/page/en`). The latter is the behavior #33 shipped and its test pins it; changing it is not in scope for this issue, so it was left alone rather than quietly redefined. Worth a look if you disagree. ## Verification Written test-first: the three repros from the issue were encoded as tests against the broken code and observed to fail for the right reasons (URL never repairs / `history.length` grows / lang left broken) before any fix was written. - `web/src/test/ShellRouting.test.tsx` — 5 new tests: repair with the tree collapsed, repair costing no history entry, a rename leaving no empty-state entry behind, an unknown lang keeping its page, and an unknown lang plus a nonexistent page repaired in one step. - `web/src/test/routerepair.test.ts` — 16 table-driven unit tests for the pure judgment, including the tree-not-loaded case that must repair nothing. - Guard re-entrancy and the bridge-hijack scenario have unit and UI tests; both were proven by temporarily installing the wrong implementation and confirming the tests fail (the naive pending-select fix breaks 3 existing bridge tests — the transition-window trap). Full gate green: `tsc --noEmit` clean, `npm run build` OK, **418 web tests pass** (389 on base), `go vet` clean, all Go packages pass. Closes #35

dominik.polakovics added 1 commit

2026-07-14 21:31:52 +02:00

fix(web): lift route repair out of TreePane into the shell

ci / build-and-test (pull_request) Successful in 6m1s

Details

87073c671e

Route repair lived in TreePane -- a conditionally-mounted component -- and
navigated with a push. Three defects followed from that one placement.

A URL naming a page the tree does not have was only repaired while the tree
column happened to be mounted: collapse the tree (persisted across reloads)
and a stale bookmark stranded the editor on a permanent error pane, as would
any module declaring `tree: false`. The repair also pushed, so Back returned
to the bad URL, which repaired and pushed again -- the editor could never
navigate back past it -- and every rename, move and bundle conversion left a
spurious empty-state entry in history on the way to the new path. An unknown
language was never repaired at all: the page fetch 400s ("unknown lang") and
the editor parked on a route no tree click could correct.

Repair is a rule about the route, so it now lives at the route's level, in
routerepair.ts: a pure `repairRoute` judging module, language and page against
the registry and the tree, and a shell-level effect that REPLACES. It runs
whether or not any tree is mounted, and the module body is held back until the
route is worth rendering -- a route awaiting repair can only fetch a 404 or a
400, so the doomed request is never issued and the error pane never flashes.

Rename, move and bundle conversion change the tree and the route together and
cannot do it atomically: the fresh tree lands while the URL still names the old
path. `follow()` suspends repair for the length of such a mutation-follow,
stating the invariant honestly -- the two are allowed to disagree in that window
-- and re-judges the now-consistent pair on the way out. selectPage gains an
explicit history intent: tree clicks push, follows of a server mutation replace,
because the URL they would leave behind names a file the site no longer has.

Also fixes the two defects recorded alongside these: saveAndContinue was
re-entrant (a double-click on "Save & continue" posted the same page file
twice, the very race its sequential loop exists to prevent), and a pending
preview-bridge selection was only ever cleared when its own page loaded, so
navigating away first left it lying in wait to hijack that page's element
selection on a later visit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dominik.polakovics commented

2026-07-14 22:07:47 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PR #36

Verdict: PASS (one non-blocking note).

Verification signal relied on: Forgejo Actions CI — ci / build-and-test (pull_request) → success (6m01s, run 26). Not re-run locally; the PR is mergeable into main with no conflicts.

Checks performed

Conventions — title fix(web): lift route repair out of TreePane into the shell is Conventional Commits. Body carries a working Closes #35, so the merge will auto-close the issue and release the afk/35 claim.
Intent vs. issue #35 — all three defects are genuinely addressed at the root cause the issue names (placement, not symptom):
1. Repair now runs in the shell via createRouteRepair, so a collapsed tree column (or a future tree: false module) no longer strands a stale bookmark on a permanent error pane.
2. Every repair replaces; selectPage gains an explicit push/replace intent so tree clicks still push and mutation-follows replace.
3. repairRoute judges language and page independently and keeps the survivors, so /page/de/about.md lands on /page/en/about.md rather than dropping the page.
Language model — repairRoute hard-forces the tree's reported language. Verified this is correct for v1: internal/server/server.go:55 declares lang string // configured language (v1: single). It will need revisiting if the site ever speaks more than one language, but it is not a bug today.
Guard re-entrancy — the saving() bail-out is correctly placed before the one-shot retry continuation is taken, so a dropped double-click does not strand the pending navigation.
Suspension counter — follow() uses a counter rather than a flag, so nested follows (a conversion inside a rename) cannot lift the suspension early.

Non-blocking note — one overclaim in the PR body. The body states the doomed request "is never issued and the error pane never flashes." That holds for any repair after the tree has loaded, but not on a cold load: repair.pending() is false while lang === "", so on first paint of /page/en/ghost.md the module body renders and GET /api/page is issued in parallel with GET /api/tree. If the 404 returns first, page-error renders until the tree lands and the repair replaces the route. This is transient and cosmetic, it is strictly better than main (where the same route is a permanent error page with the tree collapsed), and it matches the "flashes for one frame" aside the issue itself recorded — so it is not a blocker, just a claim worth not believing verbatim.

Also verified in scope: the two "Not blocking" defects from the issue (re-entrant saveAndContinue, the pending preview-bridge selection that could hijack a later visit) are both fixed and tested.

> *This was generated by AI while landing a PR.* ## Landing audit — PR #36 **Verdict: PASS** (one non-blocking note). **Verification signal relied on:** Forgejo Actions CI — `ci / build-and-test (pull_request)` → **success** (6m01s, run 26). Not re-run locally; the PR is mergeable into `main` with no conflicts. **Checks performed** - **Conventions** — title `fix(web): lift route repair out of TreePane into the shell` is Conventional Commits. Body carries a working `Closes #35`, so the merge will auto-close the issue and release the `afk/35` claim. - **Intent vs. issue #35** — all three defects are genuinely addressed at the root cause the issue names (placement, not symptom): 1. Repair now runs in the shell via `createRouteRepair`, so a collapsed tree column (or a future `tree: false` module) no longer strands a stale bookmark on a permanent error pane. 2. Every repair `replace`s; `selectPage` gains an explicit push/replace intent so tree clicks still push and mutation-follows replace. 3. `repairRoute` judges language and page independently and keeps the survivors, so `/page/de/about.md` lands on `/page/en/about.md` rather than dropping the page. - **Language model** — `repairRoute` hard-forces the tree's reported language. Verified this is correct for v1: `internal/server/server.go:55` declares `lang string // configured language (v1: single)`. It will need revisiting if the site ever speaks more than one language, but it is not a bug today. - **Guard re-entrancy** — the `saving()` bail-out is correctly placed *before* the one-shot `retry` continuation is taken, so a dropped double-click does not strand the pending navigation. - **Suspension counter** — `follow()` uses a counter rather than a flag, so nested follows (a conversion inside a rename) cannot lift the suspension early. **Non-blocking note — one overclaim in the PR body.** The body states the doomed request "is never issued and the error pane never flashes." That holds for any repair *after* the tree has loaded, but not on a cold load: `repair.pending()` is false while `lang === ""`, so on first paint of `/page/en/ghost.md` the module body renders and `GET /api/page` is issued in parallel with `GET /api/tree`. If the 404 returns first, `page-error` renders until the tree lands and the repair replaces the route. This is transient and cosmetic, it is strictly better than `main` (where the same route is a *permanent* error page with the tree collapsed), and it matches the "flashes for one frame" aside the issue itself recorded — so it is not a blocker, just a claim worth not believing verbatim. **Also verified in scope:** the two "Not blocking" defects from the issue (re-entrant `saveAndContinue`, the pending preview-bridge selection that could hijack a later visit) are both fixed and tested.

dominik.polakovics merged commit 754dec9a4d into main

2026-07-14 22:08:41 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-14 22:08:43 +02:00

Merge pull request 'fix(web): lift route repair out of TreePane into the shell' (#36) from afk/35 into main