100 lines
5.6 KiB
Markdown
100 lines
5.6 KiB
Markdown
# Release tooling review findings
|
||
|
||
Status: two open findings. 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
|
||
|
||
**Location:** [scripts/release](../../scripts/release), lines 9–10, delegating to
|
||
the first-argument dispatch in [scripts/releasekit](../../scripts/releasekit),
|
||
lines 10–15. The invalid-form cases in
|
||
[scripts/release_test.go](../../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](../../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](https://pkg.go.dev/os/exec#hdr-Overview) 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.
|