fix(sandbox): recover a gateway wedged in any not-serving state

[TonyLuo-NV]

Summary

The sandbox serving watchdog previously recovered an alive but unusable OpenClaw gateway only after an uninterrupted sequence of connection-refused health probes. A listener can also become unusable when a socket accepts and stalls, accepts and drops the connection, or answers /health with an error status. The watchdog treated each of those results as evidence that a listener existed. It rearmed without a log entry or recovery, so the gateway remained unable to serve. The watchdog now uses the same response requirement as the boot-time readiness check: /health must answer 200 or 401. It recovers each not-serving result. It still does not act before a gateway has served or when the probe cannot run.

Related Issue

Fixes #7377

Changes

• scripts/nemoclaw-start.sh: added gateway_watchdog_probe_gateway, which classifies a health probe as serving (/health answered 200 or 401), not serving (refused, timed out, reset, empty reply, or an HTTP error status), or inconclusive (the probe itself could not run). The watchdog now counts every not-serving outcome toward one threshold instead of requiring consecutive curl exit 7 results.
• scripts/nemoclaw-start.sh: inconclusive probes hold the watchdog's armed state and streak rather than rearming it, so a missing or failing curl can never escalate into a kill loop against a healthy gateway. The watchdog also exits early with a message when curl is absent.
• scripts/nemoclaw-start.sh: each failed probe now logs the cause it observed, such as connection refused, probe timeout, connection reset, or HTTP 503. An operator can distinguish a refused port from a listener that accepts connections but cannot serve.
• docs/reference/troubleshooting.mdx: documented the serving definition, the wedge symptoms users see (gateway_transport_error, 1006 abnormal closure, container running with no restarts, replies still arriving over the embedded fallback route), and what a health probe inconclusive line means.
• test/gateway-serving-watchdog.test.ts (new) and test/nemoclaw-start-gateway.test-helpers.ts (new): the watchdog suite moved to its own file with the shared shell-harness helpers extracted, keeping both files inside ci/test-file-size-budget.json.
• test/gateway-serving-watchdog.test.ts: the PID-swap regression now emits HTTP 200 on the predecessor's successful probe, so it proves that the replacement gateway does not inherit the predecessor's armed state.

The arming rule (never act until the gateway has served at least once), the PID-identity and cmdline checks before signalling, the respawn-marker handshake, and the NEMOCLAW_GATEWAY_WATCHDOG_* environment variable names are all unchanged.

Why the reported sandboxes never recovered

The old counter required four consecutive connection-refused probes. A wedge that alternated between refused and any other failure mode reset that counter on every other probe, which is why the reporter's manual curl showed (7) while the watchdog had still never escalated. That path is now covered by a regression test.

The non-refused outcomes were previously delegated to the Docker HEALTHCHECK. Nothing restarts an unhealthy OpenShell sandbox container, which matches the reported "container restart count: 0" with the gateway wedged indefinitely.

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)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex Desktop exact-head security review PASS for head e0c4d7ef9efdadfd19ec494e19edfcad8947d76a against base e9fb170a2e3e4c4dcfa19e398232f71b8f2bace6; all nine categories pass with no findings. The serving prerequisite, strict 200/401 classifier, inconclusive fail-safe, PID start-identity check, gateway command-line check, and signal-time identity revalidation remain intact.
• Non-success, skipped, or missing CI check accepted by maintainer — no check is accepted, skipped, or waived. The current sandbox-image and reviewed npm-audit failures are hard blockers from dependency advisories on the current base and are being remediated by fix(security): close managed runtime audit gaps #8156; this PR must rerun and pass those gates before merge.

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: docs-updated
• Evidence: docs/reference/troubleshooting.mdx; verified the watchdog behavior, user-visible impact, recovery instructions, and lifecycle limits against the exact implementation and focused tests. Documentation build passed with 0 errors and 2 baseline Fern warnings.
• Agent: Codex Desktop

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• 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 — npm run validate:pr passed at exact head e0c4d7ef9efdadfd19ec494e19edfcad8947d76a; the final maintainer commit also passed normal pre-commit and commit-msg hooks.
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project integration test/gateway-serving-watchdog.test.ts → 18 passed. The combined watchdog and gateway-health run passed 41 of 42 tests; the sole failure is the neighboring lock-cleanup fixture invoking GNU timeout, which is absent on this macOS host. CI remains authoritative for the full Linux run.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not claimed locally. Exact-head npm run validate:pr, CLI build, plugin build, focused tests, git diff --check, and npm run docs passed; required GitHub CI and E2E must pass before merge.
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only) — completed with 0 errors and 2 pre-existing non-blocking Fern warnings (local Fern authentication and light-mode accent contrast).
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only) — no new doc pages.

How the fix was verified against the reported failure

Each wedge signature was driven through the real watchdog loop with a scripted probe sequence and a real background process standing in for the gateway. Before the change, only the pure-refused sequence produced any output at all; timeout, reset, empty-reply, HTTP-error, and alternating sequences produced complete silence and no recovery. After the change, each not-serving sequence reaches the recovery threshold. A gateway that has not served, a 401 response, a recovering gateway, and an unrunnable probe leave the gateway process running.

---

Signed-off-by: Tony Luo xialuo@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Improved gateway health monitoring to distinguish serving, failed, and inconclusive checks.
  • Recognizes successful and authentication responses, connection failures, timeouts, resets, empty replies, and HTTP errors.
  • Restarts unresponsive gateways after the configured consecutive-failure threshold while avoiding action on inconclusive checks.
  • Validates the gateway process before triggering a restart and applies safe defaults for invalid settings.

• Documentation

  • Expanded troubleshooting guidance for health checks, transport errors, fallback behavior, and recovery limitations.

• Tests

  • Added comprehensive coverage for monitoring, recovery thresholds, status handling, process validation, and restart races.

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The gateway watchdog now classifies health probes, tracks consecutive not-serving results, and triggers supervised gateway recovery. Tests cover probe outcomes, thresholds, PID identity, process replacement, and restart races. Troubleshooting guidance documents the updated behavior.

Changes

Gateway watchdog recovery

Layer / File(s)	Summary
Watchdog probe and recovery behavior scripts/nemoclaw-start.sh	The watchdog accepts HTTP 200 and 401 as serving, records classified failures, ignores inconclusive probes, and terminates an identity-validated gateway after the configured streak.
Shared shell-test support test/nemoclaw-start-gateway.test-helpers.ts, test/nemoclaw-start-gateway-health.test.ts	Shared helpers now extract shell functions and create process fixtures. Gateway health tests use the shared helpers and focus on healthcheck markers and supervised lifecycle behavior.
Watchdog behavior validation test/gateway-serving-watchdog.test.ts	End-to-end tests cover probe outcomes, recovery thresholds, streak resets, PID replacement, environment overrides, unavailable probes, and restart signal races.
Troubleshooting guidance docs/reference/troubleshooting.mdx	The documentation describes serving responses, failed and inconclusive probes, fallback behavior, recovery timing, and supervisor relaunch.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Watchdog
  participant curl
  participant OpenClawGateway
  participant Supervisor
  Watchdog->>curl: Probe local health endpoint
  curl->>OpenClawGateway: Send health request
  OpenClawGateway-->>curl: Return HTTP status or transport result
  curl-->>Watchdog: Return classified probe result
  Watchdog->>Watchdog: Count consecutive not-serving probes
  Watchdog->>OpenClawGateway: Terminate identity-validated gateway
  Supervisor->>OpenClawGateway: Relaunch gateway

Loading

Suggested labels: area: sandbox, bug-fix

Suggested reviewers: cv, sandl99

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The watchdog now restarts alive gateways after consecutive not-serving probes, addressing issue #7377 and preventing silent fallback.
Out of Scope Changes check	✅ Passed	The script, documentation, tests, and shared helpers directly support the watchdog recovery objective in issue #7377.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: recovering sandbox gateways that remain alive but are not serving.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 1 blocker · 2 warnings · 1 suggestion
• Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported 1 more blocker, 2 more warnings, 1 more suggestion.

3 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

• serving at scripts/nemoclaw-start.sh:4725: selected only by the second-opinion lane as justified.
• not-serving at scripts/nemoclaw-start.sh:4776: selected only by the second-opinion lane as define.
• inconclusive at scripts/nemoclaw-start.sh:4777: selected only by the second-opinion lane as conflict.

2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

• ubuntu-repo-cloud-openclaw: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• issue-2478-crash-loop-recovery: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• define — not-serving at docs/reference/troubleshooting.mdx:686: Keep the explicit serving-response contrast and the listed not-serving outcomes.
• established — inconclusive probe at docs/reference/troubleshooting.mdx:699: Keep this established term and its explicit no-signal, counter-preservation behavior.
• established — serving watchdog at docs/reference/troubleshooting.mdx:683: Keep the established component name and the response-qualified serving definition.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: gateway-guard-recovery

1 optional E2E recommendation

• sandbox-survival

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[TonyLuo-NV]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/reference/troubleshooting.mdx`:
- Line 648: Update the troubleshooting sentence around the
gateway_transport_error description to remove both em dashes, replacing them
with guideline-compliant punctuation while preserving the existing meaning and
inline code formatting.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5eb7388f-dc10-44bd-ace6-abd68060d04c

📥 Commits

Reviewing files that changed from the base of the PR and between 61856cb and c5f3264.

📒 Files selected for processing (5)

• docs/reference/troubleshooting.mdx
• scripts/nemoclaw-start.sh
• test/gateway-serving-watchdog.test.ts
• test/nemoclaw-start-gateway-health.test.ts
• test/nemoclaw-start-gateway.test-helpers.ts

[TonyLuo-NV]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[TonyLuo-NV]

Both automated review findings are addressed:

• CodeQL js/unused-local-variable: removed the extractGatewayLogAppendFunction import that was orphaned when the watchdog suite moved to its own file (2248e82c5). The alert is no longer open against this branch.
• CodeRabbit em dashes in docs/reference/troubleshooting.mdx: rewritten with commas per the writing guide in docs/CONTRIBUTING.md (e3725bf68).

All checks now pass. The earlier PR review advisor (GPT-5.6 Terra) red was the advisor's own analysis run not completing rather than a finding; it passed on the latest run, so the quality-gate note about needing maintainer acceptance no longer applies and I have removed it from the description.

Both advisor lanes report 0 blockers, 0 warnings, 0 suggestions.

[TonyLuo-NV]

CI status update after the base moved forward.

Three checks went red on the latest run. Two were transient and cleared on a re-run with no code change:

• build-sandbox-images-arm64: the image build failed while install-reviewed-runtime.sh fetched eventsource-parser@3.1.0 from the npm registry. Passed on re-run in 2m18s.
• build-typecheck: public-cli-contracts.test.ts failed because dist/lib/state/mcp-lifecycle-lock-acquisition.js hit SyntaxError: Unexpected token '.', a bad build artifact rather than a source defect. Passed on re-run in 7m17s. This branch changes no file under src/.
• checks was only the aggregate gate reporting the build-typecheck failure, and it is green now.

E2E / PR Gate and E2E / PR Gate Coordination also completed green.

That leaves PR review advisor (Nemotron 3 Ultra), which fails with PR review advisor analysis did not complete: outcome=failure. That is the advisor's own analysis run not finishing, not a finding. I re-ran it twice with the same result, and it passed on an earlier commit of this branch. The aggregated advisor comment records recommendation: merge_as_is with 0 blockers, 0 warnings, and 0 suggestions, and the primary GPT-5.6 Terra lane completed with high confidence.

Current tally: 45 passing, 1 failing on that advisor lane. A maintainer needs to decide whether to accept it; I have restored the corresponding Quality Gates line in the description rather than marking it resolved.

Note: this branch is behind main by two commits (a931be4f0, 4cd4d64fe). Neither touches the watchdog, and CI already tests the merge result, so I have not rebased. Say the word if you would like the branch refreshed.

[cv]

Security review: PASS

Verdict

PASS for commit 8d6411a0d15a7b3fc32325b2774462e2b0bb3e9c against base SHA 4cd4d64fe67143b57707f874afa0b9d269dfeff2. The watchdog broadens recovery from connection-refused probes to all defined not-serving results, but it does not broaden process authority: it arms only after a 200 or 401 response, retains the tracked PID start identity, checks the gateway command line, and revalidates the PID identity immediately before signaling. No security findings remain.

Findings table

The review found no findings.

Detailed analysis

1. Secrets and credentials — PASS. No secret, token, credential, connection string, or credential fixture was added or logged.
2. Input validation and data sanitization — PASS. The existing validated loopback port remains a quoted curl argument. Probe exit codes and HTTP status values are classified without eval or shell-generated command text. Invalid watchdog interval and threshold values still fall back to positive defaults.
3. Authentication and authorization — PASS. No endpoint or authentication contract changes. Process signaling still requires prior serving evidence, the tracked PID/start-identity pair, a gateway command-line match, and a second live-identity check immediately before the signal.
4. Dependencies and third-party libraries — PASS. No dependency, image, package source, or version changed.
5. Error handling and logging — PASS. Probe failures are fail-safe: unknown curl failures are inconclusive and cannot trigger a signal. Logs contain only the tracked PID, loopback port, bounded failure count, and classified cause; they do not contain credentials or personal data.
6. Cryptography and data protection — PASS. No cryptography, storage, transport trust, or sensitive-data handling changed.
7. Configuration and security headers — PASS. No listener, CORS, CSP, network policy, privilege, image, or security-header setting changed. The probe remains limited to the sandbox loopback health endpoint.
8. Security testing — PASS. Tests cover pre-serving refusal, recovery and streak reset, each admitted transport failure, HTTP error responses, 401, alternating failures, missing or inconclusive curl, invalid configuration, command-line mismatch, PID replacement, and signal-time identity handling.
9. System security — PASS. The change retains least-privilege signaling and the existing PID-reuse defenses. An unavailable or unexpected probe cannot arm or advance recovery, a replacement process must establish its own serving history, and the target is revalidated before TERM or KILL.

Files reviewed

• docs/reference/troubleshooting.mdx
• scripts/nemoclaw-start.sh
• test/gateway-serving-watchdog.test.ts
• test/nemoclaw-start-gateway-health.test.ts
• test/nemoclaw-start-gateway.test-helpers.ts

GitHub CI is authoritative for validation. I did not run a duplicate local test suite; the normal commit and pre-push hooks passed, including repository checks, ShellCheck, secret scanning, formatting, and CLI TypeScript validation.

[apurvvkumaria]

Reviewed current head c1e3c19. The runtime behavior is safe: arming requires HTTP 200 or 401, transport failures remain not-serving, unknown probe failures stay inconclusive, and PID identity plus command line are rechecked before signaling. Required CI and E2E pass, so this is safe to merge.\n\nNon-blocking fast-follow: the swap-test curl stub in test/gateway-serving-watchdog.test.ts returns success without printing an HTTP status, so the classifier sees HTTP 000 and the test does not actually exercise per-PID armed-state reset. Please narrowly follow up by printing 200 in the success branch or reusing the status-aware curl-plan stub. This is a test-confidence improvement, not a merge blocker.

[apurvvkumaria]

Reviewed exact head 47accd8. The runtime and documentation behavior remain unchanged from the prior review. The only PR-specific follow-up makes the PID-swap test emit HTTP 200, so it now genuinely exercises per-PID watchdog state reset. The focused watchdog suite passes 18 of 18 tests. Current unrelated CI failures reproduce in files identical to the current base; no blocking correctness, security, regression, or compatibility defect is attributable to this PR.

[TonyLuo-NV]

Branch refreshed on current main (47accd83a). It was eight commits behind; the merge was clean with no conflicts, and the watchdog suite still passes locally (18/18), as do the sibling gateway-health and kill-marker suites.

Current CI has three distinct failures, none of them from the watchdog change. This branch touches only scripts/nemoclaw-start.sh, three test files, and one docs page.

1. reviewed-npm-audit threshold (repo-wide). install-reviewed-runtime.sh fails the audit gate on mcp-tool-discovery-runtime dependencies with 3 vulnerabilities (1 moderate, 2 high). This cascades into build-sandbox-images, build-sandbox-images-arm64, checks, cli-tests, and both E2E gate jobs, which only report PR #8092 CI did not pass. Re-running does not help. Unrelated PRs (#8183, #8182, #8179, #8174) fail the same way, so this needs a dependency bump or an entry in ci/npm-audit-exceptions.json from someone who owns that surface.

2. cli-test-shards (8). src/lib/onboard/managed-image-registry-fetch.test.ts:39 times out at 5000ms, three runs in a row. That test binds a real HTTP server and proxy and belongs to #7772, not to this branch. It passes locally against this exact merge (5/5). Shard 8 passed on this branch before the merge, so the likely cause is that the merge shifted the shard split and put this test alongside heavier neighbours rather than any behavior change. Flagging rather than re-running further, and I have not touched the upstream test's timeout.

3. Previously transient, now green. build-typecheck failed once on public-cli-contracts.test.ts (check-docs: running: [cli]) and passed on re-run, same as yesterday.

Both PR review advisor lanes pass, along with all CodeQL and ShellCheck jobs and shards 1-7.