fix(rebuild,mcp): surface errors and recover prepared destroys - #6377
Merged
Conversation
…covery Two related defects reported together (#6376): FACET A — rebuild aborts silently on a stuck MCP destroy transaction. `createRebuildCommandContext`'s non-throw bail was `(_message, code = 1) => process.exit(code)` — the message argument was discarded. Every `bail(msg, code)` call in the rebuild pipeline therefore exited without printing anything, so an actionable reason (e.g. "Failed to preserve MCP bridges before rebuild: Sandbox 'X' has an incomplete MCP destroy transaction. Re-run the sandbox destroy command …") produced `Deleting old sandbox... EXIT:1` with no diagnosis at all. Emit the message on stderr with the two-space rebuild-diagnostic prefix before exit, so `$?`-gated automation and interactive users see WHY rebuild aborted. Empty messages still exit silently to stay backward-compatible. FACET B — no non-destructive recovery for the stuck state. `mcp remove <server> --force` ran `assertMcpDestroyNotPending` before its `--force` branch, so the same guard fired and the only advertised recovery was `nemoclaw <sandbox> destroy` (full sandbox destruction). Add a new `clearMcpDestroyMarkers(sandboxName)` helper in `mcp-bridge-state.ts` that clears the `destroyPreparedAt` / `destroyPendingAt` markers in place without touching bridge state, provider records, adapters, or policy — the marker file is the only mutable state the guard reads. Wire it into `removeMcpBridgeUnlocked`: when `--force` is set and markers are present, clear them, log the recovery, re-fetch the sandbox, then run the normal removal flow (which may now find the server already gone and no-op, or continue with the actual removal). Also update the guard's error text to name the new recovery path so users can find it without reading the source. Regression coverage: - `src/lib/actions/sandbox/rebuild-preflight-confirmation.test.ts` (3 new cases) — non-throw bail prints the message before exit, empty-message backward-compat, throw-on-error path stays lossless. - `test/mcp-bridge-destroy-marker-recovery.test.ts` (new file, 5 cases) — `clearMcpDestroyMarkers` clears both markers, is a no-op when none set, handles either marker set alone; `removeMcpBridge(--force)` clears markers end-to-end and logs; `removeMcpBridge` WITHOUT `--force` still refuses (safety unchanged). Fixes #6376 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds phase-aware MCP destroy marker recovery, enables ChangesMCP Destroy Marker Recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 0 items to resolve/justify, 4 in-scope improvements
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/lib/actions/sandbox/rebuild-preflight-confirmation.ts`:
- Around line 56-58: The bail path in rebuild-preflight-confirmation currently
prints the raw failure message to stderr, bypassing the redaction used by the
existing log helper. Update the anonymous bail function so it applies the same
redact(message) handling before console.error, matching the behavior of log and
avoiding leaks from arbitrary errors bubbling through the rebuild pipeline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0201925b-1f31-46da-a0ed-de3585e80752
📒 Files selected for processing (5)
src/lib/actions/sandbox/mcp-bridge-remove.tssrc/lib/actions/sandbox/mcp-bridge-state.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.test.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.tstest/mcp-bridge-destroy-marker-recovery.test.ts
CI's ci-static-checks action runs `prek --stage pre-commit` in non-modify mode; Biome format flagged two `vi.spyOn(...).mockImpl(...)` chains and one `runNodeScript` parameter list on the branch for wrap adjustments. Apply the formatter output verbatim (no test logic changed; regression coverage still 15/15 across the two files). Refs #6376 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
cv
requested changes
Jul 7, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Three exact-head blockers remain: destroy markers are cleared before phase-specific cleanup/retry success is proven, so a failed recovery can lose its durable retry state; rebuild reaches the backup/delete path before checking those markers, so recovery is still too late; and the new bail path prints raw lower-level error text instead of passing it through the existing redaction boundary. Make marker clearing phase-aware, move the marker preflight ahead of destructive work, redact the surfaced error, and add failure/retry/redaction tests before rerunning CI and the Advisor.
…r-silent-rebuild-6376
…cted bail (#6376) Address maintainer review (three exact-head blockers) on the #6376 fix: - Phase-aware marker clearing + preserve retry state. `mcp remove --force` recovery now only clears the PREPARED (phase-one) marker — the sandbox still exists — and clears it only AFTER the removal succeeds, so a failed recovery preserves the durable retry marker (setBridgeState keeps it across the removal's own writes). The PENDING (phase-two) marker means OpenShell already deleted the sandbox; clearMcpDestroyMarkers and the guard now refuse it and point at `nemoclaw <name> destroy` instead of silently dropping the still-owed provider/policy cleanup. assertMcpDestroyNotPending gives phase-aware guidance. - Move the marker preflight ahead of destructive work. rebuild ran the only MCP marker check inside the destroy phase, after the backup phase. Add assertMcpDestroyNotPending to runRebuildPreflightPhase, before backup/delete, so a stuck sandbox fails closed before any destructive work. - Redact the surfaced bail error. createRebuildCommandContext's stderr bail now routes the message through the same `redact` boundary `log` uses, so a bailed rebuild cannot be the one path that leaks a URL/token. Tests: phase-aware clear/refuse, --force recovery clears only after success, failure preserves the prepared marker (retry state), rebuild preflight refuses before backup, and bail-message redaction. Existing lifecycle test updated for the phase-aware guard message. Refs #6376 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@test/mcp-destroy-lifecycle.test.ts`:
- Around line 435-466: The two restore lifecycle tests are only checking that
GITHUB_TOKEN still exists, which allows the tests to pass even if the secret
value was changed. Update the assertions in the test cases around
restoreMcpBridgesAfterDestroyAbort and restoreMcpBridgesAfterRebuild to verify
process.env.GITHUB_TOKEN still equals the original
ambient-value-that-must-not-rotate after restore. Keep the existing presence
check if desired, but add a value assertion so the tests actually prove the
credential was not rotated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ada6cb20-d73d-4cb2-81cd-34f4fadb31a3
📒 Files selected for processing (7)
src/lib/actions/sandbox/mcp-bridge-remove.tssrc/lib/actions/sandbox/mcp-bridge-state.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.test.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.tssrc/lib/actions/sandbox/rebuild-preflight-phase.tstest/mcp-bridge-destroy-marker-recovery.test.tstest/mcp-destroy-lifecycle.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/rebuild-preflight-confirmation.ts
- src/lib/actions/sandbox/rebuild-preflight-confirmation.test.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@test/mcp-destroy-lifecycle.test.ts`:
- Around line 435-466: The two restore lifecycle tests are only checking that
GITHUB_TOKEN still exists, which allows the tests to pass even if the secret
value was changed. Update the assertions in the test cases around
restoreMcpBridgesAfterDestroyAbort and restoreMcpBridgesAfterRebuild to verify
process.env.GITHUB_TOKEN still equals the original
ambient-value-that-must-not-rotate after restore. Keep the existing presence
check if desired, but add a value assertion so the tests actually prove the
credential was not rotated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ada6cb20-d73d-4cb2-81cd-34f4fadb31a3
📒 Files selected for processing (7)
src/lib/actions/sandbox/mcp-bridge-remove.tssrc/lib/actions/sandbox/mcp-bridge-state.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.test.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.tssrc/lib/actions/sandbox/rebuild-preflight-phase.tstest/mcp-bridge-destroy-marker-recovery.test.tstest/mcp-destroy-lifecycle.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/sandbox/rebuild-preflight-confirmation.ts
- src/lib/actions/sandbox/rebuild-preflight-confirmation.test.ts
🛑 Comments failed to post (1)
test/mcp-destroy-lifecycle.test.ts (1)
435-466: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test titles claim "without rotating" a secret/credential, but the assertions don't check the value.
Both tests set
process.env.GITHUB_TOKEN = "ambient-value-that-must-not-rotate"to prove the ambient credential isn't rotated during restore, but:
- Line 451 only checks
Object.hasOwn(process.env, "GITHUB_TOKEN")— this passes even if the value were overwritten.- Lines 525-542 have no
GITHUB_TOKENassertion at all.Neither test actually exercises its stated claim. As per path instructions, flag "conditionals that make a test pass without exercising its claim."
🧪 Proposed fix to assert the value, not just presence
- expect(Object.hasOwn(process.env, "GITHUB_TOKEN")).toBe(true); + expect(process.env.GITHUB_TOKEN).toBe("ambient-value-that-must-not-rotate");And for the second test (around line 536-541), add an equivalent assertion after
restoreMcpBridgesAfterRebuild:await bridge.restoreMcpBridgesAfterRebuild("alpha", [bridgeEntries.github]); + expect(process.env.GITHUB_TOKEN).toBe("ambient-value-that-must-not-rotate"); expect(testState.calls.some((call) => /^provider (create|update) /.test(call))).toBe(false);Also applies to: 525-542
🤖 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 `@test/mcp-destroy-lifecycle.test.ts` around lines 435 - 466, The two restore lifecycle tests are only checking that GITHUB_TOKEN still exists, which allows the tests to pass even if the secret value was changed. Update the assertions in the test cases around restoreMcpBridgesAfterDestroyAbort and restoreMcpBridgesAfterRebuild to verify process.env.GITHUB_TOKEN still equals the original ambient-value-that-must-not-rotate after restore. Keep the existing presence check if desired, but add a value assertion so the tests actually prove the credential was not rotated.Source: Path instructions
The failure/retry case injected its failure through OpenShell provider inspection, which behaves differently in CI (no live gateway) and crashed the child process. Inject the failure deterministically instead: stub ensureSandboxGatewaySelected to throw and use a provider-less bridge so the removal fails at gateway selection — before any OpenShell work — and the prepared marker must still be preserved. Refs #6376 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
Address Review Advisor required fixes on the #6376 recovery: - PRA-2: removeMcpBridgeUnlocked now returns a discriminated outcome (removedTarget / cancelledPreparedAdd / markerOnlyNoEntries / noMatchingEntry / residualPreserved), and removeMcpBridge clears the phase-one destroy marker ONLY for a proven recovery. A wrong-server `--force` no-op (other entries remain) and an allowResidual residual cleanup no longer drop the durable retry marker. - PRA-4: validateSandboxName at the top of clearMcpDestroyMarkers before any registry read/update. - PRA-1/PRA-3: document the intentionally narrow product contract as a structured annotation (invalidState / sourceBoundary / sourceFixConstraint / regressionTest / removalCondition): prepared-only markers are recoverable with `mcp remove --force`; pending/both-marker state must finish `nemoclaw <name> destroy`. Tests: add a wrong-server `--force` no-op case that must preserve the prepared marker. Refs #6376 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
wscurran
added
v0.0.77
area: cli
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
changed the title
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6377.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28900700830
|
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
cv
approved these changes
Jul 8, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed exact head d1881e0. The prior blockers are addressed: destroy-marker clearing is phase-aware and occurs only after a proven residual-free recovery with an empty bridge manifest; rebuild refuses markers in preflight before backup/deletion; surfaced bail diagnostics pass through the redaction boundary. Pending/both-marker state remains fail-closed and exact provider/policy ownership checks are preserved. Focused lifecycle, retry, residual, multi-bridge, credential-preservation, and redaction coverage passes; required CI and the trusted primary Advisor are green. The proposed focused live interrupted-marker and explicit concurrency regressions are worthwhile follow-ups but are non-blocking because the current paths share the lifecycle lock and required live MCP/rebuild lanes pass.
Closed
21 tasks
21 tasks
cv
pushed a commit
that referenced
this pull request
Jul 9, 2026
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [#3787](#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [#4960](#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [#5676](#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [#5857](#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [#5929](#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [#6068](#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [#6116](#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [#6122](#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [#6211](#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [#6283](#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [#6293](#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [#6320](#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [#6377](#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [#6412](#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [#6421](#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [#6431](#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [#6439](#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [#6450](#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [#6474](#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [#6475](#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [#6480](#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [#6481](#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [#6482](#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [#6486](#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [#6490](#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [#6494](#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [#6497](#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [#6506](#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [#6508](#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests are not applicable to this documentation-only change set. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…A#6377) <!-- markdownlint-disable MD041 --> ## Summary This PR makes rebuild surface redacted MCP destroy diagnostics before backup or deletion and adds a non-destructive recovery path for prepared-only destroy transactions. It deliberately keeps pending or both-marker transactions fail closed because `destroyPendingAt` records confirmed OpenShell deletion while provider or policy cleanup may still be owed. ## Related Issue Addresses NVIDIA#6376. The issue's literal reproduction manually injects both markers into a live sandbox. That is not a valid lifecycle transition. When `destroyPendingAt` is present, pending takes precedence and recovery must finish through `nemoclaw <name> destroy` rather than erase durable retry state. ## Recovery Contract | Registry state | Meaning | Recovery | | --- | --- | --- | | No destroy marker | No durable destroy transaction is active. | Normal MCP commands and rebuild proceed. | | `destroyPreparedAt` only | Adapter scrub and provider detach completed, but sandbox deletion is not durably confirmed. | If the sandbox is still live, run `nemoclaw <name> mcp remove <server> --force` for each registered server. The marker clears only after residual-free cleanup drains every bridge entry. | | `destroyPendingAt`, with or without `destroyPreparedAt` | The registry records confirmed OpenShell deletion and retained cleanup may still be owed. | `mcp remove --force` refuses and preserves both markers and the cleanup manifest. Run `nemoclaw <name> destroy` to finish idempotent cleanup. | ## Changes - Print non-throw rebuild bail messages on stderr through the existing redaction boundary. - Check MCP destroy markers during rebuild preflight, before target preflights, backup, or deletion. - Clear a prepared marker only after the requested removal succeeds without residuals and no bridge entries remain. - Preserve markers and manifests after failures, wrong-server no-ops, tolerated residuals, partial multi-bridge cleanup, and pending or both-marker refusal. - Assert that destroy-abort and rebuild restoration do not rotate the ambient host credential value. - Document the phase-aware recovery contract in the MCP guide and command references. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — marker ordering, pending cleanup ownership, failure preservation, and the prepared-only recovery boundary were reviewed against the durable state transitions and locked with regression tests. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed - [x] Targeted behavior tests pass for the current change set: - `npx vitest run --project integration test/mcp-bridge-destroy-marker-recovery.test.ts test/mcp-destroy-lifecycle.test.ts` — 32/32 - `npx vitest run --project cli src/lib/actions/sandbox/mcp-bridge-*.test.ts` — 106/106 - [ ] Applicable broad gate passed — not run because the change is scoped to MCP lifecycle behavior; the full MCP CLI scope and focused integration suite passed. - [x] Quality Gates section completed with required justifications - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings — completed with 0 errors and 2 existing Fern warnings - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) - [ ] New doc pages include SPDX header and frontmatter — no new pages Additional checks: - `npm run typecheck:cli` - `npm run test:titles:check` - `npm run test-size:check` - Biome check on all touched TypeScript and test files - Generated agent-variant documentation consistency check ## AI Disclosure - [x] AI-assisted — tools: Claude Code and Codex --- Signed-off-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> --------- Signed-off-by: Yanyun Liao <yanyunl@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [NVIDIA#3787](NVIDIA#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [NVIDIA#4960](NVIDIA#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [NVIDIA#5676](NVIDIA#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [NVIDIA#5857](NVIDIA#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [NVIDIA#5929](NVIDIA#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [NVIDIA#6068](NVIDIA#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [NVIDIA#6116](NVIDIA#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [NVIDIA#6122](NVIDIA#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [NVIDIA#6211](NVIDIA#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [NVIDIA#6283](NVIDIA#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [NVIDIA#6293](NVIDIA#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [NVIDIA#6320](NVIDIA#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [NVIDIA#6377](NVIDIA#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [NVIDIA#6412](NVIDIA#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [NVIDIA#6421](NVIDIA#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [NVIDIA#6431](NVIDIA#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [NVIDIA#6439](NVIDIA#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [NVIDIA#6450](NVIDIA#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [NVIDIA#6474](NVIDIA#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [NVIDIA#6475](NVIDIA#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [NVIDIA#6480](NVIDIA#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [NVIDIA#6481](NVIDIA#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [NVIDIA#6482](NVIDIA#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [NVIDIA#6486](NVIDIA#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [NVIDIA#6490](NVIDIA#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [NVIDIA#6494](NVIDIA#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [NVIDIA#6497](NVIDIA#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [NVIDIA#6506](NVIDIA#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [NVIDIA#6508](NVIDIA#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests are not applicable to this documentation-only change set. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR makes rebuild surface redacted MCP destroy diagnostics before backup or deletion and adds a non-destructive recovery path for prepared-only destroy transactions.
It deliberately keeps pending or both-marker transactions fail closed because
destroyPendingAtrecords confirmed OpenShell deletion while provider or policy cleanup may still be owed.Related Issue
Addresses #6376.
The issue's literal reproduction manually injects both markers into a live sandbox. That is not a valid lifecycle transition. When
destroyPendingAtis present, pending takes precedence and recovery must finish throughnemoclaw <name> destroyrather than erase durable retry state.Recovery Contract
destroyPreparedAtonlynemoclaw <name> mcp remove <server> --forcefor each registered server. The marker clears only after residual-free cleanup drains every bridge entry.destroyPendingAt, with or withoutdestroyPreparedAtmcp remove --forcerefuses and preserves both markers and the cleanup manifest. Runnemoclaw <name> destroyto finish idempotent cleanup.Changes
Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passednpx vitest run --project integration test/mcp-bridge-destroy-marker-recovery.test.ts test/mcp-destroy-lifecycle.test.ts— 32/32npx vitest run --project cli src/lib/actions/sandbox/mcp-bridge-*.test.ts— 106/106npm run docsbuilds without warnings — completed with 0 errors and 2 existing Fern warningsAdditional checks:
npm run typecheck:clinpm run test:titles:checknpm run test-size:checkAI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com