Record what each app answered when one of them refuses an offboarding

[Hotragn]

What this changes

Follow-up to #574, which @zopeVaibhav asked for on that thread after we established the case in review. This is his preferred option of the two I offered: a catch per app that records the answer that app actually got, rethrowing after the loop, so the revoke-before-delete order the docblock argues for is kept.

retireConnectionsFor asked the broker to withdraw every brokered account in one loop, then deleted every row, then wrote every audit row:

for (const connection of brokered) {
  vendorRevocationRequested.set(connection.toolkit, broker ? await broker.revoke(...) : false);
}                                           // ← BrokerRefusalError
await database.delete(composioConnections).where(eq(composioConnections.userId, userId));
for (const connection of brokered) { /* mcp.account_disconnected */ }

ComposioBroker.revoke throws, so one app refusing ended the act before the delete and before the trail. With five apps and a refusal on the fourth, apps one to three were already withdrawn at Composio, kept their composio_connections row, kept their (toolkit, user_id) gate, and left nothing behind saying the account had ended.

Why it matters now rather than before

That was survivable while repeating the act did nothing. #574 made repeating it the documented recovery and its changelog line tells an administrator to take it — so the second pass asks again for those three apps, Composio answers false because the accounts are already gone, and each writes vendorRevocationRequested: false about a withdrawal this deployment asked for and got.

That field exists for exactly that distinction, and its own docblock says so:

a reader has to be able to tell an account this deployment acted on from one that outlives it somewhere else

After a partial refusal, those rows said the wrong one — permanently, with no later act able to correct them.

Credit where it is due: zopeVaibhav corrected my account of the mechanism in review. The false does not come from ours.length === 0 at composio-adapter.ts:3783 — the configs are still this deployment's and still readable. It comes one listing further in, at the terminal return ids.length > 0 (:3931), because the accounts are gone. Configs intact, accounts withdrawn, answer false, which is harder to spot than a missing config would be.

Where it runs

• New state that outlives a request? None. Two locals inside one method.
• What happens on the second replica? The same. Both halves still key on the user id; two administrators offboarding the same person concurrently each ask the broker and each delete only what answered them, and a row already deleted is not found.
• Anything serialised? The audit payload, unchanged in shape — vendorRevocationRequested now carries this app's own answer rather than a map lookup that may not have been reached.
• Anything fanned out to a browser? No.
• New listener, port, or schedule? No.

Boundary and audit

• Every acting call still goes through the gateway. Untouched.
• New refusals and new failures each write a row. This is the change: apps that were withdrawn during a partly-refused act now get their mcp.account_disconnected row, where previously they got none. No new event type; no row is written for an app that was refused, because nothing happened to it.
• Nothing new is trusted from the client.

The refusal is still loud. Holding refusals is only correct if they are still thrown, so the first is rethrown after the loop and the route still answers 500. an offboarding the vendor refuses leaves the connection standing is unchanged and still passes — the second new test pins the inverse risk, that collecting refusals could turn a failed offboarding into a reported one.

The delete is now scoped to the apps that answered rather than to the person. Deleting by user id would take the rows of apps that were refused or never reached, and those are exactly the rows the recovery needs — which is the criterion that existing test already states. What changes is that the rule now applies to the app it is about rather than to every app in the same act.

Changelog

• CHANGELOG.md, under Unreleased.

Proof

Two tests in server/tests/composio-connections.test.ts, on the existing two-app fixture.

Against a migrated openbot_test on pgvector/pgvector:pg17 — the same image and credentials CI uses — with the fix reverted and the tests present:

(fail) an offboarding one app refuses still records the app that answered
(fail) a refusal on the first app still fails the act and still asks the second
 69 pass, 2 fail

On this branch:

bun test server/tests/composio-connections.test.ts     71 pass, 0 fail

Whole server suite, both with the database up:

	pass	fail
main	3236	7
this branch	3238	7

The seven are identical on both and none is in this file — four are tool-selection fixture model environment restoration, plus production server loader boundary, blank package endpoint…, and one 5s timeout. +2 is exactly the two tests added.

cd server && bunx tsc --noEmit is clean. Biome check on the three changed files: one formatting fix, applied.

What is not covered

• A refusal from recordAuditEvent itself. The order is still delete-then-record, so if the audit write fails after a successful delete, that app's row is gone and no trail row exists, and the retry finds nothing to redo. That is unchanged from before this PR and is really a question about what to do when the trail is the thing that is unavailable — worth its own issue, not a clause here.
• Only the first refusal is rethrown. The others are lost as exceptions. The route turns this into a 500 and one sentence is what reaches the administrator, and the trail now says which apps did not end by their absence — but if a reader would rather see an aggregate error naming all of them, that is a reasonable ask and a small change.

🤖 Generated with Claude Code

[davidmckayv]

Code-verified clean; CI green on this sha.