fix(e2e): recover common egress OpenClaw retries

[sandl99]

Summary

Make the common-egress OpenClaw agent assertion retry transient network and inference failures instead of treating them as policy blocks. The assertion now detects first-run OpenClaw scope-upgrade fallback, runs nemoclaw <sandbox> recover, and retries the agent turn so the test does not fail on the known post-onboard pairing handoff race.

Related Issue

Fixes #5191

Changes

• Add a reusable NemoClaw CLI wrapper in test/e2e/test-common-egress-agent-e2e.sh so cleanup and recovery use the source CLI when available.
• Split OpenClaw assertion errors into hard SSRF policy blocks versus transient network/inference failures.
• Capture stderr from openclaw agent, recover pending scope-upgrade approvals, and retry with progressive backoff for DNS, fetch, gateway, and NVIDIA inference timeout/503 signals.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Verification

• npx prek run --all-files passes
• npm test passes
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

Additional validation run locally:

• bash -n test/e2e/test-common-egress-agent-e2e.sh
• shellcheck test/e2e/test-common-egress-agent-e2e.sh
• git diff --check
• Git commit/pre-push hooks passed for the changed shell script.
• Partial live run: env NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 test/e2e/test-common-egress-agent-e2e.sh reached C3 after C1 and C2 passed, then was interrupted before final summary.

---

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

• Tests
  • Improved testing infrastructure for agent assertions with enhanced error capture and logging capabilities.
  • Strengthened retry logic with better error detection, recovery mechanisms, and richer context reporting.
  • Enhanced robustness of test execution with improved fallback handling and timeout management.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5fae351e-96c3-4009-b031-c1c6a4b4ffc2

📥 Commits

Reviewing files that changed from the base of the PR and between d2b8d6b and 9ccbb24.

📒 Files selected for processing (1)

• test/e2e/test-common-egress-agent-e2e.sh

---

📝 Walkthrough

Walkthrough

The pull request refactors the OpenClaw e2e test assertions to handle transient network errors more robustly. A centralized CLI helper reduces duplication, and the assertion loop now captures and combines stderr with stdout to distinguish permanent SSRF policy blocks from transient DNS and network failures that should retry.

Changes

OpenClaw agent assertion resilience

Layer / File(s)	Summary
CLI invocation helper test/e2e/test-common-egress-agent-e2e.sh	Added run_nemoclaw_cli_with_timeout() helper that selects between repo source node bin/nemoclaw.js and system nemoclaw binary, returning exit code 127 if neither is available.
Sandbox destruction refactoring test/e2e/test-common-egress-agent-e2e.sh	destroy_sandbox_best_effort() delegates destroy --yes invocation to the new CLI helper instead of inline binary selection.
Stderr capture and transient error retry enhancement test/e2e/test-common-egress-agent-e2e.sh	Agent assertion loop now captures stderr and constructs combined stdout+stderr output. Policy/transport block detection (SSRF, hostname blocks) scans combined output for permanent failures. Retry logic detects scope upgrade/pairing prompts as recovery triggers and broadens transient error patterns (EAI_AGAIN, ECONNREFUSED, DNS, connection, timeout, fetch failures) to allow retries with progressive backoff instead of immediate failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

integration: openclaw, area: sandbox, bug-fix

Suggested reviewers

• cv
• prekshivyas

Poem

A script that once gave up too fast,
Now retries through the network's blast,
With stderr caught and combined tight,
DNS hiccups won't dim the light,
Policy blocks still fail—but not the night! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: fixing OpenClaw retry behavior for egress e2e tests.
Linked Issues check	✅ Passed	The PR fully implements the objectives from issue #5191: splits policy/transport errors into permanent SSRF blocks and transient failures, adds retry logic with backoff, handles scope-upgrade recovery, and captures stderr for improved diagnostics.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue requirements: the helper function, stderr capturing, error classification logic, recovery handling, and retry conditions all address the stated objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/common-egress-openclaw-recover-retry

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

E2E Advisor Recommendation

Required E2E: None
Optional E2E: common-egress-agent-e2e

Dispatch hint: common-egress-agent-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• None.

Optional E2E

• common-egress-agent-e2e (high): Directly exercises the modified E2E script and verifies the updated OpenClaw recover/retry and transient inference error handling in the common-egress agent flow.

New E2E recommendations

• None.

Dispatch hint

• Workflow: .github/workflows/nightly-e2e.yaml
• jobs input: common-egress-agent-e2e

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• None. Changed file is outside the Vitest scenario system and does not affect test/e2e-scenario fixtures, live support, scenario registry, runtime support, or .github/workflows/e2e-vitest-scenarios.yaml.

Optional Vitest E2E scenarios

• None.

Relevant changed files

• None.

[github-actions]

PR Review Advisor

Findings: 0 needs attention, 4 worth checking, 0 nice ideas
Top item: Document and test the scope-upgrade recover workaround

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: test/e2e/test-common-egress-agent-e2e.sh OpenClaw scope-upgrade recover retry: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: Lines 314-323 detect pending scope-upgrade/pairing text, run `nemoclaw <sandbox> recover`, ignore nonzero recover aside from an info line, sleep, and retry.
• Scope-upgrade recover retry is a localized workaround without a deterministic guardrail (test/e2e/test-common-egress-agent-e2e.sh:314): The new branch treats OpenClaw scope-upgrade/pairing messages as a recoverable post-onboard race, runs `nemoclaw <sandbox> recover`, ignores a nonzero recover exit, and retries. That may be the right live-E2E stabilization, but the touched test does not explain why the invalid pending-scope state cannot be prevented at onboarding, what deterministic regression covers this branch, or when this workaround can be removed. Without that source-of-truth boundary, future product regressions in auto-pair/recover could be hidden as test flake handling.
  • Recommendation: Add a brief inline rationale tied to the product boundary/removal condition, and add or reference deterministic coverage for the helper behavior: pending scope-upgrade triggers exactly one recover before retry, policy blocks do not recover, and recover failures remain diagnosable.
  • Evidence: Lines 314-323 detect `scope upgrade pending approval|pairing required: device is asking for more scopes`, run `run_nemoclaw_cli_with_timeout 120 "$sandbox" recover`, log nonzero `recover_rc`, then continue retrying.
• Recover stdout/stderr is appended to the E2E artifact without redaction (test/e2e/test-common-egress-agent-e2e.sh:320): The new recovery path appends all `nemoclaw <sandbox> recover` output to `/tmp/nemoclaw-e2e-common-egress-${sandbox}-agent.log`. Current `sandbox recover` mostly prints probe status and its auto-pair approval pass uses ignored stdio, so I did not find a confirmed secret leak. However, this is a security-sensitive recovery path around gateway/pairing state, and CI E2E logs often become artifacts; future recovery diagnostics could include token-shaped values, endpoint URLs, or other sensitive runtime details.
  • Recommendation: Keep recover diagnostics useful but constrain them: redirect only known-safe status lines, redact token-shaped values before appending, or add a comment/test that `sandbox recover` output is intentionally secret-free on this path.
  • Evidence: Line 320: `run_nemoclaw_cli_with_timeout 120 "$sandbox" recover >>"$log_file" 2>&1 || recover_rc=$?`.
• Linked issue's Hermes timeout observation is left unchanged (test/e2e/test-common-egress-agent-e2e.sh:357): The linked issue also records a C3 Hermes failure where all three attempts timed out because `restcountries.com` was unreachable. This PR intentionally focuses on the OpenClaw guard and leaves `run_hermes_agent_assertion` retry behavior unchanged. That is consistent with the issue title and proposed OpenClaw design, but it means the PR only partially addresses the full problem statement if maintainers expect `Fixes nightly-e2e: common-egress-agent-e2e fails due to transient DNS errors in agent assertion guard #5191` to cover the Hermes clause too.
  • Recommendation: Either clarify that nightly-e2e: common-egress-agent-e2e fails due to transient DNS errors in agent assertion guard #5191 is fixed only for the OpenClaw early-exit guard, or extend/justify the Hermes behavior for the C3 external-timeout clause.
  • Evidence: The diff changes only `run_openclaw_agent_assertion`; `run_hermes_agent_assertion` still performs three attempts with the existing fixed 5-second sleep and no new classification/backoff.

🌱 Nice ideas

• None.

Consider writing more tests for

• **Runtime validation** — Simulate an OpenClaw agent attempt containing `SsrFBlockedError` or `Blocked hostname` and assert the helper fails immediately without retrying or running recover.. The change is in a live E2E shell harness, so unit tests are not strictly required for runtime product behavior. However, the new branching controls whether a security-sensitive egress assertion hard-fails, recovers, or retries; deterministic helper coverage or tightly scoped runtime validation would reduce the chance of masking policy or recover regressions.
• **Runtime validation** — Simulate `DNS error` or `EAI_AGAIN` on attempt 1 followed by the expected token on attempt 2 and assert the helper retries instead of classifying the transient as a policy block.. The change is in a live E2E shell harness, so unit tests are not strictly required for runtime product behavior. However, the new branching controls whether a security-sensitive egress assertion hard-fails, recovers, or retries; deterministic helper coverage or tightly scoped runtime validation would reduce the chance of masking policy or recover regressions.
• **Runtime validation** — Simulate `scope upgrade pending approval` on attempt 1 and assert `nemoclaw <sandbox> recover` is invoked exactly once before the next agent turn.. The change is in a live E2E shell harness, so unit tests are not strictly required for runtime product behavior. However, the new branching controls whether a security-sensitive egress assertion hard-fails, recovers, or retries; deterministic helper coverage or tightly scoped runtime validation would reduce the chance of masking policy or recover regressions.
• **Runtime validation** — Simulate a nonzero `recover` exit followed by final assertion failure and assert the diagnostic preserves recover failure context for triage.. The change is in a live E2E shell harness, so unit tests are not strictly required for runtime product behavior. However, the new branching controls whether a security-sensitive egress assertion hard-fails, recovers, or retries; deterministic helper coverage or tightly scoped runtime validation would reduce the chance of masking policy or recover regressions.
• **Runtime validation** — Exercise the unchanged Hermes timeout path or document why the linked C3 timeout clause is intentionally out of scope.. The change is in a live E2E shell harness, so unit tests are not strictly required for runtime product behavior. However, the new branching controls whether a security-sensitive egress assertion hard-fails, recovers, or retries; deterministic helper coverage or tightly scoped runtime validation would reduce the chance of masking policy or recover regressions.
• **Scope-upgrade recover retry is a localized workaround without a deterministic guardrail** — Add a brief inline rationale tied to the product boundary/removal condition, and add or reference deterministic coverage for the helper behavior: pending scope-upgrade triggers exactly one recover before retry, policy blocks do not recover, and recover failures remain diagnosable.
• **Acceptance clause:** **C3 (Hermes open public reference)**: All 3 retry attempts timed out (curl exit 28, 120s each) because the external API was completely unreachable. — add test evidence or identify existing coverage. The PR leaves `run_hermes_agent_assertion` unchanged; it still has the pre-existing three attempts and fixed 5-second sleeps. The OpenClaw-specific proposed design is addressed, but this Hermes observation is not changed.
• **test/e2e/test-common-egress-agent-e2e.sh OpenClaw scope-upgrade recover retry** — No deterministic test in this PR proves the recovery branch: pending-scope output triggers recover before retry, SSRF policy blocks bypass recover, and recover failure remains visible.. Lines 314-323 detect pending scope-upgrade/pairing text, run `nemoclaw <sandbox> recover`, ignore nonzero recover aside from an info line, sleep, and retry.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

[github-actions]

Selective E2E Results — ✅ All requested jobs passed

Run: 27325921464
Target ref: fix/common-egress-openclaw-recover-retry
Requested jobs: common-egress-agent-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job	Result
common-egress-agent-e2e	✅ success