wysi/client
Watch
1
0
Fork
You've already forked client
 0
Code Issues Pull requests 1 Projects Activity Actions

feature/deployment-readiness #8

Open
marti wants to merge 28 commits from feature/deployment-readiness into dev
pull from: feature/deployment-readiness
merge into: wysi:dev
Conversation 0 Commits 28 Files changed 68 +2667 -369
marti commented 2026-10-01 17:23:10 +02:00
Owner
No description provided. 
The app configured i18next inline in main.tsx, which tests never imported.
An uninitialised i18next makes useTranslation render nothing, so a component
test asserting on visible text would have passed against an empty DOM.

Both entry points now call into src/lib/i18n.ts. The test initialiser bundles
the English catalogue instead of loading it over HTTP, which a test has no
server to serve.
Only two routes defined an errorComponent, so a throw anywhere else rendered
a blank page and an unmatched URL rendered the router's bare default. Both now
fall back to the root route's boundary, which offers a retry, a link home, and
the server's request id so a bug report can be traced to a log line.

FourOuFour.tsx existed as a zero-byte file with no callers, so this finishes
the placeholder it was standing in for.
Badge.tsx was an empty module with no importers. /test was a BBCode scratch
route that shipped as its own production chunk and was publicly reachable.
Ten keys used in code were absent from the English catalogue, so they rendered
as the raw key: user_not_found and beatmap_not_found were both on error paths
users actually hit. Spanish and Catalan were each missing 36 keys.

All three catalogues now carry the same 231 keys. The new test fails on a key
that exists in code but not in a locale, on a locale drifting out of sync, and
on a translation that drops an interpolation placeholder -- none of which a
build or a typecheck can catch.
Both sinks that insert third-party HTML go through this: a beatmapset
description from the osu! API, and a medal's instructions from osekai. Neither
was sanitised, and both are written by other people. Since the access and
refresh tokens live in localStorage, script execution there is account
takeover rather than a cosmetic bug.

The policy lives in one module so it cannot drift between call sites, and it
forces rel=noopener on the links it keeps.
The tests hover the tooltip and wait for the payload to appear before
asserting it is inert, because the content is portalled in only on open --
asserting "no script element" against a closed tooltip would have passed
without proving anything. Reverting the sanitiser call makes four of the five
fail.
DOMPurify keeps its parser current against the browser attack surface, which
is the reason to use it rather than a hand-written element allowlist.
React escapes attribute values, so BBCode could not break out of the markup.
It could still hand an anchor an href of "javascript:", which a browser runs
the moment the reader clicks the link they were reading a profile to reach.

[url], [img], [audio] and imagemap redirects now go through safeUrl, and
[youtube] accepts only a bare video id so it cannot traverse out of /embed/
or append parameters. A rejected link renders as an anchor with no href, which
is neither focusable nor clickable.
The package's ESM build uses extensionless relative imports, which Node's
resolver rejects when Vitest externalises it. Without this the renderer could
not be rendered in a test at all, which is why the URL handling above went
uncovered for as long as it did.
The CI audit gate runs `bun audit --audit-level=high` and was failing on 18
advisories. All of them were in build or test tooling rather than the shipped
bundle, but a gate that is always red stops being read.

Vite moves to 7 so the app and Vitest resolve to one copy; two copies made the
plugin types in vite.config.ts mutually incompatible. esbuild and js-yaml are
pinned through overrides for the same reason.
Vite's defineConfig does not know about the `test` key, so it only typechecked
while the packages happened to share a Vite version.
A rate-limited response set a timer on the shared Fetcher, and while it ran
every request threw without reaching the network. Echo's limiter sends no
X-RateLimit headers, so the fallback was a flat 60 seconds -- one rate-limited
endpoint took the whole app offline.

The timer is gone. A 429 is now an ordinary error carrying the server's own
Retry-After, and the retry policy does the waiting. The `raw.includes("429")`
heuristic goes with it: it matched any error body containing those digits, so a
user called 429429 was treated as rate limited.

Separately, every request now has a timeout. Without one a single unresponsive
upstream -- and four of the five APIs this app calls are third parties -- held
its query open forever instead of surfacing an error.
retry was false, so one dropped connection left a query in an error state until
the user happened to interact with the page. Two attempts now, with 404s and
aborts excluded: a missing resource should not keep a spinner running, and an
aborted request has nobody waiting for the result.

A rate limit is only retried when the server asked for a wait this backoff can
cover, so a 429 does not burn the remaining attempts before its budget is back.
The app keeps its tokens in localStorage, so a script injection is account
takeover. The HTML sanitiser is the first line of defence; this is the second.

connect-src has to name four third-party APIs the browser calls directly, and
img-src the hosts that serve covers, avatars, medals and flags. Everything else
is closed: scripts are self-only, objects and form submissions are denied, and
the page cannot be framed. Styles keep 'unsafe-inline' because React inline
style props carry the tier gradients.

The API origin is substituted by Caddy at startup rather than baked into the
file, so one image can be promoted between environments. compose.prod.yaml
passes it and refuses to start without it.
Bun 1.3 cannot parse the lockfileVersion 2 lockfile and fails with
"Unknown lockfile version", which reads like a corrupt lockfile rather than an
old toolchain. docs/installation.md now says 1.4+.
The server now returns 400 for a malformed user id, mode, or category, and the
spec was regenerated there. This is the manual step the two CI pipelines cannot
do for each other.
index.html still carried create-tsrouter-app boilerplate as its description, and
the app had no Open Graph tags, no canonical, and no sitemap despite robots.txt
allowing everything. The html element also used className, which is React
syntax in a static file, so scroll-smooth was silently doing nothing.

The manifest claimed a white background against a dark-only theme, which flashes
white on install, and declared only a 64x64 ICO, which is not installable on
Android.
It renders as the raw key in every language until one exists.
The About panel's tab titles bypassed t() while the one next to them did not, so
a Spanish profile showed four English tabs. The collections importer logged
progress in English regardless of language, which is the one place a user is
watched for a couple of minutes.

The supporter badge rendered three identical hearts, which reads as a
placeholder; it is one heart with a label now.
The lang attribute is what a screen reader picks a pronunciation from and what
a crawler reads to decide whether a page is worth translating. Left at the
static "en" in index.html, a Spanish page was announced with English phonemes.
Declared in both the Vite and TypeScript configs, referenced by nothing.
All the exhaustive-deps warnings are gone, and three of them were real rather
than cosmetic:

- UserMedalsPanel built its fallback array and map inside the component, so
  every useMemo below had a fresh dependency each render and none of them
  memoised anything.
- SocialsRow called form.setFieldValue from inside a useMemo -- a side effect
  during render, skipped whenever the memo happened to be reused. Moved to an
  effect.
- global-context's subdivision guard read a stale map, so it was not the guard
  it appeared to be.

Two more were perf traps rather than bugs: useHotkeys defaulted to a fresh
array literal, which re-bound the keydown listener every render, and
BeatmapsForm would have looped had it depended on its props object instead of
the callback.
/scores/{scoreId} fetched the score, logged it, and rendered
<Hello "/scores/"!>. ScoreCard -- which already links here from every score
list -- was written and complete, so the route now renders it, with a link to
the beatmapset and a per-route title. The stray console.log goes with it.

This was a third unfinished surface alongside Skins and Timeline, which are
still stubs by request.
docs: describe the sanitiser, the CSP, and the locale rule
Some checks failed
CI / check (pull_request) Successful in 1m23s
Details
CI / types-drift (pull_request) Successful in 37s
Details
CI / audit (pull_request) Successful in 37s
Details
CI / docker (pull_request) Failing after 7s
Details
CI / nix (pull_request) Failing after 13s
Details
10883b1cef 
The sanitiser and the CSP are the two things standing between a third-party
payload and the session in localStorage, and neither was written down anywhere
an agent would look. The locale rule is here because adding a key to one file
is the easy mistake and it renders as a raw string on screen.
state was the path the user started from, and any value was accepted. That is
login CSRF: an attacker completes their own osu! authorisation, then sends a
victim a link carrying the attacker's code, and the victim ends up operating
inside the attacker's account -- uploading collections, linking socials, all of
it.

state is now a nonce the client issued and stored. The return path stays in
sessionStorage, so it no longer travels through a URL someone else can compose,
and it cannot be pointed anywhere by the callback. The record is consumed on
read, so a captured callback cannot be replayed.

PKCE is still absent, because osu! does not document supporting it and sending
parameters it ignores would only look like protection.
The entry is now ~675 kB (211 kB gzipped), down from ~710 kB (221 kB), and the
icons cache independently of the app.

Recharts was tried and reverted. Forcing it into one manual chunk defeated
tree-shaking and took it from 215 kB to 347 kB raw to save 10 kB gzipped on
entry -- a bad trade on a site whose rank history is a chart. Vite was already
splitting it per route, lazily.

The deployment note claimed charting and icons were the bulk of the entry chunk.
That was wrong, and is now corrected: what remains is React, Router, Query and
i18next.
test(e2e): drive the app in a browser
Some checks failed
CI / check (pull_request) Successful in 53s
Details
CI / types-drift (pull_request) Successful in 19s
Details
CI / audit (pull_request) Successful in 16s
Details
CI / e2e (pull_request) Successful in 1m30s
Details
CI / docker (pull_request) Failing after 4s
Details
CI / nix (pull_request) Failing after 8s
Details
56b98de7ac 
Playwright, running against the production bundle rather than the dev server,
with the API stubbed at the network layer -- which is also the only way to
exercise the signed-in state, since the real one needs an osu! OAuth round trip.

Four tests: a profile load, the signed-out state, the 404 page, and a
route-level error boundary. Together they cover the wiring that unit tests
cannot reach: routing, the providers, the query cache, and what a user ends up
looking at.

The user fixture is complete and typed as `User` on purpose. An earlier
hand-written JSON fixture was missing `join_date`, which InfoRow formats
unconditionally, so the profile threw and rendered "User not found" for reasons
unrelated to the code under test -- three tests failing for one absent field.
TypeScript now refuses to let a field go missing again.

Not covered: editing a setup. The form could not be driven reliably from the
test, and docs/deployment.md says so rather than leaving it looking tested.
Some checks failed
CI / check (pull_request) Successful in 53s
Details
CI / types-drift (pull_request) Successful in 19s
Details
CI / audit (pull_request) Successful in 16s
Details
CI / e2e (pull_request) Successful in 1m30s
Details
CI / docker (pull_request) Failing after 4s
Details
CI / nix (pull_request) Failing after 8s
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'feature/deployment-readiness:feature/deployment-readiness'
git switch 'feature/deployment-readiness'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'dev'
git merge --no-ff 'feature/deployment-readiness'
git switch 'feature/deployment-readiness'
git rebase 'dev'
git switch 'dev'
git merge --ff-only 'feature/deployment-readiness'
git switch 'feature/deployment-readiness'
git rebase 'dev'
git switch 'dev'
git merge --no-ff 'feature/deployment-readiness'
git switch 'dev'
git merge --squash 'feature/deployment-readiness'
git switch 'dev'
git merge --ff-only 'feature/deployment-readiness'
git switch 'dev'
git merge 'feature/deployment-readiness'
git push origin 'dev'
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
wysi/client!8
No description provided.