fix(api): Untransacted revoke-then-save can lose an admin user mutation

[bitbiter-dev]

Problem

AdminUserService.UpdateAsync and DeleteAsync revoke a user's sessions and then persist a user mutation, without a transaction. The revocation commits immediately, so a failure on the save leaves the two writes inconsistent.

RevokeAllActiveByUserIdAsync uses ExecuteUpdateAsync, which bypasses the change tracker and issues SQL straight away:

// src/Anichron.Core/Data/Repository/RefreshTokenRepository.cs:26-29
public Task RevokeAllActiveByUserIdAsync(Guid userId, Instant revokedAt, CancellationToken ct)
    => db.RefreshTokens
        .Where(t => t.UserId == userId && t.RevokedAt == null)
        .ExecuteUpdateAsync(s => s.SetProperty(t => t.RevokedAt, revokedAt), ct);

So in AdminUserService.UpdateAsync:

if (shouldRevokeSessions)
    await tokenService.MarkAllSessionsRevokedAsync(targetId, clock.GetCurrentInstant(), ct);  // committed now
await unitOfWork.SaveChangesAsync(ct);                                                        // may throw

Failure scenario. An admin disables a user. MarkAllSessionsRevokedAsync commits the revocation. SaveChangesAsync then throws (constraint violation, connection drop, timeout). Result: every session for that user is revoked, but IsDisabled was never persisted — the user can simply log in again and is not disabled. DeleteAsync has the same shape: sessions revoked, user row never deleted.

The inconsistency is the tell

AdminResetService wraps the identical revoke-then-save pairing correctly:

// src/Anichron.API/Services/AdminResetService.cs:33-37
await unitOfWork.ExecuteInTransactionAsync(async () =>
{
    await tokenService.MarkAllSessionsRevokedAsync(userId, now, ct);
    await unitOfWork.SaveChangesAsync(ct);
}, ct);

Across the API, ExecuteInTransactionAsync is used in 4 places and bare SaveChangesAsync in 10. The same two-write shape is transacted in one module and not in two others.

Why tests don't catch it

All three occurrences of ExecuteInTransactionAsync in the test project are stubs that simply invoke the lambda:

// src/Anichron.API.Tests.Unit/Services/AuthServiceTests.cs:34-39
_unitOfWork.ExecuteInTransactionAsync(Arg.Any<Func<Task<AuthTokens>>>(), Arg.Any<CancellationToken>())
    .Returns(callInfo => callInfo.Arg<Func<Task<AuthTokens>>>()());

There is no Received() assertion on ExecuteInTransactionAsync anywhere in the suite, so transacted and untransacted code paths are indistinguishable to tests. Any fix should come with a test that actually asserts the transaction.

Related depth issue in the same seam

TokenService.IssueAsync:43 calls SaveChangesAsync itself, so the callee owns the save. That is why AuthService has to compensate:

// src/Anichron.API/Services/AuthService.cs:106-110
return AuthResult.Ok(await unitOfWork.ExecuteInTransactionAsync(async () =>
{
    await unitOfWork.SaveChangesAsync(ct);            // save #1
    return await tokenService.IssueAsync(user, ct);   // saves again inside
}, ct));

Two round trips and an explicit transaction to work around a callee that commits. Same pattern at AuthService.cs:160-164. Worth considering as part of the fix: have IssueAsync stage the token and let the caller save once, so the interface reads "issuance stages a token" rather than "issuance also commits, so wrap me".

Verification

Confirmed by reading the source directly, not inferred:

• src/Anichron.API/Services/AdminUserService.cs — UpdateAsync lines 47/49, DeleteAsync lines 62/64: no transaction
• src/Anichron.API/Services/AdminResetService.cs:33-37: transaction present on the same pairing
• src/Anichron.Core/Data/Repository/RefreshTokenRepository.cs:26-29: ExecuteUpdateAsync, immediate SQL

Found during an architecture review of the API auth cluster.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com