From 98e7e9933353cf43f2be4a96cb0436a0bce068d2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 3 Nov 2026 15:53:45 +0000 Subject: [PATCH] Record release tooling review findings --- docs/roadmap/remediation.md | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/roadmap/remediation.md diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..dd80ed6 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,99 @@ +# 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.