Review: #488 Set expires_at=30d on all 6 Stripe Session create calls

Verdict: NEEDS_REFINEMENT

Template Completeness

• [x] Type — "Bug"
• [x] Lineage — references #486, discovery narrative present
• [x] Repo — forgejo_admin/basketball-api
• [x] What Broke — Stripe API evidence + stranded revenue quantified
• [x] Repro Steps — both inline code path and Stripe API retrieve path
• [x] Expected Behavior — 30-day TTL, shared constant suggestion, runbook correction requirement
• [x] Environment — commit pin, namespace, affected data counts
• [x] Acceptance Criteria — 7 items, each verifiable
• [x] Related — #486 (recovery), #487 (metric), pal-e-platform#295 (alert)

Traceability

• [x] story:payment-reliability label — present on board item
• [ ] story note MISSING — project-westside-basketball user-stories uses WS-S{N} convention (WS-S1 through WS-S33). The label payment-reliability is a bare slug that doesn't match the project's story taxonomy. The closest existing story is WS-S11 ("As an admin, I want to create Stripe checkout sessions for jerseys/fees so that payments are automated"). Note: siblings #486 (story:payment-recovery) and #487 (story:observability) share the same pattern — the whole payment-pipeline story cluster is off-taxonomy.
• [x] arch:stripe-checkout label — present on board item
• [ ] arch note MISSING — search_notes(query="arch-stripe-checkout") returned zero results. No backing architecture note exists in pal-e-docs.
• [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/forgejo_admin/basketball-api/issues/488 is open and contains the full spec.

File Targets

• [x] src/basketball_api/services/tournament_checkout.py:78 — verified, stripe.checkout.Session.create( at line 78
• [x] src/basketball_api/routes/jersey.py:291 — verified, call is at line 291
• [ ] src/basketball_api/routes/checkout.py:246 — ISSUE: line 246 is the comment # Create Stripe Checkout Session; the actual call is at line 247. Off by one.
• [ ] src/basketball_api/routes/checkout.py:398 — ISSUE: line 398 is order = Order(...); the actual call is at line 410. Off by 12 lines.
• [ ] src/basketball_api/routes/register.py:1373 — ISSUE: line 1373 is amount_cents = 3000 if reg_type == "tryout" else 4000; the actual call is at line 1378. Off by 5 lines.
• [ ] src/basketball_api/routes/register.py:1418 — ISSUE: line 1418 is amount_cents = 3000 if reg_type == "tryout" else 4000; the actual call is at line 1423. Off by 5 lines.
• [ ] MISSING CALL SITE: src/basketball_api/routes/checkout.py:553 — confirmed stripe.checkout.Session.create( call. Appears to be a helper path that builds the shared order_id/player_id/product_id metadata and then mints a session. Not referenced in the ticket.
• [ ] MISSING CALL SITE: src/basketball_api/routes/admin.py:2005 — confirmed stripe.checkout.Session.create( call inside an admin "regenerate session" handler (preceded by # Create a fresh Stripe Checkout Session). Not referenced in the ticket.

• The ticket title says "all 6," the first acceptance criterion says "All 6 call sites," and the regression test AC says "parametrizes across all 6 routes/helpers." If a Dev agent trusts the count, admin.py:2005 and checkout.py:553 will be left unfixed and the test will pass while still shipping two 24-hour-TTL sessions.
• Bad line numbers slow Dev agents down but do not break correctness (they will locate the call sites by pattern). Bad counts do break correctness.

Repo Placement

Dependencies

• Sibling #486 (board item #1022) — recovery blast for the 18 stranded Utah Invitational orders. #486's body explicitly says the regen script re-mints sessions via the blessed helper; if #488 lands first, those regenerated sessions inherit the new 30-day TTL. If #486 ships first, parents get regenerated links that still expire in 24h and the whole recovery blast wastes one approval cycle. #488 is correctly ordered before #486. Dev agent must not start #486's regen step until #488 is merged and deployed.
• Sibling #487 (board item #1024) — expired-session metric, observability follow-up. Independent; can run in parallel.
• Sibling #489 (board item #1025) — spike comparing Payment Links vs lazy-mint vs 30d TTL. #488 is the interim fix; #489 picks the long-term pattern. Don't couple them.
• Commit fd081e0 (#480) — "require amount match when reusing fresh pending Stripe session" touched Stripe session reuse logic. Confirmed this commit is ancestor of current HEAD; no rebase needed.

Acceptance Criteria

• AC-1 "All 6 call sites" — must be updated to "All 8 call sites" (see File Targets).
• AC-2 "parametrizes across all 6 routes/helpers" — must be updated to 8.
• AC-5 Runbook update — adequately scoped. The runbook's sanctioned-call-sites table (lines 23-29) is also stale: it lists 5 non-tournament sites with the same outdated line numbers the ticket has, and omits routes/checkout.py:553 and routes/admin.py:2005. Dev agent should fix the table row list + line numbers in the same PR as the root-cause narrative fix.
• AC-7 "expires_at - created ≈ 2592000s" — math confirmed: 30 × 24 × 3600 = 2,592,000, which is Stripe's maximum for payment mode. Correct value.
• Consider adding: "Shared constant CHECKOUT_SESSION_TTL_SECONDS lives in exactly one module (suggest src/basketball_api/stripe_config.py or similar) and is imported at all 8 sites — no duplicated literal." This hardens the "one knob" invariant and makes the spike #489's cutover easier.

Blast Radius

• Regression test file: tests/test_checkout_session_ttl.py does not exist in main — safe to create. Existing tests/test_checkout.py is unrelated (generic checkout coverage) and won't collide.
• Non-tournament flow risk: 30-day TTL is benign for all 8 sites. Stripe sessions are only "active" until paid or expired; a longer TTL just keeps more expired links reachable, which is the desired behavior for blast-delivered URLs.
• Webhook handlers: No change. expires_at is a client-side mint parameter; webhook signatures and checkout.session.completed semantics are untouched.
• Runbook blast radius: The order_id-metadata narrative is wrong on the Utah incident (ticket correctly flags this), but order_id metadata is still a real correctness invariant for tournament flows — the "forbidden ad-hoc create" rule in the runbook's TL;DR should be preserved, only the "What broke" section needs the TTL rewrite + move the metadata hypothesis to a "Historical note" subsection. Ticket AC already says this explicitly.
• Other repos: Grep confirms no stripe.checkout.Session.create calls in westside-app, westside-contracts, or pal-e-app. basketball-api is the only mint point.

Decomposition Assessment

Recommendation

• [BODY] Fix the count: replace "all 6" with "all 8" in the title, the "Affected call sites (all 6)" header, AC-1 ("All 6 call sites"), and AC-2 ("parametrizes across all 6 routes/helpers").
• [BODY] Fix the file-target list to the verified lines on origin/main @ a4047d2:
• [BODY] Add explicit note in AC-5 that docs/tournament-billing-runbook.md's "sanctioned call sites" table (currently lines 23-29) must also be updated to reflect the 7 non-tournament sites with correct line numbers — not just the "What broke" narrative.
• [BODY] (Optional hardening) Add AC: "A single CHECKOUT_SESSION_TTL_SECONDS = 30 * 24 * 3600 constant lives in one module and is imported at all 8 call sites; no inline duplication of the literal." Makes the spike #489's future cutover a one-line change.
• [SCOPE] Create architecture note arch-stripe-checkout covering the 8-site mint surface, TTL invariant, webhook handler routing (order_id metadata contract), and the relationship to #489's pattern evaluation. This is also missing for siblings #486 (arch:stripe-checkout) and #487 (arch:payment-pipeline) — one note likely covers all three.
• [SCOPE] Reconcile story taxonomy: either add WS-S34 (or similar) to project-westside-basketball user-stories section with a payment-reliability narrative, OR retag #488/#486/#489 with an existing WS-S* story (WS-S11 is the nearest fit). Same reconciliation applies to #487's story:observability. This is a project-wide convention drift, not a #488-specific problem, but #488 inherits it.