Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-native-update-false-success.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---

Fix the native self-updater reporting a successful update when the install command actually failed.
9 changes: 7 additions & 2 deletions apps/kimi-code/src/cli/update/preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ interface SpawnCommand {
readonly args: readonly string[];
}

function spawnForSource(
export function spawnForSource(
source: InstallSource,
version: string,
platform: NodeJS.Platform,
Expand All @@ -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');
}
Expand Down
46 changes: 37 additions & 9 deletions apps/kimi-code/test/cli/update/preflight.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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');
Expand All @@ -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 });
}
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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);
},
);
});
Loading