85 lines
4.6 KiB
Markdown
85 lines
4.6 KiB
Markdown
# Release Workflow Review Findings
|
||
|
||
Status: both findings addressed. The original findings below are retained as
|
||
review history until normal release-preparation retirement.
|
||
|
||
## Resolution and verification
|
||
|
||
- Publication, fetch, and recovery now use the same validated named `origin`.
|
||
Pushes retain exact refspecs and disabled implicit tags, and override the
|
||
remote's mirror default for that invocation only. No new gate was introduced.
|
||
- A disposable-Git regression failed against the original publisher and passes
|
||
with the fix for normal publication and lost main/tag push responses. It also
|
||
checks that broad push defaults do not publish or delete unrelated refs and
|
||
that the rewrite destination receives no refs.
|
||
- Optional consumer verification explicitly initializes the selected version.
|
||
Executing the documented block with a local Go stub verified the exact module
|
||
argument with both an initially unset and an initially stale version variable,
|
||
without network retrieval.
|
||
- Focused publication tests passed. Final validation passed: Bash syntax,
|
||
whitespace review, and the Go 1.25.14 release checker (race suite including
|
||
documentation links, full vet, and both Linux target builds).
|
||
|
||
Reviewed implementation through `3d0093e` against the release roadmap and
|
||
completed implementation plan. The ordinary Go 1.25.14 release checker passed
|
||
(offline race suite, full vet, Linux amd64/arm64 builds), as did Bash syntax
|
||
checks. These findings concern publication destination integrity and a runbook
|
||
example; no library changes or additional release gates are needed.
|
||
|
||
## 1. Align destination validation with actual push resolution (high priority)
|
||
|
||
**Locations:** `scripts/release.sh:56–59`, `:91`, and `:116`;
|
||
regression coverage in `release_publish_test.go`.
|
||
|
||
The guard inspects `git remote get-url --push --all origin`, but both pushes use
|
||
the literal URL instead of the inspected remote. Git treats these differently
|
||
when `origin` has an explicit push URL and a matching `url.<other>.pushInsteadOf`
|
||
rule. The explicit remote push URL bypasses that rule for the guard; passing the
|
||
literal URL to `git push` applies the rule.
|
||
|
||
Reproduced with two disposable bare repositories A (authoritative) and B:
|
||
|
||
1. Set origin's fetch URL and explicit push URL to A.
|
||
2. Configure `url.<B>.pushInsteadOf` to match A.
|
||
3. Prepare main and a matching release note. Run a fixture copy of the actual
|
||
publisher with A substituted for the authoritative URL and a successful
|
||
checker stub, as in the existing publication fixtures.
|
||
|
||
Both guard queries return A. The publisher nevertheless pushes main and the
|
||
annotated tag to B and reports `Published`; A still has the old main and no
|
||
release tag. Recovery queries also inspect A rather than the push destination.
|
||
This violates the already-selected authoritative-destination and completion
|
||
contract; it is not a request for broader Git hardening.
|
||
|
||
**Required correction:** make guard, push, and recovery destination resolution
|
||
agree. Use a consistent validated target rather than assuming a literal URL
|
||
bypasses Git URL rewriting. Preserve explicit refspecs, non-force publication,
|
||
and suppression of unrelated tags/configured broad push behavior. Do not add
|
||
successful-push read-back, blanket Git configuration bans, or new human gates.
|
||
|
||
**Acceptance:** add a real local-Git regression for the configuration above.
|
||
Publication must either use the confirmed authoritative destination or refuse
|
||
before any publication; B must receive no refs. Preserve existing ordinary
|
||
success, multiple-destination refusal, and ambiguous-push recovery coverage.
|
||
|
||
## 2. Initialize the version in optional consumer verification (low priority)
|
||
|
||
**Location:** `docs/release.md:177–194`.
|
||
|
||
The optional `go get` example expands `$release_version`, but the successful
|
||
publication example passes a literal version to the script and never initializes
|
||
that shell variable. Its only assignment is in the failure-recovery section,
|
||
which successful releases do not need to execute. The optional example therefore
|
||
uses an unset variable (or a stale version from the caller's shell) instead of
|
||
the release just published.
|
||
|
||
**Required correction:** explicitly set the selected version in the optional
|
||
example, or consistently establish it in the normal preparation/publication
|
||
instructions and explain that prerequisite. Keep verification optional and
|
||
non-gating.
|
||
|
||
**Acceptance:** follow the successful path in a fresh shell without running
|
||
recovery instructions and confirm that the optional example constructs the
|
||
intended exact module/version argument. A local stub is sufficient; no production
|
||
module retrieval or new recurring test/gate is needed.
|