diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 7a4356da86c..64db401e7cb 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -630,6 +630,37 @@ describe("doctor config flow", () => { }); }); + it("migrates top-level heartbeat visibility into channels.defaults.heartbeat on repair", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + heartbeat: { + showOk: true, + showAlerts: false, + }, + }, + run: loadAndMaybeMigrateDoctorConfig, + }); + + const cfg = result.cfg as { + heartbeat?: unknown; + channels?: { + defaults?: { + heartbeat?: { + showOk?: boolean; + showAlerts?: boolean; + useIndicator?: boolean; + }; + }; + }; + }; + expect(cfg.heartbeat).toBeUndefined(); + expect(cfg.channels?.defaults?.heartbeat).toMatchObject({ + showOk: true, + showAlerts: false, + }); + }); + it("repairs googlechat account dm.policy open by setting dm.allowFrom on repair", async () => { const result = await runDoctorConfigWithInput({ repair: true, diff --git a/src/config/legacy-migrate.test.ts b/src/config/legacy-migrate.test.ts index 6e116772e54..94182b41e63 100644 --- a/src/config/legacy-migrate.test.ts +++ b/src/config/legacy-migrate.test.ts @@ -113,6 +113,24 @@ describe("legacy migrate heartbeat config", () => { expect((res.config as { heartbeat?: unknown } | null)?.heartbeat).toBeUndefined(); }); + it("moves top-level heartbeat visibility into channels.defaults.heartbeat", () => { + const res = migrateLegacyConfig({ + heartbeat: { + showOk: true, + showAlerts: false, + useIndicator: false, + }, + }); + + expect(res.changes).toContain("Moved heartbeat visibility → channels.defaults.heartbeat."); + expect(res.config?.channels?.defaults?.heartbeat).toEqual({ + showOk: true, + showAlerts: false, + useIndicator: false, + }); + expect((res.config as { heartbeat?: unknown } | null)?.heartbeat).toBeUndefined(); + }); + it("keeps explicit agents.defaults.heartbeat values when merging top-level heartbeat", () => { const res = migrateLegacyConfig({ heartbeat: { @@ -139,6 +157,57 @@ describe("legacy migrate heartbeat config", () => { }); expect((res.config as { heartbeat?: unknown } | null)?.heartbeat).toBeUndefined(); }); + + it("keeps explicit channels.defaults.heartbeat values when merging top-level heartbeat visibility", () => { + const res = migrateLegacyConfig({ + heartbeat: { + showOk: true, + showAlerts: true, + }, + channels: { + defaults: { + heartbeat: { + showOk: false, + useIndicator: false, + }, + }, + }, + }); + + expect(res.changes).toContain( + "Merged heartbeat visibility → channels.defaults.heartbeat (filled missing fields from legacy; kept explicit channels.defaults values).", + ); + expect(res.config?.channels?.defaults?.heartbeat).toEqual({ + showOk: false, + showAlerts: true, + useIndicator: false, + }); + expect((res.config as { heartbeat?: unknown } | null)?.heartbeat).toBeUndefined(); + }); + + it("preserves agent.heartbeat precedence over top-level heartbeat legacy key", () => { + const res = migrateLegacyConfig({ + agent: { + heartbeat: { + every: "1h", + target: "telegram", + }, + }, + heartbeat: { + every: "30m", + target: "discord", + model: "anthropic/claude-3-5-haiku-20241022", + }, + }); + + expect(res.config?.agents?.defaults?.heartbeat).toEqual({ + every: "1h", + target: "telegram", + model: "anthropic/claude-3-5-haiku-20241022", + }); + expect((res.config as { heartbeat?: unknown } | null)?.heartbeat).toBeUndefined(); + expect((res.config as { agent?: unknown } | null)?.agent).toBeUndefined(); + }); }); describe("legacy migrate controlUi.allowedOrigins seed (issue #29385)", () => { diff --git a/src/config/legacy.migrations.part-3.ts b/src/config/legacy.migrations.part-3.ts index 2ac8fa20e73..dbc097480e5 100644 --- a/src/config/legacy.migrations.part-3.ts +++ b/src/config/legacy.migrations.part-3.ts @@ -16,6 +16,51 @@ import { } from "./legacy.shared.js"; import { DEFAULT_GATEWAY_PORT } from "./paths.js"; +const AGENT_HEARTBEAT_KEYS = new Set([ + "every", + "activeHours", + "model", + "session", + "includeReasoning", + "target", + "directPolicy", + "to", + "accountId", + "prompt", + "ackMaxChars", + "suppressToolErrorWarnings", + "lightContext", +]); + +const CHANNEL_HEARTBEAT_KEYS = new Set(["showOk", "showAlerts", "useIndicator"]); + +function splitLegacyHeartbeat(legacyHeartbeat: Record): { + agentHeartbeat: Record | null; + channelHeartbeat: Record | null; +} { + const agentHeartbeat: Record = {}; + const channelHeartbeat: Record = {}; + + for (const [key, value] of Object.entries(legacyHeartbeat)) { + if (CHANNEL_HEARTBEAT_KEYS.has(key)) { + channelHeartbeat[key] = value; + continue; + } + if (AGENT_HEARTBEAT_KEYS.has(key)) { + agentHeartbeat[key] = value; + continue; + } + // Preserve unknown fields under the agent heartbeat namespace so validation + // still surfaces unsupported keys instead of silently dropping user input. + agentHeartbeat[key] = value; + } + + return { + agentHeartbeat: Object.keys(agentHeartbeat).length > 0 ? agentHeartbeat : null, + channelHeartbeat: Object.keys(channelHeartbeat).length > 0 ? channelHeartbeat : null, + }; +} + // NOTE: tools.alsoAllow was introduced after legacy migrations; no legacy migration needed. // tools.alsoAllow legacy migration intentionally omitted (field not shipped in prod). @@ -95,36 +140,6 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ delete raw.memorySearch; }, }, - { - id: "heartbeat->agents.defaults.heartbeat", - describe: "Move top-level heartbeat to agents.defaults.heartbeat", - apply: (raw, changes) => { - const legacyHeartbeat = getRecord(raw.heartbeat); - if (!legacyHeartbeat) { - return; - } - - const agents = ensureRecord(raw, "agents"); - const defaults = ensureRecord(agents, "defaults"); - const existing = getRecord(defaults.heartbeat); - if (!existing) { - defaults.heartbeat = legacyHeartbeat; - changes.push("Moved heartbeat → agents.defaults.heartbeat."); - } else { - // agents.defaults stays authoritative; legacy top-level config only fills gaps. - const merged = structuredClone(existing); - mergeMissing(merged, legacyHeartbeat); - defaults.heartbeat = merged; - changes.push( - "Merged heartbeat → agents.defaults.heartbeat (filled missing fields from legacy; kept explicit agents.defaults values).", - ); - } - - agents.defaults = defaults; - raw.agents = agents; - delete raw.heartbeat; - }, - }, { id: "auth.anthropic-claude-cli-mode-oauth", describe: "Switch anthropic:claude-cli auth profile mode to oauth", @@ -275,6 +290,62 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ changes.push("Moved agent → agents.defaults."); }, }, + { + id: "heartbeat->agents.defaults.heartbeat", + describe: "Move top-level heartbeat to agents.defaults.heartbeat/channels.defaults.heartbeat", + apply: (raw, changes) => { + const legacyHeartbeat = getRecord(raw.heartbeat); + if (!legacyHeartbeat) { + return; + } + + const { agentHeartbeat, channelHeartbeat } = splitLegacyHeartbeat(legacyHeartbeat); + + if (agentHeartbeat) { + const agents = ensureRecord(raw, "agents"); + const defaults = ensureRecord(agents, "defaults"); + const existing = getRecord(defaults.heartbeat); + if (!existing) { + defaults.heartbeat = agentHeartbeat; + changes.push("Moved heartbeat → agents.defaults.heartbeat."); + } else { + // agents.defaults stays authoritative; legacy top-level config only fills gaps. + const merged = structuredClone(existing); + mergeMissing(merged, agentHeartbeat); + defaults.heartbeat = merged; + changes.push( + "Merged heartbeat → agents.defaults.heartbeat (filled missing fields from legacy; kept explicit agents.defaults values).", + ); + } + + agents.defaults = defaults; + raw.agents = agents; + } + + if (channelHeartbeat) { + const channels = ensureRecord(raw, "channels"); + const defaults = ensureRecord(channels, "defaults"); + const existing = getRecord(defaults.heartbeat); + if (!existing) { + defaults.heartbeat = channelHeartbeat; + changes.push("Moved heartbeat visibility → channels.defaults.heartbeat."); + } else { + // channels.defaults stays authoritative; legacy top-level config only fills gaps. + const merged = structuredClone(existing); + mergeMissing(merged, channelHeartbeat); + defaults.heartbeat = merged; + changes.push( + "Merged heartbeat visibility → channels.defaults.heartbeat (filled missing fields from legacy; kept explicit channels.defaults values).", + ); + } + + channels.defaults = defaults; + raw.channels = channels; + } + + delete raw.heartbeat; + }, + }, { id: "identity->agents.list", describe: "Move identity to agents.list[].identity", diff --git a/src/config/legacy.rules.ts b/src/config/legacy.rules.ts index f61ae6ccdbb..420f6a4685d 100644 --- a/src/config/legacy.rules.ts +++ b/src/config/legacy.rules.ts @@ -207,6 +207,6 @@ export const LEGACY_CONFIG_RULES: LegacyConfigRule[] = [ { path: ["heartbeat"], message: - "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat instead.", + "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat (cadence/target/model settings) or channels.defaults.heartbeat (showOk/showAlerts/useIndicator).", }, ]; diff --git a/src/gateway/server.legacy-migration.test.ts b/src/gateway/server.legacy-migration.test.ts index 6e306e8a2d0..0522f8a858e 100644 --- a/src/gateway/server.legacy-migration.test.ts +++ b/src/gateway/server.legacy-migration.test.ts @@ -14,7 +14,7 @@ describe("gateway startup legacy migration fallback", () => { { path: "heartbeat", message: - "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat instead.", + "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat (cadence/target/model settings) or channels.defaults.heartbeat (showOk/showAlerts/useIndicator).", }, ]; testState.legacyParsed = { @@ -39,7 +39,7 @@ describe("gateway startup legacy migration fallback", () => { const message = String((thrown as Error).message); expect(message).toContain("Invalid config at"); expect(message).toContain( - "heartbeat: top-level heartbeat is not a valid config path; use agents.defaults.heartbeat instead.", + "heartbeat: top-level heartbeat is not a valid config path; use agents.defaults.heartbeat (cadence/target/model settings) or channels.defaults.heartbeat (showOk/showAlerts/useIndicator).", ); expect(message).not.toContain("Legacy config entries detected but auto-migration failed."); }); @@ -49,7 +49,7 @@ describe("gateway startup legacy migration fallback", () => { { path: "heartbeat", message: - "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat instead.", + "top-level heartbeat is not a valid config path; use agents.defaults.heartbeat (cadence/target/model settings) or channels.defaults.heartbeat (showOk/showAlerts/useIndicator).", }, ]; // Simulate a parsed source that only contains include directives, while @@ -76,7 +76,7 @@ describe("gateway startup legacy migration fallback", () => { const message = String((thrown as Error).message); expect(message).toContain("Invalid config at"); expect(message).toContain( - "heartbeat: top-level heartbeat is not a valid config path; use agents.defaults.heartbeat instead.", + "heartbeat: top-level heartbeat is not a valid config path; use agents.defaults.heartbeat (cadence/target/model settings) or channels.defaults.heartbeat (showOk/showAlerts/useIndicator).", ); expect(message).not.toContain("Legacy config entries detected but auto-migration failed."); });