6.0 KiB
Release tooling review findings
Status: R1 complete; R2 open. Review baseline:
124a9fa57fb3f815bedbafe2d4819f6aa265ee7d, covering the implementation commits
after cb40f1d. This record requests corrections; no implementation was changed
during review.
The selected architecture is substantially present: three declarative profiles, one host-normalized registered-tool helper, separate publication and page-only delivery, retained credential-directory ignoring, and focused wiring tests. The following issues should be addressed before considering the refactor fully complete.
R1 — P2: Reject raw helper command names as legacy module selectors
Completed. The legacy entry point now checks its three module selectors before delegating to the helper, retaining the single profile/module mapping there. Regression cases cover all three raw-command collisions and require usage status 2 with no Go invocation. The new regression failed before the fix; the complete focused scripts suite and shell syntax checks passed afterward with Go 1.26.8, including valid legacy and direct raw-command forwarding.
Location: scripts/release, lines 9–10, delegating to the first-argument dispatch in scripts/releasekit, lines 10–15. The invalid-form cases in scripts/release_test.go, lines 186–200, miss these collisions.
The legacy interface is scripts/release MODULE VERSION, with three module
selectors. It now delegates its first argument directly into a helper that also
accepts version, publication-attempts, and reconcile as raw commands. These
invalid module names consequently bypass selector rejection and invoke Go.
For example, ./scripts/release version v1.2.3 reaches the tool as
version publish v1.2.3. With the real pinned tool it returns upstream's
"version takes no arguments" diagnostic after tool resolution. In a disposable
copy with a controlled Go boundary returning status 73, each of the three raw
command names returned 73 and invoked Go; an ordinary unknown selector returned
2 without invoking Go. Thus invalid legacy input depends on tool availability
and behavior instead of being rejected at the wrapper boundary.
Required correction: Make legacy module selection unambiguous before raw command dispatch. Keep one profile/module mapping and the supported raw helper forms; a small selector check is preferable to another routing framework. Preserve upstream ownership of version and flag validation.
Verification: Add all three raw command names as invalid legacy selectors. Each must return usage status 2 before any Go command runs. Valid legacy publication and the helper's direct raw forms must retain their forwarding and exit-status behavior. This restores the requested malformed-wrapper handling; it adds no new accepted-version restriction.
R2 — P2: Restore the tests' optional POSIX-shell execution boundary
Location: scripts/release_test.go, especially
fixture.run at line 63 and the launcher test setup.
The rewritten tests execute extensionless POSIX shell scripts directly through
exec.Command. They neither discover a shell nor skip launcher execution when
it is unavailable. The previous suite resolved sh, skipped shell-dependent
tests when it was missing, and passed the script as an argument to that shell.
Direct execution works on this review's Linux host, but relies on Unix script execution semantics. Native Windows process creation does not supply a POSIX interpreter for these files. Go's os/exec documentation explicitly notes that it does not invoke a system shell. Consequently the default repository suite now fails on hosts without compatible script execution rather than preserving the previous optional-shell behavior. This portability finding is based on the changed execution path and platform implementation; a native Windows test run was not available during review.
Required correction: Discover a POSIX shell for launcher execution tests, invoke it explicitly with the script path, and skip those tests when the shell is unavailable. Keep profile-only JSON assertions runnable without requiring a shell. Preserve the reduced test architecture and existing argv/status checks. No native Windows release launcher or additional supported-platform guarantee is requested.
Verification: Confirm that launcher tests use the discovered interpreter and handle its absence explicitly, while profile assertions still run. Rerun the focused suite on the available POSIX host. Broader platform matrices are not a new release gate; this correction removes an incidental environment restriction introduced by the test rewrite.
Review evidence and limits
- Go 1.26.8 offline root tests and the temporary-workspace suites for both integrations passed. Both production scripts passed shell syntax checks.
- The real helper reported ReleaseKit v0.8.0 under foreign target settings.
- ReleaseKit checks passed for a disposable committed source snapshot using the actual profiles: ordinary root validation, then both integration checks with the implementation plan's temporary workspace-backed test process. Selected file checks continued to inspect the committed declarations.
- Local documentation destinations and applicable Markdown fragments passed inspection. The credential file remained ignored and its contents were not read. No real publication or hosted-delivery operation was invoked.
- Library code and dependency declarations were unchanged by the sprint. The documented integration dependency-version mismatch remains a pre-existing independent-release-readiness limitation; the workspace-backed results do not establish that published dependencies are aligned.
No additional architecture gaps or unrequested runtime/public-API guarantees were identified. The selected file checks and forbidden token path match the finalized scope. R2 concerns the default tests' execution environment rather than a new production validator or library contract.