Fix-Runde 2: Drossel-Marke erst nach Erfolg, Kommentar richtiggestellt

Die Marke stand bisher VOR dem Versandversuch - scheiterte das Einreihen,
schwieg das Subjekt fuer den Rest der Stunde, obwohl null statt einer Mail
angekommen war. Jetzt steht sie in notifyInstanceOwner() erst nach dem
erfolgreichen queue() im try-Block, in notifyHostManagers() erst nach
mindestens einem geglueckten Einreihen (Merker ueber die Schleife, da ein
einzelnes abgelehntes Postfach weder die uebrigen Betreiber noch die Marke
fuer alle blockieren darf). Kein Sturm-Risiko: Sperren entstehen ohnehin
nur ab der Zehner-Schwelle, nicht bei jedem Fehlversuch.

Die Kommentare behaupteten außerdem, das try/catch finge Zustellungsfehler
ab - tatsaechlich faengt es nur, was beim EINREIHEN schiefgeht (synchron,
vor der Warteschlange); ein Zustellungsfehler passiert spaeter im
Warteschlangen-Arbeiter und steht in dessen Protokoll. Beide Docblocks
richtiggestellt.

Neuer Testfall haengt einen Wrapper vor die gefakte Mail-Fassade, dessen
erster to()-Aufruf wirft und ab dem zweiten an die echte Fake-Instanz
durchreicht - MailFake::queue() selbst kann einen Fehlschlag nicht
simulieren, weil es den Mailable nur ablegt und dabei nie wirft. Als
Gegenprobe testweise auf den alten Code zurueckgesetzt: Testfall lief rot
mit der erwarteten Meldung, Datei danach byte-identisch wiederhergestellt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

app/Services/Security/BlockAddress.php

@@ -89,13 +89,21 @@ class BlockAddress
      * Empfängerlogik (Betreiber statt Kunde), gleiche Drossel-Idee.
      *
      * Zustellung ist nicht die Bedingung für Schutz: die Sperre steht bereits,
-     * bevor hier auch nur versucht wird zu verschicken, und die Drossel wird
-     * VOR dem Versandversuch gesetzt, nicht danach — sonst würde ein
-     * dauerhaft kaputtes Postfach bei jeder weiteren Sperre an derselben
-     * Instanz erneut versuchen und denselben Fehler immer wieder melden. Ein
-     * Throwable beim Verschicken wird über `report()` gemeldet und
-     * verschluckt: ein kaputtes Postfach darf die Sperre nicht rückgängig
-     * machen, die es eigentlich ankündigen sollte.
+     * bevor hier auch nur versucht wird zu verschicken. Die Drossel-Marke
+     * wird erst NACH dem erfolgreichen Einreihen gesetzt, nicht davor —
+     * sonst bliebe die Instanz nach einem einzigen fehlgeschlagenen Versuch
+     * für den Rest der Stunde ganz ohne Meldung, ausgerechnet dann, wenn statt
+     * einer Mail null angekommen sind. Ein Sturm bei dauerhaft kaputtem
+     * Postfach ist trotzdem ausgeschlossen: Sperren selbst entstehen schon
+     * nur ab der Zehner-Schwelle, nicht bei jedem Fehlversuch.
+     *
+     * `try/catch` fängt hier nur, was beim EINREIHEN schiefgeht — ein
+     * synchroner Fehler, bevor die Mail überhaupt in der Warteschlange liegt
+     * (`SecurityBlockMail` implementiert `ShouldQueue`). Was bei der
+     * tatsächlichen ZUSTELLUNG schiefgeht, passiert später, in einem
+     * Warteschlangen-Arbeiter, lange nachdem diese Methode zurückgekehrt ist,
+     * und steht in dessen Protokoll, nicht hier. Die Sperre bleibt in beiden
+     * Fällen stehen — das ist die eigentliche Zusage.
      */
     private function notifyInstanceOwner(Instance $instance, SecurityBlock $block): void
     {
@@ -106,10 +114,9 @@ class BlockAddress
             return;
         }
 
-        Settings::set($key, now()->toIso8601String());
-
         try {
             Mail::to($instance->customer->email)->queue(new SecurityBlockMail($block));
+            Settings::set($key, now()->toIso8601String());
         } catch (Throwable $e) {
             report($e);
         }
@@ -132,6 +139,17 @@ class BlockAddress
      * neben dem Owner, oder eine, in der `hosts.manage` gerade niemandem
      * zugewiesen ist, bekommt schlicht keine Mail — die Sperre selbst steht
      * trotzdem, unabhängig davon.
+     *
+     * Die Drossel-Marke wird erst gesetzt, wenn mindestens ein Einreihen
+     * geglückt ist — aus demselben Grund wie bei `notifyInstanceOwner()`: ein
+     * Host, bei dem in einer Stunde ausgerechnet null statt einer Mail
+     * ankamen, soll nicht auch noch für den Rest der Stunde verstummen. Jede
+     * Adresse steht in ihrem EIGENEN `try/catch`, das nur das EINREIHEN
+     * abdeckt (synchron, vor der Warteschlange) — nicht die tatsächliche
+     * Zustellung, die später im Warteschlangen-Arbeiter passiert und dort
+     * protokolliert wird. Ein einzelnes abgelehntes Postfach darf so weder
+     * die übrigen Betreiber um ihre Meldung bringen noch die Marke für alle
+     * verhindern, solange wenigstens einer sie bekommen hat.
      */
     private function notifyHostManagers(Host $host, SecurityBlock $block): void
     {
@@ -151,17 +169,20 @@ class BlockAddress
             return;
         }
 
-        Settings::set($key, now()->toIso8601String());
+        $queued = false;
 
-        // Einzeln umschlossen: ein Postfach, das ablehnt, darf die übrigen
-        // Betreiber nicht auch noch um ihre Meldung bringen.
         foreach ($recipients as $email) {
             try {
                 Mail::to($email)->queue(new SecurityBlockMail($block));
+                $queued = true;
             } catch (Throwable $e) {
                 report($e);
             }
         }
+
+        if ($queued) {
+            Settings::set($key, now()->toIso8601String());
+        }
     }
 
     /** Läuft für diese Adresse an diesem Subjekt schon eine Sperre? */

tests/Feature/Security/SecurityBlockMailTest.php

@@ -55,6 +55,44 @@ it('laesst die Sperre stehen, wenn die Mail scheitert', function () {
         ->and(SecurityBlock::count())->toBe(1);
 });
 
+it('drosselt nicht, wenn gar keine Mail hinausging', function () {
+    // Sonst schweigt eine Instanz eine volle Stunde, obwohl NULL Meldungen
+    // ankamen — genau dann, wenn ohnehin schon etwas klemmt.
+    Mail::fake();
+
+    // Ein Postfach, das den ERSTEN Aufruf von Mail::to() wirft und danach an
+    // die echte Fake-Implementierung durchreicht — so laesst sich "der erste
+    // Versandversuch scheitert, der zweite geht durch" nachstellen, ohne die
+    // Zusicherung am Ende (Mail::assertQueued) aufzugeben.
+    $fake = Mail::getFacadeRoot();
+    Mail::swap(new class($fake)
+    {
+        private int $calls = 0;
+
+        public function __construct(private readonly object $inner) {}
+
+        public function __call(string $name, array $arguments): mixed
+        {
+            if ($name === 'to' && ++$this->calls === 1) {
+                throw new RuntimeException('Postfach kaputt');
+            }
+
+            return $this->inner->{$name}(...$arguments);
+        }
+    });
+
+    $instance = Instance::factory()->create(['status' => 'active', 'vmid' => 101]);
+
+    // erster Versuch: Versand scheitert
+    app(BlockAddress::class)->forInstance($instance, '203.0.113.7', 12);
+    // zweiter Versuch kurz darauf: Versand geht durch
+    app(BlockAddress::class)->forInstance($instance, '203.0.113.8', 12);
+
+    // Die zweite Sperre muss eine Mail erzeugen, obwohl die erste eine
+    // Stunde 'verbraucht' zu haben schien.
+    Mail::assertQueued(SecurityBlockMail::class);
+});
+
 it('geht standardmaessig aus dem System-Postfach und folgt der Wegwahl', function () {
     Mailbox::factory()->create(['key' => 'no-reply', 'address' => 'no-reply@clupilot.com', 'active' => true]);
     $info = Mailbox::factory()->create(['key' => 'info', 'active' => true]);