Skip to content

review follow-up: monorepo merge findings (PR #41) #43

Description

@InstaZDLL

Tracking the post-merge review findings from PR #41. Triage below; we'll cherry-pick and ship in batches.

P0 — broken by the waveflow-web archive

These reference URLs that will 404 once InstaZDLL/waveflow-web is archived + replaced by a README pointer. Ship them in the same PR as the archive.

  • web/package.jsonrepository.url + bugs still point at waveflow-web. Change both to https://github.com/InstaZDLL/waveflow-server (or the /web subpath for bugs if GitHub honours it — verify).
  • .github/ISSUE_TEMPLATE/config.yml — contact_links still target waveflow-web/.... Update Security + Q&A + Ideas + Show-and-tell to the monorepo URLs.
  • web/public/manifest.json"short_name": "TanStack App", "name": "Create TanStack App Sample". Replace with real WaveFlow branding.
  • web/README.md line 46 — references the obsolete src/routes/profiles.tsx (file no longer exists; the actual route is _authed.profiles.tsx).

P1 — security / correctness

  • .github/workflows/ci-web.yml — add persist-credentials: false to the actions/checkout step (matches the rest of the repo's checkout posture).
  • .github/workflows/label-pr.yml — add a top-level concurrency block (key on PR number) and drop the workflow-level permissions: write; grant pull-requests: write only to the jobs that need it.
  • web/src/server-fns/session.ts — wrap auth.api.getSession in try/catch, return null on UNAUTHORIZED, rethrow others. Preserves the existing early-return shape.
  • web/src/routes/_authed.profiles.$profileId.libraries.$libraryId.artists.tsx — loader's catch returns err.message to the UI. Replace with a generic message; log the raw err server-side. Same fix on the playlists loader if shaped identically.
  • web/src/lib/db.tsNumber.parseInt(BETTER_AUTH_DB_MAX) accepts "10foo". Tighten to /^\d+$/ or Number.isInteger + > 0.
  • web/src/components/ThemeToggle.tsx — bare localStorage.getItem/setItem('theme') will throw in private browsing / quota-exhausted contexts. Wrap reads + writes in try/catch.
  • web/.gitignore — broaden the env exclusion (.env.production / .env.staging slip through today). Add *.env* with an explicit !.env.example negation.

P2 — UX / a11y / quality

  • web/src/components/PlayerBar.tsxonPointerCancel clears seekScrub without committing. Mirror onPointerUp: if seekScrub !== null, call player.seek(seekScrub) before clearing.
  • web/src/lib/use-focus-trap.ts — when items.length === 0, container.focus() silently fails on a container without tabindex. Add a temporary tabindex="-1", focus, then remove.
  • web/src/routes/_authed.profiles.tsxnew Date(p.last_used_at).toLocaleDateString() is non-deterministic SSR ↔ client. Pre-format in the loader with an explicit Intl.DateTimeFormat(locale, { timeZone }).
  • web/src/components/Footer.tsx — display string says "AGPL-3.0", licence file is "AGPL-3.0-only". Update string.
  • web/src/styles.css@theme block redefines surfaces locally in rgba; consume the OKLCH tokens from @waveflow/design-tokens instead so a preset swap re-tints both halves identically.

P3 — tooling / tests

  • web/scripts/db-migrate.tsMIGRATIONS_DIR = join(process.cwd(), 'db', 'migrations') breaks when invoked from the repo root after the monorepo merge. Resolve via import.meta.url so the path is module-relative.
  • web/vitest.config.tspassWithNoTests: true hides discovery failures. Set to false (CI will fail loud if globs drift).
  • web/src/routes/sign-up.test.tsx — add coverage for empty display-name rejection + max-password-length boundary + name/email trim on success.
  • web/src/routes/_authed.profiles.$profileId.playlists.test.tsxReact.PropsWithChildren referenced without an import * as React / import { PropsWithChildren }. Fix import.
  • web/src/routes/sign-in.test.ts — rename to -sign-in.test.tsx so the TanStack Router plugin's - prefix convention excludes it from the route tree (analogous fix already applied to other route-adjacent test files).
  • .github/pull_request_template.md — MD041: first heading should be H1, not H2. Replace ## Summary with # Summary.

P4 — labeler accuracy

  • .github/labeler.ymlweb/src/routes/sign-in/** / web/src/routes/sign-up/** only match dir paths, but the actual routes are sign-in.tsx / sign-up.tsx (files at root of routes/). Switch to web/src/routes/sign-in{,/**} / web/src/routes/sign-up{,/**}.

How we'll ship this

  • P0 ships with the archive PR — fixes URLs that are about to 404 anyway, no point splitting.
  • P1-P3 batched per area — security in one PR, UX in another, tooling in a third. Each ≤ 200 LOC so review stays bounded.
  • P4 rides P0 (single-line labeler tweak, no point standalone).

Closes #(this issue) after the last batch lands.

Provenance

CodeRabbit + automated review on PR #41, captured 2026-06-12.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions