Skip to content

Serve Supabase agent skills over MCP (SEP-2640, preview) - #437

Draft
claude[bot] wants to merge 2 commits into
mainfrom
claude/skills-over-mcp
Draft

claude[bot] wants to merge 2 commits into
mainfrom
claude/skills-over-mcp

Conversation

@claude

@claude claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Requested via Slack thread

What kind of change does this PR introduce?

Feature (draft, built on an unreleased typescript-sdk preview).

What is the current behavior?

mcp-server-supabase has no way to expose Supabase's agent skills (supabase.com/.well-known/agent-skills) to an MCP client. The skills content only reaches agents that separately install a plugin/skill package (e.g. supabase-community/supabase-plugin); there's no MCP-native discovery path. See Linear AI-1230.

What is the new behavior?

Adds an opt-in skills option to createSupabaseMcpServer. When set, the server:

  • Fetches the discovery index at supabase.com/.well-known/agent-skills.
  • Downloads each skill's .tar.gz, verifies it against the index's published sha256 digest, and unpacks it in memory (no filesystem writes).
  • Computes a per-file sha256 digest and byte size for every unpacked file, building the "complete" resource list SEP-2640 requires (never "dynamic", since Supabase's skill releases are versioned and stable per release).
  • Serves skills/list and skills/get from that manifest, and the individual files (SKILL.md, references/*.md, etc.) over resources/read.
  • Caches the manifest in memory with a TTL (default 5 minutes) so a busy server doesn't re-download and re-unpack on every call.

Off by default — the skills option is undefined unless a caller opts in — since this currently depends on a preview build of the typescript-sdk Skills extension (see below), not a stable release. cli.ts/http.ts/local-http-entry.ts are unchanged; wiring this on for a given deployment is a one-line follow-up once typescript-sdk publishes the real release.

Existing tools (search_docs, etc.) are unchanged.

Follow-ons, explicitly out of scope here:

  • Moving search_docs's embedded GraphQL schema into a served skill resource instead of always being in the tool description (saves context on every tools/list).
  • Parsing full SKILL.md YAML frontmatter for fields beyond name/description (the discovery index already carries those two, sourced from the same frontmatter, so this PR uses them directly rather than re-parsing YAML — see the comment in manifest.ts).
  • Swapping the pkg.pr.new preview pin for a real catalog: range once typescript-sdk#2818 ships stable.

How to Review

  1. Wiring / entrypoint

    • packages/mcp-server-supabase/src/server.ts — the new skills option on SupabaseMcpServerOptions, and the two call sites: resources: skillsProvider && getSkillsResources(skillsProvider) passed into createMcpServer, and installSupabaseSkills(server, skillsProvider) called after it.
  2. The SDK integration point (the part most worth scrutinizing)

    • packages/mcp-server-supabase/src/skills/mcp.tsinstallSupabaseSkills. installSkills() (typescript-sdk#2818, phase 1) only accepts a static skill list captured at install time and can't be re-registered after the server connects. To still answer every request from the live TTL-cached manifest, this calls installSkills() once (for capability declaration + wire-schema validation) with an empty snapshot, then immediately replaces both handlers with versions that read through the live SkillsProvider. This relies on Server#setRequestHandler having no re-registration guard — see the docblock for the full reasoning. getSkillsResources is the mcp-utils-side counterpart: a dynamic resources list resolved fresh on every resources/list/resources/read.
  3. Fetch / unpack / digest

    • packages/mcp-server-supabase/src/skills/manifest.tsfetchSkillsManifest: fetches the index, downloads+verifies each tarball against its published digest (same check as supabase-plugin's sync-agent-skills GitHub Action, ported to run at request time instead of in CI), gunzips+untars via tar-stream (no disk writes), then computes per-file sha256/size.
    • packages/mcp-server-supabase/src/skills/provider.tscreateSkillsProvider: the TTL cache, with in-flight de-duplication so concurrent callers during a refresh share one fetch.
  4. Tests

    • packages/mcp-server-supabase/src/skills/manifest.test.ts — digest verification, missing-SKILL.md rejection, malformed index rejection, correct per-file digest/size/URI construction.
    • packages/mcp-server-supabase/src/skills/provider.test.ts — TTL expiry/no-expiry, invalidate(), concurrent-call de-duplication, retry-after-failure (no caching a rejection).
    • packages/mcp-server-supabase/src/skills/server.test.ts — end-to-end over a real Client/Server pair (StreamTransport, same pattern as server.test.ts): capability negotiation, skills/list, skills/get (including the -32602 unknown-URI case), and resources/read serving bytes that match the published digest.

Review questions

  • Is calling installSkills() once and then overriding its two handlers (rather than not using installSkills() at all) the right call, given phase 1's static-list API?
  • Should skills default to enabled somewhere (e.g. the hosted HTTP entry) in a follow-up PR, or stay purely opt-in until typescript-sdk#2818 ships stable?
  • Is pinning @modelcontextprotocol/core/client/server to an exact pkg.pr.new commit (via a root pnpm.overrides, to keep one canonical instance workspace-wide and avoid duplicate-package type mismatches) an acceptable temporary state for a draft PR?
  • Any concern with unpacking untrusted-shaped tarball bytes fully in memory (bounded by the SEP's 16 MiB per-skill limit, enforced in buildSkill)?

Verification

  • pnpm --filter @supabase/mcp-server-supabase typecheck — clean.
  • pnpm --filter @supabase/mcp-server-postgrest typecheck — clean (unaffected by the workspace-wide pnpm.overrides).
  • CI=true npx vitest run --project unit in packages/mcp-server-supabase423 passed (408 pre-existing + 15 new), 0 failed, 0 skipped.
  • pnpm --filter @supabase/mcp-utils test — 13 passed (unaffected).
  • pnpm --filter @supabase/mcp-server-supabase build — tsup build succeeds; confirmed the skills module is bundled into the shared chunk both dist/cli.js and dist/cli.cjs import, and SkillsProviderOptions appears in the published dist/index.d.ts.
  • pnpm biome check --write — applied, clean.

Additional context

  • Built on typescript-sdk#2818 ("feat(skills): add SEP-2640 Skills extension APIs (phase 1: schemas, client ops, server handlers)"), pinned to commit b0091060d73d08211c6766990aa15656d4e03271 via pkg.pr.new (verified reachable for @modelcontextprotocol/core, /client, and /server). That PR is itself a draft; this PR will need a follow-up once it (or its stable release) ships, to swap the pkg.pr.new pins for a real catalog: semver range.
  • Ported the tarball fetch/verify approach from supabase-community/supabase-plugin's .github/workflows/sync-agent-skills.yml (gh release downloadcurl the tarball → compare sha256 → tar -xzf), reimplemented in Node so it runs at request time in the MCP server process instead of in a GitHub Action at release time.
  • Linear: AI-1230.

🤖 Generated with Claude Code

https://claude.ai/code/session_01REAnGWxMnmwfehoF4c4VKS


Generated by Claude Code

…iew)

Adds a `skills` option to `createSupabaseMcpServer` that fetches Supabase's
published agent-skills index (supabase.com/.well-known/agent-skills),
downloads and digest-verifies each skill's tarball, unpacks it in memory,
and serves it over the MCP Skills extension (SEP-2640): `skills/list`,
`skills/get`, and the individual files via `resources/read`. A TTL cache
(`createSkillsProvider`) avoids re-fetching/re-unpacking on every call.

Built on the typescript-sdk Skills extension preview
(modelcontextprotocol/typescript-sdk#2818, phase 1: schemas + `installSkills`).
That phase only takes a static skill list at install time, so
`installSupabaseSkills` calls it once for capability declaration and wire
validation, then overrides both handlers to resolve live from the TTL
cache on every call — the point of this feature, so a new skill release
needs no mcp-server-supabase release.

Off by default (undefined `skills` option) since it depends on an
unreleased SDK preview build, pinned exactly via pkg.pr.new rather than a
floating tag.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REAnGWxMnmwfehoF4c4VKS
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

The failing check job (Check Management API Types, run 35380410347) is unrelated to this PR.

That job regenerates packages/mcp-server-supabase/src/management-api/types.ts / management-api-v2/types.ts from the live https://api.supabase.com/api/v1-json OpenAPI spec and fails if the checked-in file doesn't match. This PR doesn't touch either file (git diff --stat against main shows no changes to them).

Verified independently: running pnpm --filter @supabase/mcp-server-supabase generate:management-api-types on a fresh, unmodified clone of main (no relation to this branch) produces the same one-line diff — schema_version: stringschema_version?: string in management-api/types.ts — meaning the live Management API spec has drifted since the committed types were last regenerated. This is a pre-existing staleness issue on main, not something introduced here.

Not fixing it in this PR since it's an unrelated, orthogonal change (regenerated API types, no connection to the skills feature). Will retrigger the check now that this is confirmed as base-branch drift; a maintainer may want a separate PR running pnpm generate:management-api-types to catch main back up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01REAnGWxMnmwfehoF4c4VKS

@coveralls

coveralls commented Sep 18, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 35396694366

Coverage decreased (-0.3%) to 96.761%

Details

  • Coverage decreased (-0.3%) from the base build.
  • Patch coverage: 17 uncovered changes across 3 files (289 of 306 lines covered, 94.44%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
packages/mcp-server-supabase/src/skills/manifest.ts 161 151 93.79%
packages/mcp-server-supabase/src/skills/mcp.ts 67 62 92.54%
packages/mcp-server-supabase/src/skills/test-helpers.ts 30 28 93.33%
Total (7 files) 306 289 94.44%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 4480
Covered Lines: 4361
Line Coverage: 97.34%
Relevant Branches: 769
Covered Branches: 718
Branch Coverage: 93.37%
Branches in Coverage %: Yes
Coverage Strength: 84.96 hits per line

💛 - Coveralls

@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

The Check Management API Types job (check_run 105714979019) failed, but it's unrelated to this PR's changes.

This PR does not touch packages/mcp-server-supabase/src/management-api/types.ts or .../management-api-v2/types.ts at all. The job regenerates those files by fetching Supabase's live Management API OpenAPI spec and diffing against the checked-in generated types. The live spec has since changed schema_version on one schema from required to optional:

-                schema_version: string;
+                schema_version?: string;

I reproduced this locally on both this branch and main (pnpm --filter @supabase/mcp-server-supabase generate:management-api-types) — both produce the identical diff and both fail the check the same way. So this is pre-existing drift between the live API spec and the repo's checked-in generated types, not something introduced here. Leaving it as-is rather than bundling an unrelated regeneration into this PR; re-running the check below in case it was transient, but expect it to fail again until the types are regenerated separately (e.g. via the sync-management-api-types workflow).

@Rodriguespn Rodriguespn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Honestly I was expecting that the mcp ask did more heavy lifting that this preview version seems to do. Hopefully the one that ends up published can help us reduce the amount of boiler plate code in this PR.

Comment thread package.json
},
"pnpm": {
"overrides": {
"@modelcontextprotocol/core": "https://pkg.pr.new/@modelcontextprotocol/core@b0091060d73d08211c6766990aa15656d4e03271",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Replace for the published version once this version of the mcp sdk lands

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed, that's already the plan — no code change here for now. Once modelcontextprotocol/typescript-sdk#2818 ships a real semver release (not just the pkg.pr.new preview), the follow-up swaps these three pnpm.overrides entries for a normal catalog: range. That's called out explicitly in the PR description's "Additional context" and "Follow-ons" sections so it doesn't get lost.

I looked for a way to add an inline comment directly in package.json noting the pin is temporary, but JSON has no comment syntax and this repo doesn't use JSON5/JSONC anywhere (package.json is also excluded from biome's formatting/linting). Since the temporary nature is already documented in the PR description rather than left implicit, I didn't add a workaround like a stray _comment key inside pnpm.overrides. Happy to add a note elsewhere (e.g. CONTRIBUTING.md) if you'd find that clearer than the PR description.


Generated by Claude Code

import { z } from 'zod/v4';
import { AGENT_SKILLS_INDEX_URL } from './constants.js';

const SHA256_DIGEST_PATTERN = /^sha256:[0-9a-f]{64}$/;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can use the crypto package to deal with the digest instead of using regex

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked this against what crypto actually offers, and I don't think it's a good swap here — SHA256_DIGEST_PATTERN (and the zod .regex() built on it) is validating the string format of an untrusted, externally-supplied field (sha256: + 64 lowercase hex chars) before we do anything else with it. crypto doesn't have an API for "is this string shaped like sha256:<hex>" — Buffer.from(x, 'hex') silently drops invalid characters instead of rejecting them, so it wouldn't actually catch a malformed digest string the way the regex does, and crypto.timingSafeEqual is for constant-time comparison of two known buffers (relevant for secret/credential comparisons), not format validation — and these digests aren't secrets anyway.

The crypto module is already doing the actual digest work elsewhere in this file (sha256() at line 98 uses createHash('sha256'), and downloadTarball() compares the computed digest against the published one). That's the digest computation concern; the regex is a separate, narrower format-validation concern at the zod-schema boundary. So I kept the regex as-is rather than replacing it — let me know if you had a specific crypto API in mind that I'm missing.


Generated by Claude Code

type Server,
} from '@modelcontextprotocol/server';
import { installSkills } from '@modelcontextprotocol/server/ext/skills';
import { resource, type Resource } from '@supabase/mcp-utils';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe the skill logic should live inside mcp-utils, just like resources does

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a fair question, and I don't think it's a small nit, so I'm not pushing a refactor into this PR — flagging the split for you to decide instead.

What's genuinely Supabase-specific in skills/:

  • The .well-known/agent-skills discovery-index fetch and its URL (constants.ts's AGENT_SKILLS_INDEX_URL) — that's Supabase's own published index, not a generic concept.
  • The fact that the content served is Supabase's own skills (docs/support workflows), as opposed to something a generic consumer would bring.

What's generic/reusable, and has nothing Supabase-specific in it:

  • manifest.ts: tarball download + digest verification + gunzip/untar + per-file digest/size computation — this is "given an index of {name, url, digest} entries, fetch/verify/unpack them into a skills/list-shaped manifest," which has no Supabase-specific logic once the index URL is a parameter (it already is, via FetchSkillsManifestOptions.indexUrl).
  • provider.ts: the TTL cache + in-flight de-duplication wrapper — generic caching, no Supabase dependency at all.
  • mcp.ts: the installSkills()-once-then-override-handlers SDK integration, and getSkillsResources()'s dynamic mcp-utils resource list — this is exactly the kind of SDK-wiring resource()/resourceTemplate() already lives in mcp-utils for.

Recommendation: the generic caching/manifest/SDK-wiring pieces (provider.ts, most of manifest.ts, mcp.ts) could move to @supabase/mcp-utils as a reusable installSkills-style helper parameterized by an index URL and fetch function, keeping only the Supabase-specific discovery-index constant and the "these are Supabase's skills" wiring in mcp-server-supabase. That's a genuine cross-package move though (new public API surface in mcp-utils, changed imports in mcp-server-supabase), so I'd rather do it as a focused follow-up PR than fold it into this one — happy to pick it up once this lands, if that's the direction you want.


Generated by Claude Code

}

if (totalBytes > MAX_SKILL_TOTAL_BYTES) {
throw new Error(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this error or just ignore this skill? Also, the spec doesn't forbid large skills.Host may deal or not with large skills

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — you're right, and I checked the actual spec text to confirm. SEP-2640 says:

Servers SHOULD NOT exceed 512 files or 16 MiB per skill, including SKILL.md. Hosts MUST support skills up to these limits and MAY support larger skills.

So this is a SHOULD NOT aimed at server authors, not a hard cap, and hosts are explicitly permitted to go over it. The old code (throw new Error(...exceeding the SEP-2640 limit...)) mischaracterized this as a protocol violation, and as a bonus bug it would have aborted the whole Promise.all batch — one oversized skill would have taken down every other skill in the manifest too.

Pushed a fix in 82e3482:

  • buildSkill() now skips an oversized skill (returns undefined) and logs a warning via console.error, instead of throwing.
  • Added a code comment at the check explaining this is a local practical safeguard against unbounded memory use unpacking a whole tarball at once, not a spec requirement — nothing stops us from serving larger skills if we later want to (e.g. by streaming/paginating instead of skipping).
  • fetchSkillsManifest() filters the skipped entries out of the manifest rather than serving a hole.
  • Added manifest.test.ts coverage: an oversized skill is skipped with a logged warning, and a sibling properly-sized skill in the same index is still served.

Full unit suite (424 tests) and typecheck are green after the change.


Generated by Claude Code

SEP-2640 says servers SHOULD NOT exceed 512 files or 16 MiB per skill,
but explicitly does not forbid larger ones — hosts MUST support skills
up to the limit and MAY support larger ones. The prior code treated
this as a hard spec-mandated error and threw, which would also abort
fetching every other skill in the same Promise.all batch.

buildSkill() now skips (returns undefined for) an oversized skill and
logs a warning via console.error, as a local practical safeguard
against unbounded memory use unpacking a tarball in memory, not a
protocol violation. fetchSkillsManifest() filters skipped skills out
of the manifest rather than serving a hole. Added a test covering the
skip-and-warn behavior and that a sibling properly-sized skill is
still served.

Addresses Pedro's review comment on PR #437.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REAnGWxMnmwfehoF4c4VKS
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Agreed — this matches what we found digging into the typescript-sdk PR. installSkills() (typescript-sdk#2818) is phase 1 of a 4-phase rollout (schemas + static-list install/handlers only), which is why installSupabaseSkills in this PR has to install-then-override its handlers instead of just calling into the SDK. As later phases land (live-provider hooks, filesystem discovery helpers, etc.) the boilerplate in skills/mcp.ts and skills/manifest.ts should shrink — we'd expect to delete code here rather than add to it as the SDK PR matures.


Generated by Claude Code

@claude
claude Bot requested a review from Rodriguespn September 18, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants