Review: CI minitest/mock LoadError + placeholder k8s secrets + action_mailer example.com (re-review)

review-1875-2026-07-17-v2 Review

review ready

Verdict: APPROVED

Re-review of #85. Previous review returned NEEDS_REFINEMENT. All issues have been addressed in the updated issue body. Ticket is ready for implementation.

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — Standalone, discovered during Sprint B validation sweep
  • [x] Repo — Both repos listed (intelligentstaffingsystems + pal-e-deployments)
  • [x] What Broke — Three issues described with detail and error messages
  • [x] Repro Steps — Provided for all three bugs
  • [x] Expected Behavior — Present
  • [x] Environment — Present, includes Ruby version note and minitest version
  • [x] Acceptance Criteria — 9 criteria, all verifiable
  • [x] Related — Present with cross-references
  • [x] Production Risk — Bonus section, documents critical ordering constraint
  • [x] Validation Evidence — Bonus section, lists 7 blocked validation tickets

Traceability

  • [x] story:project-setup label — verified in project-iss user-stories section
  • [x] arch:rails label — present on board item
  • [ ] arch note MISSING — [SCOPE] No arch-rails note found in pal-e-docs. Low priority for unplanned bug fix.
  • [x] arch:infra label — present on board item
  • [ ] arch note MISSING — [SCOPE] No arch-infra note found in pal-e-docs. Low priority for unplanned bug fix.
  • [x] Forgejo issue — #85, open

File Targets

  • [x] test/test_helper.rb:4 — verified: require "minitest/mock" present at line 4
  • [x] test/models/message_test.rb:141 — verified: Turbo::StreamsChannel.stub(:broadcast_action_later_to, stub) uses minitest/mock stub method
  • [x] Gemfile — verified: no explicit gem "minitest" declaration
  • [x] Gemfile.lock — verified: minitest (6.0.6) present as transitive dep of activesupport (line 176)
  • [x] config/environments/production.rb:60 — verified: config.action_mailer.default_url_options = { host: "example.com" }
  • [x] Commit 7073cae — verified: "fix: resolve 3 Messages test failures blocking CI (#75) (#76)" — correct PR #76 reference

Repo Placement

Cross-repo fix documented correctly. Issue filed on intelligentstaffingsystems (primary), touches:
  • intelligentstaffingsystems — CI fix (Gemfile or gem path) + action_mailer fix
  • pal-e-deployments — k8s secrets fix
Both repos listed under ### Repo. Single-ticket approach justified by project owner: fixing CI without fixing secrets = production outage.

Dependencies

  • #86 (dev environment, sprint:C) — explicitly blocked by this ticket per issue body
  • #71 (action_mailer example.com) — absorbed into this ticket; board item #1867 exists separately in backlog
  • 7 validation-column tickets blocked: #49, #50, #51, #52, #58, #59, #69 — all failed validation due to these blockers
  • #77 (deployment overlays) — in todo, related infrastructure work
Dependencies are documented in the issue body.

Acceptance Criteria

9 criteria. All verifiable by an agent:
  • AC 1-2: Pipeline status observable via Woodpecker
  • AC 3-5: Pod/secret state verifiable via kubectl and argocd CLI
  • AC 6: Site liveness verifiable via curl
  • AC 7: Deployment verification via git log + argocd
  • AC 8: Code inspection (grep for action_mailer host setting)
  • AC 9: Grep for example.com in production config
Note on AC 9: config/environments/production.rb has 4 additional example.com references, all in comments (lines 22, 66, 83, 84). config/environments/test.rb:40 also uses example.com for test environment, which is appropriate. Only line 60 is an active bug.

Blast Radius

  • example.com grep: Only one active reference in production config (line 60). Four commented-out references are safe. Test environment reference is intentional.
  • minitest/mock: Only used in test/models/message_test.rb. No other test files reference it.
  • k8s secrets: Scoped to the intelligentstaffingsystems namespace. No cross-namespace impact.
  • The production risk section correctly identifies the critical ordering constraint: secrets must be fixed before or simultaneously with CI.

Decomposition Assessment

9 AC across 2 repos technically exceeds the 5-minute rule thresholds. However, the project owner has explicitly rejected decomposition with sound rationale: the CI fix and secrets fix are causally coupled — fixing CI without fixing secrets triggers a production outage. This is intentionally one ticket. Not flagging decomposition.

Recommendation

  • [SCOPE] Create architecture note arch-rails for the Rails component (low priority, not blocking)
  • [SCOPE] Create architecture note arch-infra for the infrastructure component (low priority, not blocking)
No blocking issues. Previous review findings addressed: root cause explanation corrected (minitest 6.0.6 transitive dep, investigation left to implementer), commit ref fixed (7073cae / PR #76), decomposition rejected by owner, #71 absorption acknowledged.