feat(update): verify release metadata by target#97
Merged
Conversation
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
Need the big picture first? Review this PR in Change Stack to see what changed before going file by file. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an optional Changes--target flag for platform-specific update checks
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/update/update_test.go (1)
78-109: ⚡ Quick winAdd normalization and canonical-name assertions in
TestResolveTarget.This test only validates already-normalized inputs. Add cases with whitespace/casing variants and assert
target.Nameso resolver normalization behavior is locked down.Suggested test expansion
func TestResolveTarget(t *testing.T) { tests := []struct { - target string + input string + name string goos string goarch string platform string arch string }{ - {target: "linux-x64", goos: "linux", goarch: "amd64", platform: "linux", arch: "x64"}, + {input: "linux-x64", name: "linux-x64", goos: "linux", goarch: "amd64", platform: "linux", arch: "x64"}, + {input: " WINDOWS-X64 ", name: "windows-x64", goos: "windows", goarch: "amd64", platform: "windows", arch: "x64"}, ... } for _, tt := range tests { - t.Run(tt.target, func(t *testing.T) { - target, err := ResolveTarget(tt.target) + t.Run(tt.input, func(t *testing.T) { + target, err := ResolveTarget(tt.input) if err != nil { t.Fatalf("ResolveTarget returned error: %v", err) } - if target.GOOS != tt.goos || target.GOARCH != tt.goarch || target.Platform != tt.platform || target.Arch != tt.arch { - t.Fatalf("ResolveTarget(%q) = %#v", tt.target, target) + if target.Name != tt.name || target.GOOS != tt.goos || target.GOARCH != tt.goarch || target.Platform != tt.platform || target.Arch != tt.arch { + t.Fatalf("ResolveTarget(%q) = %#v", tt.input, target) } }) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/update/update_test.go` around lines 78 - 109, Add test cases to TestResolveTarget that exercise normalization by including inputs with extra whitespace and mixed-case (e.g., " Linux-X64 ", "MacOS-Arm64") and assert the returned ResolveTarget structure fields including target.Name equal the canonical name (e.g., "linux-x64", "macos-arm64") in addition to existing checks for GOOS, GOARCH, Platform and Arch; update the subtests to call ResolveTarget on these variant strings and fail the test if err != nil or if target.Name does not match the expected canonical name so the resolver’s normalization and canonical-name behavior is locked down.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/cli/update.go`:
- Around line 112-121: The parser currently accepts empty or whitespace-only
--target values and silently sets options.target to "", causing runUpdate to use
the current machine instead of the requested target; update the handling in the
case for "--target" (the branch that calls nextFlagValue) and the
strings.HasPrefix("--target=") branch to validate after trimming: if the
resulting options.target is empty, return an error (from the flag-parsing
function) indicating that --target requires a non-empty value so the caller
(runUpdate) never receives an empty target.
---
Nitpick comments:
In `@internal/update/update_test.go`:
- Around line 78-109: Add test cases to TestResolveTarget that exercise
normalization by including inputs with extra whitespace and mixed-case (e.g., "
Linux-X64 ", "MacOS-Arm64") and assert the returned ResolveTarget structure
fields including target.Name equal the canonical name (e.g., "linux-x64",
"macos-arm64") in addition to existing checks for GOOS, GOARCH, Platform and
Arch; update the subtests to call ResolveTarget on these variant strings and
fail the test if err != nil or if target.Name does not match the expected
canonical name so the resolver’s normalization and canonical-name behavior is
locked down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 371c82fe-f5a2-4ac9-a3d9-3f1d3dfc9fe5
📒 Files selected for processing (6)
README.mddocs/UPDATE.mdinternal/cli/app_test.gointernal/cli/update.gointernal/update/update.gointernal/update/update_test.go
Vasanthdev2004
requested changes
Jun 6, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Blockers
internal/cli/update.go:--target=is accepted as if no target was supplied, so the command silently validates the current machine's release asset instead of failing the malformed target flag. I reproduced this on the PR build with a Windows data endpoint:./zero update --check --target= --endpoint <windows data: endpoint> --jsonexited 0 and reportedzero-v0.2.0-windows-x64.zip. For release metadata validation this can give false confidence in CI or scripts when an empty target variable is passed; the inline--target=form should return a usage error just like the separated empty/flag-shaped value path.
Non-Blocking
- The target mapping itself looks aligned with the release archive naming contract (
linux/macos/windows+x64/arm64), and invalid explicit targets such assolaris-sparcfail closed with exit 2.
Looks Good
- CLI option passthrough correctly maps
--target windows-x64toGOOS=windows/GOARCH=amd64before callingupdate.Check. - Documentation covers the supported target names and preserves the default current-platform behavior when no target is supplied.
- Validation run locally:
go test -count=1 ./internal/update ./internal/cli -run 'TestResolveTarget|TestCheck|TestRunUpdate'go test -count=1 -p 1 ./...go run ./cmd/zero-release build./zero update --check --target windows-x64 --endpoint <windows data: endpoint> --json./zero update --check --target solaris-sparc --jsonreturned exit 2git diff --check
Vasanthdev2004
approved these changes
Jun 6, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Blockers
- None. The previous blocker on
--target=has been fixed atda07c7300dac2f8f805a098b8c57db47abe92ae3: inline empty target values now return a usage error instead of silently falling back to the current platform, and the CLI regression test covers that path.
Non-Blocking
- None.
Looks Good
- Verified the incremental fix from the blocked head
e1791f3ef446161fcfd4594e94f381ce9f7a45e9toda07c7300dac2f8f805a098b8c57db47abe92ae3. --target windows-x64still maps to the expected Windows x64 release asset and verifies the data-endpoint payload.- Invalid targets still fail closed with exit 2.
- Validation run locally:
go test -count=1 ./internal/update ./internal/cli -run 'TestResolveTarget|TestCheck|TestRunUpdate'go test -count=1 -p 1 ./...go run ./cmd/zero-release build./zero update --check --target= --jsonreturned exit 2 with--target requires a non-empty value./zero update --check --target windows-x64 --endpoint <windows data: endpoint> --json./zero update --check --target solaris-sparc --endpoint <windows data: endpoint> --jsonreturned exit 2git diff --check
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit
New Features
--targetflag tozero update --checkto allow checking releases for specific platforms (Linux, macOS, Windows; x64/arm64), instead of only the current system.Documentation
--target <platform-arch>, supported targets, and platform-specific release checks.Tests