mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 15:10:52 +00:00
fix: harden approvals get timeout handling (#66239) (thanks @neeravmakwana)
* fix(cli): harden approvals get timeout handling * fix(cli): sanitize approvals timeout notes * fix(cli): distill approvals timeout note --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -141,19 +141,32 @@ describe("exec approvals CLI", () => {
|
||||
expect(callGatewayFromCli).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
"exec.approvals.get",
|
||||
expect.anything(),
|
||||
expect.objectContaining({ timeout: "60000" }),
|
||||
{},
|
||||
);
|
||||
expect(callGatewayFromCli).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
"config.get",
|
||||
expect.objectContaining({ timeout: "60000" }),
|
||||
{},
|
||||
);
|
||||
expect(callGatewayFromCli).toHaveBeenNthCalledWith(2, "config.get", expect.anything(), {});
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
callGatewayFromCli.mockClear();
|
||||
|
||||
await runApprovalsCommand(["approvals", "get", "--node", "macbook"]);
|
||||
|
||||
expect(callGatewayFromCli).toHaveBeenCalledWith("exec.approvals.node.get", expect.anything(), {
|
||||
nodeId: "node-1",
|
||||
});
|
||||
expect(callGatewayFromCli).toHaveBeenCalledWith("config.get", expect.anything(), {});
|
||||
expect(callGatewayFromCli).toHaveBeenCalledWith(
|
||||
"exec.approvals.node.get",
|
||||
expect.objectContaining({ timeout: "60000" }),
|
||||
{
|
||||
nodeId: "node-1",
|
||||
},
|
||||
);
|
||||
expect(callGatewayFromCli).toHaveBeenCalledWith(
|
||||
"config.get",
|
||||
expect.objectContaining({ timeout: "60000" }),
|
||||
{},
|
||||
);
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
@@ -346,6 +359,38 @@ describe("exec approvals CLI", () => {
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("reports gateway config timeout explicitly", async () => {
|
||||
callGatewayFromCli.mockImplementation(
|
||||
async (method: string, _opts: unknown, params?: unknown) => {
|
||||
if (method === "config.get") {
|
||||
throw new Error("gateway timeout after 10000ms\u001b[2K\u0007\nRPC config.get");
|
||||
}
|
||||
if (method === "exec.approvals.get") {
|
||||
return {
|
||||
path: "/tmp/exec-approvals.json",
|
||||
exists: true,
|
||||
hash: "hash-1",
|
||||
file: { version: 1, agents: {} },
|
||||
};
|
||||
}
|
||||
return { method, params };
|
||||
},
|
||||
);
|
||||
|
||||
await runApprovalsCommand(["approvals", "get", "--gateway", "--timeout", "10000", "--json"]);
|
||||
|
||||
expect(defaultRuntime.writeJson).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
effectivePolicy: {
|
||||
note: "Config fetch timed out. Re-run with a higher --timeout to inspect Effective Policy.",
|
||||
scopes: [],
|
||||
},
|
||||
}),
|
||||
0,
|
||||
);
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("keeps node approvals output when gateway config is unavailable", async () => {
|
||||
callGatewayFromCli.mockImplementation(
|
||||
async (method: string, _opts: unknown, params?: unknown) => {
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
import { formatTimeAgo } from "../infra/format-time/format-relative.ts";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { normalizeOptionalString } from "../shared/string-coerce.js";
|
||||
import { sanitizeForLog } from "../terminal/ansi.js";
|
||||
import { formatDocsLink } from "../terminal/links.js";
|
||||
import { getTerminalTableWidth, renderTable } from "../terminal/table.js";
|
||||
import { isRich, theme } from "../terminal/theme.js";
|
||||
@@ -33,11 +34,16 @@ type ExecApprovalsSnapshot = {
|
||||
type ConfigSnapshotLike = {
|
||||
config?: OpenClawConfig;
|
||||
};
|
||||
type ConfigLoadResult = {
|
||||
config: OpenClawConfig | null;
|
||||
timedOut: boolean;
|
||||
};
|
||||
type ApprovalsTargetSource = "gateway" | "node" | "local";
|
||||
type EffectivePolicyReport = {
|
||||
scopes: ExecPolicyScopeSnapshot[];
|
||||
note?: string;
|
||||
};
|
||||
const APPROVALS_GET_DEFAULT_TIMEOUT_MS = 60_000;
|
||||
|
||||
type ExecApprovalsCliOpts = NodesRpcOpts & {
|
||||
node?: string;
|
||||
@@ -159,59 +165,73 @@ async function saveSnapshotTargeted(params: {
|
||||
|
||||
function formatCliError(err: unknown): string {
|
||||
const msg = formatErrorMessage(err);
|
||||
return msg.includes("\n") ? msg.split("\n")[0] : msg;
|
||||
const firstLine = msg.includes("\n") ? msg.split("\n")[0] : msg;
|
||||
const safe = sanitizeForLog(firstLine);
|
||||
return safe.length > 300 ? `${safe.slice(0, 300)}...` : safe;
|
||||
}
|
||||
|
||||
async function loadConfigForApprovalsTarget(params: {
|
||||
opts: ExecApprovalsCliOpts;
|
||||
source: ApprovalsTargetSource;
|
||||
}): Promise<OpenClawConfig | null> {
|
||||
}): Promise<ConfigLoadResult> {
|
||||
try {
|
||||
if (params.source === "local") {
|
||||
return await readBestEffortConfig();
|
||||
return { config: await readBestEffortConfig(), timedOut: false };
|
||||
}
|
||||
const snapshot = (await callGatewayFromCli(
|
||||
"config.get",
|
||||
params.opts,
|
||||
{},
|
||||
)) as ConfigSnapshotLike;
|
||||
return snapshot.config && typeof snapshot.config === "object" ? snapshot.config : null;
|
||||
} catch {
|
||||
return null;
|
||||
return {
|
||||
config: snapshot.config && typeof snapshot.config === "object" ? snapshot.config : null,
|
||||
timedOut: false,
|
||||
};
|
||||
} catch (err) {
|
||||
return {
|
||||
config: null,
|
||||
timedOut: /^gateway timeout after \d+ms\b/i.test(formatCliError(err)),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
function buildEffectivePolicyReport(params: {
|
||||
cfg: OpenClawConfig | null;
|
||||
configLoad: ConfigLoadResult;
|
||||
source: ApprovalsTargetSource;
|
||||
approvals: ExecApprovalsFile;
|
||||
hostPath: string;
|
||||
}): EffectivePolicyReport {
|
||||
const cfg = params.configLoad.config;
|
||||
const timeoutNote = params.configLoad.timedOut
|
||||
? "Config fetch timed out. Re-run with a higher --timeout to inspect Effective Policy."
|
||||
: null;
|
||||
if (params.source === "node") {
|
||||
if (!params.cfg) {
|
||||
if (!cfg) {
|
||||
return {
|
||||
scopes: [],
|
||||
note: "Gateway config unavailable. Node output above shows host approvals state only, and final runtime policy still intersects with gateway tools.exec.",
|
||||
note:
|
||||
timeoutNote ??
|
||||
"Gateway config unavailable. Node output above shows host approvals state only, and final runtime policy still intersects with gateway tools.exec.",
|
||||
};
|
||||
}
|
||||
return {
|
||||
scopes: collectExecPolicyScopeSnapshots({
|
||||
cfg: params.cfg,
|
||||
cfg,
|
||||
approvals: params.approvals,
|
||||
hostPath: params.hostPath,
|
||||
}),
|
||||
note: "Effective exec policy is the node host approvals file intersected with gateway tools.exec policy.",
|
||||
};
|
||||
}
|
||||
if (!params.cfg) {
|
||||
if (!cfg) {
|
||||
return {
|
||||
scopes: [],
|
||||
note: "Config unavailable.",
|
||||
note: timeoutNote ?? "Config unavailable.",
|
||||
};
|
||||
}
|
||||
return {
|
||||
scopes: collectExecPolicyScopeSnapshots({
|
||||
cfg: params.cfg,
|
||||
cfg,
|
||||
approvals: params.approvals,
|
||||
hostPath: params.hostPath,
|
||||
}),
|
||||
@@ -473,9 +493,9 @@ export function registerExecApprovalsCli(program: Command) {
|
||||
.action(async (opts: ExecApprovalsCliOpts) => {
|
||||
try {
|
||||
const { snapshot, nodeId, source } = await loadSnapshotTarget(opts);
|
||||
const cfg = await loadConfigForApprovalsTarget({ opts, source });
|
||||
const configLoad = await loadConfigForApprovalsTarget({ opts, source });
|
||||
const effectivePolicy = buildEffectivePolicyReport({
|
||||
cfg,
|
||||
configLoad,
|
||||
source,
|
||||
approvals: snapshot.file,
|
||||
hostPath: snapshot.path,
|
||||
@@ -498,7 +518,7 @@ export function registerExecApprovalsCli(program: Command) {
|
||||
defaultRuntime.exit(1);
|
||||
}
|
||||
});
|
||||
nodesCallOpts(getCmd);
|
||||
nodesCallOpts(getCmd, { timeoutMs: APPROVALS_GET_DEFAULT_TIMEOUT_MS });
|
||||
|
||||
const setCmd = approvals
|
||||
.command("set")
|
||||
|
||||
Reference in New Issue
Block a user