Q103: connector changes write the local result first and delete remotely last
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0ebe0febdb
commit
b02834893f
3 files changed
+47
-14
No files matched your search
@@ -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());
|
||||
|
||||
@@ -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<void> {
|
||||
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 {
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in new issue
Block a user