[VibeSWE]

Summary

High (pre-existing): revoking roles from a user never took effect on ingestors.

• The querier commits the removal to storage, then syncs it.
• The ingestor's role/sync/remove receiver re-read storage, where the roles were already gone. Its is_subset(manual_roles) check then returned RolesManagedByOidc before Users.remove_roles ran.
• The sender only logs failures, so revoked roles stayed live on every ingestor until restart.

All five user sync receivers in ingest/ingestor_rbac.rs had the same flaw: each acted on the request instead of what storage holds.

Receiver	Problem
role/sync/remove	The bug above. Also, a repeated sync failed its "roles assigned" check
role/sync/add	Failed if the user's create sync or the role sync hadn't arrived yet. Installed roles from the request body
POST /user/{id}/sync	Installed the body, so a create sync arriving after a delete brought the user back. No UPDATE_LOCK
DELETE /user/{id}/sync	A repeated delete errored, and a late delete could remove a re-created user
password reset sync	Read storage, but without UPDATE_LOCK, so two quick resets could leave the older hash

Fix (same approach as #78):

• Every receiver calls sync_stored_user, which takes UPDATE_LOCK, re-reads the tenant's metadata and calls apply_stored_user.
• apply_stored_user installs the stored user and drops their sessions, or removes the user if storage no longer has them. It is idempotent.
• Staging-write failures are now logged instead of dropped with let _ (a follow-up deferred from fix(rbac): resolve lock-order deadlocks between sessions, users, groups and roles #31).
• Routes, request formats and the create receiver's tenant check are unchanged.

Sender fix found in review: delete_api_key synced the deletion before removing the key from storage. With storage-driven receivers, that would have reinstalled the key on every ingestor. It now commits first. On a sync failure it still returns RevocationSyncFailed and keeps the local key, so a retry re-syncs. Every other sender already committed first.

Deleting a user also removes them from live group membership on the ingestor.

Behaviour changes:

• Repeated or out-of-order syncs now return 200 where they used to return 4xx. The sender only logs failures, so nothing depends on those codes.
• A role change on an ingestor drops the user's sessions instead of refreshing their permissions. Basic-auth sessions are recreated on the next request.
• Each sync replaces the whole live user with the stored copy, which is what a restart loads.

Tests

Tests against apply_stored_user, committed red against a no-op stub:

• a revoked role is removed and the user's sessions are dropped;
• an added role is installed and the user's sessions are dropped;
• applying twice gives the same result;
• a user missing from storage is removed;
• a user missing from the live map is installed;
• a deleted user is removed from live groups;
• an API key is installed and then removed;
• deleting a userid in one tenant leaves another tenant's alone.

The async handlers (lock, read, apply) can't be unit-tested, because get_metadata initialises PARSEABLE.

Verification

• cargo fmt --check ✅, cargo clippy -- -D warnings ✅
• cargo test --lib: 1026 passed, 0 failed, 2 ignored

Not in this PR

The querier still doesn't retry a failed sync, so an ingestor that misses one stays stale until the next sync for that user or a restart.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[VibeSWE]

Adversarial review of PR #80 (at 662b45e4, fixes through 897e9edb)

One Sol 6.1 and one Opus 5.5 reviewer.

Both independently found one blocking regression, now fixed. delete_api_key sent the deletion sync before removing the key from storage. Ingestors now install what storage holds, so they reinstalled the key and returned 200, and a revoked API key stayed usable on every ingestor.

• Fix (c68fcd94): the key is removed from storage first, then synced. On a sync failure the local key is still kept, so a retried DELETE finds it and syncs again; storage no longer has it, so a restart drops it too.
• Other senders checked: every other sender (user create/delete, role add/remove, password, OIDC login, API-key create) already commits before syncing.

Severity	Finding	Outcome
High	API-key delete synced before committing (above)	Fixed in c68fcd94
Medium	Tests cover apply_stored_user, not the handlers; missing API-key and cross-tenant cases	Added tests for API-key install/remove, and for deleting a userid in one tenant leaving another tenant's alone. The handlers read storage, which tests can't reach
Low (pre-existing)	Deleting a user left them in live USER_GROUPS membership	Fixed in dd3275de, with a red test first
Low	Every user sync now drops that user's ingestor sessions	Accepted: ingestors serve no UI, and Basic/API-key sessions rebuild on the next request
Low	The module comment claimed the querier syncs after releasing its lock	Reworded
Nit	Handler comments described the old body-driven behaviour	Updated

Also checked:

• OIDC: no live-only grants are lost; login persists the effective roles and role_grants, and only bearer tokens are memory-only.
• Tenants: tenant handling is unchanged.
• Locks: lock order holds, and no guard is held across an .await.
• Mounting and mixed versions: the receivers are mounted only on ingestors; mixed-version clusters behave as before.