fix: anchor clone-vs-proxy verification to the same commit #29

Merged

heiko merged 1 commit from ai-claude-fix-proxy into master

2026-05-02 09:02:51 +02:00

heiko commented

2026-05-02 08:47:42 +02:00

Owner

Summary

Fixes a race condition in executeFromClone where the Go module proxy could resolve a branch-name ref to a different commit than the one just cloned. This caused VerifyClone to compare hashes that were never meant to match.

For a stable semver tag (v1.2.3) the tag name is still passed to the proxy directly — so the release gets the canonical version string.
For branch names, HEAD, and pseudo-versions the proxy is now queried with the commit SHA returned by CloneRepo, anchoring both sides to the same commit.

Drive-bys:

CloneRepo now returns the resolved HEAD SHA, removing a redundant git.Open in the caller.
VerifyClone accepts the already-fetched *ModDownloadInfo instead of making a second proxy round-trip.

Issue #28 tracks the adjacent problem (proxy not yet having a freshly pushed tag) which is not fixed here.

Test plan

go build ./... clean
go test ./... all pass
golangci-lint run no issues

## Summary Fixes a race condition in `executeFromClone` where the Go module proxy could resolve a branch-name ref to a **different commit** than the one just cloned. This caused `VerifyClone` to compare hashes that were never meant to match. - For a stable semver tag (`v1.2.3`) the tag name is still passed to the proxy directly — so the release gets the canonical version string. - For branch names, `HEAD`, and pseudo-versions the proxy is now queried with the commit SHA returned by `CloneRepo`, anchoring both sides to the same commit. Drive-bys: - `CloneRepo` now returns the resolved HEAD SHA, removing a redundant `git.Open` in the caller. - `VerifyClone` accepts the already-fetched `*ModDownloadInfo` instead of making a second proxy round-trip. ## Related - Issue #28 tracks the adjacent problem (proxy not yet having a freshly pushed tag) which is **not** fixed here. ## Test plan - [x] `go build ./...` clean - [x] `go test ./...` all pass - [x] `golangci-lint run` no issues

heiko added 1 commit

2026-05-02 08:47:42 +02:00

fix: anchor clone-vs-proxy verification to the same commit 9dbb68f3b0

In executeFromClone the proxy query was keyed on the user-supplied ref
(e.g. a branch name like "main"). The proxy could resolve that branch to
a different commit than the one git just cloned — for example when the
proxy serves a stale cache or someone pushes between the two calls — and
VerifyClone would then compare hashes that were never meant to line up.

For a stable semver tag the tag name is still passed through so the
release gets the canonical "v1.2.3" version string. For branch names,
"HEAD", and pseudo-versions the proxy is now queried with the commit
SHA read from the clone, so both sides are anchored to the same commit.

Drive-bys:
- CloneRepo returns the resolved HEAD SHA, so the caller does not need
  a second git.Open on the just-cloned directory.
- VerifyClone now takes the *ModDownloadInfo already fetched by the
  caller instead of doing its own ResolveModuleVersion round-trip.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

heiko commented

2026-05-02 08:56:48 +02:00

Author

Owner

[AI-Generated Review]

I have reviewed Pull Request #29. Here is my evaluation of the changes.

Summary of Changes

The PR addresses a race condition that occurs during the clone verification phase (VerifyClone). Previously, when a branch name (e.g., main) was provided as the release target, the codebase would check out the branch locally but query the Go module proxy using that same branch name. If the proxy served a cached, older commit (or if a new commit was pushed to the branch in the exact moment between the git clone and the proxy query), the cloned hash and the proxy hash would mismatch, causing the build to fail unnecessarily.

The Fix:

CloneRepo was modified to return the exact commit SHA of the checked-out clone.
The release command logic was updated to use this concrete commit SHA when querying the module proxy, except when dealing with stable semantic version tags (which are immutable).
Drive-by Refactors:
- VerifyClone no longer makes a redundant network call to ResolveModuleVersion. Instead, it now directly accepts the *ModDownloadInfo that the caller has already retrieved.
- The redundant ctx parameter in VerifyClone was removed because it no longer performs network requests.

Code Review

internal/builder/clone.go: The change to CloneRepo signature correctly propagates the SHA string. The introduction of readCloneHead provides a clean way to fetch the SHA without relying on standard output parsing of a shell command, utilizing the internal git.Open and repo.HeadCommit() mechanisms.
internal/builder/verify.go: The refactoring here is excellent. Passing *ModDownloadInfo directly avoids the second HTTP round-trip to the module proxy, making the verification slightly faster and reducing external dependency risks.
internal/cmd/release/release.go: The proxy ref determination is sound:
```
proxyRef := r.flag.commit
if !semver.IsValid(r.flag.commit) || modver.Parse(r.flag.commit).IsPseudo() {
        proxyRef = cloneHead
}
```
This correctly separates branches/pseudo-versions (which need exact SHA anchoring) from stable semver tags like v1.0.0 (which must be queried as-is so the proxy records and returns the canonical version name rather than a raw SHA).
internal/builder/verify_test.go: The test was correctly updated to align with the new VerifyClone signature.

Testing

I have run the test suite locally against the PR commit, and all packages passed successfully without issue.

Conclusion

This PR correctly identifies and elegantly fixes an annoying race condition that affects non-tag-based builds. The optimization in VerifyClone is a great drive-by improvement.

Status: Approved. The code is clean, idiomatic Go, and safe to merge.

**[AI-Generated Review]** I have reviewed Pull Request #29. Here is my evaluation of the changes. ### Summary of Changes The PR addresses a race condition that occurs during the clone verification phase (`VerifyClone`). Previously, when a branch name (e.g., `main`) was provided as the release target, the codebase would check out the branch locally but query the Go module proxy using that same branch name. If the proxy served a cached, older commit (or if a new commit was pushed to the branch in the exact moment between the git clone and the proxy query), the cloned hash and the proxy hash would mismatch, causing the build to fail unnecessarily. **The Fix:** - `CloneRepo` was modified to return the exact commit SHA of the checked-out clone. - The `release` command logic was updated to use this concrete commit SHA when querying the module proxy, except when dealing with stable semantic version tags (which are immutable). - **Drive-by Refactors:** - `VerifyClone` no longer makes a redundant network call to `ResolveModuleVersion`. Instead, it now directly accepts the `*ModDownloadInfo` that the caller has already retrieved. - The redundant `ctx` parameter in `VerifyClone` was removed because it no longer performs network requests. ### Code Review * **`internal/builder/clone.go`:** The change to `CloneRepo` signature correctly propagates the SHA string. The introduction of `readCloneHead` provides a clean way to fetch the SHA without relying on standard output parsing of a shell command, utilizing the internal `git.Open` and `repo.HeadCommit()` mechanisms. * **`internal/builder/verify.go`:** The refactoring here is excellent. Passing `*ModDownloadInfo` directly avoids the second HTTP round-trip to the module proxy, making the verification slightly faster and reducing external dependency risks. * **`internal/cmd/release/release.go`:** The proxy ref determination is sound: ```go proxyRef := r.flag.commit if !semver.IsValid(r.flag.commit) || modver.Parse(r.flag.commit).IsPseudo() { proxyRef = cloneHead } ``` This correctly separates branches/pseudo-versions (which need exact SHA anchoring) from stable semver tags like `v1.0.0` (which must be queried as-is so the proxy records and returns the canonical version name rather than a raw SHA). * **`internal/builder/verify_test.go`:** The test was correctly updated to align with the new `VerifyClone` signature. ### Testing I have run the test suite locally against the PR commit, and all packages passed successfully without issue. ### Conclusion This PR correctly identifies and elegantly fixes an annoying race condition that affects non-tag-based builds. The optimization in `VerifyClone` is a great drive-by improvement. **Status:** Approved. The code is clean, idiomatic Go, and safe to merge.

heiko merged commit 93408b1729 into master

2026-05-02 09:02:51 +02:00

heiko referenced this pull request from a commit

2026-05-02 09:02:53 +02:00

Merge pull request 'fix: anchor clone-vs-proxy verification to the same commit' (#29) from ai-claude-fix-proxy into master