Review: Proposal API — Bidirectional Wager Proposals

review-847-2026-04-05 Doc

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — V2 core, depends on schema ticket, part of #66
  • [x] Repo — forgejo_admin/twitch-2k-wager
  • [x] User Story — player proposes "my $X to your $Y", other player accepts/counters/declines
  • [x] Context — describes v1→v2 evolution clearly
  • [x] File Targets — 4 new files, all under src/routes/api/
  • [x] Acceptance Criteria — 7 criteria covering propose/accept/counter/decline/error/auth/atomicity
  • [x] Test Expectations — 6 scenarios with specific amounts and expected outcomes
  • [x] Constraints — transaction safety, amount bounds, auth, no UI
  • [x] Checklist — standard 4-item
  • [x] Related — references #66 parent, schema ticket, dashboard ticket

Traceability

  • [x] story:player-propose — verified in project-twitch-2k-wager user-stories table ("My $X to your $Y sent in under 5 seconds. Proposal visible to both sides immediately.")
  • [x] story note verified — found in project-twitch-2k-wager user-stories section
  • [x] story:player-decide — verified in project-twitch-2k-wager user-stories table ("Accept, counter, or decline within the stream moment. Funds lock on accept.")
  • [x] story note verified — found in project-twitch-2k-wager user-stories section
  • [x] arch:dataflow-twitch-2k-wager — references the data flow architecture component
  • [ ] arch note MISSING — [SCOPE] No note named arch-dataflow-twitch-2k-wager exists in pal-e-docs. The project page has an architecture section with a data model diagram, but no standalone arch note backs the label.
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/forgejo_admin/twitch-2k-wager/issues/69, state=open

File Targets

  • [x] src/routes/api/propose/+server.ts — NEW file. Parent directory src/routes/api/ exists with sibling routes (admin, checkout, webhooks). Correct SvelteKit server route pattern.
  • [x] src/routes/api/accept/+server.ts — NEW file. Same parent directory. Pattern consistent.
  • [x] src/routes/api/counter/+server.ts — NEW file. Same parent directory. Pattern consistent.
  • [x] src/routes/api/decline/+server.ts — NEW file. Same parent directory. Pattern consistent.
All 4 files are new API routes. Targets are specific enough for an agent to create without guessing. Each file target includes the expected request body, validation rules, and database operations.

Repo Placement

OK. Issue is filed on forgejo_admin/twitch-2k-wager, which is the correct repo (SvelteKit app + server routes). All file targets are in this repo. No cross-repo concerns — the ticket explicitly states "No UI" and "Dashboard ticket consumes these endpoints" (separate issue #70).

Dependencies

  • [ ] Issue #67 (v2 schema — player pools, proposals, peer-to-peer game model) — PENDING. HARD BLOCKER. The proposal API imports db functions (createProposal, lockFunds, unlockFunds, etc.) and depends on tables (player_pools, proposals) that do not exist yet. Current schema has only challenger/game/payment/payout/revenue_split with fixed tiers. Board item #845 tracks #67, currently in backlog.
  • [x] Issue #68 (deposit flow) — PENDING but NOT blocking. Players need deposits for pool balance, but proposal API can be built and tested with seeded pool data.
  • [x] Issue #70 (dashboard frontend) — consumes these endpoints, does not block them.
Execution order: #67 (schema) must merge before #69 (this ticket) can be implemented.

Acceptance Criteria

All 7 criteria are testable and specific:
  • [x] "Propose locks my_amount from proposer's pool, creates pending proposal" — verifiable via API call + DB state query
  • [x] "Accept locks your_amount from accepter's pool, creates game" — verifiable via API call + DB state query
  • [x] "Counter unlocks original, creates new proposal with swapped roles" — verifiable via sequential API calls + DB assertions
  • [x] "Decline unlocks proposer's funds, no game" — verifiable via API call + DB state query
  • [x] "Insufficient balance returns 400 with clear error" — verifiable via HTTP status check
  • [x] "Only the to_player can accept/counter/decline (auth check)" — verifiable via 403 response with wrong session
  • [x] "All pool operations are atomic (no partial locks on failure)" — verifiable via transaction rollback test
Test Expectations section adds 6 numeric scenarios with concrete amounts ($20/$10 with $50 balance, etc.). These are directly automatable as integration tests.

Blast Radius

  • The proposal API does NOT modify any existing routes. /api/checkout and /api/admin/result remain unchanged.
  • The 4 new routes import from src/lib/db.ts. Issue #67 (schema) adds the db functions these routes will call. No blast to existing db functions.
  • Auth pattern: existing auth.ts uses Keycloak OIDC with PKCE. The new routes need the same pattern. This is consistent with existing route auth.
  • No downstream consumers outside this repo. Rollback = delete the 4 new route directories.

Decomposition Assessment

Applying three-thing limit and five-minute rule:
  • File count: 4 new files in 1 repo — borderline on three-thing limit but all follow identical pattern (validate request → check auth → check pool balance → mutate pool → update status → return response).
  • AC count: 7 acceptance criteria — above the 5 threshold, but they test 4 variations of the same flow (propose/accept/counter/decline) plus 3 error cases.
  • Estimated time: ~5 minutes. The routes share imports, auth pattern, and pool mutation logic. A single agent can implement all 4 as a cohesive unit.
  • Parallelizable subtasks: No — the routes are interdependent (counter references propose, accept creates game from proposal). Splitting would create artificial seams.
No decomposition needed. The 4 routes form a single cohesive API surface and share enough internal structure that splitting would add overhead without reducing agent execution time.

Recommendation

  • [SCOPE] Create architecture note arch-dataflow-twitch-2k-wager in pal-e-docs to back the arch:dataflow-twitch-2k-wager label. The project page has the data model but no standalone arch note.
  • [BODY] The "Related" section references "Schema ticket" and "Dashboard ticket" generically. Pin these to explicit issue numbers: "Schema ticket" → #67, "Dashboard ticket" → #70.
  • [BODY] Add explicit dependency ordering note in the Constraints section: "BLOCKED by #67 (schema). Do not move to in_progress until #67 is merged."