refactor(rebuild): lower cognitive complexity ratchet to 186

[cv]

Summary

Continue the stacked cognitive-complexity ratchet by lowering the threshold from 215 to 186. This extracts rebuild credential and messaging preflight handling out of the main rebuild flow.

Changes

• Lowered complexity/noExcessiveCognitiveComplexity from 215 to 186 in biome.json.
• Extracted rebuild credential preflight into preflightRebuildCredentials.
• Extracted messaging manifest preflight staging into stageRebuildMessagingPlanOrBail.
• Preserved pre-destroy failure behavior so missing credentials or messaging plan failures leave the sandbox 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)

Verification

• npx @biomejs/biome lint --only=complexity/noExcessiveCognitiveComplexity --max-diagnostics=none .

• npm run typecheck:cli

• npx vitest run --project cli test/rebuild-credential-preflight.test.ts test/rebuild-stale-recovery.test.ts src/lib/actions/sandbox/rebuild-gateway-drift.test.ts

• Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes

• Targeted tests pass for changed behavior

• Tests added or updated for new or changed behavior

• Full npm test passes (broad runtime changes only)

• 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)

---

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

Summary by CodeRabbit

Release Notes

• New Features

  • Improved rebuild preflight to more reliably derive and validate rebuild credential requirements and provider registration before proceeding.

• Bug Fixes

  • Hardened rebuild safety so failures during messaging manifest staging now halt the process with clear logs, preventing backup/delete or sandbox changes.

• Tests

  • Expanded rebuild credential preflight coverage, including missing OPENAI_API_KEY, stale registered providers, and mismatched local session scenarios.

• Chores

  • Tightened linter complexity threshold to reduce overly complex logic.

[copy-pr-bot]

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.

[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: 8c5f3002-2db3-40ac-865d-55333ce78c65

📥 Commits

Reviewing files that changed from the base of the PR and between 1d8239e and 804dc16.

📒 Files selected for processing (2)

• src/lib/actions/sandbox/rebuild.ts
• test/rebuild-credential-preflight.test.ts

---

📝 Walkthrough

Walkthrough

Refactors rebuildSandbox by extracting inline credential preflight and messaging-plan staging into two standalone helpers: preflightRebuildCredentials and stageRebuildMessagingPlanOrBail. Adds an isLocalInferenceProvider type guard. Wires both helpers into Step 0 and tightens the biome.json cognitive complexity threshold to 186. Adds unit and integration tests for both new boundaries.

Changes

Rebuild preflight & messaging-plan staging refactor

Layer / File(s)	Summary
Helper functions: isLocalInferenceProvider, stageRebuildMessagingPlanOrBail, preflightRebuildCredentials src/lib/actions/sandbox/rebuild.ts	Adds isLocalInferenceProvider type guard used by getRebuildCredentialEnvFromRegistry. Introduces stageRebuildMessagingPlanOrBail as an error boundary that bails on plan staging failure. Extracts preflightRebuildCredentials to consolidate credential-env selection from onboard session vs. registry, legacy GH#2519 OPENAI_API_KEY clearing for local providers, Hermes preflight, and gateway existence checks.
rebuildSandbox Step 0 wiring and complexity threshold src/lib/actions/sandbox/rebuild.ts, biome.json	Replaces the prior inline credential and messaging-plan staging block in Step 0 with calls to preflightRebuildCredentials and stageRebuildMessagingPlanOrBail. Reduces maxAllowedComplexity from 215 to 186 reflecting the complexity reduction.
Unit test harness: messagingRebuildPlanSpy and messaging plan staging failure src/lib/actions/sandbox/rebuild-flow.test.ts	Extends RebuildFlowOverrides and RebuildFlowHarness with a buildMessagingRebuildPlan hook and messagingRebuildPlanSpy spy on MessagingWorkflowPlanner.prototype.buildRebuildPlanFromSandboxEntry. Loads the messaging module in the harness. Adds a test case asserting rebuildSandbox rejects with the thrown error when staging fails, logs appropriate messages, and avoids backup, deletion, and onboarding side effects.
Integration test fixture and credential preflight scenarios test/rebuild-credential-preflight.test.ts	Extends createFixture with a registeredProviders option to control which providers the fake OpenShell gateway reports. Updates the fake openshell provider get handler to conditionally succeed when the requested provider is in the list. Adds three test cases verifying openai-api rebuild preflight boundaries: session missing credentialEnv, matching session with stale provider, and mismatched sandbox/provider without GH#2519 legacy bypass. Each asserts preflight failure with "requires OPENAI_API_KEY", no destructive actions, and sandbox remains registered.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• NVIDIA/NemoClaw#5427: Directly modifies the same rebuildSandbox preflight control flow in rebuild.ts by extracting helper functions, and also adjusts biome.json cognitive-complexity thresholds.
• NVIDIA/NemoClaw#5426: Refactors rebuildSandbox into helper functions to reduce cognitive complexity (multi-agent guard extraction) and tightens the same biome.json complexity threshold.
• NVIDIA/NemoClaw#5471: Modifies rebuild-flow.test.ts to extend the rebuildSandbox flow harness and add failure/success scenario assertions, directly overlapping with the test harness changes in this PR.

Suggested labels

refactor

🐇 Two helpers spring from the old tangled vine,
Credential preflight now walks a cleaner line.
Messaging plans get their own bail-out door,
Complexity shrinks—186, not 215 more.
Tests guard the stale session, the missing key,
This rabbit hops happy, the code runs free! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 accurately reflects the main change: lowering the cognitive complexity threshold from 215 to 186 in biome.json, which is the primary driver for the refactoring work.
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 ratchet/186-rebuild-credential-preflight

---

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

[github-actions]

E2E Advisor Recommendation

Required E2E: sandbox-rebuild-vitest, channels-add-remove-vitest
Optional E2E: rebuild-openclaw-vitest, messaging-providers-vitest

Dispatch hint: sandbox-rebuild-vitest,channels-add-remove-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• sandbox-rebuild-vitest (medium): Primary live rebuild lifecycle coverage for this change: real Docker/OpenShell onboard, nemoclaw <sandbox> rebuild --yes, state preservation, registry refresh, and backup credential hygiene. This directly exercises the refactored rebuild flow around backup/delete/recreate/restore.
• channels-add-remove-vitest (medium): Required because rebuild.ts now stages and reapplies messaging manifest plans. This live OpenClaw channel flow covers channels add, rebuild, gateway credential reuse, policy-list, runtime config persistence, channels remove, and rebuild cleanup across the real Docker/OpenShell boundary.

Optional E2E

• rebuild-openclaw-vitest (high): Useful broader confidence for old OpenClaw sandbox rebuilds, policy reapplication, gateway token rotation, and backup hygiene. It overlaps the rebuild lifecycle changes but is longer and less directly focused than sandbox-rebuild-vitest.
• messaging-providers-vitest (high): Optional broader messaging confidence: exercises provider creation, credential isolation, policy, and WhatsApp/channel rebuild behavior. Run if the PR owner wants extra coverage for messaging manifest staging beyond channels-add-remove-vitest.

New E2E recommendations

• rebuild credential preflight (medium): Existing live E2E coverage exercises successful rebuilds with credentials or gateway providers, but the newly added fail-closed cases for matching/mismatched stale sessions and missing remote-provider credentials are currently covered by non-E2E tests only. A live or hermetic workflow-dispatched E2E should assert rebuild aborts before backup/delete and preserves the registry/sandbox when the target registry provider requires a missing credential.
  • Suggested test: Add a rebuild-credential-preflight E2E scenario/job that creates a sandbox registry/session fixture for a remote provider with no host env and no gateway provider, runs nemoclaw <sandbox> rebuild --yes, and verifies nonzero exit, untouched sandbox/registry, and no backup/delete output.

Dispatch hint

• Workflow: .github/workflows/e2e-vitest-scenarios.yaml
• jobs input: sandbox-rebuild-vitest,channels-add-remove-vitest

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: sandbox-rebuild-vitest
Optional Vitest E2E scenarios: rebuild-openclaw-vitest

Dispatch required Vitest E2E scenarios:

• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=sandbox-rebuild-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• sandbox-rebuild-vitest: The PR changes the live sandbox rebuild implementation, including credential preflight resolution and messaging manifest staging before destructive backup/delete work. The sandbox-rebuild Vitest job is the smallest wired live E2E job that runs a real nemoclaw <sandbox> rebuild --yes flow and validates rebuild state preservation, registry refresh, and backup credential hygiene.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=sandbox-rebuild-vitest

Optional Vitest E2E scenarios

• rebuild-openclaw-vitest: Optional adjacent rebuild coverage for the older OpenClaw rebuild path, including real OpenShell sandbox creation, state preservation, and gateway token rotation. Useful if reviewers want broader confidence around rebuild refactoring beyond the current-version sandbox rebuild lane.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=rebuild-openclaw-vitest

Relevant changed files

• src/lib/actions/sandbox/rebuild.ts

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the ratchet/186-rebuild-... 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	ratchet/186-rebuild-... 804dc16	+/-
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 ratchet/186-rebuild-... 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	ratchet/186-rebuild-... 804dc16	+/-
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	—	62%	—
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 22:23 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

PR Review Advisor

Findings: 0 needs attention, 1 worth checking, 1 nice ideas
Since last review: 1 prior item resolved, 0 still apply, 1 new item found

Review findings

🛠️ Needs attention

• None.

🔎 Worth checking

• Source-of-truth review needed: Messaging manifest staging failure boundary: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `stageRebuildMessagingPlanOrBail(...)` catches staging errors and bails with `Sandbox is untouched`; `src/lib/actions/sandbox/rebuild-flow.test.ts` exercises the path through a prototype spy rather than persisted invalid state.

🌱 Nice ideas

• Add spawned coverage for real messaging-plan staging failures (src/lib/actions/sandbox/rebuild-flow.test.ts:174): The new messaging preflight wrapper correctly catches planner failures before backup/delete, but the regression test triggers that path by monkeypatching `MessagingWorkflowPlanner.prototype.buildRebuildPlanFromSandboxEntry`. That verifies control flow, but it does not exercise the durable-state boundary called out in the production comment: invalid persisted registry messaging plans or agent manifests from prior onboarding.
  • Recommendation: Add or identify a spawned rebuild test that writes a persisted invalid registry messaging plan or invalid agent manifest, lets the real messaging planner fail, and asserts `messaging manifest plan could not be staged`, `Sandbox is untouched`, no backup/delete/onboard, and registry retention.
  • Evidence: The production helper catches `stageMessagingManifestPlanForRebuild(...)` failures before destructive work in `src/lib/actions/sandbox/rebuild.ts`, while the added test uses `.spyOn(messaging.MessagingWorkflowPlanner.prototype, "buildRebuildPlanFromSandboxEntry")` to throw instead of exercising a real persisted invalid plan.

Consider writing more tests for

• **Runtime validation** — Spawn rebuild with a persisted invalid registry messaging plan or invalid agent manifest that makes the real messaging planner fail, and assert `messaging manifest plan could not be staged`, `Sandbox is untouched`, no backup/delete/onboard, and registry retention.. This PR changes sandbox lifecycle and credential preflight behavior. The added spawned credential tests are strong and cover the prior bypass; messaging staging failure coverage is currently mocked and would benefit from one real persisted-state runtime test.
• **Runtime validation** — For future hardening, spawn rebuild with registry provider `openai-api`, matching session `credentialEnv: NVIDIA_INFERENCE_API_KEY`, no `OPENAI_API_KEY`, and no `openai-api` gateway provider, then verify rebuild requires the target OpenAI credential rather than trusting a stale non-null session credential env.. This PR changes sandbox lifecycle and credential preflight behavior. The added spawned credential tests are strong and cover the prior bypass; messaging staging failure coverage is currently mocked and would benefit from one real persisted-state runtime test.
• **Add spawned coverage for real messaging-plan staging failures** — Add or identify a spawned rebuild test that writes a persisted invalid registry messaging plan or invalid agent manifest, lets the real messaging planner fail, and asserts `messaging manifest plan could not be staged`, `Sandbox is untouched`, no backup/delete/onboard, and registry retention.
• **Messaging manifest staging failure boundary** — Current coverage monkeypatches `MessagingWorkflowPlanner.prototype.buildRebuildPlanFromSandboxEntry` to throw and verifies no backup/delete/onboard calls. A real persisted invalid-plan spawned test would better prove the source boundary.. `stageRebuildMessagingPlanOrBail(...)` catches staging errors and bails with `Sandbox is untouched`; `src/lib/actions/sandbox/rebuild-flow.test.ts` exercises the path through a prototype spy rather than persisted invalid state.

Since last review details

Current findings:

• Source-of-truth review needed: Messaging manifest staging failure boundary: The advisor marked localized patch analysis as needs_followup.
  • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Evidence: `stageRebuildMessagingPlanOrBail(...)` catches staging errors and bails with `Sandbox is untouched`; `src/lib/actions/sandbox/rebuild-flow.test.ts` exercises the path through a prototype spy rather than persisted invalid state.
• Add spawned coverage for real messaging-plan staging failures (src/lib/actions/sandbox/rebuild-flow.test.ts:174): The new messaging preflight wrapper correctly catches planner failures before backup/delete, but the regression test triggers that path by monkeypatching `MessagingWorkflowPlanner.prototype.buildRebuildPlanFromSandboxEntry`. That verifies control flow, but it does not exercise the durable-state boundary called out in the production comment: invalid persisted registry messaging plans or agent manifests from prior onboarding.
  • Recommendation: Add or identify a spawned rebuild test that writes a persisted invalid registry messaging plan or invalid agent manifest, lets the real messaging planner fail, and asserts `messaging manifest plan could not be staged`, `Sandbox is untouched`, no backup/delete/onboard, and registry retention.
  • Evidence: The production helper catches `stageMessagingManifestPlanForRebuild(...)` failures before destructive work in `src/lib/actions/sandbox/rebuild.ts`, while the added test uses `.spyOn(messaging.MessagingWorkflowPlanner.prototype, "buildRebuildPlanFromSandboxEntry")` to throw instead of exercising a real persisted invalid plan.

Workflow run details

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

[copy-pr-bot]

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.

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/actions/sandbox/rebuild.ts (1)

623-629: Run the channels stop/start rebuild E2E for this Step 0 extraction.

Given this path now stages messaging rebuild plans before destroy, it’s worth running the targeted lifecycle E2E to confirm disabled-channel persistence and restart reattachment behavior across rebuild.

gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=channels-stop-start-e2e

As per coding guidelines, changes in src/lib/actions/sandbox/rebuild.ts should be validated with channels-stop-start-e2e for stop/start lifecycle across rebuild.

🤖 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 `@src/lib/actions/sandbox/rebuild.ts` around lines 623 - 629, Validate the
messaging rebuild plan staging changes by running the channels-stop-start-e2e
test. Execute the provided gh workflow command with the channels-stop-start-e2e
job to confirm that disabled-channel persistence and restart reattachment
behavior work correctly across the rebuild lifecycle when using the
stageRebuildMessagingPlanOrBail function in the rebuild.ts file. This E2E
validation is required per coding guidelines for changes affecting the sandbox
rebuild and messaging plan staging logic.

🤖 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 `@src/lib/actions/sandbox/rebuild.ts`:
- Around line 623-629: Validate the messaging rebuild plan staging changes by
running the channels-stop-start-e2e test. Execute the provided gh workflow
command with the channels-stop-start-e2e job to confirm that disabled-channel
persistence and restart reattachment behavior work correctly across the rebuild
lifecycle when using the stageRebuildMessagingPlanOrBail function in the
rebuild.ts file. This E2E validation is required per coding guidelines for
changes affecting the sandbox rebuild and messaging plan staging logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3eb27a9a-ab6f-4efb-880b-b432395f3a19

📥 Commits

Reviewing files that changed from the base of the PR and between c70bd4b and 8eca2e2.

📒 Files selected for processing (4)

• biome.json
• src/lib/actions/sandbox/rebuild-flow.test.ts
• src/lib/actions/sandbox/rebuild.ts
• test/rebuild-credential-preflight.test.ts