feat: add query_logs tool for custom log queries (CI mirror of #333) - #341
feat: add query_logs tool for custom log queries (CI mirror of #333)#341jordienr wants to merge 19 commits into
Conversation
Coverage Report for CI Build 31382367652Warning No base build found for commit Coverage: 96.48%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
|
Structured review done against this head (c1bdc7f, same SHA as #333, so everything transfers). CI green here confirms the #333 failures were fork-secrets only. The One blocker before the checkmark: the types regen drops fields that exist on main. Optional while you're in there: |
Removes content that depends on supabase/agent-skills#112 and supabase/mcp#341 before those PRs have merged: - Delete ai-agents.mdx (new page documenting query_logs / get_logs / skill) - Remove MCP tip admonition from debugging.mdx - Remove telemetry-ai ContentListings panel from overview - Remove telemetryAI export from telemetry.data.ts - Remove telemetryAI from content-listings index - Remove "AI & automation" nav group from NavigationMenu.constants.ts These changes move to docs/telemetry-mcp-tools which targets this branch as its base and will not merge until the two external PRs land. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a query_logs debugging tool that runs a custom read-only ClickHouse SQL query against a project's unified logs stream, for cases where the get_logs service presets are too coarse. Reuses the existing analytics logs endpoint and validates that queries are SELECT/WITH only.
- add function_edge_logs to the sql source-hint list so models can reach edge function invocation logs - require a non-empty sql query (.min(1)), matching execute_sql - add execution tests for query_logs: sql passthrough + timestamp defaulting, custom window forwarding, and empty-query rejection
- mark get_logs as deprecated on hosted projects in favour of query_logs, while keeping it as the path for CLI/self-hosted - document that query_logs (ClickHouse) is hosted-only and will not work on CLI/self-hosted yet
The permissive mention of iso_timestamp_start/iso_timestamp_end wasn't steering model behaviour, so narrow time-range questions silently inherited the 24h default and over-counted. Make it a directive instruction in both get_logs and query_logs, matching the mechanism that flipped tool selection.
The description promises iso_timestamp_start defaults to 24h before the end, but the handler always computed start from now(), so supplying only iso_timestamp_end produced an inverted/empty window. Derive the end first (supplied or now), then default start to end - 24h, shared by get_logs and query_logs.
resolveLogWindow now rejects a malformed iso_timestamp_start/end with a clear error instead of throwing a raw "Invalid time value", and rejects a start at or after the end. Also rebases onto main to pick up the regenerated management API types.
c1bdc7f to
09e521e
Compare
Both tools currently ship an identical description to every client regardless of platform (hosted vs local/self-hosted), so labeling get_logs "Deprecated" risked a client universally hiding or deprioritizing it, which would break self-hosted users since get_logs is their only working logs tool. Reframe as environment-scoped preference (prefer query_logs on hosted, use get_logs on local/self-hosted) instead of an unqualified deprecation.
…registration DebuggingOperations.queryLogs is used by external SupabasePlatform implementers (CLI, studio, mcp.supabase.com controller) outside this repo. Making it required would break them on upgrade: a stale implementer still passes the existing `if (debugging)` group check, so query_logs gets listed in tools/list and then crashes with "debugging.queryLogs is not a function" at call time. Make queryLogs optional and only register the query_logs tool when the platform actually implements it, so an implementer without ClickHouse support (self-hosted/CLI today) simply doesn't get the tool listed instead of erroring. This makes the DebuggingOperations change purely additive.
resolveLogWindow now:
- enforces the 24h API cap client-side with a clear error, instead of
relying on an unvalidated description promise
- normalizes accepted timestamps to canonical UTC ISO strings before
forwarding them, instead of passing the original strings through verbatim
- is exported and unit-tested directly (default anchoring, offset
normalization, malformed/inverted/oversized-window rejection), covering
get_logs and query_logs' shared behavior in one place
Also enforces ISO 8601 with an explicit UTC "Z" suffix or offset at the
schema level via z.iso.datetime({ offset: true }), so offset-less
timestamps (ambiguous local-time interpretation) are rejected before
reaching resolveLogWindow, and the constraint shows up in the tool's JSON
schema.
- add an equal-timestamps case to the start-at-or-after-end rejection test - assert on the actual rejection message (Invalid ISO datetime, must be before, min-length) instead of a bare rejects.toThrow() - assert the exact default window (end near now, start = end - 24h) instead of just checking the params are truthy
Per team alignment: on platforms that support ClickHouse-backed querying (hosted/production), query_logs is the tool that shows up in tools/list and get_logs is hidden from discovery (but remains callable via tools/call for compatibility). On platforms without it (CLI/self-hosted), the reverse holds: get_logs is listed and query_logs is not registered at all. Also strips environment-specific wording (hosted, production, self-hosted, CLI) from both tool descriptions, since the description is a frozen field for some clients and shouldn't encode environment context the model can't verify; the routing is now handled structurally via registration/hiding instead of prose.
## What Adds a new `query_logs` tool to the `debugging` feature group. It runs a custom **ClickHouse SQL query** against a project's unified logs stream, for cases where the `get_logs` service presets are too coarse (filtering, aggregating, or joining across log fields). Tracks **AI-701**. Builds on the ClickHouse logs endpoint work in O11Y-1813 and the `get_logs` ClickHouse migration (#326). ## Breaking change This ships as `feat!:` per team alignment (see PR discussion): on platforms that implement ClickHouse-backed querying (hosted/production), `query_logs` is the tool discoverable via `tools/list`, and `get_logs` is hidden from discovery — though it remains callable via `tools/call` for any client still holding an older tool list. On platforms without ClickHouse support (CLI/self-hosted), the reverse holds: `get_logs` stays listed and `query_logs` is not registered at all. `DebuggingOperations.queryLogs` is optional on the `SupabasePlatform` type (published via the `./platform` subpath export). Implementers outside this repo (CLI, studio, the hosted controller) are unaffected if they don't implement it — they simply keep `get_logs` as their only logs tool. ## How it addresses the observability team's concerns 1. **Prompt injection / security** — log content is user-controllable (same class of risk as `execute_sql`), so the result is wrapped in `wrapWithUntrustedDataBoundary`, the same best-effort guardrail `execute_sql` uses. Not foolproof, but consistent with the existing arbitrary-query tool. 2. **Cost / scalability** — starts conservative: defaults to a 24h window, and both the client (`resolveLogWindow`) and the analytics endpoint cap the requested range at 24h. No unbounded queries. 3. **ClickHouse, not BigQuery** — hits `GET /v1/projects/{ref}/analytics/endpoints/logs` (ClickHouse / logs.all.otel) and takes raw ClickHouse-dialect SQL. Does not touch the deprecated BigQuery-backed `logs.all`. 4. **No POST on v1** — uses `GET` with `sql` as a query param. No new POST handler. 5. **CLI / self-hosted** — `query_logs` is only registered when the platform implements `queryLogs`; CLI/self-hosted platforms don't, so they keep `get_logs` as their only logs tool and never see `query_logs` at all (see Breaking change above). ## Details - `query_logs` params: `project_id`, `sql`, optional `iso_timestamp_start`/`iso_timestamp_end`. - Timestamps are validated as ISO 8601 with an explicit UTC `Z` suffix or offset (`z.iso.datetime({ offset: true })`), so the constraint is visible in the tool's JSON schema and offset-less (ambiguous local-time) timestamps are rejected up front. - `resolveLogWindow` (shared by `get_logs` and `query_logs`) anchors the default start to 24h before the resolved end, normalizes accepted timestamps to canonical UTC ISO strings, and rejects an inverted/equal window or one exceeding 24h — unit-tested directly in `debugging-tools.test.ts`. - Read-only enforcement of the SQL itself is left to the backend; no client-side SQL parsing. - Tool descriptions carry no environment-specific wording (hosted/self-hosted/CLI) — routing is handled structurally via registration/hiding, not prose the model has to reason about. ## Verification - `tsc --noEmit` clean, `biome ci` clean. - All unit + integration suites pass (incl. `logs.test.ts`, `server.test.ts`, `debugging-tools.test.ts`). The 5 failing `|e2e|` checks need live `SUPABASE_ACCESS_TOKEN` / `ANTHROPIC_API_KEY`, which this fork PR doesn't receive — confirmed passing on an identical-commit mirror pushed directly to the repo (#341). - Manually verified `query_logs` end-to-end against a real staging project (ClickHouse query executed, results returned, untrusted-data wrapping applied). - Eval coverage: supabase/evals#100 (draft, blocked on this shipping in a released `@supabase/mcp-server-supabase` version) verifies the agent successfully uses `query_logs` on a hosted-shaped fixture where it's the only logs tool listed. ## Post-merge follow-ups Per Rodriguespn — this adds a tool and changes `get_logs`'s discoverability, so per [CONTRIBUTING's breaking-change guidance](https://github.com/supabase/mcp/blob/main/CONTRIBUTING.md#breaking-changes): - [ ] Release a new `@supabase/mcp-server-supabase` version (automatic via release-please once merged as `feat!:`) - [ ] Update the tool list at [supabase.com/mcp](https://supabase.com/mcp#available-tools) - [ ] Bump `MCP_SERVER_VERSION` in `supabase/evals` and un-draft evals#100 - [ ] Open a PR on `platform` to add `query_logs` to the remote MCP server - [ ] Update the `@supabase/mcp-server-supabase` version pin inside `studio` - [ ] Resubmit the ChatGPT app to pick up the new tool and reworded `get_logs` description (frozen fields) > [!NOTE] > Per CONTRIBUTING, feature PRs should track an accepted issue — this tracks AI-701.
|
Closing this one as #333 was merged |
Same changes as #333, pushed to a branch on the repo (not a fork) so the e2e tests run with CI secrets — #333's
testcheck can't pass from the fork because forks don't receive secrets.Do not merge this in place of #333 unless we decide to; it exists to verify the e2e suite goes green with secrets available. #333 remains the review PR.
Identical head commit as #333 (
c1bdc7f).