diff --git a/.changeset/fix-native-update-false-success.md b/.changeset/fix-native-update-false-success.md new file mode 100644 index 000000000..b373dd789 --- /dev/null +++ b/.changeset/fix-native-update-false-success.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix the native self-updater reporting a successful update when the install command actually failed. diff --git a/apps/kimi-code/src/cli/update/preflight.ts b/apps/kimi-code/src/cli/update/preflight.ts index 385f1807e..692784d4e 100644 --- a/apps/kimi-code/src/cli/update/preflight.ts +++ b/apps/kimi-code/src/cli/update/preflight.ts @@ -77,7 +77,7 @@ interface SpawnCommand { readonly args: readonly string[]; } -function spawnForSource( +export function spawnForSource( source: InstallSource, version: string, platform: NodeJS.Platform, @@ -92,7 +92,12 @@ function spawnForSource( case 'bun-global': return { cmd: bunCommand(platform), args: ['add', '-g', `${NPM_PACKAGE_NAME}@${version}`] }; case 'native': - return { cmd: 'bash', args: ['-c', NATIVE_INSTALL_COMMAND_UNIX] }; + // `curl … | bash` reports only the trailing bash's exit status, so a + // failed download (curl can't connect → empty stdin → bash exits 0) + // would look like a successful update. `pipefail` makes the pipeline + // surface curl's non-zero status so installUpdate() rejects and we warn + // instead of printing "Updated …". + return { cmd: 'bash', args: ['-c', `set -o pipefail; ${NATIVE_INSTALL_COMMAND_UNIX}`] }; case 'unsupported': throw new Error('unsupported install source cannot be auto-installed'); } diff --git a/apps/kimi-code/test/cli/update/preflight.test.ts b/apps/kimi-code/test/cli/update/preflight.test.ts index dc115c9e4..e497cba53 100644 --- a/apps/kimi-code/test/cli/update/preflight.test.ts +++ b/apps/kimi-code/test/cli/update/preflight.test.ts @@ -1,10 +1,11 @@ import type * as ChildProcess from 'node:child_process'; +import { spawnSync } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { readUpdateCache } from '#/cli/update/cache'; -import { runUpdatePreflight } from '#/cli/update/preflight'; +import { runUpdatePreflight, spawnForSource } from '#/cli/update/preflight'; import { promptForInstallConfirmation } from '#/cli/update/prompt'; import type * as PromptModule from '#/cli/update/prompt'; import { refreshUpdateCache } from '#/cli/update/refresh'; @@ -182,7 +183,7 @@ describe('runUpdatePreflight', () => { ); }); - it('native on darwin: spawns bash -c curl|bash', async () => { + it('native on darwin: spawns bash -c with pipefail-guarded curl|bash', async () => { mocks.readUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.refreshUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.detectInstallSource.mockResolvedValue('native'); @@ -193,11 +194,16 @@ describe('runUpdatePreflight', () => { try { const { options } = captureOutput(); await runUpdatePreflight('0.4.0', options); - expect(mocks.spawn).toHaveBeenCalledWith( - 'bash', - ['-c', expect.stringContaining('curl -fsSL https://code.kimi.com/kimi-code/install.sh')], - { stdio: 'inherit' }, - ); + const call = mocks.spawn.mock.calls[0]; + expect(call?.[0]).toBe('bash'); + expect(call?.[2]).toEqual({ stdio: 'inherit' }); + const [flag, script] = call?.[1] as string[]; + expect(flag).toBe('-c'); + // pipefail must come before the pipeline so a failed `curl` is not masked + // by the trailing `bash` exiting 0 (see "surfaces a failed curl" below). + expect(script).toContain('set -o pipefail'); + expect(script).toContain('curl -fsSL https://code.kimi.com/kimi-code/install.sh'); + expect(script).toContain('| bash'); } finally { Object.defineProperty(process, 'platform', { value: originalPlatform }); } @@ -240,15 +246,17 @@ describe('runUpdatePreflight', () => { expect(mocks.spawn).not.toHaveBeenCalled(); }); - it('warns and continues when spawn exits non-zero', async () => { + it('warns and continues when spawn exits non-zero, without claiming success', async () => { mocks.readUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.refreshUpdateCache.mockResolvedValue(cacheWith('0.5.0')); mocks.detectInstallSource.mockResolvedValue('npm-global'); mocks.promptForInstallConfirmation.mockResolvedValue(true); mockSpawnExit(1); - const { stderr, options } = captureOutput(); + const { stdout, stderr, options } = captureOutput(); await expect(runUpdatePreflight('0.4.0', options)).resolves.toBe('continue'); expect(stderr.join('')).toContain('warning: failed to install'); + // A failed install must never print the "Updated …" success line. + expect(stdout.join('')).not.toContain('Updated @moonshot-ai/kimi-code'); }); it('tracks update_prompted telemetry', async () => { @@ -267,3 +275,23 @@ describe('runUpdatePreflight', () => { })); }); }); + +describe('spawnForSource native', () => { + // No spawn mock here — we run real bash to prove the failure contract + // end-to-end. `curl … | bash` reports only the trailing bash's exit status, + // so a curl that never connects (exit 7, empty stdin → bash exits 0) is + // masked and the update is wrongly reported as successful. `set -o pipefail` + // makes the pipeline surface curl's failure. Shadowing `curl` with a shell + // function keeps this offline and deterministic; skipped on Windows (no bash, + // and native auto-install is unsupported there anyway). + it.skipIf(process.platform === 'win32')( + 'surfaces a failed curl download as a non-zero exit', + () => { + const { cmd, args } = spawnForSource('native', '0.5.0', 'darwin'); + const script = `curl() { return 7; }\n${args[1] ?? ''}`; + const result = spawnSync(cmd, [args[0] ?? '-c', script], { encoding: 'utf8' }); + expect(result.error).toBeUndefined(); + expect(result.status).toBeGreaterThan(0); + }, + ); +});