diff --git a/src/lib/messaging/applier/host-state-applier.test.ts b/src/lib/messaging/applier/host-state-applier.test.ts index b438efa498..1bd6232c08 100644 --- a/src/lib/messaging/applier/host-state-applier.test.ts +++ b/src/lib/messaging/applier/host-state-applier.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { SandboxMessagingPlan } from "../manifest"; +import { compactSandboxMessagingPlanForPersistence } from "../persistence"; import { MessagingHostStateApplier } from "./host-state-applier"; import { MessagingSetupApplier } from "./setup-applier"; import * as registry from "../../state/registry"; @@ -73,6 +74,38 @@ describe("MessagingHostStateApplier", () => { }); }); + it("hydrates compact existing plans before merging host state", () => { + registryMock.__setSandbox("demo", { + name: "demo", + messaging: { + schemaVersion: 1, + plan: compactSandboxMessagingPlanForPersistence(makePlan(["telegram"])), + }, + }); + + const updated = MessagingHostStateApplier.applyPlanToRegistry( + "demo", + makePlan(["slack"], { + credentialBindings: [ + makeCredentialBinding("slack", "bot"), + makeCredentialBinding("slack", "app"), + ], + }), + { mode: "merge" }, + ); + + expect(updated).toBe(true); + const entry = registryMock.__getSandbox("demo"); + const plan = (entry?.messaging as { plan: SandboxMessagingPlan }).plan; + expect(plan.channels.map((channel) => channel.channelId)).toEqual(["telegram", "slack"]); + expect( + plan.channels + .find((channel) => channel.channelId === "telegram") + ?.hooks.some((hook) => hook.channelId === "telegram"), + ).toBe(true); + expect(plan.agentRender.some((entry) => entry.channelId === "telegram")).toBe(true); + }); + it("can merge a single-channel add plan into existing messaging state", () => { registryMock.__setSandbox("demo", { name: "demo", diff --git a/src/lib/messaging/applier/host-state-applier.ts b/src/lib/messaging/applier/host-state-applier.ts index c3e5b8644a..b25a471c2f 100644 --- a/src/lib/messaging/applier/host-state-applier.ts +++ b/src/lib/messaging/applier/host-state-applier.ts @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import type { SandboxMessagingPlan } from "../manifest"; +import { hydrateDerivedSandboxMessagingPlanFields } from "../persistence"; +import { parseSandboxMessagingPlan } from "../plan-validation"; import * as registry from "../../state/registry"; import { MessagingSetupApplier } from "./setup-applier"; import type { MessagingSetupEnvOptions } from "./types"; @@ -42,9 +44,13 @@ export class MessagingHostStateApplier { if (plan.sandboxName !== sandboxName) return false; const entry = registry.getSandbox(sandboxName); if (!entry) return false; + const existingPlan = parseSandboxMessagingPlan(entry.messaging?.plan); + const hydratedExistingPlan = existingPlan + ? hydrateDerivedSandboxMessagingPlanFields(existingPlan) + : null; const nextPlan = - options.mode === "merge" && entry.messaging?.plan - ? mergeSandboxMessagingPlans(entry.messaging.plan, plan) + options.mode === "merge" && hydratedExistingPlan + ? mergeSandboxMessagingPlans(hydratedExistingPlan, plan) : clonePlan(plan); return registry.updateSandbox(sandboxName, { messaging: { diff --git a/src/lib/messaging/compiler/workflow-planner.test.ts b/src/lib/messaging/compiler/workflow-planner.test.ts index 1c51b12d83..2fa264c360 100644 --- a/src/lib/messaging/compiler/workflow-planner.test.ts +++ b/src/lib/messaging/compiler/workflow-planner.test.ts @@ -8,6 +8,7 @@ import { createBuiltInRenderTemplateResolver, } from "../channels"; import { createBuiltInMessagingHookRegistry, MessagingHookRegistry } from "../hooks"; +import { createChannelManifestRegistry, type ChannelManifest } from "../manifest"; import { MessagingWorkflowPlanner } from "./workflow-planner"; const TEST_CREDENTIALS: Readonly> = { @@ -24,6 +25,34 @@ const TEST_WECHAT_LOGIN = { userId: "test-wechat-user", } as const; +const CREDENTIAL_ONLY_MANIFEST: ChannelManifest = { + schemaVersion: 1, + id: "matrix", + displayName: "Matrix", + supportedAgents: ["openclaw"], + auth: { mode: "none" }, + inputs: [ + { + id: "botToken", + kind: "secret", + required: true, + envKey: "MATRIX_BOT_TOKEN", + }, + ], + credentials: [ + { + id: "botToken", + sourceInput: "botToken", + providerName: "{sandboxName}-matrix", + providerEnvKey: "MATRIX_BOT_TOKEN", + placeholder: "MATRIX_BOT_TOKEN", + }, + ], + render: [], + state: {}, + hooks: [], +}; + function planner(): MessagingWorkflowPlanner { return new MessagingWorkflowPlanner( createBuiltInChannelManifestRegistry(), @@ -446,55 +475,6 @@ describe("MessagingWorkflowPlanner", () => { ]); }); - it("refreshes missing manifest render entries from stale rebuild plans", async () => { - const existingPlan = await planner().buildPlan({ - sandboxName: "demo", - agent: "hermes", - workflow: "onboard", - isInteractive: false, - configuredChannels: ["discord"], - credentialAvailability: { - DISCORD_BOT_TOKEN: true, - }, - }); - const stalePlan = { - ...existingPlan, - credentialBindings: existingPlan.credentialBindings.map((binding) => ({ - ...binding, - credentialHash: "hash-discord-token", - })), - agentRender: [], - buildSteps: [], - }; - - const plan = await planner().buildRebuildPlanFromSandboxEntry({ - sandboxName: "demo", - agent: "hermes", - sandboxEntry: { - name: "demo", - agent: "hermes", - messaging: { schemaVersion: 1, plan: stalePlan }, - }, - supportedChannelIds: ["discord"], - }); - - expect(plan?.workflow).toBe("rebuild"); - expect( - plan?.credentialBindings.find((binding) => binding.providerEnvKey === "DISCORD_BOT_TOKEN") - ?.credentialHash, - ).toBe("hash-discord-token"); - const discordEnvRender = plan?.agentRender.find( - (entry) => - entry.channelId === "discord" && - entry.kind === "env-lines" && - entry.target === "~/.hermes/.env", - ); - expect(discordEnvRender).toMatchObject({ - kind: "env-lines", - lines: expect.arrayContaining(["DISCORD_BOT_TOKEN=openshell:resolve:env:DISCORD_BOT_TOKEN"]), - }); - }); - it("adds one manifest channel into an existing sandbox entry plan", async () => { const existingPlan = await planner().buildPlan({ sandboxName: "demo", @@ -578,6 +558,51 @@ describe("MessagingWorkflowPlanner", () => { ]); }); + it("does not trust credential availability from mismatched sandbox entry plans", async () => { + const registry = createChannelManifestRegistry([CREDENTIAL_ONLY_MANIFEST]); + const localPlanner = new MessagingWorkflowPlanner( + registry, + new MessagingHookRegistry(), + createBuiltInRenderTemplateResolver(), + ); + const stalePlan = await localPlanner.buildPlan({ + sandboxName: "other-sandbox", + agent: "openclaw", + workflow: "onboard", + isInteractive: false, + configuredChannels: ["matrix"], + credentialAvailability: { + MATRIX_BOT_TOKEN: true, + }, + }); + + const plan = await localPlanner.buildChannelAddPlanFromSandboxEntry({ + sandboxName: "demo", + agent: "openclaw", + sandboxEntry: { + name: "demo", + messaging: { + schemaVersion: 1, + plan: stalePlan, + }, + }, + channelId: "matrix", + isInteractive: false, + supportedChannelIds: ["matrix"], + }); + + expect(plan.channels).toHaveLength(1); + expect(plan.channels[0]).toMatchObject({ + channelId: "matrix", + active: false, + configured: true, + }); + expect(plan.credentialBindings[0]).toMatchObject({ + channelId: "matrix", + credentialAvailable: false, + }); + }); + it("mutates disabled channel state in an existing sandbox entry plan", async () => { const existingPlan = await planner().buildPlan({ sandboxName: "demo", diff --git a/src/lib/messaging/compiler/workflow-planner.ts b/src/lib/messaging/compiler/workflow-planner.ts index 0c188de1ab..d868c00a89 100644 --- a/src/lib/messaging/compiler/workflow-planner.ts +++ b/src/lib/messaging/compiler/workflow-planner.ts @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import { MessagingHookRegistry } from "../hooks"; +import { hydrateDerivedSandboxMessagingPlanFields } from "../persistence"; +import { parseSandboxMessagingPlan } from "../plan-validation"; import type { ChannelManifestRegistry, MessagingAgentId, @@ -68,7 +70,7 @@ export class MessagingWorkflowPlanner { supportedChannelIds: context.supportedChannelIds, credentialAvailability: mergeAvailability( credentialAvailabilityFromPlan(existingPlan), - this.credentialAvailabilityFromSandboxEntry(context.sandboxEntry, [context.channelId]), + this.credentialAvailabilityFromSandboxEntry(context, [context.channelId]), context.credentialAvailability, ), }); @@ -101,34 +103,11 @@ export class MessagingWorkflowPlanner { ): Promise { const existingPlan = readSandboxEntryPlan(context); if (existingPlan) { - const normalizedPlan = setPlanDisabledChannels( + return setPlanDisabledChannels( existingPlan, disabledChannelsFromSandboxEntry(context.sandboxEntry, existingPlan), "rebuild", ); - if (!planMissingActiveChannelRender(normalizedPlan)) return normalizedPlan; - - const configuredChannels = uniqueChannels( - normalizedPlan.channels.map((channel) => channel.channelId), - ); - const refreshedPlan = await this.buildPlan({ - sandboxName: context.sandboxName, - agent: context.agent, - workflow: "rebuild", - isInteractive: false, - configuredChannels, - disabledChannels: normalizedPlan.disabledChannels, - supportedChannelIds: context.supportedChannelIds, - credentialAvailability: mergeAvailability( - credentialAvailabilityFromPlan(normalizedPlan), - this.credentialAvailabilityFromSandboxEntry(context.sandboxEntry, configuredChannels), - context.credentialAvailability, - ), - }); - return mergeSandboxMessagingPlans( - normalizedPlan, - preserveCredentialBindingHashes(normalizedPlan, refreshedPlan), - ); } return null; } @@ -174,10 +153,10 @@ export class MessagingWorkflowPlanner { } private credentialAvailabilityFromSandboxEntry( - sandboxEntry: MessagingWorkflowPlannerSandboxEntry | null | undefined, + context: Pick, channelIds: readonly MessagingChannelId[], ): MessagingCompilerCredentialAvailability | undefined { - const plan = sandboxEntry?.messaging?.plan; + const plan = readSandboxEntryPlan(context); if (!plan) return undefined; const availability: Record = {}; @@ -247,16 +226,11 @@ function onlyConfiguredChannels( function readSandboxEntryPlan( context: Pick, ): SandboxMessagingPlan | null { - const plan = context.sandboxEntry?.messaging?.plan; - if ( - !plan || - plan.schemaVersion !== 1 || - plan.sandboxName !== context.sandboxName || - plan.agent !== context.agent - ) { - return null; - } - return clonePlan(plan); + const plan = parseSandboxMessagingPlan(context.sandboxEntry?.messaging?.plan, { + sandboxName: context.sandboxName, + agent: context.agent, + }); + return plan ? hydrateDerivedSandboxMessagingPlanFields(plan) : null; } function disabledChannelsFromSandboxEntry( @@ -376,39 +350,6 @@ function setPlanDisabledChannels( }); } -function preserveCredentialBindingHashes( - existing: SandboxMessagingPlan, - incoming: SandboxMessagingPlan, -): SandboxMessagingPlan { - const existingHashes = new Map( - existing.credentialBindings - .filter((binding) => binding.credentialHash) - .map((binding) => [credentialBindingKey(binding), binding.credentialHash] as const), - ); - if (existingHashes.size === 0) return incoming; - - return clonePlan({ - ...incoming, - credentialBindings: incoming.credentialBindings.map((binding) => ({ - ...binding, - credentialHash: binding.credentialHash ?? existingHashes.get(credentialBindingKey(binding)), - })), - }); -} - -function credentialBindingKey( - binding: Pick, -): string { - return binding.channelId + "\0" + binding.providerEnvKey; -} - -function planMissingActiveChannelRender(plan: SandboxMessagingPlan): boolean { - const renderedChannels = new Set(plan.agentRender.map((entry) => entry.channelId)); - return plan.channels.some( - (channel) => channel.active && !channel.disabled && !renderedChannels.has(channel.channelId), - ); -} - function removePlanChannel( plan: SandboxMessagingPlan, channelId: MessagingChannelId, diff --git a/src/lib/messaging/index.ts b/src/lib/messaging/index.ts index c0e99120ba..69fdc0117b 100644 --- a/src/lib/messaging/index.ts +++ b/src/lib/messaging/index.ts @@ -6,4 +6,5 @@ export * from "./compiler"; export * from "./hooks"; export * from "./applier"; export * from "./manifest"; +export * from "./persistence"; export * from "./utils"; diff --git a/src/lib/messaging/persistence.ts b/src/lib/messaging/persistence.ts new file mode 100644 index 0000000000..34831cf585 --- /dev/null +++ b/src/lib/messaging/persistence.ts @@ -0,0 +1,195 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { + createBuiltInChannelManifestRegistry, + createBuiltInRenderTemplateResolver, +} from "./channels"; +import { + collectTemplateReferencesInLines, + collectTemplateReferencesInValue, + isTruthyRenderTemplate, + resolveCredentialTemplatesInLines, + resolveCredentialTemplatesInValue, + resolveRenderTemplatesInLines, + resolveRenderTemplatesInValue, +} from "./compiler/engines/template"; +import type { + ChannelHookSpec, + ChannelManifest, + MessagingAgentId, + MessagingChannelId, + SandboxMessagingAgentRenderPlan, + SandboxMessagingChannelPlan, + SandboxMessagingEnvLinesRenderPlan, + SandboxMessagingHookReferencePlan, + SandboxMessagingJsonRenderPlan, + SandboxMessagingPlan, +} from "./manifest"; + +export type PersistedSandboxMessagingChannelPlan = Omit & { + readonly hooks?: readonly SandboxMessagingHookReferencePlan[]; +}; + +export type PersistedSandboxMessagingPlan = Omit< + SandboxMessagingPlan, + "channels" | "agentRender" +> & { + readonly channels: readonly PersistedSandboxMessagingChannelPlan[]; + readonly agentRender?: readonly SandboxMessagingAgentRenderPlan[]; +}; + +export function compactSandboxMessagingPlanForPersistence( + plan: SandboxMessagingPlan, +): PersistedSandboxMessagingPlan { + const { agentRender: _agentRender, channels, ...rest } = clonePlan(plan); + return { + ...rest, + channels: channels.map(({ hooks: _hooks, ...channel }) => channel), + }; +} + +export function hydrateDerivedSandboxMessagingPlanFields( + plan: SandboxMessagingPlan, +): SandboxMessagingPlan { + const manifestRegistry = createBuiltInChannelManifestRegistry(); + const channels = plan.channels.map((channel) => { + if (channel.hooks.length > 0) return channel; + return { + ...channel, + hooks: channelHooksFromManifest( + plan.agent, + channel.channelId, + manifestRegistry.get(channel.channelId), + ), + }; + }); + const hydratedPlan = { ...plan, channels }; + return { + ...hydratedPlan, + agentRender: + hydratedPlan.agentRender.length > 0 + ? hydratedPlan.agentRender + : agentRenderFromManifests(hydratedPlan, manifestRegistry), + }; +} + +function channelHooksFromManifest( + agent: MessagingAgentId, + channelId: MessagingChannelId, + manifest: ChannelManifest | undefined, +): SandboxMessagingHookReferencePlan[] { + if (!manifest) return []; + return manifest.hooks + .filter((hook) => isHookForAgent(hook, agent)) + .map((hook) => cloneHookReference(channelId, hook)); +} + +function agentRenderFromManifests( + plan: SandboxMessagingPlan, + manifestRegistry: ReturnType, +): SandboxMessagingAgentRenderPlan[] { + const render: SandboxMessagingAgentRenderPlan[] = []; + const referenceResolver = createBuiltInRenderTemplateResolver(); + for (const channel of plan.channels) { + const manifest = manifestRegistry.get(channel.channelId); + if (!manifest) continue; + const context = { + inputs: channel.inputs, + env: process.env, + referenceResolver, + }; + + for (const [index, entry] of manifest.render.entries()) { + if (entry.agent !== plan.agent) continue; + if (!isTruthyRenderTemplate(entry.when, context)) continue; + const renderId = entry.id ?? `${manifest.id}-render-${index}`; + const hookId = renderId; + const handler = "common.staticOutputs"; + + if (entry.kind === "json-fragment") { + const credentialResolved = resolveCredentialTemplatesInValue( + entry.fragment.value, + manifest.credentials, + ); + const value = resolveRenderTemplatesInValue(credentialResolved, context); + if (value === undefined) continue; + render.push({ + channelId: manifest.id, + renderId, + hookId, + handler, + kind: "json-fragment", + agent: entry.agent, + target: entry.target, + path: entry.fragment.path, + value, + templateRefs: collectTemplateReferencesInValue(value), + } satisfies SandboxMessagingJsonRenderPlan); + continue; + } + + const credentialResolved = resolveCredentialTemplatesInLines( + entry.lines, + manifest.credentials, + ); + const lines = resolveRenderTemplatesInLines(credentialResolved, context); + if (lines.length === 0) continue; + assertSingleLineEnvRenderLines(manifest.id, renderId, lines); + render.push({ + channelId: manifest.id, + renderId, + hookId, + handler, + kind: "env-lines", + agent: entry.agent, + target: entry.target, + lines, + templateRefs: collectTemplateReferencesInLines(lines), + } satisfies SandboxMessagingEnvLinesRenderPlan); + } + } + return render; +} + +function cloneHookReference( + channelId: MessagingChannelId, + hook: ChannelHookSpec, +): SandboxMessagingHookReferencePlan { + return { + channelId, + id: hook.id, + phase: hook.phase, + handler: hook.handler, + agents: hook.agents ? [...hook.agents] : undefined, + inputs: hook.inputs ? [...hook.inputs] : undefined, + outputs: hook.outputs?.map((output) => ({ ...output })), + onFailure: hook.onFailure, + }; +} + +function isHookForAgent(hook: ChannelHookSpec, agent: MessagingAgentId): boolean { + return !hook.agents || hook.agents.includes(agent); +} + +function assertSingleLineEnvRenderLines( + channelId: string, + renderId: string, + lines: readonly string[], +): void { + for (const line of lines) { + if (/[\r\n]/.test(line)) { + throw new Error( + "Messaging env render '" + + renderId + + "' for " + + channelId + + " must not contain line breaks.", + ); + } + } +} + +function clonePlan(plan: SandboxMessagingPlan): SandboxMessagingPlan { + return JSON.parse(JSON.stringify(plan)) as SandboxMessagingPlan; +} diff --git a/src/lib/messaging/plan-validation.test.ts b/src/lib/messaging/plan-validation.test.ts index c7a9d49078..a9d381d5c7 100644 --- a/src/lib/messaging/plan-validation.test.ts +++ b/src/lib/messaging/plan-validation.test.ts @@ -11,6 +11,7 @@ import { getMessagingChannelConfigFromPlan, parseSandboxMessagingPlan, } from "./plan-validation"; +import { compactSandboxMessagingPlanForPersistence } from "./persistence"; function makePlan(overrides: Partial = {}): SandboxMessagingPlan { return { @@ -64,6 +65,18 @@ describe("parseSandboxMessagingPlan", () => { expect(parsed).not.toBe(source); }); + it("accepts compact persisted plans without render or channel hooks", () => { + const source = makePlan(); + const compact = compactSandboxMessagingPlanForPersistence(source); + const parsed = parseSandboxMessagingPlan(compact); + + expect(parsed).toEqual({ + ...source, + agentRender: [], + channels: source.channels.map((channel) => ({ ...channel, hooks: [] })), + }); + }); + it("rejects mismatched selectors, duplicate channels, and unsupported channels", () => { expect(parseSandboxMessagingPlan(makePlan(), { sandboxName: "other" })).toBeNull(); expect(parseSandboxMessagingPlan(makePlan(), { agent: "hermes" })).toBeNull(); diff --git a/src/lib/messaging/plan-validation.ts b/src/lib/messaging/plan-validation.ts index a10c114132..6439d0e2fe 100644 --- a/src/lib/messaging/plan-validation.ts +++ b/src/lib/messaging/plan-validation.ts @@ -24,7 +24,7 @@ export function parseSandboxMessagingPlan( !Array.isArray(value.disabledChannels) || !Array.isArray(value.credentialBindings) || !isObject(value.networkPolicy) || - !Array.isArray(value.agentRender) || + (Object.hasOwn(value, "agentRender") && !Array.isArray(value.agentRender)) || !Array.isArray(value.buildSteps) || !Array.isArray(value.stateUpdates) || !Array.isArray(value.healthChecks) @@ -45,6 +45,7 @@ export function parseSandboxMessagingPlan( if (typeof channel.active !== "boolean") return null; if (typeof channel.disabled !== "boolean") return null; if (!Array.isArray(channel.inputs)) return null; + if (Object.hasOwn(channel, "hooks") && !Array.isArray(channel.hooks)) return null; if (supported && !supported.has(channel.channelId)) return null; if ( value.channels.findIndex( @@ -56,7 +57,9 @@ export function parseSandboxMessagingPlan( } if (!value.disabledChannels.every((channelId) => typeof channelId === "string")) return null; - return cloneSandboxMessagingPlan(value as unknown as SandboxMessagingPlan); + return cloneSandboxMessagingPlan( + normalizePersistedSandboxMessagingPlanShape(value as unknown as MaybeCompactMessagingPlan), + ); } export function cloneSandboxMessagingPlan(plan: SandboxMessagingPlan): SandboxMessagingPlan { @@ -100,6 +103,28 @@ export function getMessagingChannelConfigFromPlan( return Object.keys(config).length > 0 ? config : null; } +type MaybeCompactMessagingChannelPlan = Omit & { + readonly hooks?: SandboxMessagingPlan["channels"][number]["hooks"]; +}; + +type MaybeCompactMessagingPlan = Omit & { + readonly agentRender?: SandboxMessagingPlan["agentRender"]; + readonly channels: readonly MaybeCompactMessagingChannelPlan[]; +}; + +function normalizePersistedSandboxMessagingPlanShape( + plan: MaybeCompactMessagingPlan, +): SandboxMessagingPlan { + return { + ...plan, + agentRender: Array.isArray(plan.agentRender) ? [...plan.agentRender] : [], + channels: plan.channels.map((channel) => ({ + ...channel, + hooks: Array.isArray(channel.hooks) ? [...channel.hooks] : [], + })), + }; +} + function isObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/src/lib/onboard/messaging-channel-setup.ts b/src/lib/onboard/messaging-channel-setup.ts index 1ad05a807e..f9514e15ff 100644 --- a/src/lib/onboard/messaging-channel-setup.ts +++ b/src/lib/onboard/messaging-channel-setup.ts @@ -270,7 +270,7 @@ export function writePlanToEnv(plan: SandboxMessagingPlan): void { } export function getRegistrySandboxMessagingPlan(sandboxName: string): SandboxMessagingPlan | null { - return registry.getSandbox(sandboxName)?.messaging?.plan ?? null; + return registry.getHydratedMessagingPlanFromEntry(registry.getSandbox(sandboxName)); } function resolveMessagingSetupSandboxName(options: SetupSelectedMessagingChannelsOptions): string { diff --git a/src/lib/state/onboard-session.test.ts b/src/lib/state/onboard-session.test.ts index a633eaa4e7..c17b41ef69 100644 --- a/src/lib/state/onboard-session.test.ts +++ b/src/lib/state/onboard-session.test.ts @@ -582,6 +582,49 @@ describe("onboard session", () => { expect(loaded.messagingPlan).toEqual(created.messagingPlan); }); + it("writes compact messagingPlan derived fields to onboard-session.json", () => { + const created = session.createSession(); + created.messagingPlan = { + ...makeMessagingPlan("my-assistant", ["telegram"]), + channels: [ + { + ...makeMessagingPlan("my-assistant", ["telegram"]).channels[0], + hooks: [ + { + channelId: "telegram", + id: "telegram-token-paste", + phase: "enroll", + handler: "common.tokenPaste", + }, + ], + }, + ], + agentRender: [ + { + channelId: "telegram", + renderId: "telegram-openclaw-channel", + hookId: "telegram-openclaw-channel", + handler: "common.staticOutputs", + kind: "json-fragment", + agent: "openclaw", + target: "openclaw.json", + path: "channels.telegram", + value: { enabled: true }, + templateRefs: [], + }, + ], + }; + + session.saveSession(created); + + const raw = JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf-8")); + expect(raw.messagingPlan.agentRender).toBeUndefined(); + expect(raw.messagingPlan.channels[0].hooks).toBeUndefined(); + const reloadedPlan = requireLoadedSession(session.loadSession()).messagingPlan; + expect(reloadedPlan?.agentRender).toEqual([]); + expect(reloadedPlan?.channels[0]?.hooks).toEqual([]); + }); + it("drops malformed persisted messagingPlan on load", () => { const created = session.createSession(); fs.mkdirSync(path.dirname(session.SESSION_FILE), { recursive: true }); diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index 3d8737fce8..61168de62a 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -15,6 +15,7 @@ import { isErrnoException } from "../core/errno"; import type { JsonObject, JsonValue } from "../core/json-types"; import type { WebSearchConfig } from "../inference/web-search"; import type { SandboxMessagingPlan } from "../messaging/manifest"; +import { compactSandboxMessagingPlanForPersistence } from "../messaging/persistence"; import { parseSandboxMessagingPlan } from "../messaging/plan-validation"; import { createOnboardMachineEvent, @@ -528,6 +529,15 @@ export function loadSession(): Session | null { } } +function serializeSessionForDisk(session: Session): Record { + return { + ...session, + messagingPlan: session.messagingPlan + ? compactSandboxMessagingPlanForPersistence(session.messagingPlan) + : session.messagingPlan, + }; +} + export function saveSession(session: Session): Session { const normalized = normalizeSession(session) || createSession(); normalized.updatedAt = new Date().toISOString(); @@ -536,7 +546,9 @@ export function saveSession(session: Session): Session { SESSION_DIR, `.onboard-session.${process.pid}.${Date.now()}.${randomUUID()}.tmp`, ); - fs.writeFileSync(tmpFile, JSON.stringify(normalized, null, 2), { mode: 0o600 }); + fs.writeFileSync(tmpFile, JSON.stringify(serializeSessionForDisk(normalized), null, 2), { + mode: 0o600, + }); fs.renameSync(tmpFile, SESSION_FILE); return normalized; } diff --git a/src/lib/state/registry-messaging.ts b/src/lib/state/registry-messaging.ts index b03b89140f..910d5e6e02 100644 --- a/src/lib/state/registry-messaging.ts +++ b/src/lib/state/registry-messaging.ts @@ -2,6 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 import type { SandboxMessagingPlan } from "../messaging/manifest"; +import { + compactSandboxMessagingPlanForPersistence, + hydrateDerivedSandboxMessagingPlanFields, +} from "../messaging/persistence"; import { getActiveChannelIdsFromPlan, getConfiguredChannelIdsFromPlan, @@ -36,6 +40,17 @@ export function cloneSandboxMessagingState( return plan ? { schemaVersion: 1, plan } : undefined; } +export function serializeSandboxMessagingStateForDisk( + messaging: SandboxMessagingState | null | undefined, +): SandboxMessagingState | undefined { + const state = cloneSandboxMessagingState(messaging); + if (!state) return undefined; + return { + schemaVersion: 1, + plan: compactSandboxMessagingPlanForPersistence(state.plan) as unknown as SandboxMessagingPlan, + }; +} + export function getMessagingPlanFromEntry( entry: EntryWithMessaging | null | undefined, ): SandboxMessagingPlan | null { @@ -43,6 +58,13 @@ export function getMessagingPlanFromEntry( return parseSandboxMessagingPlan(entry.messaging.plan); } +export function getHydratedMessagingPlanFromEntry( + entry: EntryWithMessaging | null | undefined, +): SandboxMessagingPlan | null { + const plan = getMessagingPlanFromEntry(entry); + return plan ? hydrateDerivedSandboxMessagingPlanFields(plan) : null; +} + export function getConfiguredMessagingChannelsFromEntry( entry: EntryWithMessaging | null | undefined, ): string[] { diff --git a/src/lib/state/registry.ts b/src/lib/state/registry.ts index 20c0edccbb..43e64e2c6c 100644 --- a/src/lib/state/registry.ts +++ b/src/lib/state/registry.ts @@ -7,6 +7,7 @@ import { isErrnoException } from "../core/errno"; import { ensureConfigDir, readConfigFile, writeConfigFile } from "./config-io"; import { cloneSandboxMessagingState, + serializeSandboxMessagingStateForDisk, getConfiguredMessagingChannels as getRegistryConfiguredMessagingChannels, getDisabledChannels as getRegistryDisabledChannels, setChannelDisabled as setRegistryChannelDisabled, @@ -16,6 +17,7 @@ export { getActiveMessagingChannelsFromEntry, getConfiguredMessagingChannelsFromEntry, getDisabledMessagingChannelsFromEntry, + getHydratedMessagingPlanFromEntry, getMessagingPlanFromEntry, type SandboxMessagingState, } from "./registry-messaging"; @@ -305,11 +307,67 @@ export function withLock(fn: () => T): T { } export function load(): SandboxRegistry { - return readConfigFile(REGISTRY_FILE, { sandboxes: {}, defaultSandbox: null }); + return normalizeRegistry( + readConfigFile(REGISTRY_FILE, { sandboxes: {}, defaultSandbox: null }), + ); } export function save(data: SandboxRegistry): void { - writeConfigFile(REGISTRY_FILE, data); + writeConfigFile(REGISTRY_FILE, serializeRegistryForDisk(data)); +} + +function normalizeRegistry(data: SandboxRegistry): SandboxRegistry { + return { + defaultSandbox: data.defaultSandbox ?? null, + sandboxes: Object.fromEntries( + sandboxRegistryEntries(data).map(([name, entry]) => [name, normalizeSandboxEntry(entry)]), + ), + }; +} + +function serializeRegistryForDisk(data: SandboxRegistry): SandboxRegistry { + return { + defaultSandbox: data.defaultSandbox ?? null, + sandboxes: Object.fromEntries( + sandboxRegistryEntries(data).map(([name, entry]) => [ + name, + serializeSandboxEntryForDisk(entry), + ]), + ), + }; +} + +function sandboxRegistryEntries(data: SandboxRegistry): Array<[string, SandboxEntry]> { + const sandboxes = isRecord(data.sandboxes) ? data.sandboxes : {}; + return Object.entries(sandboxes).filter((entry): entry is [string, SandboxEntry] => + isSandboxEntryLike(entry[1]), + ); +} + +function isSandboxEntryLike(entry: unknown): entry is SandboxEntry { + return isRecord(entry); +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function normalizeSandboxEntry(entry: SandboxEntry): SandboxEntry { + const messaging = cloneSandboxMessagingState(entry.messaging); + if (!messaging) { + const { messaging: _messaging, ...rest } = entry; + return rest; + } + return { ...entry, messaging }; +} + +function serializeSandboxEntryForDisk(entry: SandboxEntry): SandboxEntry { + const messaging = serializeSandboxMessagingStateForDisk(entry.messaging); + if (!messaging) { + const { messaging: _messaging, ...rest } = entry; + return rest; + } + return { ...entry, messaging }; } export function getSandbox(name: string): SandboxEntry | null { diff --git a/test/e2e/test-channels-add-remove.sh b/test/e2e/test-channels-add-remove.sh index 6d887b69f3..45033ffbd4 100755 --- a/test/e2e/test-channels-add-remove.sh +++ b/test/e2e/test-channels-add-remove.sh @@ -225,7 +225,8 @@ const disabledChannels = Array.isArray(plan.disabledChannels) ? plan.disabledCha const credentialBindings = Array.isArray(plan.credentialBindings) ? plan.credentialBindings : []; const networkEntries = Array.isArray(plan.networkPolicy?.entries) ? plan.networkPolicy.entries : []; const networkPresets = Array.isArray(plan.networkPolicy?.presets) ? plan.networkPolicy.presets : []; -const agentRender = Array.isArray(plan.agentRender) ? plan.agentRender : []; +if (Object.hasOwn(plan, "agentRender")) fail("messaging.plan.agentRender should not be persisted"); +if (channels.some((item) => item && Object.hasOwn(item, "hooks"))) fail("messaging.plan.channels hooks should not be persisted"); if (expected === "active") { if (!channel) fail("telegram channel missing from messaging.plan.channels"); if (channel.active !== true) fail("telegram plan active expected true, got " + JSON.stringify(channel.active)); @@ -237,9 +238,6 @@ if (expected === "active") { if (!credentialBindings.some((entry) => entry?.channelId === "telegram" && entry?.providerEnvKey === "TELEGRAM_BOT_TOKEN")) { fail("telegram TELEGRAM_BOT_TOKEN credential binding missing from messaging.plan"); } - if (!agentRender.some((entry) => entry?.channelId === "telegram" && entry?.agent === "openclaw")) { - fail("telegram openclaw agent render entry missing from messaging.plan"); - } if (disabledChannels.includes("telegram")) fail("telegram unexpectedly listed in messaging.plan.disabledChannels"); } else if (expected === "removed") { if (channel) fail("telegram still present in messaging.plan.channels"); @@ -251,9 +249,6 @@ if (expected === "active") { if (credentialBindings.some((entry) => entry?.channelId === "telegram")) { fail("telegram credential binding still present in messaging.plan"); } - if (agentRender.some((entry) => entry?.channelId === "telegram")) { - fail("telegram agent render entry still present in messaging.plan"); - } } else { fail("unknown expected plan state: " + expected); } diff --git a/test/e2e/test-channels-stop-start.sh b/test/e2e/test-channels-stop-start.sh index 11d414b05d..c788d224ce 100755 --- a/test/e2e/test-channels-stop-start.sh +++ b/test/e2e/test-channels-stop-start.sh @@ -472,13 +472,8 @@ const credentialBindings = Array.isArray(plan.credentialBindings) ? plan.credent if (channelId !== "whatsapp" && !credentialBindings.some((entry) => entry?.channelId === channelId)) { fail(channelId + " credential binding missing from messaging.plan"); } -const agentRender = Array.isArray(plan.agentRender) ? plan.agentRender : []; -const buildSteps = Array.isArray(plan.buildSteps) ? plan.buildSteps : []; -const hasAgentRender = agentRender.some((entry) => entry?.channelId === channelId && entry?.agent === agent); -const hasBuildStep = buildSteps.some((entry) => entry?.channelId === channelId); -if (!hasAgentRender && !hasBuildStep) { - fail(channelId + " " + agent + " render/build-step entry missing from messaging.plan"); -} +if (Object.hasOwn(plan, "agentRender")) fail("messaging.plan.agentRender should not be persisted"); +if (channels.some((item) => item && Object.hasOwn(item, "hooks"))) fail("messaging.plan.channels hooks should not be persisted"); ' "$REGISTRY" "$ACTIVE_SANDBOX" "$ACTIVE_AGENT" "$channel" "$expected" 2>&1)"; then msg="${ACTIVE_AGENT}/${channel}: host registry messaging.plan has channel ${expected} ${context}" pass_msg "$msg" diff --git a/test/registry.test.ts b/test/registry.test.ts index e9b5591b84..e83d7c5a2a 100644 --- a/test/registry.test.ts +++ b/test/registry.test.ts @@ -241,8 +241,24 @@ describe("registry", () => { expect(rawSandbox.messagingChannels).toBeUndefined(); expect(rawSandbox.messagingChannelConfig).toBeUndefined(); expect(registry.getConfiguredMessagingChannels("messaging")).toEqual(["telegram"]); + const hydrated = registry.getHydratedMessagingPlanFromEntry(sb); + expect( + hydrated.agentRender.some((entry: { channelId: string }) => entry.channelId === "telegram"), + ).toBe(true); + expect( + hydrated.channels[0].hooks.some( + (hook: { channelId: string }) => hook.channelId === "telegram", + ), + ).toBe(true); const data = JSON.parse(fs.readFileSync(regFile, "utf-8")); - expect(data.sandboxes.messaging.messaging).toEqual({ schemaVersion: 1, plan }); + expect(data.sandboxes.messaging.messaging.schemaVersion).toBe(1); + expect(data.sandboxes.messaging.messaging.plan).toMatchObject({ + schemaVersion: 1, + sandboxName: "messaging", + channels: [{ channelId: "telegram" }], + }); + expect(data.sandboxes.messaging.messaging.plan.agentRender).toBeUndefined(); + expect(data.sandboxes.messaging.messaging.plan.channels[0].hooks).toBeUndefined(); expect(data.sandboxes.messaging.messagingChannels).toBeUndefined(); expect(data.sandboxes.messaging.messagingChannelConfig).toBeUndefined(); }); @@ -267,6 +283,27 @@ describe("registry", () => { expect(sandboxes.length).toBe(0); }); + it("skips malformed sandbox entries while loading the registry", () => { + fs.mkdirSync(path.dirname(regFile), { recursive: true }); + fs.writeFileSync( + regFile, + JSON.stringify({ + defaultSandbox: "broken", + sandboxes: { + good: { name: "good", model: "m1" }, + broken: null, + text: "not-an-entry", + }, + }), + ); + + expect(registry.getSandbox("broken")).toBe(null); + expect(registry.getDefault()).toBe("good"); + expect( + registry.listSandboxes().sandboxes.map((sandbox: { name: string }) => sandbox.name), + ).toEqual(["good"]); + }); + it("setChannelDisabled toggles a channel on and off for a sandbox", () => { registry.registerSandbox({ name: "s1",