Repository navigation
feat(kilo-mcp): publish all non-admin/debug mutations - #6315
Conversation
The catalog published only the 10 allowlisted agentProfiles mutations (MCP_MUTATION_ALLOWLIST). The router exposes 671 mutations and 517 are non-denylisted (non admin/debug/test), so every other mutation was invisible to search and call. - Drop MCP_MUTATION_ALLOWLIST and its exact-path gate; buildCatalogRows keeps every query and mutation that survives the top-level denylist (admin, debug, test) and still drops subscriptions. - Replace the allowlist rot guard with a mutation drift guard: a query-only enumeration throws instead of silently removing write support from every MCP client. - Regenerate services/kilo-mcp/catalog.json: 897 rows (380 queries, 517 mutations), every row with a summary. The Vectorize upsert job for the original PR (#6224) succeeded, so the missing mutations were a catalog-scope bug, not a failed index update.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit Files Reviewed (3 files)
Previous Review Summaries (3 snapshots, latest commit a544a6f)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a544a6f)Status: 1 Issues Found | Recommendation: Address before merge Executive SummaryThe chained-input schema composition added in Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 994fed8)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the follow-up commit: both prior warnings are resolved. Verified against the regenerated catalog: 853 rows (491 mutations, 362 queries), zero denylisted paths present, every row has a non-empty summary with a Files Reviewed (3 files)
Previous review (commit 95b2a85)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryPublishing every non-top-level-denylisted mutation also exposes nested Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
Address the Kilo review on this PR. - Match admin/debug/dev/test as path segments anywhere, not only at the top level. This drops 21 nested admin mutations, 5 dev-only mutations, and 18 nested admin queries that the previous predicate admitted. `test` stays exact-only so user-facing calls such as `slack.testConnection` remain published. - Count exposed mutation procedures in the drift guard and word the error accordingly: a mutation without a committed summary is a legitimate `missing` entry that the dump generates, not a router drift. Regenerate catalog.json: 853 rows (362 queries, 491 mutations).
The catalog published only the first `.input()` schema. A chained procedure such as `workspaceFolders.create` adds a second `.input()` that requires `name` and `color`. MCP clients saw `organizationId` alone, so a request passed local validation and failed upstream. Compose every chained input into one schema. Object schemas merge their properties and required keys. A key that two schemas declare keeps both constraints through `allOf`. A non-object chain falls back to a top-level `allOf`. Tags and search blobs now cover every input field. Regenerate catalog.json: 853 rows; 207 rows get expanded input schemas.
Review guidance from the #6315 review: ask for justification when a newly published MCP mutation issues or rotates credentials or tokens, touches secrets, changes organization membership or authorization, deploys, charges or purchases, or deletes data.
The flat merge kept only `properties` and `required`. A chained `.strict()` input then lost `additionalProperties: false`, so the published schema accepted unknown keys that upstream tRPC rejects. Merge only plain object schemas. Any chain with a keyword such as `additionalProperties` falls back to a top-level `allOf`, which preserves each schema's own keywords. `organizations.cloudAgentNext.listRepositoryBranches` and `organizations.groups.getPolicyEditorData` now publish their strict constraint. Regenerate catalog.json: 32 rows use the faithful intersection.
2 participants
Problem
PR #6224 added mutation support to the Kilo MCP, but the catalog only published the 10 paths in
MCP_MUTATION_ALLOWLIST(agentProfiles). Every other mutation was invisible tosearchand rejected bycall.Root cause
buildCatalogRowsgated mutations on an exact-path allowlist:The
rootRouterexposes 671 mutations. 517 are non-denylisted (top-level segment notadmin,debug, ortest); only 10 were published. This is a catalog-scope bug, not a deployment failure: the catalog-merge job for #6224 succeeded and itsUpsert Vectorize indexstep passed (run 35289442683).Fix
mutations.ts(MCP_MUTATION_ALLOWLIST,isAllowedMutation) and its exact-path gate.buildCatalogRowsnow keeps every query and mutation that survives the existing top-level denylist, and still drops subscriptions.services/kilo-mcp/catalog.json: 897 rows (380 queries, 517 mutations), every row with a summary.catalog.test.tsasserts the committed catalog regenerates byte-for-byte and that every non-denylisted router mutation is present.Verification
apps/webjestsrc/scripts/mcp-catalog: 31 passed.services/kilo-mcpvitest: 282 passed.node --test scripts/kilo-mcp-catalog.test.mjs: 29 passed.oxlintandoxfmt --list-differentclean on the changed files.tsgo --noEmitreports no error inmcp-catalog(remaining output is pre-existing, in unrelated packages).Note
The denylist is top-level-segment based, as before. Nested paths whose segment name merely contains
admin(for examplecodingPlans.adminMarkRevocationFailed) stay published, consistent with how queries are already filtered.