From 2c924b369a1d352cb88310843cde6adfe44013f4 Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Mon, 5 Oct 2026 18:46:52 +0200 Subject: [PATCH] fix(mcp): avoid SDK spinner in organizations --- lib/organizations.mts | 90 +++++++++++++--------- test/repo/integration/http-server.test.mts | 33 ++++++++ test/repo/unit/organizations.test.mts | 77 +++++++++--------- 3 files changed, 126 insertions(+), 74 deletions(-) diff --git a/lib/organizations.mts b/lib/organizations.mts index a3ffa61d..60d18872 100644 --- a/lib/organizations.mts +++ b/lib/organizations.mts @@ -1,53 +1,69 @@ -import { SocketSdk } from '@socketsecurity/sdk' +import { httpRequest } from '@socketsecurity/lib/http-request/request' +import { HttpResponseError } from '@socketsecurity/lib/http-request/response-types' + +const MAX_RESPONSE_SIZE = 10 * 1024 * 1024 export interface FetchOrganizationsConfig { baseUrl: string userAgent?: string | undefined - // Socket access token. The SDK sends it as HTTP Basic auth (token as the - // username, empty password). + // Socket access token. HTTP Basic uses it as the username. authToken?: string | undefined } -// The failure fields the SDK reports on a non-2xx `listOrganizations()`. -export interface OrganizationsFailure { - cause?: string | undefined - error: string - status: number -} - -// Render an SDK failure into the message `fetchOrganizations` throws. The SDK -// reports `cause` only for some failures, so it is appended in parentheses -// when present and omitted otherwise. -export function buildOrganizationsErrorMessage( - failure: OrganizationsFailure, -): string { - return `organizations endpoint ${failure.status}: ${failure.error}${ - failure.cause ? ` (${failure.cause})` : '' - }` -} - /** - * Fetch the organizations the authenticated user belongs to via the Socket - * SDK's `listOrganizations()` (`GET /v0/organizations`). Returns the parsed - * JSON body untouched — downstream callers decide how to render it. Throws with - * the SDK-reported status + error on a non-2xx response. - * - * The SDK's `baseUrl` already carries the `/v0/` path segment, so the caller's - * `baseUrl` (the bare API origin) gets `/v0/` appended. + * Fetch the organizations the authenticated user belongs to from + * `GET /v0/organizations`. Returns the parsed JSON body untouched. */ export async function fetchOrganizations( config: FetchOrganizationsConfig, ): Promise { config = { __proto__: null, ...config } as typeof config - const baseUrl = `${config.baseUrl.replace(/\/$/u, '')}/v0/` - const sdk = new SocketSdk(config.authToken ?? '', { - baseUrl, - ...(config.userAgent ? { userAgent: config.userAgent } : {}), - }) + const token = config.authToken?.trim() + if (!token) { + throw new Error('Socket API token is required for organizations') + } + const url = `${config.baseUrl.replace(/\/$/u, '')}/v0/organizations` + try { + const res = await httpRequest(url, { + headers: { + accept: 'application/json', + authorization: `Basic ${Buffer.from(`${token}:`).toString('base64')}`, + ...(config.userAgent ? { 'user-agent': config.userAgent } : {}), + }, + maxResponseSize: MAX_RESPONSE_SIZE, + onRetry: (_attempt, error, delay) => retryDelay(error, delay), + retries: 3, + throwOnError: true, + }) + const body = res.text() + return body === '' ? {} : JSON.parse(body) + } catch (error) { + if (error instanceof HttpResponseError) { + throw new Error( + `organizations endpoint ${error.response.status}: ${error.response.text()}`, + ) + } + throw error + } +} - const result = await sdk.listOrganizations() - if (!result.success) { - throw new Error(buildOrganizationsErrorMessage(result)) +export function retryDelay(error: unknown, delay: number): boolean | number { + if (!(error instanceof HttpResponseError)) { + return delay + } + const { status, headers } = error.response + if (status >= 400 && status < 500 && status !== 429) { + return false + } + const retryAfter = headers['retry-after'] + if (status !== 429 || !retryAfter) { + return delay + } + const value = Array.isArray(retryAfter) ? retryAfter[0] : retryAfter + const seconds = Number(value) + if (Number.isFinite(seconds) && seconds >= 0) { + return seconds * 1000 } - return result.data + const dateDelay = Date.parse(value) - Date.now() + return dateDelay > 0 ? dateDelay : delay } diff --git a/test/repo/integration/http-server.test.mts b/test/repo/integration/http-server.test.mts index 86799507..cdc6fcb6 100644 --- a/test/repo/integration/http-server.test.mts +++ b/test/repo/integration/http-server.test.mts @@ -168,6 +168,17 @@ describe('stateless serving', () => { expect(message.result.tools.map(t => t.name)).toContain('depscore') }) + test('serves a modern request carrying a stale session id', async () => { + const res = await postModern('tools/list', undefined, { + 'mcp-session-id': 'stale-session', + }) + expect(res.status).toBe(200) + expect(res.headers['mcp-session-id']).toBeUndefined() + expect( + res.json().result.tools.map(t => t.name), + ).toContain('depscore') + }) + test('stamps the configured cache metadata onto a 2026-era tools/list', async () => { const message = ( await postModern('tools/list') @@ -240,6 +251,28 @@ describe('2025-era compatibility', () => { }) }) + test('serves a legacy initialize carrying a stale session id', async () => { + const res = await postJson( + { + jsonrpc: '2.0', + id: 3, + method: 'initialize', + params: { + protocolVersion: LEGACY_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: 'legacy', version: '0.0.0' }, + }, + }, + { 'mcp-session-id': 'stale-session' }, + ) + expect(res.status).toBe(200) + expect(res.headers['mcp-session-id']).toBeUndefined() + expect(parseSseData(res.text())).toMatchObject({ + id: 3, + result: { protocolVersion: LEGACY_PROTOCOL_VERSION }, + }) + }) + test('serves a bare tools/list with no handshake at all', async () => { const res = await postJson({ jsonrpc: '2.0', id: 2, method: 'tools/list' }) expect(res.status).toBe(200) diff --git a/test/repo/unit/organizations.test.mts b/test/repo/unit/organizations.test.mts index 1a14959d..a3fc18a3 100644 --- a/test/repo/unit/organizations.test.mts +++ b/test/repo/unit/organizations.test.mts @@ -1,10 +1,7 @@ import nock from 'nock' import { afterEach, beforeEach, describe, expect, test } from 'vitest' -import { - buildOrganizationsErrorMessage, - fetchOrganizations, -} from '../../../lib/organizations.mts' +import { fetchOrganizations } from '../../../lib/organizations.mts' const API = 'https://api.socket.dev' @@ -19,8 +16,7 @@ afterEach(() => { describe('fetchOrganizations', () => { test('GETs /v0/organizations with Basic auth + returns the body', async () => { - // The Socket SDK authenticates with HTTP Basic (token as the username, - // empty password) — base64('tok:') === 'dG9rOg=='. + // HTTP Basic uses the token as the username and an empty password. nock(API) .matchHeader('authorization', 'Basic dG9rOg==') .get('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/v0/organizations') @@ -42,43 +38,50 @@ describe('fetchOrganizations', () => { fetchOrganizations({ baseUrl: API, authToken: 'tok' }), ).rejects.toThrow(/organizations endpoint 401/) }) -}) -describe('fetchOrganizations auth and error shape', () => { - test('refuses to build an SDK client with no token', async () => { - // `authToken ?? ''` hands the SDK an empty credential, and the SDK - // refuses it up front rather than sending an unauthenticated request. - await expect(fetchOrganizations({ baseUrl: API })).rejects.toThrow( - '"apiToken" cannot be empty or whitespace-only', - ) - expect(nock.pendingMocks()).toEqual([]) + test('retries a transient response and respects Retry-After', async () => { + const scope = nock(API) + .get('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/v0/organizations') + .reply(429, { error: 'rate limited' }, { 'retry-after': '0' }) + .get('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/v0/organizations') + .reply(200, { organizations: {} }) + await expect( + fetchOrganizations({ baseUrl: API, authToken: 'tok' }), + ).resolves.toEqual({ organizations: {} }) + expect(scope.isDone()).toBe(true) }) -}) -describe('buildOrganizationsErrorMessage', () => { - test('appends the SDK cause in parentheses when it is reported', () => { - expect( - buildOrganizationsErrorMessage({ - cause: 'token expired', - error: 'Unauthorized', - status: 401, - }), - ).toBe('organizations endpoint 401: Unauthorized (token expired)') + test('does not retry a non-rate-limit client error', async () => { + const scope = nock(API).get('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/v0/organizations').reply(403, 'forbidden') + await expect( + fetchOrganizations({ baseUrl: API, authToken: 'tok' }), + ).rejects.toThrow('organizations endpoint 403: forbidden') + expect(scope.isDone()).toBe(true) }) - test('omits the parenthetical when the SDK reports no cause', () => { - expect( - buildOrganizationsErrorMessage({ error: 'Server Error', status: 500 }), - ).toBe('organizations endpoint 500: Server Error') + test('treats an empty successful body as an empty object', async () => { + nock(API).get('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/v0/organizations').reply(200) + await expect( + fetchOrganizations({ baseUrl: API, authToken: 'tok' }), + ).resolves.toEqual({}) }) - test('omits the parenthetical for an empty cause', () => { - expect( - buildOrganizationsErrorMessage({ - cause: '', - error: 'Server Error', - status: 500, - }), - ).toBe('organizations endpoint 500: Server Error') + test('limits the response body to 10 MiB', async () => { + nock(API) + .get('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/v0/organizations') + .times(4) + .reply(200, 'x'.repeat(10 * 1024 * 1024 + 1)) + await expect( + fetchOrganizations({ baseUrl: API, authToken: 'tok' }), + ).rejects.toThrow(/maxResponseSize|response size|exceeds/iu) + }) +}) + +describe('fetchOrganizations auth and error shape', () => { + test('refuses to send a request with no token', async () => { + await expect(fetchOrganizations({ baseUrl: API })).rejects.toThrow( + 'Socket API token is required for organizations', + ) + expect(nock.pendingMocks()).toEqual([]) }) })