From b02834893f2a1e7a2580b35de5b5691f6ab7125b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 16:52:57 -0400 Subject: [PATCH] Q103: connector changes write the local result first and delete remotely last Co-Authored-By: Claude Opus 5.5 --- src/main/app/ipc-handlers.ts | 13 ++++++---- .../composio/composio-connection-service.ts | 23 ++++++++++------- tests/unit/composio-reconnect.test.ts | 25 +++++++++++++++++++ 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/main/app/ipc-handlers.ts b/src/main/app/ipc-handlers.ts index 37e5c03f..34e75050 100644 --- a/src/main/app/ipc-handlers.ts +++ b/src/main/app/ipc-handlers.ts @@ -397,29 +397,32 @@ export function registerIpcHandlers(deps: IpcDeps): void { }); ipcMain.handle(IPC_CHANNELS.connectComposioToolkit, async (_event, input) => { try { - const result = await composioConnections.connectToolkit(input.orgId, input.toolkitSlug, input.label); - notifyDataChanged(); - return ok(result); + return ok(await composioConnections.connectToolkit(input.orgId, input.toolkitSlug, input.label)); } catch (error) { return fail("composio_connect_failed", error instanceof Error ? error.message : String(error)); + } finally { + // A connection change that failed partway may still have written; the page reloads either way. + notifyDataChanged(); } }); ipcMain.handle(IPC_CHANNELS.disconnectComposioToolkit, async (_event, input) => { try { await composioConnections.disconnectToolkit(input.orgId, input.toolkitSlug, input.label); - notifyDataChanged(); return ok(snapshot.load()); } catch (error) { return fail("composio_disconnect_failed", error instanceof Error ? error.message : String(error)); + } finally { + notifyDataChanged(); } }); ipcMain.handle(IPC_CHANNELS.refreshComposioAccounts, async (_event, input) => { try { await composioConnections.refreshAccounts(input.orgId); - notifyDataChanged(); return ok(snapshot.load()); } catch (error) { return fail("composio_refresh_failed", error instanceof Error ? error.message : String(error)); + } finally { + notifyDataChanged(); } }); ipcMain.handle(IPC_CHANNELS.windowMinimize, () => deps.mainWindow.minimize()); diff --git a/src/main/services/composio/composio-connection-service.ts b/src/main/services/composio/composio-connection-service.ts index cf9c134e..ecef223d 100644 --- a/src/main/services/composio/composio-connection-service.ts +++ b/src/main/services/composio/composio-connection-service.ts @@ -1,4 +1,5 @@ import type { DatabaseSync } from "node:sqlite"; +import { withSavepoint } from "../../database/connection"; import type { ConnectorCatalogEntry, ConnectComposioToolkitResult } from "../../../shared/commands"; import { ComposioClient, type ComposioConnectedAccount } from "./composio-client"; import { IntegrationKeyStore } from "../integration-key-store"; @@ -139,17 +140,18 @@ export class ComposioConnectionService { oldIsLive: boolean, ): Promise { const now = new Date().toISOString(); + // The row is settled before the remote delete: an account that cannot be deleted is left as an orphan refresh ignores. if (pending && (pending.status === "ACTIVE" || !oldIsLive)) { - if (oldIsLive) await client.deleteConnectedAccount(row.id); this.db.prepare("update connector_accounts set id = ?, auth_config_id = ?, status = ?, pending_account_id = null, updated_at = ? where id = ?") .run(pending.id, pending.authConfigId, pending.status, now, row.id); console.info(`[connectors] ${row.toolkit_slug} reconnect ${pending.id} replaced ${row.id}`); + if (oldIsLive) await client.deleteConnectedAccount(row.id); return; } if (pending && WAITING_STATUSES.has(pending.status)) return; - if (pending) await client.deleteConnectedAccount(pending.id); this.db.prepare("update connector_accounts set pending_account_id = null, updated_at = ? where id = ?").run(now, row.id); console.warn(`[connectors] ${row.toolkit_slug} reconnect ${row.pending_account_id} ended ${pending?.status ?? "missing"}; ${row.id} stays in use`); + if (pending) await client.deleteConnectedAccount(pending.id); } /** Deletes one account of a toolkit in Composio, with untracked leftovers of that toolkit, then its row and the grants that named it. */ @@ -167,11 +169,12 @@ export class ComposioConnectionService { const client = this.requireClient(); // Untracked accounts of the toolkit go too, so an orphan from a failed delete cannot be adopted later. const tracked = this.trackedIds(orgId); - const ids = new Set([row.id, ...(row.pending_account_id ? [row.pending_account_id] : [])]); - for (const account of await client.listConnectedAccounts(getComposioUserId(this.db, orgId))) { - if (account.toolkitSlug === slug && !tracked.has(account.id)) ids.add(account.id); - } - for (const id of ids) await client.deleteConnectedAccount(id); + const leftovers = (await client.listConnectedAccounts(getComposioUserId(this.db, orgId))) + .filter((account) => account.toolkitSlug === slug && !tracked.has(account.id)) + .map((account) => account.id); + for (const id of [...leftovers, ...(row.pending_account_id ? [row.pending_account_id] : [])]) await client.deleteConnectedAccount(id); + // The working account goes last and its row right after, so a failure earlier leaves it both working and shown. + await client.deleteConnectedAccount(row.id); this.removeLocalAccount(orgId, slug, label); } @@ -183,8 +186,10 @@ export class ComposioConnectionService { } private removeLocalAccount(orgId: string, slug: string, label: string): void { - this.db.prepare("delete from connector_accounts where org_id = ? and toolkit_slug = ? and label = ?").run(orgId, slug, label); - this.db.prepare("delete from agent_connector_grants where org_id = ? and toolkit_slug = ? and account_label = ?").run(orgId, slug, label); + withSavepoint(this.db, "remove_connector_account", () => { + this.db.prepare("delete from connector_accounts where org_id = ? and toolkit_slug = ? and label = ?").run(orgId, slug, label); + this.db.prepare("delete from agent_connector_grants where org_id = ? and toolkit_slug = ? and account_label = ?").run(orgId, slug, label); + }); } private requireClient(): ComposioClient { diff --git a/tests/unit/composio-reconnect.test.ts b/tests/unit/composio-reconnect.test.ts index d00e2817..f70e75c3 100644 --- a/tests/unit/composio-reconnect.test.ts +++ b/tests/unit/composio-reconnect.test.ts @@ -138,6 +138,31 @@ describe("reconnecting a connector keeps the working account until the new one i h.connection.close(); }); + test("a disconnect that fails partway leaves the account both working and shown", async () => { + const h = await connectedCompany(); + remote.set("ca_leftover", { id: "ca_leftover", toolkitSlug: "github", status: "INITIATED", authConfigId: "ac_gh", createdAt: new Date().toISOString() }); + failDelete.add("ca_leftover"); + await expect(h.service.disconnectToolkit(h.orgId, "github", "main")).rejects.toThrow("Composio could not delete ca_leftover"); + expect(remote.has(h.oldId)).toBe(true); + expect(h.account()).toEqual({ id: h.oldId, status: "ACTIVE", pending: null }); + expect(h.grants()).toBe(1); + h.connection.close(); + }); + + test("a reconnect whose old account cannot be deleted still moves the row and its grants to the new account", async () => { + const h = await connectedCompany(); + const again = await h.service.connectToolkit(h.orgId, "github", "main"); + remote.get(again.accountId)!.status = "ACTIVE"; + failDelete.add(h.oldId); + await expect(h.service.refreshAccounts(h.orgId)).rejects.toThrow(`Composio could not delete ${h.oldId}`); + expect(h.account()).toEqual({ id: again.accountId, status: "ACTIVE", pending: null }); + expect(h.grants()).toBe(1); + await h.service.refreshAccounts(h.orgId); + expect(h.account()).toEqual({ id: again.accountId, status: "ACTIVE", pending: null }); + expect(h.grants()).toBe(1); + h.connection.close(); + }); + test("a reconnect that cannot start touches nothing", async () => { const h = await connectedCompany(); failCreate = true;