Fix multi-account channel binding drift for #836 (#841)

2026-04-13 15:30:13 +08:00
parent 4ff6861042
commit b2c478d554
5 changed files with 600 additions and 19 deletions
--- a/tests/e2e/channels-binding-regression.spec.ts
+++ b/tests/e2e/channels-binding-regression.spec.ts
@@ -0,0 +1,130 @@
+import { completeSetup, expect, test } from './fixtures/electron';
+
+test.describe('Channels binding regression', () => {
+  test('keeps newly added non-default Feishu accounts unassigned until the user binds an agent', async ({ electronApp, page }) => {
+    await electronApp.evaluate(({ ipcMain }) => {
+      const state = {
+        nextAccountId: 'feishu-a1b2c3d4',
+        saveCount: 0,
+        bindingCount: 0,
+        channels: [
+          {
+            channelType: 'feishu',
+            defaultAccountId: 'default',
+            status: 'connected',
+            accounts: [
+              {
+                accountId: 'default',
+                name: 'Primary Account',
+                configured: true,
+                status: 'connected',
+                isDefault: true,
+                agentId: 'main',
+              },
+            ],
+          },
+        ],
+        agents: [
+          { id: 'main', name: 'Main Agent' },
+          { id: 'code', name: 'Code Agent' },
+        ],
+      };
+
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (globalThis as any).__clawxE2eBindingRegression = state;
+
+      ipcMain.removeHandler('hostapi:fetch');
+      ipcMain.handle('hostapi:fetch', async (_event, request: { path?: string; method?: string; body?: string }) => {
+        const method = request?.method ?? 'GET';
+        const path = request?.path ?? '';
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const current = (globalThis as any).__clawxE2eBindingRegression as typeof state;
+
+        if (path === '/api/channels/accounts' && method === 'GET') {
+          return { ok: true, data: { status: 200, ok: true, json: { success: true, channels: current.channels } } };
+        }
+        if (path === '/api/agents' && method === 'GET') {
+          return { ok: true, data: { status: 200, ok: true, json: { success: true, agents: current.agents } } };
+        }
+        if (path === '/api/channels/credentials/validate' && method === 'POST') {
+          return { ok: true, data: { status: 200, ok: true, json: { success: true, valid: true, warnings: [] } } };
+        }
+        if (path === '/api/channels/config' && method === 'POST') {
+          current.saveCount += 1;
+          const body = JSON.parse(request?.body ?? '{}') as { accountId?: string };
+          const accountId = body.accountId || current.nextAccountId;
+          const feishu = current.channels[0];
+          if (!feishu.accounts.some((account) => account.accountId === accountId)) {
+            feishu.accounts.push({
+              accountId,
+              name: accountId,
+              configured: true,
+              status: 'connected',
+              isDefault: false,
+            });
+          }
+          return { ok: true, data: { status: 200, ok: true, json: { success: true } } };
+        }
+        if (path === '/api/channels/binding' && method === 'PUT') {
+          current.bindingCount += 1;
+          const body = JSON.parse(request?.body ?? '{}') as { channelType?: string; accountId?: string; agentId?: string };
+          if (body.channelType === 'feishu' && body.accountId) {
+            const feishu = current.channels[0];
+            const account = feishu.accounts.find((entry) => entry.accountId === body.accountId);
+            if (account) {
+              account.agentId = body.agentId;
+            }
+          }
+          return { ok: true, data: { status: 200, ok: true, json: { success: true } } };
+        }
+        if (path === '/api/channels/binding' && method === 'DELETE') {
+          current.bindingCount += 1;
+          return { ok: true, data: { status: 200, ok: true, json: { success: true } } };
+        }
+        if (path.startsWith('/api/channels/config/') && method === 'GET') {
+          return { ok: true, data: { status: 200, ok: true, json: { success: true, values: {} } } };
+        }
+
+        return {
+          ok: false,
+          error: { message: `Unexpected hostapi:fetch request: ${method} ${path}` },
+        };
+      });
+    });
+
+    await completeSetup(page);
+
+    await page.getByTestId('sidebar-nav-channels').click();
+    await expect(page.getByTestId('channels-page')).toBeVisible();
+    await expect(page.getByText('Feishu / Lark')).toBeVisible();
+
+    await page.getByRole('button', { name: /Add Account|添加账号|アカウントを追加/ }).click();
+    await expect(page.getByText(/Configure Feishu \/ Lark|dialog\.configureTitle/)).toBeVisible();
+
+    const accountIdInput = page.locator('#account-id');
+    const newAccountId = await accountIdInput.inputValue();
+    await expect(accountIdInput).toHaveValue(/feishu-/);
+    await page.locator('#appId').fill('cli_test');
+    await page.locator('#appSecret').fill('secret_test');
+
+    await page.getByRole('button', { name: /Save & Connect|dialog\.saveAndConnect/ }).click();
+    await expect(page.getByText(/Configure Feishu \/ Lark|dialog\.configureTitle/)).toBeHidden();
+
+    const newAccountRow = page.locator('div.rounded-xl').filter({ hasText: newAccountId }).first();
+    await expect(newAccountRow).toBeVisible();
+    const bindingSelect = newAccountRow.locator('select');
+    await expect(bindingSelect).toHaveValue('');
+
+    await bindingSelect.selectOption('code');
+    await expect(bindingSelect).toHaveValue('code');
+
+    const counters = await electronApp.evaluate(() => {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const state = (globalThis as any).__clawxE2eBindingRegression as { saveCount: number; bindingCount: number };
+      return { saveCount: state.saveCount, bindingCount: state.bindingCount };
+    });
+
+    expect(counters.saveCount).toBe(1);
+    expect(counters.bindingCount).toBe(1);
+  });
+});
--- a/tests/unit/agent-config.test.ts
+++ b/tests/unit/agent-config.test.ts
@@ -379,7 +379,7 @@ describe('agent config lifecycle', () => {
    expect(snapshot.channelAccountOwners['telegram:default']).toBe('main');
  });

-  it('replaces previous account binding for the same agent and channel', async () => {
+  it('keeps sibling account bindings for the same agent and channel', async () => {
    await writeOpenClawJson({
      agents: {
        list: [
@@ -404,10 +404,40 @@ describe('agent config lifecycle', () => {
    await assignChannelAccountToAgent('main', 'feishu', 'alt');

    const snapshot = await listAgentsSnapshot();
-    expect(snapshot.channelAccountOwners['feishu:default']).toBeUndefined();
+    expect(snapshot.channelAccountOwners['feishu:default']).toBe('main');
    expect(snapshot.channelAccountOwners['feishu:alt']).toBe('main');
  });

+  it('preserves original agentId casing when persisting bindings', async () => {
+    await writeOpenClawJson({
+      agents: {
+        list: [
+          { id: 'MainAgent', name: 'Main Agent', default: true },
+        ],
+      },
+      channels: {
+        feishu: {
+          enabled: true,
+          accounts: {
+            default: { enabled: true, appId: 'main-app' },
+          },
+        },
+      },
+    });
+
+    const { assignChannelAccountToAgent } = await import('@electron/utils/agent-config');
+
+    await assignChannelAccountToAgent('MainAgent', 'feishu', 'default');
+
+    const config = await readOpenClawJson();
+    expect(config.bindings).toEqual([
+      {
+        agentId: 'MainAgent',
+        match: { channel: 'feishu', accountId: 'default' },
+      },
+    ]);
+  });
+
  it('keeps a single owner for the same channel account', async () => {
    await writeOpenClawJson({
      agents: {
--- a/tests/unit/channel-routes.test.ts
+++ b/tests/unit/channel-routes.test.ts
@@ -102,6 +102,7 @@ describe('handleChannelRoutes', () => {
    listConfiguredChannelAccountsMock.mockReturnValue({});
    listAgentsSnapshotMock.mockResolvedValue({
      agents: [],
+      channelOwners: {},
      channelAccountOwners: {},
    });
    readOpenClawConfigMock.mockResolvedValue({
@@ -367,6 +368,11 @@ describe('handleChannelRoutes', () => {
        accountIds: ['default', 'Legacy_Account'],
      },
    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main Agent' }],
+      channelOwners: {},
+      channelAccountOwners: {},
+    });

    parseJsonBodyMock.mockResolvedValue({
      channelType: 'feishu',
@@ -409,6 +415,353 @@ describe('handleChannelRoutes', () => {
    expect(assignChannelAccountToAgentMock).toHaveBeenCalledWith('main', 'feishu', 'Legacy_Account');
  });

+  it('migrates legacy channel-wide fallback before manually binding a non-default account', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main' }, { id: 'code', name: 'Code Agent' }],
+      channelOwners: { telegram: 'main' },
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'main', match: { channel: 'telegram' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      agentId: 'code',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(1, 'main', 'telegram', 'default');
+    expect(clearChannelBindingMock).toHaveBeenCalledWith('telegram');
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(2, 'code', 'telegram', 'telegram-a1b2c3d4');
+  });
+
+  it('does not synthesize a default binding when no legacy channel-wide binding exists', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main' }, { id: 'code', name: 'Code Agent' }],
+      channelOwners: { telegram: 'code' },
+      channelAccountOwners: {
+        'telegram:telegram-a1b2c3d4': 'code',
+      },
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'code', match: { channel: 'telegram', accountId: 'telegram-a1b2c3d4' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-b2c3d4e5',
+      agentId: 'code',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(clearChannelBindingMock).not.toHaveBeenCalled();
+    expect(assignChannelAccountToAgentMock).toHaveBeenCalledTimes(1);
+    expect(assignChannelAccountToAgentMock).toHaveBeenCalledWith('code', 'telegram', 'telegram-b2c3d4e5');
+  });
+
+  it('preserves mixed-case agent ids when migrating a legacy channel-wide binding', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'MainAgent', name: 'Main Agent' }, { id: 'code', name: 'Code Agent' }],
+      channelOwners: { telegram: 'mainagent' },
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'MainAgent', match: { channel: 'telegram' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      agentId: 'code',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(1, 'MainAgent', 'telegram', 'default');
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(2, 'code', 'telegram', 'telegram-a1b2c3d4');
+  });
+
+  it('does not mutate legacy bindings when the requested agent does not exist', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main Agent' }],
+      channelOwners: { telegram: 'main' },
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'main', match: { channel: 'telegram' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      agentId: 'missing-agent',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(clearChannelBindingMock).not.toHaveBeenCalled();
+    expect(assignChannelAccountToAgentMock).not.toHaveBeenCalled();
+    expect(sendJsonMock).toHaveBeenCalledWith(
+      expect.anything(),
+      500,
+      expect.objectContaining({
+        success: false,
+        error: expect.stringContaining('Agent "missing-agent" not found'),
+      }),
+    );
+  });
+
+  it('rejects binding requests without accountId before legacy migration runs', async () => {
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main Agent' }],
+      channelOwners: {},
+      channelAccountOwners: {},
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      agentId: 'main',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(clearChannelBindingMock).not.toHaveBeenCalled();
+    expect(assignChannelAccountToAgentMock).not.toHaveBeenCalled();
+    expect(sendJsonMock).toHaveBeenCalledWith(
+      expect.anything(),
+      400,
+      expect.objectContaining({
+        success: false,
+        error: 'accountId is required',
+      }),
+    );
+  });
+
+  it('falls back to the legacy owner when explicit default owner is stale', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'MainAgent', name: 'Main Agent' }, { id: 'code', name: 'Code Agent' }],
+      channelOwners: {},
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'MissingAgent', match: { channel: 'telegram', accountId: 'default' } },
+        { agentId: 'MainAgent', match: { channel: 'telegram' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      agentId: 'code',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(1, 'MainAgent', 'telegram', 'default');
+    expect(assignChannelAccountToAgentMock).toHaveBeenNthCalledWith(2, 'code', 'telegram', 'telegram-a1b2c3d4');
+  });
+
+  it('skips default binding migration when both explicit and legacy owners are stale', async () => {
+    listConfiguredChannelAccountsMock.mockReturnValue({
+      telegram: {
+        defaultAccountId: 'default',
+        accountIds: ['default', 'telegram-a1b2c3d4'],
+      },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'code', name: 'Code Agent' }],
+      channelOwners: {},
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'MissingDefault', match: { channel: 'telegram', accountId: 'default' } },
+        { agentId: 'MissingLegacy', match: { channel: 'telegram' } },
+      ],
+    });
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      agentId: 'code',
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'PUT' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/binding'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(clearChannelBindingMock).toHaveBeenCalledWith('telegram');
+    expect(assignChannelAccountToAgentMock).toHaveBeenCalledTimes(1);
+    expect(assignChannelAccountToAgentMock).toHaveBeenCalledWith('code', 'telegram', 'telegram-a1b2c3d4');
+  });
+
+  it('converts legacy channel-wide fallback into an explicit default binding when saving a non-default account', async () => {
+    parseJsonBodyMock.mockResolvedValue({
+      channelType: 'telegram',
+      accountId: 'telegram-a1b2c3d4',
+      config: { botToken: 'token', allowedUsers: '123456' },
+    });
+    listAgentsSnapshotMock.mockResolvedValue({
+      agents: [{ id: 'main', name: 'Main' }],
+      channelOwners: { telegram: 'main' },
+      channelAccountOwners: {},
+    });
+    readOpenClawConfigMock.mockResolvedValue({
+      bindings: [
+        { agentId: 'main', match: { channel: 'telegram' } },
+      ],
+    });
+
+    const { handleChannelRoutes } = await import('@electron/api/routes/channels');
+    await handleChannelRoutes(
+      { method: 'POST' } as IncomingMessage,
+      {} as ServerResponse,
+      new URL('http://127.0.0.1:13210/api/channels/config'),
+      {
+        gatewayManager: {
+          rpc: vi.fn(),
+          getStatus: () => ({ state: 'running' }),
+          debouncedReload: vi.fn(),
+          debouncedRestart: vi.fn(),
+        },
+      } as never,
+    );
+
+    expect(saveChannelConfigMock).toHaveBeenCalledWith(
+      'telegram',
+      { botToken: 'token', allowedUsers: '123456' },
+      'telegram-a1b2c3d4',
+    );
+    expect(assignChannelAccountToAgentMock).toHaveBeenCalledWith('main', 'telegram', 'default');
+    expect(clearChannelBindingMock).toHaveBeenCalledWith('telegram');
+    expect(assignChannelAccountToAgentMock).not.toHaveBeenCalledWith('main', 'telegram', 'telegram-a1b2c3d4');
+  });
+
  it('keeps channel connected when one account is healthy and another errors', async () => {
    listConfiguredChannelsMock.mockResolvedValue(['telegram']);
    listConfiguredChannelAccountsMock.mockResolvedValue({