WA-VERIFY-039: Add script/verify dispatcher for verification/preflight checks#985
WA-VERIFY-039: Add script/verify dispatcher for verification/preflight checks#985kitcommerce wants to merge 1 commit intonextfrom
Conversation
WA-VERIFY-039 / Closes #896 Adds a repo-root `script/verify` entrypoint that: - Lists all supported checks via `./script/verify list` - Delegates to existing scripts via `./script/verify <name>` - Forwards the delegate's exit code unchanged - Reports clearly when a backing script is missing or not executable Registered checks (delegating to existing scripts): - default-appraisal-boot-smoke -> script/default_appraisal_boot_smoke - default-appraisal-zeitwerk-check -> script/default_appraisal_zeitwerk_check - check-service-ports -> script/check_service_ports The dispatcher is POSIX-compatible (#!/usr/bin/env sh), avoids GNU-only flags, and is read-only — it never modifies any files.
Architecture Review — PR #985 Wave 1Verdict: PASS_WITH_NOTES SummaryThe thin-dispatcher pattern is the right call here: a single stable entrypoint that delegates to purpose-built scripts keeps each script focused and avoids coupling verification logic into the router. The POSIX-sh choice is appropriate for a script that must run in CI, local dev, and Docker environments without Ruby/Bundler being available. The pipe-registry approach is unconventional but self-contained. Two dead-code items in the visible diff are the main architectural concerns: Findings
|
Simplicity Review — PR #985 Wave 1Verdict: PASS_WITH_NOTES Summary119 lines for a 3-check dispatcher is on the heavier side, primarily due to the inline pipe-subshell dispatch pattern and dead code. The Findings
|
Security Review — PR #985 Wave 1Verdict: PASS SummaryThe script is read-only and does not modify system state. The dispatch path validates user input against a fixed in-process registry before any execution, preventing injection of arbitrary script paths. Quoting is consistent throughout. Findings
|
Rails Conventions Review — PR #985 Wave 1Verdict: PASS Summary
Findings
|
Wave 1 Review Summary — PR #985
Overall: ✅ Wave 1 PassedNo blocking issues. All findings are LOW severity notes around dead code and readability. Key action items before merge (non-blocking):
Wave 2 recommended next: |
Summary
Closes #896
Adds a repo-root
script/verifyentrypoint that lists and runs existing verification/preflight scripts. The dispatcher is purely a thin router — it does not reimplement any logic, only delegates to existing scripts and forwards their exit codes.What changed
script/verify(executable, 119 lines, POSIXsh)Registered checks
default-appraisal-boot-smokescript/default_appraisal_boot_smokedefault-appraisal-zeitwerk-checkscript/default_appraisal_zeitwerk_checkzeitwerk:checkagainst the default appraisal stackcheck-service-portsscript/check_service_portsNew checks can be added by appending a single line to the
REGISTRYvariable insidescript/verify.Client impact
None. This is developer tooling only — a new
script/verifyfile that did not exist before. It does not touch any application code, gems, configuration, or existing scripts. No downstream plugin or host app is affected.Verification Plan
From the repo root:
# List all supported checks ./script/verify listExpected output:
Run the port-check (safe, read-only, no services required):
Verify unknown check returns exit 1:
Run boot-smoke / zeitwerk checks when services are available: