Review: feat: dev environment -- ephemeral DB, shared Keycloak, migration practices, port convention

review-1876-2026-07-17 Review

review ready

Verdict: READY

Re-review. Previous verdict was NEEDS_REFINEMENT. All prior findings resolved or overridden by project owner.

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Standalone, identified during Sprint B validation
  • [x] Repo — ldraney/intelligentstaffingsystems (primary), cross-repo identified
  • [x] Dependencies — #85 blocker documented
  • [x] User Story — full As-a/I-want/So-that format
  • [x] Context — exceptionally detailed: gap analysis, prior attempts (#77 decomposition), current file state, five interdependent pillars, platform patterns to study
  • [x] File Targets — 12 ISS files in table format with current state and relevance; cross-repo files in separate table; SOPs identified
  • [x] Feature Flag — none (developer tooling, no user-facing feature)
  • [x] Acceptance Criteria — 14 criteria
  • [x] Test Expectations — existing tests must pass unchanged, manual verification steps documented
  • [x] Constraints — 7 constraints including no decomposition, no separate dev Keycloak, no hardcoded secrets
  • [x] Checklist — 14 items
  • [x] Related — #85 blocker, #77/#78/#79/#80 lineage, #6/#4 auth integration history, SOPs

Traceability

  • [x] story:project-setup label — verified in project-iss user-stories section ("Repo, docs, infra, and CI exist; sprints can dispatch")
  • [x] arch:rails label — present. Arch note missing in pal-e-docs — PROJECT OWNER OVERRIDE: process gap, not a ticket issue
  • [x] arch:infra label — present. Arch note missing in pal-e-docs — PROJECT OWNER OVERRIDE: process gap, not a ticket issue
  • [x] Forgejo issue — https://forgejo.tail5b443a.ts.net/ldraney/intelligentstaffingsystems/issues/86, open

File Targets

  • [x] docker-compose.yml — verified: port 9999 on lines 13/15, no Keycloak service, pgdata volume
  • [x] Makefile — verified: wraps docker compose, targets dev/setup/migrate/seed/test/ci/lint/security, no reset target, no environment-specific targets
  • [x] Dockerfile — verified: EXPOSE 3000 on line 26, uses harbor.tail5b443a.ts.net base
  • [x] config/puma.rb — verified: port ENV.fetch("PORT", 3000) on line 32
  • [x] config/database.yml — verified: DATABASE_HOST/USER/PASSWORD env vars with rails/rails defaults
  • [x] config/initializers/omniauth.rb — verified: registers Keycloak OIDC when KEYCLOAK_URL/REALM/CLIENT_ID/CLIENT_SECRET all present
  • [x] config/environments/development.rb — verified: mailer default_url_options localhost:3000 (port mismatch with docker-compose 9999 confirmed)
  • [x] config/environments/production.rb — verified: config.hosts includes intelligentstaffingsystems.ai and iss.tail5b443a.ts.net
  • [x] db/seeds.rb — verified: fake keycloak_sub values (admin-dev-sub-001, client-dev-sub-002)
  • [x] docs/local-dev-setup.md — verified: exists
  • [x] docs/architecture.md — verified: exists
  • [x] docs/pipeline.md — verified: exists
  • [x] test/test_helper.rb — verified: OmniAuth test_mode true, sign_in_as with fake subs (kc-sub-lead, etc.)

Repo Placement

OK. Issue filed on ldraney/intelligentstaffingsystems (primary repo). Cross-repo work explicitly identified: pal-e-deployments (overlay restructuring), pal-e-platform (terraform apply for DNS/Caddy), pal-e-services (Keycloak redirect URIs). Constraints section requires separate PRs for cross-repo changes.

Dependencies

  • #85 (board item 1875) — blocker, currently in backlog. CI minitest/mock + placeholder k8s secrets must be resolved first. Documented in issue Dependencies section.
  • #77 (board item 1868) — parent ticket, in todo. Decomposed into #78/#79/#80 which are all done or validated. This ticket fills the gap those didn't cover.
  • #79 — terraform apply for dev DNS still pending. This ticket picks up that remaining work.

Acceptance Criteria

14 criteria. All are verifiable. Most require manual verification (login with real Keycloak, mobile testing via dev URL, database reset workflow), which is acknowledged in the Test Expectations section. One automated criterion: test suite must continue to pass (make test / make ci). Criteria are comprehensive and cover all five pillars. No missing criteria identified.

Blast Radius

Limited to dev tooling — no production code changes beyond config.hosts addition and potential port convention alignment. Cross-repo blast radius is well-identified: Keycloak redirect URIs, DNS/Caddy config, kustomize overlays. The existing test suite is explicitly protected (constraint: "Do not break the test suite"). No similar patterns in sibling services that would be affected — this is ISS-specific dev environment work.

Decomposition Assessment

PROJECT OWNER OVERRIDE: The ticket explicitly states "Do not decompose this ticket" with detailed rationale — all five pillars are interdependent (port convention affects Keycloak redirect URIs, seed data depends on Keycloak strategy, dev URL needs correct port, migration docs reflect actual topology). The 8-point estimate reflects this scope. Accepted as one atomic unit of work.

Recommendation

No action needed.
This is an exceptionally well-scoped ticket. Every file target claim was verified accurate against the codebase. The context section provides comprehensive background including what was attempted before and why it fell short. The five-pillar structure gives the implementer a clear mental model without prescribing specific solutions. Platform SOPs and reference implementations are identified for study.