mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-06 01:55:37 +00:00
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
This commit is contained in:
committed by
GitHub
parent
3748fcfd2c
commit
d720d78fee
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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<PublishOutputFileAtomically>(),
|
||||
}));
|
||||
|
||||
vi.mock("./output-file.runtime.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("./output-file.runtime.js")>(
|
||||
"./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<typeof import("./output-file.runtime.js")>(
|
||||
"./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(
|
||||
|
||||
@@ -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<string> {
|
||||
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))}`,
|
||||
|
||||
@@ -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<number | undefined> {
|
||||
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<T>(params: {
|
||||
filePath: string;
|
||||
writeTemp: (tempPath: string) => Promise<T>;
|
||||
}): Promise<T> {
|
||||
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;
|
||||
|
||||
@@ -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<PublishOutputFileAtomically>(),
|
||||
}));
|
||||
|
||||
@@ -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<typeof import("./media-output.js")>("./media-output.js");
|
||||
mediaOutputMocks.publishOutputFileAtomically.mockImplementation(
|
||||
vi.mock("./output-file.runtime.js", async () => {
|
||||
const actual = await vi.importActual<typeof import("./output-file.runtime.js")>(
|
||||
"./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) => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
39
src/cli/output-file.runtime.ts
Normal file
39
src/cli/output-file.runtime.ts
Normal file
@@ -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<number | undefined> {
|
||||
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<T>(params: {
|
||||
filePath: string;
|
||||
writeTemp: (tempPath: string) => Promise<T>;
|
||||
tempPrefix?: string;
|
||||
durable?: boolean;
|
||||
}): Promise<T> {
|
||||
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;
|
||||
}
|
||||
Reference in New Issue
Block a user