diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..317d202 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,130 @@ +# ReleaseKit Adoption Review: Findings and Recommended Changes + +**Reviewed:** 2026-10-07, source commit +`3c405f0524ec6c098d00d4e371abbace7648d446`. +**Scope:** ReleaseKit adoption commits after `721365e`, including CLI version +reporting, local release wiring, CI delivery, tests, and documentation. +**Status:** Findings pending remediation; no implementation changes made by this +review. + +## Architecture Assessment + +The core adoption matches [ADR 0001](../adr/0001-releasekit.md) and the +[feature roadmap](releasekit.md). Audita supplies product facts in +`releasekit.json`, a thin executable wrapper, a configuration-free version +command using the public `buildinfo` accessor, and a pinned Woodpecker +`ci-deliver` invocation. ReleaseKit owns candidate validation, annotated-tag +publication, version injection, Linux builds, checksums, and hosted delivery. +The custom builder and local publication mechanics have been retired. + +No core ownership or delivery-target gap was found. The findings concern one +CLI behavior defect, an ineffective target-selection assertion, and unnecessary +test restrictions. Main-first publication and the native-host version smoke +check are deliberately accepted workflow changes, not accidental strengthening. + +## R1 — P2: Simplify Version Help Handling and Correct Its Streams + +**Category:** CLI behavior defect and unnecessary argument restriction. + +**Evidence:** [runVersion](../../internal/cli/run.go) handles standalone `-h` +and `--help` before parsing, then rejects either spelling when combined with +other arguments. This adds a special prohibition beyond the requested usual +help behavior. For example, `audita version -h --help` returns status `2`. + +The parser's help path also writes usage twice: `FlagSet.Usage` writes to stderr, +and the `flag.ErrHelp` branch writes it again to stdout. Reproduced with +`audita version --h` and `audita version -help`: both return status `0` and emit +the same usage on both streams. The standard library recognizes these help +spellings, but the preliminary special cases do not handle them. + +**Recommended change:** Use one standard parsing path. Let help short-circuit +according to the parser's ordinary behavior, and emit successful help once to +stdout with empty stderr. Keep non-help unknown flags and unexpected positional +arguments as usage errors. Remove the bespoke mixed-help prohibition and the +test that promotes it into a requirement. Update the CLI wording if needed so +it describes ordinary help handling without promising validation of trailing +arguments after a help request. + +**Acceptance:** Normal version output remains configuration-free and unchanged. +Representative parser help paths produce status `0`, stdout usage, and empty +stderr. Non-help invalid invocations still return status `2`. Tests protect +streams and meaningful behavior without establishing a new argument-combination +policy or a comprehensive help-alias contract. + +## R2 — P2: Make the Delivery-Target Check Compare Actual Membership + +**Category:** Test correctness gap. + +**Evidence:** [sameMembers](../../scripts/release_test.go) compares slice +lengths and checks only that each observed item belongs to the expected slice. +It never checks that every required target is present. Calling the actual helper +with `got = [linux/amd64, linux/amd64]` and +`want = [linux/amd64, linux/arm64]` returns true. A profile missing arm64 can +therefore pass the selected-target assertion. ReleaseKit deduplicates repeated +targets, so that configuration would build only amd64. + +The reverse problem also occurs: a list containing both required targets plus +a repeated amd64 entry returns false despite having the same effective target +membership. The current committed profile is correct; the defect is in its +regression protection. + +**Recommended change:** Compare the sets of configured and required targets, +including coverage of every required member and rejection of an unintended +member. Reuse straightforward standard-library data structures. Duplicate +normalization already belongs to ReleaseKit; Audita needs no duplicate ban or +order requirement. + +**Acceptance:** A repeated single target cannot conceal an omitted production +target. Either omitted required target and an additional unintended target fail +the profile check. Reordered or repeated entries with the same effective target +set remain equivalent. Focused coverage catches the demonstrated false positive. + +## R3 — P3: Remove Incidental Wiring-Test Requirements + +**Category:** Tests strengthen representation choices into contracts. + +**Evidence:** [release wrapper/profile tests](../../scripts/release_test.go) +require the exact output argument spelling `-o ./bin/releasekit` and reject the +presence of a `checks` key regardless of its value. In disposable copies of the +tests, changing the output path to the equivalent `bin/releasekit` fails the +wrapper assertion even though execution still reaches the intended tool. +Adding `"checks": []` fails the profile assertion despite configuring no +additional commands. + +The accepted requirements concern the executable destination, native argument +and status forwarding, and avoiding redundant additional checks. They do not +require one path spelling or one serialization of an empty list. The plan's +instruction to omit `checks` selects an initial file representation rather than +an enduring prohibition on equivalent configuration. + +**Recommended change:** Check the resolved output destination and observed +wrapper behavior instead of the literal relative-path spelling. Check that the +effective additional-check list is empty, accepting omission or an empty list. +Keep the existing product, target, destination, argument-boundary, failure, and +exit-status protection. Simplify fixture helpers where that reduces maintenance +without adding another parser or test framework. + +**Acceptance:** The two demonstrated equivalent edits preserve passing wiring +tests, while a wrong executable destination, changed product facts, nonempty +additional checks, lost argument boundaries, or lost tool status still fail. +The tests remain offline and do not duplicate ReleaseKit's release-mechanics +matrix. + +## Review Verification and Limits + +- `go test ./...` passed on the reviewed source. +- A normal application build succeeded. Direct executable invocations reproduced + the version-help stream defect and mixed-help rejection. +- The actual membership helper reproduced the missing-target false positive. + Disposable test copies reproduced both equivalent-configuration failures; + the repository's code and tests were left unchanged. +- The real ReleaseKit builder produced both configured Linux artifacts with + build-test identifier `v0.0.0` and passed its injected-version smoke check. + This used the available Go 1.26.8 Linux amd64 toolchain; CI remains configured + for Go 1.25.5. Temporary build artifacts were removed. +- Module selection, source attribution, product profile, secret mapping, and + CI command match the intended integration. Live Woodpecker/Gitea delivery + remains unobserved, as already recorded in the feature roadmap; it is separate + deployment work rather than an implementation finding or source-release gate. +- No tags, pushes, hosted uploads, or LLM calls were made. Remediation should + remain bounded to the findings above and add no new release gates.