Skip to content

test(onboard): make gateway TCP readiness refusal deterministic (#7250)#7277

Merged
prekshivyas merged 7 commits into
mainfrom
fix/7250-tcp-readiness-deterministic-refused
Jul 23, 2026
Merged

test(onboard): make gateway TCP readiness refusal deterministic (#7250)#7277
prekshivyas merged 7 commits into
mainfrom
fix/7250-tcp-readiness-deterministic-refused

Conversation

@jason-ma-nv

@jason-ma-nv jason-ma-nv commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

The negative case in gateway-tcp-readiness.test.ts assumed a just-closed
ephemeral loopback port becomes synchronously unreachable, which is not
portable: under WSL2 mirrored networking the mirrored loopback path keeps
accepting connections for a few milliseconds after the Linux listener closes,
so isGatewayTcpReady observes a successful connect and resolves true,
failing the CLI suite. This makes the closed/error path deterministic by
simulating ECONNREFUSED instead of racing listener teardown. Production probe
behavior is unchanged.

Related Issue

Fixes #7250

Changes

  • Removed the getLikelyClosedPort() helper, which opened and closed a real
    ephemeral loopback listener and depended on platform-specific teardown timing.
  • Rewrote "resolves false when the connection is refused (ECONNREFUSED)" to mock
    net.createConnection (the OS boundary) and emit an ECONNREFUSED error,
    exercising the real socket.once("error") -> settle(false) path in
    isGatewayTcpReadyImpl with no timing sleep and no listener race. This reuses
    the same net.createConnection mocking seam the existing minimum-timeout test
    already uses, and asserts the socket is destroyed on the error path.
  • No production source changed; the positive-listener and timeout tests are
    untouched.

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: internal test fixture; no user-facing behavior changes.
  • 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: Change is test-fixture only under src/lib/onboard/; zero production onboarding/probe code is modified, so the security surface is unchanged. Flagging for maintainer confirmation.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-not-needed
  • Evidence: The exact-head diff changes only the gateway TCP readiness test fixture; no user-facing command, output, configuration, or supported behavior changes.
  • Agent: Codex Desktop

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project cli src/lib/onboard/gateway-tcp-readiness.test.ts → 6/6 passed locally and on a fresh clone + npm ci on an ordinary Linux (Ubuntu, x86_64) host (exit 0).
  • Applicable broad gate passed — justification: test-only fixture portability fix; not a broad runtime/test-harness change.
  • 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: Jason Ma jama@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Improved coverage for gateway TCP readiness checks when a connection is refused.
    • Added deterministic simulation of ECONNREFUSED to confirm the service reports it as unavailable.
    • Verified the connection cleanup behavior via mocked socket destruction.

The negative case in gateway-tcp-readiness.test.ts opened and closed an
ephemeral loopback listener via getLikelyClosedPort() and assumed the port
was then synchronously refused. That assumption is non-portable: under WSL2
mirrored networking the mirrored loopback path keeps accepting connections
for a few milliseconds after the Linux listener closes, so isGatewayTcpReady
observes a successful connect and resolves true, failing the suite.

Simulate the ECONNREFUSED a refused socket emits by mocking net.createConnection
(the OS boundary), exercising the real socket.once("error") path in
isGatewayTcpReadyImpl without racing listener teardown or using a timing sleep.
This mirrors the mocking seam the existing minimum-timeout test already uses,
and is identical on WSL2 mirrored networking and ordinary Linux CI. Production
probe behavior is unchanged; the defect was a non-portable test fixture only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The gateway TCP readiness test replaces platform-dependent closed-port probing with a mocked ECONNREFUSED socket event, asserting that readiness resolves false and the socket is destroyed.

Changes

Gateway readiness tests

Layer / File(s) Summary
Deterministic connection-refused test
src/lib/onboard/gateway-tcp-readiness.test.ts
Removes the ephemeral closed-port helper and simulates ECONNREFUSED through net.createConnection, verifying the false result and socket cleanup.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: area: onboarding

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The test change matches #7250 by simulating ECONNREFUSED deterministically and keeping production behavior unchanged.
Out of Scope Changes check ✅ Passed The PR stays confined to the readiness test and does not introduce unrelated code or production changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: making the gateway TCP readiness refusal test deterministic.
✨ 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/7250-tcp-readiness-deterministic-refused

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

@github-code-quality

github-code-quality Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 633e34d in the fix/7250-tcp-readine... branch remains at 96%, unchanged from commit e84f5ee in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 633e34d in the fix/7250-tcp-readine... branch remains at 80%, unchanged from commit e84f5ee in the main branch.

Show a code coverage summary of the most impacted files.
File main e84f5ee fix/7250-tcp-readine... 633e34d +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/state/m...-acquisition.ts 87% 84% -3%
src/lib/state/g...way-registry.ts 95% 94% -1%
src/lib/messagi...nnels/policy.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/securit...ntial-filter.ts 93% 93% 0%
src/lib/state/m...ock-identity.ts 95% 95% 0%
src/lib/state/m...lock-storage.ts 97% 97% 0%
src/lib/trace.ts 94% 94% 0%
src/lib/platform.ts 84% 89% +5%

Updated July 23, 2026 23:53 UTC

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

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

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

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

E2E guidance

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

Recommended E2E: cloud-onboard

1 optional E2E recommendation
  • onboard-negative-paths

Workflow run details

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

The codebase-growth-guardrails gate rejects added `if` statements in changed
test files. Replace the `if (event === "error")` branch inside the mocked
`once` with the listener-map seam the minimum-timeout test already uses: capture
the registered listeners and fire the "error" callback directly with optional
chaining. Behavior is unchanged — the negative case still simulates ECONNREFUSED
deterministically with no timing sleep — and the file's `if` count returns to
the base value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow chore Build, CI, dependency, or tooling maintenance platform: wsl Affects Windows Subsystem for Linux labels Jul 21, 2026
@cv cv added the v0.0.93 label Jul 22, 2026

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the exact refreshed diff and linked regression scope. The change replaces a timing-dependent refused-port setup with a deterministic ECONNREFUSED socket mock, preserves cleanup assertions, and adds no product surface. All 53 current checks and the repository merge gate pass at 742ebbf.

@prekshivyas
prekshivyas merged commit acd5e5f into main Jul 23, 2026
82 of 83 checks passed
@prekshivyas
prekshivyas deleted the fix/7250-tcp-readiness-deterministic-refused branch July 23, 2026 23:58
@prekshivyas prekshivyas mentioned this pull request Jul 24, 2026
23 tasks
prekshivyas added a commit that referenced this pull request Jul 24, 2026
<!-- markdownlint-disable MD041 -->
## Summary

This PR adds the canonical dated release entry for NemoClaw v0.0.94
before the tag is cut.
The entry reconciles all 26 commits since v0.0.93 and links each
user-visible change to its owning documentation.

## Changes

- Add `docs/changelog/2026-07-24.mdx` with the exact `## v0.0.94`
heading, parser-safe SPDX comment, release summary, and detailed
bullets.
- Record sandbox restore and update behavior, onboarding and inference
changes, network policy behavior, security evidence, Hermes build
performance, DGX Station guidance, and E2E validation changes.
- Preserve `docs/` as the source of truth without changing the AI-agent
documentation routing skill.
- Use [E2E run
30075443016](https://github.com/NVIDIA/NemoClaw/actions/runs/30075443016)
for release QA evidence at exact tested SHA
`04e6dfa883071dda9df429c66e73168e1a995cba`.

### Source summary

- [#7461](#7461) ->
`docs/changelog/2026-07-24.mdx`: Record the ownership-preserving Hermes
image layer reduction and hosted timing comparison.
- [#7460](#7460) ->
`docs/changelog/2026-07-24.mdx`: Record removal of candidate Hermes swap
setup from E2E validation.
- [#7458](#7458) ->
`docs/security/fern-5.80.1-dependency-review.md`,
`docs/changelog/2026-07-24.mdx`: Record the reviewed Fern CLI update.
- [#7457](#7457) ->
`docs/changelog/2026-07-24.mdx`: Record periodic runner-pressure
telemetry.
- [#7455](#7455) ->
`docs/changelog/2026-07-24.mdx`: Record non-blocking absent Fern
previews.
- [#7450](#7450) ->
`docs/changelog/2026-07-24.mdx`: Record stable cancellation handling for
live-test child processes.
- [#7449](#7449) ->
`docs/changelog/2026-07-24.mdx`: Record parallel plugin EXDEV coverage.
- [#7448](#7448) ->
`docs/changelog/2026-07-24.mdx`: Record isolated long-running E2E lanes.
- [#7444](#7444) ->
`docs/changelog/2026-07-24.mdx`: Record exact-head Hermes swap
validation.
- [#7437](#7437) ->
`docs/manage-sandboxes/backup-restore.mdx`,
`docs/changelog/2026-07-24.mdx`: Record gateway pairing and
authenticated verification after cross-sandbox restore.
- [#7436](#7436) ->
`docs/manage-sandboxes/backup-restore.mdx`,
`docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record
selected stale-state cleanup and Hermes virtual-environment access
repair.
- [#7385](#7385) ->
`docs/network-policy/customize-network-policy.mdx`,
`docs/changelog/2026-07-24.mdx`: Record the read-only agent-variant
route check.
- [#7371](#7371) ->
`docs/changelog/2026-07-24.mdx`: Record host-artifact verification for
session exports.
- [#7359](#7359) ->
`docs/changelog/2026-07-24.mdx`: Record platform validation for managed
vLLM model overrides.
- [#7356](#7356) ->
`docs/changelog/2026-07-24.mdx`: Record token-shaped value redaction for
`sandbox doctor --json`.
- [#7354](#7354) ->
`docs/security/advisory-early-warning.md`,
`docs/changelog/2026-07-24.mdx`: Record advisory correlation and
retained audit provenance.
- [#7352](#7352) ->
`docs/network-policy/customize-network-policy.mdx`,
`docs/network-policy/integration-policy-examples.mdx`,
`docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record
preset reapplication and bounded `tls: skip` guidance.
- [#7345](#7345) ->
`docs/security/openclaw-2026.6.10-dependency-review.md`,
`docs/security/openclaw-2026.7.1-dependency-review.md`,
`docs/changelog/2026-07-24.mdx`: Record reviewed npm audit exception
enforcement.
- [#7340](#7340) ->
`docs/network-policy/customize-network-policy.mdx`,
`docs/changelog/2026-07-24.mdx`: Record the repaired CLI-reference
route.
- [#7334](#7334) ->
`docs/get-started/dgx-station-preparation.mdx`,
`docs/changelog/2026-07-24.mdx`: Record the qualified OTA metadata
fallback and narrowed override wording.
- [#7322](#7322) ->
`docs/changelog/2026-07-24.mdx`: Reconcile the gateway source tag added
to plugin registration banners.
- [#7284](#7284) ->
`docs/manage-sandboxes/update-sandboxes.mdx`,
`docs/changelog/2026-07-24.mdx`: Record read-only `upgrade-sandboxes
--check` behavior and recorded-gateway selection.
- [#7277](#7277) ->
`docs/changelog/2026-07-24.mdx`: Reconcile deterministic gateway TCP
refusal coverage.
- [#7234](#7234) ->
`docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`:
Record preserved DGX Spark managed vLLM Express intent on resume.
- [#7185](#7185) ->
`docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-24.mdx`:
Record IPv4 fallback DNS selection and exact resolver probing.
- [#6820](#6820) ->
`docs/reference/commands.mdx`, `docs/changelog/2026-07-24.mdx`: Record
the versioned, redacted `--events=jsonl` onboarding stream.

## Type of Change

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

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification: `npx vitest
run test/changelog-docs.test.ts` passed 6/6 tests.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-07-24.mdx`; the writing rules,
documentation style, exact release range, skip terms, published routes,
and product scope were reviewed; the changelog test passed 6/6; `npm run
docs` passed with route checking OK, zero errors, and two existing
warnings.
- Agent: Codex Desktop
<!-- docs-review-head-sha: 65368f9 -->
<!-- docs-review-agents-blob-sha: 9c9b36d -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable
- Station profile/scenario: Not applicable
- Result: Not applicable
- Supporting evidence: Not applicable. This PR does not change
`scripts/prepare-dgx-station-host.sh`.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` passed 6/6 tests.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: Not applicable to the
dated changelog entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only). The
build passed with zero errors and two existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages
only). Native dated changelog entries use the required parser-safe MDX
SPDX comment and no frontmatter.

---
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>


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

## Summary by CodeRabbit

* **Documentation**
  * Added the v0.0.94 release changelog.
  * Documented improvements to sandbox snapshot and restore behavior.
* Added updates for gateway selection, policy comparisons, onboarding
event output, and DGX recovery workflows.
* Documented enhanced diagnostics redaction, npm audit provenance, image
assembly performance, and validation stability improvements.


<!-- end of auto-generated comment: release notes by coderabbit.ai -->

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

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow chore Build, CI, dependency, or tooling maintenance platform: wsl Affects Windows Subsystem for Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Closed-port TCP readiness test fails on WSL mirrored networking

6 participants