Offboarding has two halves and only the first one is guaranteed. When the second fails, the person
is out of the deployment, the audit trail holds no record that anybody removed them, and removing them
again reports success without retrying anything.
What it looks like
POST /api/admin/people/:userId/access with {"revoked": true} calls peopleStore.revoke, and that
method does two things: a transaction writing the deny-list row and deleting the person's sessions,
then retireOwnedCredentials (server/src/people/store.ts:297). The second reaches the vault and
Composio, and nothing is caught around either — deliberately, so a broker that will not answer cannot
let the method report an ending that did not happen (server/src/plugins/store.ts:6576).
So it throws, and the throw leaves with it every line the route had after the call — including the one
that writes person.access_revoked (server/src/app.ts:776). The transaction is already committed,
so the person is denied and their sessions are gone. The route answers 500.
The administrator does the obvious thing and removes them again. person.revoked is now true, so
if (person.revoked !== revoked) is false, the whole block is skipped, and the route answers 200 with
the person drawn as removed. Nothing retried the retirement. There is no other caller:
retireConnectionsFor is reachable only from revoke (server/src/index.ts:239), so the credentials
and the composio_connections rows stay standing with nothing in the deployment able to reach them.
The two comments that disagree
revoke says a failed retirement leaves "a removal with no retirement beside it — which is the honest
record of what happened, and is recoverable by removing them again". Neither clause holds. The trail
shows nothing at all, because the row is written by the caller after the call that threw, and removing
them again is a no-op for exactly the state the first attempt created.
Reproduction
Against a migrated test database, with a retirer that throws once:
- Sign in as an administrator, and
POST /api/admin/people/<id>/access {"revoked": true}.
- The vault or the broker fails. The route answers 500.
revoked_access holds the row, and the person's sessions rows are gone — they are out.- The audit trail holds no
person.access_revoked for them.
POST the same body again. The route answers 200, the screen draws them as removed, and the
retirer is never called a second time.
Why it matters
This is the removal path, so the trail is the point of it. An auditor asking who removed somebody, and
when, gets nothing — and the screen agrees with them being gone, so nothing signals that the record is
missing. A deployment is left with a person who is out, connections of theirs that are live at
Composio, and no operation that can retire them: restoring access and removing again is the only way
back, and it hands the account back for as long as that takes.
The trigger is ordinary. Composio answering 503, a vault write failing, an OAuth revoke timing out —
any of these is a normal afternoon, and offboarding is precisely when a deployment is least able to
absorb a half-finished one.
What a fix probably has to do
Record the removal as soon as the half that must stick has stuck, rather than after the half that can
fail, and let the retirement run on a person who is already removed so that asking again finishes it.
Both fall out of separating the two halves at the store's surface, which the code already treats as
two things.
Swallowing the retirement's failure instead would be worse: the route would answer 200 over a
retirement that did not happen, which is the outcome the plugin store's comment set out to avoid.
Severity
High for a deployment that relies on the trail, medium otherwise. Nothing grants anybody anything they
did not have, and the person's access to OpenBot itself does end. What fails is the record of it and
the reach of the deployment into what they still hold elsewhere.
Offboarding has two halves and only the first one is guaranteed. When the second fails, the person
is out of the deployment, the audit trail holds no record that anybody removed them, and removing them
again reports success without retrying anything.
What it looks like
POST /api/admin/people/:userId/accesswith{"revoked": true}callspeopleStore.revoke, and thatmethod does two things: a transaction writing the deny-list row and deleting the person's sessions,
then
retireOwnedCredentials(server/src/people/store.ts:297). The second reaches the vault andComposio, and nothing is caught around either — deliberately, so a broker that will not answer cannot
let the method report an ending that did not happen (
server/src/plugins/store.ts:6576).So it throws, and the throw leaves with it every line the route had after the call — including the one
that writes
person.access_revoked(server/src/app.ts:776). The transaction is already committed,so the person is denied and their sessions are gone. The route answers 500.
The administrator does the obvious thing and removes them again.
person.revokedis now true, soif (person.revoked !== revoked)is false, the whole block is skipped, and the route answers 200 withthe person drawn as removed. Nothing retried the retirement. There is no other caller:
retireConnectionsForis reachable only fromrevoke(server/src/index.ts:239), so the credentialsand the
composio_connectionsrows stay standing with nothing in the deployment able to reach them.The two comments that disagree
revokesays a failed retirement leaves "a removal with no retirement beside it — which is the honestrecord of what happened, and is recoverable by removing them again". Neither clause holds. The trail
shows nothing at all, because the row is written by the caller after the call that threw, and removing
them again is a no-op for exactly the state the first attempt created.
Reproduction
Against a migrated test database, with a retirer that throws once:
POST /api/admin/people/<id>/access{"revoked": true}.revoked_accessholds the row, and the person'ssessionsrows are gone — they are out.person.access_revokedfor them.POSTthe same body again. The route answers 200, the screen draws them as removed, and theretirer is never called a second time.
Why it matters
This is the removal path, so the trail is the point of it. An auditor asking who removed somebody, and
when, gets nothing — and the screen agrees with them being gone, so nothing signals that the record is
missing. A deployment is left with a person who is out, connections of theirs that are live at
Composio, and no operation that can retire them: restoring access and removing again is the only way
back, and it hands the account back for as long as that takes.
The trigger is ordinary. Composio answering 503, a vault write failing, an OAuth revoke timing out —
any of these is a normal afternoon, and offboarding is precisely when a deployment is least able to
absorb a half-finished one.
What a fix probably has to do
Record the removal as soon as the half that must stick has stuck, rather than after the half that can
fail, and let the retirement run on a person who is already removed so that asking again finishes it.
Both fall out of separating the two halves at the store's surface, which the code already treats as
two things.
Swallowing the retirement's failure instead would be worse: the route would answer 200 over a
retirement that did not happen, which is the outcome the plugin store's comment set out to avoid.
Severity
High for a deployment that relies on the trail, medium otherwise. Nothing grants anybody anything they
did not have, and the person's access to OpenBot itself does end. What fails is the record of it and
the reach of the deployment into what they still hold elsewhere.