Implement P5: Add Authentication System by jhosm · Pull Request #3 · jhosm/CSharp-ACDC

jhosm · 2026-02-09T00:26:40Z

Summary

Add complete auth module with thread-safe token management (ITokenProvider, InMemoryTokenProvider with SemaphoreSlim)
Implement AuthHandler DelegatingHandler with token injection, proactive refresh, 401 retry with concurrent refresh queue via TaskCompletionSource, and single-retry semantics
Add OAuth 2.1 token refresh strategy (OAuthTokenRefreshStrategy) with RFC 1123 date support and auth/transient error classification
Add BackoffManager with exponential backoff (1s-30s, +/-10% jitter), AcdcAuthManager for logout/force-refresh orchestration, UserIdExtractor with claim priority (sub > user_id > uid) and JWT fallback
Include AcdcAuthOptions configuration record and CustomTokenRefreshStrategy for user-provided delegates

Files Added

Source (11 files):

src/CSharpAcdc/Auth/ — ITokenProvider, InMemoryTokenProvider, ITokenRefreshStrategy, OAuthTokenRefreshStrategy, CustomTokenRefreshStrategy, TokenRefreshResult, BackoffManager, AcdcAuthManager, UserIdExtractor
src/CSharpAcdc/Configuration/AcdcAuthOptions.cs
src/CSharpAcdc/Handlers/AuthHandler.cs

Tests (7 files, 51 new tests):

tests/CSharpAcdc.Tests/Auth/ — InMemoryTokenProviderTests, OAuthTokenRefreshStrategyTests, BackoffManagerTests, AcdcAuthManagerTests, UserIdExtractorTests
tests/CSharpAcdc.Tests/Handlers/ — AuthHandlerTests, AuthHandlerConcurrencyTests

Test plan

All 121 tests pass (51 new + 70 existing from P1/P2)
Build succeeds with zero warnings
Thread safety verified: concurrent token provider access, concurrent refresh queue, backoff manager concurrency
Auth error vs transient error classification tested
Request cloning for 401 retry (including content preservation) tested

🤖 Generated with Claude Code

…peline Add complete auth module: ITokenProvider/InMemoryTokenProvider for thread-safe token storage, ITokenRefreshStrategy with OAuth 2.1 and custom delegate implementations, BackoffManager with exponential backoff (1s-30s + jitter), AuthHandler DelegatingHandler with token injection/proactive refresh/401 retry with concurrent refresh queue via TaskCompletionSource, AcdcAuthManager for logout/force-refresh orchestration, UserIdExtractor with claim priority and JWT fallback, and AcdcAuthOptions configuration record. Includes 51 tests covering CRUD, concurrency, error classification, and request cloning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds a new authentication subsystem to the CSharpAcdc client, including token storage/refresh, an HTTP auth handler with 401 retry + concurrency coordination, and supporting utilities with accompanying tests.

Changes:

Introduces auth primitives (token provider, refresh strategies, backoff, auth manager, user id extraction) plus AcdcAuthOptions.
Adds AuthHandler DelegatingHandler for bearer injection, proactive refresh, and 401 single-retry refresh flow.
Adds a comprehensive new test suite covering handler behavior, refresh strategy parsing, backoff behavior, and concurrency.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 26 comments.

Show a summary per file

File	Description
src/CSharpAcdc/Handlers/AuthHandler.cs	Adds auth injection + refresh/retry handler logic.
src/CSharpAcdc/Configuration/AcdcAuthOptions.cs	Introduces configuration options for refresh/revocation and timeouts.
src/CSharpAcdc/CSharpAcdc.csproj	Adds InternalsVisibleTo and new framework reference for HttpContext-related support.
src/CSharpAcdc/Auth/ITokenProvider.cs	Defines token storage API for access/refresh/expiry.
src/CSharpAcdc/Auth/InMemoryTokenProvider.cs	Implements thread-safe in-memory token storage.
src/CSharpAcdc/Auth/ITokenRefreshStrategy.cs	Defines refresh strategy abstraction.
src/CSharpAcdc/Auth/OAuthTokenRefreshStrategy.cs	Implements OAuth refresh flow and error classification + expiry parsing.
src/CSharpAcdc/Auth/CustomTokenRefreshStrategy.cs	Allows injecting a custom refresh delegate.
src/CSharpAcdc/Auth/TokenRefreshResult.cs	Adds refresh result DTO (access/refresh/expiry).
src/CSharpAcdc/Auth/BackoffManager.cs	Adds exponential backoff for transient refresh failures.
src/CSharpAcdc/Auth/AcdcAuthManager.cs	Adds logout/force-refresh orchestration and revocation hook.
src/CSharpAcdc/Auth/UserIdExtractor.cs	Adds user id extraction from claims / JWT authorization header.
tests/CSharpAcdc.Tests/Handlers/AuthHandlerTests.cs	Verifies token injection, skip-auth, 401 retry, and request cloning behavior.
tests/CSharpAcdc.Tests/Handlers/AuthHandlerConcurrencyTests.cs	Verifies single-refresh behavior under concurrency and queue timeout behavior.
tests/CSharpAcdc.Tests/Auth/InMemoryTokenProviderTests.cs	Tests in-memory provider storage, clearing, and concurrency.
tests/CSharpAcdc.Tests/Auth/OAuthTokenRefreshStrategyTests.cs	Tests refresh request formatting, success parsing, and error classification.
tests/CSharpAcdc.Tests/Auth/BackoffManagerTests.cs	Tests backoff attempt tracking and wait behavior.
tests/CSharpAcdc.Tests/Auth/AcdcAuthManagerTests.cs	Tests logout, revocation behavior, backoff reset, and force refresh.
tests/CSharpAcdc.Tests/Auth/UserIdExtractorTests.cs	Tests claim priority and JWT fallback user id extraction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T00:33:10Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+
+            // Proactive refresh: fire-and-forget if token is near expiry
+            _ = TryProactiveRefreshAsync(cancellationToken);


Proactive refresh calls ExecuteRefreshAsync via Task.Run without using the same semaphore/TCS coordination as the 401 refresh path, so multiple concurrent requests (or proactive + reactive refresh) can trigger overlapping refreshes and race token updates. Consider routing proactive refresh through the same refresh queue/leader mechanism (or acquiring _refreshSemaphore) and avoid double fire-and-forget Task.Run; also pass an appropriate CancellationToken instead of CancellationToken.None so shutdown/caller cancellation can stop the refresh.

Suggested change

// Proactive refresh: fire-and-forget if token is near expiry

_ = TryProactiveRefreshAsync(cancellationToken);

Copilot · 2026-02-09T00:33:11Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+        // Try to acquire the refresh semaphore with immediate timeout
+        if (await _refreshSemaphore.WaitAsync(TimeSpan.Zero, ct).ConfigureAwait(false))
+        {
+            // We're the leader — execute the refresh
+            var tcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
+            _pendingRefresh = tcs;
+            try
+            {
+                await ExecuteRefreshAsync(ct).ConfigureAwait(false);
+                tcs.TrySetResult(true);
+                return true;
+            }
+            catch (Exception ex)
+            {
+                tcs.TrySetException(ex);
+                return false;
+            }
+            finally
+            {
+                _pendingRefresh = null;
+                _refreshSemaphore.Release();
+            }
+        }
+
+        // We're a follower — wait for the leader's refresh to complete
+        var pending = _pendingRefresh;
+        if (pending is null)
+            return false;
+


There’s a race where a follower can fail to acquire _refreshSemaphore and observe _pendingRefresh == null (leader sets it only after acquiring the semaphore). In that case this returns false and the request won’t wait/retry even though a refresh is in progress. Make publishing the shared TaskCompletionSource atomic with becoming the leader (e.g., Interlocked.CompareExchange on _pendingRefresh, or set _pendingRefresh before other threads can observe the semaphore-held state).

Copilot · 2026-02-09T00:33:11Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+        if (original.Content is not null)
+        {
+            var contentBytes = await original.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
+            var newContent = new ByteArrayContent(contentBytes);
+


CloneRequestAsync reads request content without a CancellationToken (ReadAsByteArrayAsync() overload without ct). For large bodies this can make cancellation ineffective during the retry clone. Consider threading the SendAsync cancellationToken through CloneRequestAsync and using the ct-aware ReadAsByteArrayAsync(ct) overload.

Copilot · 2026-02-09T00:33:11Z

src/CSharpAcdc/Auth/OAuthTokenRefreshStrategy.cs

+        if (root.TryGetProperty("expires_in", out var expiresInElement))
+        {
+            if (expiresInElement.ValueKind == JsonValueKind.Number)
+            {
+                var seconds = expiresInElement.GetInt32();
+                return DateTimeOffset.UtcNow.AddSeconds(seconds);
+            }
+
+            // Try RFC 1123 date format (fixes Dart bug)
+            var expiresInStr = expiresInElement.GetString();
+            if (expiresInStr is not null &&
+                DateTimeOffset.TryParseExact(
+                    expiresInStr,
+                    "R",
+                    CultureInfo.InvariantCulture,
+                    DateTimeStyles.None,
+                    out var parsedDate))
+            {
+                return parsedDate;
+            }
+        }


ParseExpiry only handles expires_in as a JSON number or an RFC1123 string. Some providers return expires_in as a numeric string (e.g., "3600"), which will currently fall back to the 1-hour default and produce incorrect expiry. Consider parsing a numeric string (int/long) before falling back to RFC1123/default.

Copilot · 2026-02-09T00:33:11Z

src/CSharpAcdc/Auth/AcdcAuthManager.cs

+            using var client = _httpClientFactory.CreateClient("acdc-auth");
+            using var content = new FormUrlEncodedContent(new Dictionary<string, string>
+            {
+                ["client_id"] = _options.ClientId,
+            });
+


The revocation request body only contains client_id. RFC 7009 revocation typically requires a "token" parameter (and often token_type_hint, plus client authentication when applicable). If this endpoint is a standard OAuth revocation endpoint, include the token being revoked (usually the refresh token) and any required auth fields.

Copilot · 2026-02-09T00:33:15Z

tests/CSharpAcdc.Tests/Auth/UserIdExtractorTests.cs

+    public void ExtractUserId_NonBearerScheme_ReturnsNull()
+    {
+        var extractor = new UserIdExtractor();
+        var request = new HttpRequestMessage(HttpMethod.Get, "https://api.example.com/test");


Disposable 'HttpRequestMessage' is created but not disposed.

Copilot · 2026-02-09T00:33:16Z

tests/CSharpAcdc.Tests/Handlers/AuthHandlerTests.cs

+            .Respond(HttpStatusCode.OK);
+
+        using var client = CreateClient(mockHandler);
+        var request = new HttpRequestMessage(HttpMethod.Get, "/test");


Disposable 'HttpRequestMessage' is created but not disposed.

Suggested change

var request = new HttpRequestMessage(HttpMethod.Get, "/test");

using var request = new HttpRequestMessage(HttpMethod.Get, "/test");

Copilot · 2026-02-09T00:33:16Z

tests/CSharpAcdc.Tests/Handlers/AuthHandlerTests.cs

+        });
+
+        using var client = CreateClient(mockHandler);
+        var request = new HttpRequestMessage(HttpMethod.Post, "/test")


Disposable 'HttpRequestMessage' is created but not disposed.

Suggested change

var request = new HttpRequestMessage(HttpMethod.Post, "/test")

using var request = new HttpRequestMessage(HttpMethod.Post, "/test")

Copilot · 2026-02-09T00:33:16Z

tests/CSharpAcdc.Tests/Handlers/AuthHandlerConcurrencyTests.cs

+            .Select(_ => client.GetAsync($"/test/{Guid.NewGuid()}"))
+            .ToArray();
+
+        var responses = await Task.WhenAll(tasks);


This assignment to responses is useless, since its value is never read.

Suggested change

var responses = await Task.WhenAll(tasks);

await Task.WhenAll(tasks);

Copilot · 2026-02-09T00:33:16Z

tests/CSharpAcdc.Tests/Handlers/AuthHandlerConcurrencyTests.cs

+        };
+
+        // First request: will acquire semaphore and start slow refresh
+        var leaderTask = client.GetAsync("/leader");


This assignment to leaderTask is useless, since its value is never read.

Suggested change

var leaderTask = client.GetAsync("/leader");

_ = client.GetAsync("/leader");

jhosm

Review: P5 Auth System

Strong implementation overall — the leader/follower refresh queue, auth vs transient error classification, and the full Dart-ACDC mapping are well done. 51 tests with concurrency coverage is excellent.

Must-fix (6 items)

Concurrent refresh race — follower can observe _pendingRefresh == null while leader is setting it (see comment on AuthHandler.cs:145)
Proactive refresh bypasses the refresh queue — races with 401-triggered refresh (see comment on AuthHandler.cs:86-107)
Cancellation swallowed for followers — OperationCanceledException returns false instead of propagating (see comment on AuthHandler.cs:156-164)
LogoutAsync clears tokens before revocation — can't include token in revoke request (see comment on AcdcAuthManager.cs:41-48)
Token revocation body missing token param — RFC 7009 violation (see comment on AcdcAuthManager.cs:88-94)
refresh_token required in response / numeric string expires_in — two parsing issues in OAuthTokenRefreshStrategy (see comments on lines 90 and 103-118)

Should-fix (3 items)

BackoffManager SemaphoreSlim overkill for simple int ops — use Interlocked
FrameworkReference Microsoft.AspNetCore.App constrains consumers — consider lighter dependency
CloneRequestAsync missing CancellationToken on ReadAsByteArrayAsync

Test hygiene (from Copilot review — agree)

Dispose HttpRequestMessage instances in UserIdExtractorTests
Widen timing tolerances in BackoffManagerTests or inject a time provider
Await leaderTask in ConcurrentRefresh_QueueTimeout test

jhosm · 2026-02-14T11:06:13Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+        var pending = _pendingRefresh;
+        if (pending is null)
+            return false;
+


Bug: Race condition in leader/follower refresh coordination

The leader sets _pendingRefresh after acquiring the semaphore (line 126), but a follower that loses the WaitAsync(TimeSpan.Zero) race observes _pendingRefresh at line 145. There's a window where the follower sees _pendingRefresh == null because the leader hasn't set it yet, causing the follower to give up and return false — silently dropping the retry even though a refresh is in progress.

Fix: publish the TaskCompletionSource atomically with acquiring leadership. For example, use Interlocked.CompareExchange on _pendingRefresh as the coordination primitive instead of a separate semaphore, or set the TCS before the semaphore acquire becomes visible to other threads.

jhosm · 2026-02-14T11:06:14Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+            var expiry = await _tokenProvider.GetTokenExpiryAsync(ct).ConfigureAwait(false);
+            if (expiry is null)
+                return;
+
+            var timeUntilExpiry = expiry.Value - DateTimeOffset.UtcNow;
+            if (timeUntilExpiry > _options.RefreshThreshold)
+                return;
+
+            _logger.LogDebug("Token expires in {TimeUntilExpiry}, proactively refreshing", timeUntilExpiry);
+
+            // Fire-and-forget — don't block the current request
+            _ = Task.Run(async () =>
+            {
+                try
+                {
+                    await ExecuteRefreshAsync(CancellationToken.None).ConfigureAwait(false);
+                }
+                catch (Exception ex)
+                {
+                    _logger.LogWarning(ex, "Proactive token refresh failed");
+                }
+            }, CancellationToken.None);


Bug: Proactive refresh bypasses the concurrent refresh queue

TryProactiveRefreshAsync fires off ExecuteRefreshAsync via Task.Run without acquiring _refreshSemaphore. This means a proactive refresh and a 401-triggered refresh can race concurrently, both calling _refreshStrategy.RefreshAsync and _tokenProvider.SaveTokensAsync — potentially clobbering each other's results.

The proactive refresh should go through the same leader/follower mechanism (or at minimum acquire the semaphore). Also, passing CancellationToken.None means this background refresh survives handler disposal and can outlive the request pipeline.

jhosm · 2026-02-14T11:06:15Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+        }
+        catch (AcdcAuthException)
+        {
+            throw;
+        }
+        catch
+        {
+            return false;
+        }


Bug: Cancellation swallowed for follower requests

When a follower's ct is canceled while waiting on pending.Task, the OperationCanceledException is caught by the generic catch at line 164 and returns false. This silently converts a cancellation into a 401 response instead of propagating the cancellation.

The generic catch should re-throw OperationCanceledException when ct.IsCancellationRequested:

catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; }

jhosm · 2026-02-14T11:06:16Z

src/CSharpAcdc/Handlers/AuthHandler.cs

+        {
+            var contentBytes = await original.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
+            var newContent = new ByteArrayContent(contentBytes);
+


Nit: CloneRequestAsync missing CancellationToken

ReadAsByteArrayAsync() is called without a CancellationToken. For large request bodies, this can hang despite the caller's token being canceled. Thread the SendAsync cancellation token through CloneRequestAsync and use the CT-aware overload.

jhosm · 2026-02-14T11:06:17Z

src/CSharpAcdc/Auth/AcdcAuthManager.cs

+        try
+        {
+            await _tokenProvider.ClearTokensAsync(ct).ConfigureAwait(false);
+
+            if (!string.IsNullOrEmpty(_options.RevocationEndpoint))
+            {
+                await TryRevokeTokenAsync(ct).ConfigureAwait(false);
+            }


Bug: LogoutAsync clears tokens before revocation

Tokens are cleared at line 43 before TryRevokeTokenAsync is called. This means the revocation request can't include the token being revoked. Per RFC 7009, the token parameter is required.

Fix: capture the refresh/access token before clearing, then pass it to TryRevokeTokenAsync:

var refreshToken = await _tokenProvider.GetRefreshTokenAsync(ct); await _tokenProvider.ClearTokensAsync(ct); if (!string.IsNullOrEmpty(_options.RevocationEndpoint) && refreshToken is not null) await TryRevokeTokenAsync(refreshToken, ct);

jhosm · 2026-02-14T11:06:18Z

src/CSharpAcdc/Auth/AcdcAuthManager.cs

+        {
+            using var client = _httpClientFactory.CreateClient("acdc-auth");
+            using var content = new FormUrlEncodedContent(new Dictionary<string, string>
+            {
+                ["client_id"] = _options.ClientId,
+            });
+


Bug: Token revocation body is incomplete per RFC 7009

The revocation request only sends client_id. RFC 7009 Section 2.1 requires a token parameter (the token to be revoked) and recommends token_type_hint. Without the token, the revocation endpoint has nothing to revoke.

["client_id"] = _options.ClientId, ["token"] = refreshToken, // Pass the captured token ["token_type_hint"] = "refresh_token",

jhosm · 2026-02-14T11:06:20Z

src/CSharpAcdc/Auth/OAuthTokenRefreshStrategy.cs

+
+        var newRefreshToken = root.GetProperty("refresh_token").GetString()
+            ?? throw new InvalidOperationException("Missing refresh_token in refresh response");
+


Bug: refresh_token is required but many OAuth providers omit it

root.GetProperty("refresh_token") throws KeyNotFoundException when the response omits refresh_token. Many OAuth providers only return a new access_token on refresh and keep the existing refresh token valid.

Use TryGetProperty and fall back to the input refreshToken:

var newRefreshToken = root.TryGetProperty("refresh_token", out var rtElement) ? rtElement.GetString() ?? refreshToken : refreshToken;

jhosm · 2026-02-14T11:06:21Z

src/CSharpAcdc/Auth/OAuthTokenRefreshStrategy.cs

+                return DateTimeOffset.UtcNow.AddSeconds(seconds);
+            }
+
+            // Try RFC 1123 date format (fixes Dart bug)
+            var expiresInStr = expiresInElement.GetString();
+            if (expiresInStr is not null &&
+                DateTimeOffset.TryParseExact(
+                    expiresInStr,
+                    "R",
+                    CultureInfo.InvariantCulture,
+                    DateTimeStyles.None,
+                    out var parsedDate))
+            {
+                return parsedDate;
+            }
+        }


Bug: Numeric string expires_in silently defaults to 1 hour

Some OAuth providers return expires_in as "3600" (a JSON string containing a number). The current code handles JsonValueKind.Number and then tries RFC 1123 date parsing, but a numeric string like "3600" won't parse as RFC 1123 and will silently fall through to the 1-hour default.

Add int.TryParse before the RFC 1123 fallback:

var expiresInStr = expiresInElement.GetString(); if (expiresInStr is not null) { if (int.TryParse(expiresInStr, out var numericSeconds)) return DateTimeOffset.UtcNow.AddSeconds(numericSeconds); if (DateTimeOffset.TryParseExact(expiresInStr, "R", ...)) return parsedDate; }

jhosm · 2026-02-14T11:06:22Z

src/CSharpAcdc/CSharpAcdc.csproj

+
+  <ItemGroup>
+    <FrameworkReference Include="Microsoft.AspNetCore.App" />
+  </ItemGroup>


Consider: FrameworkReference ties the library to ASP.NET Core

Adding Microsoft.AspNetCore.App as a framework reference constrains consumers to ASP.NET Core hosts. The only usage is IHttpContextAccessor in UserIdExtractor.

Consider depending on Microsoft.AspNetCore.Http.Abstractions package instead (lighter), or making UserIdExtractor's IHttpContextAccessor dependency optional via a separate interface (e.g., IUserIdProvider) so the core library doesn't require the ASP.NET shared framework.

jhosm · 2026-02-14T11:06:23Z

src/CSharpAcdc/Auth/BackoffManager.cs

+    private static readonly TimeSpan BaseDelay = TimeSpan.FromSeconds(1);
+    private static readonly TimeSpan MaxDelay = TimeSpan.FromSeconds(30);
+
+    private readonly SemaphoreSlim _semaphore = new(1, 1);


Suggestion: SemaphoreSlim is overkill for simple int operations

RecordFailureAsync, ResetAsync, and GetAttemptAsync only read/write a single int. These could use Interlocked.Increment/Interlocked.Exchange/Interlocked.CompareExchange (or volatile reads) without the overhead and async ceremony of SemaphoreSlim. The semaphore is only truly needed in WaitIfNeededAsync where you read-compute-delay atomically. This would also eliminate the async tax on what are fundamentally synchronous operations.

Copilot AI review requested due to automatic review settings February 9, 2026 00:26

Copilot started reviewing on behalf of jhosm February 9, 2026 00:27 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

jhosm commented Feb 14, 2026

View reviewed changes


		// Proactive refresh: fire-and-forget if token is near expiry
		_ = TryProactiveRefreshAsync(cancellationToken);

	var request = new HttpRequestMessage(HttpMethod.Get, "/test");
	using var request = new HttpRequestMessage(HttpMethod.Get, "/test");

	var responses = await Task.WhenAll(tasks);
	await Task.WhenAll(tasks);

	var leaderTask = client.GetAsync("/leader");
	_ = client.GetAsync("/leader");


		var newRefreshToken = root.GetProperty("refresh_token").GetString()
		?? throw new InvalidOperationException("Missing refresh_token in refresh response");

Conversation

jhosm commented Feb 9, 2026

Summary

Files Added

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

jhosm left a comment

Choose a reason for hiding this comment

Review: P5 Auth System

Must-fix (6 items)

Should-fix (3 items)

Test hygiene (from Copilot review — agree)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant