test(mcp): require stable adapter revision after add

.github/workflows/managed-images.yaml

@@ -51,6 +51,7 @@ on:
       - "src/lib/security/credential-hash.ts"
       - "src/lib/state/paths.ts"
       - "src/lib/state/state-root.ts"
+      - "test/e2e/live/mcp-bridge*.ts"
       - "test/e2e/live/managed-image-activation-e2e*.ts"
       - "src/lib/tool-disclosure.ts"
       - "tools/mcp-tool-discovery-runtime/**"

test/inference/managed/managed-image-publication-workflow.test.ts

@@ -691,6 +691,7 @@ describe("complete managed-image publication workflow", () => {
       readWorkflow("e2e.yaml").jobs?.["mcp-bridge"],
       "unified E2E workflow is missing its stable MCP job",
     );
+    expect(workflow.on?.pull_request?.paths).toContain("test/e2e/live/mcp-bridge*.ts");
     expect(discovery.needs).toBe("pr-build-and-entrypoint");
     expect(discovery.if).toContain(
       "github.event.pull_request.head.repo.full_name == github.repository",

test/mcp/mcp-add-crash-consistency.test.ts

@@ -18,6 +18,7 @@ type CrashBoundary =
   | "credential-collision"
   | "credential-command-race"
   | "credential-projection-coalesced"
+  | "credential-projection-unstable"
   | "credential-projection-delayed-hostless"
   | "registered-credential-collision"
   | "registered-late-collision"
@@ -43,10 +44,14 @@ includeSecret ? (process.env.FAKE_MCP_SECRET = "host-only-secret") : delete proc
 const fs = require("node:fs");
 const path = require("node:path");
 const crashAfter = ${JSON.stringify(crashAfter)};
-if (
+const credentialProjectionScenario =
   crashAfter === "credential-projection-coalesced" ||
-  crashAfter === "credential-projection-delayed-hostless"
-) {
+  crashAfter === "credential-projection-unstable";
+if (crashAfter === "credential-projection-coalesced") {
+  process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "3";
+} else if (crashAfter === "credential-projection-unstable") {
+  process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "2";
+} else if (crashAfter === "credential-projection-delayed-hostless") {
   process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "2";
 }
 const marker = (name) => path.join(process.env.HOME, name + ".marker");
@@ -56,7 +61,14 @@ const providerVersion = () => Number.parseInt(marked("provider-version") ? fs.re
 // OpenShell provider resource versions and child credential revisions are
 // separate identities. Keep the fixture values unrelated so readiness cannot
 // derive one from the other.
-const childCredentialRevision = () => "v" + (4067750153477477213n + BigInt(providerVersion())).toString();
+const initialChildCredentialRevision = "v4067750153477477214";
+const childCredentialRevision = () => marked("child-credential-revision")
+  ? fs.readFileSync(marker("child-credential-revision"), "utf8").trim()
+  : initialChildCredentialRevision;
+const setChildCredentialRevision = (revision) => fs.writeFileSync(marker("child-credential-revision"), revision, { mode: 0o600 });
+const advanceChildCredentialRevision = () => setChildCredentialRevision(
+  "v" + (BigInt(childCredentialRevision().slice(1)) + 1n).toString(),
+);
 const setProviderVersion = (version) => fs.writeFileSync(marker("provider-version"), String(version), { mode: 0o600 });
 const providerPresentAtStart = marked("provider");
 const providerId = "11111111-2222-4333-8444-555555555555";
@@ -70,6 +82,7 @@ let credentialRepublishBeforeObservationCountThisProcess = 0;
 let credentialRepublishAfterAbsenceCountThisProcess = 0;
 let credentialFreeRefreshBeforeObservationCountThisProcess = 0;
 let credentialFreeRefreshAfterAbsenceCountThisProcess = 0;
+let credentialObservationAfterRepublishCountThisProcess = 0;
 
 const registry = require("./src/lib/state/registry.js");
 const providerCommands = require("./src/lib/adapters/openshell/provider-command.js");
@@ -113,10 +126,17 @@ providerCommands.runOpenshellProviderCommand = (args) => {
       : { status: 1, stdout: "", stderr: "NotFound: provider" };
   }
   if (args[0] === "provider" && (args[1] === "create" || args[1] === "update")) {
+    if (credentialProjectionScenario) {
+      fs.appendFileSync(marker("provider-mutation-log"), args[1] + "\n", { mode: 0o600 });
+    }
     if (!marked("policy")) {
       return { status: 1, stdout: "", stderr: "provider mutation preceded policy attestation" };
     }
-    if (args[1] === "create") observedProviderName = args[args.indexOf("--name") + 1], setProviderVersion(1);
+    if (args[1] === "create") {
+      observedProviderName = args[args.indexOf("--name") + 1];
+      setProviderVersion(1);
+      setChildCredentialRevision(initialChildCredentialRevision);
+    }
     if (args[1] === "update") {
       const isCredentialUpdate =
         args.length === 5 &&
@@ -130,10 +150,11 @@ providerCommands.runOpenshellProviderCommand = (args) => {
       setProviderVersion(providerVersion() + 1);
       if (isCredentialUpdate) {
         credentialUpdatedThisProcess = true;
+        advanceChildCredentialRevision();
         mark("updated");
       }
       if (
-        crashAfter === "credential-projection-coalesced" &&
+        credentialProjectionScenario &&
         isCredentialUpdate &&
         marked("bound-policy") &&
         !observedCredentialAbsentThisProcess
@@ -142,7 +163,7 @@ providerCommands.runOpenshellProviderCommand = (args) => {
         fs.appendFileSync(marker("republish-before-observation"), "republish\n", { mode: 0o600 });
       }
       if (
-        crashAfter === "credential-projection-coalesced" &&
+        credentialProjectionScenario &&
         isCredentialUpdate &&
         marked("bound-policy") &&
         observedCredentialAbsentThisProcess
@@ -240,14 +261,32 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => {
     !credentialUpdatedThisProcess &&
     !attachmentAttemptedThisProcess;
   isPreupdateObservation && mark("observation");
-  if (crashAfter === "credential-projection-coalesced" && isObservation) {
-    if (credentialRepublishBeforeObservationCountThisProcess === 0) {
+  if (credentialProjectionScenario && isObservation) {
+    const credentialRepublishCount =
+      credentialRepublishBeforeObservationCountThisProcess +
+      credentialRepublishAfterAbsenceCountThisProcess;
+    if (credentialRepublishCount === 0) {
       observedCredentialAbsentThisProcess = true;
       mark("credential-observed-absent");
+    } else {
+      credentialObservationAfterRepublishCountThisProcess += 1;
+    }
+    const observation =
+      credentialRepublishCount === 0
+        ? "absent"
+        : crashAfter === "credential-projection-unstable"
+          ? credentialObservationAfterRepublishCountThisProcess % 2 === 1
+            ? initialChildCredentialRevision
+            : childCredentialRevision()
+          : credentialObservationAfterRepublishCountThisProcess === 1
+            ? initialChildCredentialRevision
+            : childCredentialRevision();
+    if (credentialRepublishCount > 0) {
+      fs.appendFileSync(marker("credential-observation-log"), observation + "\n", { mode: 0o600 });
     }
     return {
       status: 0,
-      stdout: credentialRepublishBeforeObservationCountThisProcess + credentialRepublishAfterAbsenceCountThisProcess > 0 ? childCredentialRevision() : "absent",
+      stdout: observation,
       stderr: "",
     };
   }
@@ -273,6 +312,10 @@ processRecovery.executeSandboxCommand = (_sandbox, command) => {
     return { status: 0, stdout: "/usr/local/bin/mcporter\n", stderr: "" };
   }
   if (command.includes("config' 'add") || command.includes('"config", "add"')) {
+    const adapterRevision = command.match(/openshell:resolve:env:(v[0-9]+)_FAKE_MCP_SECRET/)?.[1];
+    if (adapterRevision) {
+      fs.writeFileSync(marker("adapter-revision"), adapterRevision, { mode: 0o600 });
+    }
     mark("adapter");
     if (crashAfter === "adapter") process.exit(86);
     return { status: 0, stdout: "", stderr: "" };
@@ -291,6 +334,19 @@ processRecovery.executeSandboxCommand = (_sandbox, command) => {
   ) {
     return { status: 0, stdout: "mismatch\n", stderr: "" };
   }
+  if (
+    crashAfter === "credential-projection-coalesced" &&
+    marked("adapter") &&
+    (command.includes('["config", "get"') || command.includes('"get", expected.server'))
+  ) {
+    const expectedRevision = command.match(/openshell:resolve:env:(v[0-9]+)_FAKE_MCP_SECRET/)?.[1];
+    const adapterRevision = fs.readFileSync(marker("adapter-revision"), "utf8");
+    return {
+      status: 0,
+      stdout: expectedRevision === adapterRevision ? "registered\n" : "mismatch\n",
+      stderr: "",
+    };
+  }
   return {
     status: 0,
     stdout: marked("adapter") ? "registered\n" : "absent\n",
@@ -314,7 +370,18 @@ bridge.addMcpBridge("crash-test", {
   url: "https://8.8.8.8/mcp",
   env: [{ name: "FAKE_MCP_SECRET" }],
 }).then(
-  () => process.exit(0),
+  async () => {
+    try {
+      if (crashAfter === "credential-projection-coalesced") {
+        const [status] = await bridge.statusMcpBridge("crash-test", "fake");
+        fs.writeFileSync(marker("post-add-status"), JSON.stringify(status), { mode: 0o600 });
+      }
+      process.exit(0);
+    } catch (error) {
+      console.error(error && error.stack || error);
+      process.exit(2);
+    }
+  },
   (error) => {
     console.error(error && error.stack || error);
     process.exit(2);
@@ -617,8 +684,22 @@ function readBridge(home: string): Record<string, unknown> {
   return parsed.sandboxes["crash-test"].mcp.bridges.fake;
 }
 
+function readFixtureArtifacts(directory: string): string {
+  return fs
+    .readdirSync(directory, { withFileTypes: true })
+    .map((entry) => {
+      const target = path.join(directory, entry.name);
+      return entry.isDirectory()
+        ? readFixtureArtifacts(target)
+        : entry.isFile()
+          ? fs.readFileSync(target, "utf8")
+          : "";
+    })
+    .join("\n");
+}
+
 describe("MCP add crash consistency", () => {
-  it("commits one bridge and rejects one duplicate after delayed credential projection (#9764)", async () => {
+  it("commits one bridge at the stable credential revision and rejects one duplicate (#9764)", async () => {
     const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-concurrent-projection-"));
     try {
       // Create the fixture before either process loads the registry. The
@@ -643,11 +724,67 @@ describe("MCP add crash consistency", () => {
         .split("\n")
         .filter(Boolean).length;
       expect(credentialRepublishCount).toBe(1);
+      expect(
+        fs
+          .readFileSync(path.join(home, "provider-mutation-log.marker"), "utf8")
+          .split("\n")
+          .filter(Boolean),
+      ).toEqual(["create", "update"]);
+      expect(
+        fs
+          .readFileSync(path.join(home, "credential-observation-log.marker"), "utf8")
+          .split("\n")
+          .filter(Boolean),
+      ).toEqual([
+        "v4067750153477477214",
+        "v4067750153477477215",
+        "v4067750153477477215",
+        "v4067750153477477215",
+      ]);
+      expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe(
+        fs.readFileSync(path.join(home, "child-credential-revision.marker"), "utf8"),
+      );
+      expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).not.toBe(
+        `v${fs.readFileSync(path.join(home, "provider-version.marker"), "utf8")}`,
+      );
+      const postAddStatus = JSON.parse(
+        fs.readFileSync(path.join(home, "post-add-status.marker"), "utf8"),
+      ) as { adapter: { registered: boolean | null } };
+      expect(postAddStatus.adapter.registered).toBe(true);
       expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(true);
       expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(true);
       expect(fs.existsSync(path.join(home, "policy.marker"))).toBe(true);
       expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(true);
       expect(readBridge(home).addState).toBeUndefined();
+      expect(readFixtureArtifacts(home)).not.toContain("host-only-secret");
+    } finally {
+      fs.rmSync(home, { recursive: true, force: true });
+    }
+  });
+
+  it("times out without committing an adapter while credential revisions remain unstable (#9764)", () => {
+    const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-unstable-revision-"));
+    try {
+      const result = runAddProcess(home, "credential-projection-unstable");
+
+      expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(2);
+      expect(result.stderr).toContain("did not synchronize the expected credential revision");
+      expect(`${result.stdout}\n${result.stderr}`).not.toContain("host-only-secret");
+      expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(false);
+      expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(false);
+      expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(false);
+      expect(fs.existsSync(path.join(home, "policy.marker"))).toBe(false);
+      const observations = fs
+        .readFileSync(path.join(home, "credential-observation-log.marker"), "utf8")
+        .split("\n")
+        .filter(Boolean);
+      expect(observations.length).toBeGreaterThan(1);
+      expect(
+        observations.every(
+          (revision, index) => index === 0 || revision !== observations[index - 1],
+        ),
+      ).toBe(true);
+      expect(readFixtureArtifacts(home)).not.toContain("host-only-secret");
     } finally {
       fs.rmSync(home, { recursive: true, force: true });
     }