diff --git a/cli/release-core/launcher.js b/cli/release-core/launcher.js index 867c1d5d6e..368d80a700 100644 --- a/cli/release-core/launcher.js +++ b/cli/release-core/launcher.js @@ -426,9 +426,12 @@ function createLauncher(productConfig) { async function getLatestVersion() { try { - const res = await httpGet( - `https://registry.npmjs.org/${packageName}/latest`, - ) + // The release host is already overridable via NEXT_PUBLIC_CODEBUFF_APP_URL; + // mirror that for the registry so tests can answer the version check with + // a server they control instead of the real npm registry. + const registryUrl = + process.env.CODEBUFF_NPM_REGISTRY_URL || 'https://registry.npmjs.org' + const res = await httpGet(`${registryUrl}/${packageName}/latest`) if (res.statusCode !== 200) return null @@ -897,6 +900,28 @@ function createLauncher(productConfig) { return wrapperVersion } + /** + * The installed version to compare against the registry when deciding whether + * to update, given what getCurrentVersion() could verify. + * + * getCurrentVersion() returns null when the metadata cache is missing or + * unreadable even though the binary itself is still installed. The wrapper + * and its release binary share a version, so the wrapper version is the best + * available record of a binary this wrapper installed; using it keeps genuine + * updates flowing while stopping the full re-download that a null current + * version used to trigger on every launch. Null means there is no installed + * binary to compare — the caller must download. + */ + function getUpdateComparisonVersion(currentVersion) { + if (currentVersion !== null) { + return currentVersion + } + if (fs.existsSync(CONFIG.binaryPath) && wrapperVersion) { + return wrapperVersion + } + return null + } + async function ensureBinaryReady() { const currentVersion = getCurrentVersion() const requiredWrapperVersion = getRequiredWrapperVersion(currentVersion) @@ -905,6 +930,21 @@ function createLauncher(productConfig) { return } + // A missing or unreadable metadata file must not read as "not installed": + // the binary itself may be exactly the release this wrapper installed. The + // metadata is only a cache of what was installed, so re-downloading a + // present binary on every launch wastes a full release transfer — and when + // the release host is unreachable it turns a lost cache file into a hard + // startup failure. Whether the installed binary is genuinely stale is the + // background update check's job; here we only need the binary to exist. + if ( + currentVersion === null && + getCurrentMetadata() === null && + fs.existsSync(CONFIG.binaryPath) + ) { + return + } + // npm installs update this JavaScript wrapper but intentionally preserve the // downloaded binary. If that binary exits before the background update // check starts, it can otherwise remain stuck forever. The wrapper and its @@ -989,10 +1029,18 @@ function createLauncher(productConfig) { const latestVersion = await getLatestVersion() if (!latestVersion) return + // Download new version if the installed binary is missing or outdated. + // getCurrentVersion() returns null when the metadata cache is lost even + // though the binary itself is still installed; that must not read as + // "outdated", or a healthy install re-downloads the full platform binary + // on every launch. The wrapper and its release binary share a version, + // so the wrapper version is the best available record of what was + // installed when the cache is gone. + const comparisonVersion = getUpdateComparisonVersion(currentVersion) if ( - // Download new version if current version is unknown or outdated. - currentVersion === null || - compareVersions(currentVersion, latestVersion) < 0 + !fs.existsSync(CONFIG.binaryPath) || + (comparisonVersion !== null && + compareVersions(comparisonVersion, latestVersion) < 0) ) { const stagedBinary = await stageBinary( latestVersion, @@ -1012,7 +1060,9 @@ function createLauncher(productConfig) { stoppedForUpdate = true resetTerminal({ exitAlternateScreen: true }) - console.log(`Update available: ${currentVersion} → ${latestVersion}`) + console.log( + `Update available: ${comparisonVersion ?? 'unknown'} → ${latestVersion}`, + ) installStagedBinary(stagedBinary) @@ -1459,6 +1509,7 @@ function createLauncher(productConfig) { getCurrentVersion, getMetadataVersion, getRequiredWrapperVersion, + getUpdateComparisonVersion, ensureBinaryReady, isTargetAllowedForThisMachine, CONFIG, diff --git a/cli/src/__tests__/release/launcher-installed-binary.test.ts b/cli/src/__tests__/release/launcher-installed-binary.test.ts new file mode 100644 index 0000000000..cb65bee815 --- /dev/null +++ b/cli/src/__tests__/release/launcher-installed-binary.test.ts @@ -0,0 +1,408 @@ +/** + * The launcher's startup and background-update decisions must be driven by the + * installed binary, not by the freebuff-metadata.json cache that records it. + * + * Regression: when the metadata file was missing or unreadable (a lost cache, + * a cleaned ~/.config/manicode, an interrupted install), getCurrentVersion() + * returned null and the launcher read that as "not installed" — re-downloading + * the full platform binary on EVERY launch even though the correct binary was + * sitting at ~/.config/manicode/freebuff, and hard-failing at startup when the + * release host was unreachable. The metadata is only a cache of what was + * installed; a lost cache must not cost a full re-download. + */ +import { execFileSync } from 'child_process' +import { EventEmitter } from 'events' +import { + existsSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'fs' +import { createServer } from 'http' +import type { AddressInfo } from 'net' +import { tmpdir } from 'os' +import { join } from 'path' + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test' + +const { createLauncher } = require('../../../release-core/launcher.js') + +const VERSION = '0.0.172' +const NEXT_VERSION = '0.0.173' + +let tempConfigDir: string + +function makeLauncher(options: Record = {}) { + return createLauncher({ + packageName: 'freebuff', + displayName: 'Freebuff', + wrapperVersion: VERSION, + includeTreeSitterWasm: false, + configDir: tempConfigDir, + ...options, + }).__testing +} + +/** The target this machine's launcher would select for a fresh install. */ +function defaultTarget() { + return `${process.platform}-${process.arch}` +} + +/** Poll until `done()`, so tests wait on the event rather than on a timer. */ +async function waitFor(done: () => boolean, timeoutMs = 10000) { + const deadline = Date.now() + timeoutMs + while (!done()) { + if (Date.now() > deadline) throw new Error('timed out waiting') + await new Promise((resolve) => setTimeout(resolve, 10)) + } +} + +/** + * Capture process.exit: a relaunched child's handler firing after a test would + * otherwise call the real process.exit and take the runner down. Output is + * silenced so passing runs stay quiet. + */ +let restoreLauncherCapture = () => {} + +function captureLauncherOutput() { + const original = { + error: console.error, + write: process.stderr.write.bind(process.stderr), + exit: process.exit, + } + console.error = () => {} + ;(process.stderr as { write: unknown }).write = () => true + ;(process as { exit: unknown }).exit = (code?: number) => { + void code + } + return () => { + console.error = original.error + ;(process.stderr as { write: unknown }).write = original.write + ;(process as { exit: unknown }).exit = original.exit + } +} + +/** A tar.gz holding a single binary named like this platform's install. */ +function makeReleaseTarball(binaryName: string, script?: string) { + const stageDir = mkdtempSync(join(tmpdir(), 'launcher-release-')) + writeFileSync( + join(stageDir, binaryName), + script ? `#!/bin/sh\n${script}\n` : 'pretend binary', + { mode: 0o755 }, + ) + const archive = join(stageDir, 'out.tar.gz') + execFileSync('tar', ['-czf', archive, '-C', stageDir, binaryName]) + const contents = readFileSync(archive) + rmSync(stageDir, { recursive: true, force: true }) + return contents +} + +/** + * A running CLI process the background update can stop: emits 'exit' when + * killed and records that it was asked to stop. + */ +function makeStubProcess() { + const proc = new EventEmitter() as EventEmitter & { + exitCode: number | null + signalCode: string | null + killed: boolean + kill(signal: string): boolean + } + proc.exitCode = null + proc.signalCode = null + proc.killed = false + proc.kill = (signal) => { + proc.killed = true + proc.emit('exit', 0, null) + return true + } + return proc +} + +/** + * Stand-ins for the release host and the npm registry for one test. The + * release server counts every tarball request; leave `tarball` unset to 404, + * which the retry policy treats as final so failure paths fail fast. The + * registry answers with `registryVersion` (null 404s, which getLatestVersion + * turns into "no update available"). + */ +async function withLocalServers( + options: { registryVersion: string | null; tarball: Buffer | null }, + run: (requests: { + downloadRequests: string[] + registryRequests: string[] + }) => Promise, +) { + const downloadRequests: string[] = [] + const registryRequests: string[] = [] + + const releaseServer = createServer((request, response) => { + downloadRequests.push(request.url ?? '') + if (options.tarball && request.url?.includes('/api/releases/download/')) { + response.writeHead(200) + response.end(options.tarball) + } else { + response.writeHead(404) + response.end('missing') + } + }) + const registryServer = createServer((request, response) => { + registryRequests.push(request.url ?? '') + if (options.registryVersion) { + response.writeHead(200) + response.end(JSON.stringify({ version: options.registryVersion })) + } else { + response.writeHead(404) + response.end('missing') + } + }) + await new Promise((resolve) => + releaseServer.listen(0, '127.0.0.1', resolve), + ) + await new Promise((resolve) => + registryServer.listen(0, '127.0.0.1', resolve), + ) + const releasePort = (releaseServer.address() as AddressInfo).port + const registryPort = (registryServer.address() as AddressInfo).port + + const previous = { + app: process.env.NEXT_PUBLIC_CODEBUFF_APP_URL, + registry: process.env.CODEBUFF_NPM_REGISTRY_URL, + noProxy: process.env.NO_PROXY, + } + process.env.NEXT_PUBLIC_CODEBUFF_APP_URL = `http://127.0.0.1:${releasePort}` + process.env.CODEBUFF_NPM_REGISTRY_URL = `http://127.0.0.1:${registryPort}` + process.env.NO_PROXY = '127.0.0.1' + + try { + await run({ downloadRequests, registryRequests }) + } finally { + if (previous.app === undefined) { + delete process.env.NEXT_PUBLIC_CODEBUFF_APP_URL + } else { + process.env.NEXT_PUBLIC_CODEBUFF_APP_URL = previous.app + } + if (previous.registry === undefined) { + delete process.env.CODEBUFF_NPM_REGISTRY_URL + } else { + process.env.CODEBUFF_NPM_REGISTRY_URL = previous.registry + } + if (previous.noProxy === undefined) delete process.env.NO_PROXY + else process.env.NO_PROXY = previous.noProxy + await new Promise((resolve, reject) => + releaseServer.close((error) => (error ? reject(error) : resolve())), + ) + await new Promise((resolve, reject) => + registryServer.close((error) => (error ? reject(error) : resolve())), + ) + } +} + +beforeEach(() => { + tempConfigDir = mkdtempSync(join(tmpdir(), 'launcher-installed-')) + restoreLauncherCapture = captureLauncherOutput() +}) + +afterEach(() => { + restoreLauncherCapture() + rmSync(tempConfigDir, { recursive: true, force: true }) +}) + +describe('an already-installed binary is not re-downloaded', () => { + test('binary and metadata both present and current', async () => { + const t = makeLauncher() + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + writeFileSync( + t.CONFIG.metadataPath, + JSON.stringify({ version: VERSION, target: defaultTarget() }), + ) + + await withLocalServers( + { registryVersion: null, tarball: null }, + async ({ downloadRequests }) => { + await t.ensureBinaryReady() + expect(downloadRequests).toHaveLength(0) + expect(readFileSync(t.CONFIG.binaryPath, 'utf8')).toBe( + 'installed binary', + ) + }, + ) + }) + + test('binary present, metadata cache lost', async () => { + const t = makeLauncher() + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + + await withLocalServers( + { registryVersion: null, tarball: null }, + async ({ downloadRequests }) => { + await t.ensureBinaryReady() + expect(downloadRequests).toHaveLength(0) + expect(readFileSync(t.CONFIG.binaryPath, 'utf8')).toBe( + 'installed binary', + ) + }, + ) + }) + + test('binary present, metadata cache corrupt', async () => { + const t = makeLauncher() + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + writeFileSync(t.CONFIG.metadataPath, 'not json{') + + await withLocalServers( + { registryVersion: null, tarball: null }, + async ({ downloadRequests }) => { + await t.ensureBinaryReady() + expect(downloadRequests).toHaveLength(0) + expect(readFileSync(t.CONFIG.binaryPath, 'utf8')).toBe( + 'installed binary', + ) + }, + ) + }) + + test('background check stands down when the cache is lost but nothing is newer', async () => { + const t = makeLauncher() + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + const runningProcess = makeStubProcess() + + await withLocalServers( + { registryVersion: VERSION, tarball: null }, + async ({ downloadRequests, registryRequests }) => { + await t.checkForUpdates(runningProcess, () => {}) + // It did consult the registry, and decided nothing was worth fetching. + expect(registryRequests.length).toBeGreaterThan(0) + expect(downloadRequests).toHaveLength(0) + expect(runningProcess.killed).toBe(false) + }, + ) + }) + + test('background check stands down when binary and metadata are current', async () => { + const t = makeLauncher() + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + writeFileSync( + t.CONFIG.metadataPath, + JSON.stringify({ version: VERSION, target: defaultTarget() }), + ) + const runningProcess = makeStubProcess() + + await withLocalServers( + { registryVersion: VERSION, tarball: null }, + async ({ downloadRequests }) => { + await t.checkForUpdates(runningProcess, () => {}) + expect(downloadRequests).toHaveLength(0) + expect(runningProcess.killed).toBe(false) + }, + ) + }) +}) + +describe('the update comparison version', () => { + test('prefers the verified version, then the wrapper, then nothing', () => { + const t = makeLauncher() + + // Nothing installed: nothing to compare, the caller must download. + expect(t.getUpdateComparisonVersion(null)).toBe(null) + + // Binary present but the cache is lost: the wrapper version is the best + // available record of a binary this wrapper installed. + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + expect(t.getUpdateComparisonVersion(null)).toBe(VERSION) + + // A verified version always wins over the wrapper fallback. + writeFileSync( + t.CONFIG.metadataPath, + JSON.stringify({ version: '9.9.9', target: defaultTarget() }), + ) + expect(t.getUpdateComparisonVersion('9.9.9')).toBe('9.9.9') + }) + + test('does not invent a comparison version without an installed binary', () => { + const t = makeLauncher() + expect(t.getUpdateComparisonVersion(null)).toBe(null) + }) +}) + +describe('download fallback behavior', () => { + test('downloads when the binary is missing', async () => { + const t = makeLauncher() + writeFileSync( + t.CONFIG.metadataPath, + JSON.stringify({ version: '1.0.0', target: defaultTarget() }), + ) + const tarball = makeReleaseTarball(t.CONFIG.binaryName) + + await withLocalServers( + { registryVersion: null, tarball }, + async ({ downloadRequests }) => { + await t.ensureBinaryReady() + expect(downloadRequests).toHaveLength(1) + expect(existsSync(t.CONFIG.binaryPath)).toBe(true) + expect( + JSON.parse(readFileSync(t.CONFIG.metadataPath, 'utf8')), + ).toMatchObject({ version: VERSION }) + }, + ) + }) + + test('downloads when there is no binary and no metadata', async () => { + const t = makeLauncher() + const tarball = makeReleaseTarball(t.CONFIG.binaryName) + + await withLocalServers( + { registryVersion: null, tarball }, + async ({ downloadRequests }) => { + await t.ensureBinaryReady() + expect(downloadRequests).toHaveLength(1) + expect(existsSync(t.CONFIG.binaryPath)).toBe(true) + }, + ) + }) + + // The relaunched "binary" is a POSIX shell script (see makeReleaseTarball), + // which the launcher spawns directly and Windows cannot execute — so the + // marker the relaunch writes would never appear there. The download, install, + // and process-stop behavior this test proves is exercised on the platforms + // CI runs; skip the relaunch on Windows. + test.skipIf(process.platform === 'win32')( + 'background update still runs when the cache is lost but the wrapper is behind the registry', + async () => { + // The wrapper is a release behind the registry; the metadata cache is gone. + // The wrapper version is the only record left of what was installed, and it + // is older than the registry — so the update must still happen. + const t = makeLauncher({ wrapperVersion: '0.0.171' }) + writeFileSync(t.CONFIG.binaryPath, 'installed binary') + const marker = join(tempConfigDir, 'updated-ran') + const tarball = makeReleaseTarball( + t.CONFIG.binaryName, + `echo ran > ${marker}`, + ) + const runningProcess = makeStubProcess() + + await withLocalServers( + { registryVersion: NEXT_VERSION, tarball }, + async ({ downloadRequests }) => { + // checkForUpdates never resolves once it relaunches the CLI, so drive + // the test off the relaunched binary's marker file instead; the + // promise is only observed for its rejection. + let updateError: unknown = null + void t + .checkForUpdates(runningProcess, () => {}) + .catch((error: unknown) => { + updateError = error + }) + await waitFor(() => existsSync(marker)) + expect(updateError).toBe(null) + expect(downloadRequests).toHaveLength(1) + expect(runningProcess.killed).toBe(true) + expect( + JSON.parse(readFileSync(t.CONFIG.metadataPath, 'utf8')), + ).toMatchObject({ version: NEXT_VERSION }) + }, + ) + }, + ) +})