File size: 4,295 Bytes
fc93158
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { DEFAULT_EXEC_APPROVAL_TIMEOUT_MS } from "../../infra/exec-approvals.js";
import { parseTimeoutMs } from "../nodes-run.js";

/**
 * Regression test for #12098:
 * `openclaw nodes run` times out after 35s because the CLI transport timeout
 * (35s default) is shorter than the exec approval timeout (120s). The
 * exec.approval.request call must use a transport timeout at least as long
 * as the approval timeout so the gateway has enough time to collect the
 * user's decision.
 *
 * The root cause: callGatewayCli reads opts.timeout for the transport timeout.
 * Before the fix, nodes run called callGatewayCli("exec.approval.request", opts, ...)
 * without overriding opts.timeout, so the 35s CLI default raced against the
 * 120s approval wait on the gateway side. The CLI always lost.
 *
 * The fix: override the transport timeout for exec.approval.request to be at
 * least approvalTimeoutMs + 10_000.
 */

const callGatewaySpy = vi.fn<
  (opts: Record<string, unknown>) => Promise<{ decision: "allow-once" }>
>(async () => ({ decision: "allow-once" }));

vi.mock("../../gateway/call.js", () => ({
  callGateway: callGatewaySpy,
  randomIdempotencyKey: () => "mock-key",
}));

vi.mock("../progress.js", () => ({
  withProgress: (_opts: unknown, fn: () => unknown) => fn(),
}));

describe("nodes run: approval transport timeout (#12098)", () => {
  let callGatewayCli: typeof import("./rpc.js").callGatewayCli;

  beforeAll(async () => {
    ({ callGatewayCli } = await import("./rpc.js"));
  });

  beforeEach(() => {
    callGatewaySpy.mockClear();
    callGatewaySpy.mockResolvedValue({ decision: "allow-once" });
  });

  it("callGatewayCli forwards opts.timeout as the transport timeoutMs", async () => {
    await callGatewayCli("exec.approval.request", { timeout: "35000" } as never, {
      timeoutMs: 120_000,
    });

    expect(callGatewaySpy).toHaveBeenCalledTimes(1);
    const callOpts = callGatewaySpy.mock.calls[0][0];
    expect(callOpts.method).toBe("exec.approval.request");
    expect(callOpts.timeoutMs).toBe(35_000);
  });

  it("fix: overriding transportTimeoutMs gives the approval enough transport time", async () => {
    const approvalTimeoutMs = 120_000;
    // Mirror the production code: parseTimeoutMs(opts.timeout) ?? 0
    const transportTimeoutMs = Math.max(parseTimeoutMs("35000") ?? 0, approvalTimeoutMs + 10_000);
    expect(transportTimeoutMs).toBe(130_000);

    await callGatewayCli(
      "exec.approval.request",
      { timeout: "35000" } as never,
      { timeoutMs: approvalTimeoutMs },
      { transportTimeoutMs },
    );

    expect(callGatewaySpy).toHaveBeenCalledTimes(1);
    const callOpts = callGatewaySpy.mock.calls[0][0];
    expect(callOpts.timeoutMs).toBeGreaterThanOrEqual(approvalTimeoutMs);
    expect(callOpts.timeoutMs).toBe(130_000);
  });

  it("fix: user-specified timeout larger than approval is preserved", async () => {
    const approvalTimeoutMs = 120_000;
    const userTimeout = 200_000;
    // Mirror the production code: parseTimeoutMs preserves valid large values
    const transportTimeoutMs = Math.max(
      parseTimeoutMs(String(userTimeout)) ?? 0,
      approvalTimeoutMs + 10_000,
    );
    expect(transportTimeoutMs).toBe(200_000);

    await callGatewayCli(
      "exec.approval.request",
      { timeout: String(userTimeout) } as never,
      { timeoutMs: approvalTimeoutMs },
      { transportTimeoutMs },
    );

    const callOpts = callGatewaySpy.mock.calls[0][0];
    expect(callOpts.timeoutMs).toBe(200_000);
  });

  it("fix: non-numeric timeout falls back to approval floor", async () => {
    const approvalTimeoutMs = DEFAULT_EXEC_APPROVAL_TIMEOUT_MS;
    // parseTimeoutMs returns undefined for garbage input, ?? 0 ensures
    // Math.max picks the approval floor instead of producing NaN
    const transportTimeoutMs = Math.max(parseTimeoutMs("foo") ?? 0, approvalTimeoutMs + 10_000);
    expect(transportTimeoutMs).toBe(130_000);

    await callGatewayCli(
      "exec.approval.request",
      { timeout: "foo" } as never,
      { timeoutMs: approvalTimeoutMs },
      { transportTimeoutMs },
    );

    const callOpts = callGatewaySpy.mock.calls[0][0];
    expect(callOpts.timeoutMs).toBe(130_000);
  });
});