Review: Checkout API: guest flow (gamertag + email)
Verdict: NEEDS_REFINEMENT
Third review pass (board item #813). Prior fixes (4 downstream file targets + 6 test files + PK migration strategy) confirmed present. Two additional files discovered that are impacted by the guest flow but missing from file targets.
Template Completeness
- [x] Type — Feature
- [x] Lineage — Required by #38 and #40
- [x] Repo — forgejo_admin/twitch-2k-wager
- [x] User Story — clear role/want/so-that for guest challenger
- [x] Context — explains current twitch_id PK and migration plan
- [x] File Targets — 9 source files + 6 test files listed
- [x] Acceptance Criteria — 10 testable criteria
- [x] Test Expectations — 4 expectations with CI gate
- [x] Constraints — backward compat, dedup keys, idempotent migration
- [x] Checklist — PR + tests + no unrelated changes
- [x] Related — links #38, #40, story notes, arch note
Traceability
- [x] story:challenger-pay label — Secure Buy-In Payment
- [x] story note verified — found in project-twitch-2k-wager user-stories table (key: challenger-pay, note: story-twitch-2k-wager-challenger-pay, id 1158)
- [x] arch:dataflow-twitch-2k-wager label — Data Flow: Twitch 2K Wager
- [x] arch note verified — arch-dataflow-twitch-2k-wager note exists (id 1155, status: active)
- [x] Forgejo issue — forgejo_admin/twitch-2k-wager#39, open
File Targets
Source files (9 listed — all verified):
- [x]
src/lib/schema.sql— verified: challenger.twitch_id is TEXT PRIMARY KEY (line 7), game.challenger_id is TEXT REFERENCES challenger(twitch_id) (line 21) - [x]
src/lib/types.ts— verified: Challenger interface has twitch_id: string (line 28), no numeric id field - [x]
src/lib/db.ts— verified: getChallenger takes twitchId: string (line 51), upsertChallenger ON CONFLICT (twitch_id) (line 66), createGame takes challengerId: string (line 96) - [x]
src/routes/api/checkout/+server.ts— verified: requires twitch_id in body (line 38), calls createGame(challenger.twitch_id) (line 59) - [x]
src/lib/stripe.ts— verified: createCheckoutSession metadata uses challenger_twitch_id string key (line 84) - [x]
src/routes/api/admin/result/+server.ts— verified: getChallenger(game.challenger_id) with TEXT type (line 101), updateConnectStatus uses challenger.twitch_id (line 169) - [x]
src/routes/game/[id]/+page.server.ts— verified: ownership check session.twitchId !== game.challenger_id (line 26), getChallenger(game.challenger_id) (line 32) - [x]
src/routes/api/webhooks/stripe/+server.ts— verified: reads challenger_twitch_id from session metadata (line 112), queries challenger by twitch_id in handleAccountUpdated (line 143) - [x]
src/lib/auth.ts— verified: SessionData.twitchId is required string (line 181), used throughout session encode/decode
Test files (6 listed — all verified):
- [x]
src/routes/api/checkout/__tests__/checkout.test.ts— exists - [x]
src/routes/api/admin/result/__tests__/result.test.ts— exists - [x]
src/routes/api/webhooks/stripe/__tests__/webhook.test.ts— exists - [x]
src/routes/game/[id]/__tests__/page-server.test.ts— exists - [x]
src/lib/__tests__/stripe.test.ts— exists - [x]
src/lib/__tests__/db.test.ts— exists
Missing file targets (2):
- [ ]
src/hooks.server.ts— ISSUE: Line 13 definesPROTECTED_PREFIXES = ['/game']and line 41 redirects unauthenticated users to /auth/login. Guest checkout creates games visible at /game/[id], but guests have no session. AC #8 says "Game status page handles guest games" but the hooks auth gate prevents guests from ever reaching the page.server.ts logic. This file must be updated to allow unauthenticated /game/[id] access. - [ ]
src/app.d.ts— MINOR: Defines App.Locals.session as SessionData | null. If auth.ts makes twitchId optional per the auth.ts file target, the type flows through app.d.ts correctly since session is already nullable. Low risk, but should be listed for completeness since the agent needs to verify type coherence.
Repo Placement
OK. All file targets are in forgejo_admin/twitch-2k-wager. Single-repo change. Auth callback (
src/routes/auth/callback/+server.ts) also references twitch_id but is backward-compatible — it calls the same upsertChallenger function whose signature is preserved.Dependencies
- [x] #38 (/play guest checkout route) — depends on THIS ticket, in todo column. Not a blocker.
- [x] #40 (admin queue view) — depends on THIS ticket, in todo column. Not a blocker.
- [x] #11 (Stripe Checkout + webhook) — prerequisite, in validation column. Schema and webhook handler exist. Satisfied.
- [x] #9 (Postgres schema + db.ts) — prerequisite, done. Schema exists at src/lib/schema.sql. Satisfied.
- [x] No unresolved upstream blockers.
Acceptance Criteria
10 AC are well-defined and testable by an agent. Assessment:
- AC 1-6: Straightforward API + DB assertions. Testable via unit tests and curl.
- AC 7: Admin result endpoint with INT challenger_id — testable via result.test.ts.
- AC 8: "Game status page handles guest games" — BLOCKED by hooks.server.ts auth gate. The page.server.ts ownership logic is correctly scoped, but guests are redirected before reaching it. Requires hooks.server.ts update to be verifiable.
- AC 9: Webhook handler uses INT challenger_id — testable via webhook.test.ts.
- AC 10: All 6 test files updated and passing — testable via
npm test.
4 test expectations are clear and CI-verifiable.
Blast Radius
twitch_idappears in 16 files / 46 occurrences across src/.- 15 of 16 files are covered by file targets. The 16th (auth callback) is backward-compatible.
- hooks.server.ts is the only functional gap — it gates /game/* access and will break guest game viewing.
- Rollback: schema migration adds columns and changes PK. Needs a reverse migration. Not trivial but standard.
- No other repos affected. No downstream services consume this API yet (#38 and #40 are not implemented).
Decomposition Assessment
Apply the three-thing limit and five-minute rule:
- 11 source files (9 listed + 2 missing) + 6 test files = 17 files in one repo.
- 10 AC + 4 test expectations = 14 verification points.
- All changes are in one repo and driven by a single schema migration — tightly coupled, not independently parallelizable.
- The migration must happen first, then types, then all consumers update together. Sequential dependency chain.
- Estimated agent time: 5-8 minutes. Borderline but acceptable as a single pass because the changes are mechanical (find twitch_id references, update to INT id) after the schema/type changes.
- No decomposition needed. Single agent pass is appropriate once file targets are corrected.
Recommendation
[BODY]Addsrc/hooks.server.tsto File Targets under "Downstream consumers" with note: "Update PROTECTED_PREFIXES or add guest-aware logic so unauthenticated users can view /game/[id] for guest games."[BODY]Addsrc/app.d.tsto File Targets under "Downstream consumers" with note: "Verify type coherence after SessionData.twitchId becomes optional."[BODY]Clarify AC #8: "Game status page handles guest games" should explicitly mention that the hooks.server.ts auth redirect must be relaxed for guest game pages, not just the page.server.ts ownership check.