Skip to content

feat: GoSQLX Claude Code plugin and ChatGPT app - #823

Merged
ajitpratap0 merged 9 commits into
mainfrom
feat/ai-assistant-plugins
Oct 21, 2026
Merged

ajitpratap0 merged 9 commits into
mainfrom
feat/ai-assistant-plugins

Conversation

@ajitpratap0

Copy link
Copy Markdown
Owner

Summary

Ships GoSQLX as a Claude Code plugin and a ChatGPT app, both built on the existing gosqlx-mcp server, plus the legal pages the ChatGPT app directory requires.

Description

  • Claude Code plugin + marketplace (.claude-plugin/marketplace.json, plugins/gosqlx/): /plugin marketplace add ajitpratap0/GoSQLX → /plugin install gosqlx@gosqlx. Hosted MCP server by default (GOSQLX_MCP_URL overrides for local), 5 skills (analysis, formatting, security review, linting, dialects), /gosqlx:analyze|format|lint|security, read-only sql-reviewer agent, opt-out PostToolUse hook validating edited .sql files when the gosqlx CLI exists.
  • ChatGPT app (OpenAI Apps SDK + MCP Apps) in pkg/mcp, cmd/gosqlx-mcp:
    • Embedded, self-contained analyze_sql widget ui://gosqlx/analysis-v1.html (text/html;profile=mcp-app, empty CSP, data rendered via textContent only); tool meta in both MCP Apps (ui.resourceUri) and openai/* forms.
    • All tools return structuredContent alongside the byte-identical text JSON.
    • Server instructions, securitySchemes: noauth (only when auth is off), invocation status strings.
    • validate_sql accepts mariadb/clickhouse; server version from gosqlx.Version; /health reports the real tool count; optional /.well-known/openai-apps-challenge (GOSQLX_MCP_OPENAI_APPS_CHALLENGE, declared sync: false in render.yaml); ReadHeaderTimeout.
    • Idle MCP sessions swept after 30 min (WithSessionIdleTTL) — previously sessions from clients that never sent DELETE lived until restart.
  • Website: new /terms page (footer + sitemap); privacy policy rewritten for the hosted server / ChatGPT app / Claude plugin (removes the inaccurate "stateless" claim, documents in-memory session + rate-limit state, Render/Cloudflare as processors).
  • Docs: integrations/chatgpt/README.md (setup + directory submission checklist), docs/CHATGPT_APP.md, plugin README, README, MCP guide (also fixes the Claude Desktop example missing --stdio), CHANGELOG.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Documentation update

Test Plan

  • task check (fmt, vet, golangci-lint 0 issues, test:race) passes
  • New tests: pkg/mcp/apps_test.go (resources/list+read, widget meta, tool meta, structuredContent == text JSON), cmd/gosqlx-mcp/main_test.go (health, challenge route), config test for the challenge env var
  • End-to-end curl smoke test over streamable HTTP: initialize → tools/list → resources/read → tools/call analyze_sql
  • claude plugin validate --strict on marketplace and plugin; hook script exercised with valid/invalid/missing-CLI/no-jq/space-in-path inputs
  • Website: typecheck, eslint, npm test, check:docs, next build (/terms prerendered)
  • Widget render in ChatGPT developer mode and Claude — needs this deployed to mcp.gosqlx.dev first

Breaking Changes

None. Existing MCP clients receive identical text content; new fields are additive. Behavior notes: validate_sql enum gains two dialects; idle sessions now expire after 30 min (clients re-initialize transparently).

Performance Impact

  • No parser/tokenizer hot-path changes. Tool results now also carry structuredContent, roughly doubling payload size for large analyze_sql responses.

Additional Notes

Before ChatGPT directory submission (maintainer): legal review of /terms and /privacy (no governing-law clause yet), set the challenge token in Render, test the widget in developer mode, prepare logo/screenshots/demo video. securitySchemes is emitted in _meta only (mcp-go v0.45.0 cannot emit it at the tool top level).

🤖 Generated with Claude Code

https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

Ajit Pratap Singh and others added 3 commits October 20, 2026 00:42
Embed an analyze_sql report widget (ui://gosqlx/analysis-v1.html), return
structuredContent from all tools with unchanged text output, add server
instructions, noauth securitySchemes and invocation status metadata, accept
mariadb/clickhouse in validate_sql, report gosqlx.Version and the real tool
count, serve the optional OpenAI apps domain challenge, and sweep idle MCP
sessions after 30 minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w
Install with /plugin marketplace add ajitpratap0/GoSQLX and
/plugin install gosqlx@gosqlx: hosted MCP server, five SQL skills, four
slash commands, a read-only sql-reviewer agent and an opt-out .sql
validation hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w
Add gosqlx.dev/terms, describe hosted MCP server, ChatGPT app and Claude
plugin data handling accurately in the privacy policy, document both
integrations in README, MCP guide and CHANGELOG, and fix the Claude Desktop
stdio example.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w
@vercel

vercel Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
gosqlx-website Ready Ready Preview Oct 21, 2026 10:10am UTC

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 19, 2026
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 26s —— View job


Review of PR #823: Claude Code plugin and ChatGPT app

I reviewed the Go changes (pkg/mcp/*, cmd/gosqlx-mcp) and the plugin hook and config. I did not run the tests and only skimmed the website, docs and widget HTML. Overall the PR is clean, additive and well structured. I found no blocking bugs.

What looks good

  • toolResult now returns NewToolResultStructured(data, string(b)), so the text content stays identical and existing clients are unaffected.
  • newMux is split out of runHTTP, which makes the /health and challenge routes testable. The challenge route is only registered when a token is configured.
  • ReadHeaderTimeout closes a Slowloris gap.
  • Server version comes from gosqlx.Version and the tool count from srv.ToolCount(), so neither can drift from the code.
  • The widget declares an empty CSP and renders data via textContent, which keeps XSS risk low.
  • The hook script is careful: it exits 0 when the CLI is missing, uses -- before the path, bails on JSON escapes in the no-jq fallback, and has an opt-out.

Suggestions and nits (non-blocking)

  1. Plugin privacy default (plugins/gosqlx/.mcp.json). The plugin sends the user's SQL to https://mcp.gosqlx.dev by default. That is documented in the privacy page, but the plugin README should say so near the install steps. A line about setting GOSQLX_MCP_URL to a local gosqlx-mcp instance for proprietary schemas would help.

  2. Server.Start skips rate limiting (pkg/mcp/server.go, Start). The binary's runHTTP path wraps the handler in RateLimitMiddleware, but Start() and Handler() don't. This predates the PR. Since newMux now holds the canonical wiring, consider documenting it on Start() or moving the rate limiter into Handler().

  3. Doubled payload. structuredContent roughly doubles response size for large analyze_sql results. The PR notes this. Check that RateLimitMiddleware or any body-size limit is not tuned to response size. Otherwise this is fine.

  4. Session TTL behaviour. A 30-minute idle sweep is reasonable. Confirm that WithSessionIdleTTL counts activity on a long-lived GET/SSE stream. If it only counts POSTs, an idle-but-connected client could lose its session. Clients re-initialize, but this is worth a test or a note.

  5. Hook exit code. exit 2 on a PostToolUse failure surfaces stderr to Claude, which is the intent. The header comment says "never blocks a tool call", but a file that was already written can still be flagged on every later edit. That is acceptable, but a line in the README about GOSQLX_HOOK_DISABLE=1 for projects with dialect-specific .sql files would reduce friction. The hook validates with the generic dialect by default.

  6. Doc drift risk. The lint tool description hard-codes "10 rules (L001–L010)", while CLAUDE.md and the README describe 30 linter rules. This is accurate for the MCP tool, which uses only L001–L010. A short "MCP lint covers the 10 whitespace/style rules" note in the docs would avoid user confusion.

  7. Tests. The new coverage is good: resources list/read, meta, structuredContent == text JSON, health, challenge, and config. Two gaps:

    • a test that securitySchemes is absent when auth is enabled (the logic in toolMeta has two branches);
    • a test that /.well-known/openai-apps-challenge returns 404 when no token is set.
  8. Pre-submission items (already in your notes). The /terms page has no governing-law clause, so it needs legal review. The widget still hasn't been rendered in ChatGPT developer mode or Claude, so that checkbox stays open until the server is deployed.

No security concerns beyond the above. The challenge token is public by design, so the plain string compare/serve is fine.

Name the maintainer as controller and contracting party, add a private
contact and grievance officer, legal bases, recipients (incl. Vercel),
international transfers, retention, security, user rights, children,
DPDP Act 2023 and GDPR references; terms gain eligibility, liability cap,
consumer carve-out, indemnity, termination, Indian governing law,
boilerplate and conspicuous disclaimers. Correct the VS Code telemetry
opt-in text and replace its dead PRIVACY.md link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 18s —— View job


Review of PR #823: Claude Code plugin and ChatGPT app

I read the diffs for pkg/mcp/server.go, cmd/gosqlx-mcp/main.go and the hook script. I did not run the tests or build. The PR is additive and well structured, and I found no blocking bugs. The earlier review's points still apply, and I've added a few more below.

What looks good

  • newMux is split out of runHTTP, so /health and the challenge route can be tested. The challenge route is only registered when a token is set.
  • The server version comes from gosqlx.Version and the tool count from srv.ToolCount(), so neither can drift. The old hard-coded "tools":7 is gone.
  • ReadHeaderTimeout: 10s closes a Slowloris gap.
  • The tool annotations are explicit and shared through readOnlyAnnotations, and each tool description now says when to use it.
  • The hook script is careful. It uses -- before the path, exits 0 when the CLI is missing, bails on JSON escapes in the no-jq fallback, and has an opt-out.

Suggestions (non-blocking)

  1. Start() and Handler() skip rate limiting. newMux wraps the handler in RateLimitMiddleware, but Server.Start in pkg/mcp/server.go and Handler() do not. This was already the case before the PR. Embedders using Start() therefore get no rate limit. Either document it on Start() or apply the limiter in Handler().

  2. Session idle TTL and long-lived streams. SessionIdleTTL is 30 minutes. Please confirm that mcp-go's WithSessionIdleTTL counts activity on an open GET/SSE stream, not only POSTs. If it doesn't, a connected but quiet client could lose its session. A test or a code comment would settle it.

  3. Missing test branches.

    • securitySchemes should be absent when auth is enabled (toolMeta has two branches).
    • /.well-known/openai-apps-challenge should return 404 when no token is configured.
  4. Plugin privacy default. plugins/gosqlx/.mcp.json sends SQL to mcp.gosqlx.dev by default. State this near the install steps in plugins/gosqlx/README.md, along with how to set GOSQLX_MCP_URL to a local gosqlx-mcp for proprietary schemas.

  5. Hook friction. The header says the hook "never blocks a tool call", yet exit 2 surfaces an error to Claude on every later edit of a file that fails generic-dialect validation. Mention GOSQLX_HOOK_DISABLE=1 in the README for projects with dialect-specific .sql files.

  6. Lint rule count. The lint_sql description says "10 rules (L001–L010)", while the README and CLAUDE.md say 30 rules. The MCP description is correct, but a short note in the docs would stop users from being confused.

  7. Doubled payload. structuredContent roughly doubles large analyze_sql responses. Check that no body or response-size limit is tuned to the old size.

Before directory submission (already in your notes)

  • Get legal review of /terms and /privacy, since there is no governing-law clause yet.
  • Set the challenge token in Render.
  • Render the widget in ChatGPT developer mode and Claude after deploying.

Security: I have no concerns beyond the above. The challenge token is public by design, so serving it as plain text is fine. The widget's empty CSP and textContent-only rendering keep XSS risk low.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 20s —— View job


Review of PR #823: Claude Code plugin and ChatGPT app

I read the diffs for pkg/mcp/server.go and cmd/gosqlx-mcp/main.go, and grepped the tests. I did not run the build or tests. I found no blocking issues.

What looks good

  • newMux is split out of runHTTP, which makes the routes testable. The challenge route is only registered when a token is configured, so other paths return 404.
  • The version comes from gosqlx.Version and the tool count from srv.ToolCount(). The hard-coded "tools":7 and the "1.14.0" literal are gone.
  • ReadHeaderTimeout: 10s closes a Slowloris gap.
  • The newTool/readOnlyAnnotations helpers remove the repeated annotation boilerplate, and the descriptions now say when to use each tool.
  • The server instructions and the lint_sql description list the 10 rules (L001–L010) it applies, which avoids confusion with the 30-rule CLI linter.

Corrections to the earlier automated reviews

  • Both earlier reviews say there is no test that securitySchemes is absent when auth is enabled. pkg/mcp/apps_test.go:251-261 already covers both branches, so no change is needed.
  • The one test gap I did find is that cmd/gosqlx-mcp/main_test.go has no 404 check for /.well-known/openai-apps-challenge when no token is set. A short test for it would be worthwhile.

Suggestions (non-blocking)

  1. Start() and Handler() have no rate limiting. Only newMux wraps the handler in RateLimitMiddleware. This predates the PR, but a doc comment on Start() and Handler() would stop embedders assuming it is applied.
  2. Session idle TTL. Please confirm that mcp-go's WithSessionIdleTTL counts activity on an open GET/SSE stream, not only POSTs. If it doesn't, a quiet but connected client could lose its session. A code comment or a test would settle it. Clients re-initialize, so the impact is low.
  3. Stale comment. The registerTools doc in server.go still says "all 7 GoSQLX tools". That is accurate today but is the same kind of count that was just made dynamic elsewhere.
  4. Plugin docs. Say near the install steps in plugins/gosqlx/README.md that SQL goes to mcp.gosqlx.dev by default. Also mention GOSQLX_MCP_URL for a local server and GOSQLX_HOOK_DISABLE=1 for projects with dialect-specific .sql files.
  5. Doubled payload. structuredContent roughly doubles large analyze_sql responses. I found no response-size limit that depends on the old size in the code I read, but this is worth a quick check on the Render side.

Before directory submission (already in your notes)

  • Get legal review of /terms and /privacy. There is no governing-law clause yet.
  • Set the challenge token in Render.
  • Render the widget in ChatGPT developer mode and Claude after deploying.

Security: I have no concerns beyond the above. The challenge token is public by design. The empty CSP and textContent-only widget rendering keep XSS risk low.

@ajitpratap0

Copy link
Copy Markdown
Owner Author

Independent review (Opus + Sonnet)

Verdict: changes requested before merge.

Medium

1. Session IDs are retained and logged, contradicting the privacy policy (pkg/mcp/server.go)

  • The idle-session sweep (WithSessionIdleTTL(30m)) calls InsecureStatefulSessionIdManager.Terminate in mcp-go v0.45.0. Terminate moves the ID from sessions into a terminated sync.Map that is never pruned (streamable_http.go:1516-1526). The DELETE path does the same. Every session ID stays in memory until the process restarts, so the map grows without bound on a public no-auth server.
  • The sweeper logs each ID through the default std logger (Sweeping expired session: <id>, line 951), and Render keeps those logs.
  • website/src/app/privacy/page.tsx:86-89 and integrations/chatgpt/README.md:111-113 say the session identifier is "deleted after 30 minutes of inactivity" and "never written to disk". Neither claim holds. This page is a legal statement submitted for ChatGPT app review.
  • The per-session transport state is freed, which is a real improvement.
  • Suggested fix: Use mcpserver.WithStateLess(true). The tools are stateless, so this removes session IDs, the sweeper, and the unbounded map. Alternatively, pass a custom SessionIdManager whose Terminate deletes the ID instead of recording it, or keep a TTL-bounded terminated set. Also pass WithLogger with a logger that drops the per-session Infof line, or reword the privacy page and the ChatGPT README to describe what is actually retained and logged. Add a test asserting the session store is empty after a sweep or DELETE.

Low

2. Release notes edited from a feature PR (CHANGELOG.md, ~lines 215-231)

  • The PR adds entries under ## [Unreleased] → ### Added. The release session owns that section, so this risks merge conflicts and duplicate release notes.
  • Suggested fix: Remove the CHANGELOG hunk and put the text in the PR body under "Release note (for the release session)".

3. Shared rate-limit budget once ChatGPT and claude.ai traffic arrives (pkg/mcp/ratelimit.go)

  • This code predates the PR, but the PR makes it matter. The limiter keys on the first X-Forwarded-For entry (fallback RemoteAddr) with burst 10 rps and 120 weighted requests/min per IP. Hosted assistant connector traffic comes from a small set of provider egress IPs, so once the app is listed all users share one budget and get throttled together. The first XFF entry is also client-controlled, so per-IP limits are easy to evade.
  • Suggested fix: Track as a follow-up issue. Key on the right-most trusted-proxy XFF hop (Render or Cloudflare CF-Connecting-IP). Consider a higher or separate limit for known assistant egress ranges, or key on Mcp-Session-Id together with the IP. Mention the shared limit in docs/CHATGPT_APP.md.

4. No body or idle timeout on the HTTP server (cmd/gosqlx-mcp/main.go)

  • Only ReadHeaderTimeout was added. A client can trickle a POST body of up to 64KB and hold the goroutine and connection open indefinitely. WriteTimeout is correctly unset because of SSE.
  • Suggested fix: Add IdleTimeout (e.g. 120s). Add a body read deadline for POST /mcp using http.ResponseController.SetReadDeadline in the rate-limit middleware, or set ReadTimeout and accept that it also covers long GET SSE requests.

5. Greedy file_path match in the fallback hook (plugins/gosqlx/hooks/validate-sql.sh)

  • Minor accuracy issue. The no-jq fallback uses a greedy .*"file_path" sed match, which takes the last unescaped file_path key, not the first as the comment says. Injection checks passed: escaped occurrences inside old_string or content do not match, paths with $(...) or spaces are quoted, and paths starting with a dash are handled via gosqlx validate --. Failures exit 0 except real validation errors, which exit 2.
  • Suggested fix: Correct the comment, or make the sed non-greedy by anchoring on "tool_input"[^}]*"file_path".

6. Pre-existing lint warnings (website/src/components/ui/ThemeToggle.tsx)

  • Lint shows 7 warnings, all pre-existing react-hooks/set-state-in-effect (e.g. ThemeToggle.tsx:113). None are in PR files. 0 errors. Not blocking.

7. Contact mailbox unconfirmed (website/src/app/privacy/page.tsx)

  • Privacy and terms pages hard-code privacy@gosqlx.dev in two separate CONTACT_EMAIL constants. The maintainer should confirm the mailbox exists. Not blocking.

Informational

8. Rate-limit client IP comes from RemoteAddr or forwarded headers behind Cloudflare. The policy's "per-IP counters" claim is accurate, and the 30-minute session TTL and 10-minute rate-limit staleness match the code (pkg/mcp/server.go SessionIdleTTL, ratelimit.go staleTimeout).


Blocking item: finding 1. Findings 2 and 4 are strongly recommended before merge. The rest can be follow-ups.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

…t_sql

- widget: cap long lists (counted statement types, +N more), show failed
  tool calls as errors on both host bridges, skip clipboard API when the
  frame's permissions policy blocks it
- server: one streamable transport per Server reused by Handler/Start,
  new Server.Shutdown, Start gains read-header/idle timeouts, transport
  logger drops Info so session IDs are never logged
- format_sql: uppercase_keywords=true also uppercases operator/function
  keywords; option semantics documented precisely
- docs/privacy: session state is swept 30-45 minutes after last request
  and IDs are not tracked
- tests: full HTTP chain via newMux, session TTL wiring, sweeper leak,
  widget script structure, constants and metadata keys

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 32s —— View job


Review of PR #823 (current head 48bda6e)

I read pkg/mcp/server.go and plugins/gosqlx/hooks/validate-sql.sh, and checked the CHANGELOG. I did not run tests or builds. git diff origin/main...HEAD failed with "no merge base" in this checkout, so I reviewed the files at head rather than a diff. I did not read the mcp-go source, so I haven't verified the terminated map claim in the 20:15 review.

Where the earlier findings stand

  • Session retention and logging (finding 1):
    • Head sets WithLogger(s.transportLogger) (server.go:119) and documents that the transport is stateless: IDs are "random, not recorded", and a sweep or DELETE frees memory without invalidating an ID (server.go:106-108).
    • That reads as a fix for the logging half. I couldn't confirm it for the unbounded terminated map.
    • Please make sure a test asserts the store is empty after a sweep or DELETE, and that website/src/app/privacy/page.tsx and integrations/chatgpt/README.md say only what the code does.
  • IdleTimeout (finding 4): IdleTimeout: 120s is now set (server.go:183). A body-read deadline for POST /mcp is still missing. That is a follow-up, not a blocker.

Still open

  1. Hook comment is wrong (validate-sql.sh, fallback branch).
    • The comment says "take the first file_path", but s/.*"file_path"…/ is greedy and takes the last one.
    • Fix the comment, or anchor the pattern on "tool_input"[^}]*"file_path".
    • The jq path is correct.
  2. CHANGELOG edit from a feature PR. The PR adds entries under ## [Unreleased], and that section is owned by the release session. Move the text into the PR body to avoid merge conflicts and duplicate release notes.
  3. Rate-limit budget is shared across users.
    • RateLimitMiddleware predates this PR, but it matters once ChatGPT and claude.ai traffic arrives from a small set of egress IPs.
    • It also keys on the client-controlled first X-Forwarded-For entry.
    • Track this as a follow-up issue and mention the shared limit in docs/CHATGPT_APP.md.
  4. Doc comments. Say on Start() and Handler() that neither applies rate limiting. Only newMux in cmd/gosqlx-mcp does.
  5. Missing test. cmd/gosqlx-mcp/main_test.go needs a 404 check for /.well-known/openai-apps-challenge when no token is set. A securitySchemes-absent-with-auth test already exists in pkg/mcp/apps_test.go.
  6. Plugin README. State near the install steps that SQL goes to mcp.gosqlx.dev by default, and document GOSQLX_MCP_URL and GOSQLX_HOOK_DISABLE=1.
  7. Contact mailbox. privacy@gosqlx.dev is hard-coded in two CONTACT_EMAIL constants (privacy and terms). Confirm the mailbox exists and share one constant.

Positives: structuredContent stays byte-identical to the text JSON, so existing clients are unaffected. The widget uses an empty CSP and textContent-only rendering. The server version and tool count are no longer hard-coded.

Before merge: items 1 and 2 are small. Item 3 can be a follow-up issue. The directory-submission items already in the PR notes still apply: legal review, challenge token, widget test.
· branch feat/ai-assistant-plugins

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 24s —— View job


Review of PR #823 (head cb0051a)

I spot-checked pkg/mcp/server.go and plugins/gosqlx/hooks/validate-sql.sh. I did not run the build or tests, and I did not read the mcp-go source.

Addressed since the earlier reviews

  • The transport logger now drops Infof, so session IDs are no longer logged (server.go:143-156).
  • IdleTimeout: 120s is set (server.go:183).

Still open

  1. Hook comment is wrong (validate-sql.sh:22-25). The comment says "first file_path", but s/.*"file_path"…/ is greedy and takes the last one. Fix the comment, or anchor the match on "tool_input"[^}]*"file_path".
  2. CHANGELOG edit. The PR adds entries under [Unreleased], which the release session owns. Move that text into the PR body to avoid conflicts.
  3. Terminated-session map. I could not confirm that mcp-go's terminated map stays bounded after a sweep or DELETE. Add a test that the session store is empty afterwards. Make sure website/src/app/privacy/page.tsx and integrations/chatgpt/README.md claim only what the code does.
  4. Missing test. cmd/gosqlx-mcp/main_test.go has no 404 check for /.well-known/openai-apps-challenge when no token is set.
  5. Rate limiting.
    • Start() and Handler() don't apply it, so add doc comments saying so.
    • Limits are keyed on the client-controlled first X-Forwarded-For entry, and ChatGPT and claude.ai traffic will share a small set of egress IPs. Track this as a follow-up issue and note it in docs/CHATGPT_APP.md.
  6. Body read deadline. POST /mcp has no body read deadline, so a slow client can hold a connection open. This is a follow-up.
  7. Plugin README. Say near the install steps that SQL goes to mcp.gosqlx.dev by default. Document GOSQLX_MCP_URL and GOSQLX_HOOK_DISABLE=1.
  8. Contact mailbox. Confirm that privacy@gosqlx.dev exists, and share one CONTACT_EMAIL constant between the privacy and terms pages.

Before directory submission: get legal review of /terms and /privacy, set the challenge token in Render, and render the widget in ChatGPT developer mode and Claude.

Items 1–3 are worth fixing before merge. The rest can be follow-ups.

@ajitpratap0

Copy link
Copy Markdown
Owner Author

Correction to my earlier review

In my review, medium finding 1 claimed that session IDs pile up forever in mcp-go's terminated map. That claim is wrong. I'm withdrawing that part.

  • The never-pruned terminated sync.Map belongs to InsecureStatefulSessionIdManager (streamable_http.go:1487-1526). That manager is only used when WithStateful is set (:82-87).
  • This PR doesn't set WithStateful. NewStreamableHTTPServer therefore uses StatelessGeneratingSessionIdManager (:218). Its Validate and Terminate record nothing (:1460-1482).
  • The idle sweep from WithSessionIdleTTL(30m) only clears per-session transport state. Nothing builds up under create/terminate churn.

The logging half of that finding still stands in part, and #831 doesn't fully fix it:

  • Dropping Infof (the current head) removes the sweeper's info lines.
  • Two mcp-go error lines can still include the client's Mcp-Session-Id (streamable_http.go:363, which wraps errors from :826 and :1471, and :811). A client without auth can trigger them by POSTing a message shaped like a JSON-RPC reply.

#835, stacked on #831, fixes both remaining points:

  • It sets the stateless manager explicitly, so a change to the mcp-go default can't bring the leak back.
  • It hides error details unless the message is known to contain no session ID.

Sorry for the noise on the retention claim.

@ajitpratap0

Copy link
Copy Markdown
Owner Author

Merge train: this PR is ready otherwise (all checks green on cb0051a, no changes requested), but it now conflicts with main (mergeStateStatus DIRTY). Could the owner please rebase or merge current main into this branch and resolve the conflicts? The train will pick it up again afterwards. (The train does not push to or resolve conflicts on this branch.)

Resolve conflicts with #825 (syntax highlighting) and #829 (dialect
lists): CHANGELOG [Unreleased] Added keeps both entries; pkg/mcp/server.go
unions the imports and derives supportedDialects from
keywords.DialectNames, used for the validate_sql description and enum.
Also correct the validate-sql.sh fallback comment: the greedy sed match
takes the last file_path value, not the first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9
@ajitpratap0

Copy link
Copy Markdown
Owner Author

Merged origin/main into this branch (merge commit d786d0f, normal fast-forward push, no force-push, no rebase). Conflicts resolved:

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 23s —— View job


Review of PR #823 (head d786d0f)

I spot-checked the current head. I did not run the build or tests.

Resolved since earlier reviews

Still open, none blocking

  1. CHANGELOG: the PR still adds entries under [Unreleased], which the release session owns. Either move them into the PR body or accept the possible conflict.
  2. Rate limiting:
    • Add doc comments on Server.Start() and Handler() saying they don't apply RateLimitMiddleware. Only newMux does.
    • Open a follow-up issue about per-IP keying: the client-controlled first X-Forwarded-For entry, and hosted assistant traffic sharing a few egress IPs.
    • Mention the shared limit in docs/CHATGPT_APP.md.
  3. Body read deadline: POST /mcp still has no body read deadline. IdleTimeout is set. This can be a follow-up.
  4. Plugin README: say near the install steps that SQL goes to mcp.gosqlx.dev by default. Document GOSQLX_MCP_URL and GOSQLX_HOOK_DISABLE=1.
  5. Contact mailbox: confirm privacy@gosqlx.dev exists, and share one CONTACT_EMAIL constant between the privacy and terms pages.

Before directory submission: get legal review of /terms and /privacy, set the challenge token in Render, and test the widget in ChatGPT developer mode and Claude after deploying.

Verdict: no blocking code issues. Once GitHub shows the PR as mergeable, it is ready apart from the follow-ups above.
· branch feat/ai-assistant-plugins

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @ajitpratap0's task in 22s —— View job


Review of PR #823 (head 415c590)

I spot-checked pkg/mcp/server.go and plugins/gosqlx/hooks/validate-sql.sh, and grepped the plugin README. I did not run builds or tests.

Resolved since earlier reviews

  • Start() now documents that it applies bearer auth only. It does not apply rate limiting or the body size cap. Its doc comment tells embedders to wrap Handler.
  • The transport logger drops Infof, so session IDs are not logged. SessionIdleTTL is documented as stateless. IdleTimeout: 120s is set.
  • The hook fallback comment now says it takes the last file_path, which matches the greedy sed.
  • The plugin README warns that SQL goes to mcp.gosqlx.dev by default. It also documents GOSQLX_MCP_URL (lines 81-97) and GOSQLX_HOOK_DISABLE=1 (line 69).

Still open, none blocking

  1. Changelog: the PR still edits [Unreleased], which the release session owns. Either move the text into the PR body or accept the possible conflict.
  2. Handler() doc comment: it suggests wrapping with rate limiting, but it doesn't say that the body size cap is also missing. Add that to match Start().
  3. Rate limiting: open a follow-up issue about per-IP keying. The first X-Forwarded-For entry is client-controlled, and ChatGPT and claude.ai traffic will share a few egress IPs. Mention the shared limit in docs/CHATGPT_APP.md.
  4. Body read deadline: POST /mcp still has no body read deadline. This can be a follow-up.
  5. Contact mailbox: confirm that privacy@gosqlx.dev exists, and share one CONTACT_EMAIL constant between the privacy and terms pages.

Before directory submission: get legal review of /terms and /privacy, set the challenge token in Render, and test the widget in ChatGPT developer mode and Claude after deploying.

Verdict: no blocking code issues. Merge once GitHub shows the PR as mergeable.
· branch feat/ai-assistant-plugins

@ajitpratap0
ajitpratap0 merged commit 0e565a8 into main Oct 21, 2026
34 checks passed
ajitpratap0 added a commit that referenced this pull request Oct 21, 2026
…h, tool errors) (#831)

* feat(mcp): ChatGPT app support via OpenAI Apps SDK and MCP Apps

Embed an analyze_sql report widget (ui://gosqlx/analysis-v1.html), return
structuredContent from all tools with unchanged text output, add server
instructions, noauth securitySchemes and invocation status metadata, accept
mariadb/clickhouse in validate_sql, report gosqlx.Version and the real tool
count, serve the optional OpenAI apps domain challenge, and sweep idle MCP
sessions after 30 minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* feat(plugin): add GoSQLX Claude Code plugin and marketplace

Install with /plugin marketplace add ajitpratap0/GoSQLX and
/plugin install gosqlx@gosqlx: hosted MCP server, five SQL skills, four
slash commands, a read-only sql-reviewer agent and an opt-out .sql
validation hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* docs: add terms of service, update privacy policy and integration docs

Add gosqlx.dev/terms, describe hosted MCP server, ChatGPT app and Claude
plugin data handling accurately in the privacy policy, document both
integrations in README, MCP guide and CHANGELOG, and fix the Claude Desktop
stdio example.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* docs(legal): harden terms and privacy policy for app directory review

Name the maintainer as controller and contracting party, add a private
contact and grievance officer, legal bases, recipients (incl. Vercel),
international transfers, retention, security, user rights, children,
DPDP Act 2023 and GDPR references; terms gain eligibility, liability cap,
consumer carve-out, indemnity, termination, Indian governing law,
boilerplate and conspicuous disclaimers. Correct the VS Code telemetry
opt-in text and replace its dead PRIVACY.md link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(website): keep privacy meta description within SEO length limit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(mcp): address PR #823 test findings in widget, sessions and format_sql

- widget: cap long lists (counted statement types, +N more), show failed
  tool calls as errors on both host bridges, skip clipboard API when the
  frame's permissions policy blocks it
- server: one streamable transport per Server reused by Handler/Start,
  new Server.Shutdown, Start gains read-header/idle timeouts, transport
  logger drops Info so session IDs are never logged
- format_sql: uppercase_keywords=true also uppercases operator/function
  keywords; option semantics documented precisely
- docs/privacy: session state is swept 30-45 minutes after last request
  and IDs are not tracked
- tests: full HTTP chain via newMux, session TTL wiring, sweeper leak,
  widget script structure, constants and metadata keys

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(mcp): harden HTTP server rate limiting, timeouts, auth and tool errors

- rate limit keys on RemoteAddr; X-Forwarded-For no longer trusted by
  default; opt-in GOSQLX_MCP_TRUSTED_PROXY_HEADER / _HOPS with IP
  validation (render.yaml: CF-Connecting-IP)
- bounded limiter memory with per-shard LRU eviction
- GET/SSE streams excluded from load metric, capped at 4 per IP
- POST body read deadline (408), idle timeout, 32KB header cap, read
  deadline on non-/mcp routes so stalled bodies cannot hold connections
- oversized bodies get 413; denials are HTTP 429 with Retry-After and the
  request id (initialize keeps 200 for mcp-go clients); batches charged
  per tool call
- bearer auth: constant-time digest compare, case-insensitive scheme
- tool input problems return isError results naming the argument;
  indent_size limited to 0-16

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

---------

Co-authored-by: Ajit Pratap Singh <ajitpratapsingh@Ajits-Mac-mini-2655.local>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ajitpratap0 added a commit that referenced this pull request Oct 21, 2026
…ransport errors (#835)

* feat(mcp): ChatGPT app support via OpenAI Apps SDK and MCP Apps

Embed an analyze_sql report widget (ui://gosqlx/analysis-v1.html), return
structuredContent from all tools with unchanged text output, add server
instructions, noauth securitySchemes and invocation status metadata, accept
mariadb/clickhouse in validate_sql, report gosqlx.Version and the real tool
count, serve the optional OpenAI apps domain challenge, and sweep idle MCP
sessions after 30 minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* feat(plugin): add GoSQLX Claude Code plugin and marketplace

Install with /plugin marketplace add ajitpratap0/GoSQLX and
/plugin install gosqlx@gosqlx: hosted MCP server, five SQL skills, four
slash commands, a read-only sql-reviewer agent and an opt-out .sql
validation hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* docs: add terms of service, update privacy policy and integration docs

Add gosqlx.dev/terms, describe hosted MCP server, ChatGPT app and Claude
plugin data handling accurately in the privacy policy, document both
integrations in README, MCP guide and CHANGELOG, and fix the Claude Desktop
stdio example.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* docs(legal): harden terms and privacy policy for app directory review

Name the maintainer as controller and contracting party, add a private
contact and grievance officer, legal bases, recipients (incl. Vercel),
international transfers, retention, security, user rights, children,
DPDP Act 2023 and GDPR references; terms gain eligibility, liability cap,
consumer carve-out, indemnity, termination, Indian governing law,
boilerplate and conspicuous disclaimers. Correct the VS Code telemetry
opt-in text and replace its dead PRIVACY.md link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(website): keep privacy meta description within SEO length limit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(mcp): address PR #823 test findings in widget, sessions and format_sql

- widget: cap long lists (counted statement types, +N more), show failed
  tool calls as errors on both host bridges, skip clipboard API when the
  frame's permissions policy blocks it
- server: one streamable transport per Server reused by Handler/Start,
  new Server.Shutdown, Start gains read-header/idle timeouts, transport
  logger drops Info so session IDs are never logged
- format_sql: uppercase_keywords=true also uppercases operator/function
  keywords; option semantics documented precisely
- docs/privacy: session state is swept 30-45 minutes after last request
  and IDs are not tracked
- tests: full HTTP chain via newMux, session TTL wiring, sweeper leak,
  widget script structure, constants and metadata keys

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(mcp): harden HTTP server rate limiting, timeouts, auth and tool errors

- rate limit keys on RemoteAddr; X-Forwarded-For no longer trusted by
  default; opt-in GOSQLX_MCP_TRUSTED_PROXY_HEADER / _HOPS with IP
  validation (render.yaml: CF-Connecting-IP)
- bounded limiter memory with per-shard LRU eviction
- GET/SSE streams excluded from load metric, capped at 4 per IP
- POST body read deadline (408), idle timeout, 32KB header cap, read
  deadline on non-/mcp routes so stalled bodies cannot hold connections
- oversized bodies get 413; denials are HTTP 429 with Retry-After and the
  request id (initialize keeps 200 for mcp-go clients); batches charged
  per tool call
- bearer auth: constant-time digest compare, case-insensitive scheme
- tool input problems return isError results naming the argument;
  indent_size limited to 0-16

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L2wnDjnjXs2u8baaf2PC6w

* fix(mcp): pin stateless session IDs and stop logging session IDs in transport errors

Pin the streamable HTTP transport to mcp-go's
StatelessGeneratingSessionIdManager instead of relying on the library
default. mcp-go's InsecureStatefulSessionIdManager stores every terminated
session ID in a sync.Map that is never pruned (v0.45.0
server/streamable_http.go:1489,1523); pinning the manager keeps an upstream
default change from turning the public no-auth server into an unbounded
store of session IDs.

Redact transport error arguments unless the format is known to be safe.
mcp-go logs errors that embed the client-supplied Mcp-Session-Id header
(streamable_http.go:363 via :826 and :1471), which any client can trigger
by POSTing a response-shaped JSON-RPC message, contradicting the privacy
policy's promise that session IDs are never logged. Generated IDs are also
scrubbed from every error line as a second line of defence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

---------

Co-authored-by: Ajit Pratap Singh <ajitpratapsingh@Ajits-Mac-mini-2655.local>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview — 415c590f Deployed Oct 21, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant