Harden script/default_appraisal_zeitwerk_check (WA-VERIFY-082)

[kitcommerce]

Fixes #1068

Summary

Three hardening improvements to script/default_appraisal_zeitwerk_check:

1. Subshell pattern for engine loops — replaced the previous single cd "$ROOT_DIR/core/test/dummy" (bare directory change in the parent shell) with an explicit loop over all three engines (core, admin, storefront), each run in its own subshell (cd "$engine/test/dummy" && RAILS_ENV=test bin/rails zeitwerk:check). This isolates directory changes and prevents set -e failures from leaving the shell in an unexpected directory.

2. Redis readiness wait — added timeout 60 bash -c 'until redis-cli ping > /dev/null 2>&1; do sleep 2; done' before the checks run, so the script no longer fails with spurious connection errors when Redis is still starting up in CI.

3. Explicit RAILS_ENV=test — both the rbenv exec and plain bin/rails code paths now set RAILS_ENV=test inline rather than relying on the ambient export, making intent unambiguous.

Coverage expanded: previously only core/test/dummy was checked. Now all three engine dummy apps are verified, with a graceful SKIP if a dummy directory is absent.

Client Impact

None — CI-only script. No application code, APIs, or public interfaces changed.

[kitcommerce]

🤖 Wave 1 Automated Review — PR #1074

Architecture — PASS

Good structural improvement. Looping over engines in subshells isolates directory changes and failures cleanly. The separation of concerns (Redis readiness → engine iteration → per-engine check) is well-organized.

Simplicity — PASS

The refactored script is actually simpler in intent despite being slightly longer — the engine loop replaces implicit single-engine assumption with explicit multi-engine support. The rbenv exec fallback is clean.

Security — PASS_WITH_NOTES

{
  "reviewer": "security",
  "verdict": "PASS_WITH_NOTES",
  "severity": null,
  "summary": "No security issues. Minor observation about the Redis wait loop.",
  "findings": [
    {
      "severity": "LOW",
      "file": "script/default_appraisal_zeitwerk_check",
      "line": 42,
      "issue": "The `timeout 60` + `redis-cli ping` loop is fine for CI but could hang for 60s in local dev if Redis is not running. Not a security issue per se.",
      "suggestion": "Consider a brief log message noting what it is waiting for (already done — \"Waiting for Redis to be ready...\"). No action needed."
    }
  ]
}

Test Quality — PASS_WITH_NOTES

This is a shell script, so unit testing is limited. The script relies on CI execution for validation. The subshell pattern with set -e propagation is correctly structured to fail fast.

Rails Conventions — PASS

N/A for shell scripts. The RAILS_ENV=test is correctly hardcoded for the zeitwerk check context.

---

Wave 1 Verdict: ✅ All clear — no blocking issues.

[kitcommerce]

Wave 2 Review Results

rails-security: PASS — CI/local script hardening only.
database: PASS — no database changes.
test-quality: PASS — no test logic changes; script should reduce flakiness.

Wave 2 PASSED.

[kitcommerce]

Wave 3 reviews (performance / frontend / accessibility)

{"reviewer":"performance","verdict":"PASS","severity":null,"summary":"Build/CI script hardening only; no runtime performance impact.","findings":[]}

{"reviewer":"frontend","verdict":"PASS","severity":null,"summary":"No frontend changes.","findings":[]}

{"reviewer":"accessibility","verdict":"PASS","severity":null,"summary":"No accessibility changes.","findings":[]}

[kitcommerce]

Wave 4 (documentation) review

PASS — Script change is self-documenting via inline comments; no additional docs required.

[kitcommerce]

⚠️ Merge Conflict

This PR is labeled merge:ready and the hold window has passed (60 min), but GitHub reports CONFLICTING — there are merge conflicts preventing auto-merge.

Please rebase onto next and push to resolve. Issue #1068 has been moved to status:blocked + blocked:merge-conflict.

[kitcommerce]

⚠️ Merge conflict detected (mergeStateStatus: DIRTY). Cannot auto-merge. Blocking for resolution.

[kitcommerce]

⚠️ Merge Conflict

Auto-merge blocked: mergeStateStatus: DIRTY — branch has conflicts with next. Rebasing required before merge can proceed.

Please rebase on next and push to resolve.

[kitcommerce]

⚠️ Merge conflict detected. This PR cannot be auto-merged until the conflict is resolved. Please rebase on next and push.