Defer irreversible cleanup until the removal commits

[GeiserX]

TL;DR

Deleting a client, or a plugin cleaning up after a removed connection, can reach outside the database — the credential provider, the OAuth provider's API, a remote object. None of that enlists in the transaction doing the removal, and none of it rolls back with it.

So when the transaction aborts, the database is restored and the outside world is already changed. oauth.removeClient had exactly that shape: the client row came back and its secret stayed destroyed — a client that looks configured and can never authenticate again.

This defers such work until the removal is durable, and gives plugins the same tool, because they were exposed to the identical trap with no way out of it.

---

The defect

removeClient deletes the oauth_client row, then deletes the client secret from the credential provider. It opens no transaction of its own, but a caller can wrap it in one. If that transaction aborts afterwards, the row deletion is undone and the secret deletion is not.

Orphaning a secret is recoverable — it is an unreferenced item. Deleting one that is still referenced is not. The deletion now waits until the removal is durable and is discarded if the removal rolls back. With no transaction active it runs immediately, exactly as today.

The same trap, reachable by plugins

removeConnection and removeIntegration run inside core's removal transaction. That is deliberate and right for database work — a plugin's own rows should die atomically with the connection. It makes them exactly the wrong place for anything else, and removeConnection is precisely where a plugin would revoke the token at the provider's API.

Two things made this a trap rather than a choice:

• PluginCtx exposed transaction but nothing to defer past it. An author who saw the problem had no construct to fix it.
• The hooks' documentation said nothing. removeConnection's entire contract was "Plugin-side cleanup when a connection is removed."

PluginCtx now has afterCommit: runs the effect once the outermost transaction commits, discards it if that transaction rolls back, runs it immediately when none is active. Both hooks now document that they run inside core's transaction and that outside-world work belongs in afterCommit.

The part that is easy to get wrong

Sequencing work after your own transaction(...) call is not the same thing, and the documentation says so explicitly. transaction nests by pass-through — inside an active transaction the inner call just runs its effect — so "after the inner transaction" is still before any commit.

I mention it because I wrote that version of the fix first and it passed a typecheck, a lint and the existing suite. Only a test that drove the nested case caught it.

Tests

Both directions of the contract, deliberately:

• an ordinary removal still performs the cleanup — a deferral that silently never ran would look identical to a correct one in any test that only checked rollback;
• a rolled-back removal does not.

Each is mutation-checked, and each mutation kills exactly one of them. Running afterCommit inline kills the rollback test and correctly leaves the durable-path test passing; a mutation that killed both would have meant the tests were not distinguishing what they claim.

Scope

Independent of #1564. It shares a theme with #1568 — which fixes the same class for connections.remove — but touches different call sites, so the two can merge in either order. Both do modify executor.ts, in different functions, so expect a textual conflict rather than a logical one.

[GeiserX]

Context for this one: #1585 explains why this PR and the others in the series exist — they came out of a single pass over credential handling, asking for each credential where it ends up, how long it stays, and who can read it once it's there.

This PR stands alone and doesn't depend on any of the others.