test(e2e): assert Ollama recovery outcomes by apurvvkumaria · Pull Request #11013 · NVIDIA/NemoClaw

apurvvkumaria · 2026-09-03T23:35:50Z

Outcome

The Ollama recovery E2E test now verifies the daemon restart, the unloaded model state, a successful agent turn, and the reloaded model state. It no longer requires a stale progress sentence that the product does not emit.

Reason

The recovery flow still proves its behavior through stable state and outcome assertions, but the obsolete prose assertion caused the live test to fail after successful recovery.

Related issues

Fixes #10973

Changes

Remove the exact assertion for Checking Ollama model readiness after daemon restart.
Retain the daemon-restart, unloaded-model, successful-turn, and reloaded-model assertions in the existing GPU E2E test.
Update the mapped GPU E2E support test to identify the stable execution-metadata recovery proof.
Update the existing E2E assertion budget to record the one removed assertion.

Verification

npx vitest run --project e2e-support test/e2e/support/gpu-e2e-helpers.test.ts — 37 tests passed.
npx tsx scripts/checks/e2e-mock-parity.mts --base origin/main --head HEAD — passed.
npm run e2e:assertions:check — passed with 1,887 direct expect calls across 86 test files.
npm run test:e2e-phases:check — passed with 132 tests across 88 files.
npx vitest run --project integration test/automation/pull-requests/growth-guardrails.test.ts — 45 tests passed.
Commit hooks — pre-commit and commit-message checks passed.
Pre-push check — CLI TypeScript passed.
GitHub verification — all three PR commits are Verified.
Secret review — the diff contains no secrets, API keys, or credentials.

Review notes

This change uses the existing GPU E2E flow and its mapped support test. It does not add a test harness. The removed text was incidental progress output; the retained assertions cover the recovery behavior. This change does not alter a security boundary or user-visible behavior, so no documentation change is needed.

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

Tests
- Updated end-to-end validation baselines to reflect current coverage.
- Removed checks for specific model-readiness messages after daemon restart; recovery and model loading remain verified through execution and process-state checks.
- Renamed a recovery test to better reflect successful execution metadata; behavior remains unchanged.
- Adjusted automated validation thresholds to align with current test coverage and routing scenarios.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> # Conflicts: # ci/e2e-assertion-budget.json

coderabbitai · 2026-09-03T23:36:48Z

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

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: 4b2c54ac-d355-4c3e-9340-0e98cdb5b44c

📥 Commits

Reviewing files that changed from the base of the PR and between 9855b22 and 5bed9e7.

📒 Files selected for processing (1)

ci/e2e-assertion-budget.json

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

Walkthrough

The GPU recovery test removes two stale Ollama output assertions, retains execution and model-state checks, updates its description, and aligns assertion-budget metrics.

Changes

GPU E2E assertion alignment

Layer / File(s)	Summary
Recovery assertion updates `test/e2e/live/gpu-e2e.test.ts`, `test/e2e/support/gpu-e2e-helpers.test.ts`	The recovery test retains execution and loaded-model checks, removes two recovered-output assertions, and updates the test description to reference issue `#10973`.
Assertion-budget alignment `ci/e2e-assertion-budget.json`	The configured assertion limits and GPU test metrics decrease. The generated probe-block count remains unchanged.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 5bed9

GPU recovery testing now relies on successful recovery execution and model-state outcomes rather than obsolete Ollama output text. No current merge-blocking risk is established.

Suggested reviewers: ericksoa, rsliter

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: updating the E2E test to assert Ollama recovery outcomes instead of a stale progress message.
Linked Issues check	✅ Passed	The changes address issue `#10973` by removing stale Ollama readiness-output assertions, retaining recovery validation through successful execution and process-state checks, and updating the related ass…
Out of Scope Changes check	✅ Passed	All changes are limited to the GPU Ollama E2E test, its support-test mapping, and the corresponding assertion-budget metrics. No unrelated changes are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch codex/fix-ollama-recovery-assertion

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

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

github-code-quality · 2026-09-03T23:44:33Z

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 5bed9e7 in the codex/fix-ollama-rec... branch remains at 96%, unchanged from commit fa08360 in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 5bed9e7 in the codex/fix-ollama-rec... branch remains at 83%, unchanged from commit 5b74336 in the main branch.

Show a line coverage summary of the most impacted files.

File	`main` 5b74336	`codex/fix-ollama-rec...` 5bed9e7	+/-
`src/lib/inferen...probe-models.ts`	100%	93%	-7%
`src/lib/inference/health.ts`	91%	90%	-1%
`src/lib/onboard...eway-process.ts`	90%	89%	-1%
`src/lib/onboard.ts`	55%	55%	0%
`src/lib/onboard...uild-context.ts`	75%	75%	0%
`src/lib/onboard...im-selection.ts`	70%	71%	+1%
`src/lib/actions...dbox/destroy.ts`	88%	90%	+2%
`src/lib/onboard...ce-lifecycle.ts`	83%	85%	+2%
`src/lib/domain/...ycle/options.ts`	85%	87%	+2%
`src/lib/actions...oy-execution.ts`	91%	94%	+3%

_{Updated September 05, 2026 01:18 UTC}

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

copy-pr-bot · 2026-09-04T21:43:09Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

github-actions · 2026-09-05T01:05:30Z

PR Review Advisor finished for commit 5bed9e7. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

## Outcome `nemoclaw <sandbox> mcp status <server> --tools` now exits nonzero when live MCP initialization or tool discovery is unsuccessful, while preserving ordinary status and bounded failure context. Successful discovery remains exit zero. ## Reason #10944 requires connection and authentication failures to be verification failures. Previously, a discovery runtime could exit zero with `ok: false` and the NemoClaw command would still report success. ### Related issues Refs #10944 Part of #10942 ## Changes - Adds a versioned discovery result contract with the sandbox runtime exit status, failed stage, and bounded failure class. The `--tools` status consumer needs this distinction because the existing Boolean result could not separate runtime, connection, authentication, protocol, and tool-operation failures; focused host/runtime tests protect the contract. - Maps refusal, DNS, TLS, timeout, HTTP 401/403, protocol, and tool-operation failures to credential-safe results. Existing best-effort session cleanup remains in place, with deterministic interruption, cleanup, and redaction coverage. - Makes only explicit `mcp status <server> --tools` verification fail the NemoClaw process on an unsuccessful result. Ordinary MCP status and list behavior remain unchanged, and the owning documentation describes the exit and recovery contract. - Extends the existing MCP bridge live target to record the exact candidate, NemoClaw, OpenShell, Hermes, host and sandbox operating systems, and container runtime for Hermes, then prove both successful authenticated discovery and denied authentication in the existing OpenClaw exact-PR lane and Hermes shard. The fixture credential is restored in `finally`; no new target, workflow, or retry behavior is added. - Rejects incomplete but parseable live status JSON before retry or diagnostic creation, so invalid evidence produces the existing labeled validation failure. - Updates the managed-image runtime probe, reviewed bundle digest, publication contract tests, and the exact E2E assertion census for protocol version 2. - Completes exact managed-image protocol-shape checks, clarifies runtime-emitted and bridge-level `commandStatus` values, and uses stage-neutral redacted request details, and refreshes the owning Dockerfile instruction digest. - Normalizes the SDK's typed `RequestTimeout` error at the standalone runtime boundary. Arbitrary endpoint error text containing timeout-like words remains a redacted tool-operation failure instead of being misclassified as a local connection timeout. - Restores exact live assertions for successful discovery and denied authentication, including result status, tool count and names, truncation, failed stage, failure class, and runtime command status. - Preserves the documented trust boundary for endpoint-supplied MCP tool names. This PR does not add tool-name redaction or a new security contract. ## Verification - `npm run validate:pr` — passed for candidate `fa636a6f1b820503070a91f4fb51fcc7b0c486ee` against canonical base `b0d4650c6cc506c2a07ddf4c909035378a0626c7` in a fresh native ARM64 Node 22.23.1 container without contributor-host credentials, using checksum-verified Hadolint 2.14.0 and the base-pinned Markdownlint 0.22.0 hook. - `npx vitest run --project cli src/lib/actions/sandbox/mcp-tool-discovery-runtime.test.ts` — 33 tests passed; a direct Node import also confirmed the shared core no longer requires the nested SDK package. - `npx vitest run --project cli src/lib/actions/sandbox/mcp-bridge-tool-discovery.test.ts src/lib/actions/sandbox/mcp-tool-discovery-runtime.test.ts` — 53 tests passed. - `npx vitest run --project cli src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts -t '#10944'` — 2 tests passed. - `npx vitest run --project e2e-support test/e2e/support/mcp-bridge-tool-discovery.test.ts` — 51 tests passed, including incomplete and malformed output without retry or artifact writes. - `npx vitest run --project integration test/mcp/mcp-tool-discovery-image-contract.test.ts test/inference/managed/managed-image-publication-workflow.test.ts --testTimeout=30000` — 55 tests passed. - `npx vitest run --project integration test/networking/dashboard-remote-bind-lifecycle.test.ts` — 27 tests passed. - `npx vitest run --project integration test/repository/vitest-watch-triggers.test.ts` — 73 tests passed. - `npm --prefix tools/mcp-tool-discovery-runtime test`, `npm --prefix tools/mcp-tool-discovery-runtime run typecheck`, and `npm --prefix tools/mcp-tool-discovery-runtime run bundle:reviewed:check` — passed. - `npm run e2e:assertions:check`, `npm run test-size:check`, `npm run source-shape:check`, `npm run test:e2e-phases:check`, `npm run checks:repository`, and `npm run typecheck:cli` — passed. - `npm run docs` — passed with no errors and the five existing warnings. - [`mcp-bridge` exact-PR run 33945212794](https://github.com/NVIDIA/NemoClaw/actions/runs/33945212794) exercised candidate `a5a552e274322201ff4336db64ae2228707e790d`. OpenClaw and Deep Agents Code passed, and the Hermes lane passed the new authenticated discovery, denied HTTP 401 classification, version capture, credential restoration, artifact credential scan, and cleanup assertions. Later candidates integrate the current base and apply deterministic review corrections to the redacted detail wording and managed-image protocol probe. The live network interaction, failure classification, cleanup, credential restoration, and scenario remain unchanged. - That run later failed in the unchanged Hermes restart-without-host-secret lifecycle. The same failure independently occurred in unrelated PR #11013 on the same base while its MCP bridge owner paths were unchanged. All 11 registered cleanup operations passed, the artifact credential scan passed, and no resource or credential cleanup remains. - The existing maintainer-dispatched `mcp-bridge` matrix supplied the Hermes evidence. This change does not add a second automatic PR lane or redesign CI. - The diff contains no secrets, API keys, or credentials. ## Review notes - Sensitive paths in `NVIDIA/NemoClaw` are `.github/workflows/managed-images.yaml`, `agents/hermes/Dockerfile`, `agents/langchain-deepagents-code/Dockerfile`, `src/lib/onboard/dockerfile-remote-dashboard-bind-contract.ts`, and `tools/mcp-tool-discovery-runtime/**`. - CodeRabbit reviewed the implementation through `e69b1ee`; `532e269` addressed its last valid finding, and all seven review threads are resolved. The complete PR Review Advisor rerun for `5bb7463` succeeded with all nine specialist artifacts. Candidate `6d9ffcdb` addressed the two maintainer findings and integrated canonical main. Candidate `62710c6` exposed that the first package-boundary repair still left the SDK adapter inside root CLI type-checking. Candidate `fa636a6` moves typed SDK timeout normalization into the standalone entrypoint, keeps the shared core package-neutral, and passes root type-checking. Exact-head automated evaluation has restarted. - This review context does not claim approval or a CI waiver. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>  ## Summary by CodeRabbit * **New Features** * MCP tool discovery now reports protocol version 2 results with command status, failure stage, failure classification, and sanitized diagnostics. * Discovery failures return a nonzero command status while preserving server status information. * Added MCP denied-tool updates with replacement, clearing, journaling, rollback, and recovery handling. * Discovery responses now enforce a complete, consistent result shape. * **Bug Fixes** * Improved handling of unavailable runtimes, invalid credentials, startup failures, and interrupted responses. * MCP policy status now surfaces drifted or interrupted updates. * **Documentation** * Updated MCP command and sandbox documentation for discovery status, denied-tool management, and recovery behavior.  --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

apurvvkumaria added 2 commits September 3, 2026 12:03

test(e2e): assert Ollama recovery outcomes

346e14c

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

merge: update Ollama recovery assertion branch

dea2e9c

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> # Conflicts: # ci/e2e-assertion-budget.json

apurvvkumaria self-assigned this Sep 3, 2026

apurvvkumaria mentioned this pull request Sep 3, 2026

e2e(ollama): recovery assertion expects a breadcrumb product does not emit #10973

Closed

test(e2e): cover stable Ollama recovery proof

bd5f23f

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

apurvvkumaria added 5 commits September 3, 2026 18:24

merge: update Ollama recovery branch

0d27cc9

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

merge: sync main into ollama recovery fix

0093205

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

test(e2e): rely on ollama recovery outcomes

aeb278b

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

merge: update ollama recovery fix with main

bb982b9

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

merge: update Ollama recovery fix from main

be7b18f

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

apurvvkumaria requested a review from prekshivyas September 4, 2026 19:51

prekshivyas mentioned this pull request Sep 4, 2026

[Epic] Restore full E2E qualification on main #10979

Open

31 tasks

merge: resolve conflicts with main

9855b22

merge: update Ollama recovery fix from main

5bed9e7

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

prekshivyas approved these changes Sep 5, 2026

View reviewed changes

prekshivyas merged commit 685eab2 into main Sep 5, 2026
90 checks passed

prekshivyas deleted the codex/fix-ollama-recovery-assertion branch September 5, 2026 02:44

github-actions Bot added the v0.0.120 label Sep 5, 2026

apurvvkumaria mentioned this pull request Sep 6, 2026

fix(mcp): fail unsuccessful tool discovery #11087

Merged

Navigation Menu

Uh oh!

test(e2e): assert Ollama recovery outcomes - #11013

test(e2e): assert Ollama recovery outcomes#11013
prekshivyas merged 10 commits into
mainfrom
codex/fix-ollama-recovery-assertion

apurvvkumaria commented Sep 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 3, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Uh oh!

github-code-quality Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

apurvvkumaria commented Sep 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Outcome

Reason

Related issues

Changes

Verification

Review notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

github-code-quality Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Overview

TypeScript / code-coverage/plugin

TypeScript / code-coverage/cli

Uh oh!

copy-pr-bot Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

apurvvkumaria commented Sep 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 3, 2026 •

edited

Loading

github-code-quality Bot commented Sep 3, 2026 •

edited

Loading