-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Adding app categories arg and method to sdk #18037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
Caution Review failedThe pull request is closed. WalkthroughAdds app category types and a getAppCategories() method to the SDK, extends getApps(...) to accept a categoryIds filter (mapped to category_ids), bumps packages/sdk to 1.8.0, and updates the modelcontextprotocol README to recommend Pipedream’s remote MCP server and revise messaging. Changes
Sequence Diagram(s)sequenceDiagram
participant DevApp as Developer App
participant SDK as SDK Client
participant API as Backend API
DevApp->>SDK: getAppCategories()
SDK->>API: GET /connect/app_categories
API-->>SDK: 200 OK (AppCategory[])
SDK-->>DevApp: GetAppCategoriesResponse
sequenceDiagram
participant DevApp as Developer App
participant SDK as SDK Client
participant API as Backend API
DevApp->>SDK: getApps({ categoryIds })
alt categoryIds provided
SDK->>API: GET /connect/apps?category_ids=cat1,cat2
else no categoryIds
SDK->>API: GET /connect/apps
end
API-->>SDK: 200 OK (apps)
SDK-->>DevApp: Apps response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/sdk/src/shared/index.ts (2)
283-287
: Document how to obtain valid category IDs.
Consider adding a brief note pointing consumers to getAppCategories() as the source of valid IDs and a short example of usage for discoverability.
1275-1291
: Verify endpoint scoping; consider using makeConnectRequest for consistency.
The path used is /connect/app_categories via makeAuthorizedRequest. If app categories are intended to be project-scoped like other Connect endpoints, switching to makeConnectRequest("/app_categories") would include the project ID automatically. If they’re truly global, current approach is correct—please confirm.If project-scoped, apply this refactor:
- public getAppCategories() { - return this.makeAuthorizedRequest<GetAppCategoriesResponse>("/connect/app_categories", { - method: "GET", - }); - } + public getAppCategories() { + return this.makeConnectRequest<GetAppCategoriesResponse>("/app_categories", { + method: "GET", + }); + }To verify which endpoint is valid in your environment, please check server/API docs, or run a quick smoke test in your existing integration tests that exercise this method (ensuring no secrets are printed). If you want, I can draft a Jest test that stubs fetch and asserts the called URL matches the chosen convention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
packages/sdk/CHANGELOG.md
(2 hunks)packages/sdk/package.json
(1 hunks)packages/sdk/src/shared/index.ts
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Lint Code Base
🔇 Additional comments (6)
packages/sdk/package.json (1)
4-4
: Version bump to 1.8.0 aligns with CHANGELOG and API additions. LGTM.
Consistent with the new public APIs described in CHANGELOG. No further action from me.packages/sdk/CHANGELOG.md (2)
5-12
: Changelog entry accurately reflects the new public APIs.
Matches the code changes (getAppCategories + getApps categoryIds). Good release notes.
52-53
: Minor formatting/content fix retained.
This non-semantic correction is fine to keep in this PR.packages/sdk/src/shared/index.ts (3)
129-148
: AppCategory type addition looks correct and minimal.
Fields and docs are clear. No issues spotted.
149-153
: GetAppCategoriesResponse type is appropriate.
Array shape matches the intended API response.
1222-1224
: Correct mapping of categoryIds to category_ids.
Comma-joining client-side and passing as category_ids matches the described API. Looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
modelcontextprotocol/README.md (1)
80-81
: Fix admonition formatting: current block won’t render as a GitHub noteGitHub admonitions require the marker and content on separate quoted lines.
- > [!NOTE] > [Check out the docs](https://pipedream.com/docs/connect/mcp/app-discovery) to learn more about enabling app discovery with the MCP server. +> [!NOTE] +> [Check out the docs](https://pipedream.com/docs/connect/mcp/app-discovery) to learn more about enabling app discovery with the MCP server.
🧹 Nitpick comments (3)
modelcontextprotocol/README.md (3)
9-14
: Tighten bullet phrasing; fix punctuation/hyphenation for clarityImproves readability and addresses minor grammar/style nits flagged by static analysis.
-- **2,800+ APIs and 10,000+ tools** through a single server -- **Built-in authentication** - no manual token management required - - **Multiple tool modes**: sub-agent and full configuration +- **2,800+ APIs and 10,000+ tools** via a single server +- **Built-in authentication** (no manual token management required) +- **Multiple tool modes**: sub-agent and full-configuration - **Automatic app discovery** -- **Enterprise-grade reliability** and security +- **Enterprise-grade reliability and security**
17-17
: Hyphenate “open-source” (compound adjective)Minor grammar fix.
-> **🎮 Try it now**: Check out our [open source chat app](https://github.com/PipedreamHQ/mcp) at [chat.pipedream.com](https://chat.pipedream.com/) +> **🎮 Try it now**: Check out our [open-source chat app](https://github.com/PipedreamHQ/mcp) at [chat.pipedream.com](https://chat.pipedream.com/)
34-37
: Number consistency looks good; consider avoiding hard numbers to reduce maintenance“2,800+” appears consistently here and above. If these counts change frequently, consider “thousands of apps and APIs” to avoid future churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modelcontextprotocol/README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
modelcontextprotocol/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ... 10,000+ tools** through a single server - Built-in authentication - no manual to...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...** - no manual token management required - Multiple tool modes: sub-agent and ful...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...odes**: sub-agent and full configuration - Automatic app discovery - **Enterprise...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...onfiguration - Automatic app discovery - Enterprise-grade reliability and secur...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... started**: Pipedream MCP Documentation > 🎮 Try it now: Check out our [open sour...
(QB_NEW_EN)
[uncategorized] ~17-~17: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s) > 🎮 Try it now: Check out our [open source chat app](https://github.com/PipedreamH...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
modelcontextprotocol/README.md (1)
3-3
: Repositioning note reads wellThe strong guidance to use the hosted server for production is clear and helpful. No issues here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
modelcontextprotocol/README.md (1)
80-80
: Fix admonition formatting (GitHub Markdown).The note block should be on two lines; the current single-line format won’t render as an admonition.
- > [!NOTE] > [Check out the docs](https://pipedream.com/docs/connect/mcp/app-discovery) to learn more about enabling app discovery with the MCP server. +> [!NOTE] +> [Check out the docs](https://pipedream.com/docs/connect/mcp/app-discovery) to learn more about enabling app discovery with the MCP server.
🧹 Nitpick comments (1)
modelcontextprotocol/README.md (1)
17-17
: Hyphenate compound adjective “open‑source.”Minor copy edit for consistency and correctness.
-> **🎮 Try it now**: Check out our [open source chat app](https://github.com/PipedreamHQ/mcp) at [chat.pipedream.com](https://chat.pipedream.com/) +> **🎮 Try it now**: Check out our [open-source chat app](https://github.com/PipedreamHQ/mcp) at [chat.pipedream.com](https://chat.pipedream.com/)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modelcontextprotocol/README.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
modelcontextprotocol/README.md
[grammar] ~9-~9: There might be a mistake here.
Context: ... 10,000+ tools** through a single server - Built-in authentication: no manual tok...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...n**: no manual token management required - Multiple tool modes: sub-agent and ful...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...odes**: sub-agent and full configuration - Automatic app discovery - **Enterprise...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...onfiguration - Automatic app discovery - **Enterprise-grade reliability and securit...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... started**: Pipedream MCP Documentation > 🎮 Try it now: Check out our [open sour...
(QB_NEW_EN)
[uncategorized] ~17-~17: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...s) > 🎮 Try it now: Check out our [open source chat app](https://github.com/PipedreamH...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
modelcontextprotocol/README.md (1)
3-3
: Good call-out: clearly positions this as a reference implementation.The production recommendation is explicit and helpful. No changes needed here.
- **2,800+ APIs and 10,000+ tools** through a single server | ||
- **Built-in authentication**: no manual token management required | ||
- **Multiple tool modes**: sub-agent and full configuration | ||
- **Automatic app discovery** | ||
- **Enterprise-grade reliability and security** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify and harmonize counts (“2,800+ APIs/Apps” and “10,000+ tools”).
Two different phrasings are used:
- “2,800+ APIs and 10,000+ tools” (Line 9)
- “over 2,800 apps and APIs” (Line 36)
If both are accurate, consider harmonizing phrasing to avoid confusion. Also ensure these numbers are consistent across the repo and docs.
Run this to surface any inconsistent counts or missing schemes in links:
Also applies to: 36-36
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for count references..."
rg -n -I -S "2,700|2,800|10,000\+ tools|10,000\+|10,000" -A 1 -B 1
echo
echo "Checking links to mcp.pipedream.com without https scheme..."
rg -n -I -S '\]\((?:www\.)?mcp\.pipedream\.com\)' -A 0 -B 0 || true
Length of output: 8985
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for '2,800'..."
rg -n --hidden -S -H '2,800' || true
echo
echo "Searching for '2,700' or 'PUBLIC_APPS'..."
rg -n --hidden -S -H 'PUBLIC_APPS|2,700' || true
echo
echo "Searching for '10,000' (tools / +) ..."
rg -n --hidden -S -H '10,000\+|10,000\+ tools|10,000' || true
echo
echo "Searching for mcp.pipedream.com (any scheme / bare)..."
rg -n --hidden -S -H 'mcp\.pipedream\.com' || true
Length of output: 268186
Action: Harmonize public counts (2,700 vs 2,800) and verify "10,000+ tools"; fix bare mcp.pipedream.com link
Short: repo contains mixed counts — README uses "2,800+" while docs use PUBLIC_APPS = "2,700" (used via {process.env.PUBLIC_APPS}), and "10,000+ tools" appears separately. Also a bare mcp.pipedream.com link was found. Please standardize and confirm the authoritative numbers.
Locations to update (examples found):
- modelcontextprotocol/README.md
- line 9: "- 2,800+ APIs and 10,000+ tools through a single server"
- line 36: "- Run your own MCP server for over 2,800 apps and APIs"
- line 72: "This is currently available at mcp.pipedream.com" — missing https scheme
- docs-v2/next.config.mjs (line 649): PUBLIC_APPS: "2,700" — many docs reference {process.env.PUBLIC_APPS}
- Representative docs using the placeholder / 10,000+ wording:
- docs-v2/pages/connect/mcp/developers.mdx:5
- docs-v2/pages/connect/mcp/openai.mdx:7
- docs-v2/pages/rest-api/index.mdx:749
- docs-v2/pages/connect/index.mdx:27-28
Suggested fixes (pick one canonical approach):
- Decide the authoritative count (2,700 vs 2,800). Update docs to match the canonical value.
- Best practice: keep a single source of truth (PUBLIC_APPS in docs-v2/next.config.mjs) and reference {process.env.PUBLIC_APPS} everywhere, or update PUBLIC_APPS to the correct value.
- Verify that "10,000+ tools" is accurate; if so, harmonize phrasing across docs (e.g., "10,000+ tools" or "over 10,000 pre-built tools").
- Replace the bare link with an explicit scheme: change mcp.pipedream.com → mcp.pipedream.com.
Example replacement (README):
- Replace "- 2,800+ APIs and 10,000+ tools…" with a canonical phrasing, e.g.:
- "- over {process.env.PUBLIC_APPS}+ apps and APIs and 10,000+ tools…" (or update PUBLIC_APPS to "2,800" if that's the correct number)
Files listed above (and other docs that interpolate PUBLIC_APPS) should be updated for consistency.
🧰 Tools
🪛 LanguageTool
[grammar] ~9-~9: There might be a mistake here.
Context: ... 10,000+ tools** through a single server - Built-in authentication: no manual tok...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...n**: no manual token management required - Multiple tool modes: sub-agent and ful...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...odes**: sub-agent and full configuration - Automatic app discovery - **Enterprise...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...onfiguration - Automatic app discovery - **Enterprise-grade reliability and securit...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In modelcontextprotocol/README.md around lines 9–13 and 36, plus line 72, and in
docs-v2/next.config.mjs at line 649 (and other docs that reference PUBLIC_APPS),
the repo contains inconsistent public app counts (2,700 vs 2,800), a possibly
unverified "10,000+ tools" claim, and a bare mcp.pipedream.com link. Pick a
single authoritative source of truth (preferably PUBLIC_APPS in
docs-v2/next.config.mjs), update PUBLIC_APPS to the correct canonical number
(either 2700 or 2800) and ensure all docs reference {process.env.PUBLIC_APPS} or
the same literal value, standardize the "10,000+ tools" phrasing across files
after verification (e.g., "10,000+ tools" or "over 10,000 pre-built tools"), and
replace the bare markdown link [mcp.pipedream.com](mcp.pipedream.com) with an
explicit scheme [mcp.pipedream.com](https://mcp.pipedream.com); apply these
changes to the README and all listed docs so counts and links are consistent.
WHY
Summary by CodeRabbit
New Features
Documentation
Chores