Unify competition, achievements, and public sharing - #15

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/package.json`:
- Line 8: Update the package.json test script to use node --test src/ so test
discovery works across all supported Node runtimes, or alternatively add an
engines.node requirement of >=21 to match the quoted glob behavior. Keep the
existing .test.mjs test coverage intact.

In `@backend/src/competitionArchive.pr15.test.mjs`:
- Around line 5-32: Replace the source-text regex tests in the archive test file
with behavioral tests that build the server against a temporary SQLite database
and exercise routes via fastify.inject(). Cover schema migration without data
loss, archive/unarchive behavior and listings for leagues and tournaments, 404
responses for missing resources, and backup restore plus tournament CSV
preservation of active state, including the leagues INSERT restore path.

In `@backend/src/publicMetadata.pr15.test.mjs`:
- Around line 25-43: Expand the HTML fixture in the replacement test around
inject to include stale og:description, og:url, og:type, twitter:title,
twitter:description, and twitter:card tags alongside the existing managed tags.
Add assertions that each managed metadata tag occurs exactly once after
injection, while preserving the existing escaping, replacement-value, and
private.example checks.
- Around line 58-69: Replace the serverSource regex assertions in crawler HTML
fallthrough covers API-colliding public result routes with request-level tests
against the registered server handlers. For each listed route, request a
representative URL and assert an HTML content type plus metadata-injected HTML
containing the resolved title and canonical og:url; retain or update the image
metadata assertion only if it is validated through the response.

In `@backend/src/server.ts`:
- Around line 2461-2465: Align both soft-delete handlers with the archive helper
contract: in backend/src/server.ts lines 2461-2465, update DELETE /leagues/:id
to use the same Number.isInteger validation as setLeagueArchived and verify
callers no longer require permanent deletion; if they do, add a separate
hard-delete route. Apply the same validation and caller-contract check to DELETE
/tournaments/:id at lines 3339-3356, using setTournamentArchived.
- Around line 1381-1390: Extract the shared archive-query logic into helpers
near the existing listing handlers, including a wantsArchived-style parser for
request.query and reusable league and tournament row-selection helpers. Replace
the duplicated activeFilter setup and SQL bodies in the league handlers around
the current code and the corresponding tournament handlers with calls to those
helpers, preserving the existing includeArchived behavior and result shape.
- Around line 5318-5336: Update requestOrigin so unconfigured public origins
never derive URLs from Host or X-Forwarded-Host; return the safe
http://localhost:3003 fallback instead. If forwarded headers remain supported,
configure Fastify trustProxy and validate the resolved host against an explicit
allowlist before constructing the origin, while preserving
BOWLSENSE_PUBLIC_ORIGIN as the configured public-domain source.
- Around line 5524-5527: Update the SPA fallback around
resolvePublicPageMetadata to avoid synchronous per-request work: cache
index.html rather than calling readFileSync for every navigation, and cache the
aggregate result used by the /bowl metadata path for a short interval (or reuse
a maintained summary updated by write paths) so resolvePublicPageMetadata does
not repeatedly scan the games table.

In `@frontend/src/components/ShareCard.tsx`:
- Around line 206-211: Add role="img" to the score-card canvas in ShareCard and
provide an accessible label or equivalent hidden summary describing all rendered
details, including location, date, ball, and frame marks. Keep the existing
visual canvas rendering unchanged.

In `@frontend/src/features/competition/publicMetadata.pr15.test.tsx`:
- Around line 59-78: Add a rerender scenario to the public metadata hook tests
using the existing Harness and pre-rendered metadata setup: render with an
initial title, rerender with a second title, then unmount and assert the
original server-rendered title and metadata are restored, including any relevant
tag removal behavior. This should exercise snapshot recapture across the effect
cleanup and dependency-change cycle.

In `@frontend/src/features/competition/publicMetadata.ts`:
- Around line 65-92: Review usePublicMetadata and determine whether its cleanup
behavior restoring the previous document.title and metadata is a deliberate
requirement; if it is not required, migrate the metadata updates to React 19
native title/meta elements and remove the manual DOM override logic. If restore
semantics are required, retain the existing hook implementation and document or
preserve that requirement rather than switching approaches.

In `@frontend/src/pages/CompetitionArchiveOrder.pr15.test.tsx`:
- Around line 20-33: Replace direct window.location.pathname reads in
LeaguesPage and TournamentsPage with pathname from React Router’s useLocation(),
so route checks follow MemoryRouter navigation. Then remove the
window.history.replaceState call from renderPage and reset the browser history
in afterEach to avoid test-order dependence.

In `@frontend/src/pages/LeagueRecap.tsx`:
- Around line 29-36: Update recapNarrative to use a shared singular/plural
helper for wins, losses, ties, and games, handling “loss” explicitly so singular
output is grammatically correct. Return no narrative when data.stats.totalGames
=== 0, matching the empty-state behavior already handled by the page.
- Around line 201-207: Update the downloadPng state flow and the button label:
use downloading to display an in-progress state while the fetch and download are
active, add a separate downloaded state set only after a.click() succeeds, and
expose the catch failure instead of swallowing it so failed downloads are
reported rather than shown as successful.

In `@frontend/src/pages/Leagues.tsx`:
- Around line 344-369: The archive/restore confirmation UI and mutation wiring
are duplicated between the league and tournament pages. In
frontend/src/features/competition/CompetitionUI.tsx, add a shared component
accepting the resource noun, archived flag, pending/error states, and
confirm/cancel handlers; update frontend/src/pages/Leagues.tsx lines 344-369 to
use it with noun="league" and archived={league.active === 0}, and
frontend/src/pages/Tournaments.tsx lines 452-477 to use it with
noun="tournament" and archived={tournament.active === 0}; also consolidate the
corresponding mutation wiring near Leagues.tsx line 262 and Tournaments.tsx line
259 while preserving each page’s URL segment.
- Around line 93-98: Update the useQuery flow in frontend/src/pages/Leagues.tsx
lines 93-98 to reject non-OK fetch responses before parsing JSON, expose
isError, and render an error state alongside the existing loading state; apply
the same r.ok handling and error-state rendering to the ['tournaments'] query in
frontend/src/pages/Tournaments.tsx lines 114-117.

In `@frontend/src/pages/PerfectGames.tsx`:
- Around line 26-31: Update PerfectGameCard so nullable game.frameData is
handled explicitly: require a non-null frameData before parsing and rendering
the scoreboard, or render a clear empty/null state when it is absent. Ensure the
card does not silently render an incomplete scoreboard through marks.map(...).

In `@frontend/src/pages/PerfectGameShare.tsx`:
- Around line 76-80: Catch rejected copyText calls in both handlers: update
handleCopyLink in frontend/src/pages/PerfectGameShare.tsx lines 76-80 and
copyLink in frontend/src/pages/ShareScore.tsx lines 78-82 with try/catch, and
set copied only after a successful await inside try. Ensure both button callers
no longer produce unhandled promise rejections; optional failure feedback is not
required.
- Around line 101-113: Update the fetch effect in PerfectGameShare so every
non-OK response sets a visible failure state: preserve the notFound state for
404 responses and set the existing failure state for other statuses, including
500 and 503. Keep the loading transition intact and retain the return null guard
only as an unreachable fallback after data and failure states are handled.

In `@frontend/src/pages/SessionShare.tsx`:
- Around line 146-150: Replace the inline flex styling on the action-row div in
SessionShare with the shared public-share-actions class, preserving the existing
buttons and links unchanged so spacing, wrapping, and responsive behavior come
from the shared stylesheet.

In `@frontend/src/pages/Tournaments.tsx`:
- Around line 121-128: Guard the date comparators in the upcomingTournaments,
pastTournaments, and archivedTournaments sorts against null or missing dates so
rendering never calls localeCompare on an invalid value. Update the Tournament
interface date type to string | null and adjust each comparator to use a safe
fallback while preserving the existing sort ordering for valid dates.

In `@frontend/src/pages/TournamentShare.tsx`:
- Line 194: Update the net value formatting in the stats construction to use
Intl.NumberFormat with currency formatting instead of hardcoding the dollar sign
and interpolating the raw number; preserve the existing positive/negative net
behavior while producing locale-aware grouping and two decimal places.

In `@frontend/src/pages/TournamentStandings.tsx`:
- Around line 91-94: Update the date formatting in TournamentStandings to pass
undefined instead of the hardcoded 'en-US' locale to toLocaleDateString,
matching TournamentShare’s viewer-locale behavior. Keep the existing formatting
options and the 'T00:00:00' local-time suffix unchanged.
- Around line 98-100: Guard the image URL passed to usePublicMetadata in
TournamentStandings by passing an empty string when invalidId is true, while
retaining ogImageUrl for valid tournament IDs. Keep the hook call in its current
position so hook order remains consistent, and preserve the existing not-found
rendering flow.

In `@frontend/src/pages/TournamentStandingsShare.tsx`:
- Around line 161-165: Update the render conditions for both the PublicResult
block and the standings table in the TournamentStandingsShare component to
depend on standings בלבד, removing the unnecessary tournamentData requirement.
Preserve the existing fallback values and error handling so successful standings
data still renders primary results when the metadata query fails.

---

Outside diff comments:
In `@frontend/src/pages/LeagueRecap.tsx`:
- Around line 67-75: Update the useEffect tied to leagueId and invalidId to
reset loading to true and clear error at the start of each valid fetch. Preserve
the existing cancellation and fetch handling so route changes show a loading
state, discard stale errors, and render only the newly fetched recap.

In `@frontend/src/pages/LeagueRecapShare.tsx`:
- Around line 110-122: Separate the in-flight and successful download states in
the downloadPng flow and button rendering in LeagueRecapShare. Keep downloading
true only while the request is active, introduce or reuse a success state that
is set only after the download completes successfully and reset on failure, and
render “Downloading…” while active and “Downloaded” only after success, matching
ShareScore’s vocabulary.

In `@frontend/src/pages/Leagues.tsx`:
- Around line 443-455: Add accessibility state to the week toggle button using
the existing expanded value: set aria-expanded and add aria-controls referencing
a unique panel identifier for that week. Assign the matching id to the revealed
panel around the week content at line 507, and remove aria-hidden from the
visible Expand/Collapse state text so assistive technology receives the state
cue.

In `@frontend/src/pages/LeagueWeekShare.tsx`:
- Around line 91-99: Update the error-state copy in the LeagueWeekShare
component’s isError branch to be neutral and factual for unauthenticated
visitors: remove the instruction to log a week and avoid claiming that no league
weeks exist, since the branch also covers network and server failures. Preserve
the existing home-page Link destination and surrounding error-state layout.

In `@frontend/src/pages/PerfectGames.tsx`:
- Around line 163-190: Update the action-row layout in PerfectGames so the four
.btn controls wrap or stack at 320px, preventing overflow from the fixed card
padding, button padding, and gaps; preserve the existing desktop arrangement and
ensure labels such as “Download image” remain usable.

In `@frontend/src/pages/Tournaments.tsx`:
- Around line 392-444: Remove the unsupported menu semantics from the sharing
controls: delete role="menu" from the share-menu container, remove
role="menuitem" from each button invoking handleShare, handleCopyLink,
handleDownloadCard, and handleXShare, and change the trigger’s aria-haspopup
value from "menu" to "true".

In `@frontend/src/pages/TournamentStandings.tsx`:
- Around line 171-188: Update the public-theme controls across
frontend/src/pages/TournamentStandings.tsx lines 171-188 and 191-192,
frontend/src/pages/PublicLeagueLeaderboard.tsx lines 123-124, and
frontend/src/pages/TournamentStandingsShare.tsx lines 185-202: remove the inline
foreground and border colors from the TournamentStandings share/download
controls so btn-ghost owns both, add an explicit light foreground to each dark
loading card, and remove the TournamentStandingsShare share anchor’s inline
style so btn-ghost applies.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/package.json`:
- Around line 5-11: Update the package.json engines.node constraint to require
Node 20.6.0 or newer, matching the minimum version that supports --import, and
change the test script to pass the src directory instead of the shell-expanded
src/*.test.mjs pattern. Preserve the existing test runner and import
configuration.

In `@backend/src/competitionArchive.pr15.test.mjs`:
- Around line 68-73: Update the CSV assertions in the export test to parse the
header, locate the active column by name, and compare each tournament row’s
value at that column instead of matching with cross-row regular expressions.
Preserve the existing status, content-type, and active/archived expectations,
using row splitting for the current comma-free fixture values.
- Around line 20-43: Extend the parameterized test in “league and tournament
archive routes are reversible and validate ids” with a successful DELETE request
for the valid resource ID. Assert the response succeeds, then verify the record
remains available through the archived-inclusive GET and has active equal to 0,
ensuring DELETE archives rather than removes it.

In `@backend/src/publicMetadata.pr15.test.mjs`:
- Around line 88-105: Extend the cases table in the test loop to cover the
remaining resolvePublicPageMetadata routes, prioritizing
/leagues/:id/recap/share with its null-week title path and adding /bowl,
/score/:id, /sessions/:id/share, /perfect-games/:id, and
/leagues/:id/week/:weekId/share. Replace the title and og:url RegExp assertions
with escaped-safe plain string containment checks, while preserving descriptive
failure messages and the attacker-host assertion.
- Around line 10-22: Replace the source-slicing, transpilation, and VM
evaluation in loadHtmlInjector with the real exported injectPublicMetadataHtml
obtained from the existing buildTestApp module harness. Update buildTestApp to
expose the injector, make the two pure-function tests async as needed, and
preserve their current assertions while exercising the shipped implementation
directly.

In `@backend/src/server.ts`:
- Around line 2434-2439: Confirm the frontend’s delete URL, then update the
DELETE handlers in backend/src/server.ts at lines 2434-2439 and 3296-3301
consistently: register the same league handler for /api/leagues/:id and
tournament handler for /api/tournaments/:id when the frontend uses the prefixed
paths; otherwise remove the unused bare routes. Preserve the existing
soft-archive behavior and validation.
- Around line 1376-1386: Update backend/src/server.ts lines 1376-1386 and
2620-2630: refactor selectLeagueRows and selectTournamentRows to use SQL
builders with two module-level prepared statements per helper, one including
archived records and one filtering them, then select the appropriate cached
statement based on the boolean argument instead of calling sqlite.prepare per
request.
- Around line 5441-5443: No code change is required; retain the boot-time
CACHED_INDEX_HTML caching and ensure deployment builds the frontend before
starting or restarting the server, with server restarts on frontend-only
redeploys so asset hashes refresh.
- Around line 5250-5257: Add a brief comment immediately above escapeHtml
documenting that it is intended only for text in <title> and values in
double-quoted attributes used by injectPublicMetadataHtml, and must not be
reused for unquoted attributes, URL attributes, inline scripts, or styles.
- Around line 5288-5313: Guard the final replacement in injectPublicMetadataHtml
so missing </head> markup still preserves the generated metadata. If
withoutManagedMeta.replace does not find a closing head tag, append the
generated tags instead of returning the title-stripped HTML; retain the existing
replacement behavior when </head> is present.

In `@backend/src/test-app.mjs`:
- Around line 40-48: Add a concise comment immediately before the process.env
assignments in buildTestApp documenting that the harness relies on calls
executing sequentially because environment variables are process-global and the
unique server.ts import re-reads them. Do not change the existing setup or
import behavior.
- Around line 50-63: Update the teardown callback around fastify.close,
sqlite.close, environment restoration, and rm so each cleanup step is isolated
with error handling and later steps still execute when an earlier step fails.
Preserve the existing order—close Fastify, close SQLite, restore all environment
variables, then remove root—and ensure cleanup failures remain associated with
teardown without preventing subsequent cleanup.

In `@frontend/src/features/competition/CompetitionUI.tsx`:
- Around line 75-118: Reset the archive mutation whenever
CompetitionArchiveSheet mounts so reopening it does not display a previous
error; accept a reset function in the component’s mutation prop and invoke it on
mount. Update useCompetitionArchive call sites in Leagues.tsx and
Tournaments.tsx to pass through the hook’s reset method alongside the existing
mutation state and mutate function.

In `@frontend/src/pages/LeagueRecapShare.tsx`:
- Around line 89-93: Update the not-found body links rendered by the
invalidId/not-found branches so they use the purpose-specific accessible name
“Browse leagues on BowlSense” instead of duplicating the PublicShell brand link:
apply this in frontend/src/pages/LeagueRecapShare.tsx lines 89-93,
frontend/src/pages/LeagueShare.tsx lines 90-94,
frontend/src/pages/LeagueWeekShare.tsx lines 77-81,
frontend/src/pages/SessionShare.tsx lines 122-126,
frontend/src/pages/ShareScore.tsx lines 123-127, and
frontend/src/pages/TournamentShare.tsx lines 169-173.

In `@frontend/src/pages/LeagueWeekShare.tsx`:
- Around line 91-99: The light PublicShell leaves dark-theme inline colors in
LeagueWeekShare. In frontend/src/pages/LeagueWeekShare.tsx lines 91-99, replace
the error-state white and red values with the shared muted and alert treatments;
in lines 119-124, replace the game-number white value, and update the scoreColor
default band near line 20 so the chip uses light-surface-appropriate background
and text colors.

In `@frontend/src/pages/PerfectGames.tsx`:
- Line 31: Update the marks guard in the PerfectGames rendering flow to treat
both null frame data and an empty result from parseFrameMarks as unavailable.
Use the parsed array’s length when deciding whether to render frame details, so
unparsable data displays the existing “Frame details were not recorded” fallback
instead of an empty grid.
- Around line 35-43: Update the copyGameShareLink helper in gameShare.ts to use
the shared copyText utility from the scoring feature instead of calling
navigator.clipboard.writeText directly, preserving its existing return behavior
so handleCopyLink in PerfectGames.tsx continues to work.

In `@frontend/src/pages/ShareScore.tsx`:
- Line 147: Replace the inline flex styling on the action-row div in ShareScore
with the shared public-share-actions class, matching the wrapper used by
SessionShare, LeagueShare, and TournamentShare; preserve the existing child
actions and rely on the shared stylesheet for spacing and wrapping.
- Around line 78-86: Extract the duplicated copyLink logic from ShareScore and
the other shared-page components into a reusable useCopyLink hook. Have the hook
own copied state, call copyText with window.location.href, apply one consistent
reset delay, handle failures consistently, and clean up its timer; update each
page to use the hook instead of defining its own handler and state.
- Around line 163-169: Resolve the duplicate copy-link controls in ShareScore by
choosing one owner for copy behavior: either remove the secondary button using
copyLink and copied, or change the shell action around the existing action
button to use the native share path while keeping copyLink in this button.
Ensure only one visible control provides copy confirmation for a single action.

In `@frontend/src/pages/Tournaments.tsx`:
- Around line 96-112: Reorder the route checks in the tournament page so the
explicit `/tournaments/new` branch is evaluated before the `if (id)` detail
branch. Preserve the existing create-sheet and detail rendering behavior,
ensuring the `"new"` path opens TournamentForm instead of TournamentDetail.
- Around line 506-515: Update the BowlingScorer edit flow in the onSave block to
pass the existing game state through g.frameData and g.splits, matching the
BowlingScorer usage in SessionDetail.tsx, so editing restores the saved game
before changes are applied.

In `@frontend/src/pages/TournamentShare.tsx`:
- Around line 207-211: Update the games list in TournamentShare around the
data.games.map/GameCard rendering to use the shared public-detail-row structure
and public theme tokens instead of the dark GameCard presentation. Refactor
GameCard’s rendered surface, text, borders, and background to match the light
public-detail-row styling used by LeagueShare, while preserving each game’s
existing content and key.
- Line 204: Update handleDownload in TournamentShare to track an in-flight state
and downloadError, reset the outcome before starting, validate the download
response via res.ok, and catch rejected requests instead of allowing unhandled
errors. Set downloaded only after a successful response, disable the button
while downloading, and render the failure state consistently with the existing
LeagueRecapShare pattern.

---

Outside diff comments:
In `@frontend/src/pages/LeagueRecap.tsx`:
- Around line 214-228: Add aria-live="polite" to both buttons rendered in the
copyLink and downloadPng controls so screen readers announce their changing
labels, including copied, preparing, downloaded, and failure states.

In `@frontend/src/pages/Leagues.tsx`:
- Around line 346-383: The edit Save button in frontend/src/pages/Leagues.tsx
lines 346-383 must disable when leagueForm.name is blank after trimming or
updateLeague.isPending; add the equivalent guard to the week-edit Save button at
lines 464-481 using weekForm.date and updateWeek.isPending, matching the
existing LeagueCreate and LogWeekForm patterns.