P1: Upgrade channels (stable/beta) + pinned version + target version install#36
Conversation
Reviewer's GuideImplements upgrade-channel support (stable/beta), target-version installs (including prereleases and tag-specific releases), and persists channel/pinning settings in config while updating CLI options and UX around the upgrade flow. Sequence diagram for upgrade command with channels, pinning, and target versionsequenceDiagram
actor User
participant CLI as cloudsqlctl_upgrade
participant Config as ConfigStore
participant Updater as SelfUpdate
participant GitHub as GitHub_API
User->>CLI: run upgrade [--channel/--version/--pin/--unpin]
CLI->>Config: readConfig()
Config-->>CLI: AppConfig(updateChannel,pinnedVersion,...)
CLI->>CLI: resolve channel (options.channel || config.updateChannel || stable)
CLI->>CLI: validate channel is stable or beta
alt user requested unpin
CLI->>Config: writeConfig(pinnedVersion: undefined)
end
alt user requested pin
CLI->>Config: writeConfig(pinnedVersion: options.pin, updateChannel: channel)
else channel changed without pin
CLI->>Config: writeConfig(updateChannel: channel)
end
CLI->>CLI: determine targetVersion = options.version || options.pin || config.pinnedVersion (if not unpinned)
CLI->>CLI: log "Checking for updates (Current, channel, target)" (when not --json)
CLI->>Updater: checkForUpdates(currentVersion, { channel, targetVersion })
alt targetVersion is set
Updater->>Updater: normalizeTag(targetVersion)
Updater->>GitHub: GET /releases/tags/{tag}
GitHub-->>Updater: ReleaseInfo(tag_name,...)
else channel is beta
Updater->>GitHub: GET /releases
GitHub-->>Updater: list of releases
Updater->>Updater: find first prerelease && !draft
else stable channel
Updater->>GitHub: GET /releases/latest
GitHub-->>Updater: latest stable ReleaseInfo
end
Updater->>Updater: compare currentVersion vs release.tag_name
Updater-->>CLI: UpdateStatus
CLI->>User: print status JSON or human-readable output
Class diagram for updated self-update and configuration structuresclassDiagram
class AppConfig {
string? lastUpdateCheck
string? lastUpdateAvailableVersion
string? gcloudPath
string? updateChannel
string? pinnedVersion
}
class SelfUpdate {
+getLatestRelease() ReleaseInfo
+getReleaseByTag(tag string) ReleaseInfo
+getLatestPrerelease() ReleaseInfo
+checkForUpdates(currentVersion string, options UpdateOptions) UpdateStatus
-normalizeTag(tag string) string
}
class UpdateOptions {
string? channel
string? targetVersion
}
class ReleaseInfo {
string tag_name
bool prerelease
bool draft
string name
string body
}
class UpdateStatus {
bool updateAvailable
string currentVersion
string latestVersion
string releaseChannel
}
class UpgradeCommandOptions {
bool check_only
bool no_install
string asset
string dir
bool force
bool no_silent
bool no_elevate
string channel
string version
string pin
bool unpin
bool json
}
class UpgradeCommandHandler {
+handle(options UpgradeCommandOptions) void
-resolveChannel(options UpgradeCommandOptions, config AppConfig) string
-resolveTargetVersion(options UpgradeCommandOptions, config AppConfig) string
-updateConfigForPinAndChannel(options UpgradeCommandOptions, channel string) void
}
UpgradeCommandHandler --> SelfUpdate : uses
UpgradeCommandHandler --> AppConfig : reads/writes
SelfUpdate --> ReleaseInfo : returns
SelfUpdate --> UpdateOptions : uses
SelfUpdate --> UpdateStatus : returns
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new
getLatestPrereleasehelper picks the first prerelease returned by the/releasesendpoint, which is not guaranteed to be the latest; consider explicitly sorting bypublished_atorcreated_atand selecting the newest non-draft prerelease to make the behavior predictable. - In
upgrade.ts, multiplewriteConfigcalls (--unpin, then possibly--pinor--channel) can lead to partial updates depending on howwriteConfigmerges state; it would be more robust to build a single combined config patch and persist it with onewriteConfigcall.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `getLatestPrerelease` helper picks the first prerelease returned by the `/releases` endpoint, which is not guaranteed to be the latest; consider explicitly sorting by `published_at` or `created_at` and selecting the newest non-draft prerelease to make the behavior predictable.
- In `upgrade.ts`, multiple `writeConfig` calls (`--unpin`, then possibly `--pin` or `--channel`) can lead to partial updates depending on how `writeConfig` merges state; it would be more robust to build a single combined config patch and persist it with one `writeConfig` call.
## Individual Comments
### Comment 1
<location> `src/commands/upgrade.ts:41-43` </location>
<code_context>
try {
const currentVersion = process.env.CLOUDSQLCTL_VERSION || '0.0.0';
+ const config = await readConfig();
+ const channel = (options.channel || config.updateChannel || 'stable') as 'stable' | 'beta';
+
+ if (channel !== 'stable' && channel !== 'beta') {
+ throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`);
+ }
</code_context>
<issue_to_address>
**suggestion:** Invalid `updateChannel` values in existing config cause the command to error instead of gracefully defaulting.
With this change, any unexpected value in `config.updateChannel` will cause every `upgrade` run to fail until the config is manually fixed, making the command fragile to older configs or corruption. Consider treating invalid values as a warning and defaulting to `'stable'` (and optionally persisting that change), e.g. compute a `rawChannel`, validate it, and on invalid values log a warning and fall back to `'stable'` instead of throwing.
Suggested implementation:
```typescript
try {
const currentVersion = process.env.CLOUDSQLCTL_VERSION || '0.0.0';
const config = await readConfig();
// Determine raw channel from CLI options or config without defaulting yet
const rawChannel = options.channel ?? config.updateChannel;
let channel: 'stable' | 'beta' = 'stable';
if (rawChannel === 'stable' || rawChannel === 'beta') {
channel = rawChannel;
} else if (rawChannel !== undefined) {
// Gracefully handle invalid values by warning and falling back to 'stable'
console.warn(
`Warning: invalid update channel '${rawChannel}' in configuration. Falling back to 'stable'.`,
);
}
if (options.unpin) {
```
If your project uses a structured logger instead of `console.warn`, replace the `console.warn` call with the appropriate logging utility (e.g. `logger.warn(...)`) and ensure any required imports are added at the top of `src/commands/upgrade.ts`.
</issue_to_address>
### Comment 2
<location> `src/commands/upgrade.ts:47-53` </location>
<code_context>
+ throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`);
+ }
+
+ if (options.unpin) {
+ await writeConfig({ pinnedVersion: undefined });
+ }
+
+ if (options.pin) {
+ await writeConfig({ pinnedVersion: options.pin, updateChannel: channel });
+ } else if (options.channel) {
+ await writeConfig({ updateChannel: channel });
+ }
</code_context>
<issue_to_address>
**suggestion:** Multiple sequential `writeConfig` calls could be consolidated into a single update for consistency and to avoid potential race conditions.
In the current flow, a single invocation (e.g. `--unpin` + `--channel`) can trigger two `writeConfig` calls. Consider accumulating the changes in a `configUpdate` object and calling `writeConfig` once. This keeps the behavior easier to reason about and reduces the risk of subtle race issues if `writeConfig` becomes more complex or asynchronous in the future.
</issue_to_address>
### Comment 3
<location> `docs/commands.md:326` </location>
<code_context>
+ "auto")
+ --dir <path> Download directory (default:
+ "<USER_HOME>\\AppData\\Local\\CloudSQLCTL\\downloads\\updates")
+ --force Force update even if version is same or older
+ --no-silent Run installer in interactive mode (installer only)
+ --no-elevate Do not attempt to elevate privileges (installer only)
</code_context>
<issue_to_address>
**nitpick (typo):** Consider adding articles for smoother grammar in the `--force` description.
For example: `Force update even if the version is the same or older` makes the sentence grammatically complete while keeping the intended meaning.
```suggestion
--force Force update even if the version is the same or older
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const channel = (options.channel || config.updateChannel || 'stable') as 'stable' | 'beta'; | ||
|
|
||
| if (channel !== 'stable' && channel !== 'beta') { |
There was a problem hiding this comment.
suggestion: Invalid updateChannel values in existing config cause the command to error instead of gracefully defaulting.
With this change, any unexpected value in config.updateChannel will cause every upgrade run to fail until the config is manually fixed, making the command fragile to older configs or corruption. Consider treating invalid values as a warning and defaulting to 'stable' (and optionally persisting that change), e.g. compute a rawChannel, validate it, and on invalid values log a warning and fall back to 'stable' instead of throwing.
Suggested implementation:
try {
const currentVersion = process.env.CLOUDSQLCTL_VERSION || '0.0.0';
const config = await readConfig();
// Determine raw channel from CLI options or config without defaulting yet
const rawChannel = options.channel ?? config.updateChannel;
let channel: 'stable' | 'beta' = 'stable';
if (rawChannel === 'stable' || rawChannel === 'beta') {
channel = rawChannel;
} else if (rawChannel !== undefined) {
// Gracefully handle invalid values by warning and falling back to 'stable'
console.warn(
`Warning: invalid update channel '${rawChannel}' in configuration. Falling back to 'stable'.`,
);
}
if (options.unpin) {If your project uses a structured logger instead of console.warn, replace the console.warn call with the appropriate logging utility (e.g. logger.warn(...)) and ensure any required imports are added at the top of src/commands/upgrade.ts.
| if (options.unpin) { | ||
| await writeConfig({ pinnedVersion: undefined }); | ||
| } | ||
|
|
||
| if (options.pin) { | ||
| await writeConfig({ pinnedVersion: options.pin, updateChannel: channel }); | ||
| } else if (options.channel) { |
There was a problem hiding this comment.
suggestion: Multiple sequential writeConfig calls could be consolidated into a single update for consistency and to avoid potential race conditions.
In the current flow, a single invocation (e.g. --unpin + --channel) can trigger two writeConfig calls. Consider accumulating the changes in a configUpdate object and calling writeConfig once. This keeps the behavior easier to reason about and reduces the risk of subtle race issues if writeConfig becomes more complex or asynchronous in the future.
| "auto") | ||
| --dir <path> Download directory (default: | ||
| "<USER_HOME>\\AppData\\Local\\CloudSQLCTL\\downloads\\updates") | ||
| --force Force update even if version is same or older |
There was a problem hiding this comment.
nitpick (typo): Consider adding articles for smoother grammar in the --force description.
For example: Force update even if the version is the same or older makes the sentence grammatically complete while keeping the intended meaning.
| --force Force update even if version is same or older | |
| --force Force update even if the version is the same or older |
There was a problem hiding this comment.
Pull request overview
This PR adds support for update channels (stable/beta), pinned versions, and target version installation to the upgrade command. Users can now choose between stable releases and beta prereleases, pin to specific versions for consistent upgrades, or install any specific version on demand.
Key changes:
- Extended the self-update mechanism to support beta channel (prereleases) and specific version targeting via GitHub API
- Added
updateChannelandpinnedVersionfields to the application configuration - Introduced new CLI options:
--channel,--version,--pin, and--unpinto the upgrade command
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/core/selfUpdate.ts | Added functions to fetch prereleases and releases by tag, updated checkForUpdates to accept channel and target version options |
| src/core/config.ts | Extended AppConfig interface with updateChannel and pinnedVersion fields |
| src/commands/upgrade.ts | Added new command-line options and logic to handle channel selection, version pinning, and target version resolution |
| docs/commands.md | Updated upgrade command documentation to include the new options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const channel = (options.channel || config.updateChannel || 'stable') as 'stable' | 'beta'; | ||
|
|
||
| if (channel !== 'stable' && channel !== 'beta') { | ||
| throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`); | ||
| } | ||
|
|
||
| if (options.unpin) { | ||
| await writeConfig({ pinnedVersion: undefined }); |
There was a problem hiding this comment.
When using --unpin, the pinnedVersion is cleared but the updateChannel persists from the previous configuration. This could lead to unexpected behavior where a user unpins but remains on the beta channel if they previously set it. Consider clarifying the behavior: should --unpin also reset the channel to stable, or is keeping the channel intentional? If intentional, document this behavior clearly.
| const channel = (options.channel || config.updateChannel || 'stable') as 'stable' | 'beta'; | |
| if (channel !== 'stable' && channel !== 'beta') { | |
| throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`); | |
| } | |
| if (options.unpin) { | |
| await writeConfig({ pinnedVersion: undefined }); | |
| const channel = (options.channel || (options.unpin ? 'stable' : config.updateChannel) || 'stable') as 'stable' | 'beta'; | |
| if (channel !== 'stable' && channel !== 'beta') { | |
| throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`); | |
| } | |
| if (options.unpin) { | |
| await writeConfig({ pinnedVersion: undefined, updateChannel: 'stable' }); |
| export async function getLatestPrerelease(): Promise<ReleaseInfo> { | ||
| try { | ||
| const response = await axios.get(GITHUB_RELEASES_URL, { | ||
| timeout: TIMEOUT_MS, | ||
| headers: { 'User-Agent': 'cloudsqlctl/upgrade' } | ||
| }); | ||
| const releases = Array.isArray(response.data) ? response.data : []; | ||
| const prerelease = releases.find((r: { prerelease?: boolean; draft?: boolean }) => r.prerelease && !r.draft); | ||
| if (!prerelease) { | ||
| throw new Error('No prerelease found'); | ||
| } | ||
| return prerelease; | ||
| } catch (error) { | ||
| logger.error('Failed to fetch latest prerelease info', error); | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
The getLatestPrerelease function fetches all releases from the API without pagination parameters. If the repository has many releases, this could return a large payload and potentially miss the latest prerelease if it's not in the first page of results (GitHub API typically returns 30 items per page by default). Consider adding pagination support or at least a per_page parameter to ensure you get enough releases, or document this limitation.
| if (options.unpin) { | ||
| await writeConfig({ pinnedVersion: undefined }); | ||
| } | ||
|
|
||
| if (options.pin) { | ||
| await writeConfig({ pinnedVersion: options.pin, updateChannel: channel }); | ||
| } else if (options.channel) { | ||
| await writeConfig({ updateChannel: channel }); | ||
| } |
There was a problem hiding this comment.
The sequence of config writes could be problematic. If options.unpin is true followed by options.pin being set (though this seems unlikely in practice), line 48 would clear the pinnedVersion, then line 52 would immediately set it again. While this works, it results in unnecessary I/O. Consider restructuring the logic to have mutually exclusive branches or combining the config writes into a single operation.
| if (options.unpin) { | |
| await writeConfig({ pinnedVersion: undefined }); | |
| } | |
| if (options.pin) { | |
| await writeConfig({ pinnedVersion: options.pin, updateChannel: channel }); | |
| } else if (options.channel) { | |
| await writeConfig({ updateChannel: channel }); | |
| } | |
| const configUpdate: { pinnedVersion?: string | undefined; updateChannel?: 'stable' | 'beta' } = {}; | |
| if (options.pin) { | |
| // Pin to a specific version and ensure the channel is updated accordingly | |
| configUpdate.pinnedVersion = options.pin; | |
| configUpdate.updateChannel = channel; | |
| } else { | |
| if (options.unpin) { | |
| // Clear any existing pinned version | |
| configUpdate.pinnedVersion = undefined; | |
| } | |
| if (options.channel) { | |
| // Update only the channel when not pinning | |
| configUpdate.updateChannel = channel; | |
| } | |
| } | |
| if (Object.keys(configUpdate).length > 0) { | |
| await writeConfig(configUpdate); | |
| } |
| export async function checkForUpdates( | ||
| currentVersion: string, | ||
| options: { channel?: 'stable' | 'beta'; targetVersion?: string } = {} | ||
| ): Promise<UpdateStatus> { |
There was a problem hiding this comment.
When targetVersion is specified, the function doesn't validate whether the version comparison makes sense. For example, if a user specifies a targetVersion that is older than the currentVersion and doesn't use --force, the function will still report updateAvailable: false based on semver comparison on line 98, but this might be confusing. Consider adding a separate flag in the UpdateStatus to indicate when a specific version was requested vs. when checking for latest, or validating that targetVersion is actually a valid semver format before making the API call.
| const releases = Array.isArray(response.data) ? response.data : []; | ||
| const prerelease = releases.find((r: { prerelease?: boolean; draft?: boolean }) => r.prerelease && !r.draft); | ||
| if (!prerelease) { | ||
| throw new Error('No prerelease found'); |
There was a problem hiding this comment.
The error message "No prerelease found" could be more helpful by clarifying that this means there are no beta releases available on the configured channel. Consider changing it to something like "No beta/prerelease versions available" to be clearer to users.
| throw new Error('No prerelease found'); | |
| throw new Error('No beta/prerelease versions available on the configured channel'); |
| } else if (options.channel) { | ||
| await writeConfig({ updateChannel: channel }); | ||
| } | ||
| const targetVersion = options.version || options.pin || (!options.unpin ? config.pinnedVersion : undefined); |
There was a problem hiding this comment.
The logic for resolving the targetVersion on line 56 has a potential issue. When both options.pin and options.version are set, options.version will be ignored because options.pin takes precedence in the OR chain. This could be confusing to users. Consider validating that only one of these mutually exclusive options is set, or document the precedence order clearly in the command help.
| export async function getReleaseByTag(tag: string): Promise<ReleaseInfo> { | ||
| try { | ||
| const response = await axios.get(`${GITHUB_RELEASE_TAG_URL}/${normalizeTag(tag)}`, { | ||
| timeout: TIMEOUT_MS, | ||
| headers: { 'User-Agent': 'cloudsqlctl/upgrade' } | ||
| }); | ||
| return response.data; | ||
| } catch (error) { | ||
| logger.error('Failed to fetch release by tag', error); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| export async function getLatestPrerelease(): Promise<ReleaseInfo> { | ||
| try { | ||
| const response = await axios.get(GITHUB_RELEASES_URL, { | ||
| timeout: TIMEOUT_MS, | ||
| headers: { 'User-Agent': 'cloudsqlctl/upgrade' } | ||
| }); | ||
| const releases = Array.isArray(response.data) ? response.data : []; | ||
| const prerelease = releases.find((r: { prerelease?: boolean; draft?: boolean }) => r.prerelease && !r.draft); | ||
| if (!prerelease) { | ||
| throw new Error('No prerelease found'); | ||
| } | ||
| return prerelease; | ||
| } catch (error) { | ||
| logger.error('Failed to fetch latest prerelease info', error); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| export async function checkForUpdates( | ||
| currentVersion: string, | ||
| options: { channel?: 'stable' | 'beta'; targetVersion?: string } = {} | ||
| ): Promise<UpdateStatus> { | ||
| let release: ReleaseInfo; | ||
| if (options.targetVersion) { | ||
| release = await getReleaseByTag(options.targetVersion); | ||
| } else if (options.channel === 'beta') { | ||
| release = await getLatestPrerelease(); | ||
| } else { | ||
| release = await getLatestRelease(); | ||
| } |
There was a problem hiding this comment.
The new functions getReleaseByTag, getLatestPrerelease, and the updated checkForUpdates function with channel and targetVersion parameters lack test coverage. The existing test file only covers the basic checkForUpdates function without these new options. Consider adding tests for the beta channel, specific version targeting, and error scenarios such as when a release tag doesn't exist or when no prereleases are available.
| const channel = (options.channel || config.updateChannel || 'stable') as 'stable' | 'beta'; | ||
|
|
||
| if (channel !== 'stable' && channel !== 'beta') { | ||
| throw new Error(`Invalid channel '${channel}'. Use 'stable' or 'beta'.`); | ||
| } |
There was a problem hiding this comment.
The channel validation occurs after reading the config and before storing it, but the validation only checks the final resolved channel value. If config.updateChannel contains an invalid value (e.g., from manual config file editing), it would pass through without error until it's used. Consider moving this validation earlier or adding validation when reading from config to provide better error messages about corrupted configuration.
Closes #19
Summary:
Tests:
Rollback:
Summary by Sourcery
Add configurable upgrade channels and version targeting to the self-update flow, persisting these settings in config and documenting the new CLI options.
New Features:
Enhancements:
Documentation: