Skip to content

Refactor MCP config generation into SDK helper#1179

Open
weinong wants to merge 1 commit into
bradygaster:devfrom
weinong:refactor-mcp-config-helper
Open

Refactor MCP config generation into SDK helper#1179
weinong wants to merge 1 commit into
bradygaster:devfrom
weinong:refactor-mcp-config-helper

Conversation

@weinong

@weinong weinong commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • extract duplicated MCP server/config/frontmatter generation into @bradygaster/squad-sdk/config
  • update init and upgrade paths to use the shared MCP helper
  • align CLI SDK dependency minimum and lockfile metadata with the new SDK export
  • add behavioral package-export coverage for the shared MCP helper

Tests

  • npm test -- test/package-exports.test.ts test/cli/init.test.ts test/cli/upgrade.test.ts
  • npm run lint

Follow-up to #1166

Copilot AI review requested due to automatic review settings May 26, 2026 20:00

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR extracts MCP (Model Context Protocol) configuration helpers into a shared SDK module, re-exports them from @bradygaster/squad-sdk/config, and updates the CLI to consume the shared helpers.

Changes:

  • Added mcp-config.ts to centralize MCP server spec generation and frontmatter/config rendering helpers.
  • Refactored SDK init.ts and CLI upgrade.ts to import MCP helpers from the shared module instead of duplicating logic.
  • Added export verification + behavior tests and bumped package versions to align CLI ↔ SDK.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/package-exports.test.ts Adds export checks and behavior tests for MCP helper functions exposed via the config barrel.
packages/squad-sdk/src/config/mcp-config.ts Introduces centralized MCP server spec + config/frontmatter helper implementations.
packages/squad-sdk/src/config/init.ts Removes inlined MCP helper implementations and uses the shared mcp-config module.
packages/squad-sdk/src/config/index.ts Re-exports the new MCP config module from the SDK config barrel.
packages/squad-sdk/package.json Bumps SDK version to 0.9.7-preview.
packages/squad-cli/src/cli/core/upgrade.ts Replaces duplicated MCP helper code with imports from @bradygaster/squad-sdk/config.
packages/squad-cli/package.json Updates CLI dependency constraint to require the new SDK version.

Comment on lines +77 to +95
export function injectMcpFrontmatter(content: string, servers: McpServerSpec[]): string {
const closingStart = content.indexOf('\n---', 4);
if (!content.startsWith('---') || closingStart === -1) {
return content;
}

return content.slice(0, closingStart)
+ '\n'
+ buildMcpFrontmatterBlock(servers)
+ content.slice(closingStart);
}

export function hasMcpFrontmatter(content: string): boolean {
const frontmatterEnd = content.indexOf('\n---', 4);
if (!content.startsWith('---') || frontmatterEnd === -1) {
return false;
}
return content.slice(0, frontmatterEnd).includes('mcp-servers:');
}
expect(injected).toContain('mcp-servers:\n squad_state:');
expect(injected).toContain('\n---\n\nBody mentions mcp-servers: only here.\n');
});

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

👋 Friendly nudge — this PR has had no activity for 12 days.

What needs attention:

  • 🔴 1 CI check(s) failing: Policy Gates. Fix these first.
  • 💬 2 unresolved review thread(s) (2 from Copilot). Address and resolve them.
  • 👀 No approving reviews yet. Request a review from a teammate.

If this PR is abandoned, please close it. If it's blocked on something external, leave a comment so the team knows.
This is an automated check that runs on weekdays. It won't nudge the same PR more than once per week.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants