pal-e-docs

Review: Registration form -- Kalshi API credential capture and validation

review-1729-2026-07-04 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1729 after refinement. Previous review review-1729-2026-07-03 found NEEDS_REFINEMENT with 7 [BODY] recommendations and 1 [SCOPE] recommendation. All 7 [BODY] items have been addressed in the updated issue.

Template Completeness

  • [x] Type -- Feature
  • [x] User Story -- present
  • [x] Lineage -- present (Story: Landing Page, Sprint: 6)
  • [x] Repo -- ldraney/prediction-assistant
  • [x] Context -- present, now explicitly clarifies KalshiClient per-user integration is out of scope
  • [x] File Targets -- 8 targets, all verified (see below)
  • [x] Feature Flag -- "None" (acceptable)
  • [x] Acceptance Criteria -- 5 items (reduced from 6; credential revocation removed)
  • [x] Test Expectations -- 4 items including mailer test
  • [x] Constraints -- 5 items including Rails 8 encrypts requirement
  • [x] Checklist -- 7 items
  • [x] Related -- present
Extra sections: ### Scope, ### Follow-ups (both helpful, not in template, harmless).

Traceability

  • [x] story:landing-page label -- Landing Page & Registration
  • [x] story note verified -- landing-page entry found in project-prediction-assistant user-stories section
  • [x] arch:rails label -- present on board item
  • [ ] arch note MISSING -- [SCOPE] arch-rails note does not exist in pal-e-docs (carried over from previous review; project-level organizational task, not a scope blocker for this ticket)
  • [x] Forgejo issue -- #58, open

File Targets

  • [x] app/models/user.rb -- verified: marked as (new), does not exist, correctly describes creating User model with Rails 8 encrypts. Previous review flagged this as incorrectly described; now fixed.
  • [x] db/migrate/YYYYMMDD_create_users.rb -- verified: marked as (new), correctly says "create users table". Previous review flagged "add columns" language; now fixed.
  • [x] app/controllers/registrations_controller.rb -- verified: marked as (new), does not exist
  • [x] app/views/registrations/new.html.erb -- verified: marked as (new), does not exist
  • [x] config/routes.rb -- verified: exists, currently has Keycloak auth routes and bot resources at lines 12-21
  • [x] app/mailers/user_mailer.rb -- verified: marked as (new), app/mailers/ directory does not exist. Previous review flagged this as missing; now added.
  • [x] app/views/user_mailer/welcome.html.erb -- verified: marked as (new), does not exist. Previous review flagged this as missing; now added.
  • [x] app/services/keycloak_admin_service.rb -- verified: marked as (new), does not exist. Previous review flagged this as missing; now added.

Repo Placement

OK -- all 8 file targets are in ldraney/prediction-assistant, matching the Forgejo issue. No cross-repo concern.

Dependencies

  • Depends on #57 (Keycloak login flow) -- board item #1728, currently in validation column. Soft blocker but not blocking scope review.
  • watchdog_configs.user_id FK -- properly deferred to Follow-ups section (schema.rb line 98 has user_id column with no FK constraint).
  • No other blocking dependencies found on the board.

Acceptance Criteria

  • 5 ACs, all testable and verifiable by an agent.
  • AC #6 (credential revocation) was removed per previous review recommendation and properly deferred to Follow-ups.
  • Scope items 5 (Keycloak user creation) and 6 (welcome email) are covered by Checklist items but not by ACs. Acceptable -- the ACs cover the user-facing behavior, and test expectations cover the mailer.

Blast Radius

Properly contained. The updated issue explicitly states that KalshiClient per-user integration is out of scope. The Follow-ups section documents the blast radius (kalshi_client.rb, order_service.rb, market_scanner_job.rb, and bot consumers) as a separate ticket. No undisclosed downstream effects.

Decomposition Assessment

8 file targets in 1 repo. 5 acceptance criteria. Scope covers: User model creation, migration, controller, view, routes, mailer, Keycloak admin service, credential validation. All targets are in a single repo and follow standard Rails conventions. With credential revocation and KalshiClient per-user integration removed, estimated agent work fits within 5 minutes. No decomposition needed.

Previous Review Resolution

All 7 [BODY] recommendations from review-1729-2026-07-03 verified as fixed:
  • user.rb file target now correctly marked as (new) -- FIXED
  • Migration description now says "create users table" -- FIXED
  • Mailer file target added (user_mailer.rb + template) -- FIXED
  • Keycloak admin service file target added -- FIXED
  • Encryption approach pinned to Rails 8 encrypts macro -- FIXED
  • AC #6 (credential revocation) removed, deferred to Follow-ups -- FIXED
  • KalshiClient per-user integration explicitly out of scope in Context + Follow-ups -- FIXED
1 [SCOPE] recommendation carried forward:
  • arch-rails architecture note still missing from pal-e-docs (project-level task, non-blocking)

Recommendation

  • [SCOPE] Create architecture note arch-rails for component rails (carried over from previous review -- project-level organizational task, does not block implementation)
No other action needed. Ticket is ready for implementation.