125 lines
6.5 KiB
Markdown
125 lines
6.5 KiB
Markdown
# ReleaseKit adoption review findings
|
||
|
||
Status: two remediation items pending; implementation not changed by this review.
|
||
|
||
Date: 2026-10-08.
|
||
|
||
## Review scope and authority
|
||
|
||
Reviewed CommandKit at `c0c03285f7beac3e96f1708b655c1eb2add35e79`, covering the
|
||
six adoption commits from `ada2021` through `c0c0328` against the
|
||
[ReleaseKit target](releasekit.md), [implementation plan](implementation.md),
|
||
and applicable policies. The previously completed documentation reconciliation
|
||
record remains available in
|
||
[Git history](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/c0c03285f7beac3e96f1708b655c1eb2add35e79/docs/roadmap/remediation.md).
|
||
|
||
The target adoption is present: a released Go tool selection, minimal source-only
|
||
profile, native launcher, fast subprocess tests, opt-in local publication
|
||
exercise, and implemented runbook/navigation. The commits do not change
|
||
production Go source or public declarations. Package dependency inspection found
|
||
no ReleaseKit, Goldmark, or `golang.org/x/mod` runtime dependency in the root
|
||
library or internal Cobra adapter. `scripts` contains external test files only.
|
||
|
||
The remaining findings concern test maintainability and ambiguous normative
|
||
wording. Neither establishes a missing production feature or a demonstrated
|
||
publication failure. Fix them within the existing target; do not add release
|
||
gates, new public contracts, or additional publisher machinery.
|
||
|
||
## R-01: avoid making bootstrap command details test contracts
|
||
|
||
**Priority:** low. **Category:** code quality and unintended test constraints.
|
||
|
||
**Location:** [scripts/release_test.go](../../scripts/release_test.go),
|
||
`TestReleaseLauncherDelegatesPublication`, `assertNativeBootstrap`
|
||
(lines 222–260), and `fakeGo` (lines 363–397).
|
||
|
||
The assertion compares the entire build argument vector, including flag order
|
||
and the absolute spelling of the output path. It also requires the two exact
|
||
`env GOHOSTOS` and `env GOHOSTARCH` query forms. The executable double supplies
|
||
only the first requested environment value, so a combined query would fail even
|
||
if the launcher parsed both host values correctly.
|
||
|
||
The publication assertion similarly compares a complete argument vector,
|
||
requiring `--repo` to precede `--config` even though the upstream CLI accepts
|
||
either order before the version operand.
|
||
|
||
For example, reordering build flags, using a relative output path resolved from
|
||
the already verified repository root, or combining host queries can preserve
|
||
the selected behavior while failing these tests. The target requires native
|
||
bootstrap from the module-selected package, readonly metadata, the selected
|
||
toolchain/workspace settings, and the intended executable destination. It does
|
||
not make these equivalent command spellings permanent contracts.
|
||
|
||
The [testing policy](../policy/testing.md#test-behavior-not-implementation) and
|
||
[agent policy](../policy/agents.md#implementation-discipline) call for protecting
|
||
those selected behaviors without elevating implementation details.
|
||
|
||
Required work:
|
||
|
||
- [ ] Assert the required build settings, selected package, and resolved output
|
||
destination without requiring one incidental flag ordering or path spelling.
|
||
- [ ] Make host-query fixture handling and assertions reflect the requested
|
||
host values rather than requiring separate subprocess query forms. Keep the
|
||
fixture small; a complete Go command emulator is unnecessary.
|
||
- [ ] Check the publication command, option values, version operand, and
|
||
argument boundaries without fixing the order of independent options. Retain
|
||
the upstream requirement that options precede the version.
|
||
- [ ] Preserve checks for wrong native target, missing readonly mode, incorrect
|
||
package/output, workspace or toolchain mistakes, failed bootstrap, and
|
||
publication status/stream propagation.
|
||
|
||
Completion evidence: focused launcher checks still detect the selected failures
|
||
and accept an equivalent bootstrap form. This is a test correction, not a
|
||
requirement to refactor the working launcher or add a release-time check.
|
||
|
||
## R-02: clarify optional diagnostic validation in the runbook
|
||
|
||
**Priority:** low. **Category:** potentially strengthened prohibition.
|
||
|
||
**Location:** [docs/release.md](../release.md), lines 76–77.
|
||
|
||
The instruction “Do not require a separate `check` before `publish` or repeat
|
||
the same suite” combines the intended rule against mandatory duplicate
|
||
validation with wording that can be read as prohibiting an operator from
|
||
voluntarily rerunning diagnostics.
|
||
|
||
The [release policy](../policy/release.md#candidate-validation) permits a
|
||
validation-only diagnostic command and says it must not be a required
|
||
prerequisite before publication repeats the checks. Avoiding automatic duplicate
|
||
validation does not establish a blanket prohibition on optional diagnostic runs.
|
||
The launcher correctly delegates once; this finding concerns the runbook's
|
||
description, not its executable behavior.
|
||
|
||
Required work:
|
||
|
||
- [ ] Say explicitly that `check` is optional for diagnosis and is not a
|
||
required step before `publish`.
|
||
- [ ] Scope the duplicate-validation instruction to the automated publication
|
||
workflow and configured additional checks. Preserve operator discretion to
|
||
investigate or rerun a check.
|
||
- [ ] Retain the single publication command and its own final-candidate suite;
|
||
do not add a required preliminary check or an extra confirmation.
|
||
|
||
Completion evidence: document review confirms the runbook distinguishes an
|
||
optional diagnostic action from a release requirement and agrees with the
|
||
existing policy. No implementation change is needed for this item.
|
||
|
||
## Review evidence and limits
|
||
|
||
- Read the adoption diffs, launcher/profile, new test functions and fixtures,
|
||
runbook, policy edits, historical navigation changes, and completion records.
|
||
- Inspected package dependencies with offline module access and confirmed the
|
||
production boundary and tests-only `scripts` package. Verified the committed
|
||
launcher mode and ignored local executable storage.
|
||
- Reviewed what the tests actually assert, including the local fixture's exact
|
||
tag-object comparison, candidate commit, one suite marker, clean worktree, and
|
||
endpoint-mismatch preflight behavior. The completion record reports earlier
|
||
successful default and opt-in runs; this review did not rerun those tests or
|
||
attempt publication.
|
||
- No other adoption gap, newly imposed release gate, public API expansion, or
|
||
runtime publication-code defect was identified in the reviewed changes.
|
||
|
||
Leave source publication, hosted delivery, binary assets, and other explicitly
|
||
deferred capabilities outside this remediation work. Update item status only
|
||
after the relevant correction and evidence are complete.
|