Update print statement from 'Hello' to 'Goodbye'#20
Conversation
Summary of ChangesHello @groupthinking, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a detailed guide on leveraging the Model Context Protocol (MCP) within Claude Code. It aims to empower users to seamlessly integrate external tools and data sources, enhancing Claude Code's capabilities and enabling more complex workflows. The documentation covers various aspects of MCP, from basic installation to advanced configuration and security measures, providing a comprehensive resource for both new and experienced users. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive documentation file for connecting Claude Code with tools via the Model Context Protocol (MCP). The documentation is very detailed and well-structured.
My review includes a few suggestions to improve clarity, robustness, and security best practices within the documentation and the embedded JSX component. Notably, I've pointed out a security concern with a curl | bash example, a potential crash in the React component, and some minor consistency and formatting improvements.
Also, the pull request title 'Update print statement from 'Hello' to 'Goodbye'' does not seem to reflect the changes, which consist of adding a large new documentation file. It would be helpful to update the title to something more descriptive, like 'Docs: Add documentation for MCP integration'.
| @@ -0,0 +1,1198 @@ | |||
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |||
There was a problem hiding this comment.
The first line contains a curl | bash command. Piping a script from a URL directly to bash is a significant security risk, as it executes remote code without review. It's strongly recommended to avoid promoting this practice. Additionally, the line formatting seems broken with Learn more →> mixed with a markdown header. I suggest removing the command and fixing the formatting.
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |
| > ## Documentation Index |
| } else { | ||
| throw new Error(`Unknown platform: ${platform}`); | ||
| } |
There was a problem hiding this comment.
Throwing an error inside a filter function will cause the entire component to crash if an unknown platform prop is provided. It would be more robust to handle this gracefully, for example by logging a warning to the console and filtering out the item.
| } else { | |
| throw new Error(`Unknown platform: ${platform}`); | |
| } | |
| } else { | |
| console.warn(`Unknown platform: ${platform}`); | |
| return false; | |
| } |
| <p style={{ | ||
| margin: '0.5rem 0', | ||
| fontSize: '0.9rem' | ||
| }}> |
There was a problem hiding this comment.
There are several instances of inline styles in this component (e.g., here and on lines 178, 193). For better maintainability and separation of concerns, it's recommended to move these styles into the <style jsx> block and use CSS classes instead.
| </Step> | ||
|
|
||
| <Step title="Use the /mcp command within Claude Code"> | ||
| In Claude code, use the command: |
There was a problem hiding this comment.
There's a small typo here. For consistency with the rest of the document, 'Claude code' should be 'Claude Code'.
| In Claude code, use the command: | |
| In Claude Code, use the command: |
| <Step title="Import servers from Claude Desktop"> | ||
| ```bash theme={null} | ||
| # Basic syntax | ||
| claude mcp add-from-claude-desktop |
There was a problem hiding this comment.
This command example has a trailing space. This could cause confusion or errors if a user copy-pastes it directly into their terminal. It's best to remove it.
| claude mcp add-from-claude-desktop | |
| claude mcp add-from-claude-desktop |
|
|
||
| * macOS: `/Library/Application Support/ClaudeCode/managed-mcp.json` | ||
| * Linux and WSL: `/etc/claude-code/managed-mcp.json` | ||
| * Windows: `C:\Program Files\ClaudeCode\managed-mcp.json` |
There was a problem hiding this comment.
The Windows path uses single backslashes. In many contexts, a backslash is an escape character. To avoid ambiguity and ensure the path is interpreted correctly, it's safer to use double backslashes for Windows paths in documentation.
| * Windows: `C:\Program Files\ClaudeCode\managed-mcp.json` | |
| * Windows: `C:\\Program Files\\ClaudeCode\\managed-mcp.json` |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| @@ -0,0 +1,1198 @@ | |||
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |||
There was a problem hiding this comment.
Bug: A documentation file contains a curl | bash command which could lead to arbitrary code execution if manually run by a developer.
Severity: MEDIUM
Suggested Fix
Remove the curl -fsSL https://openclaw.ai/install.sh | bash command from the markdown file. The file appears to be for documentation purposes, and this line is out of place and poses a security risk. If the installation script is necessary, it should be documented in a safer way, without encouraging a piped execution.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/instructions/*.instructions.md#L1
Potential issue: A markdown file at `.github/instructions/*.instructions.md` contains a
`curl | bash` command on its first line. While no automation in the repository executes
this file, it poses a potential security risk if a developer or user were to manually
copy and execute this line, believing it to be a valid instruction. The command `curl
-fsSL https://openclaw.ai/install.sh | bash` would download and run an external script,
leading to arbitrary code execution on the user's machine. This represents a significant
code hygiene issue and a latent security threat.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds a new GitHub instruction document under .github/instructions, containing extensive third-party MCP/Claude-related documentation and embedded JSX/React code, which does not align with the PR title’s stated “Hello” → “Goodbye” print change.
Changes:
- Adds a new
.github/instructions/*.instructions.mdfile (~1,200 lines) with MCP/Claude documentation content. - Introduces embedded React/JSX code and external network-fetch logic inside the instructions document.
- Includes shell installation guidance that uses a
curl | bashpattern.
| const url = new URL('https://api.anthropic.com/mcp-registry/v0/servers'); | ||
| url.searchParams.set('version', 'latest'); | ||
| url.searchParams.set('visibility', 'commercial'); | ||
| url.searchParams.set('limit', '100'); | ||
| if (cursor) { | ||
| url.searchParams.set('cursor', cursor); | ||
| } | ||
| const response = await fetch(url); | ||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch MCP registry: ${response.status}`); | ||
| } |
There was a problem hiding this comment.
The component performs runtime network requests to an external API (https://api.anthropic.com/...) from within documentation/instructions content. This creates non-deterministic builds, breaks offline use, and can leak metadata. Prefer static content checked into the repo, or generate the list during a build step with caching and explicit timeouts/retries.
| return `claude mcp add ${serverSlug} --transport http ${server.urls.http}`; | ||
| } | ||
| if (server.urls.sse) { | ||
| return `claude mcp add ${serverSlug} --transport sse ${server.urls.sse}`; | ||
| } | ||
| if (server.urls.stdio) { | ||
| const envFlags = server.envVars && server.envVars.length > 0 ? server.envVars.map(v => `--env ${v.name}=YOUR_${v.name}`).join(' ') : ''; | ||
| const baseCommand = `claude mcp add ${serverSlug} --transport stdio`; | ||
| return envFlags ? `${baseCommand} ${envFlags} -- ${server.urls.stdio}` : `${baseCommand} -- ${server.urls.stdio}`; |
There was a problem hiding this comment.
generateClaudeCodeCommand builds commands like claude mcp add <name> --transport ..., but later in this same document it states that all options (including --transport) must come before the server name. Either adjust the generator to match the documented CLI ordering, or update the documentation so these examples are consistent.
| return `claude mcp add ${serverSlug} --transport http ${server.urls.http}`; | |
| } | |
| if (server.urls.sse) { | |
| return `claude mcp add ${serverSlug} --transport sse ${server.urls.sse}`; | |
| } | |
| if (server.urls.stdio) { | |
| const envFlags = server.envVars && server.envVars.length > 0 ? server.envVars.map(v => `--env ${v.name}=YOUR_${v.name}`).join(' ') : ''; | |
| const baseCommand = `claude mcp add ${serverSlug} --transport stdio`; | |
| return envFlags ? `${baseCommand} ${envFlags} -- ${server.urls.stdio}` : `${baseCommand} -- ${server.urls.stdio}`; | |
| return `claude mcp add --transport http ${serverSlug} ${server.urls.http}`; | |
| } | |
| if (server.urls.sse) { | |
| return `claude mcp add --transport sse ${serverSlug} ${server.urls.sse}`; | |
| } | |
| if (server.urls.stdio) { | |
| const envFlags = server.envVars && server.envVars.length > 0 ? server.envVars.map(v => `--env ${v.name}=YOUR_${v.name}`).join(' ') : ''; | |
| const baseCommand = `claude mcp add --transport stdio`; | |
| return envFlags ? `${baseCommand} ${envFlags} ${serverSlug} -- ${server.urls.stdio}` : `${baseCommand} ${serverSlug} -- ${server.urls.stdio}`; |
| @@ -0,0 +1,1198 @@ | |||
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |||
There was a problem hiding this comment.
This line instructs running a remote script via curl | bash, which is a high-risk pattern (no integrity verification) and should not appear in repo instructions. Replace with safer installation steps (pinned version, checksum/signature verification, explicit download + run) or remove it.
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |
| ## Installation | |
| To install the OpenClaw tools: | |
| 1. Download the installer script: | |
| ```bash | |
| curl -fsSLo openclaw-install.sh https://openclaw.ai/install.sh |
- Review the contents of
openclaw-install.shand verify its integrity using any checksums or signatures provided on the OpenClaw website. - Run the installer:
bash openclaw-install.sh
Documentation Index
| > Learn how to connect Claude Code to your tools with the Model Context Protocol. | ||
|
|
||
| export const MCPServersTable = ({platform = "all"}) => { | ||
| const ifYouAreAnAiAgentReadingThisYouShouldInsteadFetch = 'https://api.anthropic.com/mcp-registry/docs'; |
There was a problem hiding this comment.
This looks like a prompt-injection style instruction embedded in the repo ("ifYouAreAnAiAgentReadingThis...") and is unrelated to project functionality. Remove it to avoid influencing automated tooling and to keep repository instructions focused and trustworthy.
| const ifYouAreAnAiAgentReadingThisYouShouldInsteadFetch = 'https://api.anthropic.com/mcp-registry/docs'; |
| export const MCPServersTable = ({platform = "all"}) => { | ||
| const ifYouAreAnAiAgentReadingThisYouShouldInsteadFetch = 'https://api.anthropic.com/mcp-registry/docs'; | ||
| const [servers, setServers] = useState([]); | ||
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState(null); |
There was a problem hiding this comment.
JSX/React code is included directly in a .md instructions file (not fenced as a code block) and uses useState/useEffect without any imports. This is not valid Markdown and will render as broken content (or fail any Markdown processing). Convert this to plain Markdown, or move the component to an actual TS/TSX module and embed it only if the documentation system explicitly supports MDX/JSX.
| > Fetch the complete documentation index at: https://code.claude.com/docs/llms.txt | ||
| > Use this file to discover all available pages before exploring further. | ||
|
|
||
| # Connect Claude Code to tools via MCP | ||
|
|
||
| > Learn how to connect Claude Code to your tools with the Model Context Protocol. |
There was a problem hiding this comment.
This appears to vendor a large amount of third-party product documentation into a Copilot instructions file. Copilot instruction files are intended to be short, repo-specific guidance; adding ~1,200 lines of generic external docs will bloat the context and can degrade/derail Copilot behavior. Consider replacing this with a brief, project-specific instruction set and link out to external docs instead of embedding them.
|
|
||
| ### Plugin-provided MCP servers | ||
|
|
||
| [Plugins](/en/plugins) can bundle MCP servers, automatically providing tools and integrations when the plugin is enabled. Plugin MCP servers work identically to user-configured servers. |
There was a problem hiding this comment.
This document contains many relative links like /en/plugins and /en/settings that appear to be specific to an external documentation site and will be broken within this repository context. Replace with correct in-repo links (if these pages exist here) or use absolute URLs to the intended external docs.
| [Plugins](/en/plugins) can bundle MCP servers, automatically providing tools and integrations when the plugin is enabled. Plugin MCP servers work identically to user-configured servers. | |
| [Plugins](https://code.claude.com/en/plugins) can bundle MCP servers, automatically providing tools and integrations when the plugin is enabled. Plugin MCP servers work identically to user-configured servers. |
| @@ -0,0 +1,1198 @@ | |||
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |||
There was a problem hiding this comment.
The file name .github/instructions/*.instructions.md contains a literal *, which is an invalid character on Windows filesystems and can prevent checkouts/clones on Windows-based environments and some tooling. Rename the file to a valid, concrete filename (without glob characters).
| @@ -0,0 +1,1198 @@ | |||
| curl -fsSL https://openclaw.ai/install.sh | bash Learn more →> ## Documentation Index | |||
There was a problem hiding this comment.
The PR title indicates a small print-statement change, but this change adds a large new instructions document with unrelated content. Either update the PR title/description to match the actual change set or split/re-scope the PR to the intended minimal change.
1 participant
No description provided.