diff --git a/cli/src/cli.ts b/cli/src/cli.ts index a2af3af..eca1aa0 100644 --- a/cli/src/cli.ts +++ b/cli/src/cli.ts @@ -165,6 +165,10 @@ async function bootstrap(): Promise { if (earlyExit?.id === "help") { const [command, parent] = await resolveSubCommandForUsage(main, rawArgs); await showAltertableUsage(command, parent); + await maybeShowUpdateNotice({ + context: getCliContext(), + commandName: resolveTopLevelCommandName(rawArgs) ?? "help", + }); process.exit(EXIT_SUCCESS); } diff --git a/cli/src/commands/update.ts b/cli/src/commands/update.ts index aac546c..530d17e 100644 --- a/cli/src/commands/update.ts +++ b/cli/src/commands/update.ts @@ -83,7 +83,7 @@ async function runUpdateCommand(args: UpdateCommandArgs, sink: OutputSink): Prom return; } - if ((interval || args["clear-cache"]) && !args.install && !args["target-version"]) { + if (interval && !args["clear-cache"] && !args.install && !args["target-version"]) { await writeStatus(sink); return; } diff --git a/cli/src/lib/config-files.ts b/cli/src/lib/config-files.ts index f770aa8..0ddc9c2 100644 --- a/cli/src/lib/config-files.ts +++ b/cli/src/lib/config-files.ts @@ -1,12 +1,15 @@ import { randomBytes } from "node:crypto"; -import { chmodSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { chmodSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; function trim(value: string): string { return value.trim(); } +function tempSiblingPath(filePath: string): string { + return join(dirname(filePath), `.altertable-kv-${randomBytes(8).toString("hex")}`); +} + export function configDir(): string { const override = process.env.ALTERTABLE_CONFIG_HOME; if (override) { @@ -48,7 +51,7 @@ export function kvGet(filePath: string, key: string): string { export function kvSet(filePath: string, key: string, value: string): void { mkdirSync(dirname(filePath), { recursive: true }); - const tmpPath = join(tmpdir(), `altertable-kv-${randomBytes(8).toString("hex")}`); + const tmpPath = tempSiblingPath(filePath); let found = false; let lines: string[] = []; @@ -77,25 +80,29 @@ export function kvSet(filePath: string, key: string, value: string): void { output.push(`${key}=${value}`); } - writeFileSync( - tmpPath, - output - .filter((line, index, array) => { - if (index === array.length - 1 && line === "") { - return false; - } - return true; - }) - .join("\n") + (output.length > 0 ? "\n" : ""), - { mode: 0o600 }, - ); - renameSync(tmpPath, filePath); + try { + writeFileSync( + tmpPath, + output + .filter((line, index, array) => { + if (index === array.length - 1 && line === "") { + return false; + } + return true; + }) + .join("\n") + (output.length > 0 ? "\n" : ""), + { mode: 0o600 }, + ); + renameSync(tmpPath, filePath); + } finally { + rmSync(tmpPath, { force: true }); + } } export function kvUnset(filePath: string, key: string): void { try { const lines = readFileSync(filePath, "utf8").split("\n"); - const tmpPath = join(tmpdir(), `altertable-kv-${randomBytes(8).toString("hex")}`); + const tmpPath = tempSiblingPath(filePath); const output: string[] = []; for (const line of lines) { const eqIndex = line.indexOf("="); @@ -105,8 +112,12 @@ export function kvUnset(filePath: string, key: string): void { } output.push(line); } - writeFileSync(tmpPath, output.join("\n"), { mode: 0o600 }); - renameSync(tmpPath, filePath); + try { + writeFileSync(tmpPath, output.join("\n"), { mode: 0o600 }); + renameSync(tmpPath, filePath); + } finally { + rmSync(tmpPath, { force: true }); + } } catch { // file does not exist } diff --git a/cli/src/lib/updater-config.ts b/cli/src/lib/updater-config.ts index 208ebc8..f23bd15 100644 --- a/cli/src/lib/updater-config.ts +++ b/cli/src/lib/updater-config.ts @@ -111,7 +111,6 @@ export const UpdaterConfig = { }, commands: { selfUpdate: "altertable update --install", - packageManagerUpdate: "altertable update --install --install-method package-manager", }, } as const; diff --git a/cli/src/lib/updater.ts b/cli/src/lib/updater.ts index 9177cd0..bf5643e 100644 --- a/cli/src/lib/updater.ts +++ b/cli/src/lib/updater.ts @@ -1,6 +1,5 @@ import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { chmodSync } from "node:fs"; -import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; import { createHash, randomBytes } from "node:crypto"; import { spawnSync } from "node:child_process"; @@ -13,8 +12,8 @@ import { CliError, HttpError, NetworkError } from "@/lib/errors.ts"; import { hasObjectKey } from "@/lib/object.ts"; import type { OutputSink } from "@/lib/runtime.ts"; import { getOutputSink } from "@/lib/runtime.ts"; -import { terminalMetadata } from "@/ui/terminal/styles.ts"; -import { document, rows, section, text, type DisplayRow } from "@/ui/document.ts"; +import { terminalAccent, terminalMetadata, terminalSuccess } from "@/ui/terminal/styles.ts"; +import { document, rows, section, type DisplayRow } from "@/ui/document.ts"; import { renderDocumentText } from "@/ui/renderers/terminal.ts"; import { UpdaterInstallManagers, @@ -240,9 +239,13 @@ function updaterStateFile(): string { function writeJsonFileAtomic(filePath: string, data: unknown): void { mkdirSync(dirname(filePath), { recursive: true }); - const tmpPath = join(tmpdir(), `altertable-update-${randomBytes(8).toString("hex")}`); - writeFileSync(tmpPath, `${JSON.stringify(data, null, 2)}\n`, { mode: 0o600 }); - renameSync(tmpPath, filePath); + const tmpPath = tempSiblingPath(filePath, "update"); + try { + writeFileSync(tmpPath, `${JSON.stringify(data, null, 2)}\n`, { mode: 0o600 }); + renameSync(tmpPath, filePath); + } finally { + rmSync(tmpPath, { force: true }); + } try { chmodSync(filePath, 0o600); } catch { @@ -798,13 +801,7 @@ function resolveInstallMethodForInstallation( if (installation.kind === UpdaterInstallationKind.nativeBinary) { return UpdaterInstallMethod.githubBinary; } - if (installation.kind === UpdaterInstallationKind.packageManager) { - return UpdaterInstallMethod.packageManager; - } - throw new CliError("Automatic CLI install is not supported for this checkout.", { - details: - "Source checkouts should be updated with git, or pass --install-method package-manager to install the published package globally.", - }); + return UpdaterInstallMethod.packageManager; } function spawnOutputText(value: unknown): string { @@ -821,20 +818,21 @@ export function verifyInstalledCliVersion( expectedVersion: string, command = "altertable", spawnImpl: SpawnSyncImpl = spawnSync, + description = "Installed altertable", ): string { const result = spawnImpl(command, ["--version"], { encoding: "utf8" }); if (result.error) { - throw new CliError(`Unable to verify installed altertable version.`, { cause: result.error }); + throw new CliError(`Unable to verify ${description.toLowerCase()} version.`, { + cause: result.error, + }); } if (result.status !== 0) { - throw new CliError( - `Installed altertable version check failed with exit code ${result.status ?? 1}.`, - ); + throw new CliError(`${description} version check failed with exit code ${result.status ?? 1}.`); } const installedVersion = normalizeVersion(spawnOutputText(result.stdout).trim()); if (compareVersions(installedVersion, expectedVersion) !== 0) { throw new CliError( - `Installed altertable version is ${installedVersion || "unknown"}, expected ${normalizeVersion( + `${description} version is ${installedVersion || "unknown"}, expected ${normalizeVersion( expectedVersion, )}.`, ); @@ -938,18 +936,27 @@ export async function installGitHubBinaryRelease( timeoutMs, fetchImpl, }); - replaceFileAtomically(targetPath, tempPath); + try { + const verifiedVersion = + options.verify === false + ? release.version + : verifyInstalledCliVersion( + release.version, + tempPath, + options.spawnImpl, + "Downloaded altertable", + ); + replaceFileAtomically(targetPath, tempPath); - const verifiedVersion = - options.verify === false - ? release.version - : verifyInstalledCliVersion(release.version, targetPath, options.spawnImpl); - return { - installed_version: release.version, - verified_version: verifiedVersion, - method: UpdaterInstallMethod.githubBinary, - target_path: targetPath, - }; + return { + installed_version: release.version, + verified_version: verifiedVersion, + method: UpdaterInstallMethod.githubBinary, + target_path: targetPath, + }; + } finally { + rmSync(tempPath, { force: true }); + } } export function runInstallPlan(plan: InstallPlan, options: RunInstallPlanOptions = {}): string { @@ -1017,18 +1024,10 @@ export function recommendedInstallCommand( version: string, installation: CurrentInstallation = detectCurrentInstallation(), ): string { - try { - const method = resolveInstallMethodForInstallation( - UpdaterConfig.defaults.installMethod, - installation, - ); - if (method === UpdaterInstallMethod.githubBinary) { - return UpdaterConfig.commands.selfUpdate; - } - } catch { - return UpdaterConfig.commands.packageManagerUpdate; + if (installation.kind === UpdaterInstallationKind.packageManager) { + return createInstallPlan(version).display; } - return createInstallPlan(version).display; + return UpdaterConfig.commands.selfUpdate; } export async function checkForUpdate(options: FetchLatestOptions = {}): Promise { @@ -1074,6 +1073,29 @@ export function shouldRunAutomaticUpdateCheck(options: { state?: UpdateState; now?: Date; stderrIsTTY?: boolean; +}): boolean { + if (!shouldShowAutomaticUpdateNotice(options)) { + return false; + } + + const interval = getUpdateCheckInterval(); + const state = options.state ?? readUpdateState(); + if (!state.last_checked_at) { + return true; + } + + const previous = Date.parse(state.last_checked_at); + if (Number.isNaN(previous)) { + return true; + } + const now = options.now ?? new Date(); + return now.getTime() - previous >= intervalMs(interval); +} + +function shouldShowAutomaticUpdateNotice(options: { + context: CliContext; + commandName?: string; + stderrIsTTY?: boolean; }): boolean { if (isJsonOutput(options.context) || options.context.debug) { return false; @@ -1099,73 +1121,62 @@ export function shouldRunAutomaticUpdateCheck(options: { if (interval === "never") { return false; } + return true; +} - const state = options.state ?? readUpdateState(); - if (!state.last_checked_at) { - return true; - } - - const previous = Date.parse(state.last_checked_at); - if (Number.isNaN(previous)) { - return true; - } - const now = options.now ?? new Date(); - return now.getTime() - previous >= intervalMs(interval); +function writeAutomaticUpdateNotice(sink: OutputSink, latestVersion: string): void { + sink.writeMetadata([ + "", + terminalMetadata(`Update available: altertable ${latestVersion} (current ${VERSION}).`), + terminalMetadata(`Run ${recommendedInstallCommand(latestVersion)} to install it.`), + ]); } export async function maybeShowUpdateNotice(options: AutomaticNoticeOptions): Promise { - if (!shouldRunAutomaticUpdateCheck(options)) { - return; - } - - const sink = options.sink ?? getOutputSink(); try { - const result = await checkForUpdate({ - source: options.source, - fetchImpl: options.fetchImpl, - timeoutMs: options.timeoutMs ?? UpdaterConfig.timeoutsMs.automatic, - }); - if (!result.update_available) { + if (!shouldShowAutomaticUpdateNotice(options)) { return; } - sink.writeMetadata([ - "", - terminalMetadata( - `Update available: altertable ${result.latest_version} (current ${result.current_version}).`, - ), - terminalMetadata(`Run ${result.install_command} or ${UpdaterConfig.commands.selfUpdate}.`), - ]); - } catch (error) { - const message = error instanceof Error ? error.message : "Unknown update check error."; + const sink = options.sink ?? getOutputSink(); const previous = readUpdateState(); - tryWriteUpdateState({ - ...previous, - last_checked_at: new Date().toISOString(), - last_error: message, - }); + let latestVersion = previous.latest_version; + + if (shouldRunAutomaticUpdateCheck({ ...options, state: previous })) { + try { + const result = await checkForUpdate({ + source: options.source, + fetchImpl: options.fetchImpl, + timeoutMs: options.timeoutMs ?? UpdaterConfig.timeoutsMs.automatic, + }); + latestVersion = result.latest_version; + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown update check error."; + tryWriteUpdateState({ + ...previous, + last_checked_at: new Date().toISOString(), + last_error: message, + }); + } + } + + if (latestVersion && compareVersions(latestVersion, VERSION) > 0) { + writeAutomaticUpdateNotice(sink, latestVersion); + } + } catch { + // Automatic update notices must never interrupt the command being run. } } export function formatUpdateResult(result: UpdateCheckResult): string { if (!result.update_available) { - return `altertable is up to date (${result.current_version}).`; - } - - return renderDocumentText( - document( - section( - text([ - `altertable ${result.latest_version} is available (current ${result.current_version}).`, - ]), - rows([ - { label: "Install:", value: result.install_command }, - { label: "Release:", value: result.release_url, linkifyUrls: true }, - ]), - ), - ), - { labelWidth: "Release:".length }, - ); + return `${terminalSuccess("Congrats!")} You're already on the latest version of altertable ${terminalMetadata(`(which is v${normalizeVersion(result.current_version)})`)}`; + } + + return [ + `A new version of altertable is available: v${normalizeVersion(result.latest_version)} ${terminalMetadata(`(current v${normalizeVersion(result.current_version)})`)}`, + `Run ${terminalAccent(result.install_command)} to install it.`, + ].join("\n"); } export function formatUpdateStatus(interval: UpdateCheckInterval, state: UpdateState): string { diff --git a/cli/tests/updater.test.ts b/cli/tests/updater.test.ts index 0a8a4da..924be84 100644 --- a/cli/tests/updater.test.ts +++ b/cli/tests/updater.test.ts @@ -7,6 +7,7 @@ import { join, resolve } from "node:path"; import { createHash } from "node:crypto"; import { buildMainCommand, resolveTopLevelCommandName } from "@/cli.ts"; import { CLI_PACKAGE_METADATA } from "@/package-metadata.ts"; +import { VERSION } from "@/version.ts"; import { checkForUpdate, compareVersions, @@ -343,6 +344,7 @@ describe("binary self-update", () => { }); test("recommends install commands from installation origin", () => { + process.env.ALTERTABLE_UPDATE_INSTALLER = "npm"; expect( recommendedInstallCommand(UPDATE_TEST_VERSION, { kind: "native-binary", @@ -354,7 +356,19 @@ describe("binary self-update", () => { kind: "package-manager", executablePath: "/usr/local/lib/node_modules/@altertable/cli/dist/cli.js", }), - ).toContain(`${UpdaterConfig.packageName}@${UPDATE_TEST_VERSION}`); + ).toBe(`npm install -g ${UpdaterConfig.packageName}@${UPDATE_TEST_VERSION}`); + expect( + recommendedInstallCommand(UPDATE_TEST_VERSION, { + kind: "source", + executablePath: "/repo/cli/src/cli.ts", + }), + ).toBe("altertable update --install"); + expect( + recommendedInstallCommand(UPDATE_TEST_VERSION, { + kind: "unknown", + executablePath: "/custom/altertable", + }), + ).toBe("altertable update --install"); }); test("parses and verifies SHA-256 checksums", () => { @@ -426,29 +440,33 @@ describe("binary self-update", () => { expect(readFileSync(targetPath, "utf8")).toBe(originalBinary); }); - test("auto install rejects source checkouts but explicit package-manager can run", async () => { + test("verifies a downloaded GitHub binary before replacing the target", async () => { + const targetPath = join(testHome, "altertable"); + const originalBinary = "#!/bin/sh\necho 1.0.0\n"; + const unexpectedBinary = "#!/bin/sh\necho 9.9.9\n"; + writeFileSync(targetPath, originalBinary, { mode: 0o755 }); + try { - await installCliUpdate(UPDATE_TEST_VERSION, { - verify: false, - installation: { - kind: "source", - executablePath: "/repo/cli/src/cli.ts", - reason: "test", - }, - spawnImpl: (() => { - throw new Error("should not spawn"); - }) as unknown as typeof import("node:child_process").spawnSync, + await installGitHubBinaryRelease(UPDATE_TEST_VERSION, { + targetPath, + platform: "linux-x64", + fetchImpl: githubBinaryFetch({ + binary: unexpectedBinary, + checksum: sha256(unexpectedBinary), + }), }); - throw new Error("Expected automatic install to reject source checkouts."); + throw new Error("Expected downloaded binary verification to fail."); } catch (error) { expect(error).toBeInstanceOf(Error); - expect((error as Error).message).toContain("Automatic CLI install is not supported"); + expect((error as Error).message).toContain("Downloaded altertable version is 9.9.9"); } + expect(readFileSync(targetPath, "utf8")).toBe(originalBinary); + }); + test("auto install uses the package manager for source checkouts", async () => { process.env.ALTERTABLE_UPDATE_INSTALLER = "npm"; const commands: string[] = []; const result = await installCliUpdate(UPDATE_TEST_VERSION, { - method: "package-manager", verify: false, stdio: "pipe", installation: { @@ -563,6 +581,19 @@ describe("automatic update checks", () => { }); }); + test("does not reject when automatic check policy cannot read config", async () => { + const invalidConfigHome = join(testHome, "not-a-directory"); + writeFileSync(invalidConfigHome, ""); + process.env.ALTERTABLE_CONFIG_HOME = invalidConfigHome; + + await maybeShowUpdateNotice({ + context: { debug: false, json: false, agent: false }, + commandName: "context", + stderrIsTTY: true, + fetchImpl: jsonFetch({ version: UPDATE_TEST_VERSION }), + }); + }); + test("writes automatic notices to metadata output", async () => { const stderr: string[] = []; const runtime = createCliRuntime({ debug: false, json: false, agent: false }); @@ -582,13 +613,103 @@ describe("automatic update checks", () => { expect(stderr.join("\n")).toContain(`Update available: altertable ${UPDATE_TEST_VERSION}`); expect(stderr.join("\n")).toContain("altertable update --install"); }); + + test("writes cached update notices without refreshing release metadata", async () => { + await checkForUpdate({ fetchImpl: jsonFetch({ version: UPDATE_TEST_VERSION }) }); + const stderr: string[] = []; + let fetchCalls = 0; + + await maybeShowUpdateNotice({ + context: { debug: false, json: false, agent: false }, + commandName: "help", + stderrIsTTY: true, + fetchImpl: (async () => { + fetchCalls += 1; + throw new Error("cached notices should not fetch"); + }) as unknown as typeof fetch, + sink: { + json: false, + debug: false, + writeStderr() {}, + writeJson() {}, + writeRaw() {}, + writeHuman() {}, + writeMetadata(lines) { + stderr.push(...lines); + }, + }, + }); + + expect(fetchCalls).toBe(0); + expect(stderr.join("\n")).toContain(`Update available: altertable ${UPDATE_TEST_VERSION}`); + }); + + test("falls back to a cached notice when a scheduled refresh fails", async () => { + await checkForUpdate({ fetchImpl: jsonFetch({ version: UPDATE_TEST_VERSION }) }); + const state = readUpdateState(); + const stderr: string[] = []; + + await maybeShowUpdateNotice({ + context: { debug: false, json: false, agent: false }, + commandName: "context", + stderrIsTTY: true, + now: new Date(Date.parse(state.last_checked_at ?? "") + 24 * 60 * 60 * 1000), + fetchImpl: (async () => { + throw new Error("network unavailable"); + }) as unknown as typeof fetch, + sink: { + json: false, + debug: false, + writeStderr() {}, + writeJson() {}, + writeRaw() {}, + writeHuman() {}, + writeMetadata(lines) { + stderr.push(...lines); + }, + }, + }); + + expect(stderr.join("\n")).toContain(`Update available: altertable ${UPDATE_TEST_VERSION}`); + expect(readUpdateState().last_error).toBe("Unable to check for CLI updates."); + }); }); describe("update command", () => { test("reports explicit target version without network", async () => { const output = await runUpdateCommand(["update", "--target-version", UPDATE_TEST_VERSION]); - expect(output.stdout[0]).toContain(`altertable ${UPDATE_TEST_VERSION} is available`); + expect(output.stdout[0]).toBe( + [ + `A new version of altertable is available: v${UPDATE_TEST_VERSION} (current v${VERSION})`, + `Run ${UpdaterConfig.commands.selfUpdate} to install it.`, + ].join("\n"), + ); + expect(output.stdout[0]).not.toContain("Install:"); + expect(output.stdout[0]).not.toContain("Release:"); + }); + + test("reports the latest installed version like Bun", async () => { + const output = await runUpdateCommand(["update", "--target-version", VERSION]); + + expect(output.stdout[0]).toBe( + `Congrats! You're already on the latest version of altertable (which is v${VERSION})`, + ); + }); + + test("clear-cache forces a fresh update check", async () => { + const originalFetch = globalThis.fetch; + globalThis.fetch = jsonFetch({ version: UPDATE_TEST_VERSION }); + try { + const output = await runUpdateCommand(["update", "--clear-cache"]); + + expect(output.stdout[0]).toContain( + `A new version of altertable is available: v${UPDATE_TEST_VERSION}`, + ); + expect(readUpdateState().latest_version).toBe(UPDATE_TEST_VERSION); + } finally { + globalThis.fetch = originalFetch; + } }); test("configures automatic check interval", async () => { @@ -608,9 +729,11 @@ describe("update command", () => { UPDATE_TEST_VERSION, ]); - expect(output.stdout[0]).toContain(`altertable ${UPDATE_TEST_VERSION} is available`); - expect(output.stdout[0]).toContain( - `Release: ${releaseUrlForSource(source, UPDATE_TEST_VERSION)}`, + expect(output.stdout[0]).toBe( + [ + `A new version of altertable is available: v${UPDATE_TEST_VERSION} (current v${VERSION})`, + `Run ${UpdaterConfig.commands.selfUpdate} to install it.`, + ].join("\n"), ); } });