-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add upgrade notification when newer CLI version is available #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ea28425
49a22bb
16eb79f
95bc93d
e1b4f96
e58f957
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { log } from "@clack/prompts"; | ||
| import type { UpgradeInfo } from "@/cli/utils/version-check.js"; | ||
| import { checkForUpgrade } from "@/cli/utils/version-check.js"; | ||
| import { theme } from "@/cli/utils/theme.js"; | ||
|
|
||
| function formatUpgradeMessage(info: UpgradeInfo): string { | ||
| const { shinyOrange } = theme.colors; | ||
| const { bold } = theme.styles; | ||
|
|
||
| return `${shinyOrange("Update available!")} ${shinyOrange(`${info.currentVersion} → ${info.latestVersion}`)} ${shinyOrange("Run:")} ${bold(shinyOrange("npm update -g base44"))}`; | ||
| } | ||
|
|
||
| /** | ||
| * Checks for available upgrades and prints a notification if one exists. | ||
| */ | ||
| export async function printUpgradeNotificationIfAvailable(): Promise<void> { | ||
| try { | ||
| const upgradeInfo = await checkForUpgrade(); | ||
| if (upgradeInfo) { | ||
| log.message(formatUpgradeMessage(upgradeInfo)); | ||
| } | ||
| } catch { | ||
| // Silently ignore errors | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @claude this whole file should be inside the src/cli folder because it's not related to the core/ of the product There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Claude finished @kfirstri's task in 1m 34s —— View job Completed
Moved --- | Branch |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { execa } from "execa"; | ||
| import packageJson from "../../../package.json"; | ||
|
|
||
| export interface UpgradeInfo { | ||
| currentVersion: string; | ||
| latestVersion: string; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if a newer version of the CLI is available. | ||
| */ | ||
| export async function checkForUpgrade(): Promise<UpgradeInfo | null> { | ||
| try { | ||
| const { stdout } = await execa("npm", ["view", "base44", "version"], { timeout: 5000 }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. look for other places we use execa and make sure we pass all appropriate flags @claude There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Claude finished @kfirstri's task in 1m 12s —— View job Reviewing execa usage patterns
|
||
| const latestVersion = stdout.trim(); | ||
| const currentVersion = packageJson.version; | ||
|
|
||
| if (latestVersion !== currentVersion) { | ||
| return { currentVersion, latestVersion }; | ||
| } | ||
| return null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { describe, it, vi, beforeEach } from "vitest"; | ||
| import { setupCLITests } from "./testkit/index.js"; | ||
| import type { UpgradeInfo } from "@/cli/utils/version-check.js"; | ||
|
|
||
| // Mock the version-check module | ||
| vi.mock("@/cli/utils/version-check.js", () => ({ | ||
| checkForUpgrade: vi.fn(), | ||
| })); | ||
|
|
||
| describe("upgrade notification", () => { | ||
| const t = setupCLITests(); | ||
|
|
||
| beforeEach(async () => { | ||
| const { checkForUpgrade } = await import("@/cli/utils/version-check.js"); | ||
| vi.mocked(checkForUpgrade).mockReset(); | ||
| }); | ||
|
|
||
| it("displays upgrade notification when newer version is available", async () => { | ||
| const { checkForUpgrade } = await import("@/cli/utils/version-check.js"); | ||
| const upgradeInfo: UpgradeInfo = { | ||
| currentVersion: "0.0.26", | ||
| latestVersion: "1.0.0", | ||
| }; | ||
| vi.mocked(checkForUpgrade).mockResolvedValue(upgradeInfo); | ||
|
|
||
| await t.givenLoggedIn({ email: "test@example.com", name: "Test User" }); | ||
|
|
||
| const result = await t.run("whoami"); | ||
|
|
||
| t.expectResult(result).toSucceed(); | ||
| t.expectResult(result).toContain("Update available!"); | ||
| t.expectResult(result).toContain("0.0.26 → 1.0.0"); | ||
| t.expectResult(result).toContain("npm update -g base44"); | ||
| }); | ||
|
|
||
| it("does not display notification when version is current", async () => { | ||
| const { checkForUpgrade } = await import("@/cli/utils/version-check.js"); | ||
| vi.mocked(checkForUpgrade).mockResolvedValue(null); | ||
|
|
||
| await t.givenLoggedIn({ email: "test@example.com", name: "Test User" }); | ||
|
|
||
| const result = await t.run("whoami"); | ||
|
|
||
| t.expectResult(result).toSucceed(); | ||
| t.expectResult(result).not.toContain("Update available!"); | ||
|
Check failure on line 45 in tests/cli/version-check.spec.ts
|
||
| }); | ||
|
|
||
| it("does not display notification when version check fails", async () => { | ||
| const { checkForUpgrade } = await import("@/cli/utils/version-check.js"); | ||
| vi.mocked(checkForUpgrade).mockRejectedValue(new Error("Network error")); | ||
|
|
||
| await t.givenLoggedIn({ email: "test@example.com", name: "Test User" }); | ||
|
|
||
| const result = await t.run("whoami"); | ||
|
|
||
| // Command still succeeds (upgrade check doesn't block) | ||
| t.expectResult(result).toSucceed(); | ||
| t.expectResult(result).not.toContain("Update available!"); | ||
|
Check failure on line 58 in tests/cli/version-check.spec.ts
|
||
| }); | ||
| }); | ||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@claude i see were using log.message, and the
printUpgradeNotificationIfAvailableis called inrunCommandbut after we already dooutromeaning it will be a weird hevaior, maybe we can do it at the begining?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude finished @kfirstri's task in 47s —— View job
Completed
Moved the upgrade notification to display right after the intro/banner, before the main command logic runs. This prevents the notification from appearing after the
outro, which would have created strange behavior where messages appear after the command completion message.The notification now appears at the beginning of the command flow, making it more visible and avoiding any confusion with the outro message.
| Branch