Skip to content

fix(ui): keep a token-holder in the app when the boot restore fails for non-auth reasons - #244

Merged
argszero merged 1 commit into
mainfrom
fix/boot-fallback-non-401
Sep 14, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/boot-fallback-non-401

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

A refresh on a slow instance could land the user on the login page while their token was still in storage — the classic "I got logged out" illusion. The boot session restore treated every non-401 failure of GET /api/me as "not signed in":

try { await loadSession(); enterApp(); }
catch (e) { if (e.status !== 401) toast(T("login.session.fail")); }   // ← stays on the login page

Host report (2026-09-14 21:00): with the transactions aggregate crawling, /api/me was dragged to a gateway 504, and every refresh of #/sharing stopped on the login form with the URL hash still at #/sharing and the token still present — three symptoms that together read as "kicked out".

Related Issue

Host rant 2026-09-14T21:15:02 (aitokenpool), acceptance item 4 — the last of the four. Items 1–3 landed as #240 / #243 / #242.

Changes

  • ui/js/app.js: the boot IIFE now only does if (api.getToken()) await restoreSession();, and the new restoreSession() splits the outcome three ways — only the first means "not signed in":
    • 401api.js has already cleared the token and returned to the login page; never retried (retrying cannot make a dead token valid);
    • network error / 5xx (incl. 504) → retried once after 1 s, then the app is entered anyway with a "session data failed to load" toast;
    • other 4xx → not retried (pointless), but still enters the app.
  • ui/js/i18n.js: login.session.fail no longer says "please sign in again" — a user who still holds a token is not signed out.
  • src/i18n_pack.rs: new test session_failure_copy_does_not_claim_the_user_is_logged_out pins the copy rule (the two copies must differ; the load-failure copy must not read like a re-authentication demand), with a synthetic positive/negative control on the detector.
  • ui/README.md: records the invariant and the smoke-test shape.
  • ui/index.html: cache-bust i18n.js / app.js.

Deliberately not changed: the backend 401 semantics (POST /api/auth/login and change-password use 401 for "these credentials are wrong" — a separate front-end issue), and the login submit path (there the user has just acted and can retry by submitting again; boot has no such out, which is why it is the one that had to be fixed).

Tests

  • cargo test257 passed (256 before; +1 new test)
  • cargo fmt --check — clean
  • cargo clippy --all-targets — no new warnings (the one remaining collapsible_match at src/protocol.rs:662 is pre-existing and does not reproduce on CI's stable toolchain)
  • New unit test added (copy rule)

How the JS half was verified

jsdom boots the real ui/index.html + the four real scripts (only fetch is stubbed) and scripts /api/me. Checks, all green on this branch (8/8):

check /api/me expectation
A0 200 app shown, login hidden, 1 call, no error toast
A1 — (no token) login stays, 0 calls
B1 504 app shown, login hidden, token kept
B2 fetch rejects same as B1
B4 504 then 200 recovers, exactly 2 calls, no error toast
B5 403 1 call (4xx is not transient), still enters the app
B3 401 login shown, token cleared, 1 call (unchanged)
D1 504 degraded copy ≠ 401 copy, and never demands re-authentication

A/B legs, each measured on its own source (the working tree was never rewritten; variants were injected by path):

leg red checks
pre-fix app.js @ 199566c B1,B2,B4,B5,D1
retry added, but still lands on the login page (the tempting half-fix) B1,B2,B5
enters the app but never retries B4
over-reach: enters the app on 401 as well B3
this branch none

The half-fix leg is the point: it proves the instrument rejects "just add a retry".

Checklist

  • Branch name follows the convention (fix/…)
  • Conventional Commits format
  • Single responsibility, minimal change

…or non-auth reasons

The boot session restore treated every non-401 failure of `GET /api/me` as "not signed in":
it left the user on the login page while the token was still in storage and the URL hash still
pointed at the previous view — visually identical to being logged out (host report 2026-09-14
21:00: with a slow transactions aggregate, `/api/me` was dragged to a gateway 504 and every
refresh of `#/sharing` landed on the login form).

Boot now goes through `restoreSession()`, which splits the outcomes three ways and only lets the
first one mean "not signed in":

- 401        -> api.js already cleared the token and returned to the login page; never retried
                (retrying cannot make a dead token valid).
- network / 5xx (incl. gateway 504) -> retried once after 1s, then the app is entered anyway and
                a "session data failed to load" toast is shown.
- other 4xx  -> not retried (pointless), but still enters the app.

The two states are kept apart on purpose: "load failed" is carried by each view's own load-error
state (`loadErrorHtml` / `loadErrorRow` + retry), and "not signed in" only by the 401 path. If the
token really is dead, the first real request after entering gets its 401 and api.js clears the
token — that path already says the honest "session expired".

- ui/js/app.js    : add `restoreSession()`, route the boot IIFE through it
- ui/js/i18n.js   : `login.session.fail` no longer instructs a signed-in user to sign in again
- src/i18n_pack.rs: pin the copy rule (the two copies must differ; the load-failure copy must not
                    read like a re-authentication demand). The control-flow half has no CI JS test
                    runner, so it is documented in ui/README.md instead.
- ui/README.md    : record the invariant and the smoke-test shape
- ui/index.html   : cache-bust i18n.js / app.js
@argszero
argszero merged commit 3199731 into main Sep 14, 2026
1 check passed
@argszero
argszero deleted the fix/boot-fallback-non-401 branch September 14, 2026 15:04
argszero added a commit that referenced this pull request Sep 14, 2026
`ui/js/api.js` answered *every* 401 with the global sign-out (clear the token, bounce to the login
page, `toast(login.session.expired)`), but `POST /api/auth/login` deliberately answers 401 for an
unknown email or a wrong password (`src/routes/mod.rs::login`). Typing a wrong password therefore
drew "Session expired, please sign in again" **on top of** the inline "Incorrect email or
password" — telling somebody who had never signed in that their session had expired. The same
shape is worse for `POST /api/auth/change-password`, whose 401 means "wrong old password": it
would clear a still-valid token and throw the user out of the app (that endpoint has no front-end
consumer today, so only the rule is recorded).

The two meanings of 401 are indistinguishable in the throat, so the caller now declares which one
applies:

    api.post("/api/auth/login", { email, password: pass }, { on401: api.CREDENTIAL_401 })

Anything else — including an unknown value — keeps the session-expiry reading, so a forgotten
declaration fails towards "sign the user out once too often", never towards "leave a dead session
standing". With the flag set, the 401 falls through to the ordinary error construction, keeping
`status === 401` so the login form's inline error branch still fires.

The login flow also stops hand-rolling its bring-in: once the credentials are accepted it reuses
the boot entry point `restoreSession()`, so a token holder can no longer be stranded on the login
page when the session load fails for a non-auth reason — the same invariant as #244, reached
through a different door (before: `saveToken()` then a failing `loadSession()` left a saved token
behind a login form).

`src/i18n_pack.rs::credential_401_is_not_a_session_expiry` pins the shape in CI: the login call
site declares the semantics, the throat's 401 branch is guarded by that declaration, and the
global sign-out still exists (deleting it would make the login page quiet while silently disabling
sign-out for every business endpoint). The control flow itself has no JS runner in CI, so the
runtime half is carried by `ui/README.md` and its smoke-test notes.

Verified with a jsdom instrument over the real `ui/index.html` + four real scripts (only `fetch`
stubbed) driving the real login form, four legs: login401 / login500 / boot401 / loginok.
Pre-fix tree (pinned to `origin/main`): red {login401: B1, login500: E1,E3}. Fixed tree: green.
Mutation legs, each turning exactly its own assertions red: guard removed -> {B1}; call-site
declaration removed -> {B1}; bring-in reverted -> {E1,E3}; global sign-out deleted -> {C1,C3}
(disjoint from the axis, which is the point — it pins the direction, not just "a change");
inline credential error dropped -> {B3}. The pre-fix red set equals the union of the first three.

- ui/js/api.js    : `CREDENTIAL_401` + `opts.on401` guard; verbs take opts
- ui/js/app.js    : declare the login endpoint's 401 semantics; bring-in via `restoreSession()`
- src/i18n_pack.rs: CI tripwire for the shape, with detector controls
- ui/README.md    : record the rule and the smoke-test shape
- ui/index.html   : cache-bust api.js / app.js

Co-authored-by: EMRG Evolution <emrg@argszero.dev>
@argszero argszero mentioned this pull request Sep 15, 2026
10 tasks
argszero added a commit that referenced this pull request Sep 15, 2026
Ships the 18 PRs merged since v0.7.24 (#242-#259). Schema 14 -> 15 (two
covering indexes, applied at startup). No config change, so no deployment-side
config.toml edit is needed.

Two themes:

Perf on the NFS dev database
- #259: stop mapping the db (PRAGMA mmap_size 64MB -> 0) and stop a real write
  per request (dao::touch_api_key gains a 60s guard). Measured on the live dev
  db: mmap=64MB 1.7-3.1s per COUNT / 250 MiB read vs mmap=0 ~10.5ms / 80 KiB;
  mmap=0 alone still leaves ~1.2s behind any write, so the pair is required.
- #242: codify the two emergency indexes in a v15 migration and gate the
  conditional joins at the plan level.
- #243: read the sharing page's earn total from one batched aggregate.

Frontend: display must equal what it filters on, and one fact, one source
- #250 one writer for the transaction cache; #251 clear every session slot at
  the identity boundary and give the wallet view a loader; #253 one shared
  writer for the wallet/dashboard month-changes; #254 boot loads only the
  destination view; #255 a model row's identity is the model, not its index;
  #256 the marketplace source follows the session, not whether data arrived;
  #257 the sidebar advertises only digits that work; #258 the admin
  total-balance card sums the gift amount its caption names.

i18n
- #249 every backend error reaches the wordlist, and the comment stripper stops
  mangling UTF-8; #252 the backend stops inventing Chinese display labels in
  response data fields.

Forms and robustness
- #244 a non-auth boot failure no longer looks like being logged out; #245 a
  credential 401 is no longer read as a session expiry; #246 wire timestamps
  reach the renderer unsliced; #247 inline cards submit from every field; #248
  a market row's availability label comes from that row.

- Cargo.toml / Cargo.lock: 0.7.24 -> 0.7.25.
- CHANGELOG.md: v0.7.25 entry.
- ui/index.html: cache-bust left as-is; the UI PRs in this release already
  advanced it past the value deployed with v0.7.24 (app.js 20260915-13,
  i18n.js 20260915-3).

cargo test 288 passed; cargo fmt --check clean; clippy unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant