Review: App Store webhook infra

review-1610-2026-06-27 Review

review ready

Verdict: READY

Re-review round 2 (2026-06-27). All three findings from round 1 have been resolved. Scope is solid, all file targets verified, traceability complete, fits in a single agent pass.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, Sprint 6 planning
  • [x] Repo — ldraney/westside-basketball
  • [x] User Story — WS-S34, well-formed
  • [x] Context — clear explanation of Apple's auto-hold behavior, Fastlane IaC separation, and iOS-only subscription decision (2026-06-27)
  • [x] File Targets — 4 modify/create targets, 3 exclusions (all verified)
  • [x] Feature Flag — none (infrastructure, appropriate)
  • [x] Acceptance Criteria — 6 criteria, all code-deliverable
  • [x] Test Expectations — 2 integration tests with run command
  • [x] Constraints — 6 constraints listed
  • [x] Checklist — standard 3 items
  • [x] Related — story, project, sibling tickets, previous review note referenced

Traceability

  • [x] story:WS-S34 label — "As a parent, I want to subscribe to my child's team program through the iOS app so that Apple handles monthly billing and automatically enforces the delinquency gate if I fall behind on payments"
  • [x] story note verified — found in project-westside-basketball user-stories section under Parent
  • [x] arch:app-store label — App Store billing component
  • [x] arch note verified — arch-app-store note exists in pal-e-docs (created 2026-06-28, covers webhook flow, data model, dependency chain, iOS-only decision)
  • [x] Forgejo issue — ldraney/westside-basketball#65, state: open

File Targets

  • [x] Gemfile — verified: exists (2.3k), no app_store_server_library gem present yet. Ready for addition.
  • [x] config/routes.rb — verified: exists, line 50 has post "webhooks/stripe" => "webhooks#stripe". Appropriate place to add App Store webhook route alongside existing Stripe route.
  • [x] app/controllers/webhooks_controller.rb — verified: exists (4.1k), inherits from ActionController::Base, has skip_forgery_protection only: :stripe. Existing Stripe webhook implementation present (lines 7-118). Pattern matches ticket description.
  • [x] config/credentials.yml.enc — verified: exists (548 bytes).
  • [x] No webhooks/ subdirectory — verified: only admin/ and concerns/ exist under app/controllers/.

Repo Placement

OK. Issue is filed on ldraney/westside-basketball and all file targets are in the same repo. Fastlane IaC and iOS code correctly scoped out to the iOS repo. Single-repo ticket.

Dependencies

  • depends:58 label removed (was inverted — round 1 finding, now fixed).
  • Board item #1611 (westside-basketball#66, Subscription management UI, 5pts) has depends:58,depends:65 — correctly depends on this ticket for webhook data.
  • Board item #1635 (westside-basketball#78, iOS StoreKit purchase flow, 5pts, backlog) has depends:65 — correctly depends on this ticket (needs webhook endpoint before iOS can send notifications).
  • No blocking dependencies on this ticket — it can be implemented immediately.

Acceptance Criteria

6 ACs, all code-deliverable and agent-verifiable:
  • AC1 (gem in Gemfile) — trivially verifiable via grep
  • AC2 (route added) — verifiable via grep on routes.rb
  • AC3 (webhook action acknowledges) — verifiable via integration test
  • AC4 (JWS verification) — verifiable via integration test with invalid signature
  • AC5 (credentials stored) — verifiable via rails credentials:show
  • AC6 (observability logging) — verifiable via test log output inspection. NEW since round 1 — addresses the missing payload handling finding.
All criteria are complete and testable. No missing AC.

Blast Radius

  • No existing App Store or Apple code in the codebase (only CSS font-family and meta tags). This is greenfield — no risk of conflicting with existing patterns.
  • Existing WebhooksController uses ActionController::Base + skip_forgery_protection per-action. Adding a new Apple action follows the same pattern — low risk.
  • Credentials access: the app currently uses file-based credentials for Gmail (config/gmail/credentials.json), not Rails.application.credentials. The ticket specifies credentials.yml.enc for App Store secrets — correct Rails convention but introduces a new credential access pattern. Not a blocker, just awareness for the implementing agent.
  • Constraint "Do NOT delete or modify existing Stripe webhook" is explicitly stated — prevents accidental breakage of one-off payments.

Decomposition Assessment

4 file targets in 1 repo, 6 ACs, estimated agent work under 5 minutes. No decomposition needed.

Recommendation

No action needed. All round 1 findings have been resolved:
  • [SCOPE] Create architecture note arch-app-store — DONE. Note exists with full component breakdown, data model, dependency chain, and iOS-only decision.
  • [LABEL] Remove depends:58 — DONE. Board item #1610 labels no longer include depends:58.
  • [BODY] Add AC for observability logging — DONE. AC6 added: "Webhook logs the decoded notification type and subtype for observability."

Round 1 Findings — All Resolved

  • [SCOPE] arch-app-store note missing — FIXED. arch-app-store note created in pal-e-docs.
  • [LABEL] Remove depends:58 (inverted dependency) — FIXED. Label removed from board item.
  • [BODY] Add AC for payload handling after verification — FIXED. AC6 now covers observability logging.