test(dcode): exercise bare default connect - #7036
Conversation
Co-authored-by: Tony Luo <xialuo@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Deep Agents Code headless inference acceptance check now invokes bare ChangesDeep Agents Code bare connect
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DCodeCheck
participant NemoClawCLI
participant OpenShellShim
DCodeCheck->>OpenShellShim: record sandbox targets
DCodeCheck->>NemoClawCLI: run bare connect --probe-only
NemoClawCLI->>OpenShellShim: execute target
DCodeCheck->>OpenShellShim: validate expected target
DCodeCheck->>DCodeCheck: classify probe result
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the Updated |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
cv
left a comment
There was a problem hiding this comment.
Blocking exact head ce3f94b on Advisor PRA-1. The new bare-connect coverage is valuable, but setup/lifecycle evidence targets $SANDBOX_NAME while bare connect --probe-only independently resolves the registry default. The canonical happy path currently aligns them, yet a mismatched override/default could combine evidence from two sandboxes and falsely pass, violating this script’s shared-evidence contract. Before the bare probe, explicitly assert the ready registry default equals $SANDBOX_NAME, add a mismatch regression, then rerun exact-diff CI/E2E and request rereview.
E2E Target Results —
|
| Test | Result | Total wall clock time |
|---|---|---|
| live | 19s |
Co-authored-by: Tony Luo <xialuo@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Addressed the requested shared-evidence gap at verified head |
|
🌿 Preview your docs: https://nvidia-preview-pr-7036.docs.buildwithfern.com/nemoclaw |
E2E Target Results — ✅ All selected tests passedRun: 29525767493
|
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
There was a problem hiding this comment.
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/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh`:
- Around line 190-196: The connect-status handling in
validate_connect_target_trace must validate the trace before returning a nonzero
connect status, prioritizing missing or mismatched sandbox-target attribution
over the connection failure; update
test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.sh
at lines 190-196 accordingly. Add a regression case covering both a wrong target
and nonzero exit in test/langchain-deepagents-code-headless-runtime.test.ts at
lines 131-136.
🪄 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: dd268d9d-ee9b-4ced-ae6b-162d0151599c
📒 Files selected for processing (3)
test/e2e/e2e-cloud-experimental/checks/07-deepagents-code-headless-inference.shtest/langchain-deepagents-code-headless-runtime.test.tstest/langchain-deepagents-code-image.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/langchain-deepagents-code-image.test.ts
|
Exact head |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
Exercise bare
nemoclaw connect --probe-onlyin the existing Deep Agents Code live acceptance and prove that it reaches the exact ready registry default established by onboarding. The prior check supplied the sandbox name explicitly; the first bare-connect version also left a list-to-connect race that could accept a different healthy sandbox.Related Issue
Closes #7034
Validates the public default-routing behavior reported in #6627.
Changes
readyDefaultSandboxtonemoclaw list --json, sourced from the same non-pending registry selector used by bare connect after recovery.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable to this focused CLI contract and live acceptance; exact changed-rangenpm run check:diffpassed.npm run docsbuilds without warnings (doc changes only) — 0 errors and 2 pre-existing warnings; routes and all generated agent variants passed.Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Bug Fixes
nemoclaw connect --probe-onlyverification for Deep Agents Code managed inference routes, including correct “bare” route sandbox targeting after onboarding.Tests