Skip to content

Commit 124c65c

Browse files
fix(agent): reject ambiguous timeout argv (#9075)
## Summary NemoClaw now infers the host-side OpenShell deadline only when it can parse the complete forwarded OpenClaw option sequence without ambiguity. Unknown options and positional tokens leave the host wait unbounded instead of treating a later `--timeout` token as an OpenClaw deadline. ## Related Issue Follow-up to #9056. Addresses the delayed [PR Review Advisor finding](#9056 (comment)). ## Changes - Recognize the documented separated, equals-form, boolean, and JSON option forms before `--timeout`. - Leave the host wait unbounded for unknown flags, positional tokens, consumed values, malformed timeout values, and tokens after `--`. - Document the unknown-option condition in the command reference. - Add regression coverage for the ambiguous `--unknown --timeout 30` sequence. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: an exact nine-category security review passed at `4aaaf7c3` with no actionable findings. Unknown or ambiguous argv does not authorize an inferred deadline, and forwarded arguments remain an argv array without shell interpretation. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/reference/commands.mdx` now lists an unrecognized option before `--timeout` among the conditions that leave the OpenShell wait unbounded. The implementation and regression test enforce the same condition. - Agent: Codex Desktop <!-- docs-review-head-sha: 4aaaf7c --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — four focused agent-dispatch suites, 129 tests passed - [x] Applicable broad gate passed — `npm run validate:pr` passed, including repository checks, secret scanning, commit checks, and CLI type checking - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — build passed with 0 errors; Fern reported two existing repository warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved timeout option parsing for the `agent` command, including boolean and equals-form options. * Prevented unrecognized options from incorrectly creating a dispatch deadline. * Clarified that an unrecognized option before `--timeout` leaves the wait unbounded. * **Tests** * Added coverage for supported option formats and unknown-option handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
1 parent 5bb69ed commit 124c65c

3 files changed

Lines changed: 33 additions & 1 deletion

File tree

docs/reference/commands.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,7 @@ These leave the OpenShell wait unbounded:
13101310
- `--timeout 0`.
13111311
- A value NemoClaw cannot read as a positive whole number of seconds.
13121312
- An argv without `--timeout`.
1313+
- An unrecognized option before `--timeout`, because NemoClaw does not infer a host deadline outside the documented OpenClaw option grammar.
13131314
- A `--timeout` after the `--` argv terminator, which OpenClaw reads as payload rather than as its own flag.
13141315

13151316
When the captured output reports that the turn's deadline fired, the wrapper replays the partial output and writes deadline guidance to `stderr`.

src/lib/actions/sandbox/agent/passthrough-dispatch.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,14 @@ describe("requestedAgentTimeoutSeconds", () => {
176176
expect(requestedAgentTimeoutSeconds(agent("--timeout=45", "-m", "hi"))).toBe(45);
177177
});
178178

179+
it("reads a timeout after documented boolean and equals-form options (#8723)", () => {
180+
expect(
181+
requestedAgentTimeoutSeconds(
182+
agent("--deliver", "--agent=main", "--json=false", "--timeout", "30"),
183+
),
184+
).toBe(30);
185+
});
186+
179187
it("requests no deadline when the argv carries no --timeout (#8723)", () => {
180188
expect(requestedAgentTimeoutSeconds(agent("--agent", "main", "-m", "hi"))).toBeNull();
181189
});
@@ -192,6 +200,12 @@ describe("requestedAgentTimeoutSeconds", () => {
192200
expect(requestedAgentTimeoutSeconds(agent("--", "--timeout", "30"))).toBeNull();
193201
});
194202

203+
it("keeps the host unbounded after an unknown option (#8723)", () => {
204+
const argv = agent("--unknown", "--timeout", "30");
205+
expect(requestedAgentTimeoutSeconds(argv)).toBeNull();
206+
expect(agentDispatchDeadlineSeconds(argv)).toBeUndefined();
207+
});
208+
195209
it("refuses a value that cannot be a deadline (#8723)", () => {
196210
for (const raw of ["-5", "1.5", "abc", "", "1e3"]) {
197211
expect(requestedAgentTimeoutSeconds(agent("--timeout", raw))).toBeNull();

src/lib/actions/sandbox/agent/passthrough-dispatch.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,8 @@ export const AGENT_DISPATCH_DEADLINE_BUFFER_SECONDS = 30;
331331
* for the same reason.
332332
*/
333333
export function requestedAgentTimeoutSeconds(argv: readonly string[]): number | null {
334-
for (let index = 0; index < argv.length; index += 1) {
334+
if (argv[0] !== "openclaw" || argv[1] !== "agent") return null;
335+
for (let index = 2; index < argv.length; index += 1) {
335336
const arg = argv[index] as string;
336337
if (arg === "--") return null;
337338
if (arg === "--timeout") return parseDeadlineSeconds(argv[index + 1]);
@@ -340,6 +341,22 @@ export function requestedAgentTimeoutSeconds(argv: readonly string[]): number |
340341
index += 1;
341342
continue;
342343
}
344+
const equalsIndex = arg.indexOf("=");
345+
if (
346+
equalsIndex > 0 &&
347+
arg.startsWith("--") &&
348+
OPENCLAW_AGENT_VALUE_FLAGS.has(arg.slice(0, equalsIndex))
349+
) {
350+
continue;
351+
}
352+
if (
353+
arg === "--json" ||
354+
arg.startsWith("--json=") ||
355+
OPENCLAW_AGENT_BOOLEAN_FLAGS.has(arg)
356+
) {
357+
continue;
358+
}
359+
return null;
343360
}
344361
return null;
345362
}

0 commit comments

Comments
 (0)