Skip to content

fix(onboard): detach terminal agent create stream on ready - #5768

Merged
cv merged 3 commits into
mainfrom
fix/deepagents-terminal-ready-detach
Jun 26, 2026
Merged

fix(onboard): detach terminal agent create stream on ready#5768
cv merged 3 commits into
mainfrom
fix/deepagents-terminal-ready-detach

Conversation

@sandl99

@sandl99 sandl99 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Terminal-runtime sandboxes now continue onboarding once OpenShell reports Ready, even when the VM create stream does not emit gateway-style startup output. This fixes DeepAgents Code onboarding hanging on macOS VM-driver hosts after the sandbox is already Ready.

Bugs

LangChain hang at sandbox build MacOS

  Sandbox reported Ready; waiting for startup command output before detaching.
  Still waiting for sandbox to become ready... (30s elapsed)
  Still waiting for sandbox to become ready... (45s elapsed)
  Still waiting for sandbox to become ready... (60s elapsed)
  Still waiting for sandbox to become ready... (75s elapsed)
  Still waiting for sandbox to become ready... (90s elapsed)
  Still waiting for sandbox to become ready... (105s elapsed)
  Still waiting for sandbox to become ready... (120s elapsed)
  Still waiting for sandbox to become ready... (135s elapsed)
  Still waiting for sandbox to become ready... (150s elapsed)
  Still waiting for sandbox to become ready... (165s elapsed)
  Still waiting for sandbox to become ready... (180s elapsed)
  Still waiting for sandbox to become ready... (195s elapsed)
  Still waiting for sandbox to become ready... (210s elapsed)

Related Issue

Fixes #5766

Changes

  • Disable the create-stream startup-output guard for terminal agents while preserving the OpenShell Ready check.
  • Keep the DeepAgents Code entrypoint alive with a portable tail -f /dev/null idle path and emit a setup marker.
  • Extend terminal-agent onboarding and DeepAgents entrypoint tests to cover VM-driver Ready detachment without startup output.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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)

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

Summary by CodeRabbit

  • New Features
    • Improved startup behavior for idle runtimes: when launched without a command, it now prints a clearer runtime setup message before idling.
  • Bug Fixes
    • Enhanced sandbox onboarding for terminal-based flows by adjusting how “ready” is detected during sandbox creation.
    • Updated and expanded test coverage to validate the new startup message, idle behavior, and readiness parsing.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this Jun 25, 2026
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The DeepAgents code entrypoint now logs a startup message and idles with tail -f /dev/null. Terminal-agent sandbox onboarding also changes ready-check output handling, and the related tests update for the new startup and scenario flow.

Changes

Terminal-agent startup and onboarding

Layer / File(s) Summary
Entry point keepalive
agents/langchain-deepagents-code/start.sh, test/dcode-start-keepalive.test.ts, test/langchain-deepagents-code-image.test.ts
The no-command entrypoint now prints the NemoClaw Deep Agents Code runtime setup message and stays alive with tail -f /dev/null; the keep-alive and image-contract tests assert the new stdout and script contents.
Terminal-agent ready check
src/lib/onboard.ts, test/onboard-terminal-dashboard.test.ts
Terminal-agent sandbox creation disables ready-check output-pattern matching, and the onboarding scenario test updates JSON parsing, process keepalive handling, spawn mocking, and driver/timeout setup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5197: Shares the same terminal-agent harness and onboarding flow, including agents/langchain-deepagents-code/start.sh and src/lib/onboard.ts.
  • NVIDIA/NemoClaw#5725: Also changes the DeepAgents code keepalive behavior in start.sh and updates the corresponding keep-alive test expectations.

Suggested labels

area: sandbox, bug-fix, integration: dcode

Suggested reviewers

  • cv

Poem

A rabbit hopped through the startup glen,
Whispering, “tail -f keeps me here again.”
🐇✨ Ready checks blinked, then softly slid,
While terminal dreams stayed neatly hid.
Hop, hop—onboarding now feels bright and zen.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.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
Title check ✅ Passed The title clearly reflects the main change: detaching terminal-agent onboarding when the sandbox is ready.
Linked Issues check ✅ Passed The code disables startup-output gating for terminal agents while preserving Ready-based detachment, matching issue #5766.
Out of Scope Changes check ✅ Passed The changes are focused on the onboarding hang fix and related entrypoint/tests, with no clear unrelated additions.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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/deepagents-terminal-ready-detach

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

@github-code-quality

github-code-quality Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/deepagents-termi... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/deepagents-termi... 6513e53 +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the fix/deepagents-termi... branch is 47%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/deepagents-termi... 6513e53 +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 71%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 60%
src/lib/state/sandbox.ts 55%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 19%

Updated June 25, 2026 18:24 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e
Optional E2E: sandbox-operations-e2e

Dispatch hint: cloud-onboard-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (medium): Runs a real hosted install/onboard/create/wait-for-ready flow through src/lib/onboard.ts, providing merge-blocking confidence that the sandbox creation/readiness changes did not regress the standard hosted onboarding path.

Optional E2E

  • sandbox-operations-e2e (high): Useful adjacent coverage for real OpenShell sandbox lifecycle operations after a change in createSandbox readiness handling, but it does not specifically exercise the terminal-agent Deep Agents Code path.

New E2E recommendations

  • terminal-agent onboarding and keepalive (high): No existing selective E2E job appears to onboard NEMOCLAW_AGENT=langchain-deepagents-code and verify the real OpenShell sandbox reaches/stays Ready with the terminal-runtime entrypoint. The available Deep Agents checks under cloud-onboard skip unless the sandbox is already Deep Agents Code.
    • Suggested test: Add a deepagents-code-onboard-e2e selective job that installs from the PR ref with NEMOCLAW_AGENT=langchain-deepagents-code, creates a sandbox, asserts it reaches Ready and remains Ready for a short stability window, verifies no dashboard health/forward is expected, and runs a minimal dcode/terminal smoke check via openshell sandbox exec or nemoclaw connect --probe-only.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-openclaw
Optional Vitest E2E scenarios: gpu-e2e-vitest

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-openclaw: src/lib/onboard.ts changes the live sandbox creation readiness path. The closest live-supported typed Vitest scenario exercises real Ubuntu/Docker cloud onboarding through createSandbox and validates the resulting sandbox state. The exact LangChain Deep Agents Code typed scenario is present in the registry but is not live-supported by the trusted runtime-support wiring, so it cannot be targeted directly.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional Vitest E2E scenarios

  • gpu-e2e-vitest: Optional special-runner coverage for the Docker GPU sandbox creation/reconnect path adjacent to the readiness changes and the Deep Agents Code keep-alive regression context. This requires a GPU runner and is not the primary live path for the PR.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=gpu-e2e-vitest

Relevant changed files

  • agents/langchain-deepagents-code/start.sh
  • src/lib/onboard.ts

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: Terminal-agent VM create-stream Ready detachment.
Open items: 0 required · 4 warnings · 0 suggestions · 8 test follow-ups
Since last review: 0 prior items resolved · 2 still apply · 1 new item found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: Terminal-agent VM create-stream Ready detachment
  • PRA-2 Resolve or justify: Resolve the terminal-agent VM startup-output guard bypass against the new setup marker in src/lib/onboard.ts:3067
  • PRA-3 Resolve or justify: Cover non-terminal VM create-stream guard preservation in src/lib/onboard.ts:3067
  • PRA-4 Resolve or justify: Acceptance evidence remains simulated for the macOS VM and in-sandbox diagnostic clauses in test/onboard-terminal-dashboard.test.ts:141
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Cover non-terminal VM create-stream guard preservation
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify architecture src/lib/onboard.ts:3067 Prefer one source of truth in this PR. If the new DeepAgents setup marker is expected to reach the create stream, remove the terminal-agent `readyCheckOutputPatterns: []` override and add a test that proves the default VM marker guard detaches after that marker. If the bypass is still required because terminal entrypoint stdout is not observable through OpenShell VM create streams, document that boundary in code and constrain the exception as narrowly as possible.
PRA-3 Resolve/justify tests src/lib/onboard.ts:3067 Add a focused regression around `streamSandboxCreate` or the onboard create path for a VM-driver, non-terminal/gateway agent. Have `readyCheck()` return true before any `Setting up NemoClaw` output and assert the create stream records the wait message and does not force-detach until the marker appears or the child exits.
PRA-4 Resolve/justify acceptance test/onboard-terminal-dashboard.test.ts:141 Either identify existing automated coverage that covers the DeepAgents terminal smoke/policy checks for the live acceptance clauses, or add a narrowly scoped test note/fixture assertion for the closest local behavior. Do not replace the focused unit-style regression with a large new E2E framework.
Review findings by urgency: 0 required fixes, 4 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Terminal-agent VM create-stream Ready detachment

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Positive terminal Ready-without-marker coverage exists in `test/onboard-terminal-dashboard.test.ts`; missing coverage is either marker-based terminal detachment with the default guard or a documented intentional bypass plus non-terminal negative guard coverage.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `agents/langchain-deepagents-code/start.sh` prints `Setting up NemoClaw Deep Agents Code runtime...`; `src/lib/sandbox/create-stream.ts` defaults VM patterns to `/Setting up NemoClaw/`; `src/lib/onboard.ts:3067` passes `[]` for terminal agents.

PRA-2 Resolve/justify — Resolve the terminal-agent VM startup-output guard bypass against the new setup marker

  • Location: src/lib/onboard.ts:3067
  • Category: architecture
  • Problem: This PR adds `Setting up NemoClaw Deep Agents Code runtime...` to the DeepAgents entrypoint, which matches the existing VM create-stream default `/Setting up NemoClaw/` marker contract. The same PR also passes `readyCheckOutputPatterns: []` for every terminal agent, disabling that marker guard entirely for terminal VM creates. Those two fixes point at different sources of truth: either the entrypoint marker is the source fix, or terminal-agent Ready state is intended to be sufficient without startup-output evidence.
  • Impact: NemoClaw can proceed from a terminal VM sandbox solely because OpenShell reports Ready, with no streamed evidence that the expected startup command ran. That may mask future terminal-agent startup regressions and weakens sandbox lifecycle diagnostics around a security-sensitive trusted-state transition.
  • Recommended action: Prefer one source of truth in this PR. If the new DeepAgents setup marker is expected to reach the create stream, remove the terminal-agent `readyCheckOutputPatterns: []` override and add a test that proves the default VM marker guard detaches after that marker. If the bypass is still required because terminal entrypoint stdout is not observable through OpenShell VM create streams, document that boundary in code and constrain the exception as narrowly as possible.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `agents/langchain-deepagents-code/start.sh` for the new `Setting up NemoClaw Deep Agents Code runtime...` line, `src/lib/sandbox/create-stream.ts` for `VM_READY_DETACH_OUTPUT_PATTERNS = [/Setting up NemoClaw/]`, and `src/lib/onboard.ts:3067` for the terminal-agent empty-pattern override.
  • Missing regression test: Add `terminal VM create stream detaches after DeepAgents setup marker with default readyCheckOutputPatterns`: use `OPENSHELL_DRIVERS=vm`, a fake create child that emits `Created sandbox:` and `Setting up NemoClaw Deep Agents Code runtime...`, `readyCheck()` returning true, and assert `forcedReady`/registration occurs without passing an empty pattern list. If keeping the bypass, add `terminal VM Ready without setup marker intentionally bypasses startup-output wait` and assert the wait message is absent.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `agents/langchain-deepagents-code/start.sh` for the new `Setting up NemoClaw Deep Agents Code runtime...` line, `src/lib/sandbox/create-stream.ts` for `VM_READY_DETACH_OUTPUT_PATTERNS = [/Setting up NemoClaw/]`, and `src/lib/onboard.ts:3067` for the terminal-agent empty-pattern override.
  • Evidence: `start.sh` now prints a line matching `/Setting up NemoClaw/`, while `streamSandboxCreate` already uses that regex as the VM default when `readyCheckOutputPatterns` is `undefined`; `onboard.ts` instead passes `[]` for all terminal agents.

PRA-3 Resolve/justify — Cover non-terminal VM create-stream guard preservation

  • Location: src/lib/onboard.ts:3067
  • Category: tests
  • Problem: The changed terminal-agent test exercises a VM create stream where OpenShell reports Ready before any setup marker, but there is still no nearby negative regression proving gateway/non-terminal VM creates continue to wait for the default `Setting up NemoClaw` marker before forced-ready detachment.
  • Impact: A later refactor could accidentally broaden the terminal empty-pattern override to gateway agents, causing NemoClaw to register or continue from a VM sandbox based only on `openshell sandbox list` Ready state. That weakens the startup-output guard used for gateway-style create streams and reduces lifecycle integrity and diagnostics.
  • Recommended action: Add a focused regression around `streamSandboxCreate` or the onboard create path for a VM-driver, non-terminal/gateway agent. Have `readyCheck()` return true before any `Setting up NemoClaw` output and assert the create stream records the wait message and does not force-detach until the marker appears or the child exits.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/sandbox/create-stream.ts` around `getReadyCheckOutputPatterns`; then search tests for `Sandbox reported Ready; waiting for startup command output`, `readyCheckOutputPatterns`, or a non-terminal VM Ready-before-marker case.
  • Missing regression test: Add `non-terminal VM create waits for startup-output marker before forced-ready detach`: set `OPENSHELL_DRIVERS=vm`, use a non-terminal agent or direct `streamSandboxCreate` call, return Ready early, emit no marker initially, and assert no forced-ready result until `Setting up NemoClaw` is emitted or the child closes.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/sandbox/create-stream.ts` around `getReadyCheckOutputPatterns`; then search tests for `Sandbox reported Ready; waiting for startup command output`, `readyCheckOutputPatterns`, or a non-terminal VM Ready-before-marker case.
  • Evidence: `streamSandboxCreate` defaults VM creates to `[/Setting up NemoClaw/]` when `readyCheckOutputPatterns` is `undefined`; this PR passes `[]` only for terminal agents, but changed tests model only the terminal DeepAgents path.

PRA-4 Resolve/justify — Acceptance evidence remains simulated for the macOS VM and in-sandbox diagnostic clauses

  • Location: test/onboard-terminal-dashboard.test.ts:141
  • Category: acceptance
  • Problem: The linked issue includes live macOS/OpenShell VM, `openshell sandbox get`, in-sandbox `dcode --version`, and OpenShell log diagnostic clauses. The changed tests simulate the VM driver and Ready state, which is useful bug-path coverage, but they do not provide automated evidence for the live in-sandbox policy/smoke/log parts of those clauses.
  • Impact: The core hang regression is covered by a controlled fake, but reviewers cannot tell from this diff whether the specific macOS VM/in-sandbox acceptance observations are covered elsewhere or intentionally left to manual validation.
  • Recommended action: Either identify existing automated coverage that covers the DeepAgents terminal smoke/policy checks for the live acceptance clauses, or add a narrowly scoped test note/fixture assertion for the closest local behavior. Do not replace the focused unit-style regression with a large new E2E framework.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/onboard-terminal-dashboard.test.ts` for the simulated `OPENSHELL_DRIVERS=vm` create path, and `test/onboard-terminal.test.ts` plus `test/langchain-deepagents-code-image.test.ts` for existing DeepAgents smoke/policy contract coverage.
  • Missing regression test: If existing coverage is not considered sufficient, add `DeepAgents terminal onboard verifies smoke command after VM Ready registration` using the existing terminal-smoke fixtures, or document which existing `onboard-terminal` and image-contract tests cover `/sandbox/.deepagents/config.toml` and `dcode --version` behavior.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/onboard-terminal-dashboard.test.ts` for the simulated `OPENSHELL_DRIVERS=vm` create path, and `test/onboard-terminal.test.ts` plus `test/langchain-deepagents-code-image.test.ts` for existing DeepAgents smoke/policy contract coverage.
  • Evidence: The create scenario sets `OPENSHELL_DRIVERS: scenario === "create" ? "vm" : "docker"` and fakes `sandbox list` as Ready, but no changed test runs `openshell sandbox get`, validates `/sandbox/.deepagents/config.toml`, executes `dcode --version` in a live sandbox, or inspects OpenShell logs.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Simplification opportunities: 1 possible cut, net -1 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-2 yagni (src/lib/onboard.ts:3067): `readyCheckOutputPatterns: agentDefs.isTerminalAgent(agent) ? [] : undefined`
    • Replacement: Rely on the existing VM default marker guard if the new DeepAgents setup marker is observable by the create stream; otherwise replace the broad bypass with a documented, narrow exception.
    • Net: -1 lines
    • Safety boundary: Do not remove the OpenShell Ready check or the default non-terminal VM `/Setting up NemoClaw/` startup-output guard; preserve credential redaction, sandbox readiness diagnostics, and create failure handling.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — Add `terminal VM create stream detaches after DeepAgents setup marker with default readyCheckOutputPatterns`: fake a VM create stream that emits `Created sandbox:` and `Setting up NemoClaw Deep Agents Code runtime...`, then assert Ready detachment works without passing `[]`.. The PR touches sandbox entrypoint and onboarding lifecycle behavior. Changed tests cover the DeepAgents keepalive contract and the terminal Ready-detach bug path with mocks, but lifecycle safety would be stronger with negative VM guard coverage and a clear marker-vs-bypass source-of-truth test.
  • PRA-T2 Runtime validation — Add `non-terminal VM create waits for startup-output marker before forced-ready detach`: use `OPENSHELL_DRIVERS=vm`, return Ready before any marker, and assert the wait message is recorded and forced-ready does not happen until `Setting up NemoClaw` appears or the child exits.. The PR touches sandbox entrypoint and onboarding lifecycle behavior. Changed tests cover the DeepAgents keepalive contract and the terminal Ready-detach bug path with mocks, but lifecycle safety would be stronger with negative VM guard coverage and a clear marker-vs-bypass source-of-truth test.
  • PRA-T3 Runtime validation — If the terminal bypass is intentionally retained, add `terminal VM Ready without setup marker bypasses startup-output wait by design`: assert the wait message is absent and document why OpenShell Ready is authoritative for terminal runtimes.. The PR touches sandbox entrypoint and onboarding lifecycle behavior. Changed tests cover the DeepAgents keepalive contract and the terminal Ready-detach bug path with mocks, but lifecycle safety would be stronger with negative VM guard coverage and a clear marker-vs-bypass source-of-truth test.
  • PRA-T4 Runtime validation — Identify existing coverage or add a focused terminal smoke/policy assertion for the linked issue's `/sandbox/.deepagents/config.toml` and `dcode --version` acceptance evidence.. The PR touches sandbox entrypoint and onboarding lifecycle behavior. Changed tests cover the DeepAgents keepalive contract and the terminal Ready-detach bug path with mocks, but lifecycle safety would be stronger with negative VM guard coverage and a clear marker-vs-bypass source-of-truth test.
  • PRA-T5 Cover non-terminal VM create-stream guard preservation — Add a focused regression around `streamSandboxCreate` or the onboard create path for a VM-driver, non-terminal/gateway agent. Have `readyCheck()` return true before any `Setting up NemoClaw` output and assert the create stream records the wait message and does not force-detach until the marker appears or the child exits.
  • PRA-T6 Acceptance clause — Loaded `nemoclaw-contributor-create-pr` for branch/PR workflow. — add test evidence or identify existing coverage. This is diagnostic workflow context from the linked issue, not product behavior changed by the diff.
  • PRA-T7 Acceptance clause — Investigated a live `langchain-deepagents-code` onboard run on macOS/OpenShell VM driver. — add test evidence or identify existing coverage. `test/onboard-terminal-dashboard.test.ts` simulates the create path with `OPENSHELL_DRIVERS=vm`, but the diff does not include a live macOS/OpenShell VM validation artifact.
  • PRA-T8 Acceptance clause — `openshell sandbox get dp` showed DeepAgents/terminal policy state (`/sandbox/.deepagents`, `dcode`). — add test evidence or identify existing coverage. Existing nearby DeepAgents terminal/image tests cover terminal manifests, smoke commands, and policy contracts, but the changed tests do not inspect live `sandbox get` output.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: Terminal-agent VM create-stream Ready detachment

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Positive terminal Ready-without-marker coverage exists in `test/onboard-terminal-dashboard.test.ts`; missing coverage is either marker-based terminal detachment with the default guard or a documented intentional bypass plus non-terminal negative guard coverage.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `agents/langchain-deepagents-code/start.sh` prints `Setting up NemoClaw Deep Agents Code runtime...`; `src/lib/sandbox/create-stream.ts` defaults VM patterns to `/Setting up NemoClaw/`; `src/lib/onboard.ts:3067` passes `[]` for terminal agents.

PRA-2 Resolve/justify — Resolve the terminal-agent VM startup-output guard bypass against the new setup marker

  • Location: src/lib/onboard.ts:3067
  • Category: architecture
  • Problem: This PR adds `Setting up NemoClaw Deep Agents Code runtime...` to the DeepAgents entrypoint, which matches the existing VM create-stream default `/Setting up NemoClaw/` marker contract. The same PR also passes `readyCheckOutputPatterns: []` for every terminal agent, disabling that marker guard entirely for terminal VM creates. Those two fixes point at different sources of truth: either the entrypoint marker is the source fix, or terminal-agent Ready state is intended to be sufficient without startup-output evidence.
  • Impact: NemoClaw can proceed from a terminal VM sandbox solely because OpenShell reports Ready, with no streamed evidence that the expected startup command ran. That may mask future terminal-agent startup regressions and weakens sandbox lifecycle diagnostics around a security-sensitive trusted-state transition.
  • Recommended action: Prefer one source of truth in this PR. If the new DeepAgents setup marker is expected to reach the create stream, remove the terminal-agent `readyCheckOutputPatterns: []` override and add a test that proves the default VM marker guard detaches after that marker. If the bypass is still required because terminal entrypoint stdout is not observable through OpenShell VM create streams, document that boundary in code and constrain the exception as narrowly as possible.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `agents/langchain-deepagents-code/start.sh` for the new `Setting up NemoClaw Deep Agents Code runtime...` line, `src/lib/sandbox/create-stream.ts` for `VM_READY_DETACH_OUTPUT_PATTERNS = [/Setting up NemoClaw/]`, and `src/lib/onboard.ts:3067` for the terminal-agent empty-pattern override.
  • Missing regression test: Add `terminal VM create stream detaches after DeepAgents setup marker with default readyCheckOutputPatterns`: use `OPENSHELL_DRIVERS=vm`, a fake create child that emits `Created sandbox:` and `Setting up NemoClaw Deep Agents Code runtime...`, `readyCheck()` returning true, and assert `forcedReady`/registration occurs without passing an empty pattern list. If keeping the bypass, add `terminal VM Ready without setup marker intentionally bypasses startup-output wait` and assert the wait message is absent.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `agents/langchain-deepagents-code/start.sh` for the new `Setting up NemoClaw Deep Agents Code runtime...` line, `src/lib/sandbox/create-stream.ts` for `VM_READY_DETACH_OUTPUT_PATTERNS = [/Setting up NemoClaw/]`, and `src/lib/onboard.ts:3067` for the terminal-agent empty-pattern override.
  • Evidence: `start.sh` now prints a line matching `/Setting up NemoClaw/`, while `streamSandboxCreate` already uses that regex as the VM default when `readyCheckOutputPatterns` is `undefined`; `onboard.ts` instead passes `[]` for all terminal agents.

PRA-3 Resolve/justify — Cover non-terminal VM create-stream guard preservation

  • Location: src/lib/onboard.ts:3067
  • Category: tests
  • Problem: The changed terminal-agent test exercises a VM create stream where OpenShell reports Ready before any setup marker, but there is still no nearby negative regression proving gateway/non-terminal VM creates continue to wait for the default `Setting up NemoClaw` marker before forced-ready detachment.
  • Impact: A later refactor could accidentally broaden the terminal empty-pattern override to gateway agents, causing NemoClaw to register or continue from a VM sandbox based only on `openshell sandbox list` Ready state. That weakens the startup-output guard used for gateway-style create streams and reduces lifecycle integrity and diagnostics.
  • Recommended action: Add a focused regression around `streamSandboxCreate` or the onboard create path for a VM-driver, non-terminal/gateway agent. Have `readyCheck()` return true before any `Setting up NemoClaw` output and assert the create stream records the wait message and does not force-detach until the marker appears or the child exits.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/sandbox/create-stream.ts` around `getReadyCheckOutputPatterns`; then search tests for `Sandbox reported Ready; waiting for startup command output`, `readyCheckOutputPatterns`, or a non-terminal VM Ready-before-marker case.
  • Missing regression test: Add `non-terminal VM create waits for startup-output marker before forced-ready detach`: set `OPENSHELL_DRIVERS=vm`, use a non-terminal agent or direct `streamSandboxCreate` call, return Ready early, emit no marker initially, and assert no forced-ready result until `Setting up NemoClaw` is emitted or the child closes.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/sandbox/create-stream.ts` around `getReadyCheckOutputPatterns`; then search tests for `Sandbox reported Ready; waiting for startup command output`, `readyCheckOutputPatterns`, or a non-terminal VM Ready-before-marker case.
  • Evidence: `streamSandboxCreate` defaults VM creates to `[/Setting up NemoClaw/]` when `readyCheckOutputPatterns` is `undefined`; this PR passes `[]` only for terminal agents, but changed tests model only the terminal DeepAgents path.

PRA-4 Resolve/justify — Acceptance evidence remains simulated for the macOS VM and in-sandbox diagnostic clauses

  • Location: test/onboard-terminal-dashboard.test.ts:141
  • Category: acceptance
  • Problem: The linked issue includes live macOS/OpenShell VM, `openshell sandbox get`, in-sandbox `dcode --version`, and OpenShell log diagnostic clauses. The changed tests simulate the VM driver and Ready state, which is useful bug-path coverage, but they do not provide automated evidence for the live in-sandbox policy/smoke/log parts of those clauses.
  • Impact: The core hang regression is covered by a controlled fake, but reviewers cannot tell from this diff whether the specific macOS VM/in-sandbox acceptance observations are covered elsewhere or intentionally left to manual validation.
  • Recommended action: Either identify existing automated coverage that covers the DeepAgents terminal smoke/policy checks for the live acceptance clauses, or add a narrowly scoped test note/fixture assertion for the closest local behavior. Do not replace the focused unit-style regression with a large new E2E framework.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/onboard-terminal-dashboard.test.ts` for the simulated `OPENSHELL_DRIVERS=vm` create path, and `test/onboard-terminal.test.ts` plus `test/langchain-deepagents-code-image.test.ts` for existing DeepAgents smoke/policy contract coverage.
  • Missing regression test: If existing coverage is not considered sufficient, add `DeepAgents terminal onboard verifies smoke command after VM Ready registration` using the existing terminal-smoke fixtures, or document which existing `onboard-terminal` and image-contract tests cover `/sandbox/.deepagents/config.toml` and `dcode --version` behavior.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/onboard-terminal-dashboard.test.ts` for the simulated `OPENSHELL_DRIVERS=vm` create path, and `test/onboard-terminal.test.ts` plus `test/langchain-deepagents-code-image.test.ts` for existing DeepAgents smoke/policy contract coverage.
  • Evidence: The create scenario sets `OPENSHELL_DRIVERS: scenario === "create" ? "vm" : "docker"` and fakes `sandbox list` as Ready, but no changed test runs `openshell sandbox get`, validates `/sandbox/.deepagents/config.toml`, executes `dcode --version` in a live sandbox, or inspects OpenShell logs.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

Signed-off-by: San Dang <sdang@nvidia.com>
@jyaunches jyaunches added v0.0.69 and removed v0.0.68 labels Jun 25, 2026
@wscurran wscurran removed the bug label Jun 25, 2026
@cv
cv merged commit 9ad2256 into main Jun 26, 2026
41 checks passed
@cv
cv deleted the fix/deepagents-terminal-ready-detach branch June 26, 2026 02:00
cv added a commit that referenced this pull request Jun 26, 2026
## Summary
Derive supported messaging agents and channels from channel manifests
instead of agent manifests, so `supportedAgents` is the source of truth.
This also removes artifact-only DeepAgents messaging wiring and rejects
unsupported channel/agent pairs before policy, provider, credential,
registry, or rebuild mutation.

## Related Issue
Related PRs: #5743, #5673. Stacked on #5768, Fixes #5729

## Acceptance Note
DeepAgents messaging is intentionally fail-closed in this PR. NemoClaw
does not claim Discord, Telegram, Slack, or other channel delivery for
LangChain Deep Agents Code until a real DeepAgents messaging bridge
exists. The fix for #5729 is to reject unsupported DeepAgents channel
setup before policy, credential, registry, rebuild, or Dockerfile
mutation, instead of injecting a stale messaging plan into a sandbox
that cannot consume inbound channel traffic.

## Changes
- Remove `messaging_platforms` parsing from agent manifests and derive
available channels from channel manifest `supportedAgents`.
- Remove DeepAgents messaging build/runtime artifacts until a real
messaging bridge exists.
- Gate `channels add`, `channels list`, onboard filtering, and rebuild
staging from manifest-derived channel support.
- Update regression tests for DeepAgents rejection, stale messaging
cleanup, rebuild skipping, and manifest helper behavior.
- Update platform-support docs to reference channel manifest
`supportedAgents`.

## 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)

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: San Dang <sdang@nvidia.com>

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

* **Bug Fixes**
* Messaging channel availability is now derived from each channel
manifest’s `supportedAgents`, improving enable/disable, rebuild
planning, and channel status matching.
* DeepAgents startup and rebuild no longer include unsupported messaging
runtime wiring, env handling, or messaging artifacts.
* Channel add/cleanup gates now reject unsupported agent/channel pairs
earlier with clearer “channel-supported agents” guidance.
* **Documentation**
* Updated platform support and DeepAgents eligibility guidance to make
`supportedAgents` the single source of truth for messaging support.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Terminal-runtime sandboxes now continue onboarding once OpenShell
reports Ready, even when the VM create stream does not emit
gateway-style startup output. This fixes DeepAgents Code onboarding
hanging on macOS VM-driver hosts after the sandbox is already Ready.

## Bugs
LangChain hang at sandbox build MacOS
```
  Sandbox reported Ready; waiting for startup command output before detaching.
  Still waiting for sandbox to become ready... (30s elapsed)
  Still waiting for sandbox to become ready... (45s elapsed)
  Still waiting for sandbox to become ready... (60s elapsed)
  Still waiting for sandbox to become ready... (75s elapsed)
  Still waiting for sandbox to become ready... (90s elapsed)
  Still waiting for sandbox to become ready... (105s elapsed)
  Still waiting for sandbox to become ready... (120s elapsed)
  Still waiting for sandbox to become ready... (135s elapsed)
  Still waiting for sandbox to become ready... (150s elapsed)
  Still waiting for sandbox to become ready... (165s elapsed)
  Still waiting for sandbox to become ready... (180s elapsed)
  Still waiting for sandbox to become ready... (195s elapsed)
  Still waiting for sandbox to become ready... (210s elapsed)
```
## Related Issue
Fixes NVIDIA#5766

## Changes
- Disable the create-stream startup-output guard for terminal agents
while preserving the OpenShell Ready check.
- Keep the DeepAgents Code entrypoint alive with a portable `tail -f
/dev/null` idle path and emit a setup marker.
- Extend terminal-agent onboarding and DeepAgents entrypoint tests to
cover VM-driver Ready detachment without startup output.

## Type of Change

- [x] 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
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] 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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: San Dang <sdang@nvidia.com>

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

* **New Features**
* Improved startup behavior for idle runtimes: when launched without a
command, it now prints a clearer runtime setup message before idling.
* **Bug Fixes**
* Enhanced sandbox onboarding for terminal-based flows by adjusting how
“ready” is detected during sandbox creation.
* Updated and expanded test coverage to validate the new startup
message, idle behavior, and readiness parsing.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Derive supported messaging agents and channels from channel manifests
instead of agent manifests, so `supportedAgents` is the source of truth.
This also removes artifact-only DeepAgents messaging wiring and rejects
unsupported channel/agent pairs before policy, provider, credential,
registry, or rebuild mutation.

## Related Issue
Related PRs: NVIDIA#5743, NVIDIA#5673. Stacked on NVIDIA#5768, Fixes NVIDIA#5729

## Acceptance Note
DeepAgents messaging is intentionally fail-closed in this PR. NemoClaw
does not claim Discord, Telegram, Slack, or other channel delivery for
LangChain Deep Agents Code until a real DeepAgents messaging bridge
exists. The fix for NVIDIA#5729 is to reject unsupported DeepAgents channel
setup before policy, credential, registry, rebuild, or Dockerfile
mutation, instead of injecting a stale messaging plan into a sandbox
that cannot consume inbound channel traffic.

## Changes
- Remove `messaging_platforms` parsing from agent manifests and derive
available channels from channel manifest `supportedAgents`.
- Remove DeepAgents messaging build/runtime artifacts until a real
messaging bridge exists.
- Gate `channels add`, `channels list`, onboard filtering, and rebuild
staging from manifest-derived channel support.
- Update regression tests for DeepAgents rejection, stale messaging
cleanup, rebuild skipping, and manifest helper behavior.
- Update platform-support docs to reference channel manifest
`supportedAgents`.

## 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)

## Verification
- [x] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [x] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [ ] `npm run docs` builds without warnings (doc changes only)
- [ ] 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)

---
<!-- DCO sign-off is required in this PR description, and every commit
must appear as Verified in GitHub. Run: git config user.name && git
config user.email -->
Signed-off-by: San Dang <sdang@nvidia.com>

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

* **Bug Fixes**
* Messaging channel availability is now derived from each channel
manifest’s `supportedAgents`, improving enable/disable, rebuild
planning, and channel status matching.
* DeepAgents startup and rebuild no longer include unsupported messaging
runtime wiring, env handling, or messaging artifacts.
* Channel add/cleanup gates now reject unsupported agent/channel pairs
earlier with clearer “channel-supported agents” guidance.
* **Documentation**
* Updated platform support and DeepAgents eligibility guidance to make
`supportedAgents` the single source of truth for messaging support.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration: dcode LangChain Deep Code integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DeepAgents onboard hangs waiting for startup output on macOS VM driver

4 participants