pal-e-docs

Review Pass 2: #497 Recover stranded monthly-fee parents (analog of #486)

review-1033-2026-04-17-pass-2 Review

review approved
Project

Verdict: APPROVED

Pass-2 review of board item #1033 (Forgejo basketball-api#497). All six pass-1 refinements are landed correctly. The single-ticket decision is defensible given the audit data now cited in the body (4 real parents / $455 blast radius). Ready to move backlog → todo.

Pass-1 Refinement Verification

  • [x] (1) HARD STOP banner reproduced verbatim — #497 lines 4–16 match #486 lines 4–16 word-for-word. All 6 numbered gates present, "Approved draft ≠ approved to send. One approval = one send." paragraph present, feedback refs (feedback_email_blast_nuclear_gate, feedback_no_email_without_five_approvals, feedback_one_approval_one_send) present, 2026-04-04 + 2026-04-13 incident refs present, Test recipients paragraph present. The only delta is Gate 5's parenthetical count ("recipient count (17)" in #486 vs. "recipient count" in #497) — correct, since the count is finalized during Phase A.
  • [x] (2) Hard constraint against email drafting in Phase A — Present as an explicit Constraint bullet (line 111): "Phase A produces NO email drafts. Drafting begins only in Phase B after Lucas approves the recipient list." Reinforced in the Phase A AC heading (line 68: "NO EMAIL WORK IN THIS PHASE"). Satisfies pass-1 ask — it's a hard Constraint, not implicit AC.
  • [x] (3) Phase A AC per-parent classification structure — Line 71 specifies full tabular deliverable: parent_id, parent_name, player_name, email, latest_order_id, amount_cents, classification (TRUE_STRANDED / PAID_VIA_RETRY / NORMAL_PENDING / WITHDRAWN / TEST_ACCOUNT), rationale. Enum is explicit and per-parent. Satisfies pass-1 ask.
  • [x] (4) Phase C hard scope cap — Phase C shrunk to 3 AC with an explicit "pick one; do not build all three" on output surface (line 100), bounded to one metric class (line 99: status=pending AND session.status=expired). Reinforced by Constraint line 113: "Phase C MUST NOT expand into a generic analytics/reporting feature. Single output channel, single metric class." Satisfies pass-1 ask — the previous "at minimum" escape hatch is gone.
  • [x] (5) Six approval gates as separate checkbox lines — Phase B approval block (lines 78–85) contains 8 individual checkboxes: template draft, Gate 1 (Lucas approves), Gate 2 (Marcus approves), test email sent, Gate 3 (Lucas confirms test), Gate 4 (Lucas says BLAST), Gate 5 (Ava reads back), Gate 6 (Lucas confirms BLAST #2). The six gates are bolded and numbered verbatim, matching #486 lines 54–61 exactly. No collapsing.
  • [x] (6) Checklist section present, covers A/B/C — Lines 119–128, 9 checkboxes: Phase A classification note, dry-run shared with Lucas, email template PR, 6 gates completed in order, mechanical recovery complete, blast sent, ≥1 payment confirmed, Phase C observability shipped (one channel / one metric), Phase D runbook update. Covers all three phases plus Phase D. Mirrors #486's Checklist shape.

Single-Ticket Decision (pass-1 recommendation 4/5 rejected)

Defensible. The Lineage block (lines 21–27) explicitly documents the rejection: "Decomposition considered and rejected: the 2026-04-17 audit (agent a1a654212264ebceb) classified the blast radius at 4 real parents / $455 (baseline of 5 orders / $460 dilated when order 79 was identified as the Westside Admin test account). At this size, one-ticket recovery is correct."
Pass-1 recommended decomposition based on:
  • Three distinct phases, three exit criteria, three risk profiles
  • Concern that coupling A + B lets a dev agent "finish" A and drift into drafting emails
  • Concern that Phase C is orthogonal to recovery
Pass-2 evaluates whether the now-landed controls defuse these concerns:
  • A→B drift risk: mitigated by the "NO EMAIL WORK IN THIS PHASE" header on Phase A AC, the hard Constraint forbidding drafts in Phase A, and the "Phase A deliverable logged as a pal-e-docs note referenced from this ticket" gate (line 75) which forces an explicit handoff checkpoint. A dev agent cannot silently cross the boundary.
  • Phase C orthogonality risk: mitigated by the scope cap ("pick one; do not build all three") and the anti-scope-creep Constraint. Phase C is explicitly marked "can ship independently" (line 98), so if it drifts, it can be spun out mid-execution without derailing the recovery.
  • Blast-radius reality: 4 parents / $455 is ~24% of #486's 17 parents / $985. At this size, decomposing into three tickets would impose more coordination overhead than it removes risk. Splitting made sense for a 17-parent blast; it doesn't at 4.
Pass-2 endorses the single-ticket shape as the right call.

Scope Creep Check

No scope creep beyond the six pass-1 items. New additions in the patched body are all evidence/data surfaces that reduce the dev agent's discovery burden:
  • Context section now contains the audit category table (TRUE_STRANDED, NORMAL_PENDING, PAID_VIA_RETRY, PAID_ORIGINAL, TEST_ACCOUNT counts and dollars).
  • Context section now contains the 4-row true-stranded parent table with order IDs, names, emails, amounts, ages.
  • Context section now flags order 95 (Artyom Litvinau) for re-audit the day of execution.
  • Constraints section enumerates specific paid-original and paid-via-retry parents not to touch, plus parent 170 (Westside Admin test).
These are data handoffs from the 2026-04-17 audit, not scope additions — they prevent the dev agent from re-running the audit.

Template Completeness

  • [x] Type (Task)
  • [x] HARD STOP banner (verbatim per #486)
  • [x] Lineage (references #486, audit agent ID, decomposition rejection rationale)
  • [x] Repo (forgejo_admin/basketball-api)
  • [x] User Story (matches story:WS-S22)
  • [x] Context (Stripe mechanism, audit tables, re-audit flag for order 95)
  • [x] Scope (three phases labeled, one-line summaries)
  • [x] Acceptance Criteria (Phase A investigation, Phase B approval gates, Phase B mechanical, Phase B execution, Phase C observability, Phase D runbook)
  • [x] Constraints (9 bullets including the hard no-drafting-in-Phase-A rule)
  • [x] Checklist (9 items spanning A/B/C/D)
  • [x] Related (links #486, #498, audit artifacts, memory notes)

Traceability

  • [x] story:WS-S22 label — "clear email communications with action links so that I never miss a deadline"
  • [x] story note verified (pass 1)
  • [x] arch:dataflow-westside-basketball label
  • [x] arch note verified (pass 1)
  • [x] Forgejo issue — basketball-api#497, open, body well-formed
  • [!] Forgejo issue currently has no labels applied at the platform level (API returns labels: []). Labels live on the board item, so this is cosmetic, not blocking — noting for Ava in case she wants label parity on Forgejo itself.

File Targets

Not applicable — Task type. Claims in body verified against live repo:
  • [x] src/basketball_api/routes/checkout.py — first_payment_checkout def at line 306, mint at line 410 (stripe.checkout.Session.create). Claim matches.
  • [x] services/email.py:202 (_brand_wrapper) — reference is consistent with feedback_two_email_systems.md. Not re-verified this pass (pass 1 did not flag this).
  • [x] No CHECKOUT_SESSION_TTL_SECONDS constant reintroduced — consistent with #493 revert context.

Repo Placement

OK — forgejo_admin/basketball-api. Recovery data access + email blast both live in that repo.

Dependencies

  • #486 (tournament recovery) — precedent pattern, already executed; template mirrored verbatim.
  • #498 (monthly Payment Link helper) — soft dependency for Phase B mechanical recovery. Fallback (30-day TTL Sessions via #486's helper) explicitly documented. Correct framing.
  • Order 95 re-audit — internal dependency on Phase A re-retrieve step. Captured as the first Phase A AC bullet.

Acceptance Criteria

Phase A (7 AC) — testable, gated on Lucas's explicit approval of recipient list. Investigation-only; no drafting. OK.
Phase B approval gates (8 AC) — each gate is a separate, bolded, numbered checkbox. Matches #486 verbatim. OK.
Phase B mechanical (4 AC) — #498-gated with documented fallback; spot-check requirement present. OK.
Phase B execution (3 AC) — only fires after all gates green AND mechanical complete. OK.
Phase C (3 AC) — one job, one output surface, one alert. Scope cap is the AC itself. OK.
Phase D (1 AC) — runbook update. OK.

Blast Radius

  • 4 real parents / $455 — confirmed by the audit table in the body.
  • 1 pending re-audit (order 95, Artyom Litvinau, $150) — captured as the first Phase A AC.
  • Test-account exclusion (parent 170, Westside Admin) — explicit Constraint.
  • Paid-original and paid-via-retry exclusion lists — named parents in Constraints, prevents noise.

Decomposition Assessment

No decomposition needed. 4-parent / $455 blast radius is below the threshold where three-ticket coordination overhead pays for itself. Controls landed in the body (hard Constraint on Phase A, scope cap on Phase C, per-phase deliverable gates) provide the same drift protection that decomposition would have enforced, without the overhead.

Recommendation

APPROVED. Move board item #1033 from backlog to todo.
No further refinements required. Optional nit (not blocking):
  • [LABEL] Consider adding type:task, story:WS-S22, arch:dataflow-westside-basketball labels on Forgejo issue #497 for parity with the board item. Cosmetic.
  • Pass 1 review: review-1033-2026-04-17
  • Precedent ticket: basketball-api#486 (tournament recovery)
  • Skill: skill-review-ticket
  • Memory: feedback_email_blast_nuclear_gate, feedback_one_approval_one_send, feedback_no_email_without_five_approvals, feedback_retrieve_before_theorize