Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/deployment/set-up-mcp-bridge.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,28 @@ Provider deletion and registry cleanup happen only after OpenShell confirms that
NemoClaw prechecks the recorded provider ID and credential-key shape before mutation and uses a random per-add provider-name suffix to avoid accidental name reuse.
The stable OpenShell limitations section describes why these ownership checks do not form an atomic identity binding.

An interrupted MCP destroy can leave durable transaction state.
A prepared-only transaction means deletion is not durably confirmed.
If the sandbox is still live, recover without destroying it by removing the affected server with `--force`.

```bash
$$nemoclaw my-sandbox mcp remove <server> --force
```

NemoClaw clears the prepared marker only after cleanup succeeds without residuals and no bridge entries remain.
For a sandbox with multiple bridge entries, repeat the command with each registered server name until the manifest is empty.
A failed cleanup, a wrong server name, residual resources, or any remaining bridge entry preserves the marker for another retry.

A pending marker, including a transaction with both prepared and pending markers, means the registry records that OpenShell deletion was already confirmed.
`mcp remove --force` refuses this state because provider or policy cleanup can still be owed.
Finish the idempotent destroy instead.

```bash
$$nemoclaw my-sandbox destroy
```

While either marker remains, `rebuild` refuses during preflight before backup or deletion and prints a redacted diagnostic with the applicable recovery command.

`remove --force` may remove a modified same-name agent adapter entry so an operator can clear local config.
Provider deletion still requires the exact recorded provider ID, type, and credential key, and policy deletion still requires exact owned content; force never claims an unowned or drifted provider or same-key live policy.
If any cleanup step leaves a residual, the command exits nonzero and preserves the managed MCP registry entry so cleanup can be retried.
Expand Down
14 changes: 13 additions & 1 deletion docs/reference/commands.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1855,13 +1855,22 @@ The command fails closed on observed drift.
Residuals preserve registry state.
OpenShell `0.0.72` mutates providers by name, so do not concurrently replace a managed provider through another OpenShell client during this command.

When an interrupted destroy leaves a prepared-only transaction, deletion is not durably confirmed.
If the sandbox is still live, run `$$nemoclaw <name> mcp remove <server> --force` with the affected server name.
NemoClaw clears the prepared marker only after cleanup succeeds without residuals and no bridge entries remain.
A failed cleanup, a wrong server name, residual resources, or any remaining bridge entry preserves the marker for another retry.

A pending marker, including a transaction with both prepared and pending markers, means the registry records that OpenShell deletion was already confirmed.
`mcp remove --force` refuses this state.
Run `$$nemoclaw <name> destroy` to finish the idempotent provider and policy cleanup.

```bash
$$nemoclaw my-assistant mcp remove github [--force]
```

| Flag | Description |
|------|-------------|
| `--force` | Remove same-name adapter config and continue exact-ownership provider/policy cleanup; preserve registry state when residuals remain |
| `--force` | Remove same-name adapter config and continue exact-ownership provider and policy cleanup. For a prepared-only destroy, attempt recovery when the sandbox is still live and clear the marker only after residual-free cleanup drains every bridge entry. |

### `$$nemoclaw <name> skill install <path>`

Expand Down Expand Up @@ -2235,6 +2244,9 @@ Pass `--yes`, `-y`, or `--force` to skip the prompt in scripted workflows.
The sandbox must be running for the backup step to succeed.
If an archive command reports partial output while still producing usable data, `rebuild` keeps the captured backup entries and reports only the manifest-defined paths that could not be archived.
If any required state path still cannot be backed up, `rebuild` exits before destroying the original sandbox.
Before backup or deletion, `rebuild` also refuses an incomplete MCP destroy transaction.
For a prepared-only transaction, the redacted diagnostic points to `$$nemoclaw <name> mcp remove <server> --force` when the sandbox is still live.
For a pending or both-marker transaction, it points to `$$nemoclaw <name> destroy` because the registry records that OpenShell deletion was already confirmed.
Before backup or deletion, rebuild checks the staged messaging configuration for credentials or channel resources already used by another registered sandbox.
A conflict aborts with the original sandbox registered and intact so you can resolve the conflict before retrying.
When rebuild starts with shields up, NemoClaw opens a 30-minute shields-down window for backup and recreation.
Expand Down
90 changes: 81 additions & 9 deletions src/lib/actions/sandbox/mcp-bridge-remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
import {
assertMcpDestroyNotPending,
bridgeState,
clearMcpDestroyMarkers,
ensureSandboxGatewaySelected,
getBridgeAdapter,
getSandboxAgent,
Expand Down Expand Up @@ -84,32 +85,100 @@ function assertExactMcpRemoveProvider(
}
}

// Discriminated outcome of a single `mcp remove` so the caller can prove the
// requested entry was handled. Clearing the transaction marker additionally
// requires that no bridge entries remain: phase one scrubbed/detached every
// bridge, so one successful removal cannot mark a multi-bridge recovery done.
type McpRemovalOutcome =
| "removedTarget" // the requested committed server was cleaned up
| "cancelledPreparedAdd" // an incomplete add for the requested server was cancelled
| "markerOnlyNoEntries" // no bridges remain; a --force removal is a marker-only recovery
| "noMatchingEntry" // bridges remain but not the requested server (no-op / wrong server)
| "residualPreserved"; // force cleanup left residual resources; the entry is preserved

const PROVEN_MCP_RECOVERY_OUTCOMES: ReadonlySet<McpRemovalOutcome> = new Set([
"removedTarget",
"cancelledPreparedAdd",
"markerOnlyNoEntries",
]);

function completedPreparedDestroyRecovery(
sandboxName: string,
outcome: McpRemovalOutcome,
): boolean {
if (!PROVEN_MCP_RECOVERY_OUTCOMES.has(outcome)) return false;
return Object.keys(bridgeState(getSandboxOrThrow(sandboxName))).length === 0;
}

export async function removeMcpBridge(
sandboxName: string,
server: string,
options: { force?: boolean; allowResidual?: boolean } = {},
): Promise<void> {
return withMcpLifecycleLock(sandboxName, () =>
removeMcpBridgeUnlocked(sandboxName, server, options),
);
return withMcpLifecycleLock(sandboxName, async () => {
// #6376: capture the recoverable prepared-destroy phase BEFORE the removal.
const before = getSandboxOrThrow(sandboxName).mcp;
const recoverPreparedDestroy =
!!options.force && !!before?.destroyPreparedAt && !before?.destroyPendingAt;
const outcome = await removeMcpBridgeUnlocked(sandboxName, server, options);
// Clear the phase-one destroy marker only after the requested entry was
// handled AND no bridge entries remain. A failed removal, wrong-server
// no-op, residual-preserving cleanup, or partial multi-bridge recovery must
// retain the durable marker. `setBridgeState` preserves it across each
// removal until this explicit, phase-aware clear.
if (
recoverPreparedDestroy &&
completedPreparedDestroyRecovery(sandboxName, outcome) &&
clearMcpDestroyMarkers(sandboxName)
) {
console.log(
` Cleared incomplete MCP destroy transaction on sandbox '${sandboxName}' (--force).`,
);
}
});
}

async function removeMcpBridgeUnlocked(
sandboxName: string,
server: string,
options: { force?: boolean; allowResidual?: boolean } = {},
): Promise<void> {
): Promise<McpRemovalOutcome> {
validateSandboxName(sandboxName);
validateMcpServerName(server);
const sandbox = getSandboxOrThrow(sandboxName);
assertMcpDestroyNotPending(sandbox);
const entry = bridgeState(sandbox)[server];
// #6376: `--force` on `mcp remove` is the documented non-destructive recovery
// for a stuck MCP destroy transaction. It is PHASE-AWARE: only the prepared
// (phase-one) marker — in-sandbox scrub + provider detach done, deletion not
// durably confirmed — is recoverable here when the sandbox is still live. In
// that case skip the guard and attempt the requested removal; removeMcpBridge
// clears the marker only after the full bridge manifest is drained, so a
// failure or an absent sandbox preserves the durable retry state. Every other
// state still hits the guard:
// - no `--force`: refuse (the guard's normal behavior);
// - the pending (phase-two) marker: the registry records confirmed OpenShell
// deletion, so the guard points at `nemoclaw <name> destroy`; clearing that
// marker would abandon still-owed provider/policy cleanup.
const recoverPreparedDestroy =
!!options.force && !!sandbox.mcp?.destroyPreparedAt && !sandbox.mcp?.destroyPendingAt;
if (!recoverPreparedDestroy) {
assertMcpDestroyNotPending(sandbox);
}
const currentBridges = bridgeState(sandbox);
const entry = currentBridges[server];
if (!entry) {
if (!options.force) {
throw new McpBridgeError(`MCP server '${server}' not found on sandbox '${sandboxName}'.`);
}
// Distinguish a genuine marker-only recovery (no bridge entries remain, so
// the requested `--force` removal has nothing to clean up beyond the stuck
// marker) from a wrong-server no-op (other entries exist but not this one).
// Only the former is a proven recovery that may clear the destroy marker.
if (Object.keys(currentBridges).length === 0) {
console.log(` No MCP servers are registered on sandbox '${sandboxName}'.`);
return "markerOnlyNoEntries";
}
console.log(` No MCP server '${server}' is registered on sandbox '${sandboxName}'.`);
return;
return "noMatchingEntry";
}
if (entry.addState === "prepared") {
// `prepared` is persisted before gateway selection and is advanced only
Expand All @@ -118,7 +187,7 @@ async function removeMcpBridgeUnlocked(
// state another workflow may own.
removeBridgeEntry(sandboxName, server);
console.log(` Cancelled incomplete MCP add for '${server}' on sandbox '${sandboxName}'.`);
return;
return "cancelledPreparedAdd";
}
// Cleanup follows the adapter persisted with the bridge. Requiring the
// sandbox's current agent to still advertise MCP support would strand old
Expand Down Expand Up @@ -333,8 +402,11 @@ async function removeMcpBridgeUnlocked(
`MCP force cleanup left residual resources for '${server}'. The registry entry was preserved so cleanup can be retried.`,
);
}
return;
// allowResidual: the caller accepted leftover resources. This is NOT a proven
// recovery — residual state remains — so the destroy marker must be preserved.
return "residualPreserved";
}
removeBridgeEntry(sandboxName, server);
console.log(` Removed MCP server '${server}' from sandbox '${sandboxName}'.`);
return "removedTarget";
}
88 changes: 87 additions & 1 deletion src/lib/actions/sandbox/mcp-bridge-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
MCP_BRIDGE_POLICY_SOURCE,
McpBridgeError,
} from "./mcp-bridge-contracts";
import { validateSandboxName } from "./mcp-bridge-validation";

export function nowIso(): string {
return new Date().toISOString();
Expand Down Expand Up @@ -99,11 +100,96 @@ export function setBridgeState(sandboxName: string, bridges: Record<string, McpB

export function assertMcpDestroyNotPending(sandbox: SandboxEntry): void {
if (!sandbox.mcp?.destroyPreparedAt && !sandbox.mcp?.destroyPendingAt) return;
// Phase-aware recovery guidance. `destroyPendingAt` is written only after
// OpenShell confirms deletion (mcp-bridge-destroy.ts), so the only safe action
// is to finish the idempotent destroy. A prepared-only marker does not prove
// deletion; `mcp remove --force` may recover in place if the sandbox is still
// live, while failures preserve the marker.
if (sandbox.mcp?.destroyPendingAt) {
throw new McpBridgeError(
`Sandbox '${sandbox.name}' is mid-destroy past the point of no return — the registry records that OpenShell deletion was already confirmed. Run \`nemoclaw ${sandbox.name} destroy\` to finish the (idempotent) cleanup.`,
);
}
throw new McpBridgeError(
`Sandbox '${sandbox.name}' has an incomplete MCP destroy transaction. Re-run the sandbox destroy command to finish cleanup before using MCP commands.`,
`Sandbox '${sandbox.name}' has an incomplete MCP destroy transaction. Re-run the sandbox destroy command to finish cleanup, or, if the sandbox is still live, recover non-destructively with \`nemoclaw ${sandbox.name} mcp remove <server> --force\`.`,
);
}

/**
* Non-destructive recovery for a stuck MCP destroy transaction — PHASE-AWARE.
*
* When a prior destroy leaves a `destroyPreparedAt` marker behind (phase one:
* in-sandbox scrub + provider detach done, deletion not durably confirmed)
* every MCP command is refused by `assertMcpDestroyNotPending`, and rebuild
* refuses up front with the same guard. Before #6376 the only advertised
* recovery was `nemoclaw <name> destroy` — full sandbox destruction.
*
* This helper clears ONLY the prepared (phase-one) marker, in place, so a
* `--force` caller can attempt the requested removal if the sandbox still
* exists. It deliberately refuses the pending (phase-two) marker: that marker
* records confirmed OpenShell deletion and is the durable retry state that
* keeps still-owed provider/policy cleanup idempotent. Erasing it would silently
* abandon that cleanup, so a pending transaction must be finished with
* `nemoclaw <name> destroy`, not cleared.
*
* Callers clear the prepared marker only AFTER the requested removal succeeds
* (see removeMcpBridge), so a failed recovery preserves the retry marker.
* `setBridgeState` preserves the marker across the removal's own writes until
* then.
*
* Returns whether the marker was actually cleared, so callers can log
* accurately (no-op vs. cleared).
*
* Product contract (#6376), intentionally narrow:
* invalidState: a crash/abort mid-destroy leaves durable `destroyPreparedAt`
* and/or `destroyPendingAt` markers that fail every MCP command and rebuild.
* sourceBoundary: the markers are host-owned registry state; the sandbox does
* not write them. `destroyPreparedAt` = deletion is not durably confirmed
* (recoverable if still live); `destroyPendingAt` = the registry records
* confirmed OpenShell deletion (not recoverable in place — global
* provider/policy cleanup is still owed).
* sourceFixConstraint: there is no safe non-destructive reconciliation for the
* pending/both-marker live state, so this helper refuses it rather than
* guess. Prepared-only markers are recoverable with `mcp remove --force`;
* pending/both-marker state must finish `nemoclaw <name> destroy`.
* regressionTest: mcp-bridge-destroy-marker-recovery.test.ts (phase-aware
* clear/refuse, clear-only-after-proven-recovery, preserve-on-failure) and
* mcp-destroy-lifecycle.test.ts (phase-aware guard message).
* removalCondition: revisit if a safe pending-phase reconciliation is designed
* (proving the still-owed provider/policy cleanup is complete) — then this
* refusal could be relaxed.
*/
export function clearMcpDestroyMarkers(sandboxName: string): boolean {
// Validate the name before any registry read/update — this helper mutates
// durable state and must not trust an unvalidated identifier.
validateSandboxName(sandboxName);
const sandbox = registry.getSandbox(sandboxName);
const mcpState = sandbox?.mcp;
if (!mcpState?.destroyPreparedAt && !mcpState?.destroyPendingAt) return false;
if (mcpState.destroyPendingAt) {
throw new McpBridgeError(
`Sandbox '${sandboxName}' is mid-destroy past the point of no return — the registry records that OpenShell deletion was already confirmed. Run \`nemoclaw ${sandboxName} destroy\` to finish cleanup; the pending-destroy marker cannot be cleared non-destructively.`,
);
}
const bridges = mcpState.bridges ?? {};
const managedServerNames = mcpState.managedServerNames ?? [];
const updated = registry.updateSandbox(sandboxName, {
mcp:
Object.keys(bridges).length > 0 || managedServerNames.length > 0
? {
bridges,
...(managedServerNames.length > 0 ? { managedServerNames } : {}),
}
: undefined,
});
if (!updated) {
throw new McpBridgeError(
`Could not clear incomplete MCP destroy markers for sandbox '${sandboxName}'.`,
);
}
return true;
}

export function assertNoDerivedResourceCollision(
sandbox: SandboxEntry,
server: string,
Expand Down
Loading
Loading