fix: tighten CLI utility failure handling (#86918)

* fix: tighten cli utility failure handling

* fix: preserve completion install error cause

* fix: keep update completion refresh best effort
This commit is contained in:
Peter Steinberger 2026-05-26 18:08:44 +01:00 committed by GitHub
parent 82dae95c76
commit 0ec29289c6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 99 additions and 14 deletions

View file

@ -30,6 +30,9 @@ export function ensureCliStartupBuild(params = {}) {
env: params.env ?? process.env,
stdio: params.stdio ?? "inherit",
});
if (result.error) {
throw result.error;
}
const status = result.status ?? (result.signal ? 1 : 0);
if (status !== 0) {
throw new Error(`cliStartup build profile failed with exit code ${status}`);

View file

@ -107,4 +107,21 @@ describe("completion-runtime", () => {
await fs.rm(stateDir, { recursive: true, force: true });
}
});
it("rejects install when the completion cache is missing", async () => {
const homeDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-completion-home-"));
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-completion-state-"));
process.env.HOME = homeDir;
process.env.OPENCLAW_STATE_DIR = stateDir;
try {
await expect(installCompletion("zsh", true, "openclaw")).rejects.toThrow(
"Completion cache not found",
);
} finally {
await fs.rm(homeDir, { recursive: true, force: true });
await fs.rm(stateDir, { recursive: true, force: true });
}
});
});

View file

@ -233,17 +233,15 @@ export async function usesSlowDynamicCompletion(
export async function installCompletion(shell: string, yes: boolean, binName = "openclaw") {
const isShellSupported = isCompletionShell(shell);
if (!isShellSupported) {
console.error(`Automated installation not supported for ${shell} yet.`);
return;
throw new Error(`Automated installation not supported for ${shell} yet.`);
}
const cachePath = resolveCompletionCachePath(shell, binName);
const cacheExists = await pathExists(cachePath);
if (!cacheExists) {
console.error(
throw new Error(
`Completion cache not found at ${cachePath}. Run \`${binName} completion --write-state\` first.`,
);
return;
}
let profilePath: string;
@ -305,6 +303,7 @@ export async function installCompletion(shell: string, yes: boolean, binName = "
);
}
} catch (err) {
console.error(`Failed to install completion: ${err as string}`);
const message = err instanceof Error ? err.message : String(err);
throw new Error(`Failed to install completion: ${message}`, { cause: err });
}
}

View file

@ -427,10 +427,6 @@ export function registerNodesStatusCommands(nodes: Command) {
});
const filteredLabel =
hasFilters && filteredPaired.length !== paired.length ? ` (of ${paired.length})` : "";
defaultRuntime.log(
`Pending: ${pendingRows.length} · Paired: ${filteredPaired.length}${filteredLabel}`,
);
if (opts.json) {
defaultRuntime.writeJson({
pending: pendingRows,
@ -439,6 +435,10 @@ export function registerNodesStatusCommands(nodes: Command) {
return;
}
defaultRuntime.log(
`Pending: ${pendingRows.length} · Paired: ${filteredPaired.length}${filteredLabel}`,
);
if (pendingRows.length > 0) {
const rendered = renderPendingPairingRequestsTable({
pending: pendingRows,

View file

@ -195,7 +195,8 @@ describe("cli program (nodes basics)", () => {
expect(JSON.stringify(json)).not.toContain("paired-token");
expect(JSON.stringify(json)).not.toContain("pair-only-token");
const output = getRuntimeOutput();
expect(output).toContain("Pending: 1 · Paired: 3");
expect(output).toMatch(/^\{/);
expect(output).not.toContain("Pending: 1 · Paired: 3");
expect(output).not.toContain("Effective Only Unknown");
expect(output).not.toContain("unpaired-live");
});

View file

@ -44,6 +44,9 @@ const updateNpmInstalledPlugins = vi.fn();
const loadInstalledPluginIndexInstallRecords = vi.fn(
async (params: { config?: OpenClawConfig } = {}) => params.config?.plugins?.installs ?? {},
);
const checkShellCompletionStatus = vi.fn();
const ensureCompletionCacheExists = vi.fn();
const installCompletion = vi.fn();
const legacyConfigRepairMocks = vi.hoisted(() => ({
repairLegacyConfigForUpdateChannel: vi.fn(),
}));
@ -277,9 +280,20 @@ vi.mock("./update-cli/restart-helper.js", () => ({
vi.mock("../commands/doctor.js", () => ({
doctorCommand: vi.fn(),
}));
vi.mock("../commands/doctor-completion.js", () => ({
checkShellCompletionStatus: (...args: unknown[]) => checkShellCompletionStatus(...args),
ensureCompletionCacheExists: (...args: unknown[]) => ensureCompletionCacheExists(...args),
}));
vi.mock("../commands/doctor/legacy-config-repair.js", () => ({
repairLegacyConfigForUpdateChannel: legacyConfigRepairMocks.repairLegacyConfigForUpdateChannel,
}));
vi.mock("./completion-runtime.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("./completion-runtime.js")>();
return {
...actual,
installCompletion: (...args: unknown[]) => installCompletion(...args),
};
});
// Mock the daemon-cli module
vi.mock("./daemon-cli.js", () => ({
runDaemonInstall: mockedRunDaemonInstall,
@ -793,6 +807,15 @@ describe("update-cli", () => {
config: baseConfig,
outcomes: [],
});
checkShellCompletionStatus.mockResolvedValue({
shell: "zsh",
profileInstalled: false,
cacheExists: false,
cachePath: "/tmp/openclaw-completion.zsh",
usesSlowPattern: false,
});
ensureCompletionCacheExists.mockResolvedValue(true);
installCompletion.mockResolvedValue(undefined);
vi.mocked(runDaemonInstall).mockResolvedValue(undefined);
vi.mocked(runDaemonRestart).mockResolvedValue(true);
vi.mocked(doctorCommand).mockResolvedValue(undefined);
@ -883,6 +906,28 @@ describe("update-cli", () => {
expect(logOutput).not.toContain("Error: spawnSync");
});
it("keeps update completion refresh best-effort when profile install fails", async () => {
setTty(true);
checkShellCompletionStatus.mockResolvedValue({
shell: "zsh",
profileInstalled: true,
cacheExists: true,
cachePath: "/tmp/openclaw-completion.zsh",
usesSlowPattern: true,
});
installCompletion.mockRejectedValueOnce(new Error("EACCES: permission denied"));
await updateCommand({ yes: true, restart: false });
expect(installCompletion).toHaveBeenCalledWith("zsh", true, "openclaw");
const logOutput = vi
.mocked(runtimeCapture.log)
.mock.calls.map((call) => String(call[0]))
.join("\n");
expect(logOutput).toContain("Shell completion refresh failed: EACCES: permission denied");
expect(defaultRuntime.exit).not.toHaveBeenCalledWith(1);
});
it("respawns into the updated package root before running post-update tasks", async () => {
const { entrypoints } = setupUpdatedRootRefresh();
@ -1186,8 +1231,7 @@ describe("update-cli", () => {
.mocked(readConfigFileSnapshot)
.mock.calls.some(
([options]) =>
options?.skipPluginValidation === true &&
options.suppressFutureVersionWarning === true,
options?.skipPluginValidation === true && options.suppressFutureVersionWarning === true,
),
).toBe(true);
expect(defaultRuntime.exit).toHaveBeenCalledWith(0);

View file

@ -1268,7 +1268,7 @@ async function tryInstallShellCompletion(opts: {
defaultRuntime.log(theme.muted("Upgrading shell completion to cached version..."));
const cacheGenerated = await ensureCompletionCacheExists(CLI_NAME);
if (cacheGenerated) {
await installCompletion(status.shell, true, CLI_NAME);
await installShellCompletionForUpdate(status.shell, true);
}
return;
}
@ -1305,7 +1305,16 @@ async function tryInstallShellCompletion(opts: {
return;
}
await installCompletion(status.shell, opts.skipPrompt, CLI_NAME);
await installShellCompletionForUpdate(status.shell, opts.skipPrompt);
}
}
async function installShellCompletionForUpdate(shell: string, yes: boolean): Promise<void> {
try {
await installCompletion(shell, yes, CLI_NAME);
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
defaultRuntime.log(theme.warn(`Shell completion refresh failed: ${message}`));
}
}

View file

@ -85,4 +85,16 @@ describe("ensure-cli-startup-build", () => {
}),
).toThrow("cliStartup build profile failed with exit code 1");
});
it("fails when spawning the cliStartup build profile fails", () => {
const root = makeTempRoot();
expect(() =>
ensureCliStartupBuild({
rootDir: root,
spawnSync: () => ({ error: new Error("spawn denied") }),
stdio: "pipe",
}),
).toThrow("spawn denied");
});
});