mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-09 05:22:55 +00:00
perf(cli): defer shell env for gateway dispatch
This commit is contained in:
@@ -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" }] } },
|
||||
|
||||
@@ -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) {
|
||||
|
||||
93
src/config/io.shell-env-fallback.test.ts
Normal file
93
src/config/io.shell-env-fallback.test.ts
Normal file
@@ -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<typeof import("../infra/shell-env.js")>()),
|
||||
loadShellEnvFallback: shellEnvMocks.loadShellEnvFallback,
|
||||
resolveShellEnvFallbackTimeoutMs: shellEnvMocks.resolveShellEnvFallbackTimeoutMs,
|
||||
shouldDeferShellEnvFallback: shellEnvMocks.shouldDeferShellEnvFallback,
|
||||
shouldEnableShellEnvFallback: shellEnvMocks.shouldEnableShellEnvFallback,
|
||||
}));
|
||||
|
||||
async function withConfig(run: (params: { home: string; configPath: string }) => Promise<void>) {
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
Reference in New Issue
Block a user