fix(doctor): materialize group allowFrom fallback (#82316)

steipete · web-flow · commit cce12697a1ee · 2026-05-15T21:47:49.000+01:00
* fix(doctor): materialize group allowFrom fallback

* fix: normalize doctor account records
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ Docs: https://docs.openclaw.ai
 - Config persistence: ignore malformed array/scalar auth profile, cron job state, and session store entries instead of hydrating them into numeric profile ids, crashed cron rows, or invalid session records.
 - Providers: reject malformed successful Runway, BytePlus, and Ollama embedding responses with provider-owned errors instead of raw parser/type failures, silent bad vectors, or long bogus polling.
 - Trajectory export: skip and report malformed session/runtime JSONL rows in `manifest.json` instead of letting wrong-shaped session rows crash support bundle export.
+- Config/doctor: copy fallback-enabled channel `allowFrom` entries into explicit `groupAllowFrom` allowlists during `openclaw doctor --fix`, preserving current group access without adding runtime fallback-transition flags.
 - Hooks: raise bounded gateway lifecycle hook wait budgets to 5 seconds for shutdown and 10 seconds for pre-restart, giving short restart notification handlers time to finish before shutdown continues. (#82273) Thanks @bryanbaer.
 - Plugin releases: require external package compatibility metadata in the npm plugin publish plan, matching the ClawHub package contract before packages ship.
 - Agents/OpenAI-compatible: honor per-model `max_completion_tokens`/`max_tokens` params in embedded OpenAI-completions runs so high-token Kimi-style routes keep their configured completion cap. Fixes #82230. Thanks @albert-zen.
diff --git a/extensions/msteams/package.json b/extensions/msteams/package.json
@@ -48,7 +48,7 @@
       "doctorCapabilities": {
         "dmAllowFromMode": "topOnly",
         "groupModel": "hybrid",
-        "groupAllowFromFallbackToAllowFrom": false,
+        "groupAllowFromFallbackToAllowFrom": true,
         "warnOnEmptyGroupSenderAllowlist": true
       }
     },
diff --git a/extensions/msteams/src/channel.ts b/extensions/msteams/src/channel.ts
@@ -491,7 +491,7 @@ export const msteamsPlugin: ChannelPlugin<ResolvedMSTeamsAccount, ProbeMSTeamsRe
       doctor: {
         dmAllowFromMode: "topOnly",
         groupModel: "hybrid",
-        groupAllowFromFallbackToAllowFrom: false,
+        groupAllowFromFallbackToAllowFrom: true,
         warnOnEmptyGroupSenderAllowlist: true,
         collectMutableAllowlistWarnings: collectMSTeamsMutableAllowlistWarnings,
       },
diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts
@@ -654,7 +654,7 @@ vi.mock("./doctor/channel-capabilities.js", () => {
     msteams: {
       dmAllowFromMode: "topOnly",
       groupModel: "hybrid",
-      groupAllowFromFallbackToAllowFrom: false,
+      groupAllowFromFallbackToAllowFrom: true,
       warnOnEmptyGroupSenderAllowlist: true,
     },
     zalouser: {
diff --git a/src/commands/doctor/channel-capabilities.test.ts b/src/commands/doctor/channel-capabilities.test.ts
@@ -33,7 +33,7 @@ describe("doctor channel capabilities", () => {
     expect(getDoctorChannelCapabilities("msteams")).toEqual({
       dmAllowFromMode: "topOnly",
       groupModel: "hybrid",
-      groupAllowFromFallbackToAllowFrom: false,
+      groupAllowFromFallbackToAllowFrom: true,
       warnOnEmptyGroupSenderAllowlist: true,
     });
   });
diff --git a/src/commands/doctor/repair-sequencing.test.ts b/src/commands/doctor/repair-sequencing.test.ts
@@ -9,7 +9,9 @@ const mocks = vi.hoisted(() => ({
   getInstalledPluginRecord: vi.fn(),
   isInstalledPluginEnabled: vi.fn(),
   loadInstalledPluginIndex: vi.fn(),
+  maybeRepairGroupAllowFromFallback: vi.fn(),
   maybeRepairManagedNpmOpenClawPeerLinks: vi.fn(),
+  maybeRepairOpenPolicyAllowFrom: vi.fn(),
   maybeRepairStaleManagedNpmBundledPlugins: vi.fn(),
   maybeRepairStalePluginConfig: vi.fn(),
   repairStaleOAuthProfileShadows: vi.fn(),
@@ -101,6 +103,10 @@ vi.mock("./shared/allowlist-policy-repair.js", () => ({
   }),
 }));
 
+vi.mock("./shared/allowfrom-fallback-migration.js", () => ({
+  maybeRepairGroupAllowFromFallback: mocks.maybeRepairGroupAllowFromFallback,
+}));
+
 vi.mock("./shared/bundled-plugin-load-paths.js", () => ({
   maybeRepairBundledPluginLoadPaths: (cfg: OpenClawConfig) => ({
     config: cfg,
@@ -109,10 +115,7 @@ vi.mock("./shared/bundled-plugin-load-paths.js", () => ({
 }));
 
 vi.mock("./shared/open-policy-allowfrom.js", () => ({
-  maybeRepairOpenPolicyAllowFrom: (cfg: OpenClawConfig) => ({
-    config: cfg,
-    changes: [],
-  }),
+  maybeRepairOpenPolicyAllowFrom: mocks.maybeRepairOpenPolicyAllowFrom,
 }));
 
 vi.mock("./shared/stale-plugin-config.js", () => ({
@@ -174,6 +177,13 @@ vi.mock("./shared/exec-safe-bins.js", () => ({
   }),
 }));
 
+vi.mock("./shared/plugin-dependency-cleanup.js", () => ({
+  cleanupLegacyPluginDependencyState: async () => ({
+    changes: [],
+    warnings: [],
+  }),
+}));
+
 describe("doctor repair sequencing", () => {
   beforeEach(() => {
     vi.clearAllMocks();
@@ -192,7 +202,15 @@ describe("doctor repair sequencing", () => {
     mocks.getInstalledPluginRecord.mockReturnValue(undefined);
     mocks.isInstalledPluginEnabled.mockReturnValue(false);
     mocks.loadInstalledPluginIndex.mockReturnValue({ plugins: [] });
+    mocks.maybeRepairGroupAllowFromFallback.mockImplementation((cfg: OpenClawConfig) => ({
+      config: cfg,
+      changes: [],
+    }));
     mocks.maybeRepairManagedNpmOpenClawPeerLinks.mockResolvedValue(false);
+    mocks.maybeRepairOpenPolicyAllowFrom.mockImplementation((cfg: OpenClawConfig) => ({
+      config: cfg,
+      changes: [],
+    }));
     mocks.maybeRepairStaleManagedNpmBundledPlugins.mockReturnValue(false);
     mocks.repairMissingConfiguredPluginInstalls.mockResolvedValue({
       changes: [],
@@ -444,6 +462,72 @@ describe("doctor repair sequencing", () => {
     ]);
   });
 
+  it("runs group allowFrom fallback migration after open-policy allowFrom repair", async () => {
+    const events: string[] = [];
+    mocks.maybeRepairOpenPolicyAllowFrom.mockImplementationOnce((cfg: OpenClawConfig) => {
+      events.push("open-policy");
+      return {
+        config: {
+          ...cfg,
+          channels: {
+            ...cfg.channels,
+            signal: {
+              ...cfg.channels?.signal,
+              allowFrom: ["*"],
+            },
+          },
+        },
+        changes: ['channels.signal.allowFrom: set to ["*"]'],
+      };
+    });
+    mocks.maybeRepairGroupAllowFromFallback.mockImplementationOnce((cfg: OpenClawConfig) => {
+      events.push("group-fallback");
+      expect(cfg.channels?.signal?.allowFrom).toEqual(["*"]);
+      return {
+        config: {
+          ...cfg,
+          channels: {
+            ...cfg.channels,
+            signal: {
+              ...cfg.channels?.signal,
+              groupAllowFrom: ["*"],
+            },
+          },
+        },
+        changes: ["channels.signal.groupAllowFrom: copied 1 sender entry from allowFrom"],
+      };
+    });
+
+    const result = await runDoctorRepairSequence({
+      state: {
+        cfg: {
+          channels: {
+            signal: {
+              dmPolicy: "open",
+            },
+          },
+        } as OpenClawConfig,
+        candidate: {
+          channels: {
+            signal: {
+              dmPolicy: "open",
+            },
+          },
+        } as OpenClawConfig,
+        pendingChanges: false,
+        fixHints: [],
+      },
+      doctorFixCommand: "openclaw doctor --fix",
+    });
+
+    expect(events).toEqual(["open-policy", "group-fallback"]);
+    expect(result.state.candidate.channels?.signal?.groupAllowFrom).toEqual(["*"]);
+    expect(result.changeNotes).toContain('channels.signal.allowFrom: set to ["*"]');
+    expect(result.changeNotes).toContain(
+      "channels.signal.groupAllowFrom: copied 1 sender entry from allowFrom",
+    );
+  });
+
   it("does not remove deferred configured plugins during the package update doctor pass", async () => {
     mocks.repairMissingConfiguredPluginInstalls.mockResolvedValueOnce({
       changes: [
diff --git a/src/commands/doctor/repair-sequencing.ts b/src/commands/doctor/repair-sequencing.ts
@@ -4,6 +4,7 @@ import {
   maybeRepairManagedNpmOpenClawPeerLinks,
   maybeRepairStaleManagedNpmBundledPlugins,
 } from "../doctor-plugin-registry.js";
+import { maybeRepairGroupAllowFromFallback } from "./shared/allowfrom-fallback-migration.js";
 import { maybeRepairAllowlistPolicyAllowFrom } from "./shared/allowlist-policy-repair.js";
 import { maybeRepairBundledPluginLoadPaths } from "./shared/bundled-plugin-load-paths.js";
 import {
@@ -66,7 +67,6 @@ export async function runDoctorRepairSequence(params: {
   })) {
     applyMutation(mutation);
   }
-  applyMutation(maybeRepairOpenPolicyAllowFrom(state.candidate));
   applyMutation(maybeRepairBundledPluginLoadPaths(state.candidate, env));
   maybeRepairStaleManagedNpmBundledPlugins({
     config: state.candidate,
@@ -106,6 +106,8 @@ export async function runDoctorRepairSequence(params: {
   }
   applyMutation(maybeRepairInvalidPluginConfig(state.candidate));
   applyMutation(await maybeRepairAllowlistPolicyAllowFrom(state.candidate));
+  applyMutation(maybeRepairOpenPolicyAllowFrom(state.candidate));
+  applyMutation(maybeRepairGroupAllowFromFallback(state.candidate));
 
   const emptyAllowlistWarnings = scanEmptyAllowlistPolicyWarnings(state.candidate, {
     doctorFixCommand: params.doctorFixCommand,
diff --git a/src/commands/doctor/shared/allowfrom-fallback-migration.test.ts b/src/commands/doctor/shared/allowfrom-fallback-migration.test.ts
@@ -0,0 +1,117 @@
+import { describe, expect, it, vi } from "vitest";
+import { maybeRepairGroupAllowFromFallback } from "./allowfrom-fallback-migration.js";
+
+vi.mock("../channel-capabilities.js", () => ({
+  getDoctorChannelCapabilities: (channelName?: string) => ({
+    dmAllowFromMode: channelName === "matrix" ? "nestedOnly" : "topOnly",
+    groupModel: "sender",
+    groupAllowFromFallbackToAllowFrom: channelName !== "discord",
+    warnOnEmptyGroupSenderAllowlist: true,
+  }),
+}));
+
+describe("doctor group allowFrom fallback migration", () => {
+  it("copies fallback allowFrom into explicit groupAllowFrom", () => {
+    const result = maybeRepairGroupAllowFromFallback({
+      channels: {
+        telegram: {
+          allowFrom: [123, "accessGroup:ops", "123"],
+          groupPolicy: "allowlist",
+        },
+      },
+    });
+
+    expect(result.changes).toEqual([
+      "channels.telegram.groupAllowFrom: copied 2 sender entries from allowFrom for explicit group allowlist.",
+    ]);
+    expect(result.config.channels?.telegram?.groupAllowFrom).toEqual(["123", "accessGroup:ops"]);
+  });
+
+  it("uses canonical nested dm allowFrom for nested channels", () => {
+    const result = maybeRepairGroupAllowFromFallback({
+      channels: {
+        matrix: {
+          allowFrom: ["@legacy:example.org"],
+          dm: {
+            allowFrom: ["@alice:example.org"],
+          },
+        },
+      },
+    });
+
+    expect(result.changes).toEqual([
+      "channels.matrix.groupAllowFrom: copied 1 sender entry from allowFrom for explicit group allowlist.",
+    ]);
+    expect(result.config.channels?.matrix?.groupAllowFrom).toEqual(["@alice:example.org"]);
+  });
+
+  it("preserves account-scoped fallback without broadening to the channel", () => {
+    const result = maybeRepairGroupAllowFromFallback({
+      channels: {
+        signal: {
+          allowFrom: ["parent"],
+          accounts: {
+            work: { allowFrom: ["work-user"] },
+            personal: { groupAllowFrom: ["personal-user"], allowFrom: ["ignored"] },
+          },
+        },
+      },
+    });
+
+    expect(result.changes).toEqual([
+      "channels.signal.groupAllowFrom: copied 1 sender entry from allowFrom for explicit group allowlist.",
+      "channels.signal.accounts.work.groupAllowFrom: copied 1 sender entry from allowFrom for explicit group allowlist.",
+    ]);
+    expect(result.config.channels?.signal?.groupAllowFrom).toEqual(["parent"]);
+    expect(result.config.channels?.signal?.accounts?.work?.groupAllowFrom).toEqual(["work-user"]);
+    expect(result.config.channels?.signal?.accounts?.personal?.groupAllowFrom).toEqual([
+      "personal-user",
+    ]);
+  });
+
+  it("does not shadow an inherited channel group allowlist for accounts", () => {
+    const result = maybeRepairGroupAllowFromFallback({
+      channels: {
+        telegram: {
+          allowFrom: ["dm-user"],
+          groupAllowFrom: ["group-user"],
+          accounts: {
+            work: { allowFrom: ["work-dm-user"] },
+          },
+        },
+      },
+    });
+
+    expect(result).toEqual({
+      config: {
+        channels: {
+          telegram: {
+            allowFrom: ["dm-user"],
+            groupAllowFrom: ["group-user"],
+            accounts: {
+              work: { allowFrom: ["work-dm-user"] },
+            },
+          },
+        },
+      },
+      changes: [],
+    });
+  });
+
+  it("skips disabled channels, disabled accounts, and channels without fallback", () => {
+    const cfg = {
+      channels: {
+        disabled: { enabled: false, allowFrom: ["user"] },
+        telegram: {
+          accounts: {
+            disabled: { enabled: false, allowFrom: ["user"] },
+          },
+        },
+        discord: { allowFrom: ["user"] },
+        tools: { allowFrom: ["user"] },
+      },
+    };
+
+    expect(maybeRepairGroupAllowFromFallback(cfg)).toEqual({ config: cfg, changes: [] });
+  });
+});
diff --git a/src/commands/doctor/shared/allowfrom-fallback-migration.ts b/src/commands/doctor/shared/allowfrom-fallback-migration.ts

Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@`
`48`	`48`	`"doctorCapabilities": {`
`49`	`49`	`"dmAllowFromMode": "topOnly",`
`50`	`50`	`"groupModel": "hybrid",`
`51`		`- "groupAllowFromFallbackToAllowFrom": false,`
	`51`	`+ "groupAllowFromFallbackToAllowFrom": true,`
`52`	`52`	`"warnOnEmptyGroupSenderAllowlist": true`
`53`	`53`	`}`
`54`	`54`	`},`