Skip to content

fix(stack): pin MCP versions and document server pre-approval - #520

Open
shaurya57-drona wants to merge 1 commit into
nextlevelbuilder:mainfrom
shaurya57-drona:patch-4
Open

shaurya57-drona wants to merge 1 commit into
nextlevelbuilder:mainfrom
shaurya57-drona:patch-4

Conversation

@shaurya57-drona

@shaurya57-drona shaurya57-drona commented Oct 8, 2026 •

Copy link
Copy Markdown

Added section on MCP Servers and Permissions to README.

What does this PR change?

Why?

Checklist

  • Changes were made in src/ui-ux-pro-max/ (source of truth), not directly in .claude/ or .factory/

  • Ran npm run sync:assets && npm run check:assets in cli/ if data/scripts/templates changed

  • Added or updated tests if behavior changed (.claude/skills/*/scripts/tests/, cli/tests/e2e/)

  • Commit messages follow Conventional Commits (feat:, fix:, docs:, etc.)

  • This PR targets a feature branch, not pushed directly to main
    Addresses the audit findings reported in stack/ pre-approves every project MCP server (enableAllProjectMcpServers: true) #514:

  • Version Pinning: Pinned @playwright/mcp, chrome-devtools-mcp, and shadcn to stable versions in stack/.mcp.json.

  • Permission Transparency: Documented enableAllProjectMcpServers: true in stack/README.md so users understand the pre-approval behavior and know how to disable it.

Closes #514

Added section on MCP Servers and Permissions to README.
@shaurya57-drona

Copy link
Copy Markdown
Author

Addresses the audit findings reported in #514:

  • Version Pinning: Pinned @playwright/mcp, chrome-devtools-mcp, and shadcn to stable versions in stack/.mcp.json.
  • Permission Transparency: Documented enableAllProjectMcpServers: true in stack/README.md so users understand the pre-approval behavior and know how to disable it.

Closes #514

@clark-cant clark-cant 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.

Summary: Docs-only addition to stack/README.md documenting the three bundled MCP servers and the enableAllProjectMcpServers: true pre-approval default (addresses the audit in #514).

Risk level: Low

Mandatory gates

  • Duplicate / prior implementation: overlap — complementary to open PR #519 (same author, same issue #514; #519 carries the .mcp.json version pinning, this PR carries the README doc). No merged prior art documents MCP permissions in stack/README.md.
  • Project standards: docs found (CLAUDE.md, CONTRIBUTING.md); editing stack/ directly matches the merged precedent #517 (also touched stack/README.md).
  • Strategic necessity: clear value — documents a security-relevant default (project-wide MCP server pre-approval) and how to opt out.

Findings

Important

  1. Doc ↔ config drift on the pinned versions. The new section states the three servers are pinned (@playwright/mcp@0.0.83, chrome-devtools-mcp@1.10.1, shadcn@4.21.0), but this PR only touches stack/README.md. On main today stack/.mcp.json still uses @latest for all three — the pinning lives in the separate, unmerged #519. Merging this PR on its own makes the README describe configuration that does not exist. Please land the pinning first (or together), or reword the section so the doc matches main until #519 merges.

  2. Missing blank line breaks rendering of existing content. In the diff the new > blockquote is immediately followed by the existing closing line Built as a starter for teams who want Claude Code to ship… with no blank line between them. In CommonMark that line becomes a lazy continuation of the blockquote paragraph, so the closing tagline renders inside the security note. Add a blank line after the blockquote.

  3. Title/body over-claim the scope. The title says “pin MCP versions and document server pre-approval” and the body repeats #519’s bullet list (“Pinned … in stack/.mcp.json”), but this PR changes only README.md. Align the title/body with the doc-only scope, and keep the “pinned” claim in the PR that actually pins the versions.

Suggestion

  1. Section placement. The new heading is inserted after the License section’s horizontal rule (---), sitting between that rule and the closing tagline. Consider moving the block above ## Requirements (or into “What’s in here”) so the License rule and closing tagline stay last.

  2. Heading level. Every other section in this file is ##; the new one is ###. Use ## for consistency (or deliberately nest it under an existing section).

Verdict: Request changes — Important: version/doc drift vs main, blockquote misrender of existing content, and title/body scope over-claim.

Posted by /ck:review-pr at 2026-10-08T20:07:09Z

@shaurya57-drona shaurya57-drona left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

okay

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

Labels

None yet

2 participants