From 495845a8e434f5bb5676b292471ea0c77b7b2cca Mon Sep 17 00:00:00 2001 From: Prekshi Vyas Date: Tue, 25 Aug 2026 19:40:37 -0700 Subject: [PATCH 1/6] fix(mcp): keep adapter revision current after add Signed-off-by: Prekshi Vyas --- .../actions/sandbox/mcp-bridge-add-restart.ts | 11 -------- test/mcp/mcp-add-crash-consistency.test.ts | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts index f8bcd7a5ff6..903530b28a2 100644 --- a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts @@ -435,17 +435,6 @@ async function addMcpBridgeUnlocked( providerAttachAttempted = true; attachProvider(sandboxName, entry); applyGeneratedPolicy(sandboxName, entry, target); - if (Object.hasOwn(adapterEnvValues, entry.env[0])) { - // OpenShell 0.0.106 can miss a credential update published before the - // bound policy generation. Republish while that policy is active and - // before the first readiness exec; the exact provider identity is - // rechecked before and after this update-only mutation. - upsertMcpProvider(entry.providerName ?? "", options.env, { - allowExisting: true, - expectedProviderId: entry.providerId, - requireExisting: true, - }); - } const credentialRevision = waitForAttachedMcpCredential(sandboxName, entry, { ...(providerResult.action === "updated" ? { diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index a9099784655..dc5d0b8fa17 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -237,13 +237,16 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { !attachmentAttemptedThisProcess; isPreupdateObservation && mark("observation"); if (crashAfter === "credential-projection-coalesced" && isObservation) { - if (credentialRepublishBeforeObservationCountThisProcess === 0) { + const credentialRepublishCount = + credentialRepublishBeforeObservationCountThisProcess + + credentialRepublishAfterAbsenceCountThisProcess; + if (credentialRepublishCount === 0) { observedCredentialAbsentThisProcess = true; mark("credential-observed-absent"); } return { status: 0, - stdout: credentialRepublishBeforeObservationCountThisProcess > 0 ? "v" + providerVersion() : "absent", + stdout: credentialRepublishCount > 0 ? "v" + providerVersion() : "absent", stderr: "", }; } @@ -269,6 +272,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: "" }; @@ -614,7 +621,7 @@ function readBridge(home: string): Record { } 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 post-policy 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 @@ -631,14 +638,15 @@ describe("MCP add crash consistency", () => { expect(results.map((result) => result.status).sort(), combinedOutput).toEqual([0, 2]); expect(results.find((result) => result.status === 2)?.stderr).toContain("already exists"); expect(combinedOutput).not.toContain("host-only-secret"); - expect(fs.existsSync(path.join(home, "credential-observed-absent.marker"))).toBe(false); - expect(fs.existsSync(path.join(home, "republish-before-observation.marker"))).toBe(true); - expect(fs.existsSync(path.join(home, "republish-after-observed-absence.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "credential-observed-absent.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "republish-before-observation.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "republish-after-observed-absence.marker"))).toBe(true); const credentialRepublishCount = fs - .readFileSync(path.join(home, "republish-before-observation.marker"), "utf8") + .readFileSync(path.join(home, "republish-after-observed-absence.marker"), "utf8") .split("\n") .filter(Boolean).length; expect(credentialRepublishCount).toBe(1); + expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe("v2"); 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); @@ -694,7 +702,7 @@ describe("MCP add crash consistency", () => { } }); - it("creates a fresh provider without an update-only prior revision observation", () => { + it("does not republish a projected credential before readiness observation", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-no-prior-observation-")); try { const result = runAddProcess(home, "preupdate-observation-forbidden"); @@ -702,7 +710,7 @@ describe("MCP add crash consistency", () => { expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); expect(fs.existsSync(path.join(home, "observation.marker"))).toBe(false); expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(true); - expect(fs.existsSync(path.join(home, "updated.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "updated.marker"))).toBe(false); expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(true); expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(true); expect(readBridge(home).addState).toBeUndefined(); From 8f433f666689503c5de9ba16097c8561431d4b7c Mon Sep 17 00:00:00 2001 From: San Dang Date: Wed, 26 Aug 2026 12:40:57 +0700 Subject: [PATCH 2/6] test(mcp): require stable adapter revision after add Reconcile the conflicting MCP add files with current main and preserve the contributor's crash-consistency coverage as a stable-revision regression.\n\nSigned-off-by: San Dang \nSigned-off-by: San Dang --- .../actions/sandbox/mcp-bridge-add-restart.ts | 65 ++++++++++++------- test/mcp/mcp-add-crash-consistency.test.ts | 35 ++++++---- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts index 903530b28a2..bfc6463f93f 100644 --- a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts @@ -435,35 +435,50 @@ async function addMcpBridgeUnlocked( providerAttachAttempted = true; attachProvider(sandboxName, entry); applyGeneratedPolicy(sandboxName, entry, target); + const hasHostCredential = Object.hasOwn(adapterEnvValues, entry.env[0]); + let synchronizationBaseline = + providerResult.action === "updated" ? previousCredentialRevision : undefined; + if (hasHostCredential) { + // OpenShell 0.0.106 can miss a credential update published before the + // bound policy generation. Republish while that policy is active and + // compare the opaque sandbox-wide credential revision observed + // immediately before that mutation with fresh execs afterward. Provider + // resource versions are unrelated to credential placeholder revisions, + // so this pre-mutation observation is the exact stale-revision barrier. + synchronizationBaseline = observeMcpCredentialRevision(sandboxName, entry); + upsertMcpProvider(entry.providerName ?? "", options.env, { + allowExisting: true, + expectedProviderId: entry.providerId, + requireExisting: true, + }); + } const credentialRevision = waitForAttachedMcpCredential(sandboxName, entry, { - ...(providerResult.action === "updated" + ...(synchronizationBaseline !== undefined + ? { previousRevision: synchronizationBaseline } + : {}), + // A no-field provider update advances only the provider resource version. + // Hostless recovery has no post-policy credential-bearing mutation to + // wait for, so it advances the provider revision only after a fresh exec + // proves the credential absent. A host credential was already + // republished above and must be allowed to project without another + // update racing its opaque revision. + ...(!hasHostCredential ? { - previousRevision: previousCredentialRevision, + refreshAfterObservedAbsence: () => { + // invalidState: OpenShell 0.0.106 can coalesce a no-field provider + // refresh without publishing the credential into fresh sandbox execs. + // sourceBoundary: OpenShell owns provider revision projection. + // whyNotSourceFix: NemoClaw can only observe absence after the bound + // policy is active. Hostless recovery retains the credential-free path. + // regressionTest: mcp-add-crash-consistency.test.ts covers hostless + // recovery; mcp-provider-ownership.test.ts covers loss of the + // persisted provider identity before refresh. + // removalCondition: remove this refresh when the supported OpenShell + // version guarantees bound credentials project into fresh execs. + refreshMcpProviderEnvironment(entry); + }, } : {}), - // A no-field provider update advances only the provider resource version. - // If the credential remains available, republish it after observing an - // absence; otherwise, a hostless recovery advances the provider revision. - refreshAfterObservedAbsence: () => { - // invalidState: OpenShell 0.0.106 can coalesce a no-field provider - // refresh without publishing the credential into fresh sandbox execs. - // sourceBoundary: OpenShell owns provider revision projection. - // whyNotSourceFix: NemoClaw can only observe absence after the bound - // policy is active, then republish when this process still has the host - // credential value. Hostless recovery retains the credential-free path. - // regressionTest: mcp-add-crash-consistency.test.ts covers republish - // and hostless recovery; mcp-provider-ownership.test.ts covers loss of - // the persisted provider identity before republish. - // removalCondition: remove the credential-bearing republish when the - // supported OpenShell version guarantees that a post-policy no-field - // refresh projects the bound credential into fresh sandbox execs. - const republished = upsertMcpProvider(entry.providerName ?? "", options.env, { - allowExisting: true, - expectedProviderId: entry.providerId, - requireExisting: true, - }); - if (republished.action !== "updated") refreshMcpProviderEnvironment(entry); - }, }); // The adapter was proven absent above, so cleanup is safe even when a // command commits config and then fails during its runtime reload. diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index dc5d0b8fa17..e11a9403bf4 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -43,16 +43,19 @@ 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 ( - crashAfter === "credential-projection-coalesced" || - crashAfter === "credential-projection-delayed-hostless" -) { +if (crashAfter === "credential-projection-coalesced") { + process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "3"; +} 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"); const mark = (name) => fs.writeFileSync(marker(name), "yes\n", { mode: 0o600 }); const marked = (name) => fs.existsSync(marker(name)); const providerVersion = () => Number.parseInt(marked("provider-version") ? fs.readFileSync(marker("provider-version"), "utf8") : "1", 10); +// 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 setProviderVersion = (version) => fs.writeFileSync(marker("provider-version"), String(version), { mode: 0o600 }); const providerPresentAtStart = marked("provider"); const providerId = "11111111-2222-4333-8444-555555555555"; @@ -66,6 +69,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"); @@ -243,10 +247,17 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { if (credentialRepublishCount === 0) { observedCredentialAbsentThisProcess = true; mark("credential-observed-absent"); + } else { + credentialObservationAfterRepublishCountThisProcess += 1; } return { status: 0, - stdout: credentialRepublishCount > 0 ? "v" + providerVersion() : "absent", + stdout: + credentialRepublishCount === 0 + ? "absent" + : credentialObservationAfterRepublishCountThisProcess === 1 + ? "v4067750153477477214" + : childCredentialRevision(), stderr: "", }; } @@ -257,13 +268,13 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { } return { status: 0, - stdout: credentialFreeRefreshAfterAbsenceCountThisProcess > 0 ? "v" + providerVersion() : "absent", + stdout: credentialFreeRefreshAfterAbsenceCountThisProcess > 0 ? childCredentialRevision() : "absent", stderr: "", }; } return { status: crashAfter === "preupdate-observation-forbidden" && isPreupdateObservation ? 1 : 0, - stdout: isObservation ? (marked("updated") ? "v" + providerVersion() : marked("provider") ? "v1" : "absent") : "", + stdout: isObservation ? (marked("updated") || marked("provider") ? childCredentialRevision() : "absent") : "", stderr: "", }; }; @@ -621,7 +632,7 @@ function readBridge(home: string): Record { } describe("MCP add crash consistency", () => { - it("commits one bridge at the post-policy credential revision and rejects one duplicate (#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 @@ -646,7 +657,9 @@ describe("MCP add crash consistency", () => { .split("\n") .filter(Boolean).length; expect(credentialRepublishCount).toBe(1); - expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe("v2"); + expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe( + "v4067750153477477215", + ); 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); @@ -702,7 +715,7 @@ describe("MCP add crash consistency", () => { } }); - it("does not republish a projected credential before readiness observation", () => { + it("creates a fresh provider without an update-only prior revision observation", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-no-prior-observation-")); try { const result = runAddProcess(home, "preupdate-observation-forbidden"); @@ -710,7 +723,7 @@ describe("MCP add crash consistency", () => { expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); expect(fs.existsSync(path.join(home, "observation.marker"))).toBe(false); expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(true); - expect(fs.existsSync(path.join(home, "updated.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "updated.marker"))).toBe(true); expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(true); expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(true); expect(readBridge(home).addState).toBeUndefined(); From 0ea9fbc5a3263796c723f4d940255e14349c3762 Mon Sep 17 00:00:00 2001 From: San Dang Date: Wed, 26 Aug 2026 12:42:31 +0700 Subject: [PATCH 3/6] chore(mcp): resolve current main test conflict Restore the overlapping crash-consistency fixture before updating this published branch to current main.\n\nSigned-off-by: San Dang \nSigned-off-by: San Dang --- test/mcp/mcp-add-crash-consistency.test.ts | 31 +++++----------------- 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index e11a9403bf4..7f8006276b6 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -43,9 +43,10 @@ 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 (crashAfter === "credential-projection-coalesced") { - process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "3"; -} else if (crashAfter === "credential-projection-delayed-hostless") { +if ( + crashAfter === "credential-projection-coalesced" || + crashAfter === "credential-projection-delayed-hostless" +) { process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "2"; } const marker = (name) => path.join(process.env.HOME, name + ".marker"); @@ -69,7 +70,6 @@ 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"); @@ -241,23 +241,13 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { !attachmentAttemptedThisProcess; isPreupdateObservation && mark("observation"); if (crashAfter === "credential-projection-coalesced" && isObservation) { - const credentialRepublishCount = - credentialRepublishBeforeObservationCountThisProcess + - credentialRepublishAfterAbsenceCountThisProcess; - if (credentialRepublishCount === 0) { + if (credentialRepublishBeforeObservationCountThisProcess === 0) { observedCredentialAbsentThisProcess = true; mark("credential-observed-absent"); - } else { - credentialObservationAfterRepublishCountThisProcess += 1; } return { status: 0, - stdout: - credentialRepublishCount === 0 - ? "absent" - : credentialObservationAfterRepublishCountThisProcess === 1 - ? "v4067750153477477214" - : childCredentialRevision(), + stdout: credentialRepublishBeforeObservationCountThisProcess + credentialRepublishAfterAbsenceCountThisProcess > 0 ? childCredentialRevision() : "absent", stderr: "", }; } @@ -283,10 +273,6 @@ 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: "" }; @@ -632,7 +618,7 @@ function readBridge(home: string): Record { } describe("MCP add crash consistency", () => { - it("commits one bridge at the stable credential revision and rejects one duplicate (#9764)", async () => { + it("commits one bridge and rejects one duplicate after delayed credential projection (#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 @@ -657,9 +643,6 @@ describe("MCP add crash consistency", () => { .split("\n") .filter(Boolean).length; expect(credentialRepublishCount).toBe(1); - expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe( - "v4067750153477477215", - ); 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); From fe4409c65df24189542a8442caa1919734d67d46 Mon Sep 17 00:00:00 2001 From: San Dang Date: Wed, 26 Aug 2026 12:46:00 +0700 Subject: [PATCH 4/6] test(mcp): require stable adapter revision after add Model a transient stale credential revision after the post-policy republish and require the adapter to receive only the final revision observed by two consecutive fresh execs.\n\nSigned-off-by: San Dang \nSigned-off-by: San Dang --- test/mcp/mcp-add-crash-consistency.test.ts | 31 +++++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index 7f8006276b6..e11a9403bf4 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -43,10 +43,9 @@ 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 ( - crashAfter === "credential-projection-coalesced" || - crashAfter === "credential-projection-delayed-hostless" -) { +if (crashAfter === "credential-projection-coalesced") { + process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "3"; +} 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"); @@ -70,6 +69,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"); @@ -241,13 +241,23 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { !attachmentAttemptedThisProcess; isPreupdateObservation && mark("observation"); if (crashAfter === "credential-projection-coalesced" && isObservation) { - if (credentialRepublishBeforeObservationCountThisProcess === 0) { + const credentialRepublishCount = + credentialRepublishBeforeObservationCountThisProcess + + credentialRepublishAfterAbsenceCountThisProcess; + if (credentialRepublishCount === 0) { observedCredentialAbsentThisProcess = true; mark("credential-observed-absent"); + } else { + credentialObservationAfterRepublishCountThisProcess += 1; } return { status: 0, - stdout: credentialRepublishBeforeObservationCountThisProcess + credentialRepublishAfterAbsenceCountThisProcess > 0 ? childCredentialRevision() : "absent", + stdout: + credentialRepublishCount === 0 + ? "absent" + : credentialObservationAfterRepublishCountThisProcess === 1 + ? "v4067750153477477214" + : childCredentialRevision(), stderr: "", }; } @@ -273,6 +283,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: "" }; @@ -618,7 +632,7 @@ function readBridge(home: string): Record { } 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,6 +657,9 @@ describe("MCP add crash consistency", () => { .split("\n") .filter(Boolean).length; expect(credentialRepublishCount).toBe(1); + expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe( + "v4067750153477477215", + ); 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); From 90b033f43b9826edb7fab873a7bdddc0b0d2a226 Mon Sep 17 00:00:00 2001 From: San Dang Date: Wed, 26 Aug 2026 13:19:51 +0700 Subject: [PATCH 5/6] test(mcp): cover managed-image revision convergence Expand the regression to prove stable revision readiness, fail-closed timeout behavior, concurrent add ownership, and redacted artifacts. Route MCP live-test changes through the managed-image workflow. Signed-off-by: San Dang --- .github/workflows/managed-images.yaml | 1 + ...managed-image-publication-workflow.test.ts | 1 + test/mcp/mcp-add-crash-consistency.test.ts | 125 ++++++++++++++++-- 3 files changed, 117 insertions(+), 10 deletions(-) diff --git a/.github/workflows/managed-images.yaml b/.github/workflows/managed-images.yaml index 8caf027abc1..8679c147672 100644 --- a/.github/workflows/managed-images.yaml +++ b/.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/**" diff --git a/test/inference/managed/managed-image-publication-workflow.test.ts b/test/inference/managed/managed-image-publication-workflow.test.ts index ebc234d5a73..7f2915f9459 100644 --- a/test/inference/managed/managed-image-publication-workflow.test.ts +++ b/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", diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index e11a9403bf4..415dd5ed8bc 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/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,8 +44,13 @@ 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)}; +const credentialProjectionScenario = + crashAfter === "credential-projection-coalesced" || + 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"; } @@ -113,6 +119,9 @@ 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" }; } @@ -133,7 +142,7 @@ providerCommands.runOpenshellProviderCommand = (args) => { mark("updated"); } if ( - crashAfter === "credential-projection-coalesced" && + credentialProjectionScenario && isCredentialUpdate && marked("bound-policy") && !observedCredentialAbsentThisProcess @@ -142,7 +151,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,7 +249,7 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { !credentialUpdatedThisProcess && !attachmentAttemptedThisProcess; isPreupdateObservation && mark("observation"); - if (crashAfter === "credential-projection-coalesced" && isObservation) { + if (credentialProjectionScenario && isObservation) { const credentialRepublishCount = credentialRepublishBeforeObservationCountThisProcess + credentialRepublishAfterAbsenceCountThisProcess; @@ -250,14 +259,22 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { } else { credentialObservationAfterRepublishCountThisProcess += 1; } - return { - status: 0, - stdout: - credentialRepublishCount === 0 - ? "absent" + const observation = + credentialRepublishCount === 0 + ? "absent" + : crashAfter === "credential-projection-unstable" + ? credentialObservationAfterRepublishCountThisProcess % 2 === 1 + ? "v4067750153477477214" + : childCredentialRevision() : credentialObservationAfterRepublishCountThisProcess === 1 ? "v4067750153477477214" - : childCredentialRevision(), + : childCredentialRevision(); + if (credentialRepublishCount > 0) { + fs.appendFileSync(marker("credential-observation-log"), observation + "\n", { mode: 0o600 }); + } + return { + status: 0, + stdout: observation, stderr: "", }; } @@ -305,6 +322,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", @@ -328,7 +358,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); @@ -631,6 +672,20 @@ function readBridge(home: string): Record { 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 at the stable credential revision and rejects one duplicate (#9764)", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-concurrent-projection-")); @@ -657,14 +712,64 @@ 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( "v4067750153477477215", ); + 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 }); } From 08a2c14812203edfeb883fa2870ebf91b032d82d Mon Sep 17 00:00:00 2001 From: Senthil Ravichandran Date: Wed, 26 Aug 2026 00:05:20 -0700 Subject: [PATCH 6/6] test(mcp): decouple fixture credential revision Signed-off-by: Senthil Ravichandran --- test/mcp/mcp-add-crash-consistency.test.ts | 25 +++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index 415dd5ed8bc..dcabfc28a03 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -61,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"; @@ -125,7 +132,11 @@ providerCommands.runOpenshellProviderCommand = (args) => { 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 && @@ -139,6 +150,7 @@ providerCommands.runOpenshellProviderCommand = (args) => { setProviderVersion(providerVersion() + 1); if (isCredentialUpdate) { credentialUpdatedThisProcess = true; + advanceChildCredentialRevision(); mark("updated"); } if ( @@ -264,10 +276,10 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { ? "absent" : crashAfter === "credential-projection-unstable" ? credentialObservationAfterRepublishCountThisProcess % 2 === 1 - ? "v4067750153477477214" + ? initialChildCredentialRevision : childCredentialRevision() : credentialObservationAfterRepublishCountThisProcess === 1 - ? "v4067750153477477214" + ? initialChildCredentialRevision : childCredentialRevision(); if (credentialRepublishCount > 0) { fs.appendFileSync(marker("credential-observation-log"), observation + "\n", { mode: 0o600 }); @@ -730,7 +742,10 @@ describe("MCP add crash consistency", () => { "v4067750153477477215", ]); expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe( - "v4067750153477477215", + 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"),