Review: CI minitest/mock LoadError + placeholder k8s secrets + action_mailer example.com (re-review)
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-railsnote 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-infranote 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 explicitgem "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 fixpal-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.comgrep: 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
intelligentstaffingsystemsnamespace. 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 notearch-railsfor the Rails component (low priority, not blocking)[SCOPE]Create architecture notearch-infrafor 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.