Skip to content

fix(sandbox): retry dropped OpenShell relay during recovery#7509

Merged
senthilr-nv merged 7 commits into
mainfrom
codex/fix-e2e-gateway-recovery-readiness
Jul 24, 2026
Merged

fix(sandbox): retry dropped OpenShell relay during recovery#7509
senthilr-nv merged 7 commits into
mainfrom
codex/fix-e2e-gateway-recovery-readiness

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Sandbox recovery now retries the exact OpenShell Unavailable: relay channel dropped response during replacement-supervisor registration.
Before this change, recovery stopped before the bounded readiness deadline even when the sandbox became reachable immediately afterward.

Product Scope

This change fixes the existing sandbox recovery path.
It does not add an integration, recipe, image, third-party stack, or supported product surface.

Reproduction

E2E main run 30125605207 failed gateway-guard-recovery on both attempts.
The second attempt stopped recovery after 48 seconds.
Its immediate post-failure OpenShell diagnostic reached the healthy gateway in 221 milliseconds.

OpenShell 0.0.85 reports Unavailable: relay channel dropped when a connected replacement supervisor drops its reverse relay before the readiness probe receives the stream.
The readiness classifier retried adjacent session, timeout, missing-target, and refused-target states but treated this response as terminal.

Changes

  • Retry only the exact OpenShell unavailable stream response with message: "relay channel dropped".
  • Keep the existing managed-health guard, readiness deadline, and fail-closed handling for unrelated errors.
  • Preserve the last non-empty normalized OpenShell readiness error when a later timeout has empty stderr.
  • Add focused readiness and caller-level regressions, including proof that a failed readiness check reports the final relay error without starting the primary forward.

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: Existing recovery documentation already defines bounded OpenShell re-registration retries and fail-closed forward handling.
  • 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: Independent human review is pending.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-not-needed
  • Evidence: Existing recovery documentation covers bounded OpenShell re-registration retries and stopped-forward failure handling. Preserving and reporting the final normalized error changes no retry, timeout, acceptance, forwarding, or remediation semantics.
  • Agent: Codex documentation writer reviewer

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable
  • Station profile/scenario: Not applicable
  • Result: Not applicable
  • Supporting evidence: Not applicable

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 check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — focused CLI tests passed 38/38; supervisor-relaunch integration tests passed 10/10; npm run typecheck:cli, Biome, normal pre-commit and commit-msg hooks, and git diff --check passed.
  • Applicable broad gate passed — Not applicable to the additive exact-error classifier and focused regressions.
  • Quality Gates section completed with required justifications or waivers — Sensitive-path human review is pending.
  • No secrets, API keys, or credentials committed
  • 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)

The trusted gateway-guard-recovery live target passed against the pre-merge production head in run 30131659511 and is rerunning against current-base head 159b243 in run 30132635865.


Signed-off-by: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved sandbox recovery when the OpenShell relay channel drops during reconnection.
    • Recovery now treats “relay channel dropped” as a transient condition and retries until readiness is restored after a subsequent connection attempt.
    • Enhanced readiness failure reporting by including the latest OpenShell structured error details when reconnection readiness ultimately fails.
  • Tests
    • Added coverage for the “relay channel dropped” stderr scenario and verified the readiness retry behavior.

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv senthilr-nv self-assigned this Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

OpenShell relay-channel drops are now retryable during recreated sandbox readiness checks. Readiness failures preserve the latest structured OpenShell error and include it in sandbox recovery failure details. Tests cover retry success and timeout reporting.

Changes

Sandbox recovery

Layer / File(s) Summary
Retry dropped OpenShell relay
src/lib/actions/sandbox/process-recovery.ts, src/lib/actions/sandbox/process-recovery.test.ts
The retry classifier recognizes relay-channel-dropped errors with unavailable status markers, and tests verify retry success, probe counts, and sleep intervals.
Propagate readiness errors
src/lib/actions/sandbox/process-recovery.ts, test/process-recovery-supervisor-relaunch.test.ts
Readiness failures retain the latest structured OpenShell error, include it in recovery failure details, and verify timeout reporting during supervisor relaunch recovery.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: prekshivyas

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: retrying a dropped OpenShell relay during sandbox recovery.
✨ 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 codex/fix-e2e-gateway-recovery-readiness

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

@github-code-quality

github-code-quality Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 159b243 in the codex/fix-e2e-gatewa... branch remains at 96%, unchanged from commit 2682082 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 159b243 in the codex/fix-e2e-gatewa... branch remains at 80%, unchanged from commit 2682082 in the main branch.

Show a code coverage summary of the most impacted files.
File main 2682082 codex/fix-e2e-gatewa... 159b243 +/-
src/lib/shields/index.ts 72% 71% -1%
src/lib/actions...ess-recovery.ts 81% 80% -1%
src/lib/messagi...nnels/policy.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/securit...ntial-filter.ts 93% 93% 0%
src/lib/trace.ts 94% 94% 0%
src/lib/state/m...-acquisition.ts 87% 89% +2%
src/lib/platform.ts 84% 89% +5%

Updated July 24, 2026 23:09 UTC

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 2 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 2 more warnings, the same number of suggestions.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

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

Recommended E2E: onboard-repair, onboard-resume

1 optional E2E recommendation
  • gateway-guard-recovery

Workflow run details

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

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@senthilr-nv senthilr-nv added bug-fix PR fixes a bug or regression area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery labels Jul 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@src/lib/actions/sandbox/process-recovery.ts`:
- Line 760: Update the assignment to lastOpenshellError in the process-recovery
flow so normalization results are only stored when non-empty. Preserve the
existing last structured OpenShell error when a later retry produces empty
stderr, while retaining normal updates for non-empty normalized diagnostics.
🪄 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: 55d62e26-db83-46f6-88aa-f7290a4c690c

📥 Commits

Reviewing files that changed from the base of the PR and between 93feb7d and 48f65b6.

📒 Files selected for processing (1)
  • src/lib/actions/sandbox/process-recovery.ts

Comment thread src/lib/actions/sandbox/process-recovery.ts Outdated
Signed-off-by: Senthil Ramamoorthy <senthilr@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@test/process-recovery-supervisor-relaunch.test.ts`:
- Around line 358-415: Wrap the test body in a try/finally cleanup block,
ensuring all setup, execution, and assertions remain inside try. In finally,
call vi.unstubAllEnvs() and vi.restoreAllMocks() to restore the environment
stubs and locally created spies even when assertions fail.
- Around line 358-415: Update the test setup in “reports the last structured
OpenShell error when readiness times out” to use a one-second readiness deadline
while retaining the zero polling interval, ensuring the second captureOpenshell
mock result is consumed. Add an assertion that captureOpenshell was called
twice, while preserving the existing relay-error detail and recovery assertions.
🪄 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: 9e26c80b-838e-45cc-8e11-05950b09cfbe

📥 Commits

Reviewing files that changed from the base of the PR and between 48f65b6 and 32b8795.

📒 Files selected for processing (2)
  • src/lib/actions/sandbox/process-recovery.ts
  • test/process-recovery-supervisor-relaunch.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/sandbox/process-recovery.ts

Comment thread test/process-recovery-supervisor-relaunch.test.ts
@senthilr-nv
senthilr-nv merged commit 9aaca48 into main Jul 24, 2026
80 of 81 checks passed
@senthilr-nv
senthilr-nv deleted the codex/fix-e2e-gateway-recovery-readiness branch July 24, 2026 23:18
@senthilr-nv senthilr-nv mentioned this pull request Jul 25, 2026
23 tasks
senthilr-nv added a commit that referenced this pull request Jul 25, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical pre-tag `## v0.0.95` release entry to
`docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The
entry summarizes approved user-visible changes merged since v0.0.94 and
excludes internal-only prerequisites.

## Changes

- Adds the v0.0.95 summary and detailed bullets for gateway lifecycle,
recovery, state transfer, inference compatibility, sandbox security,
Discord policy, and E2E evidence.
- Links each user-facing theme to the most specific published
documentation.
- Records the release entry in the shared native changelog used by the
OpenClaw, Hermes, and Deep Agents guides.

Source summary:

- [#7246](#7246),
[#7228](#7228),
[#7267](#7267),
[#7489](#7489),
[#7509](#7509),
[#7351](#7351), and
[#7290](#7290) ->
`docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and
retry, managed recovery, Hermes restart recovery, scoped uninstall, and
orphan-aware backup behavior.
- [#7344](#7344) and
[#7416](#7416) ->
`docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download
verification.
- [#7476](#7476),
[#7347](#7347),
[#7281](#7281),
[#7485](#7485),
[#7491](#7491), and
[#7422](#7422) ->
`docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback,
bounded OpenRouter connection setup, Nemotron-3 request compatibility,
and managed Deep Agents retry and provider-error behavior.
- [#6884](#6884),
[#7481](#7481),
[#6878](#6878),
[#7467](#7467),
[#7502](#7502),
[#7503](#7503),
[#7504](#7504), and
[#7486](#7486) ->
`docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local
rebuild images, runtime validation, config preservation, reviewed
package updates, and fewer final-image payload layers.
- [#7303](#7303) ->
`docs/changelog/2026-07-24.mdx`: Scoped Discord application-command
management.
- [#7488](#7488),
[#7465](#7465),
[#7497](#7497),
[#7464](#7464),
[#7501](#7501),
[#7494](#7494), and
[#7493](#7493) ->
`docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry
cleanup, full root-image validation, direct-main Hermes setup, executed
PR-gate evidence, nightly history, and runner wait reporting.
- [#7447](#7447) is an internal
pinned-runtime prerequisite and is intentionally excluded from canonical
supported-integration documentation.
- [#7370](#7370) adds
maintainer-only advisory reconciliation tooling and does not change
supported user behavior.
- [#7495](#7495) updates existing
documentation and does not add a new v0.0.95 behavior claim.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated changelog structure,
heading uniqueness, and published links.
- [ ] Tests not applicable — justification:
- [x] 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:
- [ ] 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/changelog/2026-07-24.mdx`; writing rules,
documentation style, factual release meaning, and published links
reviewed at exact head `58b02f2bf`.
- Agent: Codex documentation writer reviewer
<!-- docs-review-head-sha: 58b02f2 -->
<!-- docs-review-agents-blob-sha: 9c9b36d -->

## 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 check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 6 tests.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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) — the
build passed with 0 errors and 2 Fern 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: Senthil Ravichandran <senthilr@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added a new v0.0.95 changelog entry above v0.0.94.
* Documented improved externally supervised gateway lifecycle ownership.
  * Improved snapshot restore reliability and SQLite state handling.
  * Tightened CLI `backup-all` behavior and host artifact verification.
* Updated Windows onboarding guidance (including Ollama service reuse
and CDI directory fallback).
* Noted inference compatibility fixes, deeper agent failure
classification, stricter base-image validation, updated Discord bot
command permissions, and refined E2E release automation evidence
handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants