Skip to content

test(connect): cover sandbox connect flow outcomes - #5478

Merged
cv merged 1 commit into
mainfrom
test/connect-flow-coverage
Jun 15, 2026
Merged

test(connect): cover sandbox connect flow outcomes#5478
cv merged 1 commit into
mainfrom
test/connect-flow-coverage

Conversation

@cv

@cv cv commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds focused connectSandbox flow coverage for successful shell handoff and terminal-phase stop behavior. The tests exercise readiness checks, process recovery, auth-proxy recovery, auto-pair approval, connect hints, OpenShell shell spawning, and the pre-SSH failure path.

Changes

  • Added src/lib/actions/sandbox/connect-flow.test.ts with a dist-style harness for connectSandbox.
  • Covered the ready sandbox path through active-session notice, process recovery, Ollama auth proxy recovery, auto-pair approval, connect hint, and OpenShell shell handoff.
  • Covered a terminal sandbox-list phase to ensure connect exits before spawning the final SSH/OpenShell shell.

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

  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
    • npx vitest run src/lib/actions/sandbox/connect-flow.test.ts --project cli
    • npx vitest run src/lib/actions/sandbox/connect-flow.test.ts src/lib/actions/sandbox/connect-route-repair.test.ts --project cli
    • npx @biomejs/biome lint src/lib/actions/sandbox/connect-flow.test.ts
    • npm run typecheck:cli
  • 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)

Note: commit and push hooks were skipped for this test-only branch because the broad local hook suite has unrelated pre-existing CLI failures in this checkout; targeted tests and CLI typecheck passed.


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

Summary by CodeRabbit

  • Tests
    • Added comprehensive test suite for the sandbox connection flow, covering successful connection scenarios and error handling paths.

@cv cv self-assigned this Jun 15, 2026
@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d10cd481-b1d9-469d-a37e-7db086addbec

📥 Commits

Reviewing files that changed from the base of the PR and between 543bbd4 and 078264c.

📒 Files selected for processing (1)
  • src/lib/actions/sandbox/connect-flow.test.ts

📝 Walkthrough

Walkthrough

Adds a Vitest test file (connect-flow.test.ts) for the connectSandbox action. It introduces a createConnectHarness factory that loads the compiled implementation, mocks all dependencies, and clears the require cache per test. Two test cases cover the ready path (exit 0, spawnSync called) and the terminal-failure path (exit 1, spawnSync skipped).

Changes

connectSandbox Flow Tests

Layer / File(s) Summary
Test harness: types, imports, factory, and suite lifecycle
src/lib/actions/sandbox/connect-flow.test.ts
Defines ConnectSandbox, ConnectHarness, and ConnectHarnessOptions types; imports Node/Vitest modules; implements createConnectHarness to load compiled connectSandbox from dist, wire spies for all dependent modules (OpenShell runtime capture, resolution, gateway state, failure classification, session detection, staleness formatting, process recovery, Ollama auth proxy, auto-pair approval), and stub spawnSync plus console/stdout; configures beforeEach/afterEach for environment, process.exit stub, and mock restoration.
Happy path and terminal-failure test cases
src/lib/actions/sandbox/connect-flow.test.ts
First test: sandbox list signals readiness → asserts process.exit(0), spawnSync called with ["sandbox","connect",<name>] and inherited stdio, and all harness spies invoked; checks log output for connection/status messages. Second test: sandbox list signals a terminal failure phase → asserts process.exit(1), recovery/auth/approval spies still called, and spawnSync not called.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

area: sandbox

Poem

🐇 A harness is built, the spies are set in place,
The sandbox connects at a brisk little pace.
If failure arrives, exit one we'll decree,
If all is well, spawnSync runs with glee.
Mock cache cleared fresh, each test starts anew — 🎉

🚥 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 'test(connect): cover sandbox connect flow outcomes' clearly and concisely describes the main change—adding test coverage for the sandbox connect flow with multiple outcome scenarios.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 test/connect-flow-coverage

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

@github-code-quality

github-code-quality Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the test/connect-flow-co... 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 test/connect-flow-co... 078264c +/-
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 test/connect-flow-co... branch is 45%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main test/connect-flow-co... 078264c +/-
src/lib/state/o...oard-session.ts 90%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/onboard/preflight.ts 64%
src/lib/actions...dbox/rebuild.ts 60%
src/lib/state/sandbox.ts 55%
src/lib/actions...licy-channel.ts 52%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 17%

Updated June 15, 2026 21:45 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: None

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • None. No E2E is recommended because the change is tests-only and cannot alter installer/onboarding behavior, sandbox lifecycle runtime behavior, credentials, security boundaries, network policy, inference routing, deployment, or real assistant user flows.

Optional E2E

  • None.

New E2E recommendations

  • None.

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: None
Optional Vitest E2E scenarios: None

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • None. This PR only adds a non-scenario Vitest test outside test/e2e-scenario/ and does not change the Vitest E2E scenario workflow, registry, runtime support, live tests, fixtures, or scenario metadata.

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • None.

@github-actions

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@cv
cv merged commit ca98c72 into main Jun 15, 2026
45 of 46 checks passed
@cv
cv deleted the test/connect-flow-coverage branch June 15, 2026 21:55
@cv cv added the v0.0.66 label Jun 17, 2026
@wscurran wscurran added the area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery label Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants