diff --git a/ci/test-file-size-budget.json b/ci/test-file-size-budget.json index 50dba967c4..631a805c55 100644 --- a/ci/test-file-size-budget.json +++ b/ci/test-file-size-budget.json @@ -5,11 +5,11 @@ "nemoclaw/src/commands/migration-state.test.ts": 1566, "src/lib/inference/nim.test.ts": 2079, "src/lib/onboard/preflight.test.ts": 1905, - "test/channels-add-preset.test.ts": 1915, + "test/channels-add-preset.test.ts": 1901, "test/generate-openclaw-config.test.ts": 2106, "test/install-preflight.test.ts": 4397, "test/nemoclaw-start.test.ts": 5300, - "test/onboard-messaging.test.ts": 2122, + "test/onboard-messaging.test.ts": 2110, "test/onboard-selection.test.ts": 7757, "test/onboard.test.ts": 4887, "test/policies.test.ts": 2763 diff --git a/src/lib/actions/sandbox/policy-channel-conflict.test.ts b/src/lib/actions/sandbox/policy-channel-conflict.test.ts index 323b50ded8..b76f53c17d 100644 --- a/src/lib/actions/sandbox/policy-channel-conflict.test.ts +++ b/src/lib/actions/sandbox/policy-channel-conflict.test.ts @@ -18,7 +18,7 @@ // isNonInteractive is destructured at module load (`const { isNonInteractive } // = require("../../onboard")`), so it cannot be spied after load; it reads // process.env.NEMOCLAW_NON_INTERACTIVE === "1" at call time, which we drive -// directly. The real messaging-conflict, sandbox/channels, and credential-hash +// directly. The real messaging/applier, sandbox/channels, and credential-hash // modules run unmocked so the genuine hash + conflict logic is exercised. import { createRequire } from "node:module"; @@ -54,6 +54,56 @@ const { addSandboxChannel } = D("actions/sandbox/policy-channel.js") as { const TELEGRAM_TOKEN = "123456:AAH-secret-bot-token-value"; const TELEGRAM_HASH = hashCredential(TELEGRAM_TOKEN) as string; +// Build a minimal plan-backed SandboxEntry for conflict-detection fixtures. +// Callers supply credential bindings as { providerEnvKey, credentialHash? }. +function makePlanEntry( + name: string, + channelId: "telegram" | "slack" | "discord" | "wechat" | "whatsapp", + bindings: Array<{ providerEnvKey: string; credentialHash?: string }>, +): SandboxEntry { + return { + name, + messaging: { + schemaVersion: 1, + plan: { + schemaVersion: 1, + sandboxName: name, + agent: "openclaw", + workflow: "onboard", + channels: [ + { + channelId, + displayName: channelId, + authMode: "token-paste", + active: true, + selected: true, + configured: true, + disabled: false, + inputs: [], + hooks: [], + }, + ], + disabledChannels: [], + credentialBindings: bindings.map((b) => ({ + channelId, + credentialId: b.providerEnvKey.toLowerCase(), + sourceInput: b.providerEnvKey.toLowerCase(), + providerName: `${name}-${channelId}-bridge`, + providerEnvKey: b.providerEnvKey, + placeholder: `openshell:resolve:env:${b.providerEnvKey}`, + credentialAvailable: true, + ...(b.credentialHash ? { credentialHash: b.credentialHash } : {}), + })), + networkPolicy: { presets: [], entries: [] }, + agentRender: [], + buildSteps: [], + stateUpdates: [], + healthChecks: [], + }, + }, + } as unknown as SandboxEntry; +} + let spies: MockInstance[]; let logSpy: MockInstance; let errSpy: MockInstance; @@ -195,13 +245,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("interactive matching-token conflict: warns, user continues, add proceeds", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("y"); @@ -219,13 +263,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("interactive matching-token conflict: user aborts, nothing is mutated", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("n"); @@ -241,13 +279,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("interactive matching-token conflict: empty answer (default N) aborts", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue(""); // bare Enter -> default No @@ -262,13 +294,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("non-interactive matching-token conflict: aborts with exit(1) and guidance", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); process.env.NEMOCLAW_NON_INTERACTIVE = "1"; @@ -292,13 +318,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("--force bypasses the conflict even in non-interactive mode", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); process.env.NEMOCLAW_NON_INTERACTIVE = "1"; @@ -318,7 +338,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("unknown-token wording when the other sandbox has the channel but no hash", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [{ name: "bob", messagingChannels: ["telegram"] }], // no providerCredentialHashes + others: [{ name: "bob", messagingChannels: ["telegram"] }], // no plan — legacy entry, unknown-token }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("y"); @@ -334,15 +354,9 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("different hash on the other sandbox is NOT a conflict (no warning, add proceeds)", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { - TELEGRAM_BOT_TOKEN: hashCredential("a-completely-different-token") as string, - }, - }, - ], + others: [makePlanEntry("bob", "telegram", [ + { providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: hashCredential("a-completely-different-token") as string }, + ])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("n"); // would abort IF prompted; proves no prompt happens @@ -360,11 +374,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { // Scenario 6 it("idempotent same-sandbox re-add does not self-conflict", async () => { arrangeRegistry({ - current: { - name: "alpha", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, + current: makePlanEntry("alpha", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }]), }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("n"); // would abort IF prompted @@ -383,13 +393,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("--dry-run never runs the conflict check or touches credentials", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); await addSandboxChannel("alpha", { channel: "telegram", dryRun: true }); @@ -411,13 +415,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { const wechatHash = hashCredential(wechatToken) as string; arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["wechat"], - providerCredentialHashes: { WECHAT_BOT_TOKEN: wechatHash }, - }, - ], + others: [makePlanEntry("bob", "wechat", [{ providerEnvKey: "WECHAT_BOT_TOKEN", credentialHash: wechatHash }])], }); // The hook planner skips non-interactive host-QR enrollment, but the // conflict guard should still see a cached WeChat credential. @@ -439,13 +437,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("in-sandbox-qr whatsapp skips the credential conflict check", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["whatsapp"], - providerCredentialHashes: { WHATSAPP_TOKEN: "irrelevant" }, - }, - ], + others: [{ name: "bob", messagingChannels: ["whatsapp"] }], }); process.env.NEMOCLAW_NON_INTERACTIVE = "1"; @@ -463,11 +455,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, + makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }]), // Legacy entry with NO messagingChannels field — backfill probes the // (alive) gateway, gets "absent" for every provider, then writes // messagingChannels:[] for it. We make THAT write throw to genuinely @@ -511,17 +499,45 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { expect(updateSandboxMock).toHaveBeenCalledWith("alpha", expect.any(Object)); }); + it("non-interactive add aborts when the conflict check throws", async () => { + arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, others: [] }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + listSandboxesMock.mockImplementation(() => { + throw new Error("malformed messaging plan"); + }); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await expect(addSandboxChannel("alpha", { channel: "telegram" })).rejects.toThrow( + "process.exit(1)", + ); + + const text = loggedText(); + expect(text).toContain("Could not verify messaging channel conflicts"); + expect(text).toContain("rerun with --force"); + expect(upsertMock).not.toHaveBeenCalled(); + }); + + it("--force proceeds when the conflict check throws", async () => { + arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, others: [] }); + getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); + listSandboxesMock.mockImplementation(() => { + throw new Error("malformed messaging plan"); + }); + process.env.NEMOCLAW_NON_INTERACTIVE = "1"; + + await addSandboxChannel("alpha", { channel: "telegram", force: true }); + + const text = loggedText(); + expect(text).toContain("proceeding without a completed messaging channel conflict check"); + expect(exitMock).not.toHaveBeenCalled(); + expect(upsertMock).toHaveBeenCalledTimes(1); + }); + // Scenario 10 it("never prints the raw token value in any conflict output (proceed path)", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); promptMock.mockResolvedValue("y"); @@ -537,13 +553,7 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { it("non-interactive abort path also keeps the raw token out of output", async () => { arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: TELEGRAM_HASH }, - }, - ], + others: [makePlanEntry("bob", "telegram", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: TELEGRAM_HASH }])], }); getCredentialMock.mockReturnValue(TELEGRAM_TOKEN); process.env.NEMOCLAW_NON_INTERACTIVE = "1"; @@ -562,13 +572,9 @@ describe("addSandboxChannel cross-sandbox conflict check (#4305)", () => { const slackBotHash = hashCredential(slackBot) as string; arrangeRegistry({ current: { name: "alpha", messagingChannels: [] }, - others: [ - { - name: "bob", - messagingChannels: ["slack"], - providerCredentialHashes: { SLACK_BOT_TOKEN: slackBotHash }, // only bot token matches - }, - ], + // only bot token stored — app token unknown → conservative unknown-token OR + // matching-token if bot token matches; test verifies the conflict is surfaced. + others: [makePlanEntry("bob", "slack", [{ providerEnvKey: "SLACK_BOT_TOKEN", credentialHash: slackBotHash }])], }); getCredentialMock.mockImplementation((key: string) => { if (key === "SLACK_BOT_TOKEN") return slackBot; diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index 3cfbeec6b7..7f39e295e3 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -358,7 +358,7 @@ function bridgeProviderName(sandboxName: string, channelName: string, envKey: st return `${sandboxName}-${channelName}-bridge`; } -// Tri-state gateway probe for cross-sandbox messaging-conflict backfill, +// Tri-state gateway probe for cross-sandbox messaging conflict backfill, // mirroring onboard.ts makeConflictProbe(). An upfront liveness check keeps a // transient gateway failure ("error") from being mis-recorded as "no // providers" ("absent"), which would permanently suppress backfill retries. @@ -385,30 +385,34 @@ function makeChannelsConflictProbe() { // Detect whether another sandbox already uses one of this channel's // credentials. Mirrors the onboard.ts conflict check. Returns true if the // caller should PROCEED with the add, false if it should abort. Never logs -// credential values — only the non-secret hashes computed inline are passed -// to findChannelConflicts. Probe/backfill failures are swallowed -// (non-fatal): backfillMessagingChannels already skips sandboxes whose probe -// errors, so a flaky gateway degrades to "no conflict found" rather than -// blocking the add. +// credential values. Backfill probe failures are non-fatal, but core +// conflict-detection errors fail closed unless --force is set. async function checkChannelAddConflict( sandboxName: string, channelName: string, acquired: Record, force: boolean, ): Promise { - // QR-paired / tokenless adds have empty `acquired` and no host-side - // credential to hash. Skip — there is no credential to collide on, and - // findChannelConflicts with empty credentialHashes would only ever report - // "unknown-token" noise against every other sandbox holding the channel. + // Build credential hashes from the manifest's declared providerEnvKey values. + // This scopes the lookup to the channel's known credential keys, mirroring + // what planToConflictChannelRequests() produces from bindings. QR-only + // channels (e.g. WhatsApp) have no manifest credentials → early exit with no + // conflict possible. Unknown channelName → also exits early. + const channelManifest = createBuiltInChannelManifestRegistry() + .list() + .find((m) => m.id === channelName); + if (!channelManifest || channelManifest.credentials.length === 0) return true; + const credentialHashes: Record = {}; - for (const [envKey, token] of Object.entries(acquired)) { - const hash = hashCredential(token); - if (hash) credentialHashes[envKey] = hash; + for (const cred of channelManifest.credentials) { + const token = acquired[cred.providerEnvKey]; + const hash = token ? hashCredential(token) : null; + if (hash) credentialHashes[cred.providerEnvKey] = hash; } if (Object.keys(credentialHashes).length === 0) return true; const { backfillMessagingChannels, findChannelConflicts } = - require("../../messaging-conflict") as typeof import("../../messaging-conflict"); + require("../../messaging/applier") as typeof import("../../messaging/applier"); try { backfillMessagingChannels(registry, makeChannelsConflictProbe()); @@ -418,9 +422,32 @@ async function checkChannelAddConflict( let conflicts: ReturnType; try { - conflicts = findChannelConflicts(sandboxName, [{ channel: channelName, credentialHashes }], registry); - } catch { - return true; + conflicts = findChannelConflicts( + sandboxName, + [{ channel: channelName, credentialHashes }], + registry, + ); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error(` Could not verify messaging channel conflicts for ${channelName}: ${message}`); + if (force) { + console.log(" --force: proceeding without a completed messaging channel conflict check."); + return true; + } + if (isNonInteractive()) { + console.error( + ` Aborting: rerun with --force to skip the messaging channel conflict check for ${channelName}.`, + ); + process.exit(1); + } + const answer = ( + await askPrompt(" Continue without a completed conflict check? [y/N]: ") + ) + .trim() + .toLowerCase(); + if (answer === "y" || answer === "yes") return true; + console.log(" Aborting channel add."); + return false; } if (conflicts.length === 0) return true; @@ -489,16 +516,9 @@ async function applyChannelAddToGatewayAndRegistry( const enabled = new Set(entry.messagingChannels || []); enabled.add(channelName); const disabled = (entry.disabledChannels || []).filter((c: string) => c !== channelName); - const providerCredentialHashes = { ...(entry.providerCredentialHashes || {}) }; - for (const [envKey, token] of Object.entries(acquired)) { - const hash = hashCredential(token); - if (hash) providerCredentialHashes[envKey] = hash; - } registry.updateSandbox(sandboxName, { messagingChannels: Array.from(enabled).sort(), disabledChannels: disabled, - providerCredentialHashes: - Object.keys(providerCredentialHashes).length > 0 ? providerCredentialHashes : undefined, }); } } @@ -609,15 +629,7 @@ async function applyChannelRemoveToGatewayAndRegistry( const entry = registry.getSandbox(sandboxName); if (entry) { const enabled = (entry.messagingChannels || []).filter((c: string) => c !== channelName); - const providerCredentialHashes = { ...(entry.providerCredentialHashes || {}) }; - for (const envKey of channelTokenKeys) { - delete providerCredentialHashes[envKey]; - } - registry.updateSandbox(sandboxName, { - messagingChannels: enabled, - providerCredentialHashes: - Object.keys(providerCredentialHashes).length > 0 ? providerCredentialHashes : undefined, - }); + registry.updateSandbox(sandboxName, { messagingChannels: enabled }); } return { ok: residual.length === 0, residual }; @@ -1090,9 +1102,6 @@ export async function addSandboxChannel( ? [...priorEntry.messagingChannels] : []; const wasAlreadyEnabled = priorMessagingChannels.includes(canonical); - const priorHashes: Record = { - ...((priorEntry?.providerCredentialHashes as Record) || {}), - }; const channelTokenKeys = getChannelTokenKeys(channelDef); const priorCreds: Record = {}; for (const key of channelTokenKeys) { @@ -1112,7 +1121,6 @@ export async function addSandboxChannel( await rollbackChannelAdd(sandboxName, channelDef, canonical, { wasAlreadyEnabled, priorMessagingChannels, - priorHashes, priorCreds, }); process.exit(1); @@ -1132,7 +1140,6 @@ async function rollbackChannelAdd( snapshot: { wasAlreadyEnabled: boolean; priorMessagingChannels: string[]; - priorHashes: Record; priorCreds: Record; }, ): Promise<{ ok: boolean; residual: string[] }> { @@ -1142,8 +1149,6 @@ async function rollbackChannelAdd( ); registry.updateSandbox(sandboxName, { messagingChannels: snapshot.priorMessagingChannels, - providerCredentialHashes: - Object.keys(snapshot.priorHashes).length > 0 ? snapshot.priorHashes : undefined, }); clearChannelTokens(channel); if (Object.keys(snapshot.priorCreds).length > 0) { diff --git a/src/lib/actions/sandbox/rebuild.ts b/src/lib/actions/sandbox/rebuild.ts index 63f06a0219..400ab5be4f 100644 --- a/src/lib/actions/sandbox/rebuild.ts +++ b/src/lib/actions/sandbox/rebuild.ts @@ -823,7 +823,6 @@ export async function rebuildSandbox( ...(hasRebuildHermesToolGateways ? { hermesToolGateways: [...rebuildHermesToolGateways] } : {}), - ...(sb.providerCredentialHashes ? { providerCredentialHashes: sb.providerCredentialHashes } : {}), }; if (Object.keys(preservedRegistryFields).length > 0) { registry.updateSandbox(sandboxName, preservedRegistryFields); diff --git a/src/lib/inventory/index.ts b/src/lib/inventory/index.ts index fabf5b5ff6..85a1153e80 100644 --- a/src/lib/inventory/index.ts +++ b/src/lib/inventory/index.ts @@ -18,7 +18,6 @@ export interface SandboxEntry { openshellDriver?: string | null; openshellVersion?: string | null; policies?: string[] | null; - providerCredentialHashes?: Record | null; messagingChannels?: string[] | null; agent?: string | null; dashboardPort?: number | null; diff --git a/src/lib/messaging-conflict.ts b/src/lib/messaging-conflict.ts deleted file mode 100644 index 6d31d6cc64..0000000000 --- a/src/lib/messaging-conflict.ts +++ /dev/null @@ -1,232 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Cross-sandbox messaging-channel conflict detection. -// -// Telegram (getUpdates long-polling), Discord (gateway connection), and Slack -// (Socket Mode) all enforce one active consumer per channel credential. Two -// sandboxes sharing the same token silently break both bridges; see issue #1953. -// -// The registry persists which channels each sandbox uses plus a non-secret hash -// of the provider credential when available. This module detects true same-token -// overlaps and — because pre-existing sandboxes created before the field was -// added have no record — can optionally backfill the channel field by probing -// the live OpenShell gateway for known provider names. - -import type { SandboxEntry } from "./state/registry"; -import { getChannelDef, getChannelTokenKeys } from "./sandbox/channels"; - -type ProbeResult = "present" | "absent" | "error"; -type ConflictReason = "matching-token" | "unknown-token"; - -interface ConflictProbe { - // Tri-state — "error" is distinct from "absent" so a transient gateway - // failure does not get collapsed into "provider not attached" and then - // persisted as a bogus empty messagingChannels. - providerExists: (name: string) => ProbeResult; -} - -interface ConflictRegistry { - listSandboxes: () => { sandboxes: SandboxEntry[]; defaultSandbox?: string | null }; - updateSandbox: (name: string, updates: Partial) => boolean; -} - -interface RequestedChannel { - channel: string; - credentialHashes?: Record; -} - -type ChannelRequest = string | RequestedChannel; - -interface Conflict { - channel: string; - sandbox: string; - reason: ConflictReason; -} - -// NemoClaw attaches one OpenShell provider per messaging channel per sandbox. -// The provider name pattern is established in src/lib/onboard.ts at sandbox -// creation time; when a sandbox predates the messagingChannels registry field, -// the live provider is the only record of which channels it uses. -const PROVIDER_SUFFIXES: Record = { - telegram: "-telegram-bridge", - discord: "-discord-bridge", - slack: "-slack-bridge", - wechat: "-wechat-bridge", -}; - -const KNOWN_CHANNELS = Object.keys(PROVIDER_SUFFIXES); - -function normalizeRequest(request: ChannelRequest): RequestedChannel | null { - if (typeof request === "string") { - return request ? { channel: request, credentialHashes: {} } : null; - } - if (!request || typeof request.channel !== "string" || request.channel.length === 0) return null; - return request; -} - -function getTokenKeys(channel: string): string[] { - const def = getChannelDef(channel); - return def ? getChannelTokenKeys(def) : []; -} - -function hasStoredChannel(entry: SandboxEntry, channel: string): boolean { - if (!Array.isArray(entry.messagingChannels) || !entry.messagingChannels.includes(channel)) { - return false; - } - // A `channels stop` sandbox keeps the channel in messagingChannels so a later - // `channels start` can recover, but its bridge is paused — the credential is - // not in use, so it must not block another sandbox from claiming the token. - return !(Array.isArray(entry.disabledChannels) && entry.disabledChannels.includes(channel)); -} - -function conflictReasonForRequest( - entry: SandboxEntry, - request: RequestedChannel, -): ConflictReason | null { - if (!hasStoredChannel(entry, request.channel)) return null; - const requestedHashes = request.credentialHashes || {}; - const storedHashes = entry.providerCredentialHashes || {}; - const tokenKeys = getTokenKeys(request.channel); - const comparisonKeys = tokenKeys.length > 0 ? tokenKeys : Object.keys(requestedHashes); - if (comparisonKeys.length === 0) return "unknown-token"; - - let sawUnknown = false; - for (const key of comparisonKeys) { - const requestedHash = requestedHashes[key] || null; - const storedHash = storedHashes[key] || null; - if (requestedHash && storedHash) { - if (requestedHash === storedHash) return "matching-token"; - continue; - } - sawUnknown = true; - } - return sawUnknown ? "unknown-token" : null; -} - -function conflictReasonForPair( - channel: string, - left: SandboxEntry, - right: SandboxEntry, -): ConflictReason | null { - if (!hasStoredChannel(left, channel) || !hasStoredChannel(right, channel)) return null; - const tokenKeys = getTokenKeys(channel); - if (tokenKeys.length === 0) return "unknown-token"; - - let sawUnknown = false; - for (const key of tokenKeys) { - const leftHash = left.providerCredentialHashes?.[key] || null; - const rightHash = right.providerCredentialHashes?.[key] || null; - if (leftHash && rightHash) { - if (leftHash === rightHash) return "matching-token"; - continue; - } - sawUnknown = true; - } - return sawUnknown ? "unknown-token" : null; -} - -/** - * For registry entries missing `messagingChannels`, probe OpenShell to infer - * which channels the sandbox was onboarded with, and write the result back to - * the registry. Safe to call repeatedly — entries with the field set are left - * alone. Failures to probe any one sandbox are swallowed so that a flaky - * gateway does not block status or onboarding. - */ -export function backfillMessagingChannels( - registry: ConflictRegistry, - probe: ConflictProbe, -): void { - const { sandboxes } = registry.listSandboxes(); - for (const entry of sandboxes) { - if (Array.isArray(entry.messagingChannels)) continue; - const discovered: string[] = []; - let probeFailed = false; - for (const channel of KNOWN_CHANNELS) { - const providerName = `${entry.name}${PROVIDER_SUFFIXES[channel]}`; - let state: ProbeResult; - try { - state = probe.providerExists(providerName); - } catch { - state = "error"; - } - if (state === "present") { - discovered.push(channel); - } else if (state === "error") { - // Partial results can't be persisted: writing a partial/empty list - // sets messagingChannels, preventing future retries and permanently - // hiding real overlaps. Skip the write so we retry on next call. - probeFailed = true; - break; - } - } - if (!probeFailed) { - registry.updateSandbox(entry.name, { messagingChannels: discovered }); - } - } -} - -/** - * Return every (channel, other-sandbox) pair where another sandbox in the - * registry already has one of the requested channels in use with either a - * matching credential hash or insufficient hash metadata to prove it differs. - */ -export function findChannelConflicts( - currentSandbox: string | null, - enabledChannels: ChannelRequest[], - registry: ConflictRegistry, -): Conflict[] { - if (!Array.isArray(enabledChannels) || enabledChannels.length === 0) return []; - const requests = enabledChannels.map(normalizeRequest).filter((r): r is RequestedChannel => !!r); - if (requests.length === 0) return []; - const { sandboxes } = registry.listSandboxes(); - const others = sandboxes.filter( - (s) => s.name !== currentSandbox && Array.isArray(s.messagingChannels), - ); - return requests.flatMap((request) => - others.flatMap((sandbox) => { - const reason = conflictReasonForRequest(sandbox, request); - return reason ? [{ channel: request.channel, sandbox: sandbox.name, reason }] : []; - }), - ); -} - -/** - * Detect overlaps across every sandbox in the registry, returning each pair at - * most once. Used by `nemoclaw status` to warn users whose sandboxes already - * share a messaging token or whose legacy metadata is too old to verify. - */ -export function findAllOverlaps(registry: ConflictRegistry): Array<{ - channel: string; - sandboxes: [string, string]; - reason: ConflictReason; -}> { - const { sandboxes } = registry.listSandboxes(); - const byChannel = new Map(); - for (const entry of sandboxes) { - if (!Array.isArray(entry.messagingChannels)) continue; - const disabled = new Set( - Array.isArray(entry.disabledChannels) ? entry.disabledChannels : [], - ); - for (const channel of entry.messagingChannels) { - if (disabled.has(channel)) continue; - const list = byChannel.get(channel) || []; - list.push(entry); - byChannel.set(channel, list); - } - } - const overlaps: Array<{ channel: string; sandboxes: [string, string]; reason: ConflictReason }> = - []; - for (const [channel, entries] of byChannel) { - if (entries.length < 2) continue; - for (let i = 0; i < entries.length; i += 1) { - for (let j = i + 1; j < entries.length; j += 1) { - const reason = conflictReasonForPair(channel, entries[i], entries[j]); - if (reason) { - overlaps.push({ channel, sandboxes: [entries[i].name, entries[j].name], reason }); - } - } - } - } - return overlaps; -} diff --git a/src/lib/messaging/applier/conflict-detection-entry.test.ts b/src/lib/messaging/applier/conflict-detection-entry.test.ts new file mode 100644 index 0000000000..94c0fddac7 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection-entry.test.ts @@ -0,0 +1,191 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + discordBinding, + discordChannel, + makePlan, + planEntry, + slackBindings, + slackChannel, + tgBinding, + tgChannel, + whatsappChannel, +} from "../../../../test/helpers/messaging-conflict-fixtures"; +import { + conflictReasonForPair, + conflictReasonForRequest, + hasStoredChannelInEntry, +} from "./conflict-detection"; + +describe("hasStoredChannelInEntry", () => { + it("returns true for an active channel in a plan-backed entry", () => { + const entry = planEntry("sb", makePlan("sb", { channels: [tgChannel()] })); + expect(hasStoredChannelInEntry(entry, "telegram")).toBe(true); + }); + + it("returns false for disabled or inactive plan channels", () => { + expect( + hasStoredChannelInEntry( + planEntry( + "sb", + makePlan("sb", { disabledChannels: ["telegram"], channels: [tgChannel(true, true)] }), + ), + "telegram", + ), + ).toBe(false); + expect( + hasStoredChannelInEntry( + planEntry("sb", makePlan("sb", { channels: [tgChannel(false, false)] })), + "telegram", + ), + ).toBe(false); + }); +}); + +describe("conflictReasonForRequest", () => { + it("detects matching-token when same channel hash matches", () => { + const entry = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "telegram", + credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, + }), + ).toBe("matching-token"); + }); + + it("returns null when same channel hash differs", () => { + const entry = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "telegram", + credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-b" }, + }), + ).toBeNull(); + }); + + it("does not produce false positives from unrelated-channel hashes", () => { + const entry = planEntry( + "alice", + makePlan("alice", { + channels: [tgChannel(), slackChannel()], + credentialBindings: [tgBinding("hash-tg-a"), ...slackBindings("hash-slack")], + }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "telegram", + credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-tg-b" }, + }), + ).toBeNull(); + }); + + it("returns unknown-token when plan has no hashes for the channel", () => { + const entry = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding()] }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "telegram", + credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, + }), + ).toBe("unknown-token"); + }); + + it("returns null for credential-less channels with no comparison keys", () => { + const entry = planEntry("alice", makePlan("alice", { channels: [whatsappChannel()] })); + expect( + conflictReasonForRequest(entry, { + channel: "whatsapp", + credentialHashes: {}, + }), + ).toBeNull(); + }); +}); + +describe("conflictReasonForPair", () => { + it("detects matching-token between two plan-backed entries", () => { + const alice = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect(conflictReasonForPair("telegram", alice, bob)).toBe("matching-token"); + }); + + it("returns null when same-channel hashes differ", () => { + const alice = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-b")] }), + ); + expect(conflictReasonForPair("telegram", alice, bob)).toBeNull(); + }); + + it("covers Discord matching and distinct plan-backed hashes", () => { + const matchingAlice = planEntry( + "alice", + makePlan("alice", { + channels: [discordChannel()], + credentialBindings: [discordBinding("hash-discord")], + }), + ); + const matchingBob = planEntry( + "bob", + makePlan("bob", { + channels: [discordChannel()], + credentialBindings: [discordBinding("hash-discord")], + }), + ); + expect(conflictReasonForPair("discord", matchingAlice, matchingBob)).toBe("matching-token"); + + const distinctBob = planEntry( + "bob", + makePlan("bob", { + channels: [discordChannel()], + credentialBindings: [discordBinding("hash-discord-b")], + }), + ); + expect(conflictReasonForPair("discord", matchingAlice, distinctBob)).toBeNull(); + }); + + it("scopes comparison to the requested channel", () => { + const alice = planEntry( + "alice", + makePlan("alice", { + channels: [tgChannel(), slackChannel()], + credentialBindings: [tgBinding("hash-tg-a"), ...slackBindings("hash-slack")], + }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { + channels: [tgChannel(), slackChannel()], + credentialBindings: [tgBinding("hash-tg-b"), ...slackBindings("hash-slack")], + }), + ); + expect(conflictReasonForPair("telegram", alice, bob)).toBeNull(); + expect(conflictReasonForPair("slack", alice, bob)).toBe("matching-token"); + }); + + it("returns null for credential-less channel pairs with no comparison keys", () => { + const alice = planEntry("alice", makePlan("alice", { channels: [whatsappChannel()] })); + const bob = planEntry("bob", makePlan("bob", { channels: [whatsappChannel()] })); + expect(conflictReasonForPair("whatsapp", alice, bob)).toBeNull(); + }); +}); diff --git a/src/lib/messaging-conflict.test.ts b/src/lib/messaging/applier/conflict-detection-legacy.test.ts similarity index 73% rename from src/lib/messaging-conflict.test.ts rename to src/lib/messaging/applier/conflict-detection-legacy.test.ts index c366c7bf5a..07ed0ccfc3 100644 --- a/src/lib/messaging-conflict.test.ts +++ b/src/lib/messaging/applier/conflict-detection-legacy.test.ts @@ -1,17 +1,20 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it, vi } from "vitest"; +// Legacy-field (messagingChannels / disabledChannels) conflict tests. +// Hash-precise plan-backed tests are split across conflict-detection-entry, conflict-detection-overlap, and conflict-detection-multi-credential tests -import type { SandboxEntry } from "./state/registry"; +import { describe, expect, it, vi } from "vitest"; +import { makePlan } from "../../../../test/helpers/messaging-conflict-fixtures"; +import type { SandboxEntry } from "../../state/registry"; import { backfillMessagingChannels, findAllOverlaps, findChannelConflicts, -} from "./messaging-conflict"; + type MessagingConflictProbe, +} from "./conflict-detection"; -type ConflictProbe = Parameters[1]; -type ProviderExists = ConflictProbe["providerExists"]; +type ProviderExists = MessagingConflictProbe["providerExists"]; function makeRegistry(sandboxes: SandboxEntry[]) { const store = new Map(sandboxes.map((s) => [s.name, { ...s }])); @@ -40,18 +43,10 @@ describe("findChannelConflicts", () => { ]); }); - it("returns conflicts only when the same channel credential hash matches", () => { + it("returns unknown-token for any legacy entry sharing the channel (no hash data)", () => { const registry = makeRegistry([ - { - name: "alice", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, - { - name: "carol", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-c" }, - }, + { name: "alice", messagingChannels: ["telegram"] }, + { name: "carol", messagingChannels: ["telegram"] }, ]); expect( findChannelConflicts( @@ -59,24 +54,10 @@ describe("findChannelConflicts", () => { [{ channel: "telegram", credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" } }], registry, ), - ).toEqual([{ channel: "telegram", sandbox: "alice", reason: "matching-token" }]); - }); - - it("allows multiple telegram sandboxes with distinct token hashes", () => { - const registry = makeRegistry([ - { - name: "alice", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, + ).toEqual([ + { channel: "telegram", sandbox: "alice", reason: "unknown-token" }, + { channel: "telegram", sandbox: "carol", reason: "unknown-token" }, ]); - expect( - findChannelConflicts( - "bob", - [{ channel: "telegram", credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-b" } }], - registry, - ), - ).toEqual([]); }); it("excludes the current sandbox from its own conflicts", () => { @@ -100,7 +81,6 @@ describe("findChannelConflicts", () => { name: "alice", messagingChannels: ["telegram"], disabledChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, }, ]); expect( @@ -138,40 +118,6 @@ describe("findAllOverlaps", () => { ]); }); - it("does not report overlaps when same-channel credential hashes differ", () => { - const registry = makeRegistry([ - { - name: "alice", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-b" }, - }, - ]); - expect(findAllOverlaps(registry)).toEqual([]); - }); - - it("reports matching-token overlaps when same-channel credential hashes match", () => { - const registry = makeRegistry([ - { - name: "alice", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, - ]); - expect(findAllOverlaps(registry)).toEqual([ - { channel: "telegram", sandboxes: ["alice", "bob"], reason: "matching-token" }, - ]); - }); - it("returns empty when channels do not overlap", () => { const registry = makeRegistry([ { name: "alice", messagingChannels: ["telegram"] }, @@ -186,13 +132,8 @@ describe("findAllOverlaps", () => { name: "alice", messagingChannels: ["telegram"], disabledChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, - }, - { - name: "bob", - messagingChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" }, }, + { name: "bob", messagingChannels: ["telegram"] }, ]); expect(findAllOverlaps(registry)).toEqual([]); }); @@ -201,7 +142,7 @@ describe("findAllOverlaps", () => { describe("backfillMessagingChannels", () => { it("fills in missing messagingChannels by probing OpenShell", () => { const registry = makeRegistry([{ name: "alice" }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn((name) => name === "alice-telegram-bridge" ? "present" : "absent", ), @@ -217,11 +158,8 @@ describe("backfillMessagingChannels", () => { }); it("backfills wechat when only the wechat bridge provider is present", () => { - // The probe-by-suffix mechanism relies on every channel having an entry - // in PROVIDER_SUFFIXES; if wechat were ever dropped from that map, this - // test starts catching the absent provider. const registry = makeRegistry([{ name: "alice" }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn((name) => name === "alice-wechat-bridge" ? "present" : "absent", ), @@ -244,7 +182,22 @@ describe("backfillMessagingChannels", () => { it("leaves entries with existing messagingChannels alone", () => { const registry = makeRegistry([{ name: "alice", messagingChannels: ["telegram"] }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { + providerExists: vi.fn(() => "present"), + }; + backfillMessagingChannels(registry, probe); + expect(registry.updateSandbox).not.toHaveBeenCalled(); + expect(probe.providerExists).not.toHaveBeenCalled(); + }); + + it("skips plan-backed entries without legacy messagingChannels", () => { + const registry = makeRegistry([ + { + name: "alice", + messaging: { schemaVersion: 1, plan: makePlan("alice") }, + } as unknown as SandboxEntry, + ]); + const probe: MessagingConflictProbe = { providerExists: vi.fn(() => "present"), }; backfillMessagingChannels(registry, probe); @@ -254,7 +207,7 @@ describe("backfillMessagingChannels", () => { it("writes an empty array when all probes return absent", () => { const registry = makeRegistry([{ name: "alice" }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn(() => "absent"), }; backfillMessagingChannels(registry, probe); @@ -262,11 +215,8 @@ describe("backfillMessagingChannels", () => { }); it("does NOT persist when a probe returns error (retry on next call)", () => { - // "error" is distinct from "absent": a transient gateway failure must not - // be collapsed into "provider not attached" and persisted, because that - // would prevent all future backfill retries and hide real overlaps. const registry = makeRegistry([{ name: "alice" }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn((name) => { if (name.endsWith("-telegram-bridge")) return "error"; return name.endsWith("-discord-bridge") ? "present" : "absent"; @@ -278,7 +228,7 @@ describe("backfillMessagingChannels", () => { it("also treats a thrown probe as error (defensive; callers should return 'error' instead)", () => { const registry = makeRegistry([{ name: "alice" }]); - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn(() => { throw new Error("unexpected"); }), @@ -290,7 +240,7 @@ describe("backfillMessagingChannels", () => { it("re-attempts backfill on a subsequent call after a prior error", () => { const registry = makeRegistry([{ name: "alice" }]); let firstPass = true; - const probe: ConflictProbe = { + const probe: MessagingConflictProbe = { providerExists: vi.fn((name) => { if (name.endsWith("-telegram-bridge") && firstPass) { firstPass = false; diff --git a/src/lib/messaging/applier/conflict-detection-multi-credential.test.ts b/src/lib/messaging/applier/conflict-detection-multi-credential.test.ts new file mode 100644 index 0000000000..0857d8fb09 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection-multi-credential.test.ts @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + makePlan, + planEntry, + slackAppBinding, + slackBotBinding, + slackChannel, +} from "../../../../test/helpers/messaging-conflict-fixtures"; +import { conflictReasonForPair, conflictReasonForRequest } from "./conflict-detection"; + +describe("multi-credential channel partial hash suppression", () => { + it("request comparison returns unknown-token when Slack app token is missing", () => { + const entry = planEntry( + "alice", + makePlan("alice", { + channels: [slackChannel()], + credentialBindings: [slackBotBinding("hash-bot-a", "alice")], + }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "slack", + credentialHashes: { SLACK_BOT_TOKEN: "hash-bot-b", SLACK_APP_TOKEN: "hash-app-x" }, + }), + ).toBe("unknown-token"); + }); + + it("request comparison returns null when both Slack token hashes differ", () => { + const entry = planEntry( + "alice", + makePlan("alice", { + channels: [slackChannel()], + credentialBindings: [ + slackBotBinding("hash-bot-a", "alice"), + slackAppBinding("hash-app-a", "alice"), + ], + }), + ); + expect( + conflictReasonForRequest(entry, { + channel: "slack", + credentialHashes: { SLACK_BOT_TOKEN: "hash-bot-b", SLACK_APP_TOKEN: "hash-app-b" }, + }), + ).toBeNull(); + }); + + it("pair comparison returns unknown-token when Slack app token is absent from both plans", () => { + const alice = planEntry( + "alice", + makePlan("alice", { + channels: [slackChannel()], + credentialBindings: [slackBotBinding("hash-bot-a", "alice")], + }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { + channels: [slackChannel()], + credentialBindings: [slackBotBinding("hash-bot-b", "bob")], + }), + ); + expect(conflictReasonForPair("slack", alice, bob)).toBe("unknown-token"); + }); + + it("pair comparison returns null when both Slack token hashes differ", () => { + const alice = planEntry( + "alice", + makePlan("alice", { + channels: [slackChannel()], + credentialBindings: [ + slackBotBinding("hash-bot-a", "alice"), + slackAppBinding("hash-app-a", "alice"), + ], + }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { + channels: [slackChannel()], + credentialBindings: [ + slackBotBinding("hash-bot-b", "bob"), + slackAppBinding("hash-app-b", "bob"), + ], + }), + ); + expect(conflictReasonForPair("slack", alice, bob)).toBeNull(); + }); +}); diff --git a/src/lib/messaging/applier/conflict-detection-overlap.test.ts b/src/lib/messaging/applier/conflict-detection-overlap.test.ts new file mode 100644 index 0000000000..57bb5661e1 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection-overlap.test.ts @@ -0,0 +1,85 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + makePlan, + planEntry, + tgBinding, + tgChannel, + whatsappChannel, +} from "../../../../test/helpers/messaging-conflict-fixtures"; +import { detectAllOverlapsInEntries, findConflictsInEntries } from "./conflict-detection"; + +describe("findConflictsInEntries", () => { + it("detects matching-token against a plan-only entry", () => { + const alice = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect( + findConflictsInEntries( + "bob", + [{ channel: "telegram", credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" } }], + [alice], + ), + ).toEqual([{ channel: "telegram", sandbox: "alice", reason: "matching-token" }]); + }); + + it("ignores a disabled channel in a plan-backed entry", () => { + const alice = planEntry( + "alice", + makePlan("alice", { + disabledChannels: ["telegram"], + channels: [tgChannel(true, true)], + credentialBindings: [tgBinding("hash-a")], + }), + ); + expect( + findConflictsInEntries( + "bob", + [{ channel: "telegram", credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-a" } }], + [alice], + ), + ).toEqual([]); + }); +}); + +describe("detectAllOverlapsInEntries", () => { + it("reports matching-token overlap between two plan-backed entries", () => { + const alice = planEntry( + "alice", + makePlan("alice", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect(detectAllOverlapsInEntries([alice, bob])).toEqual([ + { channel: "telegram", sandboxes: ["alice", "bob"], reason: "matching-token" }, + ]); + }); + + it("does not report overlap when shared channel is disabled in one plan", () => { + const alice = planEntry( + "alice", + makePlan("alice", { + disabledChannels: ["telegram"], + channels: [tgChannel(true, true)], + credentialBindings: [tgBinding("hash-a")], + }), + ); + const bob = planEntry( + "bob", + makePlan("bob", { channels: [tgChannel()], credentialBindings: [tgBinding("hash-a")] }), + ); + expect(detectAllOverlapsInEntries([alice, bob])).toEqual([]); + }); + + it("does not report overlap for credential-less channels", () => { + const alice = planEntry("alice", makePlan("alice", { channels: [whatsappChannel()] })); + const bob = planEntry("bob", makePlan("bob", { channels: [whatsappChannel()] })); + expect(detectAllOverlapsInEntries([alice, bob])).toEqual([]); + }); +}); diff --git a/src/lib/messaging/applier/conflict-detection-plan.test.ts b/src/lib/messaging/applier/conflict-detection-plan.test.ts new file mode 100644 index 0000000000..94a9c47273 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection-plan.test.ts @@ -0,0 +1,126 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + makePlan, + slackBindings, + slackChannel, + tgBinding, + tgChannel, + whatsappChannel, +} from "../../../../test/helpers/messaging-conflict-fixtures"; +import { + getActiveChannelIdsFromPlan, + getCredentialHashesFromPlan, + planToConflictChannelRequests, +} from "./conflict-detection"; + +describe("getActiveChannelIdsFromPlan", () => { + it("returns active channel ids", () => { + const plan = makePlan("sb", { channels: [tgChannel(true, false)] }); + expect(getActiveChannelIdsFromPlan(plan)).toEqual(["telegram"]); + }); + + it("excludes disabled and inactive channels", () => { + expect( + getActiveChannelIdsFromPlan( + makePlan("sb", { disabledChannels: ["telegram"], channels: [tgChannel(true, false)] }), + ), + ).toEqual([]); + expect(getActiveChannelIdsFromPlan(makePlan("sb", { channels: [tgChannel(true, true)] }))).toEqual([]); + expect(getActiveChannelIdsFromPlan(makePlan("sb", { channels: [tgChannel(false, false)] }))).toEqual([]); + }); +}); + +describe("getCredentialHashesFromPlan", () => { + it("returns hashes keyed by providerEnvKey", () => { + const plan = makePlan("sb", { credentialBindings: [tgBinding("hash-x")] }); + expect(getCredentialHashesFromPlan(plan)).toEqual({ TELEGRAM_BOT_TOKEN: "hash-x" }); + }); + + it("scopes to a single channel when channelId is provided", () => { + const plan = makePlan("sb", { + credentialBindings: [tgBinding("hash-tg"), ...slackBindings("hash-bot", "hash-app")], + }); + expect(getCredentialHashesFromPlan(plan, "telegram")).toEqual({ + TELEGRAM_BOT_TOKEN: "hash-tg", + }); + expect(getCredentialHashesFromPlan(plan, "slack")).toEqual({ + SLACK_BOT_TOKEN: "hash-bot", + SLACK_APP_TOKEN: "hash-app", + }); + }); + + it("omits bindings without a credentialHash", () => { + const plan = makePlan("sb", { credentialBindings: [tgBinding()] }); + expect(getCredentialHashesFromPlan(plan)).toEqual({}); + }); +}); + +describe("planToConflictChannelRequests", () => { + it("returns one request per active channel that has a credential available", () => { + const plan = makePlan("sb", { + channels: [tgChannel()], + credentialBindings: [tgBinding("hash-tg")], + }); + expect(planToConflictChannelRequests(plan)).toEqual([ + { channel: "telegram", credentialHashes: { TELEGRAM_BOT_TOKEN: "hash-tg" } }, + ]); + }); + + it("includes available credentials without a hash for unknown-token fallback", () => { + const requests = planToConflictChannelRequests( + makePlan("sb", { channels: [tgChannel()], credentialBindings: [tgBinding()] }), + ); + expect(requests).toEqual([{ channel: "telegram", credentialHashes: {} }]); + }); + + it("groups multiple bindings for the same channel", () => { + const plan = makePlan("sb", { + channels: [slackChannel()], + credentialBindings: slackBindings("hash-bot", "hash-app"), + }); + expect(planToConflictChannelRequests(plan)).toEqual([ + { + channel: "slack", + credentialHashes: { SLACK_BOT_TOKEN: "hash-bot", SLACK_APP_TOKEN: "hash-app" }, + }, + ]); + }); + + it("skips inactive, disabled, unavailable, and absent channel bindings", () => { + expect( + planToConflictChannelRequests( + makePlan("sb", { + channels: [tgChannel()], + credentialBindings: [{ ...tgBinding("hash-tg"), credentialAvailable: false }], + }), + ), + ).toEqual([]); + expect( + planToConflictChannelRequests( + makePlan("sb", { + disabledChannels: ["telegram"], + channels: [tgChannel(true, true)], + credentialBindings: [tgBinding("hash-tg")], + }), + ), + ).toEqual([]); + expect(planToConflictChannelRequests(makePlan("sb", { credentialBindings: [tgBinding("hash-tg")] }))).toEqual([]); + expect( + planToConflictChannelRequests( + makePlan("sb", { + channels: [tgChannel(false, false)], + credentialBindings: [tgBinding("hash-tg")], + }), + ), + ).toEqual([]); + }); + + it("WhatsApp no-op: empty credentials produce no conflict requests", () => { + const plan = makePlan("sb", { channels: [whatsappChannel()], credentialBindings: [] }); + expect(planToConflictChannelRequests(plan)).toEqual([]); + }); +}); diff --git a/src/lib/messaging/applier/conflict-detection/backfill.ts b/src/lib/messaging/applier/conflict-detection/backfill.ts new file mode 100644 index 0000000000..b07e78846e --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/backfill.ts @@ -0,0 +1,74 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { PROVIDER_SUFFIXES } from "./manifest-metadata"; +import type { + ConflictRegistry, + ConflictRegistryEntry, + MessagingConflictProbe, + ProbeResult, +} from "./types"; + +/** + * For pre-plan entries missing `messagingChannels`, probe OpenShell to infer + * which channels the sandbox was onboarded with. Plan-backed entries are + * skipped even when the flat legacy field is absent. Probe errors abort the + * write for that sandbox so future calls can retry. + */ +export function backfillLegacyEntryChannels( + entries: readonly ConflictRegistryEntry[], + probe: MessagingConflictProbe, + updateEntry: (name: string, channels: string[]) => void, + providerSuffixes: Record, +): void { + for (const entry of entries) { + if (entry.messaging?.plan || Array.isArray(entry.messagingChannels)) continue; + const discovered: string[] = []; + let probeFailed = false; + for (const channel of Object.keys(providerSuffixes)) { + let channelPresent = false; + for (const suffix of providerSuffixes[channel]) { + let state: ProbeResult; + try { + state = probe.providerExists(`${entry.name}${suffix}`); + } catch { + state = "error"; + } + if (state === "present") { + channelPresent = true; + break; + } + if (state === "error") { + probeFailed = true; + break; + } + } + if (probeFailed) break; + if (channelPresent) discovered.push(channel); + } + if (!probeFailed) { + updateEntry(entry.name, discovered); + } + } +} + +/** + * Backfill pre-plan registry entries using built-in manifest provider names. + * This infers channel presence only; it must not restore legacy credential + * hashes. Remove with the `messagingChannels`/`disabledChannels` fallback once + * pre-plan registry rows are no longer supported. + */ +export function backfillMessagingChannels( + registry: ConflictRegistry, + probe: MessagingConflictProbe, +): void { + const { sandboxes } = registry.listSandboxes(); + backfillLegacyEntryChannels( + sandboxes, + probe, + (name, channels) => { + registry.updateSandbox(name, { messagingChannels: channels }); + }, + PROVIDER_SUFFIXES, + ); +} diff --git a/src/lib/messaging/applier/conflict-detection/entries.ts b/src/lib/messaging/applier/conflict-detection/entries.ts new file mode 100644 index 0000000000..cbb241db5d --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/entries.ts @@ -0,0 +1,201 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { CHANNEL_CREDENTIAL_ENV_KEYS } from "./manifest-metadata"; +import { getActiveChannelIdsFromPlan, getCredentialHashesFromPlan } from "./plan"; +import type { + ConflictMatch, + ConflictReason, + ConflictRegistryEntry, + ConflictRequest, +} from "./types"; + +/** + * Return the active (non-disabled) channel IDs for a registry entry. + * Uses `entry.messaging.plan` when available. Pre-plan registry entries are + * supported only for channel presence via the legacy + * `messagingChannels`/`disabledChannels` flat fields; legacy credential hashes + * are deliberately not recovered. Remove this branch when flat pre-plan + * messaging registry fields are no longer supported. Returns `null` when the + * entry has neither shape. + */ +export function resolveActiveChannelsFromEntry( + entry: ConflictRegistryEntry, +): string[] | null { + if (entry.messaging?.plan) { + return getActiveChannelIdsFromPlan(entry.messaging.plan); + } + if (!Array.isArray(entry.messagingChannels)) return null; + const disabled = new Set(Array.isArray(entry.disabledChannels) ? entry.disabledChannels : []); + return (entry.messagingChannels as string[]).filter((c) => !disabled.has(c)); +} + +/** + * Return credential hashes scoped to `channelId` for a registry entry. + * Plan-backed entries return channel-scoped hashes from `getCredentialHashesFromPlan`. + * Legacy entries without a plan return an empty map, which falls through to + * conservative `"unknown-token"` detection in the callers. + */ +function resolveChannelHashesFromEntry( + entry: ConflictRegistryEntry, + channelId: string, +): Record { + if (entry.messaging?.plan) { + return getCredentialHashesFromPlan(entry.messaging.plan, channelId); + } + return {}; +} + +/** + * True when `channel` is active (present and not disabled) in `entry`. + * Disabled channels must not block another sandbox from claiming the same + * token: the bridge is paused so the credential is not in use. + */ +export function hasStoredChannelInEntry( + entry: ConflictRegistryEntry, + channel: string, +): boolean { + return resolveActiveChannelsFromEntry(entry)?.includes(channel) ?? false; +} + +/** + * Determine the conflict reason between `entry`'s stored state and a new + * channel request, or `null` if there is no conflict. + * + * Comparison keys are taken from manifest-declared credentials for the channel + * so that a missing hash for one of multiple required credentials (e.g. Slack's + * SLACK_APP_TOKEN when only SLACK_BOT_TOKEN differs) conservatively marks the + * result as "unknown-token" rather than silently returning null. Falls back to + * the union of present stored/requested keys for channels not in the manifest. + */ +export function conflictReasonForRequest( + entry: ConflictRegistryEntry, + request: ConflictRequest, +): ConflictReason | null { + if (!hasStoredChannelInEntry(entry, request.channel)) return null; + const requestedHashes = request.credentialHashes ?? {}; + const storedHashes = resolveChannelHashesFromEntry(entry, request.channel); + const manifestKeys = CHANNEL_CREDENTIAL_ENV_KEYS[request.channel]; + const keys = + manifestKeys && manifestKeys.length > 0 + ? [...manifestKeys] + : Object.keys(storedHashes).length > 0 + ? Object.keys(storedHashes) + : Object.keys(requestedHashes); + if (keys.length === 0) return null; + + let sawUnknown = false; + for (const key of keys) { + const rh = (requestedHashes[key] as string | null | undefined) ?? null; + const sh = storedHashes[key] ?? null; + if (rh && sh) { + if (rh === sh) return "matching-token"; + continue; + } + sawUnknown = true; + } + return sawUnknown ? "unknown-token" : null; +} + +/** + * Determine the conflict reason between two registry entries sharing `channel`, + * or `null` if there is no conflict. Returns each pair at most once (the + * caller is responsible for ordered iteration). + * + * Comparison keys are taken from manifest-declared credentials for the channel + * so that a missing hash on either side conservatively produces "unknown-token" + * rather than null for multi-credential channels like Slack. + */ +export function conflictReasonForPair( + channel: string, + left: ConflictRegistryEntry, + right: ConflictRegistryEntry, +): ConflictReason | null { + if (!hasStoredChannelInEntry(left, channel) || !hasStoredChannelInEntry(right, channel)) { + return null; + } + const lh = resolveChannelHashesFromEntry(left, channel); + const rh = resolveChannelHashesFromEntry(right, channel); + const manifestKeys = CHANNEL_CREDENTIAL_ENV_KEYS[channel]; + const keys = + manifestKeys && manifestKeys.length > 0 + ? [...manifestKeys] + : [...new Set([...Object.keys(lh), ...Object.keys(rh)])]; + if (keys.length === 0) return null; + + let sawUnknown = false; + for (const key of keys) { + const l = lh[key] ?? null; + const r = rh[key] ?? null; + if (l && r) { + if (l === r) return "matching-token"; + continue; + } + sawUnknown = true; + } + return sawUnknown ? "unknown-token" : null; +} + +/** + * Return every (channel, other-sandbox) pair where another entry already has + * one of the requested channels in use with either a matching credential hash + * or insufficient hash metadata to prove it differs. + */ +export function findConflictsInEntries( + currentSandbox: string | null, + requests: readonly ConflictRequest[], + entries: readonly ConflictRegistryEntry[], +): ConflictMatch[] { + const others = entries.filter( + (e) => + e.name !== currentSandbox && + (Array.isArray(e.messagingChannels) || e.messaging?.plan != null), + ); + return requests.flatMap((request) => + others.flatMap((entry) => { + const reason = conflictReasonForRequest(entry, request); + return reason ? [{ channel: request.channel, sandbox: entry.name, reason }] : []; + }), + ); +} + +/** + * Detect overlaps across all entries, returning each pair at most once. + * Used by `nemoclaw status` to surface sandboxes that already share a token. + */ +export function detectAllOverlapsInEntries( + entries: readonly ConflictRegistryEntry[], +): Array<{ channel: string; sandboxes: [string, string]; reason: ConflictReason }> { + const byChannel = new Map(); + for (const entry of entries) { + const activeChannels = resolveActiveChannelsFromEntry(entry); + if (!activeChannels) continue; + for (const channel of activeChannels) { + const list = byChannel.get(channel) ?? []; + list.push(entry); + byChannel.set(channel, list); + } + } + + const overlaps: Array<{ + channel: string; + sandboxes: [string, string]; + reason: ConflictReason; + }> = []; + for (const [channel, channelEntries] of byChannel) { + if (channelEntries.length < 2) continue; + for (let i = 0; i < channelEntries.length; i += 1) { + for (let j = i + 1; j < channelEntries.length; j += 1) { + const reason = conflictReasonForPair(channel, channelEntries[i], channelEntries[j]); + if (reason) { + overlaps.push({ + channel, + sandboxes: [channelEntries[i].name, channelEntries[j].name], + reason, + }); + } + } + } + } + return overlaps; +} diff --git a/src/lib/messaging/applier/conflict-detection/index.ts b/src/lib/messaging/applier/conflict-detection/index.ts new file mode 100644 index 0000000000..2622b19fcf --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/index.ts @@ -0,0 +1,9 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export * from "./backfill"; +export * from "./entries"; +export * from "./plan"; +export * from "./probe"; +export * from "./registry"; +export type * from "./types"; diff --git a/src/lib/messaging/applier/conflict-detection/manifest-metadata.ts b/src/lib/messaging/applier/conflict-detection/manifest-metadata.ts new file mode 100644 index 0000000000..d793d97db6 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/manifest-metadata.ts @@ -0,0 +1,21 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { BUILT_IN_CHANNEL_MANIFESTS } from "../../channels"; + +// Map channelId to providerEnvKey values declared in built-in manifests. +// This is the primary key set for hash comparison so a missing credential for +// one of a channel's required credentials conservatively marks the comparison +// as unknown-token rather than silently returning null. +export const CHANNEL_CREDENTIAL_ENV_KEYS: Readonly> = + Object.fromEntries( + BUILT_IN_CHANNEL_MANIFESTS.map((m) => [m.id, m.credentials.map((c) => c.providerEnvKey)]), + ); + +export const PROVIDER_SUFFIXES: Record = Object.fromEntries( + BUILT_IN_CHANNEL_MANIFESTS.flatMap((m) => { + const suffixes = m.credentials.map((c) => c.providerName.replace("{sandboxName}", "")); + if (suffixes.length === 0) return []; + return [[m.id, suffixes]]; + }), +); diff --git a/src/lib/messaging/applier/conflict-detection/plan.ts b/src/lib/messaging/applier/conflict-detection/plan.ts new file mode 100644 index 0000000000..f6ef92aa93 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/plan.ts @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { SandboxMessagingPlan } from "../../manifest"; +import type { ConflictRequest } from "./types"; + +/** + * Return the channel IDs that are active (not disabled) in a compiled plan. + * Aligns with `enabledPlanChannels()` in plan-filter.ts: a channel is active + * only when `channel.active && !channel.disabled` AND it is not in + * `plan.disabledChannels`. + */ +export function getActiveChannelIdsFromPlan(plan: SandboxMessagingPlan): string[] { + const disabled = new Set(plan.disabledChannels); + return plan.channels + .filter((c) => c.active && !c.disabled && !disabled.has(c.channelId)) + .map((c) => c.channelId); +} + +/** + * Return credential hashes keyed by providerEnvKey from a compiled plan, + * optionally scoped to a single channel. + * + * Only bindings that carry a `credentialHash` are included. When `channelId` + * is provided only that channel's bindings are returned, which prevents + * hashes from other channels in the same sandbox from contaminating + * single-channel conflict comparisons. + */ +export function getCredentialHashesFromPlan( + plan: SandboxMessagingPlan, + channelId?: string, +): Record { + const hashes: Record = {}; + for (const b of plan.credentialBindings) { + if (channelId !== undefined && b.channelId !== channelId) continue; + if (b.credentialHash) hashes[b.providerEnvKey] = b.credentialHash; + } + return hashes; +} + +/** + * Build a `ConflictRequest[]` from a compiled plan's credential bindings. + * + * Groups bindings by channelId (e.g. Slack has SLACK_BOT_TOKEN and + * SLACK_APP_TOKEN) and excludes: + * - channels in `plan.disabledChannels` (bridge is paused, not in use) + * - bindings where the credential is not available (`credentialAvailable` + * false) - e.g. WhatsApp, which has no host-side token provider + * + * When a binding has no `credentialHash` (e.g. a registry-only resume that + * did not re-run the compiler), the channel is still included with an empty + * `credentialHashes` map, which falls through to `"unknown-token"` conservative + * detection. + */ +export function planToConflictChannelRequests(plan: SandboxMessagingPlan): ConflictRequest[] { + const activeChannelIds = new Set(getActiveChannelIdsFromPlan(plan)); + const byChannel = new Map>(); + + for (const binding of plan.credentialBindings) { + if (!activeChannelIds.has(binding.channelId) || !binding.credentialAvailable) continue; + const hashes = byChannel.get(binding.channelId) ?? {}; + if (binding.credentialHash) hashes[binding.providerEnvKey] = binding.credentialHash; + byChannel.set(binding.channelId, hashes); + } + + return Array.from(byChannel.entries()).map(([channel, credentialHashes]) => ({ + channel, + credentialHashes, + })); +} diff --git a/src/lib/messaging/applier/conflict-detection/probe.ts b/src/lib/messaging/applier/conflict-detection/probe.ts new file mode 100644 index 0000000000..96f2325628 --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/probe.ts @@ -0,0 +1,26 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { MessagingConflictProbe, MessagingConflictProbeGatewayDeps } from "./types"; + +/** + * Build a tri-state `MessagingConflictProbe` from plain openshell runner deps. + * + * The liveness result is cached so the `sandbox list` call is issued at most + * once per probe instance. A transient gateway failure (`checkGatewayLiveness` + * returns false) causes all subsequent `providerExists` calls to return "error" + * rather than "absent", preventing a flaky gateway from being mis-recorded as + * "no providers" and permanently suppressing future backfill retries. + */ +export function createMessagingConflictProbe( + deps: MessagingConflictProbeGatewayDeps, +): MessagingConflictProbe { + let alive: boolean | null = null; + return { + providerExists: (name) => { + if (alive === null) alive = deps.checkGatewayLiveness(); + if (!alive) return "error"; + return deps.providerExists(name) ? "present" : "absent"; + }, + }; +} diff --git a/src/lib/messaging/applier/conflict-detection/registry.ts b/src/lib/messaging/applier/conflict-detection/registry.ts new file mode 100644 index 0000000000..6b506b2d3a --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/registry.ts @@ -0,0 +1,61 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { SandboxMessagingPlan } from "../../manifest"; +import { detectAllOverlapsInEntries, findConflictsInEntries } from "./entries"; +import { planToConflictChannelRequests } from "./plan"; +import type { + ChannelConflictRequest, + ConflictMatch, + ConflictReason, + ConflictRegistry, + ConflictRequest, +} from "./types"; + +function normalizeRequest(request: ChannelConflictRequest): ConflictRequest | null { + if (typeof request === "string") { + return request ? { channel: request, credentialHashes: {} } : null; + } + if (!request || typeof request.channel !== "string" || request.channel.length === 0) return null; + return request; +} + +/** + * Registry-backed conflict lookup for callers that do not already have a + * compiled plan request list. + */ +export function findChannelConflicts( + currentSandbox: string | null, + enabledChannels: ChannelConflictRequest[], + registry: ConflictRegistry, +): ConflictMatch[] { + if (!Array.isArray(enabledChannels) || enabledChannels.length === 0) return []; + const requests = enabledChannels + .map(normalizeRequest) + .filter((request): request is ConflictRequest => request !== null); + if (requests.length === 0) return []; + const { sandboxes } = registry.listSandboxes(); + return findConflictsInEntries(currentSandbox, requests, sandboxes); +} + +/** + * Plan-driven variant of `findChannelConflicts`. Derives the channel request + * list from a compiled `SandboxMessagingPlan`. + */ +export function findChannelConflictsFromPlan( + currentSandbox: string | null, + plan: SandboxMessagingPlan, + registry: ConflictRegistry, +): ConflictMatch[] { + return findChannelConflicts(currentSandbox, planToConflictChannelRequests(plan), registry); +} + +/** + * Registry-backed overlap lookup used by status. + */ +export function findAllOverlaps( + registry: ConflictRegistry, +): Array<{ channel: string; sandboxes: [string, string]; reason: ConflictReason }> { + const { sandboxes } = registry.listSandboxes(); + return detectAllOverlapsInEntries(sandboxes); +} diff --git a/src/lib/messaging/applier/conflict-detection/types.ts b/src/lib/messaging/applier/conflict-detection/types.ts new file mode 100644 index 0000000000..fc33eaaefa --- /dev/null +++ b/src/lib/messaging/applier/conflict-detection/types.ts @@ -0,0 +1,55 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { SandboxMessagingPlan } from "../../manifest"; + +export type ProbeResult = "present" | "absent" | "error"; +export type ConflictReason = "matching-token" | "unknown-token"; + +export interface MessagingConflictProbe { + // Tri-state: "error" is distinct from "absent" so a transient gateway + // failure does not get collapsed into "provider not attached" and then + // persisted as bogus empty messagingChannels. + providerExists: (name: string) => ProbeResult; +} + +export interface MessagingConflictProbeGatewayDeps { + /** Run `openshell sandbox list`; return true if the gateway answered. */ + checkGatewayLiveness: () => boolean; + /** Check if the named OpenShell provider exists; assumes gateway is alive. */ + providerExists: (name: string) => boolean; +} + +export interface ConflictRequest { + readonly channel: string; + readonly credentialHashes?: Record; +} + +export interface ConflictMatch { + readonly channel: string; + readonly sandbox: string; + readonly reason: ConflictReason; +} + +export type ChannelConflictRequest = + | string + | { channel: string; credentialHashes?: Record }; + +/** + * Minimal shape of a registry entry that conflict detection needs. + * Satisfied by `SandboxEntry` from `./state/registry`. + */ +export interface ConflictRegistryEntry { + readonly name: string; + readonly messaging?: { readonly plan: SandboxMessagingPlan } | null; + readonly messagingChannels?: readonly string[] | null; + readonly disabledChannels?: readonly string[] | null; +} + +export interface ConflictRegistry { + listSandboxes: () => { + sandboxes: ConflictRegistryEntry[]; + defaultSandbox?: string | null; + }; + updateSandbox: (name: string, updates: { messagingChannels?: string[] }) => boolean; +} diff --git a/src/lib/messaging/applier/index.ts b/src/lib/messaging/applier/index.ts index f9e4dcf486..85a6eae92a 100644 --- a/src/lib/messaging/applier/index.ts +++ b/src/lib/messaging/applier/index.ts @@ -4,6 +4,7 @@ export * from "./setup-applier"; export * from "./host-state-applier"; export * from "./agent-config"; +export * from "./conflict-detection"; export * from "./openshell-provider"; export * from "./policy"; export type * from "./types"; diff --git a/src/lib/messaging/compiler/engines/credential-binding-engine.ts b/src/lib/messaging/compiler/engines/credential-binding-engine.ts index 523f94ea2a..605babc5e0 100644 --- a/src/lib/messaging/compiler/engines/credential-binding-engine.ts +++ b/src/lib/messaging/compiler/engines/credential-binding-engine.ts @@ -7,6 +7,7 @@ import type { SandboxMessagingInputReference, } from "../../manifest"; import type { ManifestCompilerContext } from "../types"; +import { hashCredential } from "../../../security/credential-hash"; import { resolveSandboxNameTemplate } from "./template"; export function planCredentialBindings( @@ -16,6 +17,14 @@ export function planCredentialBindings( ): SandboxMessagingCredentialBindingPlan[] { return manifest.credentials.map((credential) => { const sourceInput = inputs.find((input) => input.inputId === credential.sourceInput); + const credentialAvailable = + sourceInput?.credentialAvailable === true || + context.credentialAvailability?.[credential.id] === true || + context.credentialAvailability?.[`${manifest.id}.${credential.id}`] === true; + + const envKey = sourceInput?.sourceEnv ?? credential.providerEnvKey; + const credentialHash = + credentialAvailable ? (hashCredential(process.env[envKey]) ?? undefined) : undefined; return { channelId: manifest.id, @@ -24,10 +33,8 @@ export function planCredentialBindings( providerName: resolveSandboxNameTemplate(credential.providerName, context.sandboxName), providerEnvKey: credential.providerEnvKey, placeholder: credential.placeholder, - credentialAvailable: - sourceInput?.credentialAvailable === true || - context.credentialAvailability?.[credential.id] === true || - context.credentialAvailability?.[`${manifest.id}.${credential.id}`] === true, + credentialAvailable, + ...(credentialHash !== undefined ? { credentialHash } : {}), }; }); } diff --git a/src/lib/messaging/compiler/workflow-planner.test.ts b/src/lib/messaging/compiler/workflow-planner.test.ts index 9a07b5f307..b9c8c651a5 100644 --- a/src/lib/messaging/compiler/workflow-planner.test.ts +++ b/src/lib/messaging/compiler/workflow-planner.test.ts @@ -666,9 +666,6 @@ describe("MessagingWorkflowPlanner", () => { sandboxEntry: { name: "demo", messagingChannels: ["telegram"], - providerCredentialHashes: { - TELEGRAM_BOT_TOKEN: "sha256:test", - }, }, }); diff --git a/src/lib/messaging/compiler/workflow-planner.ts b/src/lib/messaging/compiler/workflow-planner.ts index 853ff39456..adc28b360f 100644 --- a/src/lib/messaging/compiler/workflow-planner.ts +++ b/src/lib/messaging/compiler/workflow-planner.ts @@ -166,15 +166,18 @@ export class MessagingWorkflowPlanner { sandboxEntry: MessagingWorkflowPlannerSandboxEntry | null | undefined, channelIds: readonly MessagingChannelId[], ): MessagingCompilerCredentialAvailability | undefined { - const hashes = sandboxEntry?.providerCredentialHashes; - if (!hashes || Object.keys(hashes).length === 0) return undefined; + const plan = sandboxEntry?.messaging?.plan; + if (!plan) return undefined; const availability: Record = {}; for (const channelId of channelIds) { const manifest = this.registry.get(channelId); if (!manifest) continue; for (const credential of manifest.credentials) { - if (!hashes[credential.providerEnvKey]) continue; + const binding = plan.credentialBindings.find( + (b) => b.channelId === channelId && b.providerEnvKey === credential.providerEnvKey, + ); + if (!binding?.credentialAvailable) continue; availability[credential.sourceInput] = true; availability[`${manifest.id}.${credential.sourceInput}`] = true; availability[credential.id] = true; @@ -191,7 +194,6 @@ export interface MessagingWorkflowPlannerSandboxEntry { readonly agent?: string | null; readonly messagingChannels?: readonly MessagingChannelId[] | null; readonly disabledChannels?: readonly MessagingChannelId[] | null; - readonly providerCredentialHashes?: Readonly> | null; readonly messaging?: { readonly schemaVersion: 1; readonly plan: SandboxMessagingPlan; diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index b31d0f12f2..f8aa511a4c 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -894,30 +894,6 @@ function upsertMessagingProviders( } const providerExistsInGateway = (name: string) => onboardProviders.providerExistsInGateway(name, runOpenshell); -// Tri-state probe factory for messaging-conflict backfill. An upfront liveness -// check is necessary because `openshell provider get` exits non-zero for both -// "provider not attached" and "gateway unreachable"; without the liveness -// gate, a transient gateway failure would be recorded as "no providers" and -// permanently suppress future backfill retries. -function makeConflictProbe() { - let gatewayAlive: boolean | null = null; - const isGatewayAlive = () => { - if (gatewayAlive === null) { - const result = runCaptureOpenshell(["sandbox", "list"], { ignoreError: true }); - // runCaptureOpenshell returns stdout/stderr as a single string; treat - // any non-empty output as a sign openshell answered. Empty output with - // ignoreError typically means the binary failed to produce anything. - gatewayAlive = typeof result === "string" && result.length > 0; - } - return gatewayAlive; - }; - return { - providerExists: (name: string) => { - if (!isGatewayAlive()) return "error"; - return providerExistsInGateway(name) ? "present" : "absent"; - }, - }; -} function verifyInferenceRoute(_provider: string, _model: string): void { const output = runCaptureOpenshell(["inference", "get"], { ignoreError: true }); @@ -2785,33 +2761,30 @@ async function createSandbox( // the sandbox reuse decision so we can detect stale sandboxes that were created // without provider attachments (security: prevents legacy raw-env-var leaks). - // The UI toggle list can include channels the user toggled on but then - // skipped the token prompt for. Only channels with a real token will have a - // provider attached, so the conflict check must filter out the skipped ones - // (otherwise we warn about phantom channels that will never poll). - const conflictCheckChannels = Array.isArray(enabledChannels) - ? enabledChannels.flatMap((name) => { - const def = MESSAGING_CHANNELS.find((c) => c.name === name); - if (!def || !def.envKey || !getValidatedMessagingToken(def, def.envKey)) return []; - const tokenEnvKeys = getChannelTokenKeys(def); - const credentialHashes: Record = {}; - for (const envKey of tokenEnvKeys) { - const hash = hashCredential(getValidatedMessagingToken(def, envKey)); - if (hash) credentialHashes[envKey] = hash; - } - if (Object.keys(credentialHashes).length === 0) return []; - return [{ channel: name, credentialHashes }]; - }) - : []; - // Messaging channels like Telegram (getUpdates), Discord (gateway), and Slack // (Socket Mode) enforce one consumer per channel credential. Two sandboxes // sharing a credential silently break both bridges (see #1953). Warn before // we commit. - if (conflictCheckChannels.length > 0) { - const { backfillMessagingChannels, findChannelConflicts } = require("./messaging-conflict"); - backfillMessagingChannels(registry, makeConflictProbe()); - const conflicts = findChannelConflicts(sandboxName, conflictCheckChannels, registry); + // + // The compiled plan (written to env by setupMessagingChannels) is the source + // of truth: credential hashes and active-channel membership are read from + // plan.credentialBindings rather than from MESSAGING_CHANNELS constants. + // Validate sandbox identity before trusting the env plan: a stale plan from a + // prior run of a different sandbox must not gate or bypass conflict detection + // for the current sandbox creation. + const envPlan = readMessagingPlanFromEnv(); + const currentPlan = envPlan?.sandboxName === sandboxName ? envPlan : null; + const hasPlanCredentials = currentPlan?.credentialBindings.some((b) => b.credentialAvailable) ?? false; + if (hasPlanCredentials) { + const { backfillMessagingChannels, findChannelConflictsFromPlan, createMessagingConflictProbe } = + require("./messaging/applier") as typeof import("./messaging/applier"); + const probe = createMessagingConflictProbe({ + checkGatewayLiveness: () => + runOpenshell(["sandbox", "list"], { ignoreError: true, suppressOutput: true }).status === 0, + providerExists: (name) => providerExistsInGateway(name), + }); + backfillMessagingChannels(registry, probe); + const conflicts = findChannelConflictsFromPlan(sandboxName, currentPlan!, registry); if (conflicts.length > 0) { for (const { channel, sandbox, reason } of conflicts) { const detail = @@ -2903,12 +2876,9 @@ async function createSandbox( } if (braveWebSearchEnabled) messagingTokenDefs.push({ name: `${sandboxName}-brave-search`, envKey: webSearch.BRAVE_API_KEY_ENV, token: braveApiKey, providerType: braveProviderProfile.BRAVE_PROVIDER_PROFILE_ID }); const extraPlaceholderKeys: string[] = require("./onboard/extra-placeholder-keys").registerExtraPlaceholderProviders(sandboxName, messagingTokenDefs); - const previousProviderCredentialHashes = - registry.getSandbox(sandboxName)?.providerCredentialHashes ?? {}; const hasMessagingTokens = messagingTokenDefs.some(({ token }) => !!token); const reusableMessagingProviders: string[] = []; const reusableMessagingChannels: string[] = []; - const reusableMessagingEnvKeys = new Set(); if (enabledChannels != null) { for (const { name, envKey, token } of messagingTokenDefs) { if (token) continue; @@ -2917,7 +2887,6 @@ async function createSandbox( if (!channel || !enabledChannels.includes(channel)) continue; if (!providerExistsInGateway(name)) continue; reusableMessagingProviders.push(name); - reusableMessagingEnvKeys.add(envKey); if (!reusableMessagingChannels.includes(channel)) { reusableMessagingChannels.push(channel); } @@ -3706,20 +3675,6 @@ async function createSandbox( hermesDashboardForwarding.resolveStateForPort(actualDashboardPort); hermesDashboardForwarding.ensureForState(finalHermesDashboardState, sandboxName, true); - // Register only after confirmed ready — prevents phantom entries - const providerCredentialHashes: Record = {}; - for (const { envKey, token } of messagingTokenDefs) { - const hash = token ? hashCredential(token) : null; - if (hash) { - providerCredentialHashes[envKey] = hash; - } - } - for (const envKey of reusableMessagingEnvKeys) { - const previousHash = previousProviderCredentialHashes[envKey]; - if (typeof previousHash === "string" && previousHash) { - providerCredentialHashes[envKey] = previousHash; - } - } // openshell tags images with seconds; buildId is ms. Parse actual tag from output. Fixes #2672. const resolvedImageTag = resolveSandboxImageTagFromCreateOutput(createResult.output, buildId); @@ -3734,8 +3689,6 @@ async function createSandbox( ...sandboxRuntimeFields, ...getSandboxAgentRegistryFields(agent, !fromDockerfile), imageTag: resolvedImageTag, - providerCredentialHashes: - Object.keys(providerCredentialHashes).length > 0 ? providerCredentialHashes : undefined, policies: initialSandboxPolicy.appliedPresets, // Persist the operator's configured channel set, not the post-disabled-filter // active set. After `channels stop X` + rebuild, activeMessagingChannels drops diff --git a/src/lib/onboard/messaging-credentials.ts b/src/lib/onboard/messaging-credentials.ts index cb03f31dfc..1eaf62eeab 100644 --- a/src/lib/onboard/messaging-credentials.ts +++ b/src/lib/onboard/messaging-credentials.ts @@ -54,17 +54,22 @@ export function getMessagingChannelForEnvKey(envKey: string): string | null { /** * Detect whether any messaging provider credential has been rotated since * the sandbox was created, by comparing SHA-256 hashes of the current - * token values against hashes stored in the sandbox registry. + * token values against hashes stored in the compiled messaging plan. * - * Returns `changed: false` for legacy sandboxes that have no stored hashes - * (conservative — avoids unnecessary rebuilds after upgrade). + * Returns `changed: false` for sandboxes that have no plan (conservative — + * avoids unnecessary rebuilds for sandboxes that pre-date plan storage). */ export function detectMessagingCredentialRotation( sandboxName: string, tokenDefs: MessagingTokenDefinition[], ): { changed: boolean; changedProviders: string[] } { const sb = registry.getSandbox(sandboxName); - const storedHashes = sb?.providerCredentialHashes || {}; + const bindings = sb?.messaging?.plan?.credentialBindings ?? []; + const storedHashes: Record = {}; + for (const b of bindings) { + if (b.credentialHash) storedHashes[b.providerEnvKey] = b.credentialHash; + } + if (Object.keys(storedHashes).length === 0) return { changed: false, changedProviders: [] }; const changedProviders = []; for (const { name, envKey, token } of tokenDefs) { const storedHash = storedHashes[envKey]; diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 8b1680bef5..f7af8637b3 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -60,7 +60,6 @@ export interface SandboxEntry { agent?: string | null; agentVersion?: string | null; imageTag?: string | null; - providerCredentialHashes?: Record; messagingChannels?: string[]; messagingChannelConfig?: MessagingChannelConfig; messaging?: SandboxMessagingState; @@ -257,7 +256,6 @@ export function registerSandbox(entry: SandboxEntry): void { agent: entry.agent || null, agentVersion: entry.agentVersion || null, imageTag: entry.imageTag || null, - providerCredentialHashes: entry.providerCredentialHashes || undefined, messagingChannels: entry.messagingChannels || [], messagingChannelConfig: entry.messagingChannelConfig && Object.keys(entry.messagingChannelConfig).length > 0 diff --git a/src/lib/status-command-deps.ts b/src/lib/status-command-deps.ts index dcb20832cf..db1ed6ea42 100644 --- a/src/lib/status-command-deps.ts +++ b/src/lib/status-command-deps.ts @@ -6,7 +6,7 @@ import { spawnSync } from "node:child_process"; import { getNamedGatewayLifecycleState } from "./gateway-runtime-action"; import { getLiveGatewayInference } from "./inference/live"; import type { GatewayHealth, MessagingBridgeHealth, ShowStatusCommandDeps } from "./inventory"; -import { backfillMessagingChannels, findAllOverlaps } from "./messaging-conflict"; +import { backfillMessagingChannels, findAllOverlaps } from "./messaging/applier"; import type { CaptureOpenshellResult } from "./adapters/openshell/client"; import { captureOpenshellCommand, stripAnsi } from "./adapters/openshell/client"; import { OPENSHELL_PROBE_TIMEOUT_MS } from "./adapters/openshell/timeouts"; diff --git a/test/channels-add-preset.test.ts b/test/channels-add-preset.test.ts index b66369d904..b625861b48 100644 --- a/test/channels-add-preset.test.ts +++ b/test/channels-add-preset.test.ts @@ -143,7 +143,6 @@ registry.getSandbox = () => ({ agent: ${JSON.stringify(sandboxAgent)}, messagingChannels: [], disabledChannels: [], - providerCredentialHashes: {}, }); registry.updateSandbox = (name, updates) => { registryUpdates.push({ name, updates }); @@ -934,7 +933,6 @@ registry.getSandbox = () => ({ agent: "openclaw", messagingChannels: ["telegram"], disabledChannels: [], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "prior-hash" }, }); credentials.getCredential = (key) => key === "TELEGRAM_BOT_TOKEN" ? "prior-telegram-token" : null; const ctx = module.exports; @@ -980,11 +978,6 @@ process.exit = (code) => { ["telegram"], `re-add failure must keep prior 'telegram' in messagingChannels; got ${JSON.stringify(payload.registryUpdates)}`, ); - assert.deepEqual( - lastRegistry.updates.providerCredentialHashes, - { TELEGRAM_BOT_TOKEN: "prior-hash" }, - `re-add failure must restore prior credential hashes; got ${JSON.stringify(payload.registryUpdates)}`, - ); assert.ok( payload.savedCredentialKeys.includes("TELEGRAM_BOT_TOKEN"), `re-add failure must restore prior credentials via saveCredential; got ${JSON.stringify(payload.savedCredentialKeys)}`, @@ -1011,7 +1004,6 @@ registry.getSandbox = () => ({ agent: "openclaw", messagingChannels: ["telegram"], disabledChannels: [], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "prior-hash" }, }); credentials.getCredential = (key) => key === "TELEGRAM_BOT_TOKEN" ? "prior-telegram-token" : null; let upsertCalls = 0; @@ -1060,11 +1052,6 @@ process.exit = (code) => { ["telegram"], `registry restoration must precede gateway re-upsert so an upsert failure cannot orphan the channel; got ${JSON.stringify(payload.registryUpdates)}`, ); - assert.deepEqual( - lastRegistry.updates.providerCredentialHashes, - { TELEGRAM_BOT_TOKEN: "prior-hash" }, - `prior credential hashes must be restored before any gateway side effect; got ${JSON.stringify(payload.registryUpdates)}`, - ); assert.ok( payload.savedCredentialKeys.includes("TELEGRAM_BOT_TOKEN"), `re-add failure must restore staged environment credentials; got ${JSON.stringify(payload.savedCredentialKeys)}`, @@ -1666,7 +1653,6 @@ registry.getSandbox = () => ({ agent: global.__testAgent || "openclaw", messagingChannels: [], disabledChannels: [], - providerCredentialHashes: {}, }); registry.updateSandbox = () => true; diff --git a/test/channels-remove-full-teardown.test.ts b/test/channels-remove-full-teardown.test.ts index 1bbb8679fc..50a83c6983 100644 --- a/test/channels-remove-full-teardown.test.ts +++ b/test/channels-remove-full-teardown.test.ts @@ -135,7 +135,6 @@ registry.getSandbox = () => ({ agent: ${JSON.stringify(sandboxAgent)}, messagingChannels: [${JSON.stringify(channelInRegistry)}], disabledChannels: [], - providerCredentialHashes: {}, policies: ${JSON.stringify(presetNamesApplied)}, }); registry.updateSandbox = (name, updates) => { @@ -382,7 +381,6 @@ registryOverride.getSandbox = () => ({ agent: "openclaw", messagingChannels: [], disabledChannels: [], - providerCredentialHashes: {}, policies: [], }); const policiesOverride = require(${JSON.stringify(path.join(repoRoot, "dist", "lib", "policy/index.js"))}); diff --git a/test/cli/status-health.test.ts b/test/cli/status-health.test.ts index fa413cabff..0a98c9572b 100644 --- a/test/cli/status-health.test.ts +++ b/test/cli/status-health.test.ts @@ -43,9 +43,6 @@ describe("CLI dispatch", () => { policies: ["npm"], agent: "openclaw", dashboardPort: 18789, - providerCredentialHashes: { - OPENAI_API_KEY: "sk-should-not-render-000000000000", - }, messagingChannels: ["slack"], dashboardUrl: "http://127.0.0.1:18789/?token=dashboard-secret", logs: "Bearer should-not-render xoxb-should-not-render-000000", @@ -125,7 +122,7 @@ describe("CLI dispatch", () => { ], }); expect(r.out).not.toMatch( - /Bearer|nvapi-|sk-|xoxb-|xapp-|password|api[-_]?key|providerCredentialHashes|dashboard-secret|should-not-render/i, + /Bearer|nvapi-|sk-|xoxb-|xapp-|password|api[-_]?key|dashboard-secret|should-not-render/i, ); } finally { fs.rmSync(serviceDir, { recursive: true, force: true }); diff --git a/test/credential-rotation.test.ts b/test/credential-rotation.test.ts index 8c89c7ca2a..a1572525e2 100644 --- a/test/credential-rotation.test.ts +++ b/test/credential-rotation.test.ts @@ -113,12 +113,41 @@ describe("credential rotation detection", () => { }); }); + function makePlanEntry(name: string, bindings: Array<{ providerEnvKey: string; credentialHash?: string }>) { + return { + name, + messaging: { + schemaVersion: 1 as const, + plan: { + schemaVersion: 1 as const, + sandboxName: name, + agent: "openclaw" as const, + workflow: "onboard" as const, + channels: [], + disabledChannels: [], + credentialBindings: bindings.map((b) => ({ + channelId: "telegram" as const, + credentialId: "telegramBotToken", + sourceInput: "botToken", + providerName: `${name}-telegram-bridge`, + providerEnvKey: b.providerEnvKey, + placeholder: `openshell:resolve:env:${b.providerEnvKey}`, + credentialAvailable: true, + ...(b.credentialHash ? { credentialHash: b.credentialHash } : {}), + })), + networkPolicy: { presets: [], entries: [] }, + agentRender: [], + buildSteps: [], + stateUpdates: [], + healthChecks: [], + }, + }, + }; + } + describe("detectMessagingCredentialRotation", () => { - it("returns changed: false when no hashes are stored (legacy sandbox)", () => { - vi.spyOn(registry, "getSandbox").mockReturnValue({ - name: "test-sandbox", - // no providerCredentialHashes - }); + it("returns changed: false when no plan is stored (pre-plan sandbox)", () => { + vi.spyOn(registry, "getSandbox").mockReturnValue({ name: "test-sandbox" }); const result = detectMessagingCredentialRotation("test-sandbox", [ { name: "test-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "new-token" }, @@ -131,10 +160,9 @@ describe("credential rotation detection", () => { it("returns changed: false when hashes match", () => { const tokenHash = hashCredentialOrThrow("same-token"); - vi.spyOn(registry, "getSandbox").mockReturnValue({ - name: "test-sandbox", - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: tokenHash }, - }); + vi.spyOn(registry, "getSandbox").mockReturnValue( + makePlanEntry("test-sandbox", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: tokenHash }]), + ); const result = detectMessagingCredentialRotation("test-sandbox", [ { name: "test-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "same-token" }, @@ -147,10 +175,9 @@ describe("credential rotation detection", () => { it("returns changed: true with correct provider names when hashes differ", () => { const oldHash = hashCredentialOrThrow("old-token"); - vi.spyOn(registry, "getSandbox").mockReturnValue({ - name: "test-sandbox", - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: oldHash }, - }); + vi.spyOn(registry, "getSandbox").mockReturnValue( + makePlanEntry("test-sandbox", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: oldHash }]), + ); const result = detectMessagingCredentialRotation("test-sandbox", [ { name: "test-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "new-token" }, @@ -164,13 +191,12 @@ describe("credential rotation detection", () => { it("detects rotation across multiple providers", () => { const telegramHash = hashCredentialOrThrow("tg-old"); const discordHash = hashCredentialOrThrow("dc-same"); - vi.spyOn(registry, "getSandbox").mockReturnValue({ - name: "test-sandbox", - providerCredentialHashes: { - TELEGRAM_BOT_TOKEN: telegramHash, - DISCORD_BOT_TOKEN: discordHash, - }, - }); + vi.spyOn(registry, "getSandbox").mockReturnValue( + makePlanEntry("test-sandbox", [ + { providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: telegramHash }, + { providerEnvKey: "DISCORD_BOT_TOKEN", credentialHash: discordHash }, + ]), + ); const result = detectMessagingCredentialRotation("test-sandbox", [ { name: "test-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: "tg-new" }, @@ -184,10 +210,9 @@ describe("credential rotation detection", () => { it("treats removed tokens as changed providers", () => { const hash = hashCredentialOrThrow("old-token"); - vi.spyOn(registry, "getSandbox").mockReturnValue({ - name: "test-sandbox", - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: hash }, - }); + vi.spyOn(registry, "getSandbox").mockReturnValue( + makePlanEntry("test-sandbox", [{ providerEnvKey: "TELEGRAM_BOT_TOKEN", credentialHash: hash }]), + ); const result = detectMessagingCredentialRotation("test-sandbox", [ { name: "test-telegram-bridge", envKey: "TELEGRAM_BOT_TOKEN", token: null }, diff --git a/test/e2e/test-token-rotation.sh b/test/e2e/test-token-rotation.sh index bf22eb4998..8d3f48ad50 100755 --- a/test/e2e/test-token-rotation.sh +++ b/test/e2e/test-token-rotation.sh @@ -163,6 +163,22 @@ is_fake_slack_token() { esac } +registry_has_messaging_credential_hash() { + local env_key="$1" + [ -f "$REGISTRY" ] && node -e " +const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); +const sandbox = (r.sandboxes || {})[process.argv[2]]; +const bindings = sandbox?.messaging?.plan?.credentialBindings; +if (!Array.isArray(bindings)) process.exit(1); +const found = bindings.some((entry) => + entry?.providerEnvKey === process.argv[3] && + typeof entry.credentialHash === 'string' && + entry.credentialHash.length > 0, +); +process.exit(found ? 0 : 1); +" "$REGISTRY" "$SANDBOX_NAME" "$env_key" 2>/dev/null +} + # ── Phase 0: Install NemoClaw with token A ──────────────────────── section "Phase 0: Install NemoClaw and first onboard with token A" @@ -293,45 +309,29 @@ else fail "Provider ${SANDBOX_NAME}-slack-app not found" fi - # Verify credential hashes are stored for this sandbox in the registry - if [ -f "$REGISTRY" ] && node -e " -const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); -const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; -process.exit('TELEGRAM_BOT_TOKEN' in h ? 0 : 1); -" "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then - pass "Telegram credential hash stored for $SANDBOX_NAME" + # Verify credential hashes are stored in the persisted messaging plan. + if registry_has_messaging_credential_hash "TELEGRAM_BOT_TOKEN"; then + pass "Telegram credential hash stored in messaging plan for $SANDBOX_NAME" else - fail "Telegram credential hash not found for $SANDBOX_NAME in registry" + fail "Telegram credential hash not found in messaging plan for $SANDBOX_NAME" fi - if [ -f "$REGISTRY" ] && node -e " -const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); -const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; -process.exit('DISCORD_BOT_TOKEN' in h ? 0 : 1); -" "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then - pass "Discord credential hash stored for $SANDBOX_NAME" + if registry_has_messaging_credential_hash "DISCORD_BOT_TOKEN"; then + pass "Discord credential hash stored in messaging plan for $SANDBOX_NAME" else - fail "Discord credential hash not found for $SANDBOX_NAME in registry" + fail "Discord credential hash not found in messaging plan for $SANDBOX_NAME" fi - if [ -f "$REGISTRY" ] && node -e " -const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); -const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; -process.exit('SLACK_BOT_TOKEN' in h ? 0 : 1); -" "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then - pass "Slack bot credential hash stored for $SANDBOX_NAME" + if registry_has_messaging_credential_hash "SLACK_BOT_TOKEN"; then + pass "Slack bot credential hash stored in messaging plan for $SANDBOX_NAME" else - fail "Slack bot credential hash not found for $SANDBOX_NAME in registry" + fail "Slack bot credential hash not found in messaging plan for $SANDBOX_NAME" fi - if [ -f "$REGISTRY" ] && node -e " -const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); -const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; -process.exit('SLACK_APP_TOKEN' in h ? 0 : 1); -" "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then - pass "Slack app credential hash stored for $SANDBOX_NAME" + if registry_has_messaging_credential_hash "SLACK_APP_TOKEN"; then + pass "Slack app credential hash stored in messaging plan for $SANDBOX_NAME" else - fail "Slack app credential hash not found for $SANDBOX_NAME in registry" + fail "Slack app credential hash not found in messaging plan for $SANDBOX_NAME" fi # ── Phase 2: Rotate Telegram token only (re-onboard with token B) ─ diff --git a/test/helpers/messaging-conflict-fixtures.ts b/test/helpers/messaging-conflict-fixtures.ts new file mode 100644 index 0000000000..4c2ff3b5cd --- /dev/null +++ b/test/helpers/messaging-conflict-fixtures.ts @@ -0,0 +1,150 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { SandboxMessagingPlan } from "../../src/lib/messaging/manifest"; +import type { SandboxMessagingState } from "../../src/lib/state/registry"; +import type { ConflictRegistryEntry } from "../../src/lib/messaging/applier/conflict-detection"; + +export function makePlan( + sandboxName: string, + overrides: Partial = {}, +): SandboxMessagingPlan { + return { + schemaVersion: 1, + sandboxName, + agent: "openclaw", + workflow: "onboard", + channels: [], + disabledChannels: [], + credentialBindings: [], + networkPolicy: { presets: [], entries: [] }, + agentRender: [], + buildSteps: [], + stateUpdates: [], + healthChecks: [], + ...overrides, + }; +} + +export function tgChannel(active = true, disabled = false) { + return { + channelId: "telegram" as const, + displayName: "Telegram", + authMode: "token-paste" as const, + active, + selected: true, + configured: true, + disabled, + inputs: [], + hooks: [], + }; +} + +export function slackChannel() { + return { + channelId: "slack" as const, + displayName: "Slack", + authMode: "token-paste" as const, + active: true, + selected: true, + configured: true, + disabled: false, + inputs: [], + hooks: [], + }; +} + +export function discordChannel() { + return { + channelId: "discord" as const, + displayName: "Discord", + authMode: "token-paste" as const, + active: true, + selected: true, + configured: true, + disabled: false, + inputs: [], + hooks: [], + }; +} + +export function whatsappChannel() { + return { + channelId: "whatsapp" as const, + displayName: "WhatsApp", + authMode: "in-sandbox-qr" as const, + active: true, + selected: true, + configured: true, + disabled: false, + inputs: [], + hooks: [], + }; +} + +export function tgBinding(hash?: string): SandboxMessagingPlan["credentialBindings"][number] { + return { + channelId: "telegram", + credentialId: "telegramBotToken", + sourceInput: "botToken", + providerName: "sb-telegram-bridge", + providerEnvKey: "TELEGRAM_BOT_TOKEN", + placeholder: "openshell:resolve:env:TELEGRAM_BOT_TOKEN", + credentialAvailable: true, + ...(hash !== undefined ? { credentialHash: hash } : {}), + }; +} + +export function discordBinding(hash?: string): SandboxMessagingPlan["credentialBindings"][number] { + return { + channelId: "discord", + credentialId: "discordBotToken", + sourceInput: "botToken", + providerName: "sb-discord-bridge", + providerEnvKey: "DISCORD_BOT_TOKEN", + placeholder: "openshell:resolve:env:DISCORD_BOT_TOKEN", + credentialAvailable: true, + ...(hash !== undefined ? { credentialHash: hash } : {}), + }; +} + +export function slackBotBinding( + hash?: string, + sandboxName = "sb", +): SandboxMessagingPlan["credentialBindings"][number] { + return { + channelId: "slack", + credentialId: "slackBotToken", + sourceInput: "botToken", + providerName: `${sandboxName}-slack-bridge`, + providerEnvKey: "SLACK_BOT_TOKEN", + placeholder: "openshell:resolve:env:SLACK_BOT_TOKEN", + credentialAvailable: true, + ...(hash !== undefined ? { credentialHash: hash } : {}), + }; +} + +export function slackAppBinding( + hash?: string, + sandboxName = "sb", +): SandboxMessagingPlan["credentialBindings"][number] { + return { + channelId: "slack", + credentialId: "slackAppToken", + sourceInput: "appToken", + providerName: `${sandboxName}-slack-app`, + providerEnvKey: "SLACK_APP_TOKEN", + placeholder: "openshell:resolve:env:SLACK_APP_TOKEN", + credentialAvailable: true, + ...(hash !== undefined ? { credentialHash: hash } : {}), + }; +} + +export function slackBindings(botHash?: string, appHash?: string, sandboxName = "sb") { + return [slackBotBinding(botHash, sandboxName), slackAppBinding(appHash, sandboxName)]; +} + +export function planEntry(name: string, plan: SandboxMessagingPlan): ConflictRegistryEntry { + const state: SandboxMessagingState = { schemaVersion: 1, plan }; + return { name, messaging: state }; +} diff --git a/test/onboard-messaging.test.ts b/test/onboard-messaging.test.ts index 687d874e14..14884fd4e7 100644 --- a/test/onboard-messaging.test.ts +++ b/test/onboard-messaging.test.ts @@ -516,12 +516,6 @@ const registerCalls = []; registry.registerSandbox({ name: "my-assistant", messagingChannels: ["discord", "slack"], - providerCredentialHashes: { - DISCORD_BOT_TOKEN: "hash-discord", - SLACK_BOT_TOKEN: "hash-slack-bot", - SLACK_APP_TOKEN: "hash-slack-app", - TELEGRAM_BOT_TOKEN: "hash-telegram", - }, }); runner.run = (command, opts = {}) => { const normalized = _n(command); @@ -645,11 +639,6 @@ const { createSandbox } = require(${onboardPath}); const channels = JSON.parse(Buffer.from(channelsLine.split("=")[1], "base64").toString()); assert.deepEqual(channels, ["discord", "slack"]); assert.deepEqual(payload.registerCalls[0]?.messagingChannels, ["discord", "slack"]); - assert.deepEqual(payload.registerCalls[0]?.providerCredentialHashes, { - DISCORD_BOT_TOKEN: "hash-discord", - SLACK_BOT_TOKEN: "hash-slack-bot", - SLACK_APP_TOKEN: "hash-slack-app", - }); }, ); @@ -690,7 +679,6 @@ registry.registerSandbox({ name: "my-assistant", messagingChannels: ["telegram"], disabledChannels: ["telegram"], - providerCredentialHashes: { TELEGRAM_BOT_TOKEN: "hash-telegram" }, }); runner.run = (command, opts = {}) => { const normalized = _n(command); diff --git a/test/rebuild-credential-preflight.test.ts b/test/rebuild-credential-preflight.test.ts index e8733ce55b..91adfd835e 100644 --- a/test/rebuild-credential-preflight.test.ts +++ b/test/rebuild-credential-preflight.test.ts @@ -51,7 +51,6 @@ function createFixture(opts: { agent?: string | null; hermesAuthMethod?: string | null; messagingChannels?: string[] | null; - providerCredentialHashes?: Record; dockerBuildExitCode?: number; providerRegistered?: boolean; }) { @@ -64,7 +63,6 @@ function createFixture(opts: { agent = null, hermesAuthMethod = null, messagingChannels = null, - providerCredentialHashes, dockerBuildExitCode = 0, providerRegistered = true, } = opts; @@ -87,7 +85,6 @@ function createFixture(opts: { policies: [], agent, messagingChannels, - ...(providerCredentialHashes ? { providerCredentialHashes } : {}), }, }, }), @@ -368,7 +365,6 @@ describe("Issue #2273: atomic rebuild", () => { const f = createFixture({ agent: "hermes", messagingChannels: ["discord"], - providerCredentialHashes: { DISCORD_BOT_TOKEN: "hash-discord" }, credentialEnv: "NVIDIA_API_KEY", savedCredential: { key: "NVIDIA_API_KEY",