Skip to content

test(e2e): validate dcode TUI startup - #5839

Merged
cv merged 11 commits into
mainfrom
codex/add-dcode-tui-startup-check
Jun 26, 2026
Merged

cv merged 11 commits into
mainfrom
codex/add-dcode-tui-startup-check

Conversation

@cv

@cv cv commented Jun 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds a bounded PTY/expect live check that proves the Deep Agents Code dcode TUI reaches an interactive startup state inside a managed sandbox. The check captures sanitized failure artifacts, scans them for secret-shaped values, and registers the validation with the Deep Agents cloud-experimental scenario.

Related Issue

Fixes #5620

Changes

  • Added 09-deepagents-code-tui-startup.sh to launch dcode under openshell sandbox exec --tty, detect a prompt-ready startup signature, send Ctrl-C, and assert a clean exit.
  • Registered the new check in DEEPAGENTS_CLOUD_EXPERIMENTAL_CHECKS for cloud-langchain-deepagents-code.
  • Extended contract/support tests to pin the new check, assert it remains executable, and cover timeout and secret-pattern helper behavior.

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: validation-only change; docs-impact review found no CLI/config/user-facing behavior change and the existing Deep Agents quickstart already documents running dcode.
  • 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:

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)
  • 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)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features
    • Added a new live end-to-end check for interactive Deep Agents Code TUI startup.
    • Updated onboarding expected checks to include the new TUI startup script.
  • Bug Fixes
    • Strengthened TUI preflight and validation for DEEPAGENTS_TUI_TIMEOUT.
    • Improved readiness/exit handling and hardened sanitization/redaction to prevent secret leakage, including improved failure behavior.
  • Tests
    • Expanded cloud experimental parity coverage to assert the check list matches exactly and every referenced script is executable.
    • Extended TUI runtime tests for readiness detection, marker parsing, cleanup of raw artifacts, and robust secret scanning/redaction.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: dcode LangChain Deep Code integration behavior labels Jun 26, 2026
@cv cv self-assigned this Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

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

Adds a Deep Agents Code TUI startup check script, registers it in the cloud experimental list, and expands the e2e suites to verify script registration, executable permissions, startup markers, timeout validation, secret handling, and artifact cleanup.

Changes

Deep Agents Code TUI startup checks

Layer / File(s) Summary
Check registration and evidence
test/e2e-scenario/live/cloud-experimental-check-list.ts, test/e2e-scenario/live/cloud-experimental-checks.ts
Adds the new TUI check script to the cloud experimental list and routes evidence JSON through the typed builder with terminal connect metadata.
TUI startup script
test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh
Adds the dcode interactive startup check, PTY capture and sanitization, timeout validation, secret detection/redaction, and exit-status validation.
Parity and contract tests
test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
Asserts the check list matches the expected paths, the scripts are executable, and the evidence payload includes the new interactive hint.
Image and runtime assertions
test/langchain-deepagents-code-image.test.ts
Updates the Deep Agents Code image tests to cover the new TUI artifact, fixture rewriting, startup-script assertions, timeout validation, secret scanning, and cleanup behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5823: Also updates cloud experimental check discovery and onboarding-related coverage with the same TUI startup path.

Suggested reviewers

  • prekshivyas
  • sandl99

Poem

🐰 I hopped through the sandbox with ears held high,
dcode said hello as the PTYs flew by.
Secrets got masked and the markers stayed bright,
Now the startup check purrs cleanly at night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: validating Deep Agents Code TUI startup.
Linked Issues check ✅ Passed The PR implements the interactive PTY/expect startup check, sanitized failure artifacts, clean exit verification, and dcode-handoff evidence required by #5620.
Out of Scope Changes check ✅ Passed The added tests, evidence helper, and live check all support the TUI startup validation and secret-safe artifact requirements.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/add-dcode-tui-startup-check

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

@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/e2e/e2e-cloud-experimental/checks/09-deepagents-code-tui-startup.sh`:
- Around line 140-159: The Deep Agents Code TUI startup check is leaving
unsanitized capture artifacts on disk, which can leak sensitive PTY output.
Update the shell logic around run_tui_expect, strip_terminal_control_sequences,
and the capture files to avoid persisting ${PREFIX}.raw.log,
${PREFIX}.expect.log, and ${PREFIX}.combined.log after use, or delete them as
soon as the sanitized output is produced. Keep only the sanitized
${PREFIX}.sanitized.log for scanning and ensure cleanup still runs on both
success and failure paths.
- Around line 135-137: The Deep Agents sandbox probe in the startup check is
treating every non-zero return from sandbox_exec as “not a Deep Agents Code
sandbox,” which hides real exec/auth/missing-sandbox failures. Update the check
around sandbox_exec so it first distinguishes a successful probe from execution
errors, and only print the SKIP message when the probe explicitly shows the
sandbox lacks the Deep Agents markers. Use the existing sandbox_exec and info
flow in this startup script to preserve a true skip only for “not a Deep Agents
sandbox,” while surfacing probe failures as test failures or distinct errors.
🪄 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: db361da6-859a-4510-85ba-6b51888af44e

📥 Commits

Reviewing files that changed from the base of the PR and between 152e854 and bbdd981.

📒 Files selected for processing (4)
  • test/e2e-scenario/live/cloud-experimental-check-list.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/09-deepagents-code-tui-startup.sh
  • test/langchain-deepagents-code-image.test.ts

Comment thread test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh Outdated
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv added the v0.0.69 label Jun 26, 2026
@github-code-quality

github-code-quality Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/add-dcode-tui-... 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 codex/add-dcode-tui-... f152bd0 +/-
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 codex/add-dcode-tui-... 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 codex/add-dcode-tui-... f152bd0 +/-
src/lib/state/o...oard-session.ts — 91% —
src/lib/actions...dbox/rebuild.ts — 73% —
src/lib/sandbox/config.ts — 72% —
src/lib/onboard/preflight.ts — 62% —
src/lib/shields/index.ts — 62% —
src/lib/actions...licy-channel.ts — 60% —
src/lib/state/sandbox.ts — 56% —
src/lib/policy/index.ts — 48% —
src/lib/onboard...er-gpu-patch.ts — 47% —
src/lib/onboard.ts — 19% —

Updated June 26, 2026 19:10 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E: None

Dispatch hint: scenarios=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • ubuntu-repo-cloud-langchain-deepagents-code (high): This is the existing live Vitest scenario that performs cloud LangChain Deep Agents Code onboarding and runs cloudExperimentalChecksForOnboarding("cloud-langchain-deepagents-code"). The PR adds a required TUI startup check to that list, so the scenario must run to validate the new real sandbox PTY/TUI flow and artifact redaction behavior.

Optional E2E

  • None.

New E2E recommendations

  • None.

Dispatch hint

  • Workflow: e2e-vitest-scenarios.yaml
  • jobs input: scenarios=ubuntu-repo-cloud-langchain-deepagents-code

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: ubuntu-repo-cloud-langchain-deepagents-code
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

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

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • ubuntu-repo-cloud-langchain-deepagents-code: The PR changes the live cloud-experimental check list and runner helpers for the LangChain Deep Agents Code onboarding, adding a new TUI startup shell check that is executed by the live Vitest scenario. The smallest live-supported typed scenario that exercises this surface is the Ubuntu repo LangChain Deep Agents Code scenario.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-langchain-deepagents-code

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • test/e2e-scenario/live/cloud-experimental-check-list.ts
  • test/e2e-scenario/live/cloud-experimental-checks.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh

Comment thread test/langchain-deepagents-code-image.test.ts Fixed

@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.

🧹 Nitpick comments (1)
test/langchain-deepagents-code-image.test.ts (1)

120-125: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Assert the start.sh template rewrites actually happened.

Lines 120-125 use two exact-string .replace(...) calls, but both silently no-op if start.sh changes formatting. In that case this fixture can start writing back to /tmp/nemoclaw-proxy-env.sh again, which breaks the hermetic temp-dir isolation these tests are trying to enforce.

Suggested guard
-  const fixture = readAgentFile("start.sh")
-    .replace("local target=/tmp/nemoclaw-proxy-env.sh", `local target="${envFile}"`)
-    .replace(
-      'tmp="$(mktemp /tmp/nemoclaw-proxy-env.XXXXXX)"',
-      `tmp="$(mktemp "${tempDir}/nemoclaw-proxy-env.XXXXXX")"`,
-    );
+  const original = readAgentFile("start.sh");
+  expect(original).toContain("local target=/tmp/nemoclaw-proxy-env.sh");
+  expect(original).toContain('tmp="$(mktemp /tmp/nemoclaw-proxy-env.XXXXXX)"');
+  const fixture = original
+    .replace("local target=/tmp/nemoclaw-proxy-env.sh", `local target="${envFile}"`)
+    .replace(
+      'tmp="$(mktemp /tmp/nemoclaw-proxy-env.XXXXXX)"',
+      `tmp="$(mktemp "${tempDir}/nemoclaw-proxy-env.XXXXXX")"`,
+    );

Based on PR objectives, the check should remain hermetic enough to avoid flakes.

🤖 Prompt for 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.

In `@test/langchain-deepagents-code-image.test.ts` around lines 120 - 125, The
`start.sh` fixture rewrite in the test setup is currently using exact-string
`replace` calls that can silently do nothing if the template changes, which
would undermine the hermetic temp-dir isolation. Update the test around the
`readAgentFile("start.sh")` fixture preparation to verify both substitutions
actually occurred after the rewrites, and fail fast if either the
`/tmp/nemoclaw-proxy-env.sh` target or the `mktemp` path was not rewritten as
expected.
🤖 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.

Nitpick comments:
In `@test/langchain-deepagents-code-image.test.ts`:
- Around line 120-125: The `start.sh` fixture rewrite in the test setup is
currently using exact-string `replace` calls that can silently do nothing if the
template changes, which would undermine the hermetic temp-dir isolation. Update
the test around the `readAgentFile("start.sh")` fixture preparation to verify
both substitutions actually occurred after the rewrites, and fail fast if either
the `/tmp/nemoclaw-proxy-env.sh` target or the `mktemp` path was not rewritten
as expected.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 095dd24d-97dc-4bf6-b00f-9edb1e8a55cf

📥 Commits

Reviewing files that changed from the base of the PR and between bbdd981 and de29c37.

📒 Files selected for processing (4)
  • test/e2e-scenario/live/cloud-experimental-check-list.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh
  • test/langchain-deepagents-code-image.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/e2e-scenario/live/cloud-experimental-check-list.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts

@github-actions

github-actions Bot commented Jun 26, 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: Caller-provided TUI capture directories can clobber fixed-name files.
Open items: 0 required · 1 warning · 0 suggestions · 3 test follow-ups
Since last review: 1 prior item resolved · 1 still applies · 0 new items found

Action checklist

  • PRA-1 Resolve or justify: Caller-provided TUI capture directories can clobber fixed-name files in test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh:112
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Acceptance clause
  • PRA-T3 Add or justify test follow-up: Acceptance clause

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify security test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh:112 Allocate capture files with `mktemp` inside a trusted private directory, or create a fresh private subdirectory under `DEEPAGENTS_TUI_CAPTURE_DIR` with restrictive permissions and refuse pre-existing or symlinked artifact paths before any redirect or `mv`. Keep the existing redaction and sensitive-capture cleanup behavior.
Review findings by urgency: 0 required fixes, 1 item 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 — Caller-provided TUI capture directories can clobber fixed-name files

  • Location: test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh:112
  • Category: security
  • Problem: `make_capture_dir` accepts `DEEPAGENTS_TUI_CAPTURE_DIR` directly and `main` then writes fixed artifact names under it using truncating redirects and `mv`. A reused or attacker-prepared directory can contain symlinks or pre-existing paths for `10-deepagents-code-tui-startup.raw.log`, `.expect.log`, `.combined.log`, `.sanitized.log`, or the redaction temp path.
  • Impact: A local or CI caller that controls the capture directory can cause destructive writes outside the intended artifact set, and raw or sanitized TUI capture content could be redirected to an unexpected host file before cleanup/redaction completes.
  • Recommended action: Allocate capture files with `mktemp` inside a trusted private directory, or create a fresh private subdirectory under `DEEPAGENTS_TUI_CAPTURE_DIR` with restrictive permissions and refuse pre-existing or symlinked artifact paths before any redirect or `mv`. Keep the existing redaction and sensitive-capture cleanup behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `make_capture_dir` and the artifact assignments/writes around lines 112 and 229-247; confirm no fixed path is opened with `: >`, `cat >`, `strip_terminal_control_sequences >`, or `mv` unless it was safely created as a new non-symlink file.
  • Missing regression test: Add a Vitest helper test named `DEEPAGENTS_TUI_CAPTURE_DIR rejects or isolates symlinked fixed artifact names without modifying sentinel files` that creates a temp capture dir with `10-deepagents-code-tui-startup.raw.log` as a symlink to a sentinel file, invokes the helper path, and asserts the script refuses the directory or uses fresh `mktemp` paths without changing the sentinel.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `make_capture_dir` and the artifact assignments/writes around lines 112 and 229-247; confirm no fixed path is opened with `: >`, `cat >`, `strip_terminal_control_sequences >`, or `mv` unless it was safely created as a new non-symlink file.
  • Evidence: `make_capture_dir` returns `$DEEPAGENTS_TUI_CAPTURE_DIR` after `mkdir -p`; `main` then sets fixed artifact paths under that directory, truncates raw and expect captures with `: >`, writes combined and sanitized captures with redirects, and `redact_secrets_in_file` replaces the sanitized path with `mv`.

💡 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.
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 — DEEPAGENTS_TUI_CAPTURE_DIR rejects or isolates symlinked fixed artifact names without modifying sentinel files. The PR adds a real PTY/OpenShell/dcode live check plus focused helper tests for most local shell behavior. The remaining confidence gap is not model-output behavior but host artifact safety for the caller-controlled capture directory.
  • PRA-T2 Acceptance clause — The test captures sanitized logs/artifacts for failures. — add test evidence or identify existing coverage. The script combines raw and expect logs, strips terminal controls, scans/redacts secret-shaped values, deletes raw/expect/combined captures, and retains the sanitized capture; however the caller-provided capture directory still uses fixed filenames that can be symlinked or pre-existing, as covered by the security finding.
  • PRA-T3 Acceptance clause — 3. Strip ANSI/OSC and write a sanitized capture artifact (reuse the redact/`perl` pattern from the `[DGX Spark][CLI&UX] openclaw tui shows indefinite spinner with no error when inference endpoint is unreachable #4434` script); assert no provider secrets (`nvapi-`/`sk-`) in the capture. — add test evidence or identify existing coverage. `strip_terminal_control_sequences`, `contains_secret`, and `redact_secrets_in_file` implement the sanitize/redact flow and tests cover canonical secret families; the remaining caveat is fixed-name capture paths in a caller-provided directory, covered by the security finding.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Caller-provided TUI capture directories can clobber fixed-name files

  • Location: test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh:112
  • Category: security
  • Problem: `make_capture_dir` accepts `DEEPAGENTS_TUI_CAPTURE_DIR` directly and `main` then writes fixed artifact names under it using truncating redirects and `mv`. A reused or attacker-prepared directory can contain symlinks or pre-existing paths for `10-deepagents-code-tui-startup.raw.log`, `.expect.log`, `.combined.log`, `.sanitized.log`, or the redaction temp path.
  • Impact: A local or CI caller that controls the capture directory can cause destructive writes outside the intended artifact set, and raw or sanitized TUI capture content could be redirected to an unexpected host file before cleanup/redaction completes.
  • Recommended action: Allocate capture files with `mktemp` inside a trusted private directory, or create a fresh private subdirectory under `DEEPAGENTS_TUI_CAPTURE_DIR` with restrictive permissions and refuse pre-existing or symlinked artifact paths before any redirect or `mv`. Keep the existing redaction and sensitive-capture cleanup behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `make_capture_dir` and the artifact assignments/writes around lines 112 and 229-247; confirm no fixed path is opened with `: >`, `cat >`, `strip_terminal_control_sequences >`, or `mv` unless it was safely created as a new non-symlink file.
  • Missing regression test: Add a Vitest helper test named `DEEPAGENTS_TUI_CAPTURE_DIR rejects or isolates symlinked fixed artifact names without modifying sentinel files` that creates a temp capture dir with `10-deepagents-code-tui-startup.raw.log` as a symlink to a sentinel file, invokes the helper path, and asserts the script refuses the directory or uses fresh `mktemp` paths without changing the sentinel.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `make_capture_dir` and the artifact assignments/writes around lines 112 and 229-247; confirm no fixed path is opened with `: >`, `cat >`, `strip_terminal_control_sequences >`, or `mv` unless it was safely created as a new non-symlink file.
  • Evidence: `make_capture_dir` returns `$DEEPAGENTS_TUI_CAPTURE_DIR` after `mkdir -p`; `main` then sets fixed artifact paths under that directory, truncates raw and expect captures with `: >`, writes combined and sanitized captures with redirects, and `redact_secrets_in_file` replaces the sanitized path with `mv`.

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.

Comment thread test/langchain-deepagents-code-image.test.ts Fixed

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh (1)

192-196: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Fail fast when the secret scanner dependency is unavailable.

contains_secret and redact_secrets_in_file depend on perl, but main only checks for expect. If perl is missing, if contains_secret <"$plain_capture_file" treats exit 127 as “no secret”, leaving a potentially secret-bearing sanitized artifact behind.

Suggested fix
   if ! command -v expect >/dev/null 2>&1; then
     fail_test "expect is required for the Deep Agents Code TUI startup check"
     printf '%s\n' "${PREFIX}: $PASSED passed, $FAILED failed"
     exit 1
   fi
+
+  if ! command -v perl >/dev/null 2>&1; then
+    fail_test "perl is required for Deep Agents Code TUI secret scanning"
+    printf '%s\n' "${PREFIX}: $PASSED passed, $FAILED failed"
+    exit 1
+  fi

Also applies to: 238-245

🤖 Prompt for 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.

In `@test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh`
around lines 192 - 196, The startup check only verifies expect, but
contains_secret and redact_secrets_in_file also require perl, so missing perl
can cause a false “no secret” result and leave unsanitized artifacts behind.
Update main in 10-deepagents-code-tui-startup.sh to fail fast when perl is
unavailable, using the same dependency-check pattern as the existing expect
guard, and make sure the secret-scanning flow that calls contains_secret and
redact_secrets_in_file cannot proceed without perl.
🤖 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/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh`:
- Around line 21-23: The readiness heuristic in TUI_READY_PATTERN is too broad
because it treats standalone “press enter” as a ready signal, which can match
banner text before a usable prompt appears. Tighten the pattern in the TUI
startup check so it only matches actual task/prompt wording and does not accept
“press enter” by itself; keep the heuristic localized to the existing startup
script and preserve the NEMOCLAW_TUI_READY emission flow.

---

Outside diff comments:
In `@test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh`:
- Around line 192-196: The startup check only verifies expect, but
contains_secret and redact_secrets_in_file also require perl, so missing perl
can cause a false “no secret” result and leave unsanitized artifacts behind.
Update main in 10-deepagents-code-tui-startup.sh to fail fast when perl is
unavailable, using the same dependency-check pattern as the existing expect
guard, and make sure the secret-scanning flow that calls contains_secret and
redact_secrets_in_file cannot proceed without perl.
🪄 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: 87bc5871-2dd0-4407-8c83-c4f0e53104f2

📥 Commits

Reviewing files that changed from the base of the PR and between c734e86 and c29bdef.

📒 Files selected for processing (4)
  • test/e2e-scenario/live/cloud-experimental-checks.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh
  • test/langchain-deepagents-code-image.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts

Comment thread test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh Outdated

@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 `@test/langchain-deepagents-code-image.test.ts`:
- Around line 651-665: Split the Slack token fixtures in the secret redaction
test so the source no longer contains raw `xoxa-...` and `xapp-...` literals
that trigger scanner hits. Update the `secretSamples` entries in
`test/langchain-deepagents-code-image.test.ts` to construct those sample strings
from smaller fragments at runtime, while keeping the same values passed through
`redactsSecret` and the rest of the test behavior unchanged.
🪄 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: 1f8342f0-e161-4a26-a22c-483c49762d17

📥 Commits

Reviewing files that changed from the base of the PR and between c29bdef and 24d1d9c.

📒 Files selected for processing (4)
  • test/e2e-scenario/live/cloud-experimental-checks.ts
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh
  • test/langchain-deepagents-code-image.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/e2e-scenario/support-tests/platform-parity-cloud-experimental.test.ts
  • test/e2e/e2e-cloud-experimental/checks/10-deepagents-code-tui-startup.sh

Comment on lines +651 to +665
const redactsSecret = (token: string) =>
runTuiStartupCheckHelper('printf "%s" "$TOKEN" | redact_secrets', { TOKEN: token });
const secretSamples: Array<{ name: string; sample: string; rawSecret?: string }> = [
{ name: "nvapi", sample: "nvapi-abcdefghijklmnop" },
{ name: "nvcf", sample: "nvcf-abcdefghijklmnopq" },
{ name: "ghp", sample: "ghp_abcdefghijklmnopqr" },
{ name: "github_pat", sample: "github_pat_abcdefghijklmnopqrstuvwxyz0123" },
{ name: "sk_proj", sample: "sk-proj-abcdefghij" },
{ name: "sk_ant", sample: "sk-ant-abcdefghijk" },
{ name: "sk", sample: "sk-abcdefghijklmnopqrstuvwx" },
{ name: "xoxb", sample: "xoxb-1234567890" },
{ name: "xoxp", sample: "xoxp-1234567890" },
{ name: "xoxa", sample: "xoxa-1234567890" },
{ name: "xoxs", sample: "xoxs-1234567890" },
{ name: "xapp", sample: "xapp-1-A1B2C3-12345-abcde" },

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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Split Slack-shaped fixtures to avoid secret-scanner hits.

Betterleaks flags the raw xoxa-... and xapp-... literals. Keep the same runtime samples, but build them from fragments so the committed test source does not look like live Slack tokens.

Proposed fixture adjustment
     const redactsSecret = (token: string) =>
       runTuiStartupCheckHelper('printf "%s" "$TOKEN" | redact_secrets', { TOKEN: token });
+    const secretFixture = (...parts: string[]) => parts.join("");
     const secretSamples: Array<{ name: string; sample: string; rawSecret?: string }> = [
@@
-      { name: "xoxb", sample: "xoxb-1234567890" },
-      { name: "xoxp", sample: "xoxp-1234567890" },
-      { name: "xoxa", sample: "xoxa-1234567890" },
-      { name: "xoxs", sample: "xoxs-1234567890" },
-      { name: "xapp", sample: "xapp-1-A1B2C3-12345-abcde" },
+      { name: "xoxb", sample: secretFixture("xox", "b", "-", "1234567890") },
+      { name: "xoxp", sample: secretFixture("xox", "p", "-", "1234567890") },
+      { name: "xoxa", sample: secretFixture("xox", "a", "-", "1234567890") },
+      { name: "xoxs", sample: secretFixture("xox", "s", "-", "1234567890") },
+      { name: "xapp", sample: secretFixture("x", "app", "-", "1", "-", "A1B2C3", "-", "12345", "-", "abcde") },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const redactsSecret = (token: string) =>
runTuiStartupCheckHelper('printf "%s" "$TOKEN" | redact_secrets', { TOKEN: token });
const secretSamples: Array<{ name: string; sample: string; rawSecret?: string }> = [
{ name: "nvapi", sample: "nvapi-abcdefghijklmnop" },
{ name: "nvcf", sample: "nvcf-abcdefghijklmnopq" },
{ name: "ghp", sample: "ghp_abcdefghijklmnopqr" },
{ name: "github_pat", sample: "github_pat_abcdefghijklmnopqrstuvwxyz0123" },
{ name: "sk_proj", sample: "sk-proj-abcdefghij" },
{ name: "sk_ant", sample: "sk-ant-abcdefghijk" },
{ name: "sk", sample: "sk-abcdefghijklmnopqrstuvwx" },
{ name: "xoxb", sample: "xoxb-1234567890" },
{ name: "xoxp", sample: "xoxp-1234567890" },
{ name: "xoxa", sample: "xoxa-1234567890" },
{ name: "xoxs", sample: "xoxs-1234567890" },
{ name: "xapp", sample: "xapp-1-A1B2C3-12345-abcde" },
const redactsSecret = (token: string) =>
runTuiStartupCheckHelper('printf "%s" "$TOKEN" | redact_secrets', { TOKEN: token });
const secretFixture = (...parts: string[]) => parts.join("");
const secretSamples: Array<{ name: string; sample: string; rawSecret?: string }> = [
{ name: "nvapi", sample: "nvapi-abcdefghijklmnop" },
{ name: "nvcf", sample: "nvcf-abcdefghijklmnopq" },
{ name: "ghp", sample: "ghp_abcdefghijklmnopqr" },
{ name: "github_pat", sample: "github_pat_abcdefghijklmnopqrstuvwxyz0123" },
{ name: "sk_proj", sample: "sk-proj-abcdefghij" },
{ name: "sk_ant", sample: "sk-ant-abcdefghijk" },
{ name: "sk", sample: "sk-abcdefghijklmnopqrstuvwx" },
{ name: "xoxb", sample: secretFixture("xox", "b", "-", "1234567890") },
{ name: "xoxp", sample: secretFixture("xox", "p", "-", "1234567890") },
{ name: "xoxa", sample: secretFixture("xox", "a", "-", "1234567890") },
{ name: "xoxs", sample: secretFixture("xox", "s", "-", "1234567890") },
{ name: "xapp", sample: secretFixture("x", "app", "-", "1", "-", "A1B2C3", "-", "12345", "-", "abcde") },
🧰 Tools
🪛 Betterleaks (1.5.0)

[high] 663-663: Identified a Slack Legacy Workspace token, potentially compromising access to workspace data and legacy features.

(slack-legacy-workspace-token)


[high] 665-665: Detected a Slack App-level token, risking unauthorized access to Slack applications and workspace data.

(slack-app-token)

🤖 Prompt for 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.

In `@test/langchain-deepagents-code-image.test.ts` around lines 651 - 665, Split
the Slack token fixtures in the secret redaction test so the source no longer
contains raw `xoxa-...` and `xapp-...` literals that trigger scanner hits.
Update the `secretSamples` entries in
`test/langchain-deepagents-code-image.test.ts` to construct those sample strings
from smaller fragments at runtime, while keeping the same values passed through
`redactsSecret` and the rest of the test behavior unchanged.

Source: Linters/SAST tools

Comment thread test/deepagents-code-tui-startup-check.test.ts Fixed
Comment thread test/deepagents-code-tui-startup-check.test.ts Fixed
@github-actions

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28252215297
Target ref: bd0bcbf68c766e5fef13c2c70bfde24e375f62f4
Workflow ref: main
Requested jobs: onboard-resume-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
onboard-resume-e2e ✅ success

@github-actions

github-actions Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Informational

Merge posture: Informational / low confidence
Primary next action: Resolve or justify PRA-1: PR review advisor unavailable.
Open items: 0 required · 1 warning · 0 suggestions · 0 test follow-ups
Top item: PR review advisor unavailable

Action checklist

  • PRA-1 Resolve or justify: PR review advisor unavailable

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify correctness — Re-run the PR Review Advisor or perform a manual review.
Review findings by urgency: 0 required fixes, 1 item 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 — PR review advisor unavailable

  • Location: not file-specific
  • Category: correctness
  • Problem: The automated advisor could not complete: Could not parse JSON from PR review advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/pr-review-advisor-nemotron-ultra/pr-review-advisor-retry-raw-output.txt
  • Impact: Automated review evidence is incomplete, so human review must cover the changed code manually.
  • Recommended action: Re-run the PR Review Advisor or perform a manual review.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the workflow logs and raw advisor artifact for the execution failure.
  • Missing regression test: No regression test recommendation is available because the advisor did not complete.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the workflow logs and raw advisor artifact for the execution failure.
  • Evidence: Could not parse JSON from PR review advisor output; see /home/runner/work/NemoClaw/NemoClaw/artifacts/pr-review-advisor-nemotron-ultra/pr-review-advisor-retry-raw-output.txt

💡 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.

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.

@cv
cv merged commit 9f48114 into main Jun 26, 2026
41 checks passed
@cv
cv deleted the codex/add-dcode-tui-startup-check branch June 26, 2026 19:30
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary
Adds a bounded PTY/expect live check that proves the Deep Agents Code
`dcode` TUI reaches an interactive startup state inside a managed
sandbox. The check captures sanitized failure artifacts, scans them for
secret-shaped values, and registers the validation with the Deep Agents
cloud-experimental scenario.

## Related Issue
Fixes NVIDIA#5620

## Changes
- Added `09-deepagents-code-tui-startup.sh` to launch `dcode` under
`openshell sandbox exec --tty`, detect a prompt-ready startup signature,
send Ctrl-C, and assert a clean exit.
- Registered the new check in `DEEPAGENTS_CLOUD_EXPERIMENTAL_CHECKS` for
`cloud-langchain-deepagents-code`.
- Extended contract/support tests to pin the new check, assert it
remains executable, and cover timeout and secret-pattern helper
behavior.

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

## Quality Gates
<!-- Check all that apply. For any "covered by existing tests", "not
applicable", or waiver entry, add a brief justification on the same line
or in the Changes section. -->
- [x] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [ ] Tests not applicable — justification:
- [ ] Docs updated for user-facing behavior changes
- [x] Docs not applicable — justification: validation-only change;
docs-impact review found no CLI/config/user-facing behavior change and
the existing Deep Agents quickstart already documents running `dcode`.
- [ ] 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:

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. Doc-only changes do not require npm test unless you ran it. -->
- [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] 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)
- [ ] 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: Carlos Villela <cvillela@nvidia.com>


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

* **New Features**
* Added a new live end-to-end check for interactive Deep Agents Code TUI
startup.
* Updated onboarding expected checks to include the new TUI startup
script.
* **Bug Fixes**
* Strengthened TUI preflight and validation for
`DEEPAGENTS_TUI_TIMEOUT`.
* Improved readiness/exit handling and hardened sanitization/redaction
to prevent secret leakage, including improved failure behavior.
* **Tests**
* Expanded cloud experimental parity coverage to assert the check list
matches exactly and every referenced script is executable.
* Extended TUI runtime tests for readiness detection, marker parsing,
cleanup of raw artifacts, and robust secret scanning/redaction.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-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

area: e2e End-to-end tests, nightly failures, or validation infrastructure integration: dcode LangChain Deep Code integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate interactive Deep Agents Code TUI startup

3 participants