From c899d41946ffa85f8eda02c90afb7b7c16907f2f Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Mon, 27 Jul 2026 23:59:05 +0200
Subject: [PATCH] Pin CloudReady's null-mailbox guard and share it with the
 trait

A mutation the reviewer ran (if ($box !== null) -> if (true)) left every
test green: CloudReady hand-rolled the same null-mailbox guard as
SendsFromMailbox, but nothing exercised its null branch. Added the
missing test, then extracted the decision itself (null mailbox -> no
sender fields, no_reply -> no Reply-To) into one shared method so an
Envelope-shaped and a MailMessage-shaped consumer can no longer disagree.
---
 app/Mail/Concerns/SendsFromMailbox.php   | 39 ++++++++++++++++++------
 app/Notifications/CloudReady.php         | 17 +++++------
 tests/Feature/Mail/SenderAddressTest.php | 19 ++++++++++++
 3 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/app/Mail/Concerns/SendsFromMailbox.php b/app/Mail/Concerns/SendsFromMailbox.php
index c06bf9e..7cb3ace 100644
--- a/app/Mail/Concerns/SendsFromMailbox.php
+++ b/app/Mail/Concerns/SendsFromMailbox.php
@@ -20,18 +20,39 @@ trait SendsFromMailbox
 {
     protected function mailboxEnvelope(string $purpose, string $subject): Envelope
     {
-        $box = app(MailboxResolver::class)->for($purpose);
-
-        if ($box === null) {
-            // No mailbox configured yet — fall back to the framework default so
-            // an installation mid-setup still sends rather than crashing.
-            return new Envelope(subject: $subject);
-        }
+        [$from, $replyTo] = $this->mailboxAddresses($purpose);
 
         return new Envelope(
-            from: new Address($box->address, $box->display_name ?: null),
-            replyTo: $box->no_reply ? [] : [new Address($box->address, $box->display_name ?: null)],
+            from: $from,
+            replyTo: $replyTo ? [$replyTo] : [],
             subject: $subject,
         );
     }
+
+    /**
+     * The From and Reply-To addresses for $purpose's mailbox — the ONE place
+     * that decides "no configured mailbox means no sender fields at all" and
+     * "no_reply means no Reply-To". Shared by every consumer of a purpose
+     * mailbox regardless of the shape it builds — an Envelope here (a
+     * Mailable), a MailMessage in CloudReady (a Notification, which cannot
+     * use mailboxEnvelope() directly since it is not building an Envelope).
+     * Two independent copies of this same decision is exactly the shape of
+     * gap Task 4 found and fixed in MailboxTransport::resolution().
+     *
+     * @return array{0: ?Address, 1: ?Address} [from, replyTo]
+     */
+    protected function mailboxAddresses(string $purpose): array
+    {
+        $box = app(MailboxResolver::class)->for($purpose);
+
+        if ($box === null) {
+            // No mailbox configured yet — both null, so a caller falls back
+            // to its own framework default rather than dereferencing null.
+            return [null, null];
+        }
+
+        $from = new Address($box->address, $box->display_name ?: null);
+
+        return [$from, $box->no_reply ? null : $from];
+    }
 }
diff --git a/app/Notifications/CloudReady.php b/app/Notifications/CloudReady.php
index 8d02112..8ab58c0 100644
--- a/app/Notifications/CloudReady.php
+++ b/app/Notifications/CloudReady.php
@@ -2,8 +2,8 @@
 
 namespace App\Notifications;
 
+use App\Mail\Concerns\SendsFromMailbox;
 use App\Models\Instance;
-use App\Services\Mail\MailboxResolver;
 use App\Services\Mail\MailPurpose;
 use Illuminate\Bus\Queueable;
 use Illuminate\Notifications\Messages\MailMessage;
@@ -22,7 +22,7 @@ use Illuminate\Support\Facades\Crypt;
  */
 class CloudReady extends Notification
 {
-    use Queueable;
+    use Queueable, SendsFromMailbox;
 
     public function __construct(
         public Instance $instance,
@@ -40,17 +40,16 @@ class CloudReady extends Notification
     {
         $url = 'https://'.$this->instance->subdomain.'.'.config('provisioning.dns.zone');
 
-        $box = app(MailboxResolver::class)
-            ->for(MailPurpose::PROVISIONING);
+        [$from, $replyTo] = $this->mailboxAddresses(MailPurpose::PROVISIONING);
 
         $message = (new MailMessage)->mailer('cp_'.MailPurpose::PROVISIONING);
 
-        if ($box !== null) {
-            $message->from($box->address, $box->display_name ?: null);
+        if ($from !== null) {
+            $message->from($from->address, $from->name);
+        }
 
-            if (! $box->no_reply) {
-                $message->replyTo($box->address, $box->display_name ?: null);
-            }
+        if ($replyTo !== null) {
+            $message->replyTo($replyTo->address, $replyTo->name);
         }
 
         return $message
diff --git a/tests/Feature/Mail/SenderAddressTest.php b/tests/Feature/Mail/SenderAddressTest.php
index 3bc1153..bf09e45 100644
--- a/tests/Feature/Mail/SenderAddressTest.php
+++ b/tests/Feature/Mail/SenderAddressTest.php
@@ -137,3 +137,22 @@ it('sets Reply-To on the provisioning notification when its mailbox allows repli
 
     expect($message->replyTo[0][0])->toBe('support@clupilot.com');
 });
+
+it("leaves the provisioning notification's sender fields at MailMessage's defaults when no mailbox is configured", function () {
+    // The mirror of the trait's own null-mailbox fallback test above, but for
+    // CloudReady's own guard: neither a mailbox row nor a provisioning/system
+    // Settings mapping exists, so MailboxResolver::for() returns null. Must
+    // not throw dereferencing a null mailbox, and must leave from/replyTo
+    // exactly as `new MailMessage` starts them — [] and [].
+    $notification = new CloudReady(
+        Instance::factory()->create(),
+        'admin',
+        Crypt::encryptString('geheim'),
+    );
+
+    $message = $notification->toMail(new stdClass);
+
+    expect($message->from)->toBeEmpty()
+        ->and($message->replyTo)->toBeEmpty()
+        ->and($message->mailer)->toBe('cp_provisioning');
+});