Skip to content

refactor(rebuild): lower cognitive complexity ratchet to 243 - #5422

Merged
cv merged 21 commits into
mainfrom
ratchet/243-rebuild-confirm-cognitive-complexity
Jun 15, 2026
Merged

refactor(rebuild): lower cognitive complexity ratchet to 243#5422
cv merged 21 commits into
mainfrom
ratchet/243-rebuild-confirm-cognitive-complexity

Conversation

@cv

@cv cv commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Continue the stacked cognitive-complexity ratchet by lowering the threshold from 244 to 243. This extracts rebuild confirmation prompting into a helper while keeping behavior unchanged.

Changes

  • Lowered complexity/noExcessiveCognitiveComplexity from 244 to 243 in biome.json.
  • Extracted rebuild confirmation and active-session warning output into confirmSandboxRebuildIfNeeded.
  • Added focused coverage for interactive rebuild cancellation and affirmative prompt handling.
  • Kept rebuild cancellation and --yes/--force behavior equivalent while reducing rebuildSandbox complexity.

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

  • 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

  • Tests

    • Added test coverage for rebuild confirmation scenarios, including negative responses and formatted input variations.
  • Chores

    • Refactored rebuild confirmation logic structure.
    • Updated linter configuration with stricter code quality rules.

cv added 3 commits June 14, 2026 11:27
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
(cherry picked from commit 41fcd6a)
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this Jun 14, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jun 14, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: c4053c59-d77f-4a60-829c-0d8f25e39eb7

📥 Commits

Reviewing files that changed from the base of the PR and between a57d508 and 94402c3.

📒 Files selected for processing (3)
  • biome.json
  • src/lib/actions/sandbox/rebuild.ts
  • test/rebuild-credential-preflight.test.ts

📝 Walkthrough

Walkthrough

Extracts the interactive rebuild confirmation prompt from rebuildSandbox into a new async helper confirmSandboxRebuildIfNeeded. The call site is updated with an early-return guard. Two new tests cover cancel and case-insensitive affirmative inputs. The Biome maxAllowedComplexity threshold is reduced from 244 to 243.

Changes

Rebuild Confirmation Refactor

Layer / File(s) Summary
confirmSandboxRebuildIfNeeded helper and call site
src/lib/actions/sandbox/rebuild.ts
New async helper encapsulates active-session warnings and the Proceed? [y/N] prompt, returning a boolean; rebuildSandbox calls it and exits early when the result is false.
Confirmation tests
test/rebuild-credential-preflight.test.ts
Two new Vitest cases assert that a negative reply cancels before credential preflight and backup, and that a trimmed uppercase YES input reaches the backup step.
Biome complexity threshold
biome.json
maxAllowedComplexity for noExcessiveCognitiveComplexity reduced from 244 to 243 to match the extracted helper's lower complexity.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5421: Companion refactor that centralizes active-session detection for the same rebuildSandbox confirmation prompt, directly related to this helper extraction.
  • NVIDIA/NemoClaw#5419: Also modifies biome.json complexity.noExcessiveCognitiveComplexity.maxAllowedComplexity, targeting the same linter rule.
  • NVIDIA/NemoClaw#5420: Touches the same Biome maxAllowedComplexity setting in biome.json.

Poem

🐇 A helper was born in the rebuild domain,
To ask "Proceed? [y/N]" and check what you'd say.
With YES trimmed and lowered, the backup proceeds,
While "n" stops the chaos before preflight bleeds.
Complexity drops by one — what a tidy day! 🎉

🚥 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 accurately describes the primary change: lowering the cognitive complexity threshold in biome.json from 244 to 243, which is the main objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/243-rebuild-confirm-cognitive-complexity

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

@github-code-quality

github-code-quality Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the ratchet/243-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/243-rebuild-... 94402c3 +/-
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/243-rebuild-... branch is 44%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main ratchet/243-rebuild-... 94402c3 +/-
src/lib/state/o...oard-session.ts 90%
src/lib/inference/local.ts 78%
src/lib/sandbox/config.ts 72%
src/lib/inference/nim.ts 72%
src/lib/onboard/preflight.ts 64%
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 19:53 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: sandbox-rebuild-vitest
Optional E2E: rebuild-openclaw-vitest, rebuild-hermes-e2e

Dispatch hint: sandbox-rebuild-vitest

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • sandbox-rebuild-vitest (high): Most targeted existing live E2E for the changed rebuild lifecycle path. It onboards a real sandbox, runs nemoclaw <sandbox> rebuild --yes, and verifies state preservation, registry refresh, and backup credential hygiene.

Optional E2E

  • rebuild-openclaw-vitest (high): Broader OpenClaw-specific rebuild coverage for the same source file. It exercises rebuild against an older OpenClaw sandbox, verifies workspace and policy preservation, gateway token rotation, and post-rebuild inference.
  • rebuild-hermes-e2e (high): Useful adjacent confidence because the changed rebuild action is shared by Hermes rebuild flows and contains Hermes credential preflight logic, but the PR change itself is not Hermes-specific.

New E2E recommendations

  • interactive sandbox rebuild confirmation (medium): Existing live rebuild E2Es use --yes, so they do not validate the changed interactive prompt path. The PR adds unit coverage with fakes, but there is no existing live E2E that proves answering n cancels before backup/delete/recreate or that answering yes continues in an interactive terminal.
    • Suggested test: Add a live or hermetic E2E scenario for nemoclaw <sandbox> rebuild without --yes that sends non-affirmative input, asserts Cancelled., and verifies no sandbox backup/delete/recreate occurred; include the active SSH session warning case if feasible.

Dispatch hint

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

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

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 real sandbox rebuild implementation and its confirmation/preflight ordering. The sandbox rebuild free-standing Vitest job exercises nemoclaw <sandbox> rebuild --yes through the live Docker/OpenShell boundary and verifies rebuild state preservation, registry refresh, and backup 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: Adjacent rebuild coverage for an old OpenClaw sandbox can provide additional confidence that the rebuild path still preserves OpenClaw state and rotates gateway token state after the confirmation refactor, but it is heavier than the primary sandbox rebuild job.
    • 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-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Consider writing more tests for
  • **Runtime validation** — Assert an interactive affirmative rebuild with fake OpenShell, Docker, SSH, and a saved credential exits with status 0 and reaches the expected recreate/restore or recovery boundary, not only the `Backing up sandbox state` message.. The changed code sits in destructive sandbox rebuild lifecycle logic, so spawned/runtime-style validation is appropriate. The PR adds useful direct CLI coverage with fake OpenShell, Docker, SSH, and process-list boundaries; a few behavior-specific checks would further improve confidence.
  • **Runtime validation** — Cover interactive rebuild prompt output when exactly one active SSH session is detected, including singular wording such as `1 connection`, `session`, and `terminate the active session`.. The changed code sits in destructive sandbox rebuild lifecycle logic, so spawned/runtime-style validation is appropriate. The PR adds useful direct CLI coverage with fake OpenShell, Docker, SSH, and process-list boundaries; a few behavior-specific checks would further improve confidence.
  • **Runtime validation** — Assert `rebuild --force` skips `Proceed? [y/N]:` and propagates recreate auto-yes behavior the same way as `--yes`.. The changed code sits in destructive sandbox rebuild lifecycle logic, so spawned/runtime-style validation is appropriate. The PR adds useful direct CLI coverage with fake OpenShell, Docker, SSH, and process-list boundaries; a few behavior-specific checks would further improve confidence.

Workflow run details

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

cv added 15 commits June 14, 2026 16:45
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…ognitive-complexity

# Conflicts:
#	biome.json
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv marked this pull request as ready for review June 15, 2026 18:53
cv added 2 commits June 15, 2026 12:16
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
…243-rebuild-confirm-cognitive-complexity

# Conflicts:
#	test/rebuild-credential-preflight.test.ts
Base automatically changed from ratchet/244-rebuild-cognitive-complexity to main June 15, 2026 19:46
…onfirm-cognitive-complexity

# Conflicts:
#	biome.json
#	src/lib/actions/sandbox/rebuild.ts
#	test/rebuild-credential-preflight.test.ts
@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

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.

@cv
cv merged commit 4840162 into main Jun 15, 2026
37 checks passed
@cv
cv deleted the ratchet/243-rebuild-confirm-cognitive-complexity branch June 15, 2026 20:28
@cv cv added the v0.0.65 label Jun 15, 2026
@wscurran wscurran added area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sandbox OpenShell sandbox lifecycle, runtime, config, or recovery refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants