From b317b8dbb2243014784ae20ce1cbbc9ebbd9da36 Mon Sep 17 00:00:00 2001
From: Patrick Schrall <Schrall@schrall.com>
Date: Thu, 11 Jun 2026 17:37:16 +0200
Subject: [PATCH] Address Copilot review: normalize endpoints, quieter logs,
 robust tests

- resolveHandleToDid: rtrim trailing slash on the service URL before building
  the request URL and before the `=== $service` auth check (avoids double-slash
  URLs and mis-detected auth).
- Log per-endpoint resolution attempts at debug; emit a single warning only
  after all endpoints are exhausted (less noise during transient outages).
- Tests: fake resolveHandle with a wildcard so the case is isolated from the
  configured service URL; read request payload via $request->data().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 app/Services/Social/BlueskyPublisher.php      | 19 ++++++++++++++++++-
 .../Services/Social/BlueskyPublisherTest.php  |  9 ++++++---
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/app/Services/Social/BlueskyPublisher.php b/app/Services/Social/BlueskyPublisher.php
index abc4a04e..b3a7943e 100644
--- a/app/Services/Social/BlueskyPublisher.php
+++ b/app/Services/Social/BlueskyPublisher.php
@@ -278,12 +278,18 @@ private function parseFacets(string $text, ?string $service = null, ?SocialAccou
      */
     private function resolveHandleToDid(string $handle, ?string $service = null, ?SocialAccount $account = null): ?string
     {
+        // Normalize so a trailing slash neither produces a double-slash URL nor
+        // breaks the `=== $service` check below (which decides authentication).
+        $service = $service !== null ? rtrim($service, '/') : null;
+
         $endpoints = array_values(array_unique(array_filter([
             $service,
             'https://public.api.bsky.app',
             'https://bsky.social',
         ])));
 
+        $lastError = null;
+        $lastEndpoint = null;
         foreach ($endpoints as $endpoint) {
             try {
                 $request = $this->socialHttp();
@@ -301,7 +307,12 @@ private function resolveHandleToDid(string $handle, ?string $service = null, ?So
                     return $did;
                 }
             } catch (\Throwable $e) {
-                Log::warning('Bluesky handle resolution failed', [
+                // Per-endpoint failures are expected during transient outages;
+                // keep them at debug to avoid noisy logs and surface a single
+                // warning below only when every endpoint has been exhausted.
+                $lastError = $e->getMessage();
+                $lastEndpoint = $endpoint;
+                Log::debug('Bluesky handle resolution attempt failed', [
                     'handle' => $handle,
                     'endpoint' => $endpoint,
                     'error' => $e->getMessage(),
@@ -309,6 +320,12 @@ private function resolveHandleToDid(string $handle, ?string $service = null, ?So
             }
         }
 
+        Log::warning('Bluesky handle resolution failed; mention will be sent as plain text', [
+            'handle' => $handle,
+            'endpoint' => $lastEndpoint,
+            'error' => $lastError,
+        ]);
+
         return null;
     }
 
diff --git a/tests/Feature/Services/Social/BlueskyPublisherTest.php b/tests/Feature/Services/Social/BlueskyPublisherTest.php
index 71610c69..6ce6df4a 100644
--- a/tests/Feature/Services/Social/BlueskyPublisherTest.php
+++ b/tests/Feature/Services/Social/BlueskyPublisherTest.php
@@ -105,7 +105,10 @@
     $this->post->update(['content' => 'Shout out to @friend.bsky.social']);
 
     Http::fake([
-        'https://bsky.social/xrpc/com.atproto.identity.resolveHandle*' => Http::response([
+        // Wildcard so the fake matches whichever endpoint resolveHandleToDid()
+        // tries first (the account PDS, public AppView, or bsky.social) and the
+        // test stays isolated from the configured service URL.
+        '*/xrpc/com.atproto.identity.resolveHandle*' => Http::response([
             'did' => 'did:plc:friend456',
         ], 200),
         'https://bsky.social/xrpc/com.atproto.repo.createRecord' => Http::response([
@@ -117,7 +120,7 @@
     $this->publisher->publish($this->postPlatform);
 
     Http::assertSent(function ($request) {
-        $record = $request['record'] ?? null;
+        $record = $request->data()['record'] ?? null;
 
         if (! $record || ! isset($record['facets'])) {
             return false;
@@ -152,7 +155,7 @@
     expect($result['id'])->toBe('3abc123xyz');
 
     Http::assertSent(function ($request) {
-        $record = $request['record'] ?? null;
+        $record = $request->data()['record'] ?? null;
 
         if (! $record) {
             return false;