-
Notifications
You must be signed in to change notification settings - Fork 87
[generate_manifest] deprecate the former workflow & improve the script #247
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
WalkthroughRenames a GitHub Actions workflow display name to mark deprecation. Updates scripts/get_manifest.py to adopt an installations array format, adjust validation wording and processing, build cleaned_installations, reassign to manifest.installations when valid, and add before/after debug logging. No public function signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/CI
participant GM as get_manifest.py
participant VAL as Validator (API/LLM)
participant SCH as Schema & Rules
Dev->>GM: Run manifest generation
GM->>GM: Build draft manifest
GM->>GM: Log "Before: …"
GM->>VAL: Submit validation request (expect installations[])
VAL->>SCH: Check schema + exact-args rules
SCH-->>VAL: Validated entries or errors
VAL-->>GM: Validation result (installations[])
alt Valid entries present
GM->>GM: Build cleaned_installations
GM->>GM: manifest.installations = cleaned_installations
else No valid entries
GM->>GM: Leave manifest.installations unchanged/empty
end
GM->>GM: Log "After: …"
GM-->>Dev: Output final manifest
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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. ✨ 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 (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
CI Feedback 🧐(Feedback updated until commit 709eae1)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/get_manifest.py (1)
234-240: Resolve contradictory rules and correct return shape to an array.The prompt says “Use exact arguments” but then forces “-y” for npx. Also it asks to return a map while examples show an array. Align the rules to avoid the model producing output your code rejects.
-CRITICAL RULES: -- Use exact argument from README (don't guess or modify) -- Match the schema format as shown in examples +CRITICAL RULES: +- Use exact arguments from README (don't guess or modify) +- Match the schema format as shown in examples - Include ALL installation methods mentioned in README - Remove installation methods NOT mentioned in README -- For npx: always use type "npm" with command "npx" and args ["-y", "package-name"] +-- For npx: set type "npm" and command "npx"; use args exactly as shown in README (do not force "-y") -Return ONLY: {"installations": {{...}}} +Return ONLY: {"installations": []}Additionally, update the validator to accept an array and not require “-y” unconditionally (outside the selected lines):
# In validate_installations(), replace mapping logic with list handling validated = validated_data.get("installations") if isinstance(validated, list): cleaned_installations = [] for entry in validated: install_type = entry.get("type") if validate_installation_entry(install_type, entry): cleaned_installations.append(entry) else: print(f"⚠ Removing invalid {install_type} installation entry") if cleaned_installations: print("✓ Installations validated and corrected") manifest["installations"] = cleaned_installations else: print("⚠ No valid installations found, keeping original") return manifest print("⚠ Validation failed, keeping original installations") return manifest# In validate_installation_entry(), relax npx rule and strengthen args length if install_type == "npm": if entry.get("command") != "npx": return False args = entry.get("args", []) if not isinstance(args, list) or not args: return False # Do not force '-y'; just require the package is present somewhere if not any(isinstance(a, str) and a.startswith("@") for a in args): return False
🧹 Nitpick comments (1)
scripts/get_manifest.py (1)
318-320: Avoid dumping secrets to logs; redact env values in Before/After.Manifests may include API keys in the
envmap. Redact before logging.-print(f"Before: {json.dumps(manifest, indent=2)}") +print(f"Before: {json.dumps(redact_manifest_for_logging(manifest), indent=2)}") manifest = validate_installations(manifest, args.repo_url) -print(f"After: {json.dumps(manifest, indent=2)}") +print(f"After: {json.dumps(redact_manifest_for_logging(manifest), indent=2)}")Add this helper (outside the selected lines):
def redact_manifest_for_logging(m: dict) -> dict: def _redact(obj): if isinstance(obj, dict): redacted = {} for k, v in obj.items(): if k == "env" and isinstance(v, dict): redacted[k] = {ek: "<redacted>" for ek in v.keys()} else: redacted[k] = _redact(v) return redacted if isinstance(obj, list): return [_redact(x) for x in obj] return obj return _redact(m)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/mcp-server-info-bot.yml(1 hunks)scripts/get_manifest.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
scripts/get_manifest.py
🪛 YAMLlint (1.37.1)
.github/workflows/mcp-server-info-bot.yml
[error] 1-1: syntax error: expected , but found ''
(syntax)
| @@ -1,4 +1,4 @@ | |||
| name: MCP Server Info Bot | |||
| name: [DEPRECATED] MCP Server Info Bot | |||
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.
Quote the workflow name to fix YAML syntax error.
Unquoted square brackets are parsed as a flow sequence in YAML. YAMLlint’s error aligns with this. Quote the name to keep it a plain string.
-name: [DEPRECATED] MCP Server Info Bot
+name: "[DEPRECATED] MCP Server Info Bot"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: [DEPRECATED] MCP Server Info Bot | |
| name: "[DEPRECATED] MCP Server Info Bot" |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
.github/workflows/mcp-server-info-bot.yml lines 1-1: the workflow name contains
unquoted square brackets which YAML parses as a flow sequence; fix by
surrounding the entire name value in quotes (single or double) so it is treated
as a plain string, e.g., replace the unquoted name with a quoted string
containing the same text.
| <README> Docker | ||
| {{ | ||
| "mcpServers": {{ | ||
| "brave-search": {{ | ||
| "command": "docker", | ||
| "args": [ | ||
| "run", | ||
| "-i", | ||
| "--rm", | ||
| "-e", | ||
| "BRAVE_API_KEY", | ||
| "mcp/brave-search" | ||
| ], | ||
| "env": {{ | ||
| "BRAVE_API_KEY": "YOUR_API_KEY_HERE" | ||
| }} | ||
| }} | ||
| }} | ||
| }} | ||
| ``` | ||
| OR with git URL: | ||
| ```json | ||
| "uvx": {{ | ||
| "type": "uvx", | ||
| "command": "uvx", | ||
| "args": ["--from", "git+https://github.com/user/repo", "command-name"], | ||
| "description": "Install from git" | ||
| NPX | ||
| {{ | ||
| "mcpServers": {{ | ||
| "brave-search": {{ | ||
| "command": "npx", | ||
| "args": [ | ||
| "-y", | ||
| "@modelcontextprotocol/server-brave-search" | ||
| ], | ||
| "env": {{ | ||
| "BRAVE_API_KEY": "YOUR_API_KEY_HERE" | ||
| }} | ||
| }} | ||
| }} | ||
| }} | ||
| ``` | ||
| 3. DOCKER INSTALLATIONS: | ||
| ```json | ||
| "docker": {{ | ||
| "type": "docker", | ||
| "command": "docker", | ||
| "args": ["run", "-i", "--rm", "image-name"], | ||
| "description": "Run using Docker" | ||
| </README> | ||
| From the example README, you should get: | ||
| {{ | ||
| "installations": [ | ||
| {{ | ||
| "type": "docker", | ||
| "command": "docker", | ||
| "args": [ | ||
| "run", | ||
| "-i", | ||
| "--rm", | ||
| "-e", | ||
| "BRAVE_API_KEY", | ||
| "mcp/brave-search" | ||
| ], | ||
| "env": {{ | ||
| "BRAVE_API_KEY": "${{YOUR_API_KEY_HERE}}" | ||
| }} | ||
| }}, | ||
| {{ | ||
| "type": "npm", | ||
| "command": "npx", | ||
| "args": [ | ||
| "-y", | ||
| "@modelcontextprotocol/server-brave-search" | ||
| ], | ||
| "env": {{ | ||
| "BRAVE_API_KEY": "${{YOUR_API_KEY_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.
Fix f-string brace escaping; this block will not compile as written.
The multi-line f-string contains many literal JSON braces that aren’t escaped. Python requires doubling braces inside f-strings, otherwise you get a SyntaxError at import time. Also, the example/return-shape messaging mixes array vs. map forms for "installations". Replace the large JSON example with a prebuilt string and inject it via a single {examples_block} placeholder.
Apply this focused diff to eliminate the brace-escape problem and keep the string readable:
- INSTALLATION SCHEMA EXAMPLES:
-<README> Docker
-{
- "mcpServers": {{
- "brave-search": {{
- "command": "docker",
- "args": [
- "run",
- "-i",
- "--rm",
- "-e",
- "BRAVE_API_KEY",
- "mcp/brave-search"
- ],
- "env": {{
- "BRAVE_API_KEY": "YOUR_API_KEY_HERE"
- }}
- }}
- }}
-}
-NPX
-{
- "mcpServers": {{
- "brave-search": {{
- "command": "npx",
- "args": [
- "-y",
- "@modelcontextprotocol/server-brave-search"
- ],
- "env": {{
- "BRAVE_API_KEY": "YOUR_API_KEY_HERE"
- }}
- }}
- }}
-}
-</README>
-From the example README, you should get:
-{
- "installations": [
- {{
- "type": "docker",
- "command": "docker",
- "args": [
- "run",
- "-i",
- "--rm",
- "-e",
- "BRAVE_API_KEY",
- "mcp/brave-search"
- ],
- "env": {{
- "BRAVE_API_KEY": "${{YOUR_API_KEY_HERE}}"
- }}
- }},
- {{
- "type": "npm",
- "command": "npx",
- "args": [
- "-y",
- "@modelcontextprotocol/server-brave-search"
- ],
- "env": {{
- "BRAVE_API_KEY": "${{YOUR_API_KEY_HERE}}"
- }}
- }}
- ]
-}
+ INSTALLATION SCHEMA EXAMPLES:
+{examples_block}Add this helper (outside the selected lines) before constructing payload inside validate_installations:
from textwrap import dedent # at top-level imports
# inside validate_installations(), just before 'payload = {...}'
examples_block = dedent("""
<README> Docker
{
"mcpServers": {
"brave-search": {
"command": "docker",
"args": [
"run",
"-i",
"--rm",
"-e",
"BRAVE_API_KEY",
"mcp/brave-search"
],
"env": {
"BRAVE_API_KEY": "YOUR_API_KEY_HERE"
}
}
}
}
NPX
{
"mcpServers": {
"brave-search": {
"command": "npx",
"args": [
"-y",
"@modelcontextprotocol/server-brave-search"
],
"env": {
"BRAVE_API_KEY": "YOUR_API_KEY_HERE"
}
}
}
}
</README>
From the example README, you should get:
{
"installations": [
{
"type": "docker",
"command": "docker",
"args": [
"run",
"-i",
"--rm",
"-e",
"BRAVE_API_KEY",
"mcp/brave-search"
],
"env": {
"BRAVE_API_KEY": "${YOUR_API_KEY_HERE}"
}
},
{
"type": "npm",
"command": "npx",
"args": [
"-y",
"@modelcontextprotocol/server-brave-search"
],
"env": {
"BRAVE_API_KEY": "${YOUR_API_KEY_HERE}"
}
}
]
}
""")🤖 Prompt for AI Agents
In scripts/get_manifest.py around lines 157-223, the multi-line f-string
contains unescaped JSON braces causing SyntaxError; replace the large inline
f-string with a normal raw/regular string built via textwrap.dedent assigned to
examples_block just before constructing payload inside validate_installations,
add "from textwrap import dedent" to top-level imports, and then use the single
{examples_block} placeholder in the payload string instead of embedding the JSON
with braces directly so no f-string brace escaping is required.
|
🎉 This PR is included in version 2.7.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR Type
Enhancement
Description
Improved manifest generation script with better installation validation
Deprecated old workflow in favor of enhanced script
Enhanced schema examples and validation process
Added debug logging for installation validation
Diagram Walkthrough
File Walkthrough
get_manifest.py
Enhanced manifest generation with better validationscripts/get_manifest.py
mcp-server-info-bot.yml
Deprecated workflow marking.github/workflows/mcp-server-info-bot.yml
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores