From 1387ce1c1d1894ca70597180966f591da11d8a2f Mon Sep 17 00:00:00 2001 From: AHMET BAYHAN BAYRAMOGLU <49499275+ABB65@users.noreply.github.com> Date: Mon, 13 Jul 2026 01:59:52 +0300 Subject: [PATCH] feat(mcp): capability-aware tool listing, instructions, openWorldHint, session tenant binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the MCP surface honest per session and harden multi-tenant HTTP: - Capability-aware registration: createServer consults a new declarative TOOL_REQUIREMENTS map (exported with isToolAvailable from @contentrain/mcp/tools/availability, new subpath export) and skips tools the resolved provider + projectRoot pair can never satisfy, via a CapabilityFilteredMcpServer subclass — zero changes to the 8 register functions. Local stdio/CLI flows keep all 19 tools; remote-provider sessions now list only the 9 remote-safe tools instead of advertising 10 that always failed with a capability error. Input-dependent checks (validate --fix, apply reuse) stay as call-time guards. - instructions: CreateServerOptions.instructions threads the MCP instructions string to clients at initialize; defaults to a new DEFAULT_INSTRUCTIONS (<512 chars, describe-format-first + dry-run-first operating rules); '' omits. - openWorldHint: false on all 19 annotations — tools operate on the configured repository only. - sessionFingerprint on multi-tenant HTTP: fingerprint captured at session creation must match every follow-up request with that Mcp-Session-Id; mismatch answers 404 so the client re-initializes against its own provider. Closes cross-tenant session-id replay. Tests: new tests/server/tool-availability.test.ts (15 cases: local vs remote listing, partial capabilities, requirements sanity, instructions default/override/omit, openWorldHint on wire); cross-tenant replay test in http.test.ts; doctor/submit remote tests updated to the new not-advertised behavior. Full suite green in clean runs (667 tests across all 50 files); skills 85/85 + rules 16/16 parity pass; oxlint 0/0; tsc clean; tsdown build includes the new tools/availability entry. --- .changeset/mcp-honest-surface.md | 12 ++ docs/guides/embedding-mcp.md | 19 +- docs/guides/http-transport.md | 4 + docs/packages/mcp.md | 6 +- packages/mcp/README.md | 6 +- packages/mcp/package.json | 8 +- packages/mcp/src/server.ts | 94 ++++++++-- packages/mcp/src/server/http/server.ts | 24 ++- packages/mcp/src/tools/annotations.ts | 22 +++ packages/mcp/src/tools/availability.ts | 55 ++++++ packages/mcp/tests/server/http.test.ts | 73 +++++++- .../tests/server/tool-availability.test.ts | 162 ++++++++++++++++++ packages/mcp/tests/tools/doctor.test.ts | 10 +- 13 files changed, 461 insertions(+), 34 deletions(-) create mode 100644 .changeset/mcp-honest-surface.md create mode 100644 packages/mcp/src/tools/availability.ts create mode 100644 packages/mcp/tests/server/tool-availability.test.ts diff --git a/.changeset/mcp-honest-surface.md b/.changeset/mcp-honest-surface.md new file mode 100644 index 0000000..d0a8417 --- /dev/null +++ b/.changeset/mcp-honest-surface.md @@ -0,0 +1,12 @@ +--- +"@contentrain/mcp": minor +--- + +feat(mcp): capability-aware tool listing, server instructions, openWorldHint, session tenant binding + +**@contentrain/mcp**: the MCP surface now tells the truth about what it can do, per session. + +- **Capability-aware registration.** `createServer` consults a new declarative requirement map (`TOOL_REQUIREMENTS`, exported from `@contentrain/mcp/tools/availability` together with `isToolAvailable`) and only registers tools the resolved provider + `projectRoot` pair can satisfy. Local stdio/CLI flows keep the full 19-tool surface; remote-provider sessions (Studio MCP Cloud, GitHub/GitLab providers) now list only the remote-safe subset instead of advertising tools that always failed with a capability error. Input-dependent checks (`validate --fix`, `apply` reuse) remain call-time guards. +- **`instructions` support.** `CreateServerOptions.instructions` threads the MCP `instructions` string to clients at `initialize`. Defaults to a new `DEFAULT_INSTRUCTIONS` (< 512 chars, describes the describe-format-first and dry-run-first operating rules); pass `''` to omit. +- **`openWorldHint: false`** added to all 19 tool annotations — every tool operates on the configured repository only. +- **Session tenant binding.** Multi-tenant HTTP mode accepts `sessionFingerprint(req)`: the fingerprint captured at session creation must match on every follow-up request carrying that `Mcp-Session-Id`; a mismatch answers `404 Session not found` so the client re-initializes against its own provider. Closes cross-tenant session-id replay. diff --git a/docs/guides/embedding-mcp.md b/docs/guides/embedding-mcp.md index f4eb35e..9c3e230 100644 --- a/docs/guides/embedding-mcp.md +++ b/docs/guides/embedding-mcp.md @@ -169,6 +169,9 @@ const handle = await startHttpMcpServerWith({ const { repo, auth } = await lookupProjectFromDatabase(projectId) return createGitHubProvider({ auth, repo }) }, + // Bind each session to its tenant: follow-up requests must produce the + // same fingerprint or they get 404 and the client re-initializes. + sessionFingerprint: (req) => req.headers['x-project-id'] as string, authToken: workspaceBearerToken, port: 3333, sessionTtlMs: 15 * 60 * 1000, @@ -177,6 +180,8 @@ const handle = await startHttpMcpServerWith({ The single-provider shape (`{ provider }`) and the resolver shape (`{ resolveProvider }`) are mutually exclusive — pass one or the other. +**Session tenant binding.** A session's provider is resolved once, at `initialize`. Without `sessionFingerprint`, any caller that presents a known `Mcp-Session-Id` reaches that session's provider — fine on trusted loopback, not across tenants. Set `sessionFingerprint` to derive a stable tenant identity from each request (e.g. the same headers `resolveProvider` uses); a mismatch answers `404 Session not found`, which per the Streamable HTTP spec makes the client transparently re-initialize its own session. + ### 4. Programmatic tool calls (no transport at all) If you want to run a Contentrain tool inside your own Node.js process without MCP's JSON-RPC layer: @@ -234,17 +239,17 @@ This is only needed for remote / reader-based flows. `LocalProvider`'s transacti ### `capability_required` is a structured error -Tools that need capabilities the active provider doesn't expose return: +Tools whose requirements can never be met by the session's provider are not registered at all (see [Capability gating](#capability-gating)), so most capability mismatches never reach a handler. The structured error remains for the two input-dependent cases — `validate` with `fix: true` and `apply` in `reuse` mode: ```json { - "error": "contentrain_scan requires local filesystem access.", - "capability_required": "astScan", + "error": "contentrain_validate requires local filesystem access.", + "capability_required": "localWorktree", "hint": "This tool is unavailable when MCP is driven by a remote provider. Use a LocalProvider or the stdio transport." } ``` -Treat `capability_required` as a retry signal at the client. Typical fallback: prompt the user to switch to a local checkout, or downgrade the request (e.g. `content_list` with `resolve: true` → `resolve: false`). +Treat `capability_required` as a retry signal at the client. Typical fallback: prompt the user to switch to a local checkout, or downgrade the request (e.g. `validate` without `fix`). See [Providers & Transports](/guides/providers) for the full capability matrix. @@ -258,11 +263,11 @@ Rotate Bearer tokens regularly. MCP does not support per-tool ACLs; a valid toke ## Capability gating -Each provider advertises a `ProviderCapabilities` manifest. Tools gate on capabilities and reject uniformly when the active provider can't satisfy them. +Each provider advertises a `ProviderCapabilities` manifest. `createServer` consults it (together with `projectRoot`) at registration time: tools whose requirements can never be met by the session's provider are **not registered**, so `tools/list` only shows what can actually run. The declarative requirement map is exported as `TOOL_REQUIREMENTS` from `@contentrain/mcp/tools/availability`, alongside an `isToolAvailable(name, provider, projectRoot)` helper for embedders that want to reason about the effective surface without spinning up a server. | Capability | Local | GitHub | GitLab | Gated tools | |---|---|---|---|---| -| `localWorktree` | ✓ | — | — | `init`, `scaffold`, `validate --fix`, `submit`, `merge`, `bulk` | +| `localWorktree` | ✓ | — | — | `validate --fix`, `submit`, `merge`, `branch_list`, `branch_delete` | | `sourceRead` | ✓ | — | — | `apply` (extract) | | `sourceWrite` | ✓ | — | — | `apply` (reuse) | | `astScan` | ✓ | — | — | `scan` | @@ -270,6 +275,8 @@ Each provider advertises a `ProviderCapabilities` manifest. Tools gate on capabi | `branchProtection` | — | ✓ | ✓ | merge fallback | | `pullRequestFallback` | — | ✓ | ✓ | merge fallback | +`init`, `scaffold`, `doctor`, and `bulk` additionally require a local `projectRoot` on disk. Input-dependent checks (`validate --fix`, `apply` reuse) stay as call-time guards and return the structured `capability_required` error above. + Read-only tools (`status`, `describe`, `describe_format`, `content_list`, `validate` without `--fix`) work on every provider — they use only the reader surface. ## Extension: custom providers diff --git a/docs/guides/http-transport.md b/docs/guides/http-transport.md index 5bb857e..711b97f 100644 --- a/docs/guides/http-transport.md +++ b/docs/guides/http-transport.md @@ -73,6 +73,10 @@ const handle = await startHttpMcpServerWith({ The same pattern works for `createGitLabProvider` with a `GitLabProvider`. Both require their respective optional peers (`@octokit/rest`, `@gitbeaker/rest`). +Multi-tenant deployments (`{ resolveProvider }`) should also set `sessionFingerprint` to bind each MCP session to the tenant it was created for — follow-up requests whose fingerprint doesn't match the session's get `404` and the client re-initializes. See the [embedding guide](/guides/embedding-mcp#3a-http--per-request-provider-resolver-multi-tenant) for the full pattern. + +Note that the tool list is capability-aware: a session backed by a remote provider only advertises the tools it can actually run (no `init`/`scaffold`/`doctor`/`bulk`/`submit`/`merge`/branch lifecycle/normalize tools without a local worktree). + ## Deployment patterns ### Studio (hosted agent) diff --git a/docs/packages/mcp.md b/docs/packages/mcp.md index 16fb14a..a9cdd75 100644 --- a/docs/packages/mcp.md +++ b/docs/packages/mcp.md @@ -43,7 +43,11 @@ Optional parser support for higher-quality source scanning: ## Tool Catalog -The MCP server exposes **19 tools** organized by function. Each tool includes [MCP annotations](https://spec.modelcontextprotocol.io/specification/2025-03-26/server/tools/#annotations) (`readOnlyHint`, `destructiveHint`, `idempotentHint`) so clients can distinguish safe reads from writes and destructive operations. +The MCP server exposes **19 tools** organized by function. Each tool includes [MCP annotations](https://spec.modelcontextprotocol.io/specification/2025-03-26/server/tools/#annotations) (`readOnlyHint`, `destructiveHint`, `idempotentHint`, and `openWorldHint: false` — every tool operates on the configured repository only) so clients can distinguish safe reads from writes and destructive operations. + +::: info Capability-aware listing +`tools/list` is filtered per session: tools whose requirements (local project root, provider capabilities) cannot be met are not registered at all. A local stdio server lists all 19 tools; a remote-provider session (e.g. Studio MCP Cloud) lists only the remote-safe subset — `status`, `describe`, `describe_format`, `model_save`, `model_delete`, `content_save`, `content_delete`, `content_list`, `validate`. See `TOOL_REQUIREMENTS` in `@contentrain/mcp/tools/availability`. +::: | Tool | Title | Read-only | Destructive | |------|-------|-----------|-------------| diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 8305a55..392f52e 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -71,7 +71,9 @@ All write operations are designed around git-backed safety: ## Tool Surface -19 MCP tools with [annotations](https://spec.modelcontextprotocol.io/specification/2025-03-26/server/tools/#annotations) (`readOnlyHint`, `destructiveHint`, `idempotentHint`) for client safety hints: +19 MCP tools with [annotations](https://spec.modelcontextprotocol.io/specification/2025-03-26/server/tools/#annotations) (`readOnlyHint`, `destructiveHint`, `idempotentHint`, and `openWorldHint: false` — every tool operates on the configured repository only) for client safety hints. + +**Tool listing is capability-aware.** `tools/list` only advertises tools the resolved provider + `projectRoot` pair can actually satisfy. A local stdio server lists all 19; a session driven by a remote provider (GitHub/GitLab, no local checkout) lists only the remote-safe subset — `status`, `describe`, `describe_format`, `model_save`, `model_delete`, `content_save`, `content_delete`, `content_list`, `validate`. The requirement map lives in `TOOL_REQUIREMENTS` (`@contentrain/mcp/tools/availability`). | Tool | Purpose | Read-only | Destructive | | --- | --- | --- | --- | @@ -125,6 +127,8 @@ const transport = new StdioServerTransport() await server.connect(transport) ``` +`createServer` also accepts an options object: `{ provider, projectRoot?, instructions? }`. `instructions` sets the MCP `instructions` string clients receive at `initialize` (defaults to a built-in `DEFAULT_INSTRUCTIONS`, kept under 512 characters; pass `''` to omit). + ## Example MCP Flow Typical agent workflow: diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 89a8aa4..f09380b 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -120,6 +120,10 @@ "types": "./dist/tools/annotations.d.mts", "import": "./dist/tools/annotations.mjs" }, + "./tools/availability": { + "types": "./dist/tools/availability.d.mts", + "import": "./dist/tools/availability.mjs" + }, "./testing/conformance": { "types": "./dist/testing/conformance.d.mts", "import": "./dist/testing/conformance.mjs" @@ -148,8 +152,8 @@ "testing" ], "scripts": { - "build": "tsdown src/index.ts src/server.ts src/core/config.ts src/core/context.ts src/core/model-manager.ts src/core/content-manager.ts src/core/meta-manager.ts src/core/validator/index.ts src/core/scanner.ts src/core/scan-config.ts src/core/doctor.ts src/core/graph-builder.ts src/core/apply-manager.ts src/util/detect.ts src/util/fs.ts src/util/id.ts src/git/transaction.ts src/git/branch-lifecycle.ts src/tools/annotations.ts src/testing/conformance.ts src/templates/index.ts src/core/contracts/index.ts src/core/ops/index.ts src/core/overlay-reader.ts src/providers/local/index.ts src/providers/github/index.ts src/providers/gitlab/index.ts src/server/http/index.ts --format esm --dts --external typescript", - "dev": "tsdown src/index.ts src/server.ts src/core/config.ts src/core/context.ts src/core/model-manager.ts src/core/content-manager.ts src/core/meta-manager.ts src/core/validator/index.ts src/core/scanner.ts src/core/scan-config.ts src/core/doctor.ts src/core/graph-builder.ts src/core/apply-manager.ts src/util/detect.ts src/util/fs.ts src/util/id.ts src/git/transaction.ts src/git/branch-lifecycle.ts src/tools/annotations.ts src/testing/conformance.ts src/templates/index.ts src/core/contracts/index.ts src/core/ops/index.ts src/core/overlay-reader.ts src/providers/local/index.ts src/providers/github/index.ts src/providers/gitlab/index.ts src/server/http/index.ts --format esm --dts --external typescript --watch", + "build": "tsdown src/index.ts src/server.ts src/core/config.ts src/core/context.ts src/core/model-manager.ts src/core/content-manager.ts src/core/meta-manager.ts src/core/validator/index.ts src/core/scanner.ts src/core/scan-config.ts src/core/doctor.ts src/core/graph-builder.ts src/core/apply-manager.ts src/util/detect.ts src/util/fs.ts src/util/id.ts src/git/transaction.ts src/git/branch-lifecycle.ts src/tools/annotations.ts src/tools/availability.ts src/testing/conformance.ts src/templates/index.ts src/core/contracts/index.ts src/core/ops/index.ts src/core/overlay-reader.ts src/providers/local/index.ts src/providers/github/index.ts src/providers/gitlab/index.ts src/server/http/index.ts --format esm --dts --external typescript", + "dev": "tsdown src/index.ts src/server.ts src/core/config.ts src/core/context.ts src/core/model-manager.ts src/core/content-manager.ts src/core/meta-manager.ts src/core/validator/index.ts src/core/scanner.ts src/core/scan-config.ts src/core/doctor.ts src/core/graph-builder.ts src/core/apply-manager.ts src/util/detect.ts src/util/fs.ts src/util/id.ts src/git/transaction.ts src/git/branch-lifecycle.ts src/tools/annotations.ts src/tools/availability.ts src/testing/conformance.ts src/templates/index.ts src/core/contracts/index.ts src/core/ops/index.ts src/core/overlay-reader.ts src/providers/local/index.ts src/providers/github/index.ts src/providers/gitlab/index.ts src/server/http/index.ts --format esm --dts --external typescript --watch", "test": "vitest run", "typecheck": "tsc --noEmit", "clean": "rm -rf dist" diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 48e3efd..3ba75a1 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -1,6 +1,7 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import type { RepoProvider } from './core/contracts/index.js' import { LocalProvider } from './providers/local/index.js' +import { isToolAvailable, TOOL_REQUIREMENTS } from './tools/availability.js' /** * The provider shape tool handlers consume. Now that every provider @@ -20,6 +21,21 @@ import { registerBulkTools } from './tools/bulk.js' import { registerDoctorTools } from './tools/doctor.js' import packageJson from '../package.json' with { type: 'json' } +/** + * Default MCP `instructions` surfaced to clients at initialize time. + * Deliberately kept under 512 characters — directory listings and client + * UIs truncate longer strings. Override via `CreateServerOptions.instructions`. + */ +export const DEFAULT_INSTRUCTIONS + = 'Contentrain is git-native content governance: models define structure; ' + + 'content is canonical JSON/Markdown on a dedicated branch. Call ' + + 'contentrain_describe_format before creating models or content. Preview ' + + 'writes with dry_run:true, review the plan, then re-run with ' + + 'dry_run:false. Start with contentrain_status for models, locales, and ' + + 'workflow; inspect a model with contentrain_describe before editing its ' + + 'entries. Tools are deterministic infrastructure — content decisions ' + + 'stay with the agent.' + export interface CreateServerOptions { /** * Content provider — drives reads (and, in later phases, writes) through @@ -31,14 +47,50 @@ export interface CreateServerOptions { /** * Local project root. When the provider is a `LocalProvider`, its own * `projectRoot` is used as the fallback. Tools that require local disk - * (normalize, setup, git submit/merge) short-circuit with a capability - * error when no projectRoot is available. + * (normalize, setup, git submit/merge) are not registered when no + * projectRoot is available. */ projectRoot?: string + /** + * MCP `instructions` string sent to clients in the `initialize` response. + * Defaults to `DEFAULT_INSTRUCTIONS`; pass an empty string to omit + * instructions entirely. + */ + instructions?: string } /** - * Create an MCP server instance with every Contentrain tool registered. + * `McpServer` variant that silently skips registration for tools named in + * `skipTools`. Register functions call `server.tool(...)` unconditionally; + * this subclass is what keeps capability-gated tools out of `tools/list` + * when the provider (or missing projectRoot) could never satisfy them. + */ +class CapabilityFilteredMcpServer extends McpServer { + private readonly _skipTools: ReadonlySet + + constructor( + serverInfo: ConstructorParameters[0], + options: ConstructorParameters[1], + skipTools: ReadonlySet, + ) { + super(serverInfo, options) + this._skipTools = skipTools + } + + // McpServer.tool has six overloads; a rest-args override is the only + // signature that satisfies all of them. Skipped tools return undefined — + // register functions discard the handle, so nothing downstream observes it. + override tool(...args: unknown[]): ReturnType { + if (this._skipTools.has(args[0] as string)) { + return undefined as unknown as ReturnType + } + return (McpServer.prototype.tool as (...toolArgs: unknown[]) => ReturnType).apply(this, args) + } +} + +/** + * Create an MCP server instance with every *available* Contentrain tool + * registered. * * Two signatures: * @@ -48,21 +100,31 @@ export interface CreateServerOptions { * - `createServer({ provider, projectRoot? })` — phase 5.3 flow. Any * `RepoProvider` (including `GitHubProvider`) drives reads and writes. If * the provider is a `LocalProvider` and `projectRoot` is omitted, the - * provider's own `projectRoot` is used. Otherwise `projectRoot` stays - * undefined and tools that need local disk report a capability error. + * provider's own `projectRoot` is used. * - * Public MCP tool surface (names, parameters, response JSON shape) is - * unchanged across both signatures. + * Tool listing is capability-aware: tools whose requirements + * (`TOOL_REQUIREMENTS`) cannot be met by the resolved provider + + * projectRoot pair are not registered, so `tools/list` only advertises + * tools that can actually succeed. With a `LocalProvider` (stdio and CLI + * flows) all 19 tools remain registered — behavior there is unchanged. */ export function createServer(projectRoot: string): McpServer export function createServer(opts: CreateServerOptions): McpServer export function createServer(input: string | CreateServerOptions): McpServer { - const { provider, projectRoot } = resolveServerContext(input) + const { provider, projectRoot, instructions } = resolveServerContext(input) + + const skipTools = new Set( + Object.keys(TOOL_REQUIREMENTS).filter(name => !isToolAvailable(name, provider, projectRoot)), + ) - const server = new McpServer({ - name: 'contentrain-mcp', - version: packageJson.version, - }) + const server = new CapabilityFilteredMcpServer( + { + name: 'contentrain-mcp', + version: packageJson.version, + }, + { instructions: instructions || undefined }, + skipTools, + ) registerContextTools(server, provider, projectRoot) registerSetupTools(server, provider, projectRoot) @@ -79,19 +141,21 @@ export function createServer(input: string | CreateServerOptions): McpServer { function resolveServerContext(input: string | CreateServerOptions): { provider: ToolProvider projectRoot: string | undefined + instructions: string } { if (typeof input === 'string') { - return { provider: new LocalProvider(input), projectRoot: input } + return { provider: new LocalProvider(input), projectRoot: input, instructions: DEFAULT_INSTRUCTIONS } } + const instructions = input.instructions ?? DEFAULT_INSTRUCTIONS if (input.provider) { let projectRoot = input.projectRoot if (!projectRoot && input.provider instanceof LocalProvider) { projectRoot = input.provider.projectRoot } - return { provider: input.provider, projectRoot } + return { provider: input.provider, projectRoot, instructions } } if (input.projectRoot) { - return { provider: new LocalProvider(input.projectRoot), projectRoot: input.projectRoot } + return { provider: new LocalProvider(input.projectRoot), projectRoot: input.projectRoot, instructions } } throw new Error('createServer: either `provider` or `projectRoot` must be provided') } diff --git a/packages/mcp/src/server/http/server.ts b/packages/mcp/src/server/http/server.ts index dfc2624..f72b327 100644 --- a/packages/mcp/src/server/http/server.ts +++ b/packages/mcp/src/server/http/server.ts @@ -57,6 +57,16 @@ export interface HttpMcpServerResolverOptions { * Defaults to 15 minutes. */ sessionTtlMs?: number + /** + * Optional tenant binding. When set, the fingerprint computed on the + * session-creating request is stored with the session, and every + * follow-up request carrying that `Mcp-Session-Id` must produce the + * same fingerprint (e.g. derive it from the tenant headers the proxy + * injects). A mismatch answers 404 — per the Streamable HTTP spec the + * client then re-initializes and gets a session bound to its own + * provider. This closes session-id replay across tenants. + */ + sessionFingerprint?: (req: http.IncomingMessage) => string | undefined } export interface HttpMcpServerHandle { @@ -148,6 +158,8 @@ interface MultiTenantSession { mcp: McpServer transport: StreamableHTTPServerTransport lastUsed: number + /** Tenant fingerprint captured on the session-creating request (when `sessionFingerprint` is configured). */ + fingerprint: string | undefined } async function startMultiTenantHttpMcpServer( @@ -179,6 +191,7 @@ async function startMultiTenantHttpMcpServer( authToken: opts.authToken, resolveProvider: opts.resolveProvider, projectRoot: opts.projectRoot, + sessionFingerprint: opts.sessionFingerprint, sessions, }) }) @@ -217,6 +230,7 @@ async function handleMultiTenantRequest( authToken?: string resolveProvider: (req: http.IncomingMessage) => ToolProvider | Promise projectRoot?: string + sessionFingerprint?: (req: http.IncomingMessage) => string | undefined sessions: Map }, ): Promise { @@ -225,6 +239,13 @@ async function handleMultiTenantRequest( const existingSessionId = pickSessionId(req.headers[MCP_SESSION_HEADER]) if (existingSessionId && ctx.sessions.has(existingSessionId)) { const session = ctx.sessions.get(existingSessionId)! + // Tenant binding: a session id minted for one tenant must not be + // honored for another. Answer 404 (not 403) so a legitimate client + // that landed on a foreign session re-initializes its own. + if (ctx.sessionFingerprint && ctx.sessionFingerprint(req) !== session.fingerprint) { + writeJson(res, 404, { error: 'Session not found' }) + return + } session.lastUsed = Date.now() try { await session.transport.handleRequest(req, res) @@ -236,11 +257,12 @@ async function handleMultiTenantRequest( try { const provider = await ctx.resolveProvider(req) + const fingerprint = ctx.sessionFingerprint?.(req) const mcp = createMcpServer({ provider, projectRoot: ctx.projectRoot }) const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: () => randomUUID(), onsessioninitialized: (sessionId: string) => { - ctx.sessions.set(sessionId, { mcp, transport, lastUsed: Date.now() }) + ctx.sessions.set(sessionId, { mcp, transport, lastUsed: Date.now(), fingerprint }) }, }) await mcp.connect(transport) diff --git a/packages/mcp/src/tools/annotations.ts b/packages/mcp/src/tools/annotations.ts index 71d493f..5569c5c 100644 --- a/packages/mcp/src/tools/annotations.ts +++ b/packages/mcp/src/tools/annotations.ts @@ -3,6 +3,9 @@ import type { ToolAnnotations } from '@modelcontextprotocol/sdk/types.js' /** * Centralized tool annotations registry. * MCP clients use these hints to distinguish read-only vs. write vs. destructive tools. + * `openWorldHint` is `false` across the board: every tool operates on the + * configured content repository only — none reaches out to arbitrary + * external systems. * * Also serves as the **single source of truth for the tool name list**. Consumers * that need to enumerate every registered tool (e.g. parity tests in @@ -16,24 +19,28 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_describe: { title: 'Describe Model', readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_describe_format: { title: 'Describe Format', readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_doctor: { title: 'Project Health Report', readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, // ─── Setup (write + git) ─── @@ -42,12 +49,14 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: false, destructiveHint: false, idempotentHint: false, + openWorldHint: false, }, contentrain_scaffold: { title: 'Scaffold Template', readOnlyHint: false, destructiveHint: false, idempotentHint: false, + openWorldHint: false, }, // ─── Model (write + git) ─── @@ -56,12 +65,14 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: false, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_model_delete: { title: 'Delete Model', readOnlyHint: false, destructiveHint: true, idempotentHint: false, + openWorldHint: false, }, // ─── Content (mixed) ─── @@ -70,18 +81,21 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: false, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_content_delete: { title: 'Delete Content', readOnlyHint: false, destructiveHint: true, idempotentHint: false, + openWorldHint: false, }, contentrain_content_list: { title: 'List Content', readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, // ─── Workflow (mixed) ─── @@ -90,30 +104,35 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: false, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_submit: { title: 'Submit Branches', readOnlyHint: false, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_merge: { title: 'Merge Branch', readOnlyHint: false, destructiveHint: false, idempotentHint: false, + openWorldHint: false, }, contentrain_branch_list: { title: 'List Branches', readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_branch_delete: { title: 'Delete Branch', readOnlyHint: false, destructiveHint: true, idempotentHint: false, + openWorldHint: false, }, // ─── Normalize (mixed) ─── @@ -122,12 +141,14 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: true, destructiveHint: false, idempotentHint: true, + openWorldHint: false, }, contentrain_apply: { title: 'Apply Normalize', readOnlyHint: false, destructiveHint: false, idempotentHint: false, + openWorldHint: false, }, // ─── Bulk (write + git) ─── @@ -136,6 +157,7 @@ export const TOOL_ANNOTATIONS: Record = { readOnlyHint: false, destructiveHint: false, idempotentHint: false, + openWorldHint: false, }, } diff --git a/packages/mcp/src/tools/availability.ts b/packages/mcp/src/tools/availability.ts new file mode 100644 index 0000000..4310c48 --- /dev/null +++ b/packages/mcp/src/tools/availability.ts @@ -0,0 +1,55 @@ +import type { ProviderCapabilities } from '@contentrain/types' + +/** + * Declarative availability requirements for capability-gated tools. + * + * This is the single source of truth `createServer` uses to decide which + * tools to register for a given provider + projectRoot combination. Tools + * absent from this map are always available. The requirements mirror the + * call-time guards inside each tool handler (`capabilityError`) — those + * guards remain as defense in depth, but a client talking to a remote + * provider no longer sees tools that could never succeed in `tools/list`. + * + * `contentrain_validate` is intentionally NOT listed: read-only validate + * works over any provider; only `fix: true` needs a local worktree and + * keeps its call-time guard. + */ +export interface ToolRequirements { + /** Tool needs a local project root on disk (worktree git transactions, IDE config, AST scans). */ + projectRoot?: boolean + /** Provider capability flags that must all be `true`. */ + capabilities?: readonly (keyof ProviderCapabilities)[] +} + +export const TOOL_REQUIREMENTS: Readonly> = { + contentrain_init: { projectRoot: true }, + contentrain_scaffold: { projectRoot: true }, + contentrain_doctor: { projectRoot: true }, + contentrain_bulk: { projectRoot: true }, + contentrain_submit: { projectRoot: true, capabilities: ['localWorktree', 'pushRemote'] }, + contentrain_merge: { projectRoot: true, capabilities: ['localWorktree'] }, + contentrain_branch_list: { projectRoot: true, capabilities: ['localWorktree'] }, + contentrain_branch_delete: { projectRoot: true, capabilities: ['localWorktree'] }, + contentrain_scan: { projectRoot: true, capabilities: ['astScan'] }, + // Registration requires the extract path (sourceRead); reuse mode + // additionally needs sourceWrite, which stays a call-time check because + // it depends on the `mode` input. + contentrain_apply: { projectRoot: true, capabilities: ['sourceRead'] }, +} + +/** + * Whether a tool can ever succeed for this provider + projectRoot pair. + * Used by `createServer` to filter registration; also exported for + * embedders (e.g. Studio) that want to reason about the effective tool + * surface without spinning up a server. + */ +export function isToolAvailable( + name: string, + provider: { capabilities: ProviderCapabilities }, + projectRoot: string | undefined, +): boolean { + const req = TOOL_REQUIREMENTS[name] + if (!req) return true + if (req.projectRoot && !projectRoot) return false + return (req.capabilities ?? []).every(cap => provider.capabilities[cap]) +} diff --git a/packages/mcp/tests/server/http.test.ts b/packages/mcp/tests/server/http.test.ts index 3dd6796..18b9ffe 100644 --- a/packages/mcp/tests/server/http.test.ts +++ b/packages/mcp/tests/server/http.test.ts @@ -630,7 +630,7 @@ describe('startHttpMcpServer', () => { } }) - it('returns capability error for local-only tools on a remote provider (submit requires localWorktree)', async () => { + it('does not advertise local-only tools on a remote provider (submit requires localWorktree)', async () => { const readOnlyProvider = { capabilities: { localWorktree: false, @@ -654,15 +654,19 @@ describe('startHttpMcpServer', () => { try { // contentrain_submit ships feature branches to remote via simple-git - // — it truly needs a local worktree and rejects uniformly when the - // session's provider can't offer one. + // — it truly needs a local worktree. Capability-aware registration + // keeps it out of tools/list entirely, and calling it anyway is an + // unknown-tool error. + const tools = await client.listTools() + const names = tools.tools.map(t => t.name) + expect(names).not.toContain('contentrain_submit') const result = await client.callTool({ name: 'contentrain_submit', arguments: {}, }) - const parsed = parseResult(result) - expect(parsed['capability_required']).toBe('localWorktree') expect(result.isError).toBe(true) + const text = (result.content as Array<{ text: string }>)[0]!.text + expect(text).toMatch(/not found/i) } finally { await client.close() } @@ -794,6 +798,65 @@ describe('startHttpMcpServer', () => { } }) + it('rejects cross-tenant session-id replay when sessionFingerprint is configured', async () => { + const alpha = makeReadOnlyTenantProvider('alpha') + const bravo = makeReadOnlyTenantProvider('bravo') + const handle = await startHttpMcpServerWith({ + resolveProvider: (req) => { + const header = req.headers['x-project-id'] + return header === 'bravo' ? bravo : alpha + }, + sessionFingerprint: (req) => { + const header = req.headers['x-project-id'] + return Array.isArray(header) ? header[0] : header + }, + port: 0, + }) + try { + const clientAlpha = new Client({ name: 'alpha-client', version: '1.0.0' }) + const transportAlpha = new StreamableHTTPClientTransport(new URL(handle.url), { + requestInit: { headers: { 'x-project-id': 'alpha' } }, + }) + await clientAlpha.connect(transportAlpha) + + try { + const sessionId = transportAlpha.sessionId + expect(sessionId).toBeDefined() + + const listToolsBody = JSON.stringify({ jsonrpc: '2.0', id: 99, method: 'tools/list', params: {} }) + const baseHeaders = { + 'Content-Type': 'application/json', + 'Accept': 'application/json, text/event-stream', + 'mcp-session-id': sessionId!, + } + + // Same tenant replaying its own session id → served. + const sameTenant = await fetch(handle.url, { + method: 'POST', + headers: { ...baseHeaders, 'x-project-id': 'alpha' }, + body: listToolsBody, + }) + expect(sameTenant.status).toBe(200) + await sameTenant.body?.cancel() + + // Another tenant presenting alpha's session id → 404, so the + // client re-initializes and gets a session bound to its own provider. + const crossTenant = await fetch(handle.url, { + method: 'POST', + headers: { ...baseHeaders, 'x-project-id': 'bravo' }, + body: listToolsBody, + }) + expect(crossTenant.status).toBe(404) + const body = await crossTenant.json() as { error: string } + expect(body.error).toBe('Session not found') + } finally { + await clientAlpha.close() + } + } finally { + await handle.close() + } + }) + it('rejects resolver failures with a 500 instead of hanging the request', async () => { const handle = await startHttpMcpServerWith({ resolveProvider: () => { throw new Error('tenant not found') }, diff --git a/packages/mcp/tests/server/tool-availability.test.ts b/packages/mcp/tests/server/tool-availability.test.ts new file mode 100644 index 0000000..c13583c --- /dev/null +++ b/packages/mcp/tests/server/tool-availability.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it, vi } from 'vitest' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { createServer, DEFAULT_INSTRUCTIONS, type CreateServerOptions } from '../../src/server.js' +import { TOOL_ANNOTATIONS, TOOL_NAMES } from '../../src/tools/annotations.js' +import { isToolAvailable, TOOL_REQUIREMENTS } from '../../src/tools/availability.js' + +vi.setConfig({ testTimeout: 30_000, hookTimeout: 30_000 }) + +/** + * Capability-aware registration: `createServer` consults TOOL_REQUIREMENTS + * so `tools/list` only advertises tools that can succeed for the resolved + * provider + projectRoot pair. Local flows keep the full 19-tool surface; + * remote sessions see exactly the subset their provider supports. + */ + +const ALL_CAPS_FALSE = { + localWorktree: false, + sourceRead: false, + sourceWrite: false, + pushRemote: false, + branchProtection: false, + pullRequestFallback: false, + astScan: false, +} + +function makeProvider(capabilities: Partial = {}) { + return { + capabilities: { ...ALL_CAPS_FALSE, ...capabilities }, + async readFile() { throw new Error('no reads expected') }, + async listDirectory() { return [] }, + async fileExists() { return false }, + } +} + +async function connectedClient(input: string | CreateServerOptions): Promise { + const server = createServer(input as CreateServerOptions) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + const client = new Client({ name: 'test-client', version: '1.0.0' }) + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]) + return client +} + +async function listedNames(client: Client): Promise { + const tools = await client.listTools() + return tools.tools.map(t => t.name) +} + +const REMOTE_SAFE_TOOLS = [ + 'contentrain_status', + 'contentrain_describe', + 'contentrain_describe_format', + 'contentrain_model_save', + 'contentrain_model_delete', + 'contentrain_content_save', + 'contentrain_content_delete', + 'contentrain_content_list', + 'contentrain_validate', +] + +describe('capability-aware tool registration', () => { + it('registers all 19 tools for a local project root', async () => { + const client = await connectedClient(join(tmpdir(), 'cr-availability-local')) + const names = await listedNames(client) + expect(names.toSorted()).toEqual([...TOOL_NAMES].toSorted()) + await client.close() + }) + + it('registers only remote-safe tools for a capability-less provider without projectRoot', async () => { + const client = await connectedClient({ provider: makeProvider() }) + const names = await listedNames(client) + expect(names.toSorted()).toEqual([...REMOTE_SAFE_TOOLS].toSorted()) + await client.close() + }) + + it('keeps validate listed remotely — read-only validate works over any provider', async () => { + const client = await connectedClient({ provider: makeProvider() }) + const names = await listedNames(client) + expect(names).toContain('contentrain_validate') + await client.close() + }) + + it('gates on capability flags independently of projectRoot', async () => { + // projectRoot present, but the provider only offers source access — + // normalize tools appear, worktree/git lifecycle tools do not. + const client = await connectedClient({ + provider: makeProvider({ sourceRead: true, sourceWrite: true, astScan: true }), + projectRoot: join(tmpdir(), 'cr-availability-partial'), + }) + const names = await listedNames(client) + expect(names).toContain('contentrain_scan') + expect(names).toContain('contentrain_apply') + expect(names).toContain('contentrain_init') + expect(names).not.toContain('contentrain_submit') + expect(names).not.toContain('contentrain_merge') + expect(names).not.toContain('contentrain_branch_list') + expect(names).not.toContain('contentrain_branch_delete') + await client.close() + }) + + it('every requirement entry names a real tool', () => { + for (const name of Object.keys(TOOL_REQUIREMENTS)) { + expect(TOOL_NAMES).toContain(name) + } + }) + + it('isToolAvailable mirrors the requirements map', () => { + const remote = makeProvider() + expect(isToolAvailable('contentrain_status', remote, undefined)).toBe(true) + expect(isToolAvailable('contentrain_doctor', remote, undefined)).toBe(false) + expect(isToolAvailable('contentrain_doctor', remote, '/tmp/x')).toBe(true) + expect(isToolAvailable('contentrain_submit', makeProvider({ localWorktree: true }), '/tmp/x')).toBe(false) + expect(isToolAvailable('contentrain_submit', makeProvider({ localWorktree: true, pushRemote: true }), '/tmp/x')).toBe(true) + expect(isToolAvailable('contentrain_scan', makeProvider({ astScan: true }), undefined)).toBe(false) + }) +}) + +describe('server instructions', () => { + it('sends DEFAULT_INSTRUCTIONS when none are provided', async () => { + expect(DEFAULT_INSTRUCTIONS.length).toBeLessThanOrEqual(512) + const client = await connectedClient({ provider: makeProvider() }) + expect(client.getInstructions()).toBe(DEFAULT_INSTRUCTIONS) + await client.close() + }) + + it('passes custom instructions through', async () => { + const client = await connectedClient({ + provider: makeProvider(), + instructions: 'Custom operating manual.', + }) + expect(client.getInstructions()).toBe('Custom operating manual.') + await client.close() + }) + + it('omits instructions entirely for an empty string', async () => { + const client = await connectedClient({ provider: makeProvider(), instructions: '' }) + expect(client.getInstructions()).toBeUndefined() + await client.close() + }) +}) + +describe('tool annotations', () => { + it('every tool declares openWorldHint: false', () => { + for (const [name, annotation] of Object.entries(TOOL_ANNOTATIONS)) { + expect(annotation.openWorldHint, `${name} openWorldHint`).toBe(false) + } + }) + + it('advertised tools carry openWorldHint over the wire', async () => { + const client = await connectedClient({ provider: makeProvider() }) + const tools = await client.listTools() + for (const tool of tools.tools) { + expect(tool.annotations?.openWorldHint, `${tool.name} openWorldHint`).toBe(false) + } + await client.close() + }) +}) diff --git a/packages/mcp/tests/tools/doctor.test.ts b/packages/mcp/tests/tools/doctor.test.ts index d125a8b..de741fd 100644 --- a/packages/mcp/tests/tools/doctor.test.ts +++ b/packages/mcp/tests/tools/doctor.test.ts @@ -72,7 +72,7 @@ describe('contentrain_doctor tool', () => { expect(report.usage).toHaveProperty('missingLocaleKeys') }) - it('returns a capability error when driven by a remote provider (no projectRoot)', async () => { + it('is not advertised when driven by a remote provider (no projectRoot)', async () => { const fakeClient = {} as unknown as GitHubClient const provider = new GitHubProvider(fakeClient, { owner: 'acme', name: 'site' }) const server = createServer({ provider }) @@ -83,11 +83,15 @@ describe('contentrain_doctor tool', () => { server.connect(serverTransport), ]) + // Capability-aware registration: doctor needs a local projectRoot, so a + // remote session never sees it in tools/list and a forced call is an + // unknown-tool error rather than a capability payload. + const tools = await client.listTools() + expect(tools.tools.map(t => t.name)).not.toContain('contentrain_doctor') const result = await client.callTool({ name: 'contentrain_doctor', arguments: {} }) expect(result.isError).toBe(true) const content = result.content as Array<{ type: string, text: string }> - const data = JSON.parse(content[0]!.text) - expect(data.capability_required).toBe('localWorktree') + expect(content[0]!.text).toMatch(/not found/i) }) it('is advertised in the tools list', async () => {