fix(e2e): echo unique launch replies in mock inference - #9058
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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 (4)
📝 WalkthroughWalkthroughThe fake inference server now supports opt-in launch replies derived from prompt fragments. The inference adapter enables this mode, and end-to-end tests verify exact launch responses while preserving ChangesPrompt-derived launch reply flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to This changes only opt-in E2E mock response behavior, with targeted tests passing; no actionable merge-blocking risk remains. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant InferenceAdapterTest
participant InferenceAdapter
participant FakeOpenAiCompatibleServer
InferenceAdapterTest->>InferenceAdapter: send launch-specific prompt
InferenceAdapter->>FakeOpenAiCompatibleServer: request chat completion
FakeOpenAiCompatibleServer->>FakeOpenAiCompatibleServer: validate prompt fragments
FakeOpenAiCompatibleServer-->>InferenceAdapter: constructed launch reply
InferenceAdapter-->>InferenceAdapterTest: exact response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
2 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
3 additional E2E selections from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
<!-- markdownlint-disable MD041 --> ## Summary Add a Deferred N1x Express preview that recognizes the qualified FastOS and PCI identity and offers one-host managed vLLM. N1x remains outside the supported-platform set until physical NemoClaw Express E2E validation is complete. ## Related Issue Fixes #8574 ## Changes - Add a bounded N1x identity reader for the installer and readiness projection. Generic DMI cannot identify this device directly, so the classifier requires a trusted `/etc/fastos-release` marker and exact NVIDIA display PCI identity; focused identity and installer tests protect the boundary. - Add a distinct N1x managed-vLLM profile using `nvidia/Qwen3.6-35B-A3B-NVFP4`. A separate profile prevents N1x from inheriting DGX Spark cluster, fixed-catalog, llama.cpp, or NIM behavior; vLLM profile and menu tests protect the selection. - Keep Local NVIDIA NIM unavailable on N1x in both interactive onboarding and explicit provider selection. Provider-selection tests prove that `nim-local` is rejected before NIM setup while managed vLLM remains available. - Keep `host.platform.supported` absent and add a blocking pending-validation finding. Only explicit managed-vLLM preview intent can waive that finding after N1x identity qualifies; readiness and onboarding-admission tests protect the exception. - Document the Deferred preview, readiness contract, model, exclusions, and physical E2E promotion gate across the platform matrix, quickstarts, inference setup, command reference, and readiness reference. ## 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) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Local security review covered bounded identity input, marker ownership and mode checks, symlink and malformed-marker rejection, exact PCI matching, fail-closed readiness states, and the scoped preview waiver. No blocking findings remain. - [ ] 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: Reviewed the complete 41-file diff at `06aecf4d1247980d37af2a0550b4cc272462e8a1` against `8860d24fe17da339c56b83b08b8f2b43568adbd7`. N1x remains outside the supported-platform set and admits only explicit Deferred `install-vllm` intent; existing-server reuse is excluded, and port `8000` conflicts, preview decline, unqualified identity, and other providers fail closed. Hermes and Deep Agents preserve their explicit agent and sandbox names through Express setup. Verified the canonical platform matrix, generated Starter Prompt, and all agent variants. The #9040 recovery documentation singularly identifies the Platform Evidence workflow. Base change #9058 affects only internal E2E fixtures and support, and its unique reply behavior is already documented. `npm run docs` passed with 0 errors and 2 existing warnings; CLI typecheck, `git diff --check`, normal merge hooks, the changed E2E-support test (10/10), and the N1x installer regression (10/10) passed. - Agent: Codex Desktop <!-- docs-review-head-sha: 06aecf4 --> <!-- docs-review-agents-blob-sha: e30afb2 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## 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 validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set — `npx vitest run test/install-express-n1x.test.ts` (10 passed), including non-C locale metadata collection plus N1x decline, `NEMOCLAW_NO_EXPRESS=1`, and explicit `install-vllm` bypass coverage. - [ ] 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; this change does not modify the runtime-wide test harness or repository-wide validation configuration. - [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) — build passed with 0 errors and 2 pre-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) — no new documentation pages. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added deferred Express-install preview support for N1x FASTOS hosts. * Added managed vLLM support with the default Qwen3.6-35B-A3B-NVFP4 model. * Added N1x platform detection and readiness qualification. * **Bug Fixes** * Prevented local NVIDIA NIM from being offered or selected on N1x hosts. * **Documentation** * Updated setup, inference, platform-support, readiness, and security guidance for N1x requirements, limitations, and preview behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: San Dang <sdang@nvidia.com> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Summary
The release E2E run showed two agent-turn failures: OpenClaw rendered the expected unique reply inside terminal control sequences, and the shared mock inference endpoint returned fixed
PONGto Hermes. Currentmainalready contains the OpenClaw OSC normalization and regression from #9034; this change repairs the remaining mock behavior by deriving each unique reply from the exact launch prompt.Related Issue
Fixes #9046
Changes
PONGresponse for partial or altered prompts.PONGbefore the change and now verifies both the unique reply and fallback behavior.PONGprompt and did not cover the generated launch-turn contract.Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/inference-adapter.test.ts test/e2e/support/hosted-inference.test.ts test/e2e/support/device-auth-health-helpers.test.ts test/e2e/support/launch-agent-turn.test.ts: 4 files passed, 32 tests passed, 1 platform skip.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this isolated mock behavior. An additionalnpx vitest run --project e2e-supportpassed 197 files and failed in seven unrelated macOS host-sensitive files that require GNUfind, trusted Homebrew state, systemd or Ollama, or longer subprocess timeouts.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Prekshi Vyas prekshiv@nvidia.com
Summary by CodeRabbit