Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
65 changes: 0 additions & 65 deletions apps/web/src/scripts/mcp-catalog/catalog-rot-guard.test.ts

This file was deleted.

71 changes: 34 additions & 37 deletions apps/web/src/scripts/mcp-catalog/catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
runKiloCompletion,
type CatalogLeaf,
} from './catalog';
import { MCP_MUTATION_ALLOWLIST } from './mutations';

const queryLeaf = (path: string, firstInput?: CatalogLeaf['firstInput']): CatalogLeaf => ({
path,
Expand All @@ -39,18 +38,14 @@ const mutationLeaf = (path: string, firstInput?: CatalogLeaf['firstInput']): Cat
firstInput,
});

/** The allowlist as the real rootRouter exposes it: every listed path exists. */
const allowlistedMutationLeaves = (): CatalogLeaf[] =>
MCP_MUTATION_ALLOWLIST.map(path => mutationLeaf(path));

/**
* A synthetic leaf set that satisfies the rot guard: the fixture's own leaves
* plus every allowlisted mutation. The guard now compares the allowlist even
* when no mutation leaf is enumerated, so a query-only fixture would throw.
* A fixture leaf set that satisfies the mutation drift guard: the fixture's own
* leaves plus one non-denylisted mutation. The extra leaf carries no summary,
* so it lands in `missing` and never disturbs a `rows` assertion.
*/
const withAllowlistedMutations = (leaves: CatalogLeaf[]): CatalogLeaf[] => [
const withMutation = (leaves: CatalogLeaf[]): CatalogLeaf[] => [
...leaves,
...allowlistedMutationLeaves(),
mutationLeaf('user.updateProfile'),
];

describe('mcp-catalog catalog', () => {
Expand All @@ -72,7 +67,7 @@ describe('mcp-catalog catalog', () => {
});

describe('buildCatalogRows', () => {
it('keeps every query and only allowlisted mutations, dropping the denylist and subscriptions', () => {
it('keeps every non-denylisted query and mutation, dropping the denylist and subscriptions', () => {
const { rows, missing } = buildCatalogRows(
[
queryLeaf('admin.users.list'),
Expand All @@ -83,24 +78,26 @@ describe('mcp-catalog catalog', () => {
mutationLeaf('user.deleteAccount'),
{ path: 'user.onEvent', type: 'subscription', firstInput: undefined },
queryLeaf('user.getProfile'),
...allowlistedMutationLeaves(),
mutationLeaf('user.updateProfile'),
],
new Map([
['user.getProfile', 'Returns the profile of a user.'],
['agentProfiles.create', 'Create an agent profile.'],
['organizations.subscription.cancel', 'Cancel the subscription.'],
['user.deleteAccount', 'Delete the account.'],
['user.updateProfile', 'Update the profile.'],
['admin.users.delete', 'Delete a user (internal).'],
])
);
// Only the query and the allowlisted mutation with a summary are rows.
expect(rows.map(row => row.path)).toEqual(['user.getProfile', 'agentProfiles.create']);
expect(rows[0]?.kind).toBe('query');
expect(rows[1]?.kind).toBe('mutation');
// Non-allowlisted mutations are dropped even with a committed summary;
// allowlisted mutations without a summary wait for the LLM pass.
expect(missing.map(leaf => leaf.path)).toEqual(
MCP_MUTATION_ALLOWLIST.filter(path => path !== 'agentProfiles.create')
);
// Every non-denylisted query and mutation with a summary is a row;
// admin/debug/test and subscriptions are dropped, summary or not.
expect(rows.map(row => row.path)).toEqual([
'organizations.subscription.cancel',
'user.deleteAccount',
'user.getProfile',
'user.updateProfile',
]);
expect(rows.map(row => row.kind)).toEqual(['mutation', 'mutation', 'query', 'mutation']);
expect(missing).toEqual([]);
});

it('locks the denylist constant to the internal-only segments', () => {
Expand All @@ -112,20 +109,15 @@ describe('mcp-catalog catalog', () => {
expect(() => buildCatalogRows([])).toThrow(/zero catalog rows/);
});

it('throws naming the allowlisted paths that no longer match a mutation leaf', () => {
// The router still exposes mutations, so a missing allowlisted path is a
// rename or deletion: the dump must fail loudly instead of dropping it.
expect(() =>
buildCatalogRows([
queryLeaf('user.getProfile'),
mutationLeaf('agentProfiles.createRenamed'),
])
).toThrow(/agentProfiles\.create\b/);
it('throws when no mutation survives the denylist', () => {
// A query-only enumeration (a wholesale router drift) must not publish a
// mutation-free catalog in silence: write support would vanish.
expect(() => buildCatalogRows([queryLeaf('user.getProfile')])).toThrow(/zero mutation rows/);
});

it('shapes rows with derived tags, input schema and search blob', () => {
const { rows } = buildCatalogRows(
withAllowlistedMutations([queryLeaf('user.getProfile', z.object({ userId: z.string() }))]),
withMutation([queryLeaf('user.getProfile', z.object({ userId: z.string() }))]),
new Map([['user.getProfile', 'Returns the profile of a user.']])
);
const row = rows[0];
Expand All @@ -145,7 +137,7 @@ describe('mcp-catalog catalog', () => {

it('emits an empty input schema for procedures without input', () => {
const { rows } = buildCatalogRows(
withAllowlistedMutations([queryLeaf('user.listSessions')]),
withMutation([queryLeaf('user.listSessions')]),
new Map([['user.listSessions', 'Lists active sessions.']])
);
expect(rows[0]?.inputSchema).toEqual({});
Expand All @@ -154,7 +146,7 @@ describe('mcp-catalog catalog', () => {
it('keeps committed summaries byte-for-byte, including whitespace', () => {
const summary = ' Returns the user profile. ';
const { rows } = buildCatalogRows(
withAllowlistedMutations([queryLeaf('user.getProfile')]),
withMutation([queryLeaf('user.getProfile')]),
new Map([['user.getProfile', summary]])
);
expect(rows[0]?.summary).toBe(summary);
Expand All @@ -171,7 +163,7 @@ describe('mcp-catalog catalog', () => {
describe('buildCatalogJson', () => {
it('sorts rows by path and ends with a trailing newline', () => {
const { rows } = buildCatalogRows(
withAllowlistedMutations([queryLeaf('b.b'), queryLeaf('a.a')]),
withMutation([queryLeaf('b.b'), queryLeaf('a.a')]),
new Map([
['a.a', 'A'],
['b.b', 'B'],
Expand Down Expand Up @@ -260,12 +252,17 @@ describe('mcp-catalog catalog', () => {
expect(buildCatalogJson(rows)).toBe(onDisk);
});

it('publishes every allowlisted mutation with kind mutation and a searchable blob', () => {
it('publishes every non-denylisted mutation with kind mutation and a searchable blob', () => {
const catalog = JSON.parse(readFileSync(CATALOG_JSON_PATH, 'utf8')) as Record<
string,
{ kind?: string; searchBlob?: string } | undefined
>;
for (const path of MCP_MUTATION_ALLOWLIST) {
const denylisted = new Set<string>(DENYLISTED_TOP_LEVEL_SEGMENTS);
const expected = collectCatalogLeaves(rootRouter)
.filter(leaf => leaf.type === 'mutation' && !denylisted.has(leaf.path.split('.')[0] ?? ''))
.map(leaf => leaf.path);
expect(expected.length).toBeGreaterThan(0);
for (const path of expected) {
expect(catalog[path]?.kind).toBe('mutation');
expect(catalog[path]?.searchBlob).toContain(path);
}
Expand Down
54 changes: 19 additions & 35 deletions apps/web/src/scripts/mcp-catalog/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
* artifact, and authors edit its summaries by hand. See dump.ts for the CLI
* entry point.
*
* Queries are published wholesale; mutations only when their exact path is in
* `MCP_MUTATION_ALLOWLIST` (mutations.ts).
* Queries and mutations are published wholesale; the top-level denylist
* (`admin`, `debug`, `test`) and subscriptions stay internal.
*/
import { spawnSync } from 'node:child_process';
import { readFileSync, statSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { basename, dirname, join, relative, resolve } from 'node:path';
import { z } from 'zod';
import { MCP_MUTATION_ALLOWLIST, isAllowedMutation } from './mutations';

/** Repository root: five levels above apps/web/src/scripts/mcp-catalog. */
const REPO_ROOT = join(__dirname, '..', '..', '..', '..', '..');
Expand All @@ -39,9 +38,8 @@ export const ROOT_ROUTER_PATH = join(__dirname, '..', '..', 'routers', 'root-rou

/**
* Top-level router segments that stay internal-only. This is a denylist:
* every other query procedure is exported, allowlisted mutations are exported
* as well, and this filter applies on top of the mutation allowlist. Individual
* procedures cannot opt back in, and subscriptions are never exported.
* every other query and mutation procedure is exported. Individual procedures
* cannot opt back in, and subscriptions are never exported.
*/
export const DENYLISTED_TOP_LEVEL_SEGMENTS = ['admin', 'debug', 'test'] as const;

Expand Down Expand Up @@ -166,38 +164,28 @@ function shapeRow(leaf: CatalogLeaf, summary: string): CatalogRow {
}

/**
* Filters the leaves down to the exported catalog: every query, plus a mutation
* only when its exact path is in {@link MCP_MUTATION_ALLOWLIST}. The top-level
* denylist drops both kinds and subscriptions stay excluded. Rows whose summary
* is provided keep it byte-for-byte; the rest come back as `missing` for LLM
* generation.
* Filters the leaves down to the exported catalog: every query and mutation,
* except the top-level denylist. Subscriptions are never exported. Rows whose
* summary is provided keep it byte-for-byte; the rest come back as `missing`
* for LLM generation.
*
* Rot guard: an allowlisted path that does not reach the catalog as a mutation
* throws (naming the offending paths) instead of vanishing from the catalog —
* whether it was renamed or deleted, its top-level segment is withheld by the
* denylist, or its procedure was demoted to a query. The guard therefore
* compares the allowlist against the paths that survived the denylist *and*
* published with kind `mutation`. The comparison is unconditional: a query-only
* enumeration (a wholesale router drift) has nothing to satisfy it, so it
* throws too, rather than publishing a mutation-free catalog in silence.
* Drift guard: an enumeration that publishes no mutation row throws instead of
* emitting a query-only catalog in silence. A router refactor that stops
* exposing mutations, or a top-level segment renamed into the denylist, must
* fail the dump rather than quietly remove write support from every MCP client.
*/
export function buildCatalogRows(
leaves: CatalogLeaf[],
summaries: Map<string, string> = new Map()
): { rows: CatalogRow[]; missing: CatalogLeaf[] } {
const denylisted = new Set<string>(DENYLISTED_TOP_LEVEL_SEGMENTS);
/** Every allowlisted path the catalog will publish with kind `mutation`. */
const publishedMutationPaths = new Set<string>();
let publishedMutations = 0;
const rows: CatalogRow[] = [];
const missing: CatalogLeaf[] = [];
for (const leaf of leaves) {
const kept =
leaf.type === 'query' || (leaf.type === 'mutation' && isAllowedMutation(leaf.path));
if (!kept) continue;
if (leaf.type !== 'query' && leaf.type !== 'mutation') continue;
Comment thread
iscekic marked this conversation as resolved.
Comment thread
iscekic marked this conversation as resolved.
Comment thread
iscekic marked this conversation as resolved.
if (denylisted.has(leaf.path.split('.')[0] ?? '')) continue;
// Only a mutation leaf counts: a demoted query leaf must not satisfy the
// guard, or an allowlisted path could stop being a mutation unnoticed.
if (leaf.type === 'mutation') publishedMutationPaths.add(leaf.path);
if (leaf.type === 'mutation') publishedMutations += 1;
Comment thread
iscekic marked this conversation as resolved.
Outdated
const summary = summaries.get(leaf.path);
if (typeof summary === 'string' && summary !== '') {
rows.push(shapeRow(leaf, summary));
Expand All @@ -210,15 +198,11 @@ export function buildCatalogRows(
'Catalog enumeration produced zero catalog rows — refusing to emit an empty catalog'
);
}
// Compare unconditionally: a leaf set with no mutation leaf at all has nothing
// to satisfy the allowlist, so deletion or wholesale router drift cannot
// silently publish a query-only catalog.
const stale = MCP_MUTATION_ALLOWLIST.filter(path => !publishedMutationPaths.has(path));
if (stale.length > 0) {
if (publishedMutations === 0) {
throw new Error(
`MCP mutation allowlist entries are not published as mutations: ${stale.join(', ')}. ` +
'A renamed, deleted, denylisted, or demoted procedure must never silently vanish from the catalog — ' +
'update MCP_MUTATION_ALLOWLIST in apps/web/src/scripts/mcp-catalog/mutations.ts.'
'Catalog enumeration published zero mutation rows — refusing to emit a query-only catalog. ' +
'Every mutation is either missing from the router or withheld by its top-level segment; ' +
'check DENYLISTED_TOP_LEVEL_SEGMENTS in apps/web/src/scripts/mcp-catalog/catalog.ts.'
);
}
return { rows, missing };
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/scripts/mcp-catalog/dump.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* CLI entry point: dumps the tRPC catalog (queries plus allowlisted mutations)
* to services/kilo-mcp/catalog.json.
* CLI entry point: dumps the tRPC catalog (queries and mutations, minus the
* admin/debug/test denylist) to services/kilo-mcp/catalog.json.
*
* Usage (from apps/web):
* pnpm script src/scripts/mcp-catalog/dump.ts # write or regenerate
Expand Down
38 changes: 0 additions & 38 deletions apps/web/src/scripts/mcp-catalog/mutations.ts

This file was deleted.

Loading