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
This commit is contained in:
Paulo Castellano 2026-08-19 09:52:54 -03:00 committed by GitHub
parent 0eff22dc0d
commit 097f30c765
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 33 additions and 2 deletions

View file

@ -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<array{client_id: string, name: string, can_disconnect: bool, last_used_at: mixed}>
* @return list<array{client_id: string, name: string, can_disconnect: bool, last_used_at: ?CarbonInterface}>
*/
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();
}

View file

@ -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);