Files
audita/docs/roadmap/remediation.md

8.6 KiB
Raw Blame History

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: R1–R3 addressed. The findings below describe the reviewed baseline; completed work is recorded under Resolution Record.

Architecture Assessment

The core adoption matches ADR 0001 and the feature roadmap. 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 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 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 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.

Resolution Record

  • R1: The version command now uses one standard flag-parsing path. Successful help is written once to stdout, with no stderr usage, and help can short-circuit trailing arguments. Non-help usage errors retain status 2. Regression tests reproduced both defects before the fix; go test ./internal/cli ./cmd/audita passed afterward. The CLI contract reflects ordinary help behavior.
  • R2: Delivery targets are compared as sets using the standard library. Regression cases reject either missing Linux target concealed by duplicates and an unintended target, while accepting equivalent reordered/repeated lists. The cases reproduced the faulty membership behavior before the fix; go test ./scripts passed afterward.
  • R3: Wrapper coverage now checks the executable's actual destination, and profile coverage checks the effective additional-check list. Equivalent output paths and an explicit empty list are accepted. Fixture helpers use standard library operations where appropriate. Disposable mutation checks accepted -o bin/releasekit, -o=bin/releasekit, and "checks": [], while detecting a wrong executable destination, changed product, nonempty checks, lost argument boundaries, and lost tool status. The complete go test ./... suite passed with all three remediations in place. No new release gates were added.