Record follow-up audit findings
This commit is contained in:
134
docs/roadmap/remediation.md
Normal file
134
docs/roadmap/remediation.md
Normal file
@@ -0,0 +1,134 @@
|
||||
# ReleaseKit audit follow-up review
|
||||
|
||||
Date: 2026-10-06.
|
||||
Reviewed source: `55333b0`.
|
||||
Status: Two findings require remediation.
|
||||
|
||||
## Scope and assessment
|
||||
|
||||
Reviewed the changes since `536a579` against the [audit](audit.md), completed
|
||||
[implementation plan](implementation.md), [architecture policy](../policy/architecture.md),
|
||||
and [routine-release decision](../adr/0002-routine-releases.md). The worktree
|
||||
was clean at the start of the review.
|
||||
|
||||
The selected architecture is substantially implemented. One Go module exposes
|
||||
the root orchestration facade and lightweight `buildinfo` runtime facade.
|
||||
Internal packages own candidate validation, Git publication and recovery,
|
||||
Go environments, artifact declarations, managed delivery builds, Gitea effects,
|
||||
and explicit CI preparation. The CLI adapts profiles, credentials, output, and
|
||||
exit statuses. Standard validation precedes additive product checks; ordinary
|
||||
delivery uses one facade-owned build-and-deliver operation. The maintained
|
||||
consumer workflow invokes the shared versioned CI command.
|
||||
|
||||
The audit's F1–F8 changes are present, with behavioral tests for their selected
|
||||
contracts. R1 below identifies a new defect in the shared Go-environment owner;
|
||||
R2 identifies increased test cost and coupling introduced by additive validation.
|
||||
The review found no additional public-type ownership or dependency-direction
|
||||
gap. The deliberate additive-check, baseline-build, managed-delivery, and CI
|
||||
preparation decisions remain appropriate. The findings call for implementation
|
||||
repairs within those decisions.
|
||||
|
||||
## R1 — Medium: Go discovery treats successful diagnostics as invalid configuration
|
||||
|
||||
Status: Open.
|
||||
Category: Correctness and an accidental environment prerequisite.
|
||||
Introduced by: `0809312`.
|
||||
|
||||
**Evidence:** [Go configuration discovery](../../internal/gotool/gotool.go)
|
||||
routes both stdout and stderr into the same buffer in `discover` (lines
|
||||
189–205). `New` parses that combined output as JSON (lines 59–81). A successful
|
||||
`go env -json` invocation with a warning therefore becomes a decoding failure.
|
||||
This affects candidate validation and nonempty builds through their shared owner.
|
||||
|
||||
**Reproduction:** Create a clean `main` repository with a Go 1.25 module, a
|
||||
passing test, a matching committed note, and a source-only product profile.
|
||||
Set `GOPATH` to the installed Go toolchain's `GOROOT`. On Go 1.26.8,
|
||||
`go test -mod=readonly -count=1 ./...` succeeds with a warning on stderr.
|
||||
`releasekit check --repo DIR v1.2.3` fails before running that suite:
|
||||
|
||||
```text
|
||||
prepare Go toolchain: decode Go host settings: invalid character 'w' looking for beginning of value
|
||||
```
|
||||
|
||||
The warning-producing configuration is discouraged by Go, but the Go command
|
||||
accepts it. ReleaseKit's selected environment contract preserves effective
|
||||
operational settings; it establishes no requirement that successful discovery
|
||||
produce an empty diagnostic stream. The same decoding failure follows from any
|
||||
successful discovery command that emits stderr alongside valid stdout JSON.
|
||||
|
||||
**Required remediation:** Capture configuration stdout and diagnostics
|
||||
separately. Parse stdout as JSON and retain stderr for command-failure reporting
|
||||
or ordinary diagnostics. Preserve subprocess error identity, cancellation,
|
||||
effective operational settings, and the selected validation/build environments.
|
||||
|
||||
**Focused verification:** A controlled Go-command fixture returns valid JSON on
|
||||
stdout, a diagnostic on stderr, and exit 0; discovery succeeds and preserves
|
||||
the settings. A failing command retains useful stderr and its cause. Existing
|
||||
ambient/persistent flag, host-target, cache, and architecture-tuning regressions
|
||||
continue to pass. These checks add no warning-free configuration requirement.
|
||||
|
||||
## R2 — Medium: Wrapper regression testing repeats the project suite inside itself
|
||||
|
||||
Status: Open.
|
||||
Category: Test maintainability and development cost.
|
||||
Introduced by: `3e9b5c1`.
|
||||
|
||||
**Evidence:** [The wrapper regression](../../cmd/releasekit/release_wrapper_test.go)
|
||||
clones the complete repository for each exit-status scenario, overlays a
|
||||
hand-maintained list of production and test files (lines 32–45), and invokes
|
||||
the source publication wrapper. The success, rejected-push, and unknown-push
|
||||
scenarios each run the project's default suite inside the parent suite.
|
||||
An inherited `RELEASEKIT_SKIP_WRAPPER_TEST` flag prevents recursive execution
|
||||
of this one test, while the other publication, delivery, build, and workflow
|
||||
tests run again in each clone.
|
||||
|
||||
During this review, process inspection confirmed the nested native publisher,
|
||||
`go test ./...`, and cloned test binaries. The uncached offline suite passed;
|
||||
the root package took 40.751 seconds and the CLI package took 115.533 seconds
|
||||
on this host. These timings are observations, not proposed test-duration limits.
|
||||
|
||||
The extra executions provide little distinct evidence about wrapper exit-code
|
||||
preservation. The file overlay also couples the fixture to unrelated package
|
||||
changes and supplies only selected uncommitted files, so development checks can
|
||||
exercise a mixture of the current worktree and committed source.
|
||||
|
||||
**Required remediation:** Keep the real source-bootstrap wrapper and local Git
|
||||
exit-status scenarios, while giving their candidate validation a small,
|
||||
self-contained test fixture. Assemble the tool source consistently from the
|
||||
version being tested. Replace the manually maintained production/test overlay
|
||||
and suite-recursion skip mechanism with a fixture that exercises wrapper
|
||||
behavior without rerunning ReleaseKit's complete test suite. Retain the actual
|
||||
publication and recovery behavior already protected by this regression.
|
||||
|
||||
**Focused verification:** The real wrapper still preserves success 0, known
|
||||
failure 1, invalid input 2, and uncertain publication 3. The candidate fixture
|
||||
runs its standard suite, and captured-endpoint recovery still finds the
|
||||
intended object with separate fetch and push repositories. The outer default
|
||||
suite supplies the project's broader behavioral evidence once. Use test scope
|
||||
and command observations for verification rather than a wall-clock gate.
|
||||
|
||||
## Validation and remaining scope
|
||||
|
||||
The following checks passed on the reviewed source with Go 1.26.8 on
|
||||
Linux/amd64:
|
||||
|
||||
```sh
|
||||
GOWORK=off GOPROXY=off GONOPROXY=none GOSUMDB=off GOTOOLCHAIN=local \
|
||||
go test -mod=readonly -count=1 ./...
|
||||
GOWORK=off GOPROXY=off GOTOOLCHAIN=local go run ./internal/apicheck
|
||||
```
|
||||
|
||||
The R1 reproduction used a temporary committed consumer repository and a native
|
||||
CLI built from the reviewed source. Publication and delivery suite checks used
|
||||
local Git and HTTP fixtures. No consuming repository was changed, release was
|
||||
published, deployment was activated, or live hosted delivery was performed.
|
||||
This review did not rerun minimum-Go-1.25 validation; the completed implementation
|
||||
plan records that earlier verification. Real installation, consumer migration,
|
||||
credentials, and live CI/Gitea verification remain adoption/deployment work in
|
||||
the [product roadmap](releasekit.md).
|
||||
|
||||
R1's implicit warning-free discovery prerequisite is the additional restriction
|
||||
identified by this review. The review found no further strengthened guarantee
|
||||
requiring remediation. Preserve existing published-content immutability and the
|
||||
ordinary source-eligibility observations; the findings require no reproducible
|
||||
build promise, stronger concurrency guarantee, or additional release gate.
|
||||
Reference in New Issue
Block a user