Claude: changes for issue #186 #187

Merged

Grandiras merged 1 commit from claude/issue-186 into main

2026-09-11 00:42:23 +00:00

claude-bot commented

2026-09-11 00:39:25 +00:00

Collaborator

Closes #186.

claude-bot added 1 commit

2026-09-11 00:39:25 +00:00

Reopen the app on the page you were last on (#186 )

CI / build (pull_request) Successful in 38s

Details

92a58a8ec1

An installed app opens at the manifest's start_url, so it always opened on
the default board — and anybody whose work lives on another board or on the
calendar paid the same two taps on every launch.

start_url is now /launch, an endpoint that redirects to the page this browser
last had open. Services/LastPage.cs is the rule: one method used in both
directions, so what may be written down is exactly what may be redirected to
and a hand-edited cookie cannot make /launch an open redirect. It is a deny
list — doors and errors are not places — so a page added to the app later is
remembered without anybody coming back to say so.

Not a rule about `/`: that is the default board's own address and the wordmark
links to it, so a `/` that bounced elsewhere would leave no way home. The
manifest's `id` stays `/` too, or every existing install becomes a second app
rather than an updated one.

The page is kept in a cookie rather than localStorage — the one thing in
prefs-interop.js that is — because the only reader is the server deciding
where to send a launch before it renders anything. MainLayout writes it, on
its first render and on every LocationChanged after, since every page renders
inside it.

LastPageTests pins the rule; prefs.spec.js pins the round trip. The route is
mapped through a constant, so DocumentationTests learns to resolve one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

claude-bot referenced this pull request

2026-09-11 00:39:26 +00:00

Let the app remember the last open page and reopen it (especially for the PWA) #186

claude-bot commented

2026-09-11 00:39:30 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/461); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-187/run-461.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-11 00:41:27 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This implements the /launch reopen-where-you-left-off feature (issue #186) end to end: LastPage.cs (the single rule used both for writing the cookie and for resolving the redirect), the /launch endpoint in Program.cs, MainLayout.razor recording navigations via Nav.LocationChanged, the prefs-interop.js cookie writer, the manifest start_url change, and matching unit tests (LastPageTests), e2e tests (prefs.spec.js, icons.spec.js), and documentation updates (docs/ui.md, docs/orientation.md, docs/frontend.md, CLAUDE.md).

I checked the parts most likely to hide a bug:

LastPage.Remember correctly delegates to ReturnUrl.Sanitize for the open-redirect gate (protocol-relative, backslash-relative, control characters all rejected), so a hand-edited cookie can't turn /launch into an open redirect — verified against the actual ReturnUrl.cs implementation, not just the docstring's claim.
The manifest already had "id": "/" before this change, so the new icons.spec.js assertion (expect(manifest.id).toBe('/')) isn't asserting on a field the diff forgot to add.
/launch is intentionally left off any RequireAuthorization() call, and the app has no global fallback authorization policy (Program.cs comment: "nothing is forced as a fallback so the OAuth library's /atproto/* endpoints stay reachable"), so the endpoint really is reachable unauthenticated as the docs describe — the destination page's own [Authorize]/policy is what gates a stranger.
DocumentationTests's new ConstantRoute resolution correctly extracts LastPage.Route from Services/LastPage.cs so the documented /launch route doesn't go stale relative to the literal.
MainLayout's IAsyncDisposable correctly unhooks Nav.LocationChanged before disposing the JS module, and the module import mirrors NavHeader's existing ./js/prefs-interop.js import (ES module caching means this costs nothing extra).
The deny-list approach (NotPlaces) is deliberately fail-open for new pages, consistent with the stated rationale, and query-stripping/case-insensitivity both check out against the test cases.

One minor, non-blocking note: OnLocationChanged fires _ = InvokeAsync(RememberPageAsync) without awaiting or serializing against prior in-flight calls, so on a rapid multi-hop navigation within one circuit (e.g. a same-circuit redirect chain) the JS cookie-write calls could theoretically complete out of order and leave a slightly stale page remembered. Given this is a best-effort convenience cookie with a graceful failure story already built in, I wouldn't block on it, but it's worth being aware of if a future spec exercises fast redirect chains.

Overall this is careful, well-tested work that matches its own extensive documentation.

⏱ 1m 41s · 8 turns · tokens: 40.3k in / 8.3k out (+395.3k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This implements the `/launch` reopen-where-you-left-off feature (issue #186) end to end: `LastPage.cs` (the single rule used both for writing the cookie and for resolving the redirect), the `/launch` endpoint in `Program.cs`, `MainLayout.razor` recording navigations via `Nav.LocationChanged`, the `prefs-interop.js` cookie writer, the manifest `start_url` change, and matching unit tests (`LastPageTests`), e2e tests (`prefs.spec.js`, `icons.spec.js`), and documentation updates (`docs/ui.md`, `docs/orientation.md`, `docs/frontend.md`, `CLAUDE.md`). I checked the parts most likely to hide a bug: - `LastPage.Remember` correctly delegates to `ReturnUrl.Sanitize` for the open-redirect gate (protocol-relative, backslash-relative, control characters all rejected), so a hand-edited cookie can't turn `/launch` into an open redirect — verified against the actual `ReturnUrl.cs` implementation, not just the docstring's claim. - The manifest already had `"id": "/"` before this change, so the new `icons.spec.js` assertion (`expect(manifest.id).toBe('/')`) isn't asserting on a field the diff forgot to add. - `/launch` is intentionally left off any `RequireAuthorization()` call, and the app has no global fallback authorization policy (`Program.cs` comment: "nothing is forced as a fallback so the OAuth library's /atproto/* endpoints stay reachable"), so the endpoint really is reachable unauthenticated as the docs describe — the destination page's own `[Authorize]`/policy is what gates a stranger. - `DocumentationTests`'s new `ConstantRoute` resolution correctly extracts `LastPage.Route` from `Services/LastPage.cs` so the documented `/launch` route doesn't go stale relative to the literal. - `MainLayout`'s `IAsyncDisposable` correctly unhooks `Nav.LocationChanged` before disposing the JS module, and the module import mirrors `NavHeader`'s existing `./js/prefs-interop.js` import (ES module caching means this costs nothing extra). - The deny-list approach (`NotPlaces`) is deliberately fail-open for new pages, consistent with the stated rationale, and query-stripping/case-insensitivity both check out against the test cases. One minor, non-blocking note: `OnLocationChanged` fires `_ = InvokeAsync(RememberPageAsync)` without awaiting or serializing against prior in-flight calls, so on a rapid multi-hop navigation within one circuit (e.g. a same-circuit redirect chain) the JS cookie-write calls could theoretically complete out of order and leave a slightly stale page remembered. Given this is a best-effort convenience cookie with a graceful failure story already built in, I wouldn't block on it, but it's worth being aware of if a future spec exercises fast redirect chains. Overall this is careful, well-tested work that matches its own extensive documentation. --- ⏱ 1m 41s · 8 turns · tokens: 40.3k in / 8.3k out (+395.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/Ponente/pr-187/run-461.md)