fix(rebuild): preserve external policy authority

ci/source-architecture-budget.json

@@ -28,7 +28,7 @@
       "src/lib/security/redact.ts": 54,
       "src/lib/state/onboard-session.ts": 36,
       "src/lib/state/mcp-lifecycle-lock.ts": 21,
-      "src/lib/state/registry.ts": 100,
+      "src/lib/state/registry.ts": 103,
       "src/lib/state/state-root.ts": 21,
       "src/lib/subprocess-env.ts": 24,
       "src/lib/validation.ts": 24

src/lib/actions/sandbox/mcp-bridge-contracts.ts

@@ -1,7 +1,9 @@
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import type { AgentMcpAdapter } from "../../agent/defs";
+import type { AgentMcpAdapter } from "../../agent/definition-types";
+
+export { isAgentMcpAdapter } from "../../agent/definition-types";
 
 export const MCP_BRIDGE_POLICY_SOURCE = "generated:nemoclaw-mcp-bridge";
 export type McpBridgeErrorReasonCode = "rejected" | "unresolved";
@@ -93,7 +95,3 @@ export interface McpBridgeStatus {
   addedAt?: string;
   updatedAt?: string;
 }
-
-export function isAgentMcpAdapter(value: unknown): value is AgentMcpAdapter {
-  return value === "mcporter" || value === "hermes-config" || value === "deepagents-config";
-}

src/lib/actions/sandbox/mcp-bridge-rebuild-policy-authority.test.ts

@@ -0,0 +1,239 @@
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { PolicyAuthorityRefusalError } from "../../adapters/openshell/policy-authority";
+import type { McpBridgeEntry, SandboxEntry } from "../../state/registry";
+
+const mocks = vi.hoisted(() => ({
+  assertAdapterConfigMutationsAllowed: vi.fn(),
+  assertAdapterTeardownRuntimeCapabilities: vi.fn(),
+  assertDestroyNotPending: vi.fn(),
+  assertGeneratedPolicyMutationSafe: vi.fn(),
+  assertNoProviderCredentialCollisions: vi.fn(),
+  assertProviderRecoverable: vi.fn(),
+  attachProvider: vi.fn(),
+  bridgeState: vi.fn(),
+  detachProvider: vi.fn(),
+  discardSafeIncompleteAdds: vi.fn(),
+  ensureSandboxGatewaySelected: vi.fn(),
+  getSandboxOrThrow: vi.fn(),
+  inspectExactDestroyProvider: vi.fn(),
+  preflightEntryTargets: vi.fn(),
+  removeGeneratedPolicy: vi.fn(),
+  refreshProviderEnvironment: vi.fn(),
+  restoreRuntime: vi.fn(),
+  rollbackScrubbedAdapters: vi.fn(),
+  scrubAdapter: vi.fn(),
+  setBridgeState: vi.fn(),
+  waitForDetachedCredential: vi.fn(),
+  waitForAttachedCredential: vi.fn(),
+}));
+
+vi.mock("./mcp-bridge-adapter-teardown", () => ({
+  rollbackScrubbedMcpAdapters: mocks.rollbackScrubbedAdapters,
+  scrubManagedMcpAdapterOrThrow: mocks.scrubAdapter,
+}));
+
+vi.mock("./mcp-bridge-destroy", () => ({
+  cloneMcpBridgeEntry: (entry: McpBridgeEntry) => structuredClone(entry),
+  discardSafeIncompleteMcpAdds: mocks.discardSafeIncompleteAdds,
+  inspectExactMcpDestroyProvider: mocks.inspectExactDestroyProvider,
+}));
+
+vi.mock("./mcp-bridge-policy", () => ({
+  assertGeneratedPolicyMutationSafe: mocks.assertGeneratedPolicyMutationSafe,
+  assertGeneratedPolicyRegistrationMutationSafe: vi.fn(),
+  removeGeneratedPolicy: mocks.removeGeneratedPolicy,
+}));
+
+vi.mock("./mcp-bridge-provider", () => ({
+  assertMcpProviderRecoverable: mocks.assertProviderRecoverable,
+  assertNoProviderCredentialCollisions: mocks.assertNoProviderCredentialCollisions,
+  assertNoRegisteredProviderCredentialCollisions: vi.fn(),
+  attachProvider: mocks.attachProvider,
+  detachProvider: mocks.detachProvider,
+  preflightMcpEntryTargets: mocks.preflightEntryTargets,
+  refreshMcpProviderEnvironment: mocks.refreshProviderEnvironment,
+  waitForAttachedMcpCredential: mocks.waitForAttachedCredential,
+  waitForDetachedMcpCredential: mocks.waitForDetachedCredential,
+}));
+
+vi.mock("./mcp-bridge-restart", () => ({
+  restoreExistingMcpBridgeRuntime: mocks.restoreRuntime,
+}));
+
+vi.mock("./mcp-bridge-runtime-capabilities", () => ({
+  assertMcpAdapterConfigMutationsAllowed: mocks.assertAdapterConfigMutationsAllowed,
+  assertMcpAdapterTeardownRuntimeCapabilities: mocks.assertAdapterTeardownRuntimeCapabilities,
+}));
+
+vi.mock("./mcp-bridge-state", () => ({
+  assertMcpDestroyNotPending: mocks.assertDestroyNotPending,
+  bridgeState: mocks.bridgeState,
+  ensureSandboxGatewaySelected: mocks.ensureSandboxGatewaySelected,
+  getSandboxOrThrow: mocks.getSandboxOrThrow,
+  setBridgeState: mocks.setBridgeState,
+}));
+
+const { prepareMcpBridgesForRebuild, restoreMcpBridgesAfterRebuild } =
+  await import("./mcp-bridge-rebuild");
+
+let entry: McpBridgeEntry;
+let sandbox: SandboxEntry;
+
+describe("MCP rebuild policy authority", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    entry = {
+      server: "github",
+      agent: "openclaw",
+      adapter: "mcporter",
+      url: "https://mcp.example.test/mcp",
+      env: ["GITHUB_TOKEN"],
+      providerName: "alpha-mcp-github",
+      providerId: "provider-1",
+      policyName: "mcp-bridge-github",
+      addedAt: "2026-08-20T00:00:00.000Z",
+    };
+    sandbox = {
+      name: "alpha",
+      agent: "openclaw",
+      policyAuthority: "externally-managed",
+      mcp: { bridges: { github: entry } },
+    };
+    mocks.getSandboxOrThrow.mockReturnValue(sandbox);
+    mocks.bridgeState.mockImplementation((current: SandboxEntry) => current.mcp?.bridges ?? {});
+    mocks.discardSafeIncompleteAdds.mockResolvedValue(sandbox);
+    mocks.detachProvider.mockReturnValue("detached");
+    mocks.rollbackScrubbedAdapters.mockReturnValue([]);
+    mocks.scrubAdapter.mockImplementation((_name, _sandbox, current: McpBridgeEntry) => ({
+      ...current,
+      credentialRevision: "v1",
+    }));
+  });
+
+  it("preserves externally managed policy while preparing adapter and provider state (#9833)", async () => {
+    const validatePolicyAuthority = vi.fn().mockResolvedValue(undefined);
+
+    await expect(
+      prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority),
+    ).resolves.toMatchObject({
+      entries: [entry],
+      detachedProviderEntries: [entry],
+      scrubbedAdapterEntries: [entry],
+    });
+
+    expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled();
+    expect(validatePolicyAuthority).toHaveBeenCalledTimes(3);
+    expect(validatePolicyAuthority.mock.invocationCallOrder[1]).toBeLessThan(
+      mocks.scrubAdapter.mock.invocationCallOrder[0],
+    );
+    expect(validatePolicyAuthority.mock.invocationCallOrder[2]).toBeLessThan(
+      mocks.detachProvider.mock.invocationCallOrder[0],
+    );
+  });
+
+  it("preserves the authority refusal before MCP teardown mutation (#9833)", async () => {
+    const refusal = new PolicyAuthorityRefusalError("policy authority changed");
+    const validatePolicyAuthority = vi
+      .fn()
+      .mockResolvedValueOnce(undefined)
+      .mockRejectedValueOnce(refusal);
+
+    await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe(
+      refusal,
+    );
+
+    expect(mocks.scrubAdapter).not.toHaveBeenCalled();
+    expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled();
+    expect(mocks.detachProvider).not.toHaveBeenCalled();
+  });
+
+  it("rolls back an adapter scrub when authority changes before policy removal (#9833)", async () => {
+    sandbox.policyAuthority = "nemoclaw-managed";
+    const refusal = new PolicyAuthorityRefusalError("policy authority changed");
+    const validatePolicyAuthority = vi
+      .fn()
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(undefined)
+      .mockRejectedValueOnce(refusal);
+
+    await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe(
+      refusal,
+    );
+
+    expect(mocks.rollbackScrubbedAdapters).toHaveBeenCalledExactlyOnceWith("alpha", sandbox, [
+      expect.objectContaining({ server: "github", credentialRevision: "v1" }),
+    ]);
+    expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled();
+    expect(mocks.detachProvider).not.toHaveBeenCalled();
+    expect(mocks.restoreRuntime).not.toHaveBeenCalled();
+  });
+
+  it("reattaches a detached provider when authority changes before the next detach (#9833)", async () => {
+    const second = {
+      ...entry,
+      server: "gitlab",
+      env: ["GITLAB_TOKEN"],
+      providerName: "alpha-mcp-gitlab",
+      providerId: "provider-2",
+      policyName: "mcp-bridge-gitlab",
+    };
+    sandbox.mcp = { bridges: { github: entry, gitlab: second } };
+    const refusal = new PolicyAuthorityRefusalError("policy authority changed");
+    const validatePolicyAuthority = vi
+      .fn()
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(undefined)
+      .mockResolvedValueOnce(undefined)
+      .mockRejectedValueOnce(refusal);
+
+    await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe(
+      refusal,
+    );
+
+    expect(mocks.attachProvider).toHaveBeenCalledExactlyOnceWith("alpha", entry);
+    expect(mocks.refreshProviderEnvironment).toHaveBeenCalledExactlyOnceWith(entry);
+    expect(mocks.waitForAttachedCredential).toHaveBeenCalledExactlyOnceWith("alpha", entry);
+    expect(mocks.rollbackScrubbedAdapters).toHaveBeenCalledWith(
+      "alpha",
+      sandbox,
+      expect.arrayContaining([
+        expect.objectContaining({ server: "github", credentialRevision: "v1" }),
+        expect.objectContaining({ server: "gitlab", credentialRevision: "v1" }),
+      ]),
+    );
+    expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled();
+    expect(mocks.restoreRuntime).not.toHaveBeenCalled();
+  });
+
+  it("revalidates between registry recovery and runtime restoration (#9833)", async () => {
+    const refusal = new PolicyAuthorityRefusalError("policy authority changed");
+    const validatePolicyAuthority = vi
+      .fn()
+      .mockResolvedValueOnce(undefined)
+      .mockRejectedValueOnce(refusal);
+
+    await expect(
+      restoreMcpBridgesAfterRebuild("alpha", [entry], validatePolicyAuthority),
+    ).rejects.toBe(refusal);
+
+    expect(mocks.setBridgeState).toHaveBeenCalledOnce();
+    expect(mocks.restoreRuntime).not.toHaveBeenCalled();
+  });
+
+  it("revalidates after MCP runtime restoration before returning success (#9833)", async () => {
+    const validatePolicyAuthority = vi.fn().mockResolvedValue(undefined);
+
+    await expect(
+      restoreMcpBridgesAfterRebuild("alpha", [entry], validatePolicyAuthority),
+    ).resolves.toBeUndefined();
+
+    expect(validatePolicyAuthority).toHaveBeenCalledTimes(3);
+    expect(mocks.restoreRuntime.mock.invocationCallOrder[0]).toBeLessThan(
+      validatePolicyAuthority.mock.invocationCallOrder[2],
+    );
+  });
+});