Refresh a mailbox on the schedule under the lock a send would hold

freshAccessToken() serialises refreshes per connection, and its comment
says why: both providers rotate the refresh token as they hand out an
access token, so the token is good for exactly one use, and "a worker
racing the nightly refresh command means the slower one spends a token the
faster one has already replaced. The provider answers that with
invalid_grant, which is the same thing it says about a genuinely revoked
grant: last_error gets written, the settings page turns red, and every
admin is told to go and re-consent a connection that was never broken."

The nightly refresh command called refresh() directly, outside that lock.
It was the racer the comment names, not a party to the arrangement it
describes.

It now goes through refreshSerially(), which takes the same lock -- named
once, in one place, for both callers -- re-reads the row inside it, and
refreshes. Unlike freshAccessToken() it refreshes a token that is still
usable, which is the point of the daily run: a delegated refresh token
dies of disuse and this keeps the window sliding.

The lock is taken rather than waited for, unlike the send path. Nobody is
standing at a screen for a scheduled job, and a held lock means somebody
is refreshing this very connection right now -- which slides the window
and establishes its health just as well as doing it again would.

One test: with the lock held, the command sends no token request and
leaves the connection untouched. Without the fix it spends the refresh
token the holder is already spending.

app/Modules/Platform/Mail/Console/RefreshMailOAuthTokensCommand.php

@@ -50,7 +50,10 @@ class RefreshMailOAuthTokensCommand extends Command
             $hadError = $connection->last_error !== null;
 
             try {
-                $brokers->for($connection->provider)->refresh($connection);
+                // Serialised against sends: refresh() on its own is the
+                // other half of the race freshAccessToken()'s lock is
+                // there to stop.
+                $brokers->for($connection->provider)->refreshSerially($connection);
 
                 $this->info("Refreshed {$connection->provider->value} ({$connection->account_email}).");
 

app/Modules/Platform/Mail/MailOAuthBroker.php

@@ -37,6 +37,15 @@ interface MailOAuthBroker
      */
     public function refresh(MailOAuthConnection $connection): void;
 
+    /**
+     * A refresh that is not racing a send: the scheduled health check's
+     * way in, serialised against freshAccessToken() on the same
+     * connection.
+     *
+     * @throws MailOAuthException
+     */
+    public function refreshSerially(MailOAuthConnection $connection): void;
+
     /**
      * An access token currently valid for at least a small safety margin,
      * refreshing first when needed — what transports call at send time.

app/Modules/Platform/Mail/OAuthCodeFlowBroker.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\Modules\Platform\Mail;
 
+use Illuminate\Contracts\Cache\Lock;
 use Illuminate\Contracts\Cache\LockTimeoutException;
 use Illuminate\Http\Client\Response;
 use Illuminate\Support\Facades\Cache;
@@ -84,6 +85,48 @@ abstract class OAuthCodeFlowBroker implements MailOAuthBroker
         $this->storeTokens($connection, $response);
     }
 
+    /**
+     * The scheduled refresh, holding the same lock a send would.
+     *
+     * freshAccessToken() takes that lock because a refresh token is good
+     * for exactly one use, and it names this command as one of the racers:
+     * "a worker racing the nightly refresh command means the slower one
+     * spends a token the faster one has already replaced", which the
+     * provider answers with an invalid_grant indistinguishable from a
+     * revoked grant. The command was doing its refresh outside the lock,
+     * so it was the other half of that race rather than a party to it.
+     *
+     * Unlike freshAccessToken() this refreshes a token that is still
+     * usable, which is the point of the daily run: a delegated refresh
+     * token dies of disuse, and the refresh keeps the window sliding.
+     *
+     * Taken rather than waited for, unlike the send path: nobody is
+     * standing at a screen here, and a held lock means somebody is
+     * refreshing this very connection right now — which slides the window
+     * and establishes its health just as well as doing it again would.
+     * Spending the token behind them is the false alarm the lock exists to
+     * prevent.
+     */
+    public function refreshSerially(MailOAuthConnection $connection): void
+    {
+        $lock = $this->refreshLock($connection);
+
+        if (! $lock->get()) {
+            return;
+        }
+
+        try {
+            // Re-read first: the winner may have stored tokens while this
+            // call was waiting, and refreshing the copy walked in with
+            // would spend a refresh token that is no longer current.
+            $connection->refresh();
+
+            $this->refresh($connection);
+        } finally {
+            $lock->release();
+        }
+    }
+
     public function freshAccessToken(MailOAuthConnection $connection): string
     {
         if ($this->stillUsable($connection)) {
@@ -104,7 +147,7 @@ abstract class OAuthCodeFlowBroker implements MailOAuthBroker
         // re-read the row instead of trusting the copy it walked in with: by
         // the time the lock is theirs, the winner has already stored a token
         // they can just use.
-        $lock = Cache::lock('mail-oauth-refresh:'.$connection->provider->value, 30);
+        $lock = $this->refreshLock($connection);
 
         try {
             $lock->block(15);
@@ -135,6 +178,16 @@ abstract class OAuthCodeFlowBroker implements MailOAuthBroker
     }
 
     /** Whether the stored access token has enough life left to send with. */
+    /**
+     * One refresh at a time per connection, whoever is asking. The TTL
+     * outlives a token request and releases the claim if the holder dies
+     * mid-flight.
+     */
+    private function refreshLock(MailOAuthConnection $connection): Lock
+    {
+        return Cache::lock('mail-oauth-refresh:'.$connection->provider->value, 30);
+    }
+
     private function stillUsable(MailOAuthConnection $connection): bool
     {
         $token = $connection->access_token;

tests/Feature/Platform/MailOAuthTest.php

@@ -11,6 +11,7 @@ use App\Modules\Platform\Settings\MailConfigApplier;
 use App\Modules\Platform\Settings\MailProvider;
 use App\Modules\Platform\Settings\MailProviderSettings;
 use Illuminate\Support\Facades\Artisan;
+use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\DB;
 use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Mail;
@@ -425,6 +426,39 @@ test('the scheduled refresh keeps a healthy connection fresh', function () {
         ->and($connection->last_refreshed_at)->not->toBeNull();
 });
 
+// freshAccessToken() takes a per-connection lock because a refresh token
+// is good for one use, and its comment names this command as one of the
+// racers. The command refreshed outside that lock, so it was the other
+// half of the race rather than a party to it.
+test('the scheduled refresh stands aside for a send that is already refreshing', function () {
+    Http::fake([
+        'login.microsoftonline.com/*' => Http::response(fakeTokenResponse([
+            'access_token' => 'access-token-2',
+            'refresh_token' => 'refresh-token-2',
+        ])),
+    ]);
+
+    connectMicrosoftMailbox();
+
+    $before = MailOAuthConnection::for(MailProvider::Microsoft365);
+
+    // Somebody else is mid-refresh on this connection.
+    $held = Cache::lock('mail-oauth-refresh:microsoft365', 30);
+    expect($held->get())->toBeTrue();
+
+    Artisan::call('projectsend:refresh-mail-oauth-tokens');
+
+    // The token the holder is spending was not spent a second time.
+    Http::assertNothingSent();
+
+    $after = MailOAuthConnection::for(MailProvider::Microsoft365);
+    expect($after->access_token)->toBe($before->access_token)
+        ->and($after->refresh_token)->toBe($before->refresh_token)
+        ->and($after->last_error)->toBeNull();
+
+    $held->release();
+});
+
 test('a dead grant records the error and notifies settings admins exactly once', function () {
     Http::fake([
         'login.microsoftonline.com/*' => Http::response([