From cfba24fa3cbc268d2ad8def823f8e717c1f2ff0b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 17 Apr 2026 07:32:41 +0100 Subject: [PATCH] test: move open policy repair cases to unit seam --- src/commands/doctor-config-flow.test.ts | 133 ------------------ .../shared/open-policy-allowfrom.test.ts | 43 ++++++ 2 files changed, 43 insertions(+), 133 deletions(-) diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 8732f1b3b35..9cb5033d29e 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -896,19 +896,6 @@ function resetTerminalNoteMock() { return terminalNoteMock; } -function expectGoogleChatDmAllowFromRepaired(cfg: unknown) { - const typed = cfg as { - channels: { - googlechat: { - dm: { allowFrom: string[] }; - allowFrom?: string[]; - }; - }; - }; - expect(typed.channels.googlechat.dm.allowFrom).toEqual(["*"]); - expect(typed.channels.googlechat.allowFrom).toBeUndefined(); -} - async function collectDoctorWarnings(config: Record): Promise { const noteSpy = resetTerminalNoteMock(); await runDoctorConfigWithInput({ @@ -1794,108 +1781,6 @@ describe("doctor config flow", () => { expect(cfg.channels.discord.dmPolicy).toBe("open"); }); - it("adds * to existing allowFrom array when dmPolicy is open on repair", async () => { - const result = await runDoctorConfigWithInput({ - repair: true, - config: { - channels: { - slack: { - botToken: "xoxb-test", - appToken: "xapp-test", - dmPolicy: "open", - allowFrom: ["U123"], - }, - }, - }, - run: loadAndMaybeMigrateDoctorConfig, - }); - - const cfg = result.cfg as unknown as { - channels: { slack: { allowFrom: string[] } }; - }; - expect(cfg.channels.slack.allowFrom).toContain("*"); - expect(cfg.channels.slack.allowFrom).toContain("U123"); - }); - - it("repairs nested dm.allowFrom when top-level allowFrom is absent on repair", async () => { - const result = await runDoctorConfigWithInput({ - repair: true, - config: { - channels: { - discord: { - token: "test-token", - dmPolicy: "open", - dm: { allowFrom: ["123"] }, - }, - }, - }, - run: loadAndMaybeMigrateDoctorConfig, - }); - - const cfg = result.cfg as unknown as { - channels: { discord: { dm: { allowFrom: string[] }; allowFrom?: string[] } }; - }; - // Top-level allowFrom is canonical for Discord; nested dm.allowFrom is - // preserved when it already carried sender ids. - if (cfg.channels.discord.allowFrom) { - expect(cfg.channels.discord.allowFrom).toContain("*"); - expect(cfg.channels.discord.dm?.allowFrom).toContain("123"); - } else if (cfg.channels.discord.dm) { - expect(cfg.channels.discord.dm.allowFrom).toContain("*"); - expect(cfg.channels.discord.dm.allowFrom).toContain("123"); - } else { - expect.unreachable("expected Discord repair to preserve a DM allowFrom location"); - } - }); - - it("skips repair when allowFrom already includes *", async () => { - const result = await runDoctorConfigWithInput({ - repair: true, - config: { - channels: { - discord: { - token: "test-token", - dmPolicy: "open", - allowFrom: ["*"], - }, - }, - }, - run: loadAndMaybeMigrateDoctorConfig, - }); - - const cfg = result.cfg as unknown as { - channels: { discord: { allowFrom: string[] } }; - }; - expect(cfg.channels.discord.allowFrom).toEqual(["*"]); - }); - - it("repairs per-account dmPolicy open without allowFrom on repair", async () => { - const result = await runDoctorConfigWithInput({ - repair: true, - config: { - channels: { - discord: { - token: "test-token", - accounts: { - work: { - token: "test-token-2", - dmPolicy: "open", - }, - }, - }, - }, - }, - run: loadAndMaybeMigrateDoctorConfig, - }); - - const cfg = result.cfg as unknown as { - channels: { - discord: { accounts: { work: { allowFrom: string[]; dmPolicy: string } } }; - }; - }; - expect(cfg.channels.discord.accounts.work.allowFrom).toEqual(["*"]); - }); - it('repairs dmPolicy="allowlist" by restoring allowFrom from pairing store on repair', async () => { const result = await withTempHome( async (home) => { @@ -1986,24 +1871,6 @@ describe("doctor config flow", () => { expect(toolsBySender["*"]).toEqual({ deny: ["exec"] }); }); - it("repairs googlechat dm.policy open by setting dm.allowFrom on repair", async () => { - const result = await runDoctorConfigWithInput({ - repair: true, - config: { - channels: { - googlechat: { - dm: { - policy: "open", - }, - }, - }, - }, - run: loadAndMaybeMigrateDoctorConfig, - }); - - expectGoogleChatDmAllowFromRepaired(result.cfg); - }); - it("migrates top-level heartbeat into agents.defaults.heartbeat on repair", async () => { const result = await runDoctorConfigWithInput({ repair: true, diff --git a/src/commands/doctor/shared/open-policy-allowfrom.test.ts b/src/commands/doctor/shared/open-policy-allowfrom.test.ts index f9f91ad3cef..e404d27f730 100644 --- a/src/commands/doctor/shared/open-policy-allowfrom.test.ts +++ b/src/commands/doctor/shared/open-policy-allowfrom.test.ts @@ -85,6 +85,49 @@ describe("doctor open-policy allowFrom repair", () => { expect(result.config.channels?.discord?.dm?.allowFrom).toEqual(["123", "*"]); }); + it("appends wildcard to existing top-level allowFrom", () => { + const result = maybeRepairOpenPolicyAllowFrom({ + channels: { + slack: { + dmPolicy: "open", + allowFrom: ["U123"], + }, + }, + }); + + expect(result.config.channels?.slack?.allowFrom).toEqual(["U123", "*"]); + }); + + it("skips top-level allowFrom that already includes a wildcard", () => { + const result = maybeRepairOpenPolicyAllowFrom({ + channels: { + discord: { + dmPolicy: "open", + allowFrom: ["*"], + }, + }, + }); + + expect(result.changes).toEqual([]); + expect(result.config.channels?.discord?.allowFrom).toEqual(["*"]); + }); + + it("repairs per-account open dmPolicy without allowFrom", () => { + const result = maybeRepairOpenPolicyAllowFrom({ + channels: { + discord: { + accounts: { + work: { + dmPolicy: "open", + }, + }, + }, + }, + }); + + expect(result.config.channels?.discord?.accounts?.work?.allowFrom).toEqual(["*"]); + }); + it("formats open-policy wildcard warnings", () => { const warnings = collectOpenPolicyAllowFromWarnings({ changes: ['- channels.signal.allowFrom: set to ["*"] (required by dmPolicy="open")'],