test(release): gate minor API compatibility - #88
ben-ranford wants to merge 29 commits into
Conversation
ben-ranford
added
area:core
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e8773b96d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate compatibility-gate issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a source-based API compatibility gate against annotated stable v1 tags, including archive handling, inventory comparison, fixture tests, release integration, and documentation.
Changes:
- Adds the
releasebaselinecommand and alternate-directory inventory support. - Adds release-baseline Make targets and GA integration.
- Documents compatibility rules, baseline behavior, and caveats.
File summaries
| File | Summary and final findings |
|---|---|
scripts/rigor/generated/dependency-inventory.json |
Registers the new command package. |
scripts/rigor/cmd/rigor/main.go |
Supports inventory generation from an alternate directory. |
scripts/rigor/cmd/releasebaseline/main.go |
Implements baseline selection, archive extraction, and API comparison. Critical (2 votes): empty public packages can be deleted without detection. Moderate (2 votes): build-metadata stable tags are rejected. Moderate (1 vote each): raw signatures flag parameter renames or interface reordering; inline go comments produce an unknown floor; lightweight tags can block valid baseline selection; the child process ignores the selected GO; fixture compilation is not wired into the command. |
scripts/rigor/cmd/releasebaseline/main_test.go |
Tests compatibility, fixtures, tag selection, and archive paths. |
Makefile |
Adds release-baseline targets and GA integration. Critical (1 vote): the common non-prerelease gate is not version-aware and can force v2 releases to use a v1 baseline. |
docs/releasing.md |
Documents GA baseline gating. |
docs/compatibility.md |
Documents compatibility rules and caveats. |
Review details
Suppressed comments (7)
scripts/rigor/cmd/releasebaseline/main.go:254
- The compatibility decision is based on exact inventory strings, but the renderer includes non-semantic details such as parameter names and preserves interface-member order. Renaming
valuetoinput, or merely reordering interface methods, leaves Go consumer code type-correct yet is reported as a breaking change here. Compare normalized Go signatures/interface members rather than raw declaration text.
if declaration == current || compatibleStructFieldAddition(declaration, current) {
continue
}
failures = append(failures, pkg+": changed "+declaration+" -> "+current)
scripts/rigor/cmd/releasebaseline/main.go:118
- The development-path validation only checks the
v1.prefix and presence of a hyphen, so a tag such asv1.not-semver-foois accepted whenever an annotated tag with that name exists. That contradicts the error's v1-semver contract and allows an arbitrary non-release tag to become the development baseline; validate the complete prerelease SemVer grammar (and require a prerelease in development mode).
if !strings.HasPrefix(tag, "v1.") || (!stableV1Tag.MatchString(tag) && !strings.Contains(tag, "-")) {
return baseline{}, fmt.Errorf("baseline tag %q is not a v1 semver tag", tag)
scripts/rigor/cmd/releasebaseline/main.go:323
- For an empty public struct,
strings.Split("", "; ")yields one empty field. Consequently, adding the first exported field totype Empty struct { }is treated as an incompatible change, even though keyed consumers remain compatible and the documented additive-field rule says it should pass.
body := strings.TrimSuffix(strings.SplitN(declaration, structMarker, 2)[1], " }")
fields := map[string]bool{}
for _, field := range strings.Split(body, "; ") {
scripts/rigor/cmd/releasebaseline/main.go:233
- A valid
go.moddirective such asgo 1.22 // minimum supported Gohas more than two fields, so this returnsunknown; the command still reportsstatus=compatiblewithout recording the actual Go floor. Parse the directive after removing an inline comment (and consider failing when the floor cannot be read).
for _, line := range strings.Split(string(data), "\n") {
fields := strings.Fields(line)
if len(fields) == 2 && fields[0] == "go" {
return fields[1]
scripts/rigor/cmd/releasebaseline/main.go:105
latestStableV1Tagreturns the first semver-matching tag before checking whether it is annotated. If a newer stablev1.x.ytag is lightweight,loadBaselinerejects it instead of skipping it and selecting the next annotated ancestor, so a valid baseline can be blocked. Filter out non-tagobjects while selecting the baseline (while retaining the final validation).
commit, err := git(ctx, gitRevParse, tag+"^{commit}")
if err != nil {
return "", err
}
if strings.TrimSpace(string(commit)) != strings.TrimSpace(string(head)) {
scripts/rigor/cmd/releasebaseline/main.go:216
- This child process hardcodes
go, even though the Makefile exposesGOand invokes the release-baseline command with$(GO). WithGO=go1.22or a custom toolchain, the outer command and the inventory generator can use different Go versions, so the comparison is not reproducible under the selected toolchain. Pass the selected executable through to the child process.
command := exec.CommandContext(ctx, "go", "run", "./scripts/rigor/cmd/rigor", "public-api", "--dir", directory)
scripts/rigor/cmd/releasebaseline/main.go:241
runonly invokescompareInventories; the compiler matrix inmain_test.gois never called by the command. Thusmake release-baselinestill performs only a textual inventory comparison and does not execute the advertised source-level consumer fixtures, leaving any inventory-rendering gap undetected by the release gate. Wire fixture compilation into the gate or narrow the stated acceptance scope.
func compareInventories(baselineInventory, candidateInventory string) error {
baseline := parseInventory(baselineInventory)
candidate := parseInventory(candidateInventory)
- Files reviewed: 6/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5c647cc49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cb38b8fc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcbc796a89
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9a1b4a686
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a9a1b4a to
5598709
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25867e881d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #88 returned:
Audited via CLI at 2026-09-14T15:24:58.083101+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a05494ec0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #88 returned:
Audited via CLI at 2026-09-14T15:37:53.224919+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3927cbffd1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #88 returned:
Audited via CLI at 2026-09-14T15:47:04.229797+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c72f85e77
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #88 returned:
Audited via CLI at 2026-09-14T16:04:09.513705+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4372762e7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Sonar API auditReviewed commit: Live public SonarCloud API queries for PR #88 returned:
Audited via CLI at 2026-09-14T16:40:15.732488+00:00. Project visibility was verified as public. No findings were changed or suppressed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eae4e395dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| receiverEnd := strings.Index(rest, ") ") | ||
| if receiverEnd >= 0 { | ||
| name := strings.SplitN(rest[receiverEnd+2:], "(", 2)[0] | ||
| return methodPrefix + rest[:receiverEnd+1] + " " + name |
There was a problem hiding this comment.
When an exported method changes from func (*T) M(...) to func (T) M(...), existing calls and method expressions through *T still compile, and any interfaces previously implemented by *T remain implemented. Because this key includes the receiver's *, the candidate method receives a different key and the baseline gate reports the old method as removed, blocking this source-compatible widening; match the two receiver forms and allow only the pointer-to-value direction when the method signature is otherwise unchanged.
Useful? React with 👍 / 👎.
Summary
v1.0.0-rc.2comparison is development evidence only. Added fields remain accepted for keyed literals of defined structs, with unkeyed-literal and prior-struct-shape conversion caveats documented; those consumers require explicit compatibility review. Anonymous struct aliases do not receive this additive-field exception because their type identity changes.Validation
go test ./scripts/rigor/cmd/rigor ./scripts/rigor/cmd/releasebaseline -count=1make release-baseline-developmentmake release-baselinefails explicitly because onlyv1.0.0-rc.1andv1.0.0-rc.2existmake ciThe development comparison recorded
v1.0.0-rc.2at64b0d0d96954a64d51d3c520211f2164fdacab18and a1.22Go floor for both baseline and candidate.Archive extraction separates temporary-directory ownership from entry processing. Cleaned local paths must remain below the extraction root; links and special entries are rejected. Real tar fixtures verify valid nested files and reject traversal, absolute paths, links, devices, and FIFO entries without writing outside the root. Git resolves to an absolute executable path while preserving custom installations. Shared API-test fixture setup preserves all source fixtures and assertions.
Scope firewall
This PR is limited to issue #58’s release-baseline inventory comparator, source archive handling, compatibility fixtures, and documentation. It preserves the Go floor, selected toolchain, existing API inventory format, and stable-tag prerequisite. Replacing the CI trust model or publishing the prerequisite release is outside this implementation.
The checker is a bounded declaration and consumer-fixture gate, not a complete Go source-equivalence analyzer. The supported release matrix compares the inherited native build and all four cgo-disabled public targets. Supported non-generic sealed methods require full value/pointer interface satisfaction; generic matching remains limited.
The compatibility guide records separate backlog work for external module resolution and reachable dependency types (#75, #122), generic and alias normalization (#107, #110), cgo importing (#109), root-local and hidden-type reachability (#112, #113, #119), promoted-selector compatibility (#114), equivalent interface embedding normalization (#123), and prior-struct-shape conversion policy (#124). These follow-ups do not waive defects in the issue's supported declaration, field, archive or target-selection checks. The external dependency fixture currently fails closed at the resolver; no silent-pass reproduction is claimed for it.
Current runtime
3a05494passed full CI and 45 compiled CLI QA cases, including the real Makefile's native/cgo-disabled target modes. The later documentation-only head passed fresh candidate requirements; its remote checks and exact-head review are evaluated separately.Release Notes
Closes #58