feat(connectors): let team members edit a team-shared MCP connector

[AlexLiu190625]

What

A connector's shared configuration can be edited only by the user whose user_mcpservers row carries is_owner. An application that installs the connector team hooks can now supply a second permission source, so an MCP server a team owns becomes editable by the members that application recognises.

This is the MCP half. GET and PUT /api/mcp/servers/{id} widen their first gate, the edit predicate gains a second source, and every place that reports can_edit_global for an MCP server draws on the same verdict the gate used.

Standalone deployments are unaffected: with no hook installed every route in this diff behaves exactly as it does today, issues no additional query, and that equivalence is asserted rather than assumed.

Why

The seam that answers the question already exists on main: resolve_connector_access asks an installed application whether the caller's team links a given connector and whether it may edit it. Nothing on the MCP routes consults it yet, so the answer has no effect: a team that owns a server still sees every member except the one owner refused.

Three things follow from consulting it, and each is a separate hazard the code has to handle.

A caller with no personal row must be able to reach the server at all. Both routes read the definition row through a two-table join with the caller's own user_mcpservers row and answer 404 when there is none. A team member who never connected the server personally has none. The routes now fall back to a bare lookup of the definition row plus the caller's team verdict, and use the existing _TeamOwnedUserMCP stand-in in place of the missing association — the same stand-in the listing already constructs for this case.

A platform catalog app's shared row must not become editable. xagent provisions one shared MCPServer row per catalog app (Gmail, Slack, Stripe and the rest) and attaches every user who connects that app to that same row; a key-based app's row may additionally hold the administrator's platform fallback key in its env. That configuration is the platform's, not any one team's. An installing application may still answer can_edit=True for such a ref — the seam does not police what a hook says — so this module downgrades that answer against the fact that the row is the catalog's own.

A verdict resolved before a lock wait can be stale by the time the write happens. PUT takes a FOR UPDATE on the definition row, and that wait has no bound. The application that answers the verdict can revoke the team's link at any moment by writing its own tables, which this lock does not cover.

How

The gate

_resolve_mcp_server_for_request is the one place GET and PUT resolve three things together: the caller's association (real row or stand-in), the definition row, and the caller's team verdict. It looks up the personal link row with the same join both routes have always run, and only when that finds nothing does it look the definition row up on its own and consult the seam. A caller with neither a personal row nor a verdict still gets the same 404 every caller without an association has always got.

An owner's personal row already decides the edit answer on its own, so no hook call is made for an owner.

_check_mcp_permission's edit branch gains the verdict as a second source: the caller may edit when their own row says is_owner, when they are a platform administrator, or when the verdict grants it. The delete branch is untouched and reads no verdict — this change grants no delete right to anyone.

The helper takes an on_resolution_failure argument because only the caller knows what a hook failure means for it. PUT raises: the verdict is the gate there, and the seam's typed 503 is translated into the route's own answer. GET degrades to can_edit_global=False with a logged warning — but only for a caller who already holds a personal row, because for a caller with no personal row the verdict is the gate, and degrading it would answer "does not exist" for a server the call merely failed to ask about.

The catalog downgrade

_team_access_for_shared_row holds the rule, once. Every place a verdict is produced passes through it: the GET/PUT resolution, both loops of the /servers listing, the connect response, the post-lock re-check, and the toggle response. That is two gates and four reported fields. Gating alone would leave a Configure button lit whose save returns 403.

Only can_edit is cleared. team_owned stays as the application answered it, so the connector remains reachable and readable by the team; dropping the verdict entirely would 404 a connector the team genuinely links.

The predicate is _server_catalog_keys against the catalog's own keys — the one the apps listing already uses, so this module holds a single definition of "catalog-managed". Two neighbours were measured and rejected. _is_reserved_catalog_name answers "may a new row take this name" and reads the name alone, so it no longer recognises a builtin-OAuth catalog row an administrator renamed, even though that row still carries its app id in auth — and builtin OAuth is the majority of the built-in catalog apps. _catalog_server_has_platform_key answers "catalog row that also holds the platform key", which is False for every keyless and mcp_oauth row and for every key-based row whose key each user supplies, all of which are still platform-owned configuration.

The check runs only for a verdict that already grants edit, the one case where it can change an answer. A deployment with no access hook installed issues no additional query at all. A listing whose verdicts do grant edit builds the catalog key set once per request and shares it across every row: one extra SELECT, not one per row. That is asserted directly rather than argued: the statement count of a listing whose verdicts grant edit is exactly one more than the same listing with no hook installed, and both counts come out identical for the two population sizes the tests parametrise over, so a per-row cost would show up as a difference between them rather than as an absolute number nobody can read. The two aggregate listings additionally pin their absolute statement counts — 8 for /api/mcp/servers and 10 for /api/mcp/apps — measured by recording the statements that exact population issues.

One boundary is resolved deliberately rather than correctly, and the function says so: a server someone built themselves under a name a catalog app later took. The catalog claims that name, so the row is treated as catalog-managed and the team edit is withheld. Its creator keeps their own edit right in full, because is_owner decides the edit branch before any verdict is read; what is withheld is only a teammate editing it on the owner's behalf. Telling such a row apart from a real catalog row needs a stored "who created this definition" fact the schema does not carry.

Refusing rather than silently ignoring

A caller with no personal row has nowhere to store user_env or is_active: writing them would set a shadowing instance attribute on the stand-in that persists nothing, and the response would then read that shadow back and report a change that never happened. Such a payload now returns 400 naming the reason, on both sides of the definition-row lock — a personal row present at the gate can be gone by the time the lock is held, and the second copy of the guard catches that case on the same fresh read the write path already takes.

A stand-in whose verdict denies edit gets 403 for any other payload, rather than a 200 that wrote nothing: its writable field set is empty — the personal fields have no row to land on, the tamper check refuses every shared field it can compare, and the secrets it deliberately cannot compare are emptied out of the payload. The 403 is ordered after the 400, so a personal-only payload still gets the more precise answer.

Re-establishing authority after the lock

The route already re-reads the caller's link row and the caller's admin flag once it holds the definition row, and recomputes can_edit_global from them. The team verdict is the third input to that same decision, so it is re-resolved in the same place, from the same locked row, and feeds the same recomputation.

The caller's own freshly-read personal row is repointed in both directions rather than 404ing on one that is merely gone: a caller who reached the gate through their team's verdict alone may have acquired a personal row during the wait, and a caller who reached it through a personal row may have had that row deleted. Neither direction is refused by itself. Whether a caller with no personal row may still write is the re-resolved verdict's answer, asked next; refusing first would 404 a team editor whose team does authorise the edit. What must not survive this point is the gate's own ORM object — continuing to hold a row another session has deleted raises a stale-data error at commit for a payload that writes it, and a deleted-object error while building the response for one that does not.

The verdict is re-asked whenever the caller's freshly-read personal row does not already grant edit on its own — not, as an earlier version of this route had it, only when the pre-lock verdict had already granted edit. The narrower condition let two populations through unasked: a caller whose personal row granted edit at the gate (so no verdict was ever resolved there) and lost it during the wait, and a caller whose gate verdict granted nothing but whose personal row changed underneath them. Both got the wrong answer without a re-ask. The wider condition costs one extra hook call on a population that was previously free: a caller with a non-owning personal row whose verdict denies edit, on a payload that does write the shared definition row, now pays two hook calls instead of one — the recheck runs and confirms the same denial, changing nothing about the final answer. The full table, for a payload that writes the definition row:

Population	Hook calls
Owner	0
Platform admin (no personal row)	1
Non-owner personal row, verdict grants edit	2
Non-owner personal row, verdict denies edit	2
No personal row (stand-in), verdict grants edit	2

A caller whose team access no longer grants edit is refused with 403 after an explicit rollback, before any field is read or mutated and before the rename hook runs, so the refusal has nothing to undo — zero side effects is structural here, not something a rollback has to achieve.

There is no separate "personal-only payload" exemption: the block already runs only when the payload writes the definition row, and the field set that decides that (is_active, user_env excluded) is exactly the set of fields a personal-only payload may carry. A payload that writes only the caller's own link row takes no lock, so there is no wait for a revocation to slip into.

This narrows the window; it is not a fence. A fence needs the revoke path to take the same lock, and that path lives in the application that installs the hook, not here. The re-read is also only meaningful under READ COMMITTED, PostgreSQL's default, which nothing here overrides on the engine; under REPEATABLE READ or SERIALIZABLE the re-read reuses the transaction's original snapshot and the re-check degrades to a no-op — it stops refusing rather than starting to refuse wrongly. The code says so at the site.

Where the re-check runs, the 200 reports the verdict it produced, because that is the answer the write was authorised on.

The session boundary contract

The session contract a hook must honour, and the check that a locked call site declares against a hook that ends the caller's own transaction mid-request, are stated and enforced in connector_team_scope's own module docstring (added in #2134) and are not this change's to restate — they are shared with #2094 and with the five call sites that already existed on main before either PR. This change's own job against that contract is narrow: register its six call sites in the module docstring's table, one of them (the post-lock re-check above) declaring that it holds the definition row locked while the hook runs.

One rename falls out of that registration rather than being made for its own sake. The row this table already carried for the app-teardown call site was keyed on teardown_mcp_app_server, the coroutine route; the hook call has always lived one level down, in the plain function that coroutine dispatches to on a worker thread. The table key follows the call site, so it now names that function directly. This is not a behaviour change — the lock the row describes and the fact that nothing is committed before the hook runs are exactly as they were — only the row's own name catching up to where the call already was.

Keeping hook calls off the event loop

An installed hook is slow synchronous work: the seam is designed on the assumption that the installing application answers from its own tables. FastAPI runs a coroutine route on the event loop thread itself, so a slow hook call inside an async def delays every request the process is serving, not just its own; a plain def goes to the threadpool, where it occupies one worker.

toggle_mcp_server becomes a plain def for that reason. The invariant is asserted rather than left to per-route judgement: the functions in this module that can reach a hook are discovered by transitive reachability from the module's own imports, the discovered set is asserted equal to a named literal so the check cannot pass by finding nothing, and every member is asserted to be a plain def. One member is exempt and named rather than left implicit — delete_mcp_server awaits an external OAuth revocation call and cannot be converted without changing how that call is made. The exemption must itself carry an await that is not the seam call, so a route whose only await is the seam call cannot claim it.

Known limitations tracked separately

These are known and deliberately not addressed here. Each is either repo-wide work or a contract question wider than these routes.

• The seam's typed 503 does not reach the client on these routes (The connector seam's typed 503 never reaches a client on the connector management routes #2055). Where a hook failure is translated into an HTTP answer, the outer handler's shape means the client sees a generic error rather than the seam's own message. That mapping is the same on every connector management route and is being settled in one place.
• The session-boundary violation on the post-lock re-check surfaces as this route's own generic 500, not the application-level handler's message. update_mcp_server's outer except Exception has no arm for the seam's own session-boundary error, so a hook that ends the caller's transaction mid-recheck is reported as "Failed to update MCP server: ..." rather than through the shared handler the other five declared call sites reach. The message itself carries neither a hook name nor a slot name either way, so nothing is disclosed; the shape of the answer is simply not uniform across call sites yet, and unifying it means touching this route's broad exception handler, which this change leaves alone.
• A failing hook blocks a PUT that only touches the caller's own fields (A failing connector access hook blocks a PUT that only changes the caller's own association fields #1775). The gate resolves a verdict before it knows the payload writes nothing shared, so a hook outage refuses a request the verdict has no say over.
• The verdict is resolved twice on the write path (Refactor update_mcp_server: resolve the team verdict once, after the lock #1815), and the wider re-ask condition above raises one population's cost from one call to two. Once at the gate and once under the lock. Resolving it once, after the lock, would remove a hook round trip; doing so means restructuring the gate, which this change keeps intact.
• managed is rewritten to external on every PUT (PUT /api/mcp/servers/{id} rewrites managed to "external" on every update #1817). Pre-existing, in code this change does not touch.
• delete_mcp_server reaches the seam and stays a coroutine (delete_mcp_server calls the connector team seam synchronously on the event loop #1818). It awaits an external revocation call; converting it means changing how that call is made.
• list_mcp_apps treats its two hook failures differently (list_mcp_apps treats its two hook failures differently #1879), and its visibility-hook failure is unguarded where get_mcp_servers's is guarded (The connector visibility hook's failure is guarded in get_mcp_servers and unguarded in list_mcp_apps #1918). Both are about the listing's existing failure handling, not about the verdict this change adds.
• The visibility hook's answer is never shape-checked (The connector visibility hook's answer is never shape-checked #1878). It is the one hook answer with no validator, so a malformed one surfaces as a raw KeyError rather than the seam's typed error.
• The seam expresses "consult or skip" and "raise or degrade" five ways (The connector seam expresses "consult or skip" and "raise or degrade" five ways #1880). This change adds a sixth call site rather than unifying them; the unification is its own change.
• The hook contract does not say what lock the caller holds (The connector team hook contract does not say what lock the caller holds while a hook runs #1932), and definition-row lock waits are unbounded and held across hooks (fix(connectors): definition-row locks have no bounded wait and are held across application hooks #1946). Both are properties of the lock this change builds on, not of the verdict it adds.
• with_for_update is a no-op on SQLite (fix(db): with_for_update takes no lock on SQLite, so read-modify-write routes are unserialized on the default backend #1944) and the PostgreSQL isolation level is neither checked nor documented (fix(db): the PostgreSQL isolation level the server hands us is neither checked nor documented, so REPEATABLE READ turns concurrent edits into 500s #1945). Inherited from the lock; the re-check's dependence on READ COMMITTED is written into the code.
• Test-quality gaps in the connector team access suites (Four test-quality gaps in the connector team access suites #1816). Tracked together rather than fixed piecemeal.
• Two unlocked read-modify-write sites on this route (fix(mcp): the per-user env merge on PUT /api/mcp/servers/{id} is an unlocked read-modify-write #2087, fix(mcp): editing any field resets an external MCP server's restart_policy and auto_start #2088). The per-user env merge, and restart_policy/auto_start being reset when any field is edited. Both pre-date this change.
• Whose capability a list field describes when an administrator views another user's connectors (Whose capability do list fields describe when an admin views another user's connectors? #1703). A question about the listing's contract that this change inherits rather than creates.
• Cleartext header values with a masking comment that claims otherwise (Connector headers values round-trip in cleartext while a masking comment claims otherwise #1701). Unrelated to the verdict, carried from an earlier review.
• load_team_mcp_env never restores the shared session after a failing hook (load_team_mcp_env never restores the shared session after a failing hook #1837). The same seam shape as the door this change calls through, on a path outside these routes.
• Disconnecting the last user of a shared MCP server deletes the definition row out from under a concurrent connect (Disconnecting the last user of a shared MCP server deletes the definition row out from under a concurrent connect #1917). A defect in the disconnect path, found while working on these routes and not touched by them.
• _TeamOwnedUserMCP.is_active/is_default are fixed placeholders, not read off any row. A caller whose personal row is deleted during the lock wait, whose re-resolved verdict still authorises the edit, gets a response reporting is_active=True/is_default=False regardless of what their connection looked like before it was removed — there is no real row left to read either value from. Documented on the stand-in class rather than changed: the alternative is a schema-level "what was this connector's activation state" fact this change does not add.

Relationship to #1661

This is the MCP half of #1661, which covered both connector families and both the seam and the locks in one change. Three parts of it have already merged separately: the access seam itself (#1912), the Custom API definition-row lock (#2059) and the MCP definition-row lock (#2060). This change is built on main with all three in place, not on that branch.

The Custom API half — the same widening for GET/PUT /api/custom-apis/{id} — is #2094. The two touch disjoint production files: this one changes api/mcp.py only, that one changes api/custom_api.py only. Neither depends on the other's code.

They are not fully independent in behaviour, and the honest statement of it is this. The aggregate listings report a can_edit_global for Custom API rows as well as MCP rows, and list_mcp_apps also decides each row's can_configure from the same verdict. This change is what makes both fields reflect a team verdict, for both connector families.

Until the Custom API half lands, a team member granted edit on a Custom API sees a lit Configure affordance for it, and both GET and PUT /api/custom-apis/{id} answer 404 for that caller — not a refusal they can read as one, but a connector that appears to vanish when they open it. Nothing unauthorised is read or written; the cost is a control that does not work.

Merging the Custom API half first removes that window entirely — with it in place, the fields this change starts reporting are already true. The reverse order is never wrong in the same way: a route that permits more than the client advertises is the safe direction.

Three comments in this change describe the Custom API routes as they behave on main today, and each is hedged as such: two in api/mcp.py (on _custom_api_to_mcp_response and on the listing's verdict lookup) and the module docstring of tests/web/api/test_mcp_reported_edit_permission.py. They stop being true the moment the Custom API half merges. Whichever of the two lands second should update them in that same change.

One piece of coverage is owed to whichever lands second and is filed rather than carried: an assertion that the value the aggregate listing reports for a Custom API row equals what PUT /api/custom-apis/{id} actually does. That comparison needs both sides present, so it cannot be written on either branch on its own. The suite here says plainly, in its module docstring, that it makes no claim about the Custom API routes' write outcome.

Tests

tests/web/api/test_mcp_team_connector_edit.py (new) covers the gate itself: the resolution helper on GET and PUT, the permission predicate's team fallback, the stand-in's personal-field 400 and denying-verdict 403, the catalog downgrade across four catalog row shapes, the verdict's re-validation under the definition lock and the hook-call cost across every population that reaches it, the typed-error arm, a hook that ends the caller's transaction during the post-lock re-check, the owner's immunity to a hook failure, and the decoration that degrades after a write has committed. It also carries the coroutine guard described above.

tests/web/api/test_mcp_reported_edit_permission.py (new) covers what the routes report: can_edit_global asserted equal across the list, GET, PUT and the toggle for the same caller and connector, parametrised over every constructible caller population; the hook-call budget for both listings, asserted identical for two population sizes so it is a per-request cost and not a per-row one; per-row degradation, where an answer that omits one connector degrades only that row and a failing hook does not blank a listing; session recovery after a hook poisons the shared session, for both poisoning shapes; the four OAuth routes keeping their own personal-row-only gate; rename staying scoped to its own connector; and the standalone shape with no hook installed, asserted row by row against pre-change behaviour.

tests/web/api/test_mcp_server_edit_lock_postgresql.py additionally covers the post-lock cascade against a real row lock and a real concurrent writer rather than an in-process hook sequence: an owning personal row deleted or downgraded during the wait, with the re-resolved verdict authorising the edit; the same deletion with the re-resolved verdict also denying, landing on the gate's own 404; the lock-side personal-field guard refusing a payload that mixes a shared field with is_active after the caller's personal row is gone; and the response building correctly off the stand-in rather than a stale, deleted ORM object.

tests/web/api/test_connector_hook_session_fault_postgresql.py (new) proves the session restore against a real server. A failed raw statement aborts the surrounding transaction on PostgreSQL and every later statement on that connection is refused until a rollback runs; SQLite does not enforce that, so the SQLite suite cannot tell a working restore from a missing one. The file is registered in the existing Postgres job in test-migrations.yml, in both hand-maintained path lists and as its own step.

Run locally:

Suite	Result
test_mcp_team_connector_edit.py	54 passed
test_mcp_reported_edit_permission.py	47 passed
test_connector_team_scope.py	129 passed
test_mcp_server_edit_lock_postgresql.py + test_connector_hook_session_fault_postgresql.py + test_mcp_oauth_lifecycle_postgresql.py against PostgreSQL 17	40 passed
tests/web/api (whole directory)	2916 passed, 68 skipped

The two failures seen on an earlier run of the whole tests/web suite, in tests/web/test_health_degradations.py, pre-date this change: they pass in isolation and are reproduced by running tests/web/services ahead of them, a directory this change does not touch. Earlier tests in the same process leave two degradation flags set on a module-level global that the health check then reports.

[gemini-code-assist]

Code Review

This pull request implements team-based access control and permission fallbacks for MCP servers and Custom APIs, allowing team members to configure and view shared connectors. Key changes include batch resolution of team access verdicts to optimize performance, re-validation of permissions under database locks to prevent race conditions, and converting the toggle endpoint to a synchronous function to avoid blocking the event loop. The review feedback recommends preserving debug tracebacks by logging exceptions with exc_info=True and using f-strings for consistency across several warning logs, as well as improving type safety by casting user_mcp to UserMCPServer instead of Any.

[AlexLiu190625]

/gemini review

[gemini-code-assist]

Code Review

This pull request implements team-level connector access control for MCP servers and Custom APIs, updating several endpoints (listing, getting, connecting, updating, and toggling) to fall back to team access when personal association rows are missing. It also adds extensive test coverage for session recovery and permission consistency. The review feedback recommends that when catching ConnectorRuntimeError and degrading responses, the original exception details should be logged with exc_info=True to preserve debug traces, and f-strings should be used instead of lazy %-formatting to maintain codebase consistency.

[AlexLiu190625]

Rebuilt on top of current main, which now carries #2105.

That PR added teardown_mcp_app_server, an async def that calls
delete_team_connector on the connector team seam and takes three
with_for_update() row locks. This PR adds an invariant that no function
reaching that seam may be a coroutine: an installed team hook answers from the
installing application's own tables, so it can be slow, and on the event loop
thread a slow hook stalls every other request the process is serving. The
invariant reported the new function on the merge tree, correctly, so it is
fixed here rather than exempted.

The locked local transaction -- identity revalidation, the seam call, the row
locks, the single commit -- is now a plain def
(_teardown_mcp_app_server_locally) that the coroutine runs through
asyncio.to_thread. The coroutine keeps only the post-commit provider
revocation, which is the one step that genuinely needs to await, and which must
still run after the commit has released every lock. The signature, the return
value and the order of work are unchanged, so all existing call sites still
await it unchanged.

The session crosses into that worker thread and back. That is the pattern the
login route in web/api/auth.py already uses -- an async route handing a
Depends(get_db) session to asyncio.to_thread, including the commit -- and
the await means only one thread ever uses the session at a time.

Verified with the tests #2105 shipped, unchanged: test_mcp_oauth_flow.py and
test_mcp_oauth_lifecycle_postgresql.py (the latter against a real
PostgreSQL, where its pg_blocking_pids lock evidence still holds, since the
session keeps the same backend connection across the thread hand-off).

@OliverBryant flagging this since it touches your teardown primitive.

[AlexLiu190625]

The hook session contract and the boundary check are now carried by a separate PR, #2134, opened from main so that this PR and #2094 share one seam instead of each carrying half of it.

What lives there:

• the contract text in connector_team_scope (what a hook may and may not do with the caller's session, and what each declared call site holds when its hook runs);
• the root-transaction-end counter and the refusal path in _call_connector_hook_gate, opt-in per call site through caller_holds_lock=True; the three MCP call sites on main (rename, app teardown, delete) are declared there;
• the application-level handler that answers one fixed 500 for that refusal.

What stays here: the post-lock re-authorization gaps on the MCP edit route, this route's own caller_holds_lock=True declaration at the post-lock access call, and their tests. One note for the rebase: #2134 registers teardown_mcp_app_server by name in the contract table; this branch renames that function, so the static parity test will point at the new name and the table entry needs the same rename.

Merge order: #2134 first, then #2094, then this PR. I will rebase this branch once #2134 lands and update the description accordingly.

[AlexLiu190625]

Merged main (f69c4ec69) into this branch to clear the conflict in src/xagent/web/api/mcp.py against the owner-scoped OAuth APIs from #2111. The conflict was confined to the import block — this branch had expanded the typing import and added TYPE_CHECKING, while main added from enum import Enum — and both sides are kept; the rest of the file merged without semantic overlap, since the two changesets touch disjoint regions and neither redefines the other's helpers.

Verified after the merge: the targeted MCP OAuth, connector team scope, and team-edit suites pass (316), the full tests/web/api/ suite passes (2910 passed, 62 skipped — all PostgreSQL-gated), the PostgreSQL-marked lifecycle and edit-lock tests pass against a real PostgreSQL 17 instance (34), and pre-commit is clean over the merge range including mypy.

Head is now 91f4cd10f.

[AlexLiu190625]

Head moved from 91f4cd10f to 986da2f83, three commits:

• f6b97d58f merges main at b2b0dc028, which brings in fix(connectors): state the hook session contract and refuse a hook that ends the caller transaction #2134 (the hook session contract and the boundary check). The only conflict was in .github/workflows/test-migrations.yml, where both sides had added a PostgreSQL-only step and its path filter; both are kept.
• a8190d64d does the post-lock re-authorization this branch still owed once the seam contract landed: the team decision is re-asked whenever the freshly read personal row does not grant the edit on its own, the post-lock 404 branch is replaced by a fresh stand-in object plus a post-lock guard for the personal-only fields, and that re-ask lives in a small helper _recheck_team_access_under_definition_lock declared with caller_holds_lock=True. The contract table in connector_team_scope gains the rows this branch's call sites owe it, and the teardown row now names _teardown_mcp_app_server_locally, the function that holds the three locks on this branch.
• 986da2f83 adds the tests: one SQLite test pinning that a hook ending the transaction inside the post-lock re-ask is refused, six PostgreSQL tests for the interleavings where the personal row is deleted or downgraded while the request waits for the lock, and one more call-count cell.

Verified on this head: the targeted MCP suites (230), the full tests/web/api/ directory (2916 passed, 68 PostgreSQL-gated skips), the three PostgreSQL-marked files against a real PostgreSQL 17 (40), and pre-commit including mypy. The description is updated to match.