mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-10 01:39:25 +00:00
fix(update): don't report success when native update fails (#214)
* fix(update): don't report success when native update fails The native auto-updater spawned `bash -c "curl -fsSL … | bash"`. A pipeline's exit status is that of its last command, so when curl could not connect (e.g. a dead proxy) it produced no output, the trailing bash read empty stdin and exited 0, and the whole command looked successful — printing "Updated … Restart the CLI" while nothing had been installed. Run the spawned shell with `set -o pipefail` so curl's non-zero status propagates. installUpdate() then rejects and runUpdatePreflight() warns and continues on the current version instead of claiming success. * chore: add changeset for native update fix
This commit is contained in:
parent
2388f20bb3
commit
caaa6d83ee
3 changed files with 49 additions and 11 deletions
5
.changeset/fix-native-update-false-success.md
Normal file
5
.changeset/fix-native-update-false-success.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Fix the native self-updater reporting a successful update when the install command actually failed.
|
||||
|
|
@ -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');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue