16 KiB
Host-native launcher review findings
Reviewed the implementation from bf07bed through 0269648 against the
feature roadmap and
implementation plan.
The maintained launchers select the Go host before preparing and running the tool. API-only recovery prepares its private module on the host; printed Go handoffs discover the host again when run. The CLI retains the native fallback and saved-record selection. Application build targets still come from the profile, and the public Go facades and dependency boundaries are unchanged.
Make the adoption smoke command runnable
Completed. The smoke command runs the saved launcher through sh; verified with a non-executable file and a foreign target.
Evidence: The adoption guide
tells readers to save the launcher as releasekit and run
./releasekit version. A newly saved shell file ordinarily lacks its execute
bit. In a disposable directory, the documented command failed with status 126
and “Permission denied.” Invoking the same snippet through sh ran a
module-registered fixture tool on Linux/amd64 with inherited
GOOS=windows GOARCH=arm64.
Fix: Use sh ./releasekit version in the smoke instructions, or give an
explicit chmod +x ./releasekit step before direct execution. Verify the
published instructions from a newly created file under a foreign target.
Limit the recovery guarantee to GOOS and GOARCH
Completed. The CLI reference names the two target variables and identifies the shell syntax.
Evidence: The CLI contract says Go handoffs select the host
“independently of inherited cross-compilation settings.” The
command formatter overrides GOOS and
GOARCH and retains other build settings. For example, building the actual CLI
with the native target, CGO_ENABLED=1, and an unavailable C compiler named
by CC still fails in runtime/cgo. This is consistent with the roadmap's
scope, but the CLI wording implies broader independence.
Fix: Say that generated Go handoffs set GOOS and GOARCH from the
selected Go toolchain at invocation time. Preserve the other inherited settings
and the existing workspace and toolchain overrides. The documented handoff
uses POSIX shell syntax; identify that syntax when telling operators where to
paste the command.
Exercise a profile target that differs from the launcher host
Completed. The binary delivery fixture selects a non-host Linux target and checks the uploaded ELF machine as well as the filename.
Evidence: The binary-delivery case in
TestMultiModuleWoodpeckerDelivery
supplies a foreign target to the workflow and checks the uploaded
core-v2.1.0-linux-amd64 asset. The profile itself selects linux/amd64.
On the Linux/amd64 review host, the test cannot distinguish using the
profile target from accidentally using the normalized launcher target. The
current build code still applies the selected profile target explicitly.
Fix: In that fixture, select a supported profile target distinct from the host and assert the uploaded asset matches it. Where practical, inspect the uploaded executable header as well as the name, so the test catches a host-built payload with a profile-derived filename. Keep the existing local forge fixture and native version check; the cross-built asset need not run on the host.
Reconcile the feature roadmap with the completed implementation
Completed. The feature roadmap records implementation and labels the pre-change baseline. The release-note entry remains in the implementation plan.
Evidence: The feature roadmap still declares “implementation pending” and describes the pre-change launchers as the current baseline. All six stages in the implementation plan are marked complete, and the maintained launchers and recovery code implement the host selection described there.
Fix: Mark the roadmap implemented and label the old baseline as historical, or retire the completed planning documents after review. Preserve the planned release-note entry for the next selected release version.
Verification
go test ./..., go run ./internal/apicheck, and syntax checks for the
maintained shell scripts passed. The Go suite exercised real tool preparation,
Woodpecker command blocks with local fixtures, and later execution of printed
recovery commands. Disposable smoke probes also verified the maintainer
launcher forwards the child status and the manual retry preserves the calling
shell's target variables. The review made no changes to production code or
tests.
Integration audit
The audit below examines how the new bootstrap paths interact with existing invocation recipes, product checks, and publication recovery.
Use the external Go executable in printed recovery commands
Completed. All three Go invocations use external lookup through PATH. The handoff regression reproduces the shell-function failure before the fix and verifies reconciliation afterward.
Evidence: Before host normalization, the
command formatter emitted
env GOWORK=off GOTOOLCHAIN=local go ..., which resolves Go through PATH.
Recovery preparation still executes an
external Go command. The new printed command uses bare go for both host
queries and the later tool invocation, so a function named go in the
operator's shell can intercept it. In a local rejected-push fixture, the
actual printed command reconciled the saved record in an ordinary shell.
With go() { GOOS=windows GOARCH=arm64 command go "$@"; } defined, the same
command failed with an exec format error. The function overrode the final
assignments even though the host queries reported the correct host. In this
case both paths exited with status 1, but only one produced a reconciliation
result.
Fix: Restore external command lookup for the host queries and tool
invocation, for example by using env go -C ... throughout the subshell.
Keep selection through the operator's current PATH, including executable
shims, and keep the exact record and exit status. Exercise a generated
handoff from a POSIX shell that defines a cross-build Go function; assert
the reconciliation result as well as the status.
Explain how host normalization affects configured product checks
Completed. The adoption guide explains inherited host variables and points checks that need an application target to the existing environment overlay.
Evidence: The updated launchers export GOOS and GOARCH to the
ReleaseKit process. Existing configured checks inherit that
process environment unless their env maps override it;
RunChecks adds the configured
and ReleaseKit-owned values to the child. This differs from built-in
application compilation, which chooses its target from the profile. A
disposable consumer's check using shell defaults for an application target
passed with the previous direct go tool invocation and no target
variables, then failed through the updated launcher because it observed
linux/amd64. Adding explicit GOOS=windows and GOARCH=arm64 to that
check's env restored its behavior.
Fix: In the launcher adoption guidance, explain that host variables
reach configured child checks and that checks which need an application
target should set it explicitly in their existing env map or command.
Link to the configuration contract. Keep the current environment overlay
semantics and the separate profile selection for built-in builds.
Align direct source and standalone CI recipes with the launcher guidance
Completed. The source examples link to host selection, and the standalone install recipe scopes the same settings to go install. Both paths were verified under a foreign target.
Evidence: The README quick start and
CLI reference still recommend bare go run ./cmd/releasekit
for source invocation. The consumer delivery guide
also recommends bare go install ...@PIN for consumers who install the
delivery executable separately. These older recipes remain target-sensitive
alongside the new host-native launchers. On Linux/amd64 with inherited
GOOS=windows GOARCH=arm64, the README's actual version command failed
with an exec format error. A versioned install from a disposable local
module proxy failed under the same variables with GOBIN set; the native
install succeeded and its binary ran.
Fix: Link these recipes to the host-selection instructions and make the
native target requirement explicit at the source invocation and at the
standalone installation step. Use the existing recipe where a copyable
command is useful. Keep the selected version, install location, and
go run exit-status caveat. An already installed native binary does not
need a new bootstrap step.
Algorithmic audit
The host normalization itself adds a fixed number of Go invocations per launch. The CLI renders a fixed argument list, so quoting and formatting take time and space linear in the lengths of the directory and record paths. Go dependency inspection and compilation retain their existing roles. The findings below concern a pre-existing algorithm on the audited recovery path and a new module-selection defect in the smoke recipe.
Relocate recovery replacements in one pass without merging selectors
Completed. Recovery rewrites relative destinations in one pass. The CLI regression verifies both selectors, the selected tool, the rejected-push status, and later reconciliation.
Evidence: The API-only path in
RecoveryTool walks the parsed
go.mod replacements. It calls modfile.AddReplace for each relative
local replacement. In the pinned golang.org/x/mod implementation,
each call scans the entire replacement slice. With R total directives
and L relative local directives, relocation visits O(R × L) entries,
and takes quadratic time when all or most entries require relocation.
This loop predates host normalization and still runs before the new host
query.
The same loop can panic on a valid module that declares both an unversioned
and a version-specific replacement for the same dependency. When the
unversioned directive appears first and uses a relative path, calling
AddReplace with an empty old version treats both directives as matches
and clears the version-specific entry. The outer loop then visits that
cleared entry as if it were another relative replacement. In a disposable
API-only consumer with both forms for ReleaseKit, Go accepted the module
and validation passed. After a local hook rejected the tag push, ReleaseKit
saved the publication record, then panicked in modfile.addReplace. It
exited with status 2 instead of reporting the rejected push and a recovery
handoff.
Fix: Rewrite each relative right-hand path on its existing parsed
directive in one pass, preserving the exact left-hand path and version
selector. For example, update the typed replacement and its corresponding
syntax token before formatting. This takes O(R) directive visits plus
the bytes processed for the paths and avoids selector merging. Keep the
existing relocation policy, the separate synthetic replacement for the
original module, and the private module lifetime. Add a recovery fixture
with distinct local destinations for a wildcard and a version-specific
replacement. Check that the handoff preserves both directives, selects the
version-specific destination, and reports the original publication status.
Anchor smoke-check module paths to the intended checkout
Completed. Both nested paths use ./; verified the full smoke recipe with a conflicting CDPATH.
Evidence: The multi-module smoke recipe
replaced go -C integrations/... with subshells that run
cd integrations/... before host discovery. A shell with CDPATH set can
resolve these relative names in another checkout. In a disposable probe
with matching module paths under two directories, the published snippet
sent both nested checks to the checkout named by CDPATH and returned
success. If the other checkout has valid profiles and tools, the smoke
result can appear valid for the wrong candidate.
Fix: Use cd ./integrations/ckconfig and cd ./integrations/core/v2,
or clear CDPATH for those two directory changes. The number of lookups is
already fixed; the change corrects which directory is selected. Verify the
copyable recipe with a conflicting CDPATH entry.
Follow-up review after remediation
Reviewed all six implementation stages at 9797f43. The focused launcher,
delivery, and recovery tests and the public API and dependency checks passed.
The host discovery work remains bounded to a fixed number of calls per
invocation, and recovery replacement relocation now visits each directive
once. No additional scaling issue was identified.
Stages 1, 2, and 6: Invoke the exact registered Go tool
Evidence: The multi-module launcher
and both consumer Woodpecker templates
(single-module,
multi-module) register the
full ReleaseKit package path, but invoke the short name releasekit. The
adoption launcher and
multi-module smoke recipe also use
the short name. Go allows two different package paths with the same final
component in the tool declarations. With a second local tool ending in
/cmd/releasekit, the maintained launcher and both extracted CI command
blocks prepared the intended module, then exited with an ambiguous-tool
error. Changing only the invocation to the full package path ran the
intended host-native tool in all three probes. Recovery already invokes
the full path and is unaffected.
Fix: Invoke the same full package path used for registration in the maintained launchers, templates, and copyable consumer recipes. Preserve the selected module version, arguments, and exit status. Extend an existing launcher fixture with a second registered tool of the same short name and verify that only the intended tool runs.
Stage 3: Build the manual retry tool only from a clean tagged checkout
Evidence: The manual retry block runs git fetch and
git checkout --detach before the subshell that enables set -e. A
failed checkout can therefore be followed by dependency preparation, a
build, and execution from the wrong source. A successful checkout can
also carry local edits forward. Both cases were reproduced with the
documented block in disposable Git repositories and a local bare remote:
a checkout refused because of conflicting edits still ran a fixture
built from the edited source, and checking out the already selected tag
succeeded while retaining and executing a local edit. The fixture exited
successfully in both cases. ReleaseKit's normal delivery guards check
the tagged checkout after the executable has already been built and
started; they do not protect this bootstrap.
Fix: Include the fetch and checkout in the fail-fast subshell. Resolve the literal tag to its commit and require a clean checkout before downloading dependencies or building the CLI, as the source delivery template does. Preserve the scoped host target, dependency download before the offline build, and the existing retry arguments. Verify that checkout failure and carried-over edits both stop before the first Go command.