From d720d78fee117b4e5cb7e1e6be4e922d97b56d44 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 2 Aug 2026 06:36:43 -0700 Subject: [PATCH] fix(cli): preserve shell profiles when completion install fails (#117987) * fix(cli): publish completion profiles atomically Closes #117980 * fix(ci): restore environment variable budget * fix(cli): preserve dangling completion profile symlinks * fix(cli): narrow completion readlink failures --- docs/cli/completion.md | 2 + extensions/cua-computer/src/driver-client.ts | 8 +- src/cli/completion-runtime.test.ts | 104 ++++++++++++++++++- src/cli/completion-runtime.ts | 58 +++++++++-- src/cli/media-output.ts | 34 +----- src/cli/nodes-camera.test.ts | 19 ++-- src/cli/nodes-camera.ts | 2 +- src/cli/output-file.runtime.ts | 39 +++++++ 8 files changed, 212 insertions(+), 54 deletions(-) create mode 100644 src/cli/output-file.runtime.ts diff --git a/docs/cli/completion.md b/docs/cli/completion.md index 0a2898cb7cb9..afd8f05b6724 100644 --- a/docs/cli/completion.md +++ b/docs/cli/completion.md @@ -40,6 +40,8 @@ The install writes a small `# OpenClaw Completion` block into your shell profile | powershell | `~/.config/powershell/Microsoft.PowerShell_profile.ps1` (on Windows: `Documents/PowerShell/Microsoft.PowerShell_profile.ps1`, or `Documents/WindowsPowerShell/...` for Windows PowerShell) | | zsh | `~/.zshrc` | +Profile changes are staged beside the destination and atomically replace it only after a complete durable write. A failed install leaves an existing profile unchanged. + ## Notes - Without `--install` or `--write-state`, the command prints the script to stdout. diff --git a/extensions/cua-computer/src/driver-client.ts b/extensions/cua-computer/src/driver-client.ts index 1d39ad3486ed..aa111e84c765 100644 --- a/extensions/cua-computer/src/driver-client.ts +++ b/extensions/cua-computer/src/driver-client.ts @@ -45,7 +45,7 @@ export interface CuaDriverSession { // This is an OpenClaw-owned ceiling, not plugin configuration or tool input. // The model can request only computer.act actions; it cannot select a session // or widen this authorization after the node host starts. -const CUA_OPENCLAW_AUTHORIZATION = { +const CUA_DRIVER_AUTHORIZATION = { allowedModes: [SessionPermissionMode.Unrestricted], compatibilityMode: SessionPermissionMode.Unrestricted, unrestrictedAcknowledged: true, @@ -71,13 +71,13 @@ class DirectCuaDriverSession implements CuaDriverSession { // ceiling before a single trusted OpenClaw session is admitted. this.runtime = CuaDriver.createConfigured({ claudeCodeCompatibility: false, - authorization: { ...CUA_OPENCLAW_AUTHORIZATION }, + authorization: { ...CUA_DRIVER_AUTHORIZATION }, }); this.session = createTrustedSession(this.runtime, { publicSession: this.publicSession, mode: SessionPermissionMode.Unrestricted, - ttlSeconds: CUA_OPENCLAW_AUTHORIZATION.maxSessionTtlSeconds, - idleTtlSeconds: CUA_OPENCLAW_AUTHORIZATION.maxIdleTtlSeconds, + ttlSeconds: CUA_DRIVER_AUTHORIZATION.maxSessionTtlSeconds, + idleTtlSeconds: CUA_DRIVER_AUTHORIZATION.maxIdleTtlSeconds, }); } diff --git a/src/cli/completion-runtime.test.ts b/src/cli/completion-runtime.test.ts index 4298452068a5..f01bfca9b5f7 100644 --- a/src/cli/completion-runtime.test.ts +++ b/src/cli/completion-runtime.test.ts @@ -17,6 +17,26 @@ import { usesSlowDynamicCompletion, } from "./completion-runtime.js"; +type PublishOutputFileAtomically = + typeof import("./output-file.runtime.js").publishOutputFileAtomically; + +const outputFileMocks = vi.hoisted(() => ({ + publishOutputFileAtomically: vi.fn(), +})); + +vi.mock("./output-file.runtime.js", async () => { + const actual = await vi.importActual( + "./output-file.runtime.js", + ); + outputFileMocks.publishOutputFileAtomically.mockImplementation( + actual.publishOutputFileAtomically, + ); + return { + ...actual, + publishOutputFileAtomically: outputFileMocks.publishOutputFileAtomically, + }; +}); + const tempDirs = useAutoCleanupTempDirTracker(afterEach); async function withBashCompletionHome( @@ -303,8 +323,9 @@ describe("completion-runtime", () => { await withBashCompletionHome(async ({ homeDir }) => { const cachePath = resolveCompletionCachePath("zsh", "openclaw"); await fs.mkdir(path.dirname(cachePath), { recursive: true }); - await fs.writeFile(cachePath, "OPENCLAW_COMPLETION_LOADED=ready\n", "utf8"); + await fs.writeFile(cachePath, "# completion\n", "utf8"); await fs.writeFile(path.join(homeDir, ".zshrc"), "", "utf8"); + await fs.chmod(path.join(homeDir, ".zshrc"), 0o640); const log = vi.spyOn(console, "log").mockImplementation(() => {}); try { @@ -312,12 +333,93 @@ describe("completion-runtime", () => { expect(log).toHaveBeenCalledWith( "Completion installed. Restart your shell or run: source ~/.zshrc", ); + if (process.platform !== "win32") { + expect((await fs.stat(path.join(homeDir, ".zshrc"))).mode & 0o777).toBe(0o640); + } } finally { log.mockRestore(); } }); }); + it("preserves an existing profile when atomic publication fails", async () => { + await withBashCompletionHome(async ({ homeDir }) => { + const cachePath = resolveCompletionCachePath("zsh", "openclaw"); + const profilePath = path.join(homeDir, ".zshrc"); + await fs.mkdir(path.dirname(cachePath), { recursive: true }); + await fs.writeFile(cachePath, "# completion\n", "utf8"); + await fs.writeFile(profilePath, "export IMPORTANT=keep\n", "utf8"); + await fs.chmod(profilePath, 0o640); + const actual = await vi.importActual( + "./output-file.runtime.js", + ); + outputFileMocks.publishOutputFileAtomically.mockImplementationOnce(async (params) => { + return await actual.publishOutputFileAtomically({ + ...params, + writeTemp: async (tempPath) => { + await params.writeTemp(tempPath); + await fs.truncate(tempPath, 1); + throw new Error("injected completion profile write failure"); + }, + }); + }); + + await expect(installCompletion("zsh", true, "openclaw")).rejects.toThrow( + "Failed to install completion: injected completion profile write failure", + ); + + await expect(fs.readFile(profilePath, "utf8")).resolves.toBe("export IMPORTANT=keep\n"); + if (process.platform !== "win32") { + expect((await fs.stat(profilePath)).mode & 0o777).toBe(0o640); + } + expect(await fs.readdir(homeDir)).toEqual([".zshrc"]); + }); + }); + + it.skipIf(process.platform === "win32")( + "preserves a symlinked profile while replacing its target atomically", + async () => { + await withBashCompletionHome(async ({ homeDir }) => { + const cachePath = resolveCompletionCachePath("zsh", "openclaw"); + const targetDir = tempDirs.make("openclaw-completion-profile-target-"); + const targetPath = path.join(targetDir, "zshrc"); + const profilePath = path.join(homeDir, ".zshrc"); + await fs.mkdir(path.dirname(cachePath), { recursive: true }); + await fs.writeFile(cachePath, "# completion\n", "utf8"); + await fs.writeFile(targetPath, "export IMPORTANT=keep\n", "utf8"); + await fs.symlink(targetPath, profilePath); + + await installCompletion("zsh", true, "openclaw"); + + expect((await fs.lstat(profilePath)).isSymbolicLink()).toBe(true); + await expect(fs.readFile(targetPath, "utf8")).resolves.toContain("export IMPORTANT=keep\n"); + await expect(fs.readFile(targetPath, "utf8")).resolves.toContain(cachePath); + }); + }, + ); + + it.skipIf(process.platform === "win32")( + "preserves a dangling relative profile symlink while creating its target", + async () => { + await withBashCompletionHome(async ({ homeDir }) => { + const cachePath = resolveCompletionCachePath("zsh", "openclaw"); + const managedDir = path.join(homeDir, "managed"); + const targetPath = path.join(managedDir, "zshrc"); + const profilePath = path.join(homeDir, ".zshrc"); + await fs.mkdir(path.dirname(cachePath), { recursive: true }); + await fs.mkdir(managedDir, { recursive: true }); + await fs.writeFile(cachePath, "# completion\n", "utf8"); + await fs.symlink(path.join("managed", "zshrc"), profilePath); + + await installCompletion("zsh", true, "openclaw"); + + expect((await fs.lstat(profilePath)).isSymbolicLink()).toBe(true); + expect(await fs.readlink(profilePath)).toBe(path.join("managed", "zshrc")); + await expect(fs.readFile(targetPath, "utf8")).resolves.toContain("# OpenClaw Completion"); + }); + }, + ); + it("detects slow dynamic Bash completion in the login profile", async () => { await withBashCompletionHome(async ({ homeDir }) => { await fs.writeFile( diff --git a/src/cli/completion-runtime.ts b/src/cli/completion-runtime.ts index fd879a3d3414..6e39b38e8ab1 100644 --- a/src/cli/completion-runtime.ts +++ b/src/cli/completion-runtime.ts @@ -9,6 +9,7 @@ import { } from "@openclaw/normalization-core/string-coerce"; import { resolveStateDir } from "../config/paths.js"; import { pathExists } from "../utils.js"; +import { publishOutputFileAtomically } from "./output-file.runtime.js"; export const COMPLETION_SHELLS = ["zsh", "bash", "powershell", "fish"] as const; export type CompletionShell = (typeof COMPLETION_SHELLS)[number]; @@ -271,6 +272,38 @@ function updateCompletionProfile( return { next, changed: next !== content, hadExisting }; } +async function resolveCompletionProfileWritePath(profilePath: string): Promise { + const profileDir = path.dirname(profilePath); + // Shell startup follows a symlink before `..`; create and canonicalize that lexical parent first. + await fs.mkdir(profileDir, { recursive: true }); + const canonicalDir = await fs.realpath(profileDir); + try { + // Existing dotfile-manager symlinks must keep pointing at the atomically replaced referent. + return await fs.realpath(profilePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } + const linkTarget = await fs.readlink(profilePath).catch((error: unknown) => { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" || code === "EINVAL") { + return undefined; + } + throw error; + }); + if (linkTarget === undefined) { + return path.join(canonicalDir, path.basename(profilePath)); + } + // A dangling relative link is resolved from the directory that physically owns the link. + const targetPath = path.isAbsolute(linkTarget) + ? linkTarget + : `${canonicalDir}${path.sep}${linkTarget}`; + const targetDir = path.dirname(targetPath); + await fs.mkdir(targetDir, { recursive: true }); + return path.join(await fs.realpath(targetDir), path.basename(targetPath)); +} + /** Resolves the shell startup profile path that should contain the OpenClaw completion block. */ export function resolveCompletionProfilePath( shell: CompletionShell, @@ -404,17 +437,19 @@ export async function installCompletion(shell: string, yes: boolean, binName = " const sourceLine = formatCompletionSourceLine(shell, cachePath); try { + let content: string; try { - await fs.access(profilePath); - } catch { - if (!yes) { - console.warn(`Profile not found at ${profilePath}. Created a new one.`); + content = await fs.readFile(profilePath, "utf-8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; } - await fs.mkdir(path.dirname(profilePath), { recursive: true }); - await fs.writeFile(profilePath, "", "utf-8"); + if (!yes) { + console.warn(`Profile not found at ${profilePath}. Creating a new one.`); + } + content = ""; } - const content = await fs.readFile(profilePath, "utf-8"); const update = updateCompletionProfile(content, binName, cachePath, sourceLine); if (!update.changed) { if (!yes) { @@ -428,7 +463,14 @@ export async function installCompletion(shell: string, yes: boolean, binName = " console.log(`${action} completion in ${profilePath}...`); } - await fs.writeFile(profilePath, update.next, "utf-8"); + await publishOutputFileAtomically({ + filePath: await resolveCompletionProfileWritePath(profilePath), + tempPrefix: ".openclaw-completion-profile", + durable: true, + writeTemp: async (tempPath) => { + await fs.writeFile(tempPath, update.next, { encoding: "utf-8", flag: "wx" }); + }, + }); if (!yes) { console.log( `Completion installed. Restart your shell or run: ${formatCompletionReloadCommand(shell, resolveCompletionProfileHint(shell))}`, diff --git a/src/cli/media-output.ts b/src/cli/media-output.ts index eb43c59f9ca2..31468f6dbfc9 100644 --- a/src/cli/media-output.ts +++ b/src/cli/media-output.ts @@ -1,40 +1,10 @@ import fs from "node:fs/promises"; import path from "node:path"; import { detectMime, extensionForMime, normalizeMimeType } from "@openclaw/media-core/mime"; -import { writeSiblingTempFile } from "../infra/sibling-temp-file.js"; import { saveMediaBuffer } from "../media/store.js"; +import { publishOutputFileAtomically } from "./output-file.runtime.js"; -const GENERATED_MEDIA_OUTPUT_TEMP_PREFIX = ".openclaw-media-output"; - -async function resolveExistingOutputMode(filePath: string): Promise { - try { - return (await fs.stat(filePath)).mode & 0o7777; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - return undefined; - } - throw error; - } -} - -export async function publishOutputFileAtomically(params: { - filePath: string; - writeTemp: (tempPath: string) => Promise; -}): Promise { - const dir = path.dirname(params.filePath); - await fs.mkdir(dir, { recursive: true }); - const mode = await resolveExistingOutputMode(params.filePath); - // Stage beside the destination so producer failures never destroy prior user bytes. - const { result } = await writeSiblingTempFile({ - dir, - chmodDir: false, - tempPrefix: GENERATED_MEDIA_OUTPUT_TEMP_PREFIX, - ...(mode === undefined ? {} : { mode }), - writeTemp: params.writeTemp, - resolveFinalPath: () => params.filePath, - }); - return result; -} +export { publishOutputFileAtomically }; export async function writeOutputAsset(params: { buffer: Buffer; diff --git a/src/cli/nodes-camera.test.ts b/src/cli/nodes-camera.test.ts index fe9fae49f146..75aa81a12481 100644 --- a/src/cli/nodes-camera.test.ts +++ b/src/cli/nodes-camera.test.ts @@ -8,7 +8,8 @@ import { } from "../test-utils/camera-url-test-helpers.js"; import { withTempDir } from "../test-utils/temp-dir.js"; -type PublishOutputFileAtomically = typeof import("./media-output.js").publishOutputFileAtomically; +type PublishOutputFileAtomically = + typeof import("./output-file.runtime.js").publishOutputFileAtomically; const fetchGuardMocks = vi.hoisted(() => ({ fetchWithSsrFGuard: vi.fn( @@ -22,7 +23,7 @@ const fetchGuardMocks = vi.hoisted(() => ({ ), })); -const mediaOutputMocks = vi.hoisted(() => ({ +const outputFileMocks = vi.hoisted(() => ({ publishOutputFileAtomically: vi.fn(), })); @@ -30,14 +31,16 @@ vi.mock("../infra/net/fetch-guard.js", () => ({ fetchWithSsrFGuard: fetchGuardMocks.fetchWithSsrFGuard, })); -vi.mock("./media-output.js", async () => { - const actual = await vi.importActual("./media-output.js"); - mediaOutputMocks.publishOutputFileAtomically.mockImplementation( +vi.mock("./output-file.runtime.js", async () => { + const actual = await vi.importActual( + "./output-file.runtime.js", + ); + outputFileMocks.publishOutputFileAtomically.mockImplementation( actual.publishOutputFileAtomically, ); return { ...actual, - publishOutputFileAtomically: mediaOutputMocks.publishOutputFileAtomically, + publishOutputFileAtomically: outputFileMocks.publishOutputFileAtomically, }; }); @@ -112,7 +115,7 @@ describe("nodes camera helpers", () => { writeScreenRecordToFile, writeScreenSnapshotToFile, } = await import("./nodes-screen.js")); - ({ publishOutputFileAtomically } = await vi.importActual("./media-output.js")); + ({ publishOutputFileAtomically } = await vi.importActual("./output-file.runtime.js")); }); beforeEach(() => { @@ -298,7 +301,7 @@ describe("nodes camera helpers", () => { const out = path.join(dir, "x.bin"); await fs.writeFile(out, "existing-screen"); await fs.chmod(out, 0o640); - mediaOutputMocks.publishOutputFileAtomically.mockImplementationOnce(async (params) => { + outputFileMocks.publishOutputFileAtomically.mockImplementationOnce(async (params) => { return await publishOutputFileAtomically({ ...params, writeTemp: async (tempPath) => { diff --git a/src/cli/nodes-camera.ts b/src/cli/nodes-camera.ts index 49f944b04e00..856aa08502c5 100644 --- a/src/cli/nodes-camera.ts +++ b/src/cli/nodes-camera.ts @@ -7,7 +7,6 @@ import { toErrorObject } from "../infra/errors.js"; import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; import { normalizeHostname } from "../infra/net/hostname.js"; import { resolveCliName } from "./cli-name.js"; -import { publishOutputFileAtomically } from "./media-output.js"; import { asBoolean, asNumber, @@ -15,6 +14,7 @@ import { asString, resolveTempPathParts, } from "./nodes-media-utils.js"; +import { publishOutputFileAtomically } from "./output-file.runtime.js"; const MAX_CAMERA_URL_DOWNLOAD_BYTES = 250 * 1024 * 1024; const MAX_CAMERA_BASE64_BYTES = MAX_CAMERA_URL_DOWNLOAD_BYTES; diff --git a/src/cli/output-file.runtime.ts b/src/cli/output-file.runtime.ts new file mode 100644 index 000000000000..ecd487d594a5 --- /dev/null +++ b/src/cli/output-file.runtime.ts @@ -0,0 +1,39 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { writeSiblingTempFile } from "../infra/sibling-temp-file.js"; + +const DEFAULT_CLI_OUTPUT_TEMP_PREFIX = ".openclaw-media-output"; + +async function resolveExistingOutputMode(filePath: string): Promise { + try { + return (await fs.stat(filePath)).mode & 0o7777; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return undefined; + } + throw error; + } +} + +/** Publish a CLI-owned file only after its sibling temp write completes. */ +export async function publishOutputFileAtomically(params: { + filePath: string; + writeTemp: (tempPath: string) => Promise; + tempPrefix?: string; + durable?: boolean; +}): Promise { + const dir = path.dirname(params.filePath); + await fs.mkdir(dir, { recursive: true }); + const mode = await resolveExistingOutputMode(params.filePath); + // Keep prior destination bytes and the existing directory mode untouched until completion. + const { result } = await writeSiblingTempFile({ + dir, + chmodDir: false, + tempPrefix: params.tempPrefix ?? DEFAULT_CLI_OUTPUT_TEMP_PREFIX, + ...(mode === undefined ? {} : { mode }), + ...(params.durable ? { syncTempFile: true, syncParentDir: true } : {}), + writeTemp: params.writeTemp, + resolveFinalPath: () => params.filePath, + }); + return result; +}