From fde87f475f747d128a9311d615f1480b748ea561 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 31 May 2026 12:42:23 +0100 Subject: [PATCH] perf(cli): defer shell env for gateway dispatch --- src/commands/agent-via-gateway.test.ts | 80 +++++++++++++++++++- src/commands/agent-via-gateway.ts | 78 ++++++++++++++------ src/config/io.shell-env-fallback.test.ts | 93 ++++++++++++++++++++++++ src/config/io.ts | 20 ++++- src/gateway/call.ts | 15 +++- 5 files changed, 257 insertions(+), 29 deletions(-) create mode 100644 src/config/io.shell-env-fallback.test.ts diff --git a/src/commands/agent-via-gateway.test.ts b/src/commands/agent-via-gateway.test.ts index 492f1751c44..4b58ed9b9f5 100644 --- a/src/commands/agent-via-gateway.test.ts +++ b/src/commands/agent-via-gateway.test.ts @@ -11,6 +11,16 @@ import type { agentCommand as AgentCommand } from "./agent.js"; const loadConfig = vi.hoisted(() => vi.fn()); const callGateway = vi.hoisted(() => vi.fn()); +const isGatewayCredentialsRequiredError = vi.hoisted(() => + vi.fn( + (value: unknown) => value instanceof Error && value.name === "GatewayCredentialsRequiredError", + ), +); +const isGatewayExplicitAuthRequiredError = vi.hoisted(() => + vi.fn( + (value: unknown) => value instanceof Error && value.name === "GatewayExplicitAuthRequiredError", + ), +); const isGatewayTransportError = vi.hoisted(() => vi.fn((value: unknown) => { if (!(value instanceof Error) || value.name !== "GatewayTransportError") { @@ -210,6 +220,8 @@ function createGatewayNormalCloseError() { vi.mock("../config/io.js", () => ({ getRuntimeConfig: loadConfig, loadConfig })); vi.mock("../gateway/call.js", () => ({ callGateway, + isGatewayCredentialsRequiredError, + isGatewayExplicitAuthRequiredError, isGatewayTransportError, randomIdempotencyKey: () => "idem-1", })); @@ -332,12 +344,74 @@ describe("agentCliCommand", () => { expect(params.sessionId).toBeUndefined(); expect(params.to).toBeUndefined(); expect(request.config).toBe(loadConfig.mock.results[0]?.value); - expect(loadConfig).toHaveBeenCalledWith({ skipPluginValidation: true, pin: false }); + expect(loadConfig).toHaveBeenCalledWith({ + skipPluginValidation: true, + pin: false, + skipShellEnvFallback: true, + }); expect(agentCommand).not.toHaveBeenCalled(); expect(loadAgentSessionModuleMock).not.toHaveBeenCalled(); }); }); + it("retries gateway dispatch with shell env fallback only when credentials need it", async () => { + await withTempStore(async ({ store }) => { + const fastConfig = { + agents: { defaults: { timeoutSeconds: 600 } }, + session: { store, mainKey: "main" }, + }; + const shellEnvConfig = { + ...fastConfig, + gateway: { auth: { mode: "token" as const } }, + }; + loadConfig.mockReset(); + loadConfig.mockReturnValueOnce(fastConfig); + loadConfig.mockReturnValueOnce(shellEnvConfig); + const authError = new Error("gateway agent requires credentials"); + authError.name = "GatewayCredentialsRequiredError"; + callGateway.mockRejectedValueOnce(authError); + mockGatewaySuccessReply(); + + await agentCliCommand({ message: "hi", sessionKey: "agent:main:incident-42" }, runtime); + + expect(loadConfig.mock.calls).toEqual([ + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: true }], + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: false }], + ]); + expect(callGateway).toHaveBeenCalledTimes(2); + expect(requireRecord(callGateway.mock.calls[0]?.[0], "first gateway request").config).toBe( + fastConfig, + ); + expect(requireRecord(callGateway.mock.calls[1]?.[0], "second gateway request").config).toBe( + shellEnvConfig, + ); + }); + }); + + it("retries gateway dispatch with shell env fallback for env URL auth", async () => { + await withTempStore(async ({ store }) => { + const fastConfig = { + agents: { defaults: { timeoutSeconds: 600 } }, + session: { store, mainKey: "main" }, + }; + loadConfig.mockReset(); + loadConfig.mockReturnValueOnce(fastConfig); + loadConfig.mockReturnValueOnce(fastConfig); + const authError = new Error("gateway url override requires explicit credentials"); + authError.name = "GatewayExplicitAuthRequiredError"; + callGateway.mockRejectedValueOnce(authError); + mockGatewaySuccessReply(); + + await agentCliCommand({ message: "hi", sessionKey: "agent:main:incident-42" }, runtime); + + expect(loadConfig.mock.calls).toEqual([ + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: true }], + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: false }], + ]); + expect(callGateway).toHaveBeenCalledTimes(2); + }); + }); + it("scopes legacy explicit session keys to the requested agent", async () => { await withTempStore( async () => { @@ -1486,8 +1560,8 @@ describe("agentCliCommand", () => { expect(fallbackOpts.sessionId).toMatch(/^gateway-fallback-/); expect(fallbackOpts.sessionKey).toBe(`agent:ops:explicit:${fallbackOpts.sessionId}`); expect(loadConfig.mock.calls).toEqual([ - [{ skipPluginValidation: true, pin: false }], - [{ skipPluginValidation: true, pin: false }], + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: true }], + [{ skipPluginValidation: true, pin: false, skipShellEnvFallback: true }], ]); }, { agents: { list: [{ id: "ops", default: true }, { id: "main" }] } }, diff --git a/src/commands/agent-via-gateway.ts b/src/commands/agent-via-gateway.ts index e16278c109d..4f4e620cd07 100644 --- a/src/commands/agent-via-gateway.ts +++ b/src/commands/agent-via-gateway.ts @@ -13,10 +13,13 @@ import { getRuntimeConfig } from "../config/io.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { callGateway, + isGatewayCredentialsRequiredError, + isGatewayExplicitAuthRequiredError, isGatewayTransportError, randomIdempotencyKey, type GatewayRequestFunction, } from "../gateway/call.js"; +import { isGatewaySecretRefUnavailableError } from "../gateway/credentials.js"; import { ADMIN_SCOPE } from "../gateway/operator-scopes.js"; import { parseStrictNonNegativeInteger } from "../infra/parse-finite-number.js"; import { routeLogsToStderr } from "../logging/console.js"; @@ -175,10 +178,14 @@ function resolveGatewayAgentTimeoutMs(timeoutSeconds: number): number { return resolveTimerTimeoutMs((timeoutSeconds + 30) * 1000, 10_000, 10_000); } -function getGatewayDispatchConfig(): OpenClawConfig { +function getGatewayDispatchConfig(options?: { skipShellEnvFallback?: boolean }): OpenClawConfig { // Scoped gateway turns need core agent/session/gateway fields only. The // running gateway owns plugin validation and plugin metadata freshness. - return getRuntimeConfig({ skipPluginValidation: true, pin: false }); + return getRuntimeConfig({ + skipPluginValidation: true, + pin: false, + skipShellEnvFallback: options?.skipShellEnvFallback ?? true, + }); } async function formatPayloadForLog(payload: { @@ -218,6 +225,14 @@ function isSessionResetCommand(message: string): boolean { return /^\/(?:new|reset)(?:\s|$)/i.test(message.trim()); } +function shouldRetryGatewayDispatchWithShellEnvFallback(err: unknown): boolean { + return ( + isGatewayCredentialsRequiredError(err) || + isGatewayExplicitAuthRequiredError(err) || + isGatewaySecretRefUnavailableError(err) + ); +} + function isGatewayAgentEmbeddedFallbackError(err: unknown): boolean { return isGatewayTransportError(err); } @@ -600,7 +615,7 @@ async function agentViaGatewayCommand( ); } - const cfg = getGatewayDispatchConfig(); + let cfg = getGatewayDispatchConfig(); const agentIdRaw = opts.agent?.trim(); const agentId = agentIdRaw ? normalizeAgentId(agentIdRaw) : undefined; if (agentId) { @@ -646,9 +661,9 @@ async function agentViaGatewayCommand( let acceptedGatewayRun = false; let activeConnectionAbortAttempted = false; let activeConnectionAbortSucceeded = false; - let response: GatewayAgentResponse; - try { - response = await withProgress( + let response: GatewayAgentResponse | undefined; + const dispatchGatewayAgentCall = async (activeCfg: OpenClawConfig) => + await withProgress( { label: "Waiting for agent reply…", indeterminate: true, @@ -678,7 +693,7 @@ async function agentViaGatewayCommand( }, expectFinal: true, timeoutMs: gatewayTimeoutMs, - config: cfg, + config: activeCfg, signal: signalBridge.signal, onAccepted: (payload) => { acceptedGatewayRun = true; @@ -699,22 +714,41 @@ async function agentViaGatewayCommand( ...gatewayIdentity, }), ); - } catch (err) { - if ( - isAbortError(err) && - !activeConnectionAbortSucceeded && - (acceptedGatewayRun || activeConnectionAbortAttempted) - ) { - await abortAcceptedGatewayAgentRunWithGatewayCall({ - runId: acceptedRunId, - sessionKey: acceptedSessionKey, - signal: signalBridge.getReceivedSignal(), - runtime, - gatewayIdentity, - config: cfg, - }); + + let retriedWithShellEnvFallback = false; + for (;;) { + try { + response = await dispatchGatewayAgentCall(cfg); + break; + } catch (err) { + if ( + !retriedWithShellEnvFallback && + !acceptedGatewayRun && + shouldRetryGatewayDispatchWithShellEnvFallback(err) + ) { + retriedWithShellEnvFallback = true; + cfg = getGatewayDispatchConfig({ skipShellEnvFallback: false }); + continue; + } + if ( + isAbortError(err) && + !activeConnectionAbortSucceeded && + (acceptedGatewayRun || activeConnectionAbortAttempted) + ) { + await abortAcceptedGatewayAgentRunWithGatewayCall({ + runId: acceptedRunId, + sessionKey: acceptedSessionKey, + signal: signalBridge.getReceivedSignal(), + runtime, + gatewayIdentity, + config: cfg, + }); + } + throw err; } - throw err; + } + if (!response) { + throw new Error("gateway agent call did not return a response"); } if (opts.json) { diff --git a/src/config/io.shell-env-fallback.test.ts b/src/config/io.shell-env-fallback.test.ts new file mode 100644 index 00000000000..f43ffff6141 --- /dev/null +++ b/src/config/io.shell-env-fallback.test.ts @@ -0,0 +1,93 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createConfigIO } from "./io.js"; + +const shellEnvMocks = vi.hoisted(() => ({ + loadShellEnvFallback: vi.fn(), + resolveShellEnvFallbackTimeoutMs: vi.fn(() => 15_000), + shouldDeferShellEnvFallback: vi.fn(() => false), + shouldEnableShellEnvFallback: vi.fn(() => false), +})); + +vi.mock("../infra/shell-env.js", async (importOriginal) => ({ + ...(await importOriginal()), + loadShellEnvFallback: shellEnvMocks.loadShellEnvFallback, + resolveShellEnvFallbackTimeoutMs: shellEnvMocks.resolveShellEnvFallbackTimeoutMs, + shouldDeferShellEnvFallback: shellEnvMocks.shouldDeferShellEnvFallback, + shouldEnableShellEnvFallback: shellEnvMocks.shouldEnableShellEnvFallback, +})); + +async function withConfig(run: (params: { home: string; configPath: string }) => Promise) { + const home = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-shell-env-")); + try { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile( + configPath, + JSON.stringify({ env: { shellEnv: { enabled: true } } }, null, 2), + ); + await run({ home, configPath }); + } finally { + await fs.rm(home, { recursive: true, force: true }); + } +} + +describe("config io shell env fallback", () => { + beforeEach(() => { + vi.clearAllMocks(); + shellEnvMocks.resolveShellEnvFallbackTimeoutMs.mockReturnValue(15_000); + shellEnvMocks.shouldDeferShellEnvFallback.mockReturnValue(false); + shellEnvMocks.shouldEnableShellEnvFallback.mockReturnValue(false); + }); + + it("can defer shell env fallback during config load", async () => { + await withConfig(async ({ home, configPath }) => { + const env = {} as NodeJS.ProcessEnv; + const logger = { error: vi.fn(), warn: vi.fn() }; + const baseOptions = { + configPath, + env, + homedir: () => home, + logger, + observe: false, + }; + + createConfigIO(baseOptions).loadConfig(); + expect(shellEnvMocks.loadShellEnvFallback).toHaveBeenCalledTimes(1); + + shellEnvMocks.loadShellEnvFallback.mockClear(); + createConfigIO({ + ...baseOptions, + shellEnvFallback: "defer", + }).loadConfig(); + expect(shellEnvMocks.loadShellEnvFallback).not.toHaveBeenCalled(); + }); + }); + + it("honors deferred shell env fallback when the config file is missing", async () => { + await withConfig(async ({ home, configPath }) => { + await fs.rm(configPath); + shellEnvMocks.shouldEnableShellEnvFallback.mockReturnValue(true); + const env = {} as NodeJS.ProcessEnv; + const logger = { error: vi.fn(), warn: vi.fn() }; + const baseOptions = { + configPath, + env, + homedir: () => home, + logger, + observe: false, + }; + + createConfigIO({ + ...baseOptions, + shellEnvFallback: "defer", + }).loadConfig(); + expect(shellEnvMocks.loadShellEnvFallback).not.toHaveBeenCalled(); + + createConfigIO(baseOptions).loadConfig(); + expect(shellEnvMocks.loadShellEnvFallback).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/config/io.ts b/src/config/io.ts index 4616b4e987f..098caff0550 100644 --- a/src/config/io.ts +++ b/src/config/io.ts @@ -1355,6 +1355,7 @@ export function createConfigIO( overrides: ConfigIoDeps & { pluginValidation?: "full" | "skip"; preservedLegacyRootKeys?: readonly string[]; + shellEnvFallback?: "load" | "defer"; } = {}, ) { const deps = normalizeDeps(overrides); @@ -1377,7 +1378,11 @@ export function createConfigIO( applyConfigEnvVars(cfg, deps.env); const enabled = shouldEnableShellEnvFallback(deps.env) || cfg.env?.shellEnv?.enabled === true; - if (enabled && !shouldDeferShellEnvFallback(deps.env)) { + if ( + enabled && + overrides.shellEnvFallback !== "defer" && + !shouldDeferShellEnvFallback(deps.env) + ) { loadShellEnvFallback({ enabled: true, env: deps.env, @@ -1649,7 +1654,11 @@ export function createConfigIO( try { maybeLoadDotEnvForConfig(deps.env); if (!deps.fs.existsSync(configPath)) { - if (shouldEnableShellEnvFallback(deps.env) && !shouldDeferShellEnvFallback(deps.env)) { + if ( + overrides.shellEnvFallback !== "defer" && + shouldEnableShellEnvFallback(deps.env) && + !shouldDeferShellEnvFallback(deps.env) + ) { loadShellEnvFallback({ enabled: true, env: deps.env, @@ -2567,9 +2576,13 @@ export function projectConfigOntoRuntimeSourceSnapshot(config: OpenClawConfig): export function loadConfig(options?: { skipPluginValidation?: boolean; pin?: boolean; + skipShellEnvFallback?: boolean; }): OpenClawConfig { const loadFresh = () => - createConfigIO(options?.skipPluginValidation ? { pluginValidation: "skip" } : {}).loadConfig(); + createConfigIO({ + ...(options?.skipPluginValidation ? { pluginValidation: "skip" as const } : {}), + ...(options?.skipShellEnvFallback ? { shellEnvFallback: "defer" as const } : {}), + }).loadConfig(); if (options?.pin === false) { return loadFresh(); } @@ -2582,6 +2595,7 @@ export function loadConfig(options?: { export function getRuntimeConfig(options?: { skipPluginValidation?: boolean; pin?: boolean; + skipShellEnvFallback?: boolean; }): OpenClawConfig { return loadConfig(options); } diff --git a/src/gateway/call.ts b/src/gateway/call.ts index b7705728f78..bfd6852f8fe 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -155,6 +155,13 @@ export class GatewayCredentialsRequiredError extends Error { } } +export class GatewayExplicitAuthRequiredError extends Error { + constructor(message: string) { + super(message); + this.name = "GatewayExplicitAuthRequiredError"; + } +} + export type GatewayTransportErrorJson = { ok: false; error: { @@ -232,6 +239,12 @@ export function isGatewayCredentialsRequiredError( return typeof candidate.method === "string" && typeof candidate.configPath === "string"; } +export function isGatewayExplicitAuthRequiredError( + value: unknown, +): value is GatewayExplicitAuthRequiredError { + return value instanceof Error && value.name === "GatewayExplicitAuthRequiredError"; +} + const defaultCreateGatewayClient = (opts: GatewayClientOptions) => new GatewayClient(opts); const defaultGatewayCallDeps = { createGatewayClient: defaultCreateGatewayClient, @@ -497,7 +510,7 @@ export function ensureExplicitGatewayAuth(params: { ] .filter(Boolean) .join("\n"); - throw new Error(message); + throw new GatewayExplicitAuthRequiredError(message); } type GatewayRemoteSettings = {