Skip to content

[AskT49 SQL tool] feat(mcp): SQL analysis tools for signed-in users - #481

Open
SylvainGravejat wants to merge 5 commits into
mainfrom
sylvain/dev-13504-mcp-sql-analysis-tools-for-signed-in-users-sql_query
Open

SylvainGravejat wants to merge 5 commits into
mainfrom
sylvain/dev-13504-mcp-sql-analysis-tools-for-signed-in-users-sql_query

Conversation

@SylvainGravejat

@SylvainGravejat SylvainGravejat commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Linear: DEV-13504

Adds read-only SQL analysis to the MCP server for signed-in users whose account has SQL access. It supersedes #436: the SDK manager is cherry-picked from it with its authorship, and most of the tool, discovery and caching code is ported from it onto the tool registry.

What it adds

Four tools in a new analytics toolset:

Tool What it does
query_guide Reads how to measure for a topic, or lists the guides
describe_views Lists the queryable views, or one view's columns and named metrics
sql_query Runs one read-only SELECT over the assistant_* views; returns a sample of rows and a result_id
get_query_result Pages more rows of a stored result

Who gets the tools

  • Only user credentials (OAuth) get analytics. API keys, agent tokens and the shared token never do, even when T49_MCP_TOOLSETS lists it.
  • The gateway lists the tools only after GET /v2/assistant_views confirms the account has SQL access.
    • The answer is cached for 60 s per caller and account. The key is the caller's OAuth token, because the resolved API token changes on every request.
    • A denial hides the tools. A transient failure, or too little time left in the request, keeps them listed. The API authorizes every call anyway.
  • With T49_MCP_TOOLSETS unset, signed-in users get analytics on top of the defaults. With it set, it must list analytics, which makes the variable a kill switch.

How the SQL calls behave

  • Their client never retries and times out after 20 s, so a stalled call returns a tool error rather than hitting the function limit.
  • Identical concurrent queries from the same caller run once when they reach the same instance.
  • The API's messages for rejected queries, unknown guides, missing results and changed access pass through to the model; other failures get fixed text.
  • SQL is limited to 8,000 bytes, as the API counts it.

Docs

  • New page mcp/sql-queries.
  • analytics added to the toolset docs.
  • Regenerated SDK reference.

Tests

  • New: tools/sql-query.test.ts, and gateway cases in tests/api-handler.test.ts.
  • They cover duplicate-query handling, error text, the access-check cache (per account, per caller across requests, transient failures, the deadline), every tool over the MCP transport with payloads shaped like the API's, the byte limit, and the gateway only checking access when analytics is requested.
  • npm run agent-verify passes. The smoke test still lists 30 tools for an API-key session.

Before marking ready

Live check on a Vercel preview against staging:

  • OAuth from Claude lists the four tools for an account with SQL access, and not for one without.
  • API keys never see them.
  • A query, a guide and a page of results work end to end.

The dashboard agent keeps its own SQL tools for now. Moving it to these tools, which means allowing agent credentials, is a separate change.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

RetriggerView in GreptileConfidence Score: 3/5

Fix both timeout gaps before merging so slow upstream calls return errors instead of losing the MCP response.

Fix All in CodexFindings

  1. P1 SQL can outlast the function ▶
  2. P1 Access checks can hang requests ▶
Fix with agent prompt
### Issue 1
packages/mcp/src/tools/sql-query.ts:61
`createSqlClient` starts a fresh 20-second timeout after OAuth resolution and discovery have already used part of the same request. A 12-second resolve followed by a stalled query reaches Vercel's 30-second limit before the query times out. The caller loses the response instead of receiving a tool error.

Pass the request deadline into SQL execution and cap its wait by the time remaining.

### Issue 2
packages/mcp/src/tools/sql-query.ts:216
`probeAssistantSql` does not bound the full access check. The SDK clears its timeout when `fetch` returns response headers, then reads the body separately. If `/assistant_views` sends headers but stalls while sending JSON, this await can run past the probe budget and Vercel's function limit.

The gateway awaits discovery before serving the request, so this also blocks non-SQL calls when the user's selection includes `analytics`. Keep an abort active through the body read, or bound the whole discovery promise.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds four SQL analysis tools for signed-in users, with account access discovery, concurrent-query sharing, an SDK manager, and documentation.

  • MCP agents can query account-wide data through four SQL tools.
  • Only signed-in users with account access can see the analytics tools.
  • The TypeScript SDK can query, describe, and page through account data.

Diagram

sequenceDiagram
  participant Client
  participant Gateway
  participant API
  Client->>Gateway: MCP request with OAuth token
  Gateway->>API: Resolve user and account
  API-->>Gateway: Per-request API token
  Gateway->>API: Discover SQL access when not cached
  API-->>Gateway: Views or access error
  Gateway->>Gateway: Select allowed tools
  Gateway->>API: Run requested SQL tool
  API-->>Gateway: Result or error
  Gateway-->>Client: MCP response
Loading

Reviews (1) · Last reviewed commit: "docs(mcp): add the analytics toolset to ..." · Reviewed by Greptile

alosec and others added 5 commits October 20, 2026 22:52
client.assistantQueries wraps the user-scoped /v2/assistant_queries,
/v2/assistant_views and /v2/assistant_guides endpoints through
Transport.executeManual, so they share the SDK's auth, X-Account-ID,
error mapping and timeout path. Calls require accountId; POST queries
are not retried. These endpoints sit outside the API-key OpenAPI
contract, so their response types live in the manager.

Rebuilt from #436 onto current main.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Adds sql_query, describe_views, query_guide and get_query_result in a new
analytics toolset. They call the Terminal49 API's assistant query, view and
guide endpoints, which authorize every call.

The gateway serves analytics only to signed-in (OAuth) users, and only after
GET /v2/assistant_views confirms the account has SQL access. That answer is
cached for a minute per credential and account; a denial hides the tools and a
transient failure keeps them listed. API keys, agent tokens and the shared
token never get them, and an operator allowlist without analytics turns them
off.

The SQL tools use a client that never retries, run identical concurrent
queries once, pass the API's messages through for rejected queries and unknown
topics, and stop the model on rate limits and service failures.

Co-Authored-By: Alex Garcia <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The gateway mints a new API token for each signed-in request, so the SQL
access check and the duplicate-query check now key on the caller's own token;
before, neither could ever hit. The access check fits within the request
deadline, and SQL calls time out after 20 seconds instead of running into the
function limit. The SQL length limit counts bytes, as the API does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Drops the CSV download advice, which MCP clients can't use, and no longer
promises that duplicate queries always run once.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@vercel

vercel Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
api Ready Ready Preview Oct 20, 2026 9:27pm UTC

Request Review

@mintlify

mintlify Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
terminal49 🟢 Ready View Preview Oct 20, 2026, 9:27 PM

@linear-code

linear-code Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

DEV-13504

@github-actions github-actions Bot added the risk-shadow:high Shadow-only risk signal: high label Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Shadow risk calibration

Signal Result Authority
Path classifier risk:high Deterministic advisory policy
heuristic fallback risk-shadow:high Shadow only

Calibration result: agreement.

Shadow risk never blocks or enables merge. Keep it in observation mode until roughly 50 pull requests have human-reviewed labels, then calibrate before considering any policy use.

@github-actions github-actions Bot added risk:unknown Advisory evidence unavailable or changed; no merge permission risk:high Advisory path risk; current-head human review needed and removed risk:unknown Advisory evidence unavailable or changed; no merge permission risk:high Advisory path risk; current-head human review needed labels Oct 20, 2026
@github-actions github-actions Bot added the risk:high Advisory path risk; current-head human review needed label Oct 22, 2026
@dodeja
dodeja marked this pull request as ready for review October 23, 2026 04:43
@dodeja
dodeja self-requested a review as a code owner October 23, 2026 04:43
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T04:46:16.661699Z dd1c1d6 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd1c1d6b47

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +78 to +82
let request = pendingQueries.get(key);
if (!request) {
request = client.assistantQueries
.query(sql)
.finally(() => pendingQueries.delete(key));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include the resolved credential in SQL deduplication

When identical OAuth SQL requests overlap, the key uses the stable caller OAuth identity rather than the newly minted API credential, so the later request can reuse the earlier promise without making its own authorized API call. If the user's data access is reduced between the two resolutions, the later request can therefore receive rows fetched under the prior access state. Include the resolved credential or an access-version identifier in the key, or limit deduplication to a single request.

AGENTS.md reference: AGENTS.md:L89-L92

Useful? React with 👍 / 👎.

apiBaseUrl,
accountId,
maxRetries: 0,
timeoutMs: SQL_CALL_TIMEOUT_MS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 SQL can outlast the function

createSqlClient starts a fresh 20-second timeout after OAuth resolution and discovery have already used part of the same request. A 12-second resolve followed by a stalled query reaches Vercel's 30-second limit before the query times out. The caller loses the response instead of receiving a tool error.

Pass the request deadline into SQL execution and cap its wait by the time remaining.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp/src/tools/sql-query.ts
Line: 61

Comment:
**SQL can outlast the function**

`createSqlClient` starts a fresh 20-second timeout after OAuth resolution and discovery have already used part of the same request. A 12-second resolve followed by a stalled query reaches Vercel's 30-second limit before the query times out. The caller loses the response instead of receiving a tool error.

Pass the request deadline into SQL execution and cap its wait by the time remaining.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code

maxRetries: 0,
timeoutMs,
});
const result = await client.assistantQueries.views();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Access checks can hang requests

probeAssistantSql does not bound the full access check. The SDK clears its timeout when fetch returns response headers, then reads the body separately. If /assistant_views sends headers but stalls while sending JSON, this await can run past the probe budget and Vercel's function limit.

The gateway awaits discovery before serving the request, so this also blocks non-SQL calls when the user's selection includes analytics. Keep an abort active through the body read, or bound the whole discovery promise.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/mcp/src/tools/sql-query.ts
Line: 216

Comment:
**Access checks can hang requests**

`probeAssistantSql` does not bound the full access check. The SDK clears its timeout when `fetch` returns response headers, then reads the body separately. If `/assistant_views` sends headers but stalls while sending JSON, this await can run past the probe budget and Vercel's function limit.

The gateway awaits discovery before serving the request, so this also blocks non-SQL calls when the user's selection includes `analytics`. Keep an abort active through the body read, or bound the whole discovery promise.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code

@github-actions github-actions Bot added risk:unknown Advisory evidence unavailable or changed; no merge permission risk:high Advisory path risk; current-head human review needed and removed risk:high Advisory path risk; current-head human review needed risk:unknown Advisory evidence unavailable or changed; no merge permission labels Oct 25, 2026

This branch was successfully deployed

2 active deployments
staging - docs — dd1c1d6b Deployed Oct 20, 2026 by mintlify[bot]
Preview — dd1c1d6b Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:high Advisory path risk; current-head human review needed risk-shadow:high Shadow-only risk signal: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants