fix(server): reliably dispatch reachability notifications via event (#9843)

app/Jobs/ServerConnectionCheckJob.php

@@ -2,6 +2,7 @@
 
 namespace App\Jobs;
 
+use App\Events\ServerReachabilityChanged;
 use App\Helpers\SshMultiplexingHelper;
 use App\Models\Server;
 use App\Services\ConfigurationRepository;
@@ -43,6 +44,9 @@ private function disableSshMux(): void
 
     public function handle()
     {
+        $wasReachable = (bool) $this->server->settings->is_reachable;
+        $wasNotified = (bool) $this->server->unreachable_notification_sent;
+
         try {
             // Check if server is disabled
             if ($this->server->settings->force_disabled) {
@@ -84,6 +88,8 @@ public function handle()
                     'server_ip' => $this->server->ip,
                 ]);
 
+                $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, false);
+
                 return;
             }
 
@@ -99,6 +105,8 @@ public function handle()
                 $this->server->update(['unreachable_count' => 0]);
             }
 
+            $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, true);
+
         } catch (\Throwable $e) {
 
             Log::error('ServerConnectionCheckJob failed', [
@@ -111,6 +119,8 @@ public function handle()
             ]);
             $this->server->increment('unreachable_count');
 
+            $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, false);
+
             return;
         }
     }
@@ -118,17 +128,41 @@ public function handle()
     public function failed(?\Throwable $exception): void
     {
         if ($exception instanceof TimeoutExceededException) {
+            $wasReachable = (bool) $this->server->settings->is_reachable;
+            $wasNotified = (bool) $this->server->unreachable_notification_sent;
+
             $this->server->settings->update([
                 'is_reachable' => false,
                 'is_usable' => false,
             ]);
             $this->server->increment('unreachable_count');
 
+            $this->dispatchReachabilityChangedIfNeeded($wasReachable, $wasNotified, false);
+
             // Delete the queue job so it doesn't appear in Horizon's failed list.
             $this->job?->delete();
         }
     }
 
+    /**
+     * Fire ServerReachabilityChanged when state crosses the unreachable threshold (count >= 2)
+     * or when a previously-notified server recovers. Skips noise from single transient flaps.
+     */
+    private function dispatchReachabilityChangedIfNeeded(bool $wasReachable, bool $wasNotified, bool $isReachable): void
+    {
+        if ($isReachable) {
+            if (! $wasReachable || $wasNotified) {
+                ServerReachabilityChanged::dispatch($this->server);
+            }
+
+            return;
+        }
+
+        if ($this->server->unreachable_count >= 2 && ! $wasNotified) {
+            ServerReachabilityChanged::dispatch($this->server);
+        }
+    }
+
     private function checkHetznerStatus(): void
     {
         $status = null;

app/Models/Server.php

@@ -1236,10 +1236,8 @@ public function isReachableChanged()
         $this->refresh();
         $unreachableNotificationSent = (bool) $this->unreachable_notification_sent;
         $isReachable = (bool) $this->settings->is_reachable;
-        if ($isReachable === true) {
-            $this->unreachable_count = 0;
-            $this->save();
 
+        if ($isReachable === true) {
             if ($unreachableNotificationSent === true) {
                 $this->sendReachableNotification();
             }
@@ -1247,28 +1245,8 @@ public function isReachableChanged()
             return;
         }
 
-        $this->increment('unreachable_count');
-
-        if ($this->unreachable_count === 1) {
-            $this->settings->is_reachable = true;
-            $this->settings->save();
-
-            return;
-        }
-
         if ($this->unreachable_count >= 2 && ! $unreachableNotificationSent) {
-            $failedChecks = 0;
-            for ($i = 0; $i < 3; $i++) {
-                $status = $this->serverStatus();
-                sleep(5);
-                if (! $status) {
-                    $failedChecks++;
-                }
-            }
-
-            if ($failedChecks === 3 && ! $unreachableNotificationSent) {
-                $this->sendUnreachableNotification();
-            }
+            $this->sendUnreachableNotification();
         }
     }
 

tests/Feature/ServerReachabilityNotificationTest.php

@@ -0,0 +1,105 @@
+<?php
+
+use App\Events\ServerReachabilityChanged;
+use App\Models\Server;
+use App\Models\Team;
+use App\Notifications\Channels\EmailChannel;
+use App\Notifications\Server\Reachable;
+use App\Notifications\Server\Unreachable;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Notification;
+
+uses(RefreshDatabase::class);
+
+beforeEach(function () {
+    $this->team = Team::factory()->create();
+    $this->team->emailNotificationSettings()->update([
+        'use_instance_email_settings' => true,
+        'server_unreachable_email_notifications' => true,
+        'server_reachable_email_notifications' => true,
+    ]);
+
+    $this->server = Server::factory()->create(['team_id' => $this->team->id]);
+
+    Notification::fake();
+});
+
+it('sends Unreachable notification when threshold reached and not yet notified', function () {
+    $this->server->settings()->update(['is_reachable' => false]);
+    $this->server->forceFill([
+        'unreachable_count' => 2,
+        'unreachable_notification_sent' => false,
+    ])->save();
+
+    ServerReachabilityChanged::dispatch($this->server->fresh());
+
+    Notification::assertSentTo($this->team, Unreachable::class);
+    expect($this->server->fresh()->unreachable_notification_sent)->toBeTrue();
+});
+
+it('does not send Unreachable on first transient failure (count=1)', function () {
+    $this->server->settings()->update(['is_reachable' => false]);
+    $this->server->forceFill([
+        'unreachable_count' => 1,
+        'unreachable_notification_sent' => false,
+    ])->save();
+
+    ServerReachabilityChanged::dispatch($this->server->fresh());
+
+    Notification::assertNothingSent();
+});
+
+it('does not send Unreachable when already notified', function () {
+    $this->server->settings()->update(['is_reachable' => false]);
+    $this->server->forceFill([
+        'unreachable_count' => 5,
+        'unreachable_notification_sent' => true,
+    ])->save();
+
+    ServerReachabilityChanged::dispatch($this->server->fresh());
+
+    Notification::assertNothingSent();
+});
+
+it('sends Reachable notification on recovery when previously notified', function () {
+    $this->server->settings()->update(['is_reachable' => true]);
+    $this->server->forceFill([
+        'unreachable_count' => 0,
+        'unreachable_notification_sent' => true,
+    ])->save();
+
+    $fresh = $this->server->fresh();
+    expect($fresh->unreachable_notification_sent)->toBeTrue();
+    expect((bool) $fresh->settings->is_reachable)->toBeTrue();
+
+    ServerReachabilityChanged::dispatch($fresh);
+
+    Notification::assertSentTo($this->team, Reachable::class);
+    expect($this->server->fresh()->unreachable_notification_sent)->toBeFalse();
+});
+
+it('does not send Reachable when never notified', function () {
+    $this->server->settings()->update(['is_reachable' => true]);
+    $this->server->forceFill([
+        'unreachable_count' => 0,
+        'unreachable_notification_sent' => false,
+    ])->save();
+
+    ServerReachabilityChanged::dispatch($this->server->fresh());
+
+    Notification::assertNothingSent();
+});
+
+it('routes Unreachable notification through EmailChannel when email toggle is on', function () {
+    $this->server->settings()->update(['is_reachable' => false]);
+    $this->server->forceFill([
+        'unreachable_count' => 2,
+        'unreachable_notification_sent' => false,
+    ])->save();
+
+    ServerReachabilityChanged::dispatch($this->server->fresh());
+
+    Notification::assertSentTo($this->team, Unreachable::class, function ($notification, $channels) {
+        return in_array(EmailChannel::class, $channels);
+    });
+});

tests/Unit/IsReachableChangedTest.php

@@ -0,0 +1,61 @@
+<?php
+
+use App\Models\Server;
+
+afterEach(function () {
+    Mockery::close();
+});
+
+function makeServerForReachabilityTest(bool $isReachable, bool $notificationSent, int $unreachableCount): Server
+{
+    $settings = Mockery::mock();
+    $settings->is_reachable = $isReachable;
+
+    $server = Mockery::mock(Server::class)->makePartial()->shouldAllowMockingProtectedMethods();
+    $server->shouldReceive('refresh')->andReturnSelf();
+    $server->shouldReceive('getAttribute')->with('settings')->andReturn($settings);
+    $server->shouldReceive('getAttribute')->with('unreachable_notification_sent')->andReturn($notificationSent);
+    $server->shouldReceive('getAttribute')->with('unreachable_count')->andReturn($unreachableCount);
+
+    return $server;
+}
+
+it('sends Reachable notification when reachable and notification was previously sent', function () {
+    $server = makeServerForReachabilityTest(isReachable: true, notificationSent: true, unreachableCount: 0);
+    $server->shouldReceive('sendReachableNotification')->once();
+    $server->shouldNotReceive('sendUnreachableNotification');
+
+    $server->isReachableChanged();
+});
+
+it('does not send any notification when reachable and notification was never sent', function () {
+    $server = makeServerForReachabilityTest(isReachable: true, notificationSent: false, unreachableCount: 0);
+    $server->shouldNotReceive('sendReachableNotification');
+    $server->shouldNotReceive('sendUnreachableNotification');
+
+    $server->isReachableChanged();
+});
+
+it('sends Unreachable notification when count >= 2 and not yet notified', function () {
+    $server = makeServerForReachabilityTest(isReachable: false, notificationSent: false, unreachableCount: 2);
+    $server->shouldReceive('sendUnreachableNotification')->once();
+    $server->shouldNotReceive('sendReachableNotification');
+
+    $server->isReachableChanged();
+});
+
+it('does not send Unreachable notification on first transient failure (count=1)', function () {
+    $server = makeServerForReachabilityTest(isReachable: false, notificationSent: false, unreachableCount: 1);
+    $server->shouldNotReceive('sendUnreachableNotification');
+    $server->shouldNotReceive('sendReachableNotification');
+
+    $server->isReachableChanged();
+});
+
+it('does not double-send Unreachable when already notified', function () {
+    $server = makeServerForReachabilityTest(isReachable: false, notificationSent: true, unreachableCount: 5);
+    $server->shouldNotReceive('sendUnreachableNotification');
+    $server->shouldNotReceive('sendReachableNotification');
+
+    $server->isReachableChanged();
+});

tests/Unit/ServerBackoffTest.php

@@ -1,11 +1,13 @@
 <?php
 
+use App\Events\ServerReachabilityChanged;
 use App\Jobs\ServerCheckJob;
 use App\Jobs\ServerConnectionCheckJob;
 use App\Jobs\ServerManagerJob;
 use App\Models\Server;
 use Illuminate\Queue\TimeoutExceededException;
 use Illuminate\Support\Carbon;
+use Illuminate\Support\Facades\Event;
 use Tests\TestCase;
 
 uses(TestCase::class);
@@ -126,16 +128,21 @@
 
 describe('ServerConnectionCheckJob unreachable_count', function () {
     it('increments unreachable_count on timeout', function () {
+        Event::fake([ServerReachabilityChanged::class]);
+
         $settings = Mockery::mock();
+        $settings->is_reachable = true;
         $settings->shouldReceive('update')
             ->with(['is_reachable' => false, 'is_usable' => false])
             ->once();
 
         $server = Mockery::mock(Server::class)->makePartial()->shouldAllowMockingProtectedMethods();
         $server->shouldReceive('getAttribute')->with('settings')->andReturn($settings);
+        $server->shouldReceive('getAttribute')->with('unreachable_notification_sent')->andReturn(false);
         $server->shouldReceive('increment')->with('unreachable_count')->once();
         $server->id = 1;
         $server->name = 'test-server';
+        $server->unreachable_count = 1; // Will become 2 after increment in real code; mock keeps value as-is
 
         $job = new ServerConnectionCheckJob($server);
         $job->failed(new TimeoutExceededException);
@@ -152,6 +159,50 @@
     });
 });
 
+describe('ServerConnectionCheckJob ServerReachabilityChanged dispatch', function () {
+    // ServerReachabilityChanged's constructor calls $server->isReachableChanged() — verifying that
+    // call is a clean proxy for "the event was dispatched", and avoids serializing a Mockery proxy
+    // through the event dispatcher (which trips Eloquent static method lookups on the proxy class).
+    $invoke = function (bool $wasReachable, bool $wasNotified, bool $isReachable, int $unreachableCount, bool $expectDispatch) {
+        $server = Mockery::mock(Server::class)->makePartial()->shouldAllowMockingProtectedMethods();
+        $server->shouldReceive('getAttribute')->with('unreachable_count')->andReturn($unreachableCount);
+        $server->shouldReceive('getAttribute')->with('id')->andReturn(1);
+        if ($expectDispatch) {
+            $server->shouldReceive('isReachableChanged')->once()->andReturnNull();
+        } else {
+            $server->shouldNotReceive('isReachableChanged');
+        }
+
+        $job = new ServerConnectionCheckJob($server);
+        $method = new ReflectionMethod($job, 'dispatchReachabilityChangedIfNeeded');
+        $method->invoke($job, $wasReachable, $wasNotified, $isReachable);
+    };
+
+    it('dispatches event when count crosses unreachable threshold', function () use ($invoke) {
+        $invoke(true, false, false, 2, true);
+    });
+
+    it('does not dispatch on first transient failure (count=1)', function () use ($invoke) {
+        $invoke(true, false, false, 1, false);
+    });
+
+    it('does not dispatch when already notified and still unreachable', function () use ($invoke) {
+        $invoke(false, true, false, 5, false);
+    });
+
+    it('dispatches recovery event when previously unreachable', function () use ($invoke) {
+        $invoke(false, false, true, 0, true);
+    });
+
+    it('dispatches recovery event when previously notified', function () use ($invoke) {
+        $invoke(true, true, true, 0, true);
+    });
+
+    it('does not dispatch when consistently reachable and never notified', function () use ($invoke) {
+        $invoke(true, false, true, 0, false);
+    });
+});
+
 describe('ServerCheckJob unreachable_count', function () {
     it('increments unreachable_count on timeout', function () {
         $server = Mockery::mock(Server::class)->makePartial()->shouldAllowMockingProtectedMethods();