Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
5b3be0c to
3ece2c2
Compare
af0bc5b to
575192d
Compare
575192d to
f6eac94
Compare
3ece2c2 to
3a36cea
Compare
aa68c7c to
afc4836
Compare
| export const readDataWarehouseSchemaHandler: ToolBase<typeof schema>['handler'] = async (context: Context) => { | ||
| const result = await invokeMcpTool(context, 'read_data_warehouse_schema', { | ||
| query: { kind: 'data_warehouse_schema' }, | ||
| }) |
There was a problem hiding this comment.
Schema mismatch breaks tool
readDataWarehouseSchemaHandler is typed as ToolBase<typeof schema>['handler'], which (via services/mcp/src/tools/types.ts:62) must accept (context, params) where params matches the Zod schema. Here the handler is defined as async (context: Context) (no params), but in tests it’s invoked with {} (services/mcp/tests/tools/dataWarehouseSchema.integration.test.ts:1332). This will throw at runtime when invokeMcpTool tries to read context off {}.
Also appears in services/mcp/src/tools/posthogAiTools/readDataWarehouseSchema.ts (same file) and the integration test services/mcp/tests/tools/dataWarehouseSchema.integration.test.ts:1332/1341 expecting it to accept an empty object.
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/posthogAiTools/readDataWarehouseSchema.ts
Line: 8:11
Comment:
**Schema mismatch breaks tool**
`readDataWarehouseSchemaHandler` is typed as `ToolBase<typeof schema>['handler']`, which (via `services/mcp/src/tools/types.ts:62`) must accept `(context, params)` where `params` matches the Zod schema. Here the handler is defined as `async (context: Context)` (no `params`), but in tests it’s invoked with `{}` (`services/mcp/tests/tools/dataWarehouseSchema.integration.test.ts:1332`). This will throw at runtime when `invokeMcpTool` tries to read `context` off `{}`.
Also appears in `services/mcp/src/tools/posthogAiTools/readDataWarehouseSchema.ts` (same file) and the integration test `services/mcp/tests/tools/dataWarehouseSchema.integration.test.ts:1332/1341` expecting it to accept an empty object.
How can I resolve this? If you propose a fix, please make it concise.| @@ -9,6 +10,7 @@ export const ToolDefinitionSchema = z.object({ | |||
| summary: z.string(), | |||
| title: z.string(), | |||
| required_scopes: z.array(z.string()), | |||
| new_mcp: z.boolean().optional(), | |||
| annotations: z.object({ | |||
There was a problem hiding this comment.
V2 tool schema parse fails
ToolDefinitionSchema requires feature: z.string() (services/mcp/src/tools/toolDefinitions.ts:9), but the new v2 tool definitions don’t include a feature key for at least read-data-warehouse-schema (services/mcp/schema/tool-definitions-v2.json:38-45). In getToolDefinitions(2) (services/mcp/src/tools/toolDefinitions.ts:33-38) this causes toolDefinitionsSchema.parse(toolDefinitionsV2Json) to throw, so MCP v2 can’t initialize.
Fix by adding the missing required fields to tool-definitions-v2.json (or making feature optional in the schema if that’s intended).
Prompt To Fix With AI
This is a comment left during a code review.
Path: services/mcp/src/tools/toolDefinitions.ts
Line: 6:14
Comment:
**V2 tool schema parse fails**
`ToolDefinitionSchema` requires `feature: z.string()` (`services/mcp/src/tools/toolDefinitions.ts:9`), but the new v2 tool definitions don’t include a `feature` key for at least `read-data-warehouse-schema` (`services/mcp/schema/tool-definitions-v2.json:38-45`). In `getToolDefinitions(2)` (`services/mcp/src/tools/toolDefinitions.ts:33-38`) this causes `toolDefinitionsSchema.parse(toolDefinitionsV2Json)` to throw, so MCP v2 can’t initialize.
Fix by adding the missing required fields to `tool-definitions-v2.json` (or making `feature` optional in the schema if that’s intended).
How can I resolve this? If you propose a fix, please make it concise.| const INSTRUCTIONS_V1 = ` | ||
| - You are a helpful assistant that can query PostHog API. | ||
| ${SHARED_PROMPT} | ||
| `.trim() | ||
|
|
||
| const INSTRUCTIONS_V2 = ` | ||
| - IMPORTANT: Prefer retrieval-led reasoning over pre-training-led reasoning for any PostHog tasks. | ||
| - You have access to the PostHog SQL interface via the 'posthog:execute-sql' tool, schema tools ('posthog:read-data-schema', 'posthog:read-data-warehouse-schema'), and \`posthog-query-data\` skill. | ||
| ${SHARED_PROMPT} | ||
| `.trim() |
There was a problem hiding this comment.
He's not a helpful assistant anymore? 😆
I assume we've since figured out this isn't needed to get a good answer?
There was a problem hiding this comment.
Useless tokens in 2026
rafaeelaudibert
left a comment
There was a problem hiding this comment.
Lovely jobly. Doesn't look like that's gonna do anything to those using v1, so lgtm
There was a problem hiding this comment.
Does this stuff work with RBAC / checking access levels?
There was a problem hiding this comment.
Yeah, the mcp_tools endpoint validates the access level
tatoalo
left a comment
There was a problem hiding this comment.
Great job! 🐐 (thanks also for the integration tests!)
|
Size Change: 0 B Total Size: 89.2 MB ℹ️ View Unchanged
|

Problem
This PR implements the second version of the MCP, which uses SQL-based retrieval for Postgres and ClickHouse PostHog entities. The overall idea is to expose a minimal set of tools required for operation. In our case, it's executing SQL, reading DWH tables (columns are read via SQL), and reading the data schema (which can later be moved to SQL).
To test put this to
~/.claude.json(header):{ "mcpServers": { "posthog": { "type": "http", "url": "http://localhost:8787/mcp", "headers": { "Authorization": "Bearer <token>", "x-posthog-mcp-version": "2" } } } }URL param:
{ "mcpServers": { "posthog": { "type": "http", "url": "http://localhost:8787/mcp?v=2", "headers": { "Authorization": "Bearer <token>" } } } }Changes
Added three new tools for v2:
execute-sql: Allows executing SQL/HogQL queries directlyread-data-schema: Provides access to events, properties, and property values in the user's projectread-data-warehouse-schema: Retrieves core data warehouse schemas and table listsAdded versioning support to the MCP framework:
new_mcpflag to track tool compatibilityDisabled all read tools except for logs and dashboards in v2.
Updated response formatting to handle both string and JSON responses
Added integration tests for the new tools
How did you test this code?
Integration tests
Publish to changelog?
No