fix(blueprint): fail apply when provider or inference setup fails (#6703)

[jason-ma-nv]

Summary

The direct Blueprint runner's actionApply (nemoclaw/src/blueprint/runner.ts) ran two required OpenShell mutations with reject: false but never inspected their results:

• openshell provider create
• openshell inference set

A non-zero exit from either was silently ignored, so actionApply still persisted plan.json, emitted PROGRESS:100 / Apply complete, and reported the sandbox ready — leaving automation with a successful apply result and durable run state for a sandbox that cannot perform inference.

Fixes #6703.

Change (scoped to the runner + its co-located tests)

• provider create — inspect the result. Mirror the sandbox-create contract already in this function: tolerate an already-existing provider as a reuse (keeps re-apply idempotent), and fail on any other non-zero result.
• inference set — inspect the result; any non-zero result aborts the apply (no already exists nuance — a routed provider is mandatory).
• Surface bounded, single-line command detail via a new boundedCommandError helper. The credential is passed to these commands via the subprocess env (never argv), so the stderr stream cannot carry the secret; the helper also caps length.

The reporter's open question (provider "already exists")

The reporter asked what the contract should be when provider create reports the provider already exists (reuse / upsert / fail). This PR encodes reuse, because that's the existing, in-file contract: the sibling sandbox create call a few lines above already treats "already exists" as a reuse (runner.ts, the createResult.stderr.includes("already exists") branch). Following the same convention keeps apply idempotent and avoids a substring-accident. If maintainers want stricter semantics (e.g. verify the existing provider's config matches, or upsert), that's a separable follow-up — this PR does not regress the false-success bug in the meantime.

Verification

• npx vitest run nemoclaw/src/blueprint/runner.test.ts → 95/95 pass (locally and on the ipp2-0085 verify host, Node 22.23.1), including 3 new regression tests:
  • provider-create failure → apply rejects, no plan.json, no Apply complete / PROGRESS:100, and inference-set is not attempted;
  • provider "already exists" → apply completes and persists (reuse path);
  • inference-set failure → apply rejects, no plan.json, no completion output.
• npx biome check clean; npx tsx scripts/checks/run.ts all structural checks pass; test-conditionals:scan exits 0 (the new mock-dispatch ifs match the existing pattern in this test file).

Signed-off-by: Jason Ma jama@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved sandbox setup so failed provider creation or inference route configuration stops execution and no longer reports success.
  • Prevents persisting incomplete plans (plan.json) after failures, avoiding misleading PROGRESS:100 updates.
  • Reuses an existing provider when setup reports it already exists.
  • Makes command error output more concise by compacting and truncating verbose stderr.
• Tests
  • Added coverage for credential-redacted failure behavior and “already exists” reuse scenarios during apply.

[coderabbitai]

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: 10284d69-1c7c-4e6c-85f5-0a86f0d84574

📥 Commits

Reviewing files that changed from the base of the PR and between 3777db1 and a0cf03a.

📒 Files selected for processing (2)

• nemoclaw/src/blueprint/runner-test-fixtures.ts
• nemoclaw/src/blueprint/runner.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• nemoclaw/src/blueprint/runner.test.ts

---

📝 Walkthrough

Walkthrough

actionApply now validates provider creation and inference setup results, reuses existing providers, bounds credential-safe command errors, and tests persistence and completion behavior for success and failure paths.

Changes

Blueprint apply reliability

Layer / File(s)	Summary
Command failure handling nemoclaw/src/blueprint/runner.ts	actionApply checks provider and inference command exit codes, reuses existing providers on matching errors, and reports bounded, redacted stderr details.
Failure semantics and test fixtures nemoclaw/src/blueprint/runner-test-fixtures.ts, nemoclaw/src/blueprint/runner.test.ts	Adds reusable blueprint fixtures and tests that failed mutations reject without persisting plan.json or emitting completion output, while existing-provider reuse completes successfully.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant actionApply
  participant OpenShell
  participant plan_json
  actionApply->>OpenShell: provider create
  alt provider succeeds or already exists
    actionApply->>OpenShell: inference set
    alt inference succeeds
      actionApply->>plan_json: persist plan
      actionApply-->>actionApply: emit apply complete
    end
  end

Loading

Possibly related PRs

• NVIDIA/NemoClaw#3674: Both changes update Blueprint apply behavior involving command handling and plan.json persistence.

Suggested labels: bug-fix, security, area: cli

Suggested reviewers: cjagwani

🚥 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 states the core behavior change in blueprint apply failure handling.
Linked Issues check	✅ Passed	The changes inspect both required setup commands, handle existing providers, and add tests for #6703's failure and reuse cases.
Out of Scope Changes check	✅ Passed	The edits stay in the direct Blueprint runner and co-located tests/fixtures, with no unrelated feature work.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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/6703-blueprint-apply-provider-inference-failure

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/6703-blueprint-a... branch remains at 96%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.

File	main d683b6a	fix/6703-blueprint-a... a0cf03a	+/-
nemoclaw/src/bl...print/runner.ts	96%	96%	0%
nemoclaw/src/bl...est-fixtures.ts	0%	100%	+100%

TypeScript / code-coverage/cli

The overall coverage in the fix/6703-blueprint-a... branch remains at 79%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.

File	main d683b6a	fix/6703-blueprint-a... a0cf03a	+/-
src/lib/actions...e-classifier.ts	65%	48%	-17%
src/lib/shields/index.ts	71%	59%	-12%
src/lib/sandbox/config.ts	65%	59%	-6%
src/lib/policy/index.ts	66%	62%	-4%
src/lib/state/sandbox.ts	86%	84%	-2%
src/lib/state/o...oard-session.ts	85%	83%	-2%
src/lib/credentials/store.ts	57%	61%	+4%
src/lib/runner.ts	70%	80%	+10%
src/lib/onboard...p-nim-ollama.ts	76%	90%	+14%
src/lib/cli/com...and-registry.ts	60%	90%	+30%

---

_{Updated July 12, 2026 07:07 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

E2E Advisor Recommendation

Required E2E: inference-routing, credential-sanitization
Optional E2E: sandbox-operations

Dispatch hint: inference-routing,credential-sanitization

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• inference-routing: This existing live lane directly exercises the blueprint runner and real inference-routing behavior, including its fail-closed boundary.
• credential-sanitization: This existing live lane validates credential handling across real install/onboard and sandbox boundaries; run it because this PR changes secret redaction on a provider failure path.

Optional E2E

• sandbox-operations: Provides adjacent confidence that successful sandbox creation and hosted-inference operation remain healthy after lifecycle sequencing changes.

New E2E recommendations

• blueprint-runner-provider-failure (high): Current unit coverage verifies provider/inference command failures and stderr redaction, but no existing E2E explicitly proves that a blueprint apply against an OpenShell-compatible command boundary fails without persisting run state or emitting completion after either mutation fails.
  • Suggested test: Add a credential-free blueprint-runner E2E that injects provider-create and inference-set failures, verifies redacted diagnostics, no plan.json/completion, and idempotent reuse on an already-existing provider.

Dispatch hint

• Workflow: E2E
• jobs input: inference-routing,credential-sanitization

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Findings: 0 required · 0 warnings · 0 optional suggestions
Since last review: 1 prior item resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated review. Required findings need action before merge. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision.

[coderabbitai]

🧹 Nitpick comments (1)

nemoclaw/src/blueprint/runner.test.ts (1)

655-664: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace the mock-call assertion with an observable-outcome check.

Filtering mockExeca.mock.calls to prove "inference set" wasn't attempted is an implementation-detail assertion. The same claim can be proven via public-boundary output, consistent with how the rest of this test (and the inference-set-failure test) already verifies downstream steps didn't run.

♻️ Suggested fix

-      // The inference route must not be attempted after provider creation fails.
-      const inferenceSetCalls = mockExeca.mock.calls.filter(
-        (c) => Array.isArray(c[1]) && c[1][0] === "inference" && c[1][1] === "set",
-      );
-      expect(inferenceSetCalls).toEqual([]);
+      // The inference route must not be attempted after provider creation fails.
+      expect(stdoutText()).not.toContain("PROGRESS:70");

As per path instructions, "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."

🤖 Prompt for 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.

In `@nemoclaw/src/blueprint/runner.test.ts` around lines 655 - 664, Replace the
mockExeca call-history filtering assertion in the failed required-mutation test
with an observable public-boundary assertion that confirms the inference route
was not attempted, matching the existing output-based checks and the
inference-set-failure test. Remove the implementation-detail dependency on
mockExeca.mock.calls while preserving the test’s verification of failed provider
creation behavior.

Source: Path instructions

🤖 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.

Nitpick comments:
In `@nemoclaw/src/blueprint/runner.test.ts`:
- Around line 655-664: Replace the mockExeca call-history filtering assertion in
the failed required-mutation test with an observable public-boundary assertion
that confirms the inference route was not attempted, matching the existing
output-based checks and the inference-set-failure test. Remove the
implementation-detail dependency on mockExeca.mock.calls while preserving the
test’s verification of failed provider creation behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 20ee17f8-16b2-4c43-b5b5-f846fae00cb5

📥 Commits

Reviewing files that changed from the base of the PR and between 451f6ab and addaca7.

📒 Files selected for processing (2)

• nemoclaw/src/blueprint/runner.test.ts
• nemoclaw/src/blueprint/runner.ts

[prekshivyas]

PR Review Advisor (Nemotron Ultra) — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-T1 Add or justify test follow-up: Runtime validation

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Runtime validation — Add or identify targeted runtime/integration validation for the changed behavior; do not report external E2E job pass/fail here.. Runtime/sandbox/infrastructure paths need behavioral runtime validation: nemoclaw/src/blueprint/runner-test-fixtures.ts, nemoclaw/src/blueprint/runner.ts. Unit tests with mocked execa/openshell cover all logic changes (exit code inspection, error redaction, plan persistence gating). Runtime/sandbox integration validation would test actual OpenShell behavior — out of scope for this PR's explicit unit-test hardening scope per Blueprint apply reports success when provider or inference setup fails #6703.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

E2E Target Results — ✅ All requested tests passed

Run: 29183575668
Workflow ref: tmp/e2e/pr-6704-a0cf03a3
Requested targets: (default — all supported)
Requested test IDs: inference-routing,sandbox-operations,credential-sanitization
Summary: 3 passed, 0 failed, 0 cancelled, 0 skipped, 0 unknown

Test	Result
credential-sanitization	✅ success
inference-routing	✅ success
sandbox-operations	✅ success