From 2444cc2f37ea221fe5c8ebff0b8eb9f1c8195755 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 01:20:03 -0700 Subject: [PATCH] refactor(openshell): trim internal exports (#107291) --- .../src/backend.exec-workdir.test.ts | 26 ++- extensions/openshell/src/backend.test.ts | 41 ----- extensions/openshell/src/backend.ts | 8 +- extensions/openshell/src/cli.ts | 12 +- .../openshell/src/openshell-core.test.ts | 158 ++++++++---------- scripts/deadcode-exports.baseline.mjs | 9 - 6 files changed, 102 insertions(+), 152 deletions(-) delete mode 100644 extensions/openshell/src/backend.test.ts diff --git a/extensions/openshell/src/backend.exec-workdir.test.ts b/extensions/openshell/src/backend.exec-workdir.test.ts index 949fc6d6f01f..9a048ff887a2 100644 --- a/extensions/openshell/src/backend.exec-workdir.test.ts +++ b/extensions/openshell/src/backend.exec-workdir.test.ts @@ -96,12 +96,17 @@ describe("openshell backend exec workdir validation", () => { }); afterEach(async () => { + vi.unstubAllEnvs(); await Promise.all( tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true })), ); }); it("reuses validation-time workspace preparation for the following exec", async () => { + vi.stubEnv("OPENAI_API_KEY", "fixture"); + vi.stubEnv("ANTHROPIC_API_KEY", "fixture"); + vi.stubEnv("LANG", "en_US.UTF-8"); + vi.stubEnv("NODE_ENV", "test"); const workspaceDir = await makeTempDir("openclaw-openshell-workspace-"); await fs.writeFile(path.join(workspaceDir, "seed.txt"), "seed", "utf8"); const backendFactory = createOpenShellSandboxBackendFactory({ @@ -112,7 +117,7 @@ describe("openshell backend exec workdir validation", () => { }); const backend = await backendFactory({ sessionKey: "agent:main:turn", - scopeKey: "agent:main", + scopeKey: "agent:somalley_alice:dashboard-8", workspaceDir, agentWorkspaceDir: workspaceDir, cfg: createOpenShellBackendSandboxConfig(), @@ -130,6 +135,25 @@ describe("openshell backend exec workdir validation", () => { ([params]) => params.args[0] === "sandbox" && params.args[1] === "upload", ); expect(uploadCalls).toHaveLength(1); + expect(uploadCalls[0]?.[0]).toMatchObject({ + args: [ + "sandbox", + "upload", + "--no-git-ignore", + backend.runtimeId, + expect.stringMatching(/\/seed\.txt$/), + "/sandbox", + ], + cwd: workspaceDir, + }); + expect(backend.runtimeId).toMatch(/^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/); + expect(backend.runtimeId).toContain("somalley-alice"); + expect(backend.runtimeId).not.toContain("_"); + expect(backend.runtimeId.length).toBeLessThanOrEqual(63); + expect(execSpec.env.OPENAI_API_KEY).toBeUndefined(); + expect(execSpec.env.ANTHROPIC_API_KEY).toBeUndefined(); + expect(execSpec.env.LANG).toBe("en_US.UTF-8"); + expect(execSpec.env.NODE_ENV).toBe("test"); expect(execSpec.argv).toContain("openshell-test"); }); diff --git a/extensions/openshell/src/backend.test.ts b/extensions/openshell/src/backend.test.ts deleted file mode 100644 index 1d6b84adbadf..000000000000 --- a/extensions/openshell/src/backend.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Openshell tests cover backend plugin behavior. -import { afterEach, describe, expect, it } from "vitest"; -import { buildOpenShellSandboxName, buildOpenShellSshExecEnv } from "./backend.js"; - -describe("openshell backend env", () => { - const originalEnv = { ...process.env }; - - afterEach(() => { - for (const key of Object.keys(process.env)) { - if (!(key in originalEnv)) { - delete process.env[key]; - } - } - Object.assign(process.env, originalEnv); - }); - - it("filters blocked secrets from ssh exec env", () => { - process.env.OPENAI_API_KEY = "sk-test-secret"; - process.env.ANTHROPIC_API_KEY = "sk-ant-test-secret"; - process.env.LANG = "en_US.UTF-8"; - process.env.NODE_ENV = "test"; - - const env = buildOpenShellSshExecEnv(); - - expect(env.OPENAI_API_KEY).toBeUndefined(); - expect(env.ANTHROPIC_API_KEY).toBeUndefined(); - expect(env.LANG).toBe("en_US.UTF-8"); - expect(env.NODE_ENV).toBe("test"); - }); -}); - -describe("openshell sandbox names", () => { - it("generates Kubernetes-safe names from OpenClaw session scope keys", () => { - const name = buildOpenShellSandboxName("agent:somalley_alice:dashboard-8"); - - expect(name).toMatch(/^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/); - expect(name).toContain("somalley-alice"); - expect(name).not.toContain("_"); - expect(name.length).toBeLessThanOrEqual(63); - }); -}); diff --git a/extensions/openshell/src/backend.ts b/extensions/openshell/src/backend.ts index a296e85511d7..559c40b3e472 100644 --- a/extensions/openshell/src/backend.ts +++ b/extensions/openshell/src/backend.ts @@ -46,7 +46,7 @@ type PendingExec = { }; const MATERIALIZED_SKILLS_REMOTE_PARTS = [".openclaw", "sandbox-skills"] as const; -export function buildOpenShellDirectoryUploadArgs(params: { +function buildOpenShellDirectoryUploadArgs(params: { sandboxName: string; localPath: string; remotePath: string; @@ -203,12 +203,10 @@ export const ENSURE_OPEN_SHELL_REMOTE_REAL_DIRECTORY_SCRIPT = [ "done", ].join("\n"); -export function buildOpenShellSshExecEnv(): NodeJS.ProcessEnv { +function buildOpenShellSshExecEnv(): NodeJS.ProcessEnv { return sanitizeEnvVars(process.env).allowed; } -export type { OpenShellSandboxBackend } from "./backend.types.js"; - export function createOpenShellSandboxBackendFactory( params: CreateOpenShellSandboxBackendFactoryParams, ): SandboxBackendFactory { @@ -893,7 +891,7 @@ function resolveOpenShellPluginConfigFromConfig( return resolveOpenShellPluginConfig(pluginConfig); } -export function buildOpenShellSandboxName(scopeKey: string): string { +function buildOpenShellSandboxName(scopeKey: string): string { const trimmed = scopeKey.trim() || "session"; const safe = normalizeLowercaseStringOrEmpty(trimmed) .replace(/[^a-z0-9-]+/g, "-") diff --git a/extensions/openshell/src/cli.ts b/extensions/openshell/src/cli.ts index da42b76d63bf..a7db6c107908 100644 --- a/extensions/openshell/src/cli.ts +++ b/extensions/openshell/src/cli.ts @@ -8,10 +8,8 @@ import { import type { ResolvedOpenShellPluginConfig } from "./config.js"; export { - buildExecRemoteCommand, buildRemoteWorkdirValidationCommand, buildValidatedExecRemoteCommand, - shellEscape, } from "openclaw/plugin-sdk/sandbox"; export type OpenShellExecContext = { @@ -20,12 +18,8 @@ export type OpenShellExecContext = { timeoutMs?: number; }; -export function resolveOpenShellCommand(command: string): string { - return command; -} - -export function buildOpenShellBaseArgv(config: ResolvedOpenShellPluginConfig): string[] { - const argv = [resolveOpenShellCommand(config.command)]; +function buildOpenShellBaseArgv(config: ResolvedOpenShellPluginConfig): string[] { + const argv = [config.command]; if (config.gateway) { argv.push("--gateway", config.gateway); } @@ -39,7 +33,7 @@ export function buildRemoteCommand(argv: string[]): string { return argv.map((entry) => shellEscape(entry)).join(" "); } -export function applyGatewayEndpointToSshConfig(params: { +function applyGatewayEndpointToSshConfig(params: { configText: string; gatewayEndpoint?: string; }): string { diff --git a/extensions/openshell/src/openshell-core.test.ts b/extensions/openshell/src/openshell-core.test.ts index 5ddd2c3ffa7e..222dfcac5387 100644 --- a/extensions/openshell/src/openshell-core.test.ts +++ b/extensions/openshell/src/openshell-core.test.ts @@ -4,7 +4,12 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { expectDefined } from "@openclaw/normalization-core"; -import type { CreateSandboxBackendParams } from "openclaw/plugin-sdk/sandbox"; +import { + buildExecRemoteCommand, + disposeSshSandboxSession, + shellEscape, + type CreateSandboxBackendParams, +} from "openclaw/plugin-sdk/sandbox"; import { createSandboxBrowserConfig, createSandboxPruneConfig, @@ -12,15 +17,11 @@ import { createSandboxTestContext, } from "openclaw/plugin-sdk/test-fixtures"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import type { OpenShellSandboxBackend } from "./backend.js"; +import type { OpenShellSandboxBackend } from "./backend.types.js"; import { - applyGatewayEndpointToSshConfig, - buildExecRemoteCommand, buildValidatedExecRemoteCommand, - buildOpenShellBaseArgv, - resolveOpenShellCommand, + createOpenShellSshSession, runOpenShellCli, - shellEscape, } from "./cli.js"; import { resolveOpenShellPluginConfig } from "./config.js"; @@ -30,7 +31,6 @@ const cliMocks = vi.hoisted(() => ({ let createOpenShellSandboxBackendManager: typeof import("./backend.js").createOpenShellSandboxBackendManager; let createOpenShellSandboxBackendFactory: typeof import("./backend.js").createOpenShellSandboxBackendFactory; -let buildOpenShellDirectoryUploadArgs: typeof import("./backend.js").buildOpenShellDirectoryUploadArgs; let ensureOpenShellRemoteRealDirectoryScript: typeof import("./backend.js").ENSURE_OPEN_SHELL_REMOTE_REAL_DIRECTORY_SCRIPT; describe("openshell cli helpers", () => { @@ -45,34 +45,6 @@ describe("openshell cli helpers", () => { Object.assign(process.env, originalEnv); }); - it("builds base argv with gateway overrides", () => { - const config = resolveOpenShellPluginConfig({ - command: "/usr/local/bin/openshell", - gateway: "lab", - gatewayEndpoint: "https://lab.example", - }); - expect(buildOpenShellBaseArgv(config)).toEqual([ - "/usr/local/bin/openshell", - "--gateway", - "lab", - "--gateway-endpoint", - "https://lab.example", - ]); - }); - - it("uses the configured NVIDIA OpenShell CLI command directly", () => { - const config = resolveOpenShellPluginConfig(undefined); - - expect(resolveOpenShellCommand("openshell")).toBe("openshell"); - expect(buildOpenShellBaseArgv(config)).toEqual(["openshell"]); - }); - - it("preserves an explicit NVIDIA OpenShell CLI path", () => { - expect(resolveOpenShellCommand("/opt/openshell/bin/openshell")).toBe( - "/opt/openshell/bin/openshell", - ); - }); - it("shell escapes single quotes", () => { expect(shellEscape(`a'b`)).toBe(`'a'"'"'b'`); }); @@ -133,35 +105,43 @@ describe("openshell cli helpers", () => { ]); }); - it("adds direct gateway endpoints to generated ssh proxy configs", () => { - const configText = [ - "Host openshell-demo", - " User sandbox", - " ProxyCommand /usr/local/bin/openshell ssh-proxy --gateway-name alice --name demo", - "", - ].join("\n"); + it.runIf(process.platform !== "win32")( + "adds direct gateway endpoints to generated ssh proxy configs", + async () => { + const configText = [ + "Host openshell-demo", + " User sandbox", + " ProxyCommand /usr/local/bin/openshell ssh-proxy --gateway-name alice --name demo", + "", + ].join("\n"); - expect( - applyGatewayEndpointToSshConfig({ - configText, - gatewayEndpoint: "http://openshell.openshell-alice.svc.cluster.local:8080", - }), - ).toContain( - "ProxyCommand /usr/local/bin/openshell ssh-proxy --gateway-name alice --name demo --server 'http://openshell.openshell-alice.svc.cluster.local:8080'", - ); - }); + await expect( + readOpenShellSshConfig({ + configText, + gatewayEndpoint: "http://openshell.openshell-alice.svc.cluster.local:8080", + }), + ).resolves.toContain( + "ProxyCommand /usr/local/bin/openshell ssh-proxy --gateway-name alice --name demo --server 'http://openshell.openshell-alice.svc.cluster.local:8080'", + ); + }, + ); - it("leaves ssh proxy configs with an explicit endpoint unchanged", () => { - const configText = - "Host openshell-demo\n ProxyCommand openshell ssh-proxy --gateway-name alice --name demo --server 'http://existing'\n"; + it.runIf(process.platform !== "win32")( + "leaves ssh proxy configs with an explicit endpoint unchanged", + async () => { + const configText = + "Host openshell-demo\n ProxyCommand openshell ssh-proxy --gateway-name alice --name demo --server 'http://existing'\n"; - expect( - applyGatewayEndpointToSshConfig({ - configText, - gatewayEndpoint: "http://replacement", - }), - ).toBe(configText); - }); + await expect( + readOpenShellSshConfig({ + configText, + gatewayEndpoint: "http://replacement", + }), + ).resolves.toContain( + "ProxyCommand openshell ssh-proxy --gateway-name alice --name demo --server 'http://existing'", + ); + }, + ); }); describe("openshell backend manager", () => { @@ -174,7 +154,6 @@ describe("openshell backend manager", () => { }; }); ({ - buildOpenShellDirectoryUploadArgs, ENSURE_OPEN_SHELL_REMOTE_REAL_DIRECTORY_SCRIPT: ensureOpenShellRemoteRealDirectoryScript, createOpenShellSandboxBackendFactory, createOpenShellSandboxBackendManager, @@ -190,30 +169,6 @@ describe("openshell backend manager", () => { vi.clearAllMocks(); }); - it("uploads staged directory snapshots to the managed remote directory itself", () => { - expect( - buildOpenShellDirectoryUploadArgs({ - sandboxName: "openclaw-session", - localPath: "/tmp/openclaw-upload/sandbox/seed.txt", - remotePath: "/sandbox", - }), - ).toEqual([ - "sandbox", - "upload", - "--no-git-ignore", - "openclaw-session", - "/tmp/openclaw-upload/sandbox/seed.txt", - "/sandbox", - ]); - expect( - buildOpenShellDirectoryUploadArgs({ - sandboxName: "openclaw-session", - localPath: "/tmp/openclaw-upload/project", - remotePath: "/sandbox/./project", - }).at(-1), - ).toBe("/sandbox/project"); - }); - it.runIf(process.platform !== "win32")( "preserves caller positional args after OpenShell remote directory validation", async () => { @@ -560,6 +515,35 @@ async function makeExecutable(params: { name: string; script: string }): Promise return file; } +async function readOpenShellSshConfig(params: { + configText: string; + gatewayEndpoint: string; +}): Promise { + const command = await makeExecutable({ + name: "openshell-ssh-config", + script: [ + "#!/bin/sh", + "cat <<'OPENCLAW_SSH_CONFIG'", + params.configText, + "OPENCLAW_SSH_CONFIG", + ].join("\n"), + }); + const session = await createOpenShellSshSession({ + context: { + sandboxName: "demo", + config: resolveOpenShellPluginConfig({ + command, + gatewayEndpoint: params.gatewayEndpoint, + }), + }, + }); + try { + return await fs.readFile(session.configPath, "utf8"); + } finally { + await disposeSshSandboxSession(session); + } +} + async function expectPathMissing(targetPath: string): Promise { let error: unknown; try { diff --git a/scripts/deadcode-exports.baseline.mjs b/scripts/deadcode-exports.baseline.mjs index 45eac3b8eca8..69e31fa280dc 100644 --- a/scripts/deadcode-exports.baseline.mjs +++ b/scripts/deadcode-exports.baseline.mjs @@ -178,17 +178,8 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "extensions/ollama/src/wsl2-crash-loop-check.ts: hasWslCuda", "extensions/ollama/src/wsl2-crash-loop-check.ts: isOllamaEnabledWithRestartAlways", "extensions/ollama/src/wsl2-crash-loop-check.ts: parseSystemctlShowProperties", - "extensions/openshell/src/backend.ts: buildOpenShellDirectoryUploadArgs", - "extensions/openshell/src/backend.ts: buildOpenShellSandboxName", - "extensions/openshell/src/backend.ts: buildOpenShellSshExecEnv", "extensions/openshell/src/backend.ts: ENSURE_OPEN_SHELL_REMOTE_REAL_DIRECTORY_SCRIPT", - "extensions/openshell/src/backend.ts: OpenShellSandboxBackend", "extensions/openshell/src/backend.ts: PINNED_REMOTE_PATH_MUTATION_SCRIPT", - "extensions/openshell/src/cli.ts: applyGatewayEndpointToSshConfig", - "extensions/openshell/src/cli.ts: buildExecRemoteCommand", - "extensions/openshell/src/cli.ts: buildOpenShellBaseArgv", - "extensions/openshell/src/cli.ts: resolveOpenShellCommand", - "extensions/openshell/src/cli.ts: shellEscape", "extensions/parallel/src/parallel-mcp-search.runtime.ts: extractMcpToolPayload", "extensions/parallel/src/parallel-mcp-search.runtime.ts: iterMcpMessages", "extensions/parallel/src/parallel-mcp-search.runtime.ts: selectMcpEnvelope",