diff --git a/docs/inference/configure-inference-timeouts.mdx b/docs/inference/configure-inference-timeouts.mdx index 2b89e2a143b..e2d1c33350e 100644 --- a/docs/inference/configure-inference-timeouts.mdx +++ b/docs/inference/configure-inference-timeouts.mdx @@ -46,6 +46,18 @@ $$nemoclaw onboard This setting is baked into the sandbox image. Recreate an existing sandbox to apply a new value. +Each key bounds a different deadline. +`agents.defaults.timeoutSeconds` bounds one agent run, and `$$nemoclaw agent --timeout ` overrides it for a single run. +`models.providers..timeoutSeconds` bounds one provider request, and no flag overrides it. +Raise the provider key when a turn times out while waiting for the model server, because a longer `--timeout` does not extend the provider request. + +To change a deadline on an existing sandbox instead of recreating it, lower shields first and write the key directly. + +```bash +$$nemoclaw shields down +$$nemoclaw config set --key agents.defaults.timeoutSeconds --value 1800 --restart +``` + diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index acabd927416..bab52c59e7d 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1288,6 +1288,18 @@ Because a delivered turn always writes to one of the two streams, the wrapper re The wrapper prints recovery guidance to `stderr` and exits with status `1`. Pressing `Ctrl+C` interrupts the OpenShell child, and sending `SIGTERM` to the host wrapper forwards `SIGTERM` to that child. NemoClaw waits for OpenShell to stop the in-sandbox turn, replays captured output, and returns status `130` for `SIGINT` or `143` for `SIGTERM`. +When the forwarded argv sets `openclaw agent --timeout `, both captured paths bound the OpenShell command at that value plus 30 seconds. +The extra seconds let the in-sandbox turn report its own timeout first, so the host bound catches only a turn that stops answering. + +These leave the OpenShell wait unbounded: + +- `--timeout 0`. +- A value NemoClaw cannot read as a positive whole number of seconds. +- An argv without `--timeout`. +- A `--timeout` after the `--` argv terminator, which OpenClaw reads as payload rather than as its own flag. + +When the captured output reports that the turn's deadline fired, the wrapper replays the partial output and writes deadline guidance to `stderr`. +It exits with status `1` instead of the upstream status `0`. The diagnostic shell-quotes the sandbox name and forwarded arguments, then redacts detected credential values before writing the recovery command to `stderr`. If redaction changes the recovery command, the diagnostic tells you not to replay it; otherwise, it labels the command as runnable inside the sandbox. For a registered sandbox, both captured paths pin the sandbox's recorded gateway with an explicit `-g`. @@ -1297,11 +1309,12 @@ Raw `stderr`, including structured JSON diagnostics, is forwarded unchanged. NemoClaw appends failed-tool or untrusted-child provenance only from the `stdout` JSON. The wrapper reads completion markers only from the final matching OpenClaw response envelope: a local `{ payloads, meta }` response or a gateway `{ status, result: { payloads, meta } }` response. It ignores earlier JSON progress or log records. -It exits with status `1` when that metadata contains `error.kind: "incomplete_turn"`, `livenessState: "abandoned"`, or `replayInvalid: true`, even when the envelope reports success. +It exits with status `1` when that metadata contains `error.kind: "incomplete_turn"`, `livenessState: "abandoned"`, `replayInvalid: true`, or a `timeoutPhase` value, even when the envelope reports success. Marker-shaped values inside tool results, tool-call arguments, or other descendants do not change the exit status. A turn can run every tool successfully and still become abandoned before it produces a reply. The wrapper writes the unchanged JSON trace to `stdout` before it reports the incomplete turn, so the partial tool trace remains available. The wrapper writes the verdict, the detected markers, and verify-before-retry guidance to `stderr`. +A `timeoutPhase` value names the phase the deadline fired in, so the wrapper writes deadline guidance in place of the generic incomplete-turn text. Tool calls in a partial trace may have already applied side effects, so verify what the turn changed before you retry it. The wrapper passes through an upstream non-zero exit status unchanged. Literal `--json` values consumed by flags such as `-m` or `--reply-channel`, or arguments after `--`, stay on the normal passthrough path. diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts index 6405484c2c3..65636986b37 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts @@ -7,10 +7,15 @@ import { describe, expect, it, vi } from "vitest"; import { type AgentDispatchChild, + AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS, + agentDispatchDeadlineSeconds, agentDispatchStdio, isSilentAgentDispatch, + isTimedOutAgentDispatch, + requestedAgentTimeoutSeconds, runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, + TIMED_OUT_AGENT_TURN_EXIT_CODE, } from "./passthrough-dispatch"; import { computeExitCode, type SandboxExecSignalSource } from "../exec"; @@ -159,3 +164,109 @@ describe("SILENT_AGENT_DISPATCH_EXIT_CODE", () => { expect(SILENT_AGENT_DISPATCH_EXIT_CODE).toBe(1); }); }); + +describe("requestedAgentTimeoutSeconds", () => { + const agent = (...args: string[]) => ["openclaw", "agent", ...args]; + + it("reads a separated --timeout value (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("--agent", "main", "--timeout", "30"))).toBe(30); + }); + + it("reads an equals-form --timeout value (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("--timeout=45", "-m", "hi"))).toBe(45); + }); + + it("requests no deadline when the argv carries no --timeout (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("--agent", "main", "-m", "hi"))).toBeNull(); + }); + + it("returns null for --timeout 0 so the host stays unbounded (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("--timeout", "0"))).toBeNull(); + }); + + it("ignores a --timeout consumed as another option's value (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("-m", "--timeout", "--agent", "main"))).toBeNull(); + }); + + it("ignores anything past the -- terminator (#8723)", () => { + expect(requestedAgentTimeoutSeconds(agent("--", "--timeout", "30"))).toBeNull(); + }); + + it("refuses a value that cannot be a deadline (#8723)", () => { + for (const raw of ["-5", "1.5", "abc", "", "1e3"]) { + expect(requestedAgentTimeoutSeconds(agent("--timeout", raw))).toBeNull(); + } + expect(requestedAgentTimeoutSeconds(agent("--timeout"))).toBeNull(); + }); +}); + +describe("agentDispatchDeadlineSeconds", () => { + it("outlasts the requested deadline so the turn reports its own timeout (#8723)", () => { + expect(agentDispatchDeadlineSeconds(["openclaw", "agent", "--timeout", "30"])).toBe( + 30 + AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS, + ); + }); + + it("leaves the transport unbounded when no deadline was requested (#8723)", () => { + expect(agentDispatchDeadlineSeconds(["openclaw", "agent", "-m", "hi"])).toBeUndefined(); + }); + + it("holds the deadline buffer above the longest aborted-run finish measured (#8723)", () => { + expect(AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS).toBeGreaterThan(20); + }); + + it("stays unbounded when the buffered deadline leaves the safe-integer range (#8723)", () => { + const ceiling = String(Number.MAX_SAFE_INTEGER); + expect(requestedAgentTimeoutSeconds(["openclaw", "agent", "--timeout", ceiling])).toBe( + Number.MAX_SAFE_INTEGER, + ); + // The buffer would round past the ceiling, so the argv would carry a + // deadline that differs from the one the caller asked for. + expect(agentDispatchDeadlineSeconds(["openclaw", "agent", "--timeout", ceiling])).toBeUndefined(); + }); + + it("still bounds the largest deadline that survives the buffer (#8723)", () => { + const largest = String(Number.MAX_SAFE_INTEGER - AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS); + expect(agentDispatchDeadlineSeconds(["openclaw", "agent", "--timeout", largest])).toBe( + Number.MAX_SAFE_INTEGER, + ); + }); +}); + +describe("isTimedOutAgentDispatch", () => { + const timeoutReport = + "Request timed out before a response was generated. Please try again, or increase `agents.defaults.timeoutSeconds` in your config."; + + it("classifies the timeout report OpenClaw writes to stdout (#8723)", () => { + expect(isTimedOutAgentDispatch(`${timeoutReport}\n`, "")).toBe(true); + }); + + it("classifies a timeout report that arrives below tool-failure lines (#8723)", () => { + const captured = `LLM request failed.\nTool Call failed\n${timeoutReport}\n`; + expect(isTimedOutAgentDispatch(captured, "")).toBe(true); + }); + + it("classifies a timeout report routed to stderr instead (#8723)", () => { + expect(isTimedOutAgentDispatch("", `${timeoutReport}\n`)).toBe(true); + }); + + it("keeps classifying when the configuration advice is reworded upstream (#8723)", () => { + const reworded = "Request timed out before a response was generated. Raise the deadline."; + expect(isTimedOutAgentDispatch(reworded, "")).toBe(true); + }); + + it("leaves an ordinary answer unclassified (#8723)", () => { + expect(isTimedOutAgentDispatch("PONG\n", "openclaw warning\n")).toBe(false); + }); + + it("leaves an unrelated timed-out message unclassified (#8723)", () => { + const mcpFailure = "McpError: MCP error -32001: Request timed out\n"; + expect(isTimedOutAgentDispatch(mcpFailure, "")).toBe(false); + }); +}); + +describe("TIMED_OUT_AGENT_TURN_EXIT_CODE", () => { + it("reports a turn failure rather than success (#8723)", () => { + expect(TIMED_OUT_AGENT_TURN_EXIT_CODE).toBe(1); + }); +}); diff --git a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts index 5e25216d9a2..8d0359d30b4 100644 --- a/src/lib/actions/sandbox/agent/passthrough-dispatch.ts +++ b/src/lib/actions/sandbox/agent/passthrough-dispatch.ts @@ -53,6 +53,27 @@ // - Removal condition: none while NemoClaw owns the host-side OpenShell // child lifecycle. // +// 9. Timed-out turn guard (deadline contract). +// +// - Invalid state: the turn's deadline fires, OpenClaw reports the timeout, +// and the dispatch still exits 0 (#8723). Measured on three platforms and +// on both transports, so a run that never answered is indistinguishable +// from one that did, and a CI job or evaluation harness records the +// timed-out turn as a pass. The empty-dispatch guard above cannot catch +// it: the timeout report itself makes the streams non-empty. +// - Source boundary: OpenClaw owns the deadline and the exit code, and that +// code is the same whether this wrapper or a bare `openshell sandbox exec` +// runs the turn. NemoClaw owns what it reports to its own caller, so it +// classifies a timeout the way it already classifies an embedded-fallback +// run rather than forwarding a success. +// - Detection differs per transport because the evidence does. The JSON +// transport reads the declared `meta.timeoutPhase` field, in +// `openClawAgentIncompleteTurnSignal`. The non-JSON transport has only +// text, so it matches the sentence OpenClaw prints, exactly as the +// embedded-fallback branch matches its own banner. +// - Removal condition: drop this guard when `openclaw agent` exits non-zero +// for a turn whose deadline fired. +// // Regression tests: `passthrough-dispatch.test.ts` owns the classifier and the // supervised process lifecycle; `passthrough-help.test.ts` owns the diagnostic // text. @@ -230,3 +251,119 @@ export function isSilentAgentDispatch( ): boolean { return !result.error && result.status === 0 && stdout.length === 0 && stderr.length === 0; } + +/** + * Exit code for a turn whose deadline fired without producing a result. + * Matches the wrapper's other non-recoverable dispatch failures. + */ +export const TIMED_OUT_AGENT_TURN_EXIT_CODE = 1; + +/** + * The sentence OpenClaw prints when a turn's deadline fires. + * + * Read from the OpenClaw 2026.7.1 bundle, where it is a single string literal + * in one file, and observed verbatim on stdout, sometimes below tool-failure + * lines. Only the invariant clause is matched so the configuration advice that + * follows it can be reworded upstream without disabling the guard. + */ +const OPENCLAW_AGENT_TIMEOUT_PATTERN = + /(?:^|\r?\n)Request timed out before a response was generated[^\r\n]*(?:\r?\n)?$/i; + +/** + * True when the captured output reports that the turn's deadline fired. + * + * Text is the only evidence the non-JSON transport has. OpenClaw writes the + * report as the final line, so matching that position avoids treating a normal + * reply that quotes or explains the sentence as a timeout. Callers gate on an + * otherwise successful exit, so an upstream non-zero code is never rewritten. + */ +export function isTimedOutAgentDispatch(stdout: string, stderr: string): boolean { + return OPENCLAW_AGENT_TIMEOUT_PATTERN.test(stdout) || OPENCLAW_AGENT_TIMEOUT_PATTERN.test(stderr); +} + +/** Documented `openclaw agent` options that consume the next argv element. */ +export const OPENCLAW_AGENT_VALUE_FLAGS = new Set([ + "-a", + "--agent", + "-m", + "--message", + "--model", + "--provider", + "--reply-channel", + "--session-id", + "--session-key", + "--thinking", + "--timeout", + "--to", +]); + +/** Documented `openclaw agent` options that consume no argv element. */ +export const OPENCLAW_AGENT_BOOLEAN_FLAGS = new Set(["--deliver"]); + +/** + * Extra seconds added to a requested `--timeout` before the host transport + * stops waiting. + * + * The in-sandbox turn owns the deadline and answers first while it can still + * write to stderr: it reports the timeout, names the config key, and exits. + * Only a turn that stops answering reaches the host bound, so the extra seconds + * must outlast an ordinary late finish. + * + * This value is a choice, not a derivation. #8723 timed five aborted runs + * finishing 0.1 s to 20.8 s after their deadline, and four further aborted runs + * recorded no finish at all, so no measurement establishes an upper bound. + * Below roughly five seconds the host truncates the turn's own timeout report; + * above roughly a minute the host bound no longer catches a turn that stops + * answering. Thirty is inside that range and above every post-deadline finish + * #8723 recorded. Choose another value inside that range if a slower model or a + * busier host requires it. + */ +export const AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS = 30; + +/** + * The `--timeout` an `openclaw agent` argv requests, or null when the argv + * requests none. + * + * Mirrors the documented flag grammar only far enough to read one value. + * Anything unrecognized, malformed, or past a `--` terminator returns null so + * the host keeps the wait unbounded rather than shortening a turn without + * evidence. `--timeout 0` disables the deadline upstream and returns null here + * for the same reason. + */ +export function requestedAgentTimeoutSeconds(argv: readonly string[]): number | null { + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index] as string; + if (arg === "--") return null; + if (arg === "--timeout") return parseDeadlineSeconds(argv[index + 1]); + if (arg.startsWith("--timeout=")) return parseDeadlineSeconds(arg.slice("--timeout=".length)); + if (OPENCLAW_AGENT_VALUE_FLAGS.has(arg)) { + index += 1; + continue; + } + } + return null; +} + +function parseDeadlineSeconds(raw: string | undefined): number | null { + if (raw === undefined || !/^\d+$/.test(raw)) return null; + const seconds = Number(raw); + return Number.isSafeInteger(seconds) && seconds > 0 ? seconds : null; +} + +/** + * The host transport deadline for an `openclaw agent` argv, or undefined when + * the argv requested none. Undefined leaves `openshell sandbox exec` on its own + * default, which is no timeout. + * + * A requested deadline near the safe-integer ceiling stays unbounded rather + * than becoming a bound the host cannot represent. Past that ceiling the buffer + * addition rounds, so the wait would silently differ from the number written to + * the command line. That matches how this module treats every other value it + * cannot read. + */ +export function agentDispatchDeadlineSeconds(argv: readonly string[]): number | undefined { + const requested = requestedAgentTimeoutSeconds(argv); + if (requested === null) return undefined; + const deadline = requested + AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS; + return Number.isSafeInteger(deadline) ? deadline : undefined; +} diff --git a/src/lib/actions/sandbox/agent/passthrough-help.test.ts b/src/lib/actions/sandbox/agent/passthrough-help.test.ts index 756f65df129..61dc6b8be95 100644 --- a/src/lib/actions/sandbox/agent/passthrough-help.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-help.test.ts @@ -7,6 +7,7 @@ import { hasAgentPassthroughHelpToken, printAgentPassthroughHelp, writeSilentAgentDispatchFailure, + writeTimedOutAgentTurnFailure, } from "./passthrough-help"; function collectStderr() { @@ -157,3 +158,105 @@ describe("writeSilentAgentDispatchFailure", () => { expect(lines.every((line) => line.endsWith("\n"))).toBe(true); }); }); + +describe("writeTimedOutAgentTurnFailure", () => { + it("names the sandbox and states that the deadline fired (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant"); + + const written = lines.join(""); + expect(written).toContain("'my-assistant' timed out before producing a result"); + expect(written).toContain("the deadline fired and no result reached this command"); + }); + + it("names the phase the payload declared (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant", "provider"); + + expect(lines.join("")).toContain("timed out in the provider phase before producing a result"); + }); + + it("omits a phase label that could forge terminal output (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant", "provider\n forged\u001b[31m"); + + const written = lines.join(""); + expect(written).toContain("'my-assistant' timed out before producing a result"); + expect(written).not.toContain("forged"); + expect(written).not.toContain("\u001b"); + }); + + it.each([undefined, "provider"])( + "renders a sandbox name safely when the timeout phase is %s (#8723)", + (phase) => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "sandbox\n forged\u001b[31m", phase); + + const written = lines.join(""); + expect(written).toContain(String.raw`sandbox\u000a forged\u001b[31m`); + expect(written).not.toContain("sandbox\n forged"); + expect(written).not.toContain("\u001b"); + }, + ); + + it("warns that the partial trace may already have applied side effects (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant", "provider"); + + const written = lines.join(""); + expect(written).toContain("partial trace"); + expect(written).toContain("may have already applied side effects"); + expect(written).toContain("before retrying"); + }); + + it("offers the documented commands that read the trace and raise a deadline (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant"); + + const written = lines.join(""); + expect(written).toContain("'my-assistant' sessions list"); + expect(written).toContain("'my-assistant' sessions export "); + expect(written).toContain( + "'my-assistant' config set --key --value --restart", + ); + // Writing the config fails while shields are up, so the order is part of + // the guidance rather than a detail the reader has to discover. + expect(written.indexOf("shields down")).toBeLessThan(written.indexOf("config set")); + }); + + it("names both deadlines instead of offering --timeout as the fix (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant", "provider"); + + const written = lines.join(""); + expect(written).toContain("agents.defaults.timeoutSeconds bounds the run"); + expect(written).toContain("models.providers..timeoutSeconds"); + expect(written).toContain("no flag overrides it"); + // A provider-phase timeout does not respond to the flag, so it is never + // presented as a runnable recovery command. + expect(written).not.toMatch(/^ {4}\S*nemoclaw.* agent --timeout/m); + }); + + it("shell-quotes a sandbox name that carries shell metacharacters (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "sb; rm -rf /"); + + expect(lines.join("")).toContain("'sb; rm -rf /' sessions list"); + }); + + it("terminates every emitted line (#8723)", () => { + const { lines, proc } = collectStderr(); + + writeTimedOutAgentTurnFailure(proc, "my-assistant", "provider"); + + expect(lines.every((line) => line.endsWith("\n"))).toBe(true); + }); +}); diff --git a/src/lib/actions/sandbox/agent/passthrough-help.ts b/src/lib/actions/sandbox/agent/passthrough-help.ts index c89e25d4937..6c13f59a1c9 100644 --- a/src/lib/actions/sandbox/agent/passthrough-help.ts +++ b/src/lib/actions/sandbox/agent/passthrough-help.ts @@ -3,6 +3,7 @@ import { CLI_NAME } from "../../../cli/branding"; import { shellQuote } from "../../../core/shell-quote"; +import { sanitizeReadinessText } from "../../../readiness/sanitize"; import { redactFull } from "../../../security/redact"; /** Stderr sink for the passthrough's operator-facing failure text. */ @@ -83,6 +84,62 @@ export function writeIncompleteAgentTurnFailure( ); } +/** + * Report a turn whose deadline fired before it produced a result (#8723). The + * partial output has already been written verbatim, so this adds the verdict, + * the phase when the payload declares one, and where the deadline that fired + * actually lives. Retrying blind can re-apply side effects the timed-out turn + * already made. + * + * `--timeout` is described rather than offered as a recovery command because a + * measured provider-phase timeout does not respond to it: `--timeout N` sets the + * embedded run deadline (`embedded run timeout timeoutMs=N000`), while the + * provider request keeps the deadline from `models.providers..timeoutSeconds` + * (`[model-fetch] start ... timeoutMs=60000` was unchanged by `--timeout 150`). + */ +export function writeTimedOutAgentTurnFailure( + proc: AgentPassthroughDiagnosticProcess, + sandboxName: string, + timeoutPhase?: string, +): void { + const sandboxDisplay = sanitizeReadinessText(sandboxName, 200); + const target = shellQuote(sandboxDisplay); + const diagnosticPhase = + timeoutPhase && /^[a-z0-9][a-z0-9_-]{0,63}$/i.test(timeoutPhase) ? timeoutPhase : undefined; + proc.stderr.write( + diagnosticPhase + ? ` The agent turn in sandbox '${sandboxDisplay}' timed out in the ${diagnosticPhase} phase before producing a result.\n` + : ` The agent turn in sandbox '${sandboxDisplay}' timed out before producing a result.\n`, + ); + proc.stderr.write( + " Reporting this as a failure: the deadline fired and no result reached this command.\n", + ); + proc.stderr.write( + " The output above is a partial trace. Tool calls in it may have already applied side effects.\n", + ); + proc.stderr.write(" Documented recovery paths:\n"); + proc.stderr.write( + ` ${CLI_NAME} ${target} sessions list — locate the session key\n`, + ); + proc.stderr.write( + ` ${CLI_NAME} ${target} sessions export — export the partial transcript\n`, + ); + proc.stderr.write( + ` ${CLI_NAME} ${target} shields down — unlock configuration writes\n`, + ); + proc.stderr.write( + ` ${CLI_NAME} ${target} config set --key --value --restart — raise the deadline\n`, + ); + proc.stderr.write( + " Two keys carry a deadline. agents.defaults.timeoutSeconds bounds the run, and\n", + ); + proc.stderr.write( + " `agent --timeout ` overrides it for a single run. models.providers..timeoutSeconds\n", + ); + proc.stderr.write(" bounds the provider request, and no flag overrides it.\n"); + proc.stderr.write(" Inspect the partial output and affected resources before retrying.\n"); +} + export function hasAgentPassthroughHelpToken(args: readonly string[]): boolean { for (const arg of args) { if (arg === "--") break; diff --git a/src/lib/actions/sandbox/agent/passthrough-json.test.ts b/src/lib/actions/sandbox/agent/passthrough-json.test.ts index 3fc19c83987..73d1faa5006 100644 --- a/src/lib/actions/sandbox/agent/passthrough-json.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough-json.test.ts @@ -77,6 +77,53 @@ describe("runAgentJsonPassthrough", () => { expect(exit).toHaveBeenCalledWith(0); }); + it("bounds the host transport when the turn requests a deadline (#8723)", async () => { + const runDispatch = vi.fn(async (_binary: string, _args: readonly string[]) => ({ + status: 0, + signal: null, + stdout: "{}", + stderr: "openclaw banner\n", + })); + const { proc } = makeProc(); + + await expect( + runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json", "--timeout", "30"], proc, { + getGatewayName: () => null, + getOpenshellBinary: () => "/usr/local/bin/openshell", + stdinIsTty: () => false, + runDispatch, + }), + ).rejects.toThrow(/__exit:/); + + const argv = [...(runDispatch.mock.calls[0]?.[1] ?? [])]; + const transportFlags = argv.slice(0, argv.indexOf("--")); + // Outlasts the requested deadline so the turn still reports its own timeout. + expect(transportFlags).toContain("--timeout"); + expect(transportFlags[transportFlags.indexOf("--timeout") + 1]).toBe("60"); + }); + + it("leaves the host transport unbounded when the turn requests no deadline (#8723)", async () => { + const runDispatch = vi.fn(async (_binary: string, _args: readonly string[]) => ({ + status: 0, + signal: null, + stdout: "{}", + stderr: "openclaw banner\n", + })); + const { proc } = makeProc(); + + await expect( + runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { + getGatewayName: () => null, + getOpenshellBinary: () => "/usr/local/bin/openshell", + stdinIsTty: () => false, + runDispatch, + }), + ).rejects.toThrow(/__exit:/); + + const argv = [...(runDispatch.mock.calls[0]?.[1] ?? [])]; + expect(argv.slice(0, argv.indexOf("--"))).not.toContain("--timeout"); + }); + it("surfaces spawn errors and exits with the computed transport failure code", async () => { const runDispatch = vi.fn(async () => ({ status: null, @@ -281,6 +328,49 @@ describe("runAgentJsonPassthrough", () => { expect(exit).toHaveBeenCalledWith(1); }); + it("exits non-zero with deadline guidance for a turn the payload marks timed out (#8723)", async () => { + // The shape measured on a real timed-out run: the envelope reports a + // timeout, the payload holds the partial answer, and `livenessState` is + // whatever the run happened to reach, so only `timeoutPhase` classifies it. + const payload = JSON.stringify({ + status: "timeout", + result: { + payloads: [{ text: "1\n2\n3" }], + meta: { + replayInvalid: false, + livenessState: "blocked", + timeoutPhase: "provider", + providerStarted: true, + }, + }, + }); + const runDispatch = vi.fn(async () => ({ + status: 0, + signal: null, + stdout: payload, + stderr: "", + })); + const { exit, proc, stderr, stdout } = makeProc(); + + await expect( + runAgentJsonPassthrough("alpha", ["openclaw", "agent", "--json"], proc, { + getGatewayName: () => null, + getOpenshellBinary: () => "openshell", + runDispatch, + stdinIsTty: () => false, + }), + ).rejects.toThrow("__exit:1"); + + expect(stdout.join("")).toBe(payload); + const errText = stderr.join(""); + expect(errText).toContain("timed out in the provider phase before producing a result"); + expect(errText).toContain("nemoclaw 'alpha' sessions export "); + expect(errText).toContain("models.providers..timeoutSeconds"); + // The generic incomplete-turn text is replaced, not appended. + expect(errText).not.toContain("did not complete"); + expect(exit).toHaveBeenCalledWith(1); + }); + it("exits non-zero when an incomplete response omits optional payloads", async () => { const payload = JSON.stringify({ status: "ok", diff --git a/src/lib/actions/sandbox/agent/passthrough-json.ts b/src/lib/actions/sandbox/agent/passthrough-json.ts index 03e0b308d63..5ab35d50407 100644 --- a/src/lib/actions/sandbox/agent/passthrough-json.ts +++ b/src/lib/actions/sandbox/agent/passthrough-json.ts @@ -15,6 +15,7 @@ import { import { getKnownSandboxTargetGatewayName } from "../gateway-target"; import { type AgentDispatchRunner, + agentDispatchDeadlineSeconds, isSilentAgentDispatch, runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, @@ -22,6 +23,7 @@ import { import { writeIncompleteAgentTurnFailure, writeSilentAgentDispatchFailure, + writeTimedOutAgentTurnFailure, } from "./passthrough-help"; /** Exit code for a turn the payload itself marks incomplete or abandoned. */ @@ -72,7 +74,7 @@ export async function runAgentJsonPassthrough( buildOpenshellExecArgs( sandboxName, wrapOpenClawAgentCommandWithRuntimeEnv(command), - { tty: false }, + { tty: false, timeoutSeconds: agentDispatchDeadlineSeconds(command) }, (deps.getGatewayName ?? getKnownSandboxTargetGatewayName)(sandboxName) ?? undefined, ), { @@ -111,10 +113,17 @@ export async function runAgentJsonPassthrough( // Last, so the partial trace and its provenance are already on the wire: a // turn the payload marks incomplete must not exit 0 just because the envelope - // reported success. An upstream non-zero code is preserved as-is. + // reported success. An upstream non-zero code is preserved as-is. A payload + // that declares a timeout phase gets the deadline-specific guidance instead + // of the generic incomplete-turn text; both are the same failure to the + // caller and share one exit code. const incompleteTurn = (deps.incompleteTurnSignal ?? openClawAgentIncompleteTurnSignal)(stdout); if (incompleteTurn && code === 0) { - writeIncompleteAgentTurnFailure(proc, sandboxName, incompleteTurn.markers); + if (incompleteTurn.timeoutPhase) { + writeTimedOutAgentTurnFailure(proc, sandboxName, incompleteTurn.timeoutPhase); + } else { + writeIncompleteAgentTurnFailure(proc, sandboxName, incompleteTurn.markers); + } return proc.exit(INCOMPLETE_AGENT_TURN_EXIT_CODE); } return proc.exit(code); diff --git a/src/lib/actions/sandbox/agent/passthrough.test.ts b/src/lib/actions/sandbox/agent/passthrough.test.ts index 3ac7c9b11f3..801fa800a4e 100644 --- a/src/lib/actions/sandbox/agent/passthrough.test.ts +++ b/src/lib/actions/sandbox/agent/passthrough.test.ts @@ -34,11 +34,21 @@ const loadAgentMock = vi.hoisted(() => const isTerminalAgentMock = vi.hoisted(() => vi.fn((agent: { runtime?: { kind?: string } }) => agent.runtime?.kind === "terminal"), ); +const buildOpenshellExecArgsMock = vi.hoisted(() => + vi.fn( + ( + _sb: string, + cmd: readonly string[], + _options?: { timeoutSeconds?: number }, + _gateway?: string, + ) => cmd, + ), +); vi.mock("../exec", async (importOriginal) => ({ ...(await importOriginal()), execSandbox: execMock, - buildOpenshellExecArgs: vi.fn((_sb: string, cmd: readonly string[]) => cmd), + buildOpenshellExecArgs: buildOpenshellExecArgsMock, wrapExecCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), wrapOpenClawAgentCommandWithRuntimeEnv: vi.fn((cmd: readonly string[]) => cmd), })); @@ -835,6 +845,36 @@ describe("runAgentNonJsonPassthrough", () => { const stubBinary = () => "/usr/local/bin/openshell"; + it("bounds the host transport when the turn requests a deadline (#8723)", async () => { + const { proc } = makeNonJsonProcMock(); + const runDispatchMock = makeDispatchMock("PONG\n", "", 0); + await expect( + runAgentNonJsonPassthrough( + "my-sb", + ["openclaw", "agent", "--agent", "main", "--timeout", "30", "-m", "ping"], + proc, + { getOpenshellBinary: stubBinary, runDispatch: runDispatchMock }, + ), + ).rejects.toThrow("__exit:0"); + // Outlasts the requested deadline so the in-sandbox turn still reports its + // own timeout; the host bound only catches a turn that stops answering. + expect(buildOpenshellExecArgsMock.mock.calls[0]?.[2]?.timeoutSeconds).toBe(60); + // The turn still receives the deadline it asked for. + expect(buildOpenshellExecArgsMock.mock.calls[0]?.[1]).toContain("30"); + }); + + it("leaves the host transport unbounded when the turn requests no deadline (#8723)", async () => { + const { proc } = makeNonJsonProcMock(); + const runDispatchMock = makeDispatchMock("PONG\n", "", 0); + await expect( + runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "--agent", "main", "-m", "ping"], proc, { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }), + ).rejects.toThrow("__exit:0"); + expect(buildOpenshellExecArgsMock.mock.calls[0]?.[2]?.timeoutSeconds).toBeUndefined(); + }); + it("emits a clean embedded-fallback error and exits 1 when EMBEDDED FALLBACK appears in stdout", async () => { const { stderrWrites, stdoutWrites, exit, proc } = makeNonJsonProcMock(); const runDispatchMock = makeDispatchMock("EMBEDDED FALLBACK: using local model\nPONG\n", "", 0); @@ -889,6 +929,61 @@ describe("runAgentNonJsonPassthrough", () => { expect(stderrWrites.join("")).toBe(""); }); + it("fails loud instead of reporting success when the turn's deadline fired (#8723)", async () => { + const { stdoutWrites, stderrWrites, exit, proc } = makeNonJsonProcMock(); + const timedOut = + "LLM request failed.\nRequest timed out before a response was generated. Please try again, or increase `agents.defaults.timeoutSeconds` in your config.\n"; + const runDispatchMock = makeDispatchMock(timedOut, "", 0); + await expect( + runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "-m", "ping"], proc, { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }), + ).rejects.toThrow("__exit:1"); + expect(exit).toHaveBeenCalledWith(1); + // The partial trace still reaches the caller ahead of the verdict. + expect(stdoutWrites.join("")).toBe(timedOut); + const errText = stderrWrites.join(""); + expect(errText).toMatch(/timed out before producing a result/); + expect(errText).toContain("nemoclaw 'my-sb' sessions export "); + expect(errText).toContain("models.providers..timeoutSeconds"); + expect(errText).toMatch(/may have already applied side effects/); + }); + + it("keeps a completed reply that quotes the timeout sentence successful (#8723)", async () => { + const { stdoutWrites, stderrWrites, exit, proc } = makeNonJsonProcMock(); + const reply = + 'The message "Request timed out before a response was generated" means the deadline fired.\n'; + const runDispatchMock = makeDispatchMock(reply, "", 0); + + await expect( + runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "-m", "explain"], proc, { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }), + ).rejects.toThrow("__exit:0"); + + expect(exit).toHaveBeenCalledWith(0); + expect(stdoutWrites.join("")).toBe(reply); + expect(stderrWrites.join("")).toBe(""); + }); + + it("keeps an upstream non-zero code for a turn that also reported a timeout (#8723)", async () => { + const { exit, proc } = makeNonJsonProcMock(); + const runDispatchMock = makeDispatchMock( + "Request timed out before a response was generated.\n", + "", + 3, + ); + await expect( + runAgentNonJsonPassthrough("my-sb", ["openclaw", "agent", "-m", "ping"], proc, { + getOpenshellBinary: stubBinary, + runDispatch: runDispatchMock, + }), + ).rejects.toThrow("__exit:3"); + expect(exit).toHaveBeenCalledWith(3); + }); + it("passes through non-zero exit code on clean failure without embedded-fallback", async () => { const { stderrWrites, exit, proc } = makeNonJsonProcMock(); const runDispatchMock = makeDispatchMock("", "Error: agent session not found\n", 1); diff --git a/src/lib/actions/sandbox/agent/passthrough.ts b/src/lib/actions/sandbox/agent/passthrough.ts index 0cfa6458824..5061d3ea2c1 100644 --- a/src/lib/actions/sandbox/agent/passthrough.ts +++ b/src/lib/actions/sandbox/agent/passthrough.ts @@ -132,14 +132,20 @@ import { ensureLiveSandboxOrExit } from "../gateway-state"; import { getKnownSandboxTargetGatewayName } from "../gateway-target"; import { type AgentDispatchRunner, + agentDispatchDeadlineSeconds, isSilentAgentDispatch, + isTimedOutAgentDispatch, + OPENCLAW_AGENT_BOOLEAN_FLAGS, + OPENCLAW_AGENT_VALUE_FLAGS, runAgentDispatch, SILENT_AGENT_DISPATCH_EXIT_CODE, + TIMED_OUT_AGENT_TURN_EXIT_CODE, } from "./passthrough-dispatch"; import { hasAgentPassthroughHelpToken, printAgentPassthroughHelp, writeSilentAgentDispatchFailure, + writeTimedOutAgentTurnFailure, } from "./passthrough-help"; import { type AgentJsonPassthroughProcess, @@ -151,23 +157,6 @@ import { maybeEmitShieldsRelockWarning } from "./passthrough-shields-warning"; export { hasAgentPassthroughHelpToken, printAgentPassthroughHelp } from "./passthrough-help"; -const OPENCLAW_AGENT_VALUE_FLAGS = new Set([ - "-a", - "--agent", - "-m", - "--message", - "--model", - "--provider", - "--reply-channel", - "--session-id", - "--session-key", - "--thinking", - "--timeout", - "--to", -]); - -const OPENCLAW_AGENT_BOOLEAN_FLAGS = new Set(["--deliver"]); - // OpenClaw can exit zero after running in embedded-fallback mode and does not // expose a stable machine-readable transport discriminator. These patterns mirror // the gateway-auth live tests in restore-gateway-pairing.ts and extend them with @@ -196,7 +185,7 @@ export async function runAgentNonJsonPassthrough( buildOpenshellExecArgs( sandboxName, wrapOpenClawAgentCommandWithRuntimeEnv(command), - { tty: false }, + { tty: false, timeoutSeconds: agentDispatchDeadlineSeconds(command) }, (deps.getGatewayName ?? getKnownSandboxTargetGatewayName)(sandboxName) ?? undefined, ), { @@ -234,6 +223,14 @@ export async function runAgentNonJsonPassthrough( proc.stderr.write(` Failed to invoke openshell: ${errorMessage}\n`); proc.stderr.write(" Ensure 'openshell' is installed and on PATH.\n"); } + + // Last, so the partial trace is already on the wire: a turn whose deadline + // fired must not exit 0 just because the transport did. An upstream non-zero + // code is preserved as-is. + if (code === 0 && isTimedOutAgentDispatch(stdout, stderr)) { + writeTimedOutAgentTurnFailure(proc, sandboxName); + return proc.exit(TIMED_OUT_AGENT_TURN_EXIT_CODE); + } return proc.exit(code); } diff --git a/src/lib/openclaw/agent-json-provenance.test.ts b/src/lib/openclaw/agent-json-provenance.test.ts index 7e24ceb23dd..4ef1d231e5a 100644 --- a/src/lib/openclaw/agent-json-provenance.test.ts +++ b/src/lib/openclaw/agent-json-provenance.test.ts @@ -324,4 +324,73 @@ describe("openClawAgentIncompleteTurnSignal", () => { it("returns null when stdout carries no JSON at all", () => { expect(openClawAgentIncompleteTurnSignal("not json")).toBeNull(); }); + + it("detects a declared timeout phase on the run metadata (#8723)", () => { + const raw = JSON.stringify({ + status: "timeout", + result: { + payloads: [{ text: "1\n2\n3" }], + meta: { replayInvalid: false, livenessState: "blocked", timeoutPhase: "provider" }, + }, + }); + expect(openClawAgentIncompleteTurnSignal(raw)).toEqual({ + markers: ["timeoutPhase=provider"], + timeoutPhase: "provider", + }); + }); + + it("classifies a timeout phase the measurements never observed (#8723)", () => { + const raw = JSON.stringify({ + status: "timeout", + result: { payloads: [], meta: { timeoutPhase: "gateway_draining" } }, + }); + expect(openClawAgentIncompleteTurnSignal(raw)?.timeoutPhase).toBe("gateway_draining"); + }); + + it("ignores a timeout phase inside a successful tool result (#8723)", () => { + const raw = JSON.stringify({ + status: "ok", + result: { + messages: [{ role: "toolResult", content: { timeoutPhase: "provider" } }], + payloads: [{ text: "done" }], + }, + }); + expect(openClawAgentIncompleteTurnSignal(raw)).toBeNull(); + }); + + it("ignores a timeout phase that carries no value (#8723)", () => { + for (const timeoutPhase of ["", " ", null, 3, true]) { + const raw = JSON.stringify({ status: "ok", result: { payloads: [], meta: { timeoutPhase } } }); + expect(openClawAgentIncompleteTurnSignal(raw)).toBeNull(); + } + }); + + it("leaves the timeout phase absent for an abandoned turn that did not time out (#8723)", () => { + const raw = JSON.stringify({ + status: "ok", + result: { payloads: [], meta: { livenessState: "abandoned" } }, + }); + expect(openClawAgentIncompleteTurnSignal(raw)?.timeoutPhase).toBeUndefined(); + }); + + it("reports the timeout phase alongside every other marker present (#8723)", () => { + const raw = JSON.stringify({ + status: "timeout", + result: { + payloads: [], + meta: { + error: { kind: "incomplete_turn" }, + livenessState: "abandoned", + replayInvalid: true, + timeoutPhase: "post_turn", + }, + }, + }); + expect(openClawAgentIncompleteTurnSignal(raw)?.markers.sort()).toEqual([ + "error.kind=incomplete_turn", + "livenessState=abandoned", + "replayInvalid=true", + "timeoutPhase=post_turn", + ]); + }); }); diff --git a/src/lib/openclaw/agent-json-provenance.ts b/src/lib/openclaw/agent-json-provenance.ts index 24b323e41f3..662932d4a2b 100644 --- a/src/lib/openclaw/agent-json-provenance.ts +++ b/src/lib/openclaw/agent-json-provenance.ts @@ -306,7 +306,18 @@ export function openClawAgentJsonProvenanceLines(raw: string): string[] { // The markers themselves, all declared on EmbeddedAgentRunMeta: // replayInvalid?: boolean // livenessState?: "working" | "paused" | "blocked" | "abandoned" +// timeoutPhase?: "queue" | "preflight" | "provider" | "post_turn" | "gateway_draining" // error?: { kind: ... | "incomplete_turn" | ... } +// +// `timeoutPhase` marks a run whose deadline fired (#8723). It is optional and +// absent from a turn that answered, so its presence is the marker and any phase +// value counts; the declared phases are recorded above as documentation, not as +// an allowlist, so a phase added upstream is still classified as a timeout +// instead of being reported as a success. +// +// `livenessState` is deliberately not a timeout marker. Two identical timed-out +// runs reported `blocked` and `working`, so only `abandoned` stays tied to the +// abandonment case it was added for. const ABANDONED_LIVENESS_VALUE = "abandoned"; // Compared after `normalized()`, which lowercases and maps `_` to `-`. const INCOMPLETE_TURN_ERROR_KIND = "incomplete-turn"; @@ -314,6 +325,8 @@ const INCOMPLETE_TURN_ERROR_KIND = "incomplete-turn"; export type OpenClawIncompleteTurnSignal = { /** Human-readable `field=value` markers, deduped. */ markers: string[]; + /** The declared phase the deadline fired in, absent when the run did not time out. */ + timeoutPhase?: string; }; /** The declared run-metadata record from an agent response envelope. */ @@ -349,12 +362,22 @@ function finalAgentResponseMetaRecord(docs: unknown[]): UnknownRecord | null { return null; } +/** The phase the run's deadline fired in, or null when the run did not time out. */ +function timedOutPhase(meta: UnknownRecord): string | null { + const phase = meta.timeoutPhase; + if (typeof phase !== "string") return null; + const trimmed = phase.trim(); + return trimmed.length > 0 ? trimmed : null; +} + function turnMetaMarkers(meta: UnknownRecord): string[] { const markers: string[] = []; if (meta.replayInvalid === true) markers.push("replayInvalid=true"); if (normalized(meta.livenessState) === ABANDONED_LIVENESS_VALUE) { markers.push(`livenessState=${String(meta.livenessState)}`); } + const timeoutPhase = timedOutPhase(meta); + if (timeoutPhase) markers.push(`timeoutPhase=${timeoutPhase}`); const error = meta.error; if (isObjectRecord(error) && normalized(error.kind) === INCOMPLETE_TURN_ERROR_KIND) { markers.push(`error.kind=${String(error.kind)}`); @@ -363,8 +386,10 @@ function turnMetaMarkers(meta: UnknownRecord): string[] { } /** - * Detect a turn the run metadata itself marks incomplete or abandoned. Returns - * null when no marker is present, so a healthy turn is never reclassified. + * Detect a turn the run metadata itself marks incomplete, abandoned, or timed + * out. Returns null when no marker is present, so a healthy turn is never + * reclassified. A timed-out run also carries its declared phase, which the + * caller uses to pick deadline-specific recovery guidance. */ export function openClawAgentIncompleteTurnSignal( raw: string, @@ -374,5 +399,7 @@ export function openClawAgentIncompleteTurnSignal( const meta = finalAgentResponseMetaRecord(docs); if (!meta) return null; const markers = dedupe(turnMetaMarkers(meta)); - return markers.length > 0 ? { markers } : null; + if (markers.length === 0) return null; + const timeoutPhase = timedOutPhase(meta); + return timeoutPhase ? { markers, timeoutPhase } : { markers }; }