[VibeSWE]

Summary

Two follow-ups from the #67 review, both pre-existing, both fixed test-first.

1. Medium: Query-node role DELETE never took UPDATE_LOCK. Standalone role::delete does. Without it, a concurrent post_user (read metadata → modify → write, under the lock) and a role delete could each overwrite the other's metadata write. querier_role::delete now holds the lock through persistence and the in-memory update, and releases it before the cluster sync, like querier_role::put.
2. Low: post_gen_password read metadata before taking UPDATE_LOCK. That has the same lost-update risk.
   • Query node: the handler also skipped standalone's protected-user check. It now rejects protected users with ProtectedUser (400) and locks before reading metadata.
   • Standalone: had the same read-before-lock order, and now locks first too.
3. Medium (found in review): Query-node role DELETE didn't reject the default OIDC role. Standalone returns DefaultRoleInUse (400); the querier deleted the role and left a dangling default. Ported as-is. The check reads stored metadata, which tests can't reach.

Tests

• delete_waits_for_update_lock_before_checking_role: with the lock held, the delete doesn't return; after release it completes.
• post_gen_password_rejects_protected_user_before_storage: the password hash is unchanged.
• post_gen_password_waits_for_update_lock_before_storage: covers both the querier and standalone handlers.

Without the fixes, the lock-wait tests fail by assertion or by the test binary exiting with code 2. The exit happens when a handler reads metadata before the lock: that initialises PARSEABLE from the test arguments.

Verification

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

🤖 Generated with Claude Code

---

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

[VibeSWE]

Adversarial review of PR #77 (at 54adf114)

One Opus 5.5 and one Sol 6.1 reviewer. Neither found anything blocking in this PR's changes. Both checked:

• Nothing querier_role::delete calls before drop(guard) takes UPDATE_LOCK again, so there is no re-entrant lock.
• No parking_lot guard is held across an .await, and every early return happens before any mutation.
• The guard covers the whole read-modify-write and the in-memory publication, matching standalone role::delete.
• Every RBAC metadata writer now holds UPDATE_LOCK from before the read through publication: users, roles, default role, API keys, OIDC, and group cleanup.
• The bounded-timeout tests can't pass falsely: timeout polls first, and TEST_SERIAL is always taken before UPDATE_LOCK.

Severity	Finding	Outcome
Medium	Querier role::delete skipped standalone's DefaultRoleInUse check, so deleting the default OIDC role succeeded and left a dangling default	Fixed in 77feb033. The check reads stored metadata, which tests can't reach, so it is a direct port with no test
Medium (pre-existing, separate)	User PATCH (rbac.rs:216) updates the live user before persisting	Separate PR in progress
Low	The tests prove the handler waits for the lock, not that it keeps holding it through put_metadata	Accepted; proving that needs a storage seam
Nit	Querier post_gen_password holds the lock through the ingestor sync, unlike delete	Kept: querier post_user and user delete do the same