From 097f30c765390524c6bdaee83246476cb6be87e8 Mon Sep 17 00:00:00 2001 From: Paulo Castellano Date: Wed, 19 Aug 2026 09:52:54 -0300 Subject: [PATCH] Fix 500 on the MCP settings page when a client has never been used (#297) * Fix 500 on the MCP settings page when a client has never been used Sorting the connected clients used the raw last_used_at value with an int fallback, so a workspace holding both a used client (CarbonImmutable) and a freshly connected one (null, falling back to 0) handed arsort() a mix of object and int. PHP cannot compare the two and raises "Object of class Carbon\CarbonImmutable could not be converted to int", which Laravel's error handler turns into a 500. Sorting on the timestamp keeps the key a single type; never-used clients get 0 and land at the bottom, which is the intended order. The existing ordering test only covered clients that had all been used, which is why this shipped. The new test covers the mixed case. * Cover the all-never-used case on the MCP clients list --- .../AccessToken/ListConnectedMcpClients.php | 6 ++-- tests/Feature/McpSettingsControllerTest.php | 29 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/app/Actions/AccessToken/ListConnectedMcpClients.php b/app/Actions/AccessToken/ListConnectedMcpClients.php index 0e2922a1..26166974 100644 --- a/app/Actions/AccessToken/ListConnectedMcpClients.php +++ b/app/Actions/AccessToken/ListConnectedMcpClients.php @@ -7,6 +7,7 @@ use App\Models\AccessToken; use App\Models\User; use App\Models\Workspace; +use Carbon\CarbonInterface; use Illuminate\Support\Collection; class ListConnectedMcpClients @@ -16,7 +17,7 @@ class ListConnectedMcpClients * OAuth client. Matches API keys: each person only sees what they connected * for the workspace they are viewing. * - * @return list + * @return list */ public static function forUser(User $user, Workspace $workspace): array { @@ -41,7 +42,8 @@ public static function forUser(User $user, Workspace $workspace): array 'last_used_at' => $group->max('last_used_at'), ]; }) - ->sortByDesc(fn (array $client): mixed => $client['last_used_at'] ?? 0) + // Timestamp, not the date object: never-used clients are null. + ->sortByDesc(fn (array $client): int => $client['last_used_at']?->getTimestamp() ?? 0) ->values() ->all(); } diff --git a/tests/Feature/McpSettingsControllerTest.php b/tests/Feature/McpSettingsControllerTest.php index d4a8c38d..ca300d35 100644 --- a/tests/Feature/McpSettingsControllerTest.php +++ b/tests/Feature/McpSettingsControllerTest.php @@ -313,6 +313,35 @@ ->where('connectedClients.1.name', 'Older Agent')); }); +it('orders a never-used client last instead of crashing', function (): void { + $used = mcpAccessToken($this->user, mcpOauthClient('Used Agent'), $this->workspace); + $neverUsed = mcpAccessToken($this->user, mcpOauthClient('Fresh Agent'), $this->workspace); + + $used->forceFill(['last_used_at' => now()->subHour()])->saveQuietly(); + $neverUsed->forceFill(['last_used_at' => null])->saveQuietly(); + + $this->actingAs($this->user) + ->get(route('app.mcp.index')) + ->assertOk() + ->assertInertia(fn ($page) => $page + ->has('connectedClients', 2) + ->where('connectedClients.0.name', 'Used Agent') + ->where('connectedClients.1.name', 'Fresh Agent')); +}); + +it('lists clients that have never been used', function (): void { + $first = mcpAccessToken($this->user, mcpOauthClient('First Agent'), $this->workspace); + $second = mcpAccessToken($this->user, mcpOauthClient('Second Agent'), $this->workspace); + + $first->forceFill(['last_used_at' => null])->saveQuietly(); + $second->forceFill(['last_used_at' => null])->saveQuietly(); + + $this->actingAs($this->user) + ->get(route('app.mcp.index')) + ->assertOk() + ->assertInertia(fn ($page) => $page->has('connectedClients', 2)); +}); + it('does not list unbound mcp grants', function (): void { mcpAccessToken($this->user, mcpOauthClient('Legacy Agent'), workspace: null);