Skip to content

feat(mcp-catalog): add GitHub, Notion, and Sentry remote-OAuth connectors - #66653

Open
joelbrilliant wants to merge 2 commits into
NousResearch:mainfrom
joelbrilliant:feat/mcp-catalog-github-notion
Open

feat(mcp-catalog): add GitHub, Notion, and Sentry remote-OAuth connectors#66653
joelbrilliant wants to merge 2 commits into
NousResearch:mainfrom
joelbrilliant:feat/mcp-catalog-github-notion

Conversation

@joelbrilliant

Copy link
Copy Markdown
Contributor

Adds three first-party remote-OAuth MCP catalog manifests so users get one-click curated connectors instead of hand-configuring servers:

connector remote endpoint source
github https://api.githubcopilot.com/mcp/ github/github-mcp-server README
notion https://mcp.notion.com/mcp developers.notion.com/docs/mcp
sentry https://mcp.sentry.dev/mcp docs.sentry.io/product/sentry-mcp

Template. Each manifest mirrors the existing linear entry field-for-field (optional-mcps/linear/manifest.yaml, transport at lines 13–15, auth at lines 17–21): transport.type: http + auth.type: oauth with no provider — i.e. native MCP OAuth 2.1 (case 1), so Hermes's MCP client + mcp_oauth_manager handle discovery/PKCE/token-exchange/refresh straight from the server URL. No new manifest fields are introduced (schema authority: hermes_cli/mcp_catalog.py _parse_manifest, lines 158–278 — it reads only name, description, source, transport, auth, tools, install, post_install, so an icon/categories field would be silently ignored; none added).

Endpoints verified live (2026-07-18). Each MCP endpoint returns 401 to an unauthenticated initialize (a real OAuth-gated MCP server, not a dead host), and each serves RFC 9728 OAuth Protected Resource Metadata: Notion at /.well-known/oauth-protected-resource (200), GitHub and Sentry at the path-scoped /.well-known/oauth-protected-resource/mcp (200). So the browser OAuth flow will discover and complete on first connect.

Tests. The generic contract test tests/hermes_cli/test_mcp_catalog.py::TestShippedCatalog::test_all_shipped_manifests_parse (line 818) auto-covers every shipped manifest's shape, so these entries are validated the moment they land. This PR also adds test_first_party_remote_oauth_connectors_present pinning that the three connectors are present, http, and native-oauth (provider is None). Full file: 40 passed.

No duplicates. optional-mcps/ previously held blender, linear, n8n, unreal-engine; none of github/notion/sentry existed, and no test previously referenced them.

Mergeability — honest-low. Other first-party remote entries (e.g. Vercel / Hugging Face / AWS Knowledge) are currently unmerged, so a slow review/merge on curated remote connectors is expected. This PR doesn't change that; it adds three well-documented, contract-tested, liveness-verified manifests that are trivial to accept or defer. The full end-to-end OAuth handshake only happens on a real first-connect and is the one thing not exercised here.

…tors

Add three first-party remote-OAuth MCP catalog manifests so users get
one-click curated connectors instead of hand-configuring servers:

- github → https://api.githubcopilot.com/mcp/
- notion → https://mcp.notion.com/mcp
- sentry → https://mcp.sentry.dev/mcp

Each mirrors the existing `linear` manifest (optional-mcps/linear/
manifest.yaml): transport http + auth oauth, no `provider` field, so
the MCP client handles native OAuth 2.1 discovery/PKCE/refresh from the
server URL. No new fields introduced.

The generic contract test (tests/hermes_cli/test_mcp_catalog.py::
TestShippedCatalog::test_all_shipped_manifests_parse) already auto-covers
the new manifests' shape; add an explicit assertion that the three
connectors are present, http, and native-oauth.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@tonydwb tonydwb 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.

{
"event": "APPROVE",

Code Review Summary

Verdict: Approved

MCP catalog entries for GitHub, Notion, and Sentry — all new YAML manifest files under optional-mcps/. Clean additive changes, well-documented with clear descriptions of OAuth flows and transport configuration. No source code modifications.

Looks Good

  • Correct manifest_version: 1 format
  • Proper OAuth 2.1 / PKCE configuration for each provider
  • Clear post_install instructions for user-facing setup
  • Reasonable default_enabled omission (user prunes at install time)

Reviewed by Hermes Agent",
"comments": []
}

@joelbrilliant

Copy link
Copy Markdown
Contributor Author

Note for the reviewer on the required-check failure: it's the mcp-catalog-reviewed label gate (Supply-chain scan requires a maintainer to apply the label after reviewing catalog changes — working as designed for manifest PRs). The review above is already an approval; happy to address anything further before the label. The separate "CI timing report" failure is unrelated to this PR — it fails on every fork PR due to the token-secret restriction; fix proposed in #66665.

@teknium1 teknium1 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.

Thanks for adding these catalog entries. The native OAuth shape matches the existing catalog path: hermes_cli/mcp_catalog.py:489-493 writes auth: oauth, and tools/mcp_tool.py:2574-2582 routes that configuration through the MCP OAuth manager.

Problems

  • The new manifests are absent from [tool.setuptools.data-files]. pyproject.toml:323-334 requires one target per optional-mcps/<name> directory to preserve wheel layout, but only linear and n8n are declared at pyproject.toml:335-336. Add targets for github, notion, and sentry; otherwise wheel installs will not catalog these entries.
  • test_first_party_remote_oauth_connectors_present hard-codes catalog names and URLs. That conflicts with tests/hermes_cli/test_mcp_catalog.py:821-836, which deliberately avoids catalog-name snapshots, and the change-detector-test policy in AGENTS.md:1331-1378. The generic parse/version-lock contracts should remain the coverage here.

Suggested changes

  • Add the three wheel data-file targets in pyproject.toml.
  • Remove the hard-coded catalog-entry test.

Automated hermes-sweeper review.

Comment thread optional-mcps/github/manifest.yaml
Comment thread tests/hermes_cli/test_mcp_catalog.py Outdated
…test

Address hermes-sweeper review on NousResearch#66653:
- Add github/notion/sentry to [tool.setuptools.data-files] so wheel
  installs catalog the new manifests (one target per optional-mcps/<name>,
  matching the existing linear/n8n entries).
- Remove test_first_party_remote_oauth_connectors_present, which snapshotted
  catalog names/URLs and conflicts with the repo change-detector-test policy.
  The generic parse + version-lock contracts already cover these manifests.
@joelbrilliant
joelbrilliant requested a review from a team July 18, 2026 23:12
@joelbrilliant

Copy link
Copy Markdown
Contributor Author

Thanks for the review, both points addressed in the pushed commit:

  • Wheel packaging: added github, notion, and sentry targets to [tool.setuptools.data-files], one per optional-mcps/<name> directory alongside the existing linear/n8n entries, so wheel installs catalog these manifests.
  • Change-detector test: removed test_first_party_remote_oauth_connectors_present. The generic test_all_shipped_manifests_parse and test_all_shipped_manifests_are_version_locked contracts already cover the new manifests without snapshotting names or URLs.

Verified: tests/hermes_cli/test_mcp_catalog.py and tests/test_packaging_metadata.py both green (50 passed).

Minor, out of scope for this PR so flagging rather than touching it here: blender and unreal-engine already ship in optional-mcps/ but are also absent from data-files, and the pyproject comment says tests/test_packaging_metadata.py enforces an entry per optional-mcps/<name> though that test currently only checks locales. Happy to send a separate PR closing that gap and adding the missing per-directory assertion if useful.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 18, 2026
@joelbrilliant

Copy link
Copy Markdown
Contributor Author

The remaining red check is the repository's manual MCP catalog security gate. All code, test, Docker, critical supply chain, and OSV checks passed. A maintainer now needs to review the bundled connector commands and environment requirements, add the mcp-catalog-reviewed label, and rerun CI. I have not applied that label myself because it is the explicit security approval gate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/mcp MCP client and OAuth type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants