Skip to content

Follow-ups from issue #166: CI gate, dead listener, migration checklist, UX dead-end - #183

Merged
hturnbull93 merged 4 commits into
mainfrom
fix/issue-166-auth-expired-and-verify-cta
Jul 8, 2026
Merged

Follow-ups from issue #166: CI gate, dead listener, migration checklist, UX dead-end#183
hturnbull93 merged 4 commits into
mainfrom
fix/issue-166-auth-expired-and-verify-cta

Conversation

@hturnbull93

@hturnbull93 hturnbull93 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses items from #166 (checked each against current main before acting, since the issue is from May 22 and some code has moved since):

1. No typecheck/lint/test gate before merge
Confirmed still true — .github/workflows/ didn't exist at all, and branch protection wasn't set on main. Added .github/workflows/ci.yml with 4 parallel jobs (typecheck, lint, format, test) instead of one check-all step, so a CI failure points straight at the failing category instead of a single opaque job.

2. auth:expired listener with no dispatcher
Confirmed still true — lib/auth-context.tsx listened for the event, nothing ever fired it. Added an interceptor in lib/client.ts's oRPC link that dispatches auth:expired on any UNAUTHORIZED response, so an expired/invalid token now cleanly logs the user out instead of failing silently on every subsequent request.

3. Secrets baked into Nixpacks build via ARG/ENV
Confirmed still current — pulled today's Railway build log, same 4 vars flagged (B2_APP_KEY, B2_KEY_ID, CRON_SECRET, RESEND_API_KEY). Drafted a hand-rolled Dockerfile using BuildKit secret mounts, but decided against merging it: fixing this means dropping Nixpacks entirely (Railway defaults to it whenever no Dockerfile is present), which is an infra-level builder migration with real deploy risk, for a latent (not currently exploitable) risk. Left as a known, documented, low-priority item — no code change here.

4. Migration checklist: NOT NULL adds need a backfill plan
Audited all migrations since baseline — no other instances of the same gap (the email_confirmed migration was the only one). Added the rule to AGENTS.md's migration workflow so it's checked going forward.

5. Unconfirmed users hit a dead-end error
Confirmed still true — app/page.tsx's projects-error branch showed the raw message with no way to act on it. Added a link to /verify-email when the error is the email-confirmation FORBIDDEN.

6. Dependency hygiene
No action — informational only. Re-ran npm audit: down to 5 moderate (uuid transitive via svix→resend), Prisma still on 6.19.3 caret-pinned. Flagged for the next routine dep refresh, not this PR.

Test plan

  • npm run check-all passes locally (typecheck, lint, format, 133 e2e tests)
  • CI workflow runs green on this PR (first real run)
  • Manually verify: expired token triggers logout instead of silent failures
  • Manually verify: unconfirmed-user project-list error shows the verify-email link

Closes #166

🤖 Generated with Claude Code

https://claude.ai/code/session_01JGuCPeiGmh6LVJARhJkiu6

No CI gate existed at all — broken code could reach main undetected
(root cause of issue #166 item 1). Split into 4 parallel jobs instead
of one check-all step so failures show which category broke.
lib/client.ts listened for a auth:expired window event to clear the
session but nothing ever dispatched it — a server-side token expiry
mid-session produced opaque errors on every subsequent request instead
of a clean logout. Interceptor now dispatches it on UNAUTHORIZED.

app/page.tsx showed the raw "confirm your email" 403 message with no
way to act on it. Added a link to /verify-email when the projects
query fails with FORBIDDEN.

Both surfaced during diagnosis in issue #166.
Root cause of the email_confirmed incident (issue #166) was a NOT
NULL column add relying on the column default during SQLite's
table-rebuild, silently applying it to every existing row instead of
backfilling. Codifying the check so it isn't missed again.
@railway-app
railway-app Bot temporarily deployed to PauseAI Catalyse / catalyse-pr-183 July 8, 2026 08:15 Destroyed
Three separate jobs for sub-second checks each paid full
checkout+setup-node+npm-ci overhead. Combined into one sequential
job (fails fast at first broken step); test stays isolated since
it's the actually expensive one (playwright install + ~3min run).
@railway-app
railway-app Bot temporarily deployed to PauseAI Catalyse / catalyse-pr-183 July 8, 2026 08:18 Destroyed
@hturnbull93
hturnbull93 merged commit 18815ec into main Jul 8, 2026
3 checks passed
@railway-app
railway-app Bot temporarily deployed to PauseAI Catalyse / catalyse-pr-184 July 25, 2026 08:37 Destroyed
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.

Follow-ups from "non-admin users locked out of projects" incident

1 participant