fix(rebuild): name the shields-down step before the post-rebuild mcp restart

[Dreamstick9]

Outcome

Before: a rebuild that started with shields up relocked them, then told the user to run nemoclaw <sandbox> mcp restart for an incomplete managed MCP restoration, and that command refused with has shields up or an unreadable shields posture.
After: the same guidance names shields down --timeout 15m --reason "MCP maintenance" before the restart and shields up after it for Hermes and OpenClaw sandboxes, and the Hermes rebuild recovery and managed MCP docs state the same precondition.

Reason

Issue #10751 reports that rebuild re-locks shields and leaves the documented mcp restart recovery step unable to run.
The refusal is the documented contract for mcp add, mcp restart, and mcp remove, and the relock restores the pre-rebuild posture.
The defect is the guidance: rebuild and two docs pages prescribe mcp restart without its shields precondition, so the issue's second expected result applies (the tool tells the user that a shields-down step is needed first).

Related issues

Fixes #10751

Changes

• src/lib/actions/sandbox/rebuild-mcp-phase.ts: printMcpRestoreRecovery takes mcpRestartNeedsShieldsDown and prints the shields-down and shields-up steps after the existing mcp restart line.
• src/lib/actions/sandbox/rebuild-post-restore-phase.ts: new input shieldsUpBeforeRebuild; mcpRestartNeedsShieldsDown is true for Hermes and OpenClaw targets, whose hermes-config and mcporter adapters refuse config mutation while shields are up, and false for Deep Agents, whose adapter does not check shields. All three guidance call sites pass it, including the Hermes cron-gate failure path that prints before the pipeline relock.
• src/lib/actions/sandbox/rebuild-pipeline.ts: forwards rebuildShieldsWindow.wasLocked at both post-restore call sites. A stale or prepared recovery recreate keeps false because the recreated sandbox starts unlocked and already prints the shields up reminder.
• Tests: rebuild-mcp-phase.test.ts covers the guidance lines; rebuild-post-restore-phase.test.ts covers OpenClaw and Hermes rebuilds that restore lockdown, a rebuild that started with shields down, a Deep Agents rebuild, and the cron-gate path ordering; rebuild-flow-recovery.test.ts covers the pipeline pass-through with a locked window.
• Docs: docs/manage-sandboxes/recover-rebuild-sandboxes.mdx (Hermes recovery paragraphs) and docs/manage-sandboxes/manage-mcp-servers.mdx (OpenClaw and Hermes variant block).

The only added mechanism is one boolean phase input; its consumer is the post-restore guidance and the tests above protect it.

Verification

Every command ran on an Ubuntu 26.04 x86_64 host from a worktree at upstream main 19bb9860a plus this change.

• npx vitest run --project cli src/lib/actions/sandbox/rebuild with an isolated HOME — passed, 63 of 63 test files. An earlier run against the host's shared HOME failed 8 tests with Failed to acquire lock on /home/soul/.nemoclaw-portable-host.lock after 120 retries and 5 s timeouts in rebuild-shields-finally.test.ts; that is lock contention on the shared host, not this change.
• npm run typecheck:cli — passed, 0 errors.
• npm run checks:repository — passed (exit 0).
• npx oxfmt --check on rebuild-mcp-phase.ts, rebuild-post-restore-phase.ts, rebuild-pipeline.ts, and rebuild-flow-recovery.test.ts — passed (exit 0).
• npx oxfmt --check on rebuild-mcp-phase.test.ts and rebuild-post-restore-phase.test.ts — fails, and the unchanged upstream main copies of both files fail the same check. Running npx oxfmt on this branch's copies reproduces the fully formatted files byte for byte (md5 1d6936e0… and f9506e77…), so the added lines are formatted and only pre-existing lines differ. The repository hook tools/lint/format-added-files.sh formats added files only, so the pre-existing lines stay as they are on main.
• npx oxlint on the six changed TypeScript files — passed, 0 warnings and 0 errors.
• npm run validate:pr on a throwaway commit with this title — passed (exit 0): the pre-commit stage (repository checks, env-var documentation gate, gitleaks, source-shape test budget, growth guardrails, TypeScript CLI), commitlint, and the pre-push stage.
• npm run docs — passed (exit 0): Fern check found 0 errors, and check-docs-published-routes reported OK for 69 guarded pages.
• The diff contains no secrets, API keys, or credentials.

Review notes

• Unchanged on purpose: the mcp restart refusal while shields are up and the rebuild relock. Both are documented contracts, so the issue's first expected result would be a product decision.
• Sibling path not changed: hermesMcpReconciliationRemediationLines in src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.ts also prescribes mcp restart without the shields precondition from connect and gateway restart. It is outside the rebuild scope of [Ubuntu 26.04][Agent&Skills] rebuild re-locks shields, leaving the documented mcp restart recovery step unable to run #10751 and inside the hunks of open PR refactor(mcp): bind lifecycle to recorded runtime #10815.
• Overlap: open PR refactor(rebuild): preserve recorded runtime target #10817 changes the same rebuild files. This change adds one interface field, one destructured name, one computed constant, and two call-site lines in regions that PR does not modify, except that both PRs add a field to RebuildPostRestorePhaseInput and a line to the second runRebuildPostRestorePhase call.

---

Signed-off-by: Kushagar Garg dreamstick909@gmail.com

Summary by CodeRabbit

• New Features
  • Added clearer recovery guidance for OpenClaw and Hermes sandbox rebuilds when MCP restoration requires temporarily lowering shields.
  • Shields are restored after a successful MCP restart and relocked when appropriate.
• Documentation
  • Updated sandbox management and rebuild recovery instructions with shield management steps and maintenance timing.
• Bug Fixes
  • Improved recovery messaging for MCP and cron restoration failures.
• Tests
  • Expanded coverage for shield states, recovery guidance, and instruction ordering.

[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]

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: 25b06735-9e53-417f-9da6-e9d477f6b8e7

📥 Commits

Reviewing files that changed from the base of the PR and between 19bb986 and 9445f2c.

📒 Files selected for processing (8)

• docs/manage-sandboxes/manage-mcp-servers.mdx
• docs/manage-sandboxes/recover-rebuild-sandboxes.mdx
• src/lib/actions/sandbox/rebuild-flow-recovery.test.ts
• src/lib/actions/sandbox/rebuild-mcp-phase.test.ts
• src/lib/actions/sandbox/rebuild-mcp-phase.ts
• src/lib/actions/sandbox/rebuild-pipeline.ts
• src/lib/actions/sandbox/rebuild-post-restore-phase.test.ts
• src/lib/actions/sandbox/rebuild-post-restore-phase.ts

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

---

📝 Walkthrough

Walkthrough

The rebuild pipeline now tracks the pre-rebuild shield state. Hermes and OpenClaw MCP recovery output includes shield-down and shield-up commands when required. Tests and sandbox documentation cover the updated recovery flow.

Changes

MCP shield recovery

Layer / File(s)	Summary
Shield-aware recovery guidance src/lib/actions/sandbox/rebuild-post-restore-phase.ts, src/lib/actions/sandbox/rebuild-mcp-phase.ts	Post-restore handling identifies shield-gated Hermes and OpenClaw MCP recovery. Recovery output includes commands to lower shields before mcp restart and raise them afterward.
Pre-rebuild shield state wiring src/lib/actions/sandbox/rebuild-pipeline.ts	Both rebuild post-restore paths pass the pre-rebuild shield state to recovery handling.
Recovery validation and operational guidance src/lib/actions/sandbox/*test.ts, docs/manage-sandboxes/*	Tests verify shield-aware recovery output, command ordering, and agent-specific behavior. Documentation describes the required shield transitions.

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

Merge Risk: ⚪ Minimal · up to 9445f

The PR updates rebuild recovery instructions to place shield changes around managed MCP restart while preserving existing enforcement. The change is localized and merge-ready after normal checks, with no actionable merge-blocking risk remaining.

Suggested reviewers: apurvvkumaria, aasthaj

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address issue #10751 by passing the pre-rebuild shields state through the rebuild pipeline and adding conditional shields-down and shields-up guidance before MCP recovery for Hermes and Op…
Out of Scope Changes check	✅ Passed	The changes remain within the rebuild MCP recovery scope. OpenClaw coverage extends the same recovery behavior, Deep Agents behavior remains unchanged, and the documentation and tests support the stat…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding a shields-down step before the post-rebuild MCP restart guidance.

Full details: Linked Issues check

Explanation

The changes address issue #10751 by passing the pre-rebuild shields state through the rebuild pipeline and adding conditional shields-down and shields-up guidance before MCP recovery for Hermes and OpenClaw. Documentation and tests cover the required recovery behavior.

Full details: Out of Scope Changes check

Explanation

The changes remain within the rebuild MCP recovery scope. OpenClaw coverage extends the same recovery behavior, Deep Agents behavior remains unchanged, and the documentation and tests support the stated objective.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

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

All previous runs

[Dreamstick9]

PR #10722 removed Shields from the core. Thus this pull request is not necessary.

Thank you for the review. The approval came approximately 85 minutes before the merge of PR #10722. PR #10722 made this pull request out of date. The review is not the cause.

This pull request adds a shields-down step before the mcp restart command. The file rebuild-mcp-phase.ts on the main branch does not refer to Shields now. The variable rebuildShieldsWindow does not exist now. A merge adds a necessary parameter to the function printMcpRestoreRecovery. No caller supplies this parameter.

PR #10722 also corrects issue #10751.