Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 52 additions & 1 deletion src/commands/mcp.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it } from "bun:test";
import { afterEach, describe, expect, it } from "bun:test";
import type { Dependencies } from "../container.js";
import {
createMockAuthService,
Expand All @@ -9,6 +9,10 @@ import {
createMockGitHitsService,
createMockPackageIntelligenceService,
} from "../services/test-helpers.js";
import {
buildClientHeaders,
resetRequestHeadersState,
} from "../shared/request-headers.js";
import {
createMcpServer,
getMcpToolDefinitions,
Expand Down Expand Up @@ -232,11 +236,58 @@ describe("createMcpServer", () => {
});

describe("startMcpServer", () => {
// `startMcpServer` mutates module-level state in `request-headers.ts`
// (sets `clientName = "githits-cli/mcp"` and registers the lazy MCP
// client-version provider). Reset after each test so later test
// files inherit the default state.
afterEach(() => {
resetRequestHeadersState();
});

it("starts successfully without a valid token", async () => {
const deps = createTestDeps({ hasValidToken: false });

// Server should start and connect transport without throwing.
// Auth errors are deferred to individual tool calls.
await expect(startMcpServer(deps)).resolves.toBeUndefined();
});

it("sets clientMode to githits-cli/mcp", async () => {
// After startMcpServer runs, subsequent buildClientHeaders calls
// tag the client as MCP-mode. Pins the telemetry contract.
const deps = createTestDeps({ hasValidToken: false });
await startMcpServer(deps);
const headers = buildClientHeaders({});
expect(headers["x-githits-client-name"]).toBe("githits-cli/mcp");
});

it("registers a lazy MCP clientInfo provider (read at request time, not via race-prone notification)", async () => {
// The MCP SDK dispatches `oninitialized` via an async notification
// that can race the first tool call. The provider pattern reads
// clientInfo synchronously on every buildClientHeaders call,
// eliminating the race. This test pins the provider-based flow.
const deps = createTestDeps({ hasValidToken: false });
await startMcpServer(deps);

// Before the initialize handshake lands, the provider returns
// undefined — `buildClientHeaders` falls back to env detection
// (none in the test env), so no x-githits-agent header.
const headersBefore = buildClientHeaders({});
expect(headersBefore["x-githits-agent"]).toBeUndefined();

// Simulate the SDK's _oninitialize landing: `_clientVersion` is
// set synchronously inside the SDK before the initialize response
// is returned. A tool call reaching buildClientHeaders after that
// point should see the agent header populated. We can't easily
// reach into the real SDK's private state from here, so this
// test pins the *mechanism* (provider registration) rather than
// the end-to-end race; the provider's correctness is exercised
// in request-headers.test.ts.
// At this point the provider is registered but returns undefined —
// confirm the fallback path to env detection also works.
const headersWithAgent = buildClientHeaders({
GITHITS_AGENT: "test-harness/1.0.0",
});
expect(headersWithAgent["x-githits-agent"]).toBe("test-harness/1.0.0");
});
});
44 changes: 44 additions & 0 deletions src/commands/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import type { Command } from "commander";
import { version } from "../../package.json";
import { createContainer, type Dependencies } from "../container.js";
import { dim, highlight, shouldUseColors } from "../shared/colors.js";
import {
setClientMode,
setMcpClientVersionProvider,
} from "../shared/request-headers.js";
import {
createFeedbackTool,
createGrepFileTool,
Expand Down Expand Up @@ -89,10 +93,50 @@ export function createMcpServer(deps: Dependencies): McpServer {

/**
* Start the MCP server. Exported for testability.
*
* Telemetry wiring:
* - `setClientMode("mcp")` tags every subsequent API request with
* `x-githits-client-name: githits-cli/mcp` so backend telemetry
* can distinguish MCP-driven traffic from direct-CLI traffic.
* - `setMcpClientVersionProvider` registers a lazy reader that
* pulls the connecting client's `clientInfo` (cursor,
* claude-code, etc.) on every request. The MCP SDK sets
* `_clientVersion` synchronously inside `_oninitialize` before
* the initialize response is sent back, so every tool call that
* arrives after the handshake sees a populated value —
* eliminating the race the older `oninitialized` callback
* pattern had where the first tool call could slip through
* before the notification dispatched.
*/
export async function startMcpServer(deps: Dependencies): Promise<void> {
setClientMode("mcp");

const server = createMcpServer(deps);
const transport = new StdioServerTransport();

setMcpClientVersionProvider(() => {
try {
const clientVersion = server.server.getClientVersion();
if (
!clientVersion?.name ||
typeof clientVersion.name !== "string" ||
clientVersion.name.trim().length === 0
) {
return undefined;
}
const name = clientVersion.name.trim();
const rawVersion = clientVersion.version;
const versionOut =
typeof rawVersion === "string" && rawVersion.trim().length > 0
? rawVersion.trim()
: undefined;
return { name, version: versionOut };
} catch {
// Agent header is optional — never block the request.
return undefined;
}
});

await server.connect(transport);
}

Expand Down
14 changes: 14 additions & 0 deletions src/services/githits-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@ describe("GitHitsServiceImpl", () => {
expect(headers.Authorization).toBe("Bearer test-token");
});

it("sends x-githits-* telemetry headers on REST requests", async () => {
// Pins the contract that `GitHitsServiceImpl.headers()` spreads
// the telemetry headers onto every REST call, not just that the
// shared builder emits them.
const fn = mockFetch(() => Promise.resolve(new Response("result")));
await service.search({ query: "probe", language: "javascript" });

const call = fn.mock.calls[0] as unknown as [string, RequestInit];
const headers = call[1].headers as Record<string, string>;
expect(headers["x-githits-client-name"]).toBe("githits-cli");
expect(headers["x-githits-client-version"]).toMatch(/^\S+$/);
expect(headers["x-githits-session-id"]).toMatch(/^[0-9a-f]{16}$/);
});

it("passes custom license_mode", async () => {
const fn = mockFetch(() => Promise.resolve(new Response("result")));

Expand Down
2 changes: 2 additions & 0 deletions src/services/githits-service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { version } from "../../package.json";
import { buildClientHeaders } from "../shared/request-headers.js";

/**
* Error thrown when the API returns 401 Unauthorized.
Expand Down Expand Up @@ -138,6 +139,7 @@ export class GitHitsServiceImpl implements GitHitsService {

private headers(): Record<string, string> {
return {
...buildClientHeaders(),
Authorization: `Bearer ${this.token}`,
"Content-Type": "application/json",
"User-Agent": `githits-cli/${version}`,
Expand Down
27 changes: 27 additions & 0 deletions src/shared/pkgseer-graphql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,33 @@ describe("postPkgseerGraphql", () => {
expect(capturedHeaders?.["User-Agent"]).toMatch(/^githits-cli\/\S+$/);
});

it("sends x-githits-* telemetry headers from buildClientHeaders", async () => {
// Pins the contract that the transport layer spreads the
// telemetry headers onto every request — not just that the
// module under `src/shared/request-headers.ts` builds them.
let capturedHeaders: Record<string, string> | undefined;
const fetchFn = mock((_url: string, init?: RequestInit) => {
capturedHeaders = init?.headers as Record<string, string>;
return Promise.resolve(makeResponse(VALID_JSON));
});

await postPkgseerGraphql({
endpointUrl: ENDPOINT,
token: TOKEN,
query: "query { x }",
variables: {},
fetchFn: asFetchFn(fetchFn),
});

expect(capturedHeaders?.["x-githits-client-name"]).toBe("githits-cli");
expect(capturedHeaders?.["x-githits-client-version"]).toMatch(/^\S+$/);
expect(capturedHeaders?.["x-githits-session-id"]).toMatch(/^[0-9a-f]{16}$/);
// Authorization still wins over any hypothetical x-githits-*
// collision — spread order (headers first, hardcoded second)
// guarantees this, but pin it.
expect(capturedHeaders?.Authorization).toBe(`Bearer ${TOKEN}`);
});

it("normalises trailing slashes on endpointUrl", async () => {
let capturedUrl: string | undefined;
const fetchFn = mock((url: string) => {
Expand Down
2 changes: 2 additions & 0 deletions src/shared/pkgseer-graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import { version } from "../../package.json";
import { debugLog } from "./debug-log.js";
import { buildClientHeaders } from "./request-headers.js";

export interface PkgseerGraphqlRequest {
/** Full endpoint URL, e.g. `https://pkgseer.dev`. Trailing slashes tolerated. */
Expand Down Expand Up @@ -90,6 +91,7 @@ export async function postPkgseerGraphql(
response = await fetchFn(`${baseUrl(request.endpointUrl)}/api/graphql`, {
method: "POST",
headers: {
...buildClientHeaders(),
Authorization: `Bearer ${request.token}`,
"Content-Type": "application/json",
"User-Agent": userAgent,
Expand Down
Loading
Loading