Record the ReleaseKit adoption review findings
This commit is contained in:
130
docs/roadmap/remediation.md
Normal file
130
docs/roadmap/remediation.md
Normal file
@@ -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.
|
||||
Reference in New Issue
Block a user