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

review-1876-r3-2026-07-17 Review

review ready

Verdict: READY

Re-review (round 3). Previous verdicts: review-1876-2026-07-18 NEEDS_REFINEMENT (two arch note gaps), review-1876-2026-07-17 READY (project owner override). This round confirms the gaps are genuinely resolved -- arch notes now exist, and the dependency blocker (#85) is closed.

Template Completeness

  • [x] Type -- "Feature"
  • [x] Lineage -- Standalone, identified during Sprint B validation
  • [x] Repo -- ldraney/intelligentstaffingsystems (primary) + 3 cross-repo references (pal-e-deployments, pal-e-platform, pal-e-services)
  • [x] Dependencies -- #85 blocker documented (now resolved -- issue closed)
  • [x] User Story -- Well-formed As a / I want / So that
  • [x] Context -- Exceptionally thorough: gap analysis of #77 decomposition, current state of every relevant file, five interdependent pillars, platform patterns and SOPs to study
  • [x] File Targets -- Detailed table with current state and relevance for 12 ISS-repo files + cross-repo reference table + pal-e-docs SOPs to read
  • [x] Feature Flag -- "none -- developer tooling, no user-facing feature"
  • [x] Acceptance Criteria -- 14 criteria covering all five pillars
  • [x] Test Expectations -- Manual verification approach (correct for dev tooling), existing tests must pass unchanged
  • [x] Constraints -- 7 constraints including explicit "do not decompose" directive
  • [x] Checklist -- 14 items
  • [x] Related -- Comprehensive cross-references to parent/child/sibling tickets and SOPs
All required sections for a Feature template are present and complete.

Traceability

  • [x] story:project-setup label -- verified in project-iss user-stories section ("Repo, docs, infra, and CI exist; sprints can dispatch")
  • [x] story note verified -- found in project-iss user-stories table (key: project-setup, role: Developer)
  • [x] arch:rails label -- Rails component
  • [x] arch note verified -- arch-deployment-iss Components table includes rails row: "Rails application server (Puma) -- Port 3000 production, 9999 local dev"
  • [x] arch:infra label -- Infrastructure component
  • [x] arch note verified -- arch-deployment-iss Components table includes infra row: "Platform infrastructure (k3s cluster + edge) -- Self-hosted on Hetzner"
  • [x] Forgejo issue -- ldraney/intelligentstaffingsystems#86, open
Full ISS architecture triplet confirmed: arch-domain-iss, arch-dataflow-iss, arch-deployment-iss. Both arch label gaps from the original review are resolved.

File Targets

  • [x] docker-compose.yml -- verified: port 9999 (line 13 command, line 15 port mapping), no Keycloak service, pgdata volume, harbor base image
  • [x] Makefile -- verified: wraps docker compose, targets dev/setup/migrate/seed/test/ci/lint/security/console/logs. No reset target, no environment-specific targets
  • [x] Dockerfile -- verified: EXPOSE 3000 (line 26), harbor.tail5b443a.ts.net/library/ruby-rails-build:latest base
  • [x] config/puma.rb -- verified: port ENV.fetch("PORT", 3000) at line 32
  • [x] config/database.yml -- verified: dev/test use DATABASE_HOST, DATABASE_USER, DATABASE_PASSWORD env vars with rails/rails defaults
  • [x] config/initializers/omniauth.rb -- verified: registers Keycloak OIDC when all four KEYCLOAK_* env vars present OR in test mode. Dev without env vars skips middleware entirely. PKCE enforced.
  • [x] config/environments/development.rb -- verified: config.action_mailer.default_url_options = { host: "localhost", port: 3000 } at line 41. Port mismatch with docker-compose (9999) confirmed as pre-existing bug.
  • [x] config/environments/production.rb -- verified: config.hosts includes intelligentstaffingsystems.ai and iss.tail5b443a.ts.net (lines 87-88). No dev.intelligentstaffingsystems.ai entry yet.
  • [x] db/seeds.rb -- verified: fake keycloak_sub values (admin-dev-sub-001, client-dev-sub-002). 4 Leads, 3 CatalogEntries, sample Message threads. find_or_create_by! pattern is idempotent.
  • [x] docs/local-dev-setup.md -- verified: exists, documents current 9999 workflow
  • [x] docs/architecture.md -- verified: exists, contains port convention table showing local dev at 9999, production at 3000
  • [x] docs/pipeline.md -- verified: exists, references localhost:9999 in dev workflow section
All 12 ISS-repo file targets verified. Every description of current state in the issue body is accurate.

Repo Placement

OK. Issue filed on ldraney/intelligentstaffingsystems, which is the primary repo. Cross-repo changes (pal-e-deployments, pal-e-platform, pal-e-services) are explicitly identified in both the Repo section and the File Targets cross-repo table. Constraints section mandates separate PRs for cross-repo changes. No mismatch.

Dependencies

  • #85 (board item 1875) -- was listed as blocker. Now CLOSED. CI minitest/mock + placeholder k8s secrets resolved. No longer blocks this ticket.
  • #77 (board item 1868) -- parent ticket, in todo, marked decomposed. Its decomposition children (#78, #79, #80) are in validation/done. This ticket fills the gap that #77's decomposition missed.
  • #78 (board item 1869) -- kustomize overlays, in validation. Placeholder secrets issue was tied to #85 (now resolved).
  • #79 (board item 1870) -- dev DNS + Caddy, in validation. Terraform apply still pending. This ticket picks up that remaining work.
  • #80 (board item 1871) -- Makefile + docker-compose, in done.
  • No undocumented dependencies found.

Acceptance Criteria

14 criteria, all verifiable. Manual verification is appropriate for developer tooling (criteria 1-5, 10-11). Code-verifiable criteria cover Keycloak redirect URIs (6), config.hosts (7), port convention docs (8), mailer URL (9), test suite (12), Makefile (13), cross-repo PRs (14). Comprehensive coverage of all five pillars. No missing criteria identified.

Blast Radius

Port 9999 is referenced in 10+ locations: docker-compose.yml, Makefile, README.md, docs/architecture.md, docs/pipeline.md (3 references), docs/local-dev-setup.md (4 references). If port changes, all must be updated. The ticket is aware of this scope.
The mailer URL mismatch (development.rb says 3000, docker-compose runs 9999) is a pre-existing bug that this ticket would fix as a side effect of resolving the port convention.
Keycloak redirect URI changes in pal-e-services affect the ISS realm only, not other clients. CORS implications are noted in the ticket.
No similar patterns in sibling services that would be affected -- this is ISS-specific dev environment work.

Decomposition Assessment

By metrics: 12 file targets across 4 repos, 14 acceptance criteria. Technically exceeds decomposition thresholds. However: the ticket explicitly forbids decomposition 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 and Keycloak callback, migration docs reflect actual topology). User standing preference: "stop decomposing; write inclusive tickets." Cross-repo work is separated into distinct PRs per constraints. The 8-point sizing is appropriate. No decomposition needed.

Recommendation

No action needed.
Both gaps from the original review (review-1876-2026-07-18) are now genuinely resolved:
  • arch:rails -- backed by arch-deployment-iss Components table (rails row)
  • arch:infra -- backed by arch-deployment-iss Components table (infra row)
The dependency blocker (#85) is closed. The ticket is exceptionally well-scoped with accurate file target descriptions, comprehensive acceptance criteria, and clear implementation guidance without being prescriptive. Ready for implementation.