pal-e-docs

Review: Registration form — Kalshi API credential capture and validation

review-1729-2026-07-03 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — present
  • [x] Repo — ldraney/prediction-assistant
  • [x] User Story — present
  • [x] Context — present
  • [x] File Targets — present (but inaccurate, see below)
  • [x] Feature Flag — "None" (acceptable for this ticket)
  • [x] Acceptance Criteria — 6 items present
  • [x] Test Expectations — present
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present
Extra section: ### Scope (not in template, harmless).

Traceability

  • [x] story:landing-page label — Landing Page & Registration
  • [x] story note verified — story-prediction-assistant-landing-page exists in pal-e-docs and in project-prediction-assistant user-stories section
  • [x] arch:rails label — present on board item
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-rails
  • [x] Forgejo issue — #58, open

File Targets

  • [ ] app/models/user.rb — ISSUE: file does not exist. Issue says "add encrypted credential fields" implying modification, but user.rb and the users table need to be created from scratch. No User model or users table exists in the current schema.
  • [ ] db/migrate/ — ISSUE: description says "add credential columns" but there is no users table in the schema. Should say "create users table with credential columns."
  • [x] app/controllers/registrations_controller.rb — verified: marked as new, correctly does not exist yet
  • [x] app/views/registrations/new.html.erb — verified: marked as new, correctly does not exist yet
  • [x] config/routes.rb — verified: exists, currently has Keycloak auth routes and bot resources
Missing file targets:
  • No mailer listed — but Scope item 5 says "Send welcome email with generated password." Needs app/mailers/ target.
  • No Keycloak admin API integration — but Scope item 4 says "Create Keycloak user account on successful validation." Needs a service or initializer for Keycloak Admin REST API calls (and likely keycloak-admin gem in Gemfile).
  • No encryption gem specified — Gemfile currently has no encryption library (lockbox, attr_encrypted, etc.). Issue should specify whether to use Rails 8 encrypted attributes, lockbox, or another approach, and add the gem to Gemfile.
  • No credential management UI — but AC #6 says "Revoking API access (deleting credentials) is possible." Needs controller action and possibly a view for credential deletion.

Repo Placement

OK — all file targets are in ldraney/prediction-assistant. Keycloak admin API calls are made from the Rails app, so no cross-repo concern, though Keycloak realm configuration (client permissions for admin API) may need a separate infra ticket if not already set up.

Dependencies

  • Explicitly depends on #57 (Keycloak login flow) — board item #1728, currently in validation column. Implemented but not yet validated. Soft blocker.
  • No other blocking dependencies found on the board.
  • watchdog_configs table already has a user_id column (schema.rb line 98) but with no foreign key to a users table — this ticket will need to address that relationship.

Acceptance Criteria

  • AC #1–#5 are testable and clear.
  • AC #6 "Revoking API access (deleting credentials) is possible" — introduces credential management functionality that is not mentioned in the Scope or File Targets sections. Should either be removed (separate ticket) or explicitly scoped with file targets.
  • Missing AC for Keycloak user creation (Scope item 4) and welcome email delivery (Scope item 5).

Blast Radius

Per-user credential transition: KalshiClient (app/services/kalshi_client.rb) currently reads credentials from Rails.application.config.kalshi (global singleton). Moving to per-user credentials will require changes in how KalshiClient is instantiated across at least 4 files:
  • app/services/order_service.rb
  • app/jobs/market_scanner_job.rb
  • app/services/pregame_stacker_bot.rb
  • app/jobs/bulk_sweep_job.rb
None of these are in the file targets. The issue should clarify whether this ticket wires per-user credentials into the service layer or if that integration is a separate ticket.

Decomposition Assessment

5 listed file targets (really 8+ when missing targets are added). 6 acceptance criteria. 6 scope items spanning: User model creation, migration, controller, view, Keycloak admin API integration, email sending, encryption setup, and credential validation. Estimated agent work exceeds 5 minutes. However, if scope is tightened to just credential capture and storage (removing Keycloak user creation, email, and revocation), it could fit. Borderline — tighten scope or decompose.

Recommendations

  • [BODY] Fix file target: app/models/user.rb should say "(new — create User model with encrypted credential fields)" since the file and users table do not exist
  • [BODY] Fix migration description: "create users table with encrypted credential columns" not "add credential columns"
  • [BODY] Add missing file target: mailer for welcome email (Scope item 5)
  • [BODY] Add missing file target: Keycloak admin API service for programmatic user creation (Scope item 4)
  • [BODY] Specify encryption approach and add gem to Gemfile if needed (lockbox, attr_encrypted, or Rails encrypted attributes)
  • [BODY] Either remove AC #6 (credential revocation) to a separate ticket, or add file targets for credential management controller/view
  • [BODY] Clarify whether this ticket wires per-user credentials into KalshiClient/OrderService or if that is a separate integration ticket (blast radius: 4+ service/job files not listed)
  • [SCOPE] Create architecture note arch-rails for component rails