Add self-updater for CLI binary#274
Merged
sourya-deepsource merged 6 commits intomasterfrom Mar 10, 2026
Merged
Conversation
Contributor
jai-deepsource
commented
Mar 5, 2026
- Two-phase update: check manifest, apply on next run
- Verify archive checksum before replacing binary
- Skip in CI and dev builds, add config opt-out
- Bump version to 2.0.45
- Two-phase update: check manifest, apply on next run - Verify archive checksum before replacing binary - Skip in CI and dev builds, add config opt-out - Bump version to 2.0.45
|
|
Overall Grade |
Security Reliability Complexity Hygiene Coverage |
Feedback
- Mocks mask real-world integration failures
- Unit tests mock filesystem and network behavior, so runtime issues—partial downloads, permission errors, transport quirks—can slip by; add lightweight integration/contract tests against a sandboxed updater and real artifact samples to surface them.
- Version parsing may be brittle to format drift
- Thorough semver tests protect current cases, but upstream metadata or build-metadata changes can break consumers; centralize parsing into a single canonical parser, validate incoming update metadata with a schema, and include real release samples in regression tests.
- Tests coupled to implementation hinder refactoring
- Table-driven tests that drive internal mocks and call sequences make refactors fragile and encourage implementation leaks; prefer black-box behavioral tests and golden outputs, keeping mocks confined to a thin adapter layer so tests assert behavior, not internals.
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Go | Mar 10, 2026 6:55a.m. | Review ↗ | |
| Secrets | Mar 10, 2026 6:55a.m. | Review ↗ | |
| Test coverage | Mar 10, 2026 6:55a.m. | Review ↗ |
Code Coverage Summary
| Language | Line Coverage (New Code) | Line Coverage (Overall) |
|---|---|---|
| Aggregate | 58.9% [⤫ below threshold] |
24.9% [▲ up 3.2% from master] |
| Go | 58.9% [⤫ below threshold] |
24.9% [▲ up 3.2% from master][✓ above threshold] |
➟ Additional coverage metrics may have been reported. See full coverage report ↗
The CDN serves archives under /build/, but the URL was constructed without it, causing 404 errors during self-update downloads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
- Handle os.UserHomeDir() error to prevent state file in relative paths - Restrict config dir permissions from 0o755 to 0o750 - Limit download size with io.LimitReader (50MB cap) - Replace unused parameter `r` with `_` in test handler - Refactor replaceBinary into testable replaceBinaryAt function - Rewrite tests to call actual functions instead of reimplementing logic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Sourya Vatsyayan <sourya@deepsource.io>
- Rewrite TestCheckForUpdate_AlreadyUpToDate to call CheckForUpdate with a mock HTTP server instead of reimplementing logic - Add tests for FetchManifest, downloadFile, and error paths (HTTP errors, network errors, checksum mismatch, missing platform, nil build info, zip archive path) - Add parseSemver test cases for invalid minor/patch and pre-release - Add ClearBuildInfo helper for test setup - Update package coverage from 47.5% to ~80% Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sourya-deepsource
approved these changes
Mar 10, 2026
parth-deepsource
approved these changes
Mar 10, 2026
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
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.