Skip to content

Commit f59310a

Browse files
rballiancesteipete
rballiance
and
steipete
authored
fix(doctor): keep completion repair best-effort (#99540)
Treat wrapped shell-profile permission failures as an optional doctor repair, while preserving non-permission failures and the actual failing profile path. Cover install and upgrade flows across EACCES, EPERM, and EROFS. Fixes #99237 Co-authored-by: Peter Steinberger <[email protected]>
1 parent 8d3b459 commit f59310a

3 files changed

Lines changed: 144 additions & 14 deletions

File tree

‎src/commands/doctor-completion.test.ts‎

Lines changed: 106 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,24 @@
11
// Doctor completion tests cover final doctor status summaries and completion messaging.
22
import fs from "node:fs/promises";
3-
import os from "node:os";
43
import path from "node:path";
5-
import { afterEach, describe, expect, it } from "vitest";
4+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
5+
import * as noteModule from "../../packages/terminal-core/src/note.js";
6+
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
67
import { captureEnv, setTestEnvValue } from "../test-utils/env.js";
78
import {
89
checkShellCompletionStatus,
10+
doctorShellCompletion,
911
shellCompletionStatusToHealthFindings,
1012
shellCompletionStatusToRepairEffects,
1113
type ShellCompletionStatus,
1214
} from "./doctor-completion.js";
1315

1416
const originalEnv = captureEnv(["HOME", "OPENCLAW_STATE_DIR", "SHELL"]);
15-
const tempDirs: string[] = [];
17+
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
1618

1719
afterEach(async () => {
1820
originalEnv.restore();
19-
for (const dir of tempDirs.splice(0)) {
20-
await fs.rm(dir, { recursive: true, force: true });
21-
}
21+
vi.restoreAllMocks();
2222
});
2323

2424
function status(overrides: Partial<ShellCompletionStatus> = {}): ShellCompletionStatus {
@@ -34,9 +34,8 @@ function status(overrides: Partial<ShellCompletionStatus> = {}): ShellCompletion
3434

3535
describe("shell completion health mapping", () => {
3636
it("checks an explicit shell instead of the detected environment shell", async () => {
37-
const homeDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-completion-home-"));
38-
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-completion-state-"));
39-
tempDirs.push(homeDir, stateDir);
37+
const homeDir = tempDirs.make("openclaw-completion-home-");
38+
const stateDir = tempDirs.make("openclaw-completion-state-");
4039
setTestEnvValue("HOME", homeDir);
4140
setTestEnvValue("OPENCLAW_STATE_DIR", stateDir);
4241
setTestEnvValue("SHELL", "/bin/zsh");
@@ -102,3 +101,101 @@ describe("shell completion health mapping", () => {
102101
expect(shellCompletionStatusToRepairEffects(current)).toEqual([]);
103102
});
104103
});
104+
105+
const installCompletionMock = vi.hoisted(() => vi.fn());
106+
const spawnSyncMock = vi.hoisted(() => vi.fn(() => ({ status: 0 })));
107+
vi.mock("node:child_process", () => ({ spawnSync: spawnSyncMock }));
108+
vi.mock("../cli/completion-runtime.js", async (importOriginal) => {
109+
const actual = await importOriginal<typeof import("../cli/completion-runtime.js")>();
110+
return {
111+
...actual,
112+
installCompletion: installCompletionMock,
113+
};
114+
});
115+
116+
function mockPrompter(confirmValue = true) {
117+
return {
118+
confirm: vi.fn(async () => confirmValue),
119+
confirmAutoFix: vi.fn(async () => confirmValue),
120+
confirmAggressiveAutoFix: vi.fn(async () => confirmValue),
121+
confirmRuntimeRepair: vi.fn(async () => confirmValue),
122+
select: vi.fn(async (_params, fallback) => fallback),
123+
shouldRepair: true,
124+
shouldForce: false,
125+
repairMode: {
126+
shouldRepair: true,
127+
shouldForce: false,
128+
nonInteractive: false,
129+
canPrompt: true,
130+
updateInProgress: false,
131+
},
132+
} as never;
133+
}
134+
135+
async function setupDoctorCompletionTest(usesSlowPattern: boolean) {
136+
const homeDir = tempDirs.make("openclaw-doctor-home-");
137+
const stateDir = tempDirs.make("openclaw-doctor-state-");
138+
setTestEnvValue("HOME", homeDir);
139+
setTestEnvValue("OPENCLAW_STATE_DIR", stateDir);
140+
setTestEnvValue("SHELL", "/bin/bash");
141+
142+
const profilePath = path.join(homeDir, usesSlowPattern ? ".bashrc" : ".bash_profile");
143+
if (usesSlowPattern) {
144+
await fs.writeFile(
145+
profilePath,
146+
'# test bashrc\n[ -f "/tmp/nonexistent" ] && source <(openclaw completion bash)\n',
147+
"utf-8",
148+
);
149+
const cacheDir = path.join(stateDir, "completions");
150+
await fs.mkdir(cacheDir, { recursive: true });
151+
await fs.writeFile(path.join(cacheDir, "openclaw.bash"), "# completion cache\n", "utf-8");
152+
}
153+
return profilePath;
154+
}
155+
156+
function wrappedFsError(code: string, profilePath: string): Error {
157+
const cause = Object.assign(new Error(`${code}: profile write failed`), {
158+
code,
159+
path: profilePath,
160+
});
161+
return new Error(`Failed to install completion: ${cause.message}`, { cause });
162+
}
163+
164+
describe("doctorShellCompletion", () => {
165+
beforeEach(() => {
166+
installCompletionMock.mockReset();
167+
spawnSyncMock.mockClear();
168+
});
169+
170+
it.each([
171+
{ code: "EACCES", usesSlowPattern: true, action: "upgraded" },
172+
{ code: "EPERM", usesSlowPattern: true, action: "upgraded" },
173+
{ code: "EROFS", usesSlowPattern: true, action: "upgraded" },
174+
{ code: "EACCES", usesSlowPattern: false, action: "installed" },
175+
{ code: "EPERM", usesSlowPattern: false, action: "installed" },
176+
{ code: "EROFS", usesSlowPattern: false, action: "installed" },
177+
])("keeps $action completion best-effort for wrapped $code errors", async (testCase) => {
178+
const profilePath = await setupDoctorCompletionTest(testCase.usesSlowPattern);
179+
installCompletionMock.mockRejectedValue(wrappedFsError(testCase.code, profilePath));
180+
const noteSpy = vi.spyOn(noteModule, "note");
181+
182+
await expect(doctorShellCompletion({} as never, mockPrompter())).resolves.not.toThrow();
183+
184+
expect(noteSpy).toHaveBeenCalledWith(
185+
expect.stringMatching(
186+
new RegExp(
187+
`Shell completion not ${testCase.action}: .* is not writable.*completion --install`,
188+
),
189+
),
190+
"Shell completion",
191+
);
192+
expect(noteSpy).toHaveBeenCalledWith(expect.stringContaining(profilePath), "Shell completion");
193+
});
194+
195+
it("re-throws non-permission errors from installCompletion", async () => {
196+
const profilePath = await setupDoctorCompletionTest(true);
197+
installCompletionMock.mockRejectedValue(wrappedFsError("ENOSPC", profilePath));
198+
199+
await expect(doctorShellCompletion({} as never, mockPrompter())).rejects.toThrow("ENOSPC");
200+
});
201+
});

‎src/commands/doctor-completion.ts‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
type CompletionShell,
1616
} from "../cli/completion-runtime.js";
1717
import type { HealthFinding, HealthRepairEffect } from "../flows/health-checks.js";
18+
import { isErrno } from "../infra/errors.js";
1819
import { resolveOpenClawPackageRoot } from "../infra/openclaw-root.js";
1920
import type { RuntimeEnv } from "../runtime.js";
2021
import type { DoctorPrompter } from "./doctor-prompter.js";
@@ -25,6 +26,15 @@ export type ShellCompletionStatusOptions = {
2526
shell?: CompletionShell;
2627
};
2728

29+
const PROFILE_WRITE_ERROR_CODES = new Set(["EACCES", "EPERM", "EROFS"]);
30+
31+
function findProfileWriteError(err: unknown): NodeJS.ErrnoException | undefined {
32+
if (isErrno(err) && PROFILE_WRITE_ERROR_CODES.has(err.code ?? "")) {
33+
return err;
34+
}
35+
return err instanceof Error ? findProfileWriteError(err.cause) : undefined;
36+
}
37+
2838
function resolveCompletionReloadPath(shell: CompletionShell): string {
2939
if (shell === "powershell") {
3040
return resolveCompletionProfilePath("powershell");
@@ -40,6 +50,28 @@ function formatCompletionReloadNote(
4050
return `Shell completion ${action}. Restart your shell or run: ${formatCompletionReloadCommand(shell, profilePath)}`;
4151
}
4252

53+
async function installCompletionForDoctor(
54+
shell: CompletionShell,
55+
cliName: string,
56+
action: "installed" | "upgraded",
57+
): Promise<void> {
58+
try {
59+
await installCompletion(shell, true, cliName);
60+
note(formatCompletionReloadNote(shell, action), "Shell completion");
61+
} catch (err) {
62+
// Completion is optional, but only profile permission failures are safe to downgrade.
63+
const writeError = findProfileWriteError(err);
64+
if (!writeError) {
65+
throw err;
66+
}
67+
const profilePath = writeError.path ?? resolveCompletionProfilePath(shell);
68+
note(
69+
`Shell completion not ${action}: ${profilePath} is not writable. Run \`${cliName} completion --install\` against a writable profile file.`,
70+
"Shell completion",
71+
);
72+
}
73+
}
74+
4375
/** Generate the completion cache by spawning the CLI. */
4476
async function generateCompletionCache(
4577
options: ShellCompletionStatusOptions = {},
@@ -195,8 +227,7 @@ export async function doctorShellCompletion(
195227
}
196228
}
197229

198-
await installCompletion(status.shell, true, cliName);
199-
note(formatCompletionReloadNote(status.shell, "upgraded"), "Shell completion");
230+
await installCompletionForDoctor(status.shell, cliName, "upgraded");
200231
return;
201232
}
202233

@@ -237,8 +268,7 @@ export async function doctorShellCompletion(
237268
return;
238269
}
239270

240-
await installCompletion(status.shell, true, cliName);
241-
note(formatCompletionReloadNote(status.shell, "installed"), "Shell completion");
271+
await installCompletionForDoctor(status.shell, cliName, "installed");
242272
}
243273
}
244274
}

‎src/scripts/test-projects.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,10 @@ describe("test-projects args", () => {
959959
{
960960
config: "test/vitest/vitest.commands.config.ts",
961961
forwardedArgs: [],
962-
includePatterns: ["src/commands/status.scan.shared.test.ts"],
962+
includePatterns: [
963+
"src/commands/doctor-completion.test.ts",
964+
"src/commands/status.scan.shared.test.ts",
965+
],
963966
watchMode: false,
964967
},
965968
{

0 commit comments

Comments
 (0)