Cross-mailbox replies to outbound messages spawn duplicates (Follow-up to #5308 / #5350)

[teamrehab]

The Issue:
The patches for #5308 and #5350 successfully fixed cross-mailbox duplicates for standard incoming emails. However, if Mailbox A sends an outbound message to Mailbox B, and Mailbox B replies, Mailbox A fails to thread the reply and spawns a duplicate ticket.

Steps to Reproduce:

1. Agent in Mailbox A creates a new conversation and sends an outbound message to Mailbox B. (FreeScout sends this with a generated ID like FS_reply-12345-hash@domain.com).
2. Mailbox B fetches the email and creates a new ticket.
3. Agent in Mailbox B replies. The outbound reply contains In-Reply-To: fs-hash-of-FS_reply@domain.com.
4. Mailbox A fetches the reply.

Expected Outcome:
Mailbox A correctly identifies the original outbound thread and attaches the reply to the existing conversation.

Actual Outcome:
Mailbox A creates a brand new duplicate ticket.

Why it happens:
When Mailbox A fetches the reply, the logic correctly detects a mailbox mismatch and falls back to extracting the true Original-ID from the headers (extracting FS_reply-12345-hash@domain.com).

The fallback logic then performs a direct database lookup: Thread::where('message_id', $original_id).
However, for outbound messages (TYPE_MESSAGE), FreeScout computes the FS_reply IDs on the fly via Thread::getMessageId() and deliberately leaves the message_id column in the database blank. Because the column is blank, the database query fails to find the thread, and FreeScout gives up and creates a duplicate.

Untested Fix Suggestion:
In app/Console/Commands/FetchEmails.php, the fallback logic needs to check if the extracted $original_id is an outbound FreeScout ID. If it is, it should extract the Thread ID and look it up by primary key.

The else-branch has been expanded to search for both the literal ID and the FreeScout-hashed form (generateMessageId()), matching the existing dual-form lookup pattern used around line 688.

(Note: The code below deliberately replaces whereHas with a flat JOIN. There is currently a bug in FreeScout's Laravel 5.5 overrides at /overrides/laravel/framework/src/Illuminate/Database/Query/Builder.php on line 1243 where compact('type', 'operator', 'query', 'boolean') contains an undefined $operator variable. On PHP 8.x, triggering whereHas hits this bug and causes a fatal exception. Using JOIN bypasses the crash until Builder.php is patched).

   // Existing fallback extraction...
   $original_id = trim(\MailHelper::getHeader($prev_thread->headers, 'Message-ID'));
   $correct_thread = null;

   if ($original_id) {
       // Use formatMessageIdPrefix() for backward compat with legacy (no-FS_) IDs.
       // Narrow scope to outbound prefixes only to avoid interfering with notification flows.
       $outbound_prefixes = [
           $this->formatMessageIdPrefix(\MailHelper::MESSAGE_ID_PREFIX_REPLY_TO_CUSTOMER),
           $this->formatMessageIdPrefix(\MailHelper::MESSAGE_ID_PREFIX_AUTO_REPLY),
       ];
       $pattern = '/^(?:' . implode('|', $outbound_prefixes) . ')\-(\d+)\-([a-z0-9]+)\@/';
       preg_match($pattern, $original_id, $m);

       if (!empty($m[1]) && !empty($m[2])
           && strlen($m[2]) == 16
           && $m[2] === \MailHelper::getMessageIdHash($m[1])
       ) {
           // Outbound threads leave threads.message_id blank
           // (Thread::getMessageId() computes FS_reply on the fly);
           // look up by primary key instead using JOIN to avoid PHP 8.x Builder bug.
           $correct_thread = Thread::select('threads.*')
               ->join('conversations', 'conversations.id', '=', 'threads.conversation_id')
               ->where('threads.id', $m[1])
               ->where('conversations.mailbox_id', $mailbox->id)
               ->first();
       } else {
           // Standard lookup for normal inbound IDs.
           // Mirrors the dual-form lookup pattern used at line ~688
           // (literal ID + FreeScout-hashed form).
           $search_ids = [$original_id, \MailHelper::generateMessageId($original_id, $mailbox->id.$original_id)];
           $correct_thread = Thread::select('threads.*')
               ->join('conversations', 'conversations.id', '=', 'threads.conversation_id')
               ->whereIn('threads.message_id', $search_ids)
               ->where('conversations.mailbox_id', $mailbox->id)
               ->first();
       }
       
       if ($correct_thread) {
           $prev_thread = $correct_thread;
       } else {
           $prev_thread = null;
       }
   } else {
       $prev_thread = null;
   }

[freescout-help]

We can't reproduce the issue. Maybe some additional steps are needed.

On PHP 8.x, triggering whereHas hits this bug and causes a fatal exception. Using JOIN bypasses the crash until Builder.php is patched

It has been fixed long time ago: f4590b8

[teamrehab]

Sorry about bringing up the PHP 8.x thing. I just put that in there so somehow it wasn't reintroduced accidentally from any of my suggestions.

Let me know if the below helps? Thanks!

To reproduce this, you must use two separate FreeScout mailboxes on the same instance replying to each other.
If you reply from an external email client (like Gmail), the bug will not trigger because FreeScout takes a different code path for standard FS_reply-... headers.

Prerequisites:
Two mailboxes (Mailbox A and Mailbox B) configured in FreeScout that can send and receive emails to/from each
other.

Steps to Reproduce:

1. In Mailbox A, click "New Conversation". Set the customer to Mailbox B's email address and send a message
   ("Test 1").
2. Run php artisan freescout:fetch-emails.
3. Check Mailbox B. You should see a new incoming ticket for "Test 1".
4. In Mailbox B, open that ticket and reply back to Mailbox A ("Reply from B").
5. Run php artisan freescout:fetch-emails.
6. Check Mailbox A.

Expected:
Mailbox B's reply is threaded into the original conversation in Mailbox A.

Actual:
A brand new duplicate ticket is created in Mailbox A.

Why this specific sequence is required to trigger the bug:

1. When Mailbox A sends the message, it generates FS_reply-123-....
2. When Mailbox B fetches it, it hashes the ID to fs-{hash}@domain.com and saves it.
3. When Mailbox B replies, it sets In-Reply-To: fs-{hash}@domain.com.
4. When Mailbox A fetches the reply, isGeneratedMessageId($in_reply_to) evaluates to true (because it is an
   fs-{hash} ID, unlike FS_reply).
5. Mailbox A looks up the thread, sees it belongs to Mailbox B, and triggers the cross-mailbox fallback logic
   (introduced in Mutl mailbox replies creating new tickets. #5308).
6. The fallback logic extracts FS_reply-123-... from the headers and runs Thread::where('message_id',
   'FS_reply-123...')->first().
7. This query fails because Mailbox A's original outbound thread (TYPE_MESSAGE) has a blank/null message_id
   column in the database.

[freescout-help]

It already works fine for us. Maybe you have some custom adjustments in the code which are breaking it.