feat(self-hosted): implement queryLogs for the MCP debugging tools - #48900
feat(self-hosted): implement queryLogs for the MCP debugging tools#48900Rodriguespn wants to merge 3 commits into
Conversation
Declare `logsDialect: 'bigquery'` and implement `queryLogs` in the self-hosted `getDebuggingOperations`. `queryLogs` passes the model's SQL straight through to the same Logflare `logs.all` endpoint `getLogs` already uses, which accepts an arbitrary `sql` param. Pairs with supabase/mcp#357: once the platform declares its dialect, the MCP server exposes `query_logs` (with the BigQuery-dialect description) and hides `get_logs` on self-hosted/CLI, matching hosted. Refs AI-1046
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe debugging MCP flow now uses ChangesQuery logs MCP migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant getDebuggingOperations
participant LogsFeatureGate
participant retrieveAnalyticsData
MCPClient->>getDebuggingOperations: queryLogs(projectRef, SQL, timestamp filters)
getDebuggingOperations->>LogsFeatureGate: validate logs availability
LogsFeatureGate-->>getDebuggingOperations: enabled
getDebuggingOperations->>retrieveAnalyticsData: request logs.all with SQL and filters
retrieveAnalyticsData-->>getDebuggingOperations: analytics log results
getDebuggingOperations-->>MCPClient: query results
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
🎭 Playwright Test Results (next)Details
Skipped testsFeatures › auth-users.spec.ts › should show web3 users as enabled when the matching web3 provider is enabled |
…ging tools Bump @supabase/mcp-server-supabase to ^0.10.0, which adds query_logs and (when a platform declares queryLogs) hides get_logs. - Honor no-logs mode: query_logs throws when logs:all is disabled — the self-hosted default, enabled via the docker-compose.logs.yml override. - Remove get_logs from self-hosted: getLogs now throws (the server hides it once queryLogs exists) and the per-service getLogQuery SQL builder is deleted; the model writes its own BigQuery SQL via query_logs instead. - Switch the dashboard assistant from get_logs to query_logs (allowlist, drift guard, prompt, mocks, evals). Refs AI-1046 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/studio/lib/api/self-hosted/mcp.ts (1)
121-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
Feature[]cast.
getEnabledFeaturesOverrideDisabledList()filters environment variables againstknownFeatureKeys, so it returns only known feature keys. Expose that return value asFeature[]instead of casting it at this call site. No additional runtime validation is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/studio/lib/api/self-hosted/mcp.ts` at line 121, Update getEnabledFeaturesOverrideDisabledList to declare and return Feature[] based on its knownFeatureKeys filtering, then remove the redundant Feature[] cast from the disabledFeatures assignment. Preserve the existing runtime behavior without adding validation.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/studio/lib/api/self-hosted/mcp.test.ts`:
- Around line 126-137: Update the queryLogs test to pass both
iso_timestamp_start and iso_timestamp_end in the request, then assert the exact
params object forwarded to retrieveAnalyticsDataMock, including sql and both
timestamp bounds. Keep the existing logs-enabled setup and queryLogs invocation
coverage unchanged.
---
Nitpick comments:
In `@apps/studio/lib/api/self-hosted/mcp.ts`:
- Line 121: Update getEnabledFeaturesOverrideDisabledList to declare and return
Feature[] based on its knownFeatureKeys filtering, then remove the redundant
Feature[] cast from the disabledFeatures assignment. Preserve the existing
runtime behavior without adding validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fcbfbe9b-3ce2-45a2-a13f-ec4a8e360c99
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (12)
apps/studio/evals/dataset.tsapps/studio/lib/ai/prompts.tsapps/studio/lib/ai/tool-filter.test.tsapps/studio/lib/ai/tool-filter.tsapps/studio/lib/ai/tools/mcp-tools.test.tsapps/studio/lib/ai/tools/mcp-tools.tsapps/studio/lib/ai/tools/mock-tools.test.tsapps/studio/lib/ai/tools/mock-tools.tsapps/studio/lib/api/self-hosted/logs.tsapps/studio/lib/api/self-hosted/mcp.test.tsapps/studio/lib/api/self-hosted/mcp.tsapps/studio/package.json
💤 Files with no reviewable changes (1)
- apps/studio/lib/api/self-hosted/logs.ts
| it('queryLogs passes the model-provided SQL straight through when logs are enabled', async () => { | ||
| vi.stubEnv('ENABLED_FEATURES_LOGS_ALL', 'true') | ||
| retrieveAnalyticsDataMock.mockResolvedValue({ data: ['log entry'], error: null }) | ||
|
|
||
| const ops = getDebuggingOperations({}) | ||
| await ops.queryLogs!('default', { sql: 'select * from edge_logs' }) | ||
|
|
||
| expect(retrieveAnalyticsDataMock).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| params: expect.objectContaining({ sql: 'select * from edge_logs' }), | ||
| }) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test timestamp forwarding.
The new request contract includes iso_timestamp_start and iso_timestamp_end, but this test only asserts sql. Pass both bounds and assert the exact params object. This guards the forwarding at apps/studio/lib/api/self-hosted/mcp.ts Lines 153-155.
As per coding guidelines, “Include relevant tests by default.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/studio/lib/api/self-hosted/mcp.test.ts` around lines 126 - 137, Update
the queryLogs test to pass both iso_timestamp_start and iso_timestamp_end in the
request, then assert the exact params object forwarded to
retrieveAnalyticsDataMock, including sql and both timestamp bounds. Keep the
existing logs-enabled setup and queryLogs invocation coverage unchanged.
Source: Coding guidelines
query_logs supersedes get_logs in the MCP debugging tools; on hosted projects get_logs is already hidden in favor of query_logs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
czenko
left a comment
There was a problem hiding this comment.
Approved the documentation piece
|
|
||
| if (!isFeatureEnabled('logs:all', disabledFeatures)) { | ||
| throw new Error( | ||
| 'Logs are disabled on this instance. Enable the `docker-compose.logs.yml` override to query logs.' |
There was a problem hiding this comment.
CLI was planning to disable logs by default as well. When/if that happens, this error message should be adjusted :)
| async getLogs(projectRef: string, options: GetLogsOptions) { | ||
| const sql = getLogQuery(options.service) | ||
| // Self-hosted logs are served by Logflare, which speaks BigQuery SQL. | ||
| logsDialect: 'bigquery', |
Important
Only merge this when (https://github.com/supabase/platform/pull/36804) is merged, as the AI assistant will not have access to the
query_logstool for the remote MCP serverI have read the CONTRIBUTING.md file.
YES
What kind of change does this PR introduce?
Feature (self-hosted / CLI Studio MCP server).
What is the current behavior?
Self-hosted
getDebuggingOperations(apps/studio/lib/api/self-hosted/mcp.ts) implements onlygetLogs, so the MCPdebugginggroup exposesget_logs— a fixed per-service log dump built bygetLogQuery. Logs are served by Logflare, which speaks BigQuery SQL.What is the new behavior?
Bumps
@supabase/mcp-server-supabaseto^0.10.0(addsquery_logs+logsDialect, and hidesget_logswherever a platform declaresqueryLogs) and moves logs over to it.query_logs: declareslogsDialect: 'bigquery'and implementsqueryLogs, passing the model's SQL straight through to the same Logflarelogs.allendpoint (arbitrarysqlparam) — no new endpoint, no dialect translation.get_logsfrom self-hosted:getLogsthrows (the server hides it oncequeryLogsexists) and the per-servicegetLogQuerybuilder is deleted; the model now writes its own BigQuery SQL, guided by the dialect schema hint.query_logsthrows whenlogs:allis disabled — the self-hosted default, enabled via thedocker-compose.logs.ymloverride.get_logstoquery_logs(allowlist, drift guard, prompt, mocks, evals).Refs AI-1046
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests