Files
releasekit/docs/roadmap/remediation.md

12 KiB
Raw Blame History

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

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

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

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

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

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

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

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.