From 76e6b8b99542ff5d35227f975e64ee5c89365d96 Mon Sep 17 00:00:00 2001 From: dyoshikawa-claw Date: Tue, 31 Mar 2026 16:23:35 +0900 Subject: [PATCH 1/6] fix: preserve copilotcli mcp transport types --- src/features/mcp/copilotcli-mcp.test.ts | 110 +++++++++++++++++++++ src/features/mcp/copilotcli-mcp.ts | 40 ++++---- src/features/rules/copilotcli-rule.ts | 41 ++++++++ src/features/rules/rules-processor.test.ts | 6 +- src/features/rules/rules-processor.ts | 13 +++ 5 files changed, 187 insertions(+), 23 deletions(-) create mode 100644 src/features/rules/copilotcli-rule.ts diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts index 93ef81703..ef55f7768 100644 --- a/src/features/mcp/copilotcli-mcp.test.ts +++ b/src/features/mcp/copilotcli-mcp.test.ts @@ -364,6 +364,29 @@ describe("CopilotcliMcp", () => { ).rejects.toThrow('MCP server "no-command-server" is missing a command'); }); + it("should throw error when stdio server has unknown fields but no command", async () => { + const inputMcpServers = { + "unknown-fields-no-command": { + url: "http://localhost:3000/mcp", + headers: { + Authorization: "Bearer test-token", + }, + unknown_field: "value", + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + await expect( + CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }), + ).rejects.toThrow('MCP server "unknown-fields-no-command" is missing a command'); + }); + it("should handle command as array and merge remaining elements into args", async () => { const inputMcpServers = { "array-command-server": { @@ -391,6 +414,63 @@ describe("CopilotcliMcp", () => { }, }); }); + + it("should preserve http and sse servers without requiring command", async () => { + const inputMcpServers = { + "http-server": { + type: "http" as const, + url: "http://localhost:3000/mcp", + headers: { + Authorization: "Bearer token", + }, + tools: ["search"], + }, + "sse-server": { + type: "sse" as const, + url: "http://localhost:4000/sse", + headers: { + "X-Test": "true", + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }); + + expect(copilotCliMcp.getJson()).toEqual({ + mcpServers: inputMcpServers, + }); + }); + + it("should preserve existing non-stdio type when converting", async () => { + const inputMcpServers = { + "typed-server": { + type: "http" as const, + command: "node", + args: ["server.js"], + url: "http://localhost:3000/mcp", + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }); + + expect(copilotCliMcp.getJson()).toEqual({ + mcpServers: inputMcpServers, + }); + }); }); describe("toRulesyncMcp", () => { @@ -500,6 +580,36 @@ describe("CopilotcliMcp", () => { $schema: RULESYNC_MCP_SCHEMA_URL, }); }); + + it("should preserve non-stdio type when converting back to RulesyncMcp", () => { + const inputMcpServers = { + "http-server": { + type: "http" as const, + command: "node", + args: ["server.js"], + url: "http://localhost:3000/mcp", + }, + }; + const copilotCliMcp = new CopilotcliMcp({ + relativeDirPath: ".copilot", + relativeFilePath: "mcp-config.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + const rulesyncMcp = copilotCliMcp.toRulesyncMcp(); + + expect(rulesyncMcp.getJson()).toEqual({ + mcpServers: { + "http-server": { + type: "http", + command: "node", + args: ["server.js"], + url: "http://localhost:3000/mcp", + }, + }, + $schema: RULESYNC_MCP_SCHEMA_URL, + }); + }); }); describe("validate", () => { diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts index ff9753db8..a448fd96c 100644 --- a/src/features/mcp/copilotcli-mcp.ts +++ b/src/features/mcp/copilotcli-mcp.ts @@ -22,23 +22,29 @@ type CopilotcliMcpConfig = { /** * Adds "type": "stdio" to each MCP server config if not present. * GitHub Copilot CLI requires the "type" field for each server. - * @throws Error if a server doesn't have a command (Copilot CLI stdio servers require a command) + * @throws Error if a stdio server doesn't have a command */ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] { const result: NonNullable = {}; for (const [name, server] of Object.entries(mcpServers)) { - // Parse and validate the server config const parsed = McpServerSchema.parse(server); + const type = parsed.type ?? "stdio"; + + if (type !== "stdio") { + result[name] = { + ...parsed, + type, + }; + continue; + } - // Copilot CLI stdio servers require a non-empty command if (!parsed.command) { throw new Error( `MCP server "${name}" is missing a command. GitHub Copilot CLI stdio servers require a non-empty command.`, ); } - // Handle command as string or array let command: string; let args: string[] | undefined; @@ -46,29 +52,20 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] command = parsed.command; args = parsed.args; } else { - // command is an array: first element is command, rest are args const [cmd, ...cmdArgs] = parsed.command; if (!cmd) { throw new Error(`MCP server "${name}" has an empty command array.`); } command = cmd; - // Merge command array args with existing args args = cmdArgs.length > 0 ? [...cmdArgs, ...(parsed.args ?? [])] : parsed.args; } - // Use the parsed object for the base, then override with normalized command/args - // and ensure type is set to "stdio" if not present. - // We spread server as well to keep unknown fields as suggested by reviewers. - // eslint-disable-next-line no-type-assertion/no-type-assertion - const serverRecord = server as Record; - // eslint-disable-next-line no-type-assertion/no-type-assertion result[name] = { - ...serverRecord, ...parsed, - type: parsed.type ?? "stdio", + type, command, ...(args && { args }), - } as McpServer & Record; + }; } return result; @@ -81,6 +78,11 @@ function removeTypeField(config: CopilotcliMcpConfig): McpServers { const result: McpServers = {}; for (const [name, server] of Object.entries(config.mcpServers ?? {})) { + if (server.type !== "stdio") { + result[name] = server; + continue; + } + const { type: _, ...rest } = server; result[name] = rest; } @@ -109,13 +111,7 @@ export class CopilotcliMcp extends ToolMcp { return !this.global; } - static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { - if (global) { - return { - relativeDirPath: ".copilot", - relativeFilePath: "mcp-config.json", - }; - } + static getSettablePaths(_options: { global?: boolean } = {}): ToolMcpSettablePaths { return { relativeDirPath: ".copilot", relativeFilePath: "mcp-config.json", diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts new file mode 100644 index 000000000..8575be8d3 --- /dev/null +++ b/src/features/rules/copilotcli-rule.ts @@ -0,0 +1,41 @@ +import { ToolTarget } from "../../types/tool-targets.js"; +import { CopilotRule } from "./copilot-rule.js"; +import { RulesyncRule } from "./rulesync-rule.js"; +import { + ToolRuleForDeletionParams, + ToolRuleFromFileParams, + ToolRuleFromRulesyncRuleParams, +} from "./tool-rule.js"; + +export class CopilotcliRule extends CopilotRule { + private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { + return new CopilotcliRule({ + baseDir: copilotRule.getBaseDir(), + relativeDirPath: copilotRule.getRelativeDirPath(), + relativeFilePath: copilotRule.getRelativeFilePath(), + frontmatter: copilotRule.getFrontmatter(), + body: copilotRule.getBody(), + validate: true, + root: copilotRule.isRoot(), + }); + } + + static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { + return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); + } + + static override async fromFile(params: ToolRuleFromFileParams): Promise { + return this.fromCopilotRule(await CopilotRule.fromFile(params)); + } + + static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { + return this.fromCopilotRule(CopilotRule.forDeletion(params)); + } + + static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { + return this.isTargetedByRulesyncRuleDefault({ + rulesyncRule, + toolTarget: "copilotcli" satisfies ToolTarget, + }); + } +} diff --git a/src/features/rules/rules-processor.test.ts b/src/features/rules/rules-processor.test.ts index 60e3da0d5..cbab91f5d 100644 --- a/src/features/rules/rules-processor.test.ts +++ b/src/features/rules/rules-processor.test.ts @@ -11,6 +11,7 @@ import { AugmentcodeLegacyRule } from "./augmentcode-legacy-rule.js"; import { ClaudecodeLegacyRule } from "./claudecode-legacy-rule.js"; import { ClaudecodeRule } from "./claudecode-rule.js"; import { CopilotRule } from "./copilot-rule.js"; +import { CopilotcliRule } from "./copilotcli-rule.js"; import { CursorRule } from "./cursor-rule.js"; import { OpenCodeRule } from "./opencode-rule.js"; import { RovodevRule } from "./rovodev-rule.js"; @@ -186,6 +187,7 @@ describe("RulesProcessor", () => { it("should correctly validate and filter rules for each supported tool", async () => { const testCases = [ { toolTarget: "copilot" as const, ruleClass: CopilotRule }, + { toolTarget: "copilotcli" as const, ruleClass: CopilotcliRule }, { toolTarget: "cursor" as const, ruleClass: CursorRule }, { toolTarget: "claudecode" as const, ruleClass: ClaudecodeRule }, { toolTarget: "warp" as const, ruleClass: WarpRule }, @@ -795,6 +797,7 @@ Content that would fail parsing`; "claudecode-legacy", "codexcli", "copilot", + "copilotcli", "factorydroid", "geminicli", "goose", @@ -825,13 +828,14 @@ Content that would fail parsing`; expect(globalTargets).toContain("claudecode-legacy"); expect(globalTargets).toContain("codexcli"); expect(globalTargets).toContain("copilot"); + expect(globalTargets).toContain("copilotcli"); expect(globalTargets).toContain("factorydroid"); expect(globalTargets).toContain("geminicli"); expect(globalTargets).toContain("kilo"); expect(globalTargets).toContain("goose"); expect(globalTargets).toContain("opencode"); expect(globalTargets).toContain("rovodev"); - expect(globalTargets.length).toBe(10); + expect(globalTargets.length).toBe(11); // These targets should NOT be in global mode expect(globalTargets).not.toContain("cursor"); diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts index 2f64feb30..2886c85c1 100644 --- a/src/features/rules/rules-processor.ts +++ b/src/features/rules/rules-processor.ts @@ -39,6 +39,7 @@ import { ClaudecodeRule } from "./claudecode-rule.js"; import { ClineRule } from "./cline-rule.js"; import { CodexcliRule } from "./codexcli-rule.js"; import { CopilotRule } from "./copilot-rule.js"; +import { CopilotcliRule } from "./copilotcli-rule.js"; import { CursorRule } from "./cursor-rule.js"; import { DeepagentsRule } from "./deepagents-rule.js"; import { FactorydroidRule } from "./factorydroid-rule.js"; @@ -74,6 +75,7 @@ const rulesProcessorToolTargets: ToolTarget[] = [ "cline", "codexcli", "copilot", + "copilotcli", "cursor", "deepagents", "factorydroid", @@ -290,6 +292,17 @@ const toolRuleFactories = new Map([ }, }, ], + [ + "copilotcli", + { + class: CopilotcliRule, + meta: { + extension: "md", + supportsGlobal: true, + ruleDiscoveryMode: "auto", + }, + }, + ], [ "cursor", { From ab80217a3f34bf6726740655d675349badc57031 Mon Sep 17 00:00:00 2001 From: dyoshikawa-claw Date: Tue, 31 Mar 2026 18:50:22 +0900 Subject: [PATCH 2/6] fix: handle transport field and add copilotcli gitignore entries --- .issue-1401.md | 53 +++ .pr-1408.diff | 307 ++++++++++++++++++ .pr-1410.diff | 361 +++++++++++++++++++++ src/cli/commands/gitignore-entries.test.ts | 33 +- src/cli/commands/gitignore-entries.ts | 55 +++- src/features/mcp/copilotcli-mcp.test.ts | 61 ++++ src/features/mcp/copilotcli-mcp.ts | 24 +- src/types/mcp.ts | 2 +- 8 files changed, 882 insertions(+), 14 deletions(-) create mode 100644 .issue-1401.md create mode 100644 .pr-1408.diff create mode 100644 .pr-1410.diff diff --git a/.issue-1401.md b/.issue-1401.md new file mode 100644 index 000000000..e9a295124 --- /dev/null +++ b/.issue-1401.md @@ -0,0 +1,53 @@ +## Background + +PR #1376 adds GitHub Copilot CLI as a new MCP sync target (`copilotcli`), generating `.copilot/mcp-config.json`. A code review and security review were conducted, revealing several findings ranging from mid to low severity. This issue consolidates those findings for tracking and follow-up. + +## Details + +### Code Review Findings + +**#1 (mid) — Double type assertion and redundant spread in `addTypeField`** +In `copilotcli-mcp.ts`, `addTypeField` spreads both `server` (raw input) and `parsed` (Zod-validated output). This is redundant since `McpServerSchema` uses `z.looseObject()` and `parsed` already preserves unknown fields. The double `eslint-disable` and `as` casts weaken type safety unnecessarily. + +**#2 (mid) — `parsed.type ?? "stdio"` contradicts function contract** +The function name and JSDoc state it adds `"type": "stdio"`, but the new code preserves whatever `type` the input already has. The original code unconditionally set `type: "stdio"`. This is a behavioral regression — if Copilot CLI only supports stdio, the original hard-coded approach was correct. If other types are supported, the `command` validation should be conditional on the type. + +**#3 (mid) — Missing `rules-processor.ts` integration check** +Per `feature-change-guidelines.md`, new tool targets should be checked against `rules-processor.ts` and `additionalConvention`. The PR diff does not show changes to `rules-processor.ts`. + +**#4 (low) — Dead code in `getSettablePaths`** +Both the `global` and non-global branches return identical values (`{ relativeDirPath: ".copilot", relativeFilePath: "mcp-config.json" }`). The `if (global)` branch is dead code. + +**#5 (low) — `removeTypeField` unconditionally strips `type`** +If non-stdio types are now preserved by `addTypeField` (per #2), `removeTypeField` would lose that information when converting back to rulesync format. + +**#6 (low) — Documentation section placement in `file-formats.md`** +The new `.copilot/mcp-config.json` section is inserted before the general "optional override keys" paragraph about hooks, which may cause confusion. + +**#7 (low) — PR description test counts are misleading** +The counts (30 + 45 = 74) refer to total tests in each file, not newly added tests. + +**#8 (low) — Missing negative test for command-less servers with unknown fields** +No test verifies that a server with unknown fields but missing `command` still throws an error correctly. + +### Security Review Findings + +**#9 (low) — Theoretical prototype pollution risk in object spread** +Spreading raw `serverRecord` before Zod-validated `parsed` has a theoretical prototype pollution concern. In practice, JS object literal spread does not trigger this, and `parsed` overrides critical fields. Actual risk is minimal. + +**#10 (low) — Type assertion bypasses defense-in-depth** +The `as` casts suppress TypeScript type checking in the data transformation code, weakening a defense-in-depth layer. + +**#11 (low) — No sanitization of `command`/`args` fields** +Expected behavior for a config file generator operating on user-controlled local files. Consistent with all other tool targets in the codebase. + +**No malicious code, backdoors, obfuscated payloads, or suspicious dependency additions were detected.** + +## Solution / Next Steps + +1. **Address #2 first (highest impact):** Decide whether Copilot CLI supports only stdio or multiple transport types. If stdio-only, revert to hard-coded `type: "stdio"`. If multiple types, make `command` validation conditional on `type`. +2. **Simplify #1:** Spread only `parsed` (not raw `server`) to eliminate redundant type assertions. +3. **Verify #3:** Check `rules-processor.ts` and `additionalConvention` for the new `copilotcli` target. +4. **Clean up #4:** Remove the dead `if (global)` branch or add a TODO explaining the placeholder. +5. **Add test for #8:** Add a negative test case for servers with unknown fields but no `command`. +6. **Fix #6:** Adjust section placement in `file-formats.md` to avoid confusion with the hooks paragraph. diff --git a/.pr-1408.diff b/.pr-1408.diff new file mode 100644 index 000000000..da7cbef29 --- /dev/null +++ b/.pr-1408.diff @@ -0,0 +1,307 @@ +diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts +index 93ef8170..d5b956b9 100644 +--- a/src/features/mcp/copilotcli-mcp.test.ts ++++ b/src/features/mcp/copilotcli-mcp.test.ts +@@ -391,6 +391,60 @@ describe("CopilotcliMcp", () => { + }, + }); + }); ++ ++ it("should force stdio type even when another type is provided", async () => { ++ const inputMcpServers = { ++ "typed-server": { ++ type: "http" as const, ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }; ++ const rulesyncMcp = new RulesyncMcp({ ++ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, ++ relativeFilePath: "mcp.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ ++ rulesyncMcp, ++ }); ++ ++ expect(copilotCliMcp.getJson()).toEqual({ ++ mcpServers: { ++ "typed-server": { ++ type: "stdio", ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }, ++ }); ++ }); ++ ++ it("should throw error when server has unknown fields but no command", async () => { ++ const inputMcpServers = { ++ "unknown-fields-no-command": { ++ url: "http://localhost:3000/mcp", ++ headers: { ++ Authorization: "Bearer test-token", ++ }, ++ unknown_field: "value", ++ }, ++ }; ++ const rulesyncMcp = new RulesyncMcp({ ++ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, ++ relativeFilePath: "mcp.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ await expect( ++ CopilotcliMcp.fromRulesyncMcp({ ++ rulesyncMcp, ++ }), ++ ).rejects.toThrow('MCP server "unknown-fields-no-command" is missing a command'); ++ }); + }); + + describe("toRulesyncMcp", () => { +@@ -500,6 +554,36 @@ describe("CopilotcliMcp", () => { + $schema: RULESYNC_MCP_SCHEMA_URL, + }); + }); ++ ++ it("should preserve non-stdio type when converting back to RulesyncMcp", () => { ++ const inputMcpServers = { ++ "http-server": { ++ type: "http" as const, ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }; ++ const copilotCliMcp = new CopilotcliMcp({ ++ relativeDirPath: ".copilot", ++ relativeFilePath: "mcp-config.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ const rulesyncMcp = copilotCliMcp.toRulesyncMcp(); ++ ++ expect(rulesyncMcp.getJson()).toEqual({ ++ mcpServers: { ++ "http-server": { ++ type: "http", ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }, ++ $schema: RULESYNC_MCP_SCHEMA_URL, ++ }); ++ }); + }); + + describe("validate", () => { +diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts +index ff9753db..26371028 100644 +--- a/src/features/mcp/copilotcli-mcp.ts ++++ b/src/features/mcp/copilotcli-mcp.ts +@@ -1,7 +1,7 @@ + import { join } from "node:path"; + + import { ValidationResult } from "../../types/ai-file.js"; +-import { McpServerSchema, type McpServer, type McpServers } from "../../types/mcp.js"; ++import { McpServerSchema, type McpServers } from "../../types/mcp.js"; + import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; + import { RulesyncMcp } from "./rulesync-mcp.js"; + import { +@@ -16,11 +16,11 @@ import { + export type CopilotcliMcpParams = ToolMcpParams; + + type CopilotcliMcpConfig = { +- mcpServers?: Record>; ++ mcpServers?: McpServers; + }; + + /** +- * Adds "type": "stdio" to each MCP server config if not present. ++ * Adds "type": "stdio" to each MCP server config. + * GitHub Copilot CLI requires the "type" field for each server. + * @throws Error if a server doesn't have a command (Copilot CLI stdio servers require a command) + */ +@@ -56,19 +56,12 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] + args = cmdArgs.length > 0 ? [...cmdArgs, ...(parsed.args ?? [])] : parsed.args; + } + +- // Use the parsed object for the base, then override with normalized command/args +- // and ensure type is set to "stdio" if not present. +- // We spread server as well to keep unknown fields as suggested by reviewers. +- // eslint-disable-next-line no-type-assertion/no-type-assertion +- const serverRecord = server as Record; +- // eslint-disable-next-line no-type-assertion/no-type-assertion + result[name] = { +- ...serverRecord, + ...parsed, +- type: parsed.type ?? "stdio", ++ type: "stdio", + command, + ...(args && { args }), +- } as McpServer & Record; ++ }; + } + + return result; +@@ -81,6 +74,11 @@ function removeTypeField(config: CopilotcliMcpConfig): McpServers { + const result: McpServers = {}; + + for (const [name, server] of Object.entries(config.mcpServers ?? {})) { ++ if (server.type !== "stdio") { ++ result[name] = server; ++ continue; ++ } ++ + const { type: _, ...rest } = server; + result[name] = rest; + } +@@ -109,13 +107,7 @@ export class CopilotcliMcp extends ToolMcp { + return !this.global; + } + +- static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { +- if (global) { +- return { +- relativeDirPath: ".copilot", +- relativeFilePath: "mcp-config.json", +- }; +- } ++ static getSettablePaths({ global: _global }: { global?: boolean } = {}): ToolMcpSettablePaths { + return { + relativeDirPath: ".copilot", + relativeFilePath: "mcp-config.json", +diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts +new file mode 100644 +index 00000000..8575be8d +--- /dev/null ++++ b/src/features/rules/copilotcli-rule.ts +@@ -0,0 +1,41 @@ ++import { ToolTarget } from "../../types/tool-targets.js"; ++import { CopilotRule } from "./copilot-rule.js"; ++import { RulesyncRule } from "./rulesync-rule.js"; ++import { ++ ToolRuleForDeletionParams, ++ ToolRuleFromFileParams, ++ ToolRuleFromRulesyncRuleParams, ++} from "./tool-rule.js"; ++ ++export class CopilotcliRule extends CopilotRule { ++ private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { ++ return new CopilotcliRule({ ++ baseDir: copilotRule.getBaseDir(), ++ relativeDirPath: copilotRule.getRelativeDirPath(), ++ relativeFilePath: copilotRule.getRelativeFilePath(), ++ frontmatter: copilotRule.getFrontmatter(), ++ body: copilotRule.getBody(), ++ validate: true, ++ root: copilotRule.isRoot(), ++ }); ++ } ++ ++ static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { ++ return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); ++ } ++ ++ static override async fromFile(params: ToolRuleFromFileParams): Promise { ++ return this.fromCopilotRule(await CopilotRule.fromFile(params)); ++ } ++ ++ static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { ++ return this.fromCopilotRule(CopilotRule.forDeletion(params)); ++ } ++ ++ static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { ++ return this.isTargetedByRulesyncRuleDefault({ ++ rulesyncRule, ++ toolTarget: "copilotcli" satisfies ToolTarget, ++ }); ++ } ++} +diff --git a/src/features/rules/rules-processor.test.ts b/src/features/rules/rules-processor.test.ts +index 60e3da0d..cbab91f5 100644 +--- a/src/features/rules/rules-processor.test.ts ++++ b/src/features/rules/rules-processor.test.ts +@@ -11,6 +11,7 @@ import { AugmentcodeLegacyRule } from "./augmentcode-legacy-rule.js"; + import { ClaudecodeLegacyRule } from "./claudecode-legacy-rule.js"; + import { ClaudecodeRule } from "./claudecode-rule.js"; + import { CopilotRule } from "./copilot-rule.js"; ++import { CopilotcliRule } from "./copilotcli-rule.js"; + import { CursorRule } from "./cursor-rule.js"; + import { OpenCodeRule } from "./opencode-rule.js"; + import { RovodevRule } from "./rovodev-rule.js"; +@@ -186,6 +187,7 @@ describe("RulesProcessor", () => { + it("should correctly validate and filter rules for each supported tool", async () => { + const testCases = [ + { toolTarget: "copilot" as const, ruleClass: CopilotRule }, ++ { toolTarget: "copilotcli" as const, ruleClass: CopilotcliRule }, + { toolTarget: "cursor" as const, ruleClass: CursorRule }, + { toolTarget: "claudecode" as const, ruleClass: ClaudecodeRule }, + { toolTarget: "warp" as const, ruleClass: WarpRule }, +@@ -795,6 +797,7 @@ Content that would fail parsing`; + "claudecode-legacy", + "codexcli", + "copilot", ++ "copilotcli", + "factorydroid", + "geminicli", + "goose", +@@ -825,13 +828,14 @@ Content that would fail parsing`; + expect(globalTargets).toContain("claudecode-legacy"); + expect(globalTargets).toContain("codexcli"); + expect(globalTargets).toContain("copilot"); ++ expect(globalTargets).toContain("copilotcli"); + expect(globalTargets).toContain("factorydroid"); + expect(globalTargets).toContain("geminicli"); + expect(globalTargets).toContain("kilo"); + expect(globalTargets).toContain("goose"); + expect(globalTargets).toContain("opencode"); + expect(globalTargets).toContain("rovodev"); +- expect(globalTargets.length).toBe(10); ++ expect(globalTargets.length).toBe(11); + + // These targets should NOT be in global mode + expect(globalTargets).not.toContain("cursor"); +diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts +index 2f64feb3..2886c85c 100644 +--- a/src/features/rules/rules-processor.ts ++++ b/src/features/rules/rules-processor.ts +@@ -39,6 +39,7 @@ import { ClaudecodeRule } from "./claudecode-rule.js"; + import { ClineRule } from "./cline-rule.js"; + import { CodexcliRule } from "./codexcli-rule.js"; + import { CopilotRule } from "./copilot-rule.js"; ++import { CopilotcliRule } from "./copilotcli-rule.js"; + import { CursorRule } from "./cursor-rule.js"; + import { DeepagentsRule } from "./deepagents-rule.js"; + import { FactorydroidRule } from "./factorydroid-rule.js"; +@@ -74,6 +75,7 @@ const rulesProcessorToolTargets: ToolTarget[] = [ + "cline", + "codexcli", + "copilot", ++ "copilotcli", + "cursor", + "deepagents", + "factorydroid", +@@ -290,6 +292,17 @@ const toolRuleFactories = new Map([ + }, + }, + ], ++ [ ++ "copilotcli", ++ { ++ class: CopilotcliRule, ++ meta: { ++ extension: "md", ++ supportsGlobal: true, ++ ruleDiscoveryMode: "auto", ++ }, ++ }, ++ ], + [ + "cursor", + { diff --git a/.pr-1410.diff b/.pr-1410.diff new file mode 100644 index 000000000..580b8620f --- /dev/null +++ b/.pr-1410.diff @@ -0,0 +1,361 @@ +diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts +index 93ef81703..ef55f7768 100644 +--- a/src/features/mcp/copilotcli-mcp.test.ts ++++ b/src/features/mcp/copilotcli-mcp.test.ts +@@ -364,6 +364,29 @@ describe("CopilotcliMcp", () => { + ).rejects.toThrow('MCP server "no-command-server" is missing a command'); + }); + ++ it("should throw error when stdio server has unknown fields but no command", async () => { ++ const inputMcpServers = { ++ "unknown-fields-no-command": { ++ url: "http://localhost:3000/mcp", ++ headers: { ++ Authorization: "Bearer test-token", ++ }, ++ unknown_field: "value", ++ }, ++ }; ++ const rulesyncMcp = new RulesyncMcp({ ++ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, ++ relativeFilePath: "mcp.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ await expect( ++ CopilotcliMcp.fromRulesyncMcp({ ++ rulesyncMcp, ++ }), ++ ).rejects.toThrow('MCP server "unknown-fields-no-command" is missing a command'); ++ }); ++ + it("should handle command as array and merge remaining elements into args", async () => { + const inputMcpServers = { + "array-command-server": { +@@ -391,6 +414,63 @@ describe("CopilotcliMcp", () => { + }, + }); + }); ++ ++ it("should preserve http and sse servers without requiring command", async () => { ++ const inputMcpServers = { ++ "http-server": { ++ type: "http" as const, ++ url: "http://localhost:3000/mcp", ++ headers: { ++ Authorization: "Bearer token", ++ }, ++ tools: ["search"], ++ }, ++ "sse-server": { ++ type: "sse" as const, ++ url: "http://localhost:4000/sse", ++ headers: { ++ "X-Test": "true", ++ }, ++ }, ++ }; ++ const rulesyncMcp = new RulesyncMcp({ ++ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, ++ relativeFilePath: "mcp.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ ++ rulesyncMcp, ++ }); ++ ++ expect(copilotCliMcp.getJson()).toEqual({ ++ mcpServers: inputMcpServers, ++ }); ++ }); ++ ++ it("should preserve existing non-stdio type when converting", async () => { ++ const inputMcpServers = { ++ "typed-server": { ++ type: "http" as const, ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }; ++ const rulesyncMcp = new RulesyncMcp({ ++ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, ++ relativeFilePath: "mcp.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ ++ rulesyncMcp, ++ }); ++ ++ expect(copilotCliMcp.getJson()).toEqual({ ++ mcpServers: inputMcpServers, ++ }); ++ }); + }); + + describe("toRulesyncMcp", () => { +@@ -500,6 +580,36 @@ describe("CopilotcliMcp", () => { + $schema: RULESYNC_MCP_SCHEMA_URL, + }); + }); ++ ++ it("should preserve non-stdio type when converting back to RulesyncMcp", () => { ++ const inputMcpServers = { ++ "http-server": { ++ type: "http" as const, ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }; ++ const copilotCliMcp = new CopilotcliMcp({ ++ relativeDirPath: ".copilot", ++ relativeFilePath: "mcp-config.json", ++ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), ++ }); ++ ++ const rulesyncMcp = copilotCliMcp.toRulesyncMcp(); ++ ++ expect(rulesyncMcp.getJson()).toEqual({ ++ mcpServers: { ++ "http-server": { ++ type: "http", ++ command: "node", ++ args: ["server.js"], ++ url: "http://localhost:3000/mcp", ++ }, ++ }, ++ $schema: RULESYNC_MCP_SCHEMA_URL, ++ }); ++ }); + }); + + describe("validate", () => { +diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts +index ff9753db8..a448fd96c 100644 +--- a/src/features/mcp/copilotcli-mcp.ts ++++ b/src/features/mcp/copilotcli-mcp.ts +@@ -22,23 +22,29 @@ type CopilotcliMcpConfig = { + /** + * Adds "type": "stdio" to each MCP server config if not present. + * GitHub Copilot CLI requires the "type" field for each server. +- * @throws Error if a server doesn't have a command (Copilot CLI stdio servers require a command) ++ * @throws Error if a stdio server doesn't have a command + */ + function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] { + const result: NonNullable = {}; + + for (const [name, server] of Object.entries(mcpServers)) { +- // Parse and validate the server config + const parsed = McpServerSchema.parse(server); ++ const type = parsed.type ?? "stdio"; ++ ++ if (type !== "stdio") { ++ result[name] = { ++ ...parsed, ++ type, ++ }; ++ continue; ++ } + +- // Copilot CLI stdio servers require a non-empty command + if (!parsed.command) { + throw new Error( + `MCP server "${name}" is missing a command. GitHub Copilot CLI stdio servers require a non-empty command.`, + ); + } + +- // Handle command as string or array + let command: string; + let args: string[] | undefined; + +@@ -46,29 +52,20 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] + command = parsed.command; + args = parsed.args; + } else { +- // command is an array: first element is command, rest are args + const [cmd, ...cmdArgs] = parsed.command; + if (!cmd) { + throw new Error(`MCP server "${name}" has an empty command array.`); + } + command = cmd; +- // Merge command array args with existing args + args = cmdArgs.length > 0 ? [...cmdArgs, ...(parsed.args ?? [])] : parsed.args; + } + +- // Use the parsed object for the base, then override with normalized command/args +- // and ensure type is set to "stdio" if not present. +- // We spread server as well to keep unknown fields as suggested by reviewers. +- // eslint-disable-next-line no-type-assertion/no-type-assertion +- const serverRecord = server as Record; +- // eslint-disable-next-line no-type-assertion/no-type-assertion + result[name] = { +- ...serverRecord, + ...parsed, +- type: parsed.type ?? "stdio", ++ type, + command, + ...(args && { args }), +- } as McpServer & Record; ++ }; + } + + return result; +@@ -81,6 +78,11 @@ function removeTypeField(config: CopilotcliMcpConfig): McpServers { + const result: McpServers = {}; + + for (const [name, server] of Object.entries(config.mcpServers ?? {})) { ++ if (server.type !== "stdio") { ++ result[name] = server; ++ continue; ++ } ++ + const { type: _, ...rest } = server; + result[name] = rest; + } +@@ -109,13 +111,7 @@ export class CopilotcliMcp extends ToolMcp { + return !this.global; + } + +- static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { +- if (global) { +- return { +- relativeDirPath: ".copilot", +- relativeFilePath: "mcp-config.json", +- }; +- } ++ static getSettablePaths(_options: { global?: boolean } = {}): ToolMcpSettablePaths { + return { + relativeDirPath: ".copilot", + relativeFilePath: "mcp-config.json", +diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts +new file mode 100644 +index 000000000..8575be8d3 +--- /dev/null ++++ b/src/features/rules/copilotcli-rule.ts +@@ -0,0 +1,41 @@ ++import { ToolTarget } from "../../types/tool-targets.js"; ++import { CopilotRule } from "./copilot-rule.js"; ++import { RulesyncRule } from "./rulesync-rule.js"; ++import { ++ ToolRuleForDeletionParams, ++ ToolRuleFromFileParams, ++ ToolRuleFromRulesyncRuleParams, ++} from "./tool-rule.js"; ++ ++export class CopilotcliRule extends CopilotRule { ++ private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { ++ return new CopilotcliRule({ ++ baseDir: copilotRule.getBaseDir(), ++ relativeDirPath: copilotRule.getRelativeDirPath(), ++ relativeFilePath: copilotRule.getRelativeFilePath(), ++ frontmatter: copilotRule.getFrontmatter(), ++ body: copilotRule.getBody(), ++ validate: true, ++ root: copilotRule.isRoot(), ++ }); ++ } ++ ++ static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { ++ return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); ++ } ++ ++ static override async fromFile(params: ToolRuleFromFileParams): Promise { ++ return this.fromCopilotRule(await CopilotRule.fromFile(params)); ++ } ++ ++ static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { ++ return this.fromCopilotRule(CopilotRule.forDeletion(params)); ++ } ++ ++ static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { ++ return this.isTargetedByRulesyncRuleDefault({ ++ rulesyncRule, ++ toolTarget: "copilotcli" satisfies ToolTarget, ++ }); ++ } ++} +diff --git a/src/features/rules/rules-processor.test.ts b/src/features/rules/rules-processor.test.ts +index 60e3da0d5..cbab91f5d 100644 +--- a/src/features/rules/rules-processor.test.ts ++++ b/src/features/rules/rules-processor.test.ts +@@ -11,6 +11,7 @@ import { AugmentcodeLegacyRule } from "./augmentcode-legacy-rule.js"; + import { ClaudecodeLegacyRule } from "./claudecode-legacy-rule.js"; + import { ClaudecodeRule } from "./claudecode-rule.js"; + import { CopilotRule } from "./copilot-rule.js"; ++import { CopilotcliRule } from "./copilotcli-rule.js"; + import { CursorRule } from "./cursor-rule.js"; + import { OpenCodeRule } from "./opencode-rule.js"; + import { RovodevRule } from "./rovodev-rule.js"; +@@ -186,6 +187,7 @@ describe("RulesProcessor", () => { + it("should correctly validate and filter rules for each supported tool", async () => { + const testCases = [ + { toolTarget: "copilot" as const, ruleClass: CopilotRule }, ++ { toolTarget: "copilotcli" as const, ruleClass: CopilotcliRule }, + { toolTarget: "cursor" as const, ruleClass: CursorRule }, + { toolTarget: "claudecode" as const, ruleClass: ClaudecodeRule }, + { toolTarget: "warp" as const, ruleClass: WarpRule }, +@@ -795,6 +797,7 @@ Content that would fail parsing`; + "claudecode-legacy", + "codexcli", + "copilot", ++ "copilotcli", + "factorydroid", + "geminicli", + "goose", +@@ -825,13 +828,14 @@ Content that would fail parsing`; + expect(globalTargets).toContain("claudecode-legacy"); + expect(globalTargets).toContain("codexcli"); + expect(globalTargets).toContain("copilot"); ++ expect(globalTargets).toContain("copilotcli"); + expect(globalTargets).toContain("factorydroid"); + expect(globalTargets).toContain("geminicli"); + expect(globalTargets).toContain("kilo"); + expect(globalTargets).toContain("goose"); + expect(globalTargets).toContain("opencode"); + expect(globalTargets).toContain("rovodev"); +- expect(globalTargets.length).toBe(10); ++ expect(globalTargets.length).toBe(11); + + // These targets should NOT be in global mode + expect(globalTargets).not.toContain("cursor"); +diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts +index 2f64feb30..2886c85c1 100644 +--- a/src/features/rules/rules-processor.ts ++++ b/src/features/rules/rules-processor.ts +@@ -39,6 +39,7 @@ import { ClaudecodeRule } from "./claudecode-rule.js"; + import { ClineRule } from "./cline-rule.js"; + import { CodexcliRule } from "./codexcli-rule.js"; + import { CopilotRule } from "./copilot-rule.js"; ++import { CopilotcliRule } from "./copilotcli-rule.js"; + import { CursorRule } from "./cursor-rule.js"; + import { DeepagentsRule } from "./deepagents-rule.js"; + import { FactorydroidRule } from "./factorydroid-rule.js"; +@@ -74,6 +75,7 @@ const rulesProcessorToolTargets: ToolTarget[] = [ + "cline", + "codexcli", + "copilot", ++ "copilotcli", + "cursor", + "deepagents", + "factorydroid", +@@ -290,6 +292,17 @@ const toolRuleFactories = new Map([ + }, + }, + ], ++ [ ++ "copilotcli", ++ { ++ class: CopilotcliRule, ++ meta: { ++ extension: "md", ++ supportsGlobal: true, ++ ruleDiscoveryMode: "auto", ++ }, ++ }, ++ ], + [ + "cursor", + { diff --git a/src/cli/commands/gitignore-entries.test.ts b/src/cli/commands/gitignore-entries.test.ts index 1ef4833fa..d03ed118d 100644 --- a/src/cli/commands/gitignore-entries.test.ts +++ b/src/cli/commands/gitignore-entries.test.ts @@ -30,7 +30,11 @@ describe("GITIGNORE_ENTRY_REGISTRY", () => { }); it("should cover all tool targets except intentionally excluded ones", () => { - const registeredTargets = new Set(GITIGNORE_ENTRY_REGISTRY.map((tag) => tag.target)); + const registeredTargets = new Set( + GITIGNORE_ENTRY_REGISTRY.flatMap((tag) => + Array.isArray(tag.target) ? tag.target : [tag.target], + ), + ); for (const target of ALL_TOOL_TARGETS) { if (TARGETS_WITHOUT_GITIGNORE_ENTRIES.has(target)) { expect(registeredTargets).not.toContain(target); @@ -89,6 +93,15 @@ describe("filterGitignoreEntries", () => { expect(result).not.toContain("**/.cursor/"); }); + it("should include shared copilot rule entries for copilotcli target", () => { + const result = filterGitignoreEntries({ logger, targets: ["copilotcli"] }); + + expect(result).toContain("**/.github/copilot-instructions.md"); + expect(result).toContain("**/.github/instructions/"); + expect(result).toContain("**/.copilot/mcp-config.json"); + expect(result).not.toContain("**/.github/prompts/"); + }); + it("should return all entries when target is wildcard", () => { const result = filterGitignoreEntries({ logger, targets: ["*"] }); expect(result).toEqual([...ALL_GITIGNORE_ENTRIES]); @@ -189,8 +202,9 @@ describe("filterGitignoreEntries", () => { // copilot commands expect(result).toContain("**/.github/prompts/"); - // copilot has ["commands"] so copilot rules should NOT be included - expect(result).not.toContain("**/.github/copilot-instructions.md"); + // Shared copilot/copilotcli rule entries stay included because copilotcli + // is not restricted in this per-target feature map. + expect(result).toContain("**/.github/copilot-instructions.md"); // claudecode commands should NOT be included expect(result).not.toContain("**/.claude/commands/"); @@ -233,6 +247,19 @@ describe("filterGitignoreEntries", () => { expect(result).not.toContain("**/.github/copilot-instructions.md"); expect(result).not.toContain("**/.cursor/"); }); + + it("should include shared entries when copilotcli enables matching features", () => { + const result = filterGitignoreEntries({ + features: { + copilot: ["commands"], + copilotcli: ["rules"], + }, + }); + + expect(result).toContain("**/.github/copilot-instructions.md"); + expect(result).toContain("**/.github/instructions/"); + expect(result).toContain("**/.github/prompts/"); + }); }); describe("validation warnings", () => { diff --git a/src/cli/commands/gitignore-entries.ts b/src/cli/commands/gitignore-entries.ts index 65df1d737..6296d5244 100644 --- a/src/cli/commands/gitignore-entries.ts +++ b/src/cli/commands/gitignore-entries.ts @@ -7,12 +7,20 @@ import { import { ALL_TOOL_TARGETS_WITH_WILDCARD, type ToolTarget } from "../../types/tool-targets.js"; import type { Logger } from "../../utils/logger.js"; +type GitignoreEntryTarget = ToolTarget | "common"; + export type GitignoreEntryTag = { - readonly target: ToolTarget | "common"; + readonly target: GitignoreEntryTarget | ReadonlyArray; readonly feature: Feature | "general"; readonly entry: string; }; +const normalizeGitignoreEntryTargets = ( + target: GitignoreEntryTag["target"], +): ReadonlyArray => { + return typeof target === "string" ? [target] : target; +}; + export const GITIGNORE_ENTRY_REGISTRY: ReadonlyArray = [ // Common / general { @@ -126,11 +134,15 @@ export const GITIGNORE_ENTRY_REGISTRY: ReadonlyArray = [ // GitHub Copilot { - target: "copilot", + target: ["copilot", "copilotcli"], feature: "rules", entry: "**/.github/copilot-instructions.md", }, - { target: "copilot", feature: "rules", entry: "**/.github/instructions/" }, + { + target: ["copilot", "copilotcli"], + feature: "rules", + entry: "**/.github/instructions/", + }, { target: "copilot", feature: "commands", entry: "**/.github/prompts/" }, { target: "copilot", feature: "subagents", entry: "**/.github/agents/" }, { target: "copilot", feature: "skills", entry: "**/.github/skills/" }, @@ -216,16 +228,32 @@ type FilterGitignoreEntriesParams = { }; const isTargetSelected = ( - target: ToolTarget | "common", + target: GitignoreEntryTag["target"], selectedTargets: ReadonlyArray | undefined, ): boolean => { - if (target === "common") return true; + const targets = normalizeGitignoreEntryTargets(target); + + if (targets.includes("common")) return true; if (!selectedTargets || selectedTargets.length === 0) return true; if (selectedTargets.includes("*")) return true; - return selectedTargets.includes(target); + return targets.some((candidate) => selectedTargets.includes(candidate)); }; -const isFeatureSelected = ( +const getSelectedGitignoreEntryTargets = ( + target: GitignoreEntryTag["target"], + selectedTargets: ReadonlyArray | undefined, +): ReadonlyArray => { + const targets = normalizeGitignoreEntryTargets(target); + + if (targets.includes("common")) return ["common"]; + if (!selectedTargets || selectedTargets.length === 0 || selectedTargets.includes("*")) { + return targets; + } + + return targets.filter((candidate) => selectedTargets.includes(candidate)); +}; + +const isFeatureSelectedForTarget = ( feature: Feature | "general", target: ToolTarget | "common", features: RulesyncFeatures | undefined, @@ -251,6 +279,16 @@ const isFeatureSelected = ( return targetFeatures.includes(feature); }; +const isFeatureSelected = ( + feature: Feature | "general", + target: GitignoreEntryTag["target"], + features: RulesyncFeatures | undefined, +): boolean => { + return normalizeGitignoreEntryTargets(target).some((candidate) => + isFeatureSelectedForTarget(feature, candidate, features), + ); +}; + const warnInvalidTargets = (targets: ReadonlyArray, logger?: Logger): void => { const validTargets = new Set(ALL_TOOL_TARGETS_WITH_WILDCARD); for (const target of targets) { @@ -304,7 +342,8 @@ export const filterGitignoreEntries = ( for (const tag of GITIGNORE_ENTRY_REGISTRY) { if (!isTargetSelected(tag.target, targets)) continue; - if (!isFeatureSelected(tag.feature, tag.target, features)) continue; + const selectedTagTargets = getSelectedGitignoreEntryTargets(tag.target, targets); + if (!isFeatureSelected(tag.feature, selectedTagTargets, features)) continue; if (seen.has(tag.entry)) continue; seen.add(tag.entry); result.push(tag.entry); diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts index ef55f7768..6a6155276 100644 --- a/src/features/mcp/copilotcli-mcp.test.ts +++ b/src/features/mcp/copilotcli-mcp.test.ts @@ -448,6 +448,67 @@ describe("CopilotcliMcp", () => { }); }); + it("should preserve transport-based remote servers and add type field", async () => { + const inputMcpServers = { + "http-server": { + transport: "http" as const, + url: "http://localhost:3000/mcp", + headers: { + Authorization: "Bearer token", + }, + }, + "sse-server": { + transport: "sse" as const, + url: "http://localhost:4000/sse", + headers: { + "X-Test": "true", + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }); + + expect(copilotCliMcp.getJson()).toEqual({ + mcpServers: { + "http-server": { + type: "http", + ...inputMcpServers["http-server"], + }, + "sse-server": { + type: "sse", + ...inputMcpServers["sse-server"], + }, + }, + }); + }); + + it("should require command for local type servers", async () => { + const inputMcpServers = { + "local-server": { + type: "local" as const, + cwd: testDir, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + await expect( + CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }), + ).rejects.toThrow('MCP server "local-server" is missing a command'); + }); + it("should preserve existing non-stdio type when converting", async () => { const inputMcpServers = { "typed-server": { diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts index a448fd96c..0b1cb0feb 100644 --- a/src/features/mcp/copilotcli-mcp.ts +++ b/src/features/mcp/copilotcli-mcp.ts @@ -19,6 +19,26 @@ type CopilotcliMcpConfig = { mcpServers?: Record>; }; +type CopilotcliServerType = NonNullable; + +const isRemoteServerType = ( + type: CopilotcliServerType, +): type is Extract => { + return type === "http" || type === "sse"; +}; + +const resolveCopilotcliServerType = (server: McpServer): CopilotcliServerType => { + if (server.type) { + return server.type; + } + + if (server.transport === "http" || server.transport === "sse") { + return server.transport; + } + + return "stdio"; +}; + /** * Adds "type": "stdio" to each MCP server config if not present. * GitHub Copilot CLI requires the "type" field for each server. @@ -29,9 +49,9 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] for (const [name, server] of Object.entries(mcpServers)) { const parsed = McpServerSchema.parse(server); - const type = parsed.type ?? "stdio"; + const type = resolveCopilotcliServerType(parsed); - if (type !== "stdio") { + if (isRemoteServerType(type)) { result[name] = { ...parsed, type, diff --git a/src/types/mcp.ts b/src/types/mcp.ts index c28261dba..17a8d7aec 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -1,7 +1,7 @@ import { z } from "zod/mini"; export const McpServerSchema = z.looseObject({ - type: z.optional(z.enum(["stdio", "sse", "http"])), + type: z.optional(z.enum(["local", "stdio", "sse", "http"])), command: z.optional(z.union([z.string(), z.array(z.string())])), args: z.optional(z.array(z.string())), url: z.optional(z.string()), From 1eda18f91758479593e24488d36fd2b24f9a1ee9 Mon Sep 17 00:00:00 2001 From: dyoshikawa-claw Date: Tue, 31 Mar 2026 20:55:05 +0900 Subject: [PATCH 3/6] fix: align copilotcli rules support and docs --- .gitignore | 3 + .issue-1401.md | 53 --- .pr-1408.diff | 307 ------------------ .pr-1410.diff | 361 --------------------- README.md | 2 +- docs/reference/file-formats.md | 11 +- docs/reference/supported-tools.md | 2 +- skills/rulesync/file-formats.md | 11 +- skills/rulesync/supported-tools.md | 2 +- src/features/rules/copilotcli-rule.test.ts | 63 ++++ src/features/rules/copilotcli-rule.ts | 20 +- 11 files changed, 102 insertions(+), 733 deletions(-) delete mode 100644 .issue-1401.md delete mode 100644 .pr-1408.diff delete mode 100644 .pr-1410.diff create mode 100644 src/features/rules/copilotcli-rule.test.ts diff --git a/.gitignore b/.gitignore index e29b8b9e3..584d27f62 100644 --- a/.gitignore +++ b/.gitignore @@ -169,6 +169,9 @@ package-lock.json .pnpm-store +/.issue-*.md +/.pr-*.diff + .rulesync/rules/my-instructions.md .cspellcache diff --git a/.issue-1401.md b/.issue-1401.md deleted file mode 100644 index e9a295124..000000000 --- a/.issue-1401.md +++ /dev/null @@ -1,53 +0,0 @@ -## Background - -PR #1376 adds GitHub Copilot CLI as a new MCP sync target (`copilotcli`), generating `.copilot/mcp-config.json`. A code review and security review were conducted, revealing several findings ranging from mid to low severity. This issue consolidates those findings for tracking and follow-up. - -## Details - -### Code Review Findings - -**#1 (mid) — Double type assertion and redundant spread in `addTypeField`** -In `copilotcli-mcp.ts`, `addTypeField` spreads both `server` (raw input) and `parsed` (Zod-validated output). This is redundant since `McpServerSchema` uses `z.looseObject()` and `parsed` already preserves unknown fields. The double `eslint-disable` and `as` casts weaken type safety unnecessarily. - -**#2 (mid) — `parsed.type ?? "stdio"` contradicts function contract** -The function name and JSDoc state it adds `"type": "stdio"`, but the new code preserves whatever `type` the input already has. The original code unconditionally set `type: "stdio"`. This is a behavioral regression — if Copilot CLI only supports stdio, the original hard-coded approach was correct. If other types are supported, the `command` validation should be conditional on the type. - -**#3 (mid) — Missing `rules-processor.ts` integration check** -Per `feature-change-guidelines.md`, new tool targets should be checked against `rules-processor.ts` and `additionalConvention`. The PR diff does not show changes to `rules-processor.ts`. - -**#4 (low) — Dead code in `getSettablePaths`** -Both the `global` and non-global branches return identical values (`{ relativeDirPath: ".copilot", relativeFilePath: "mcp-config.json" }`). The `if (global)` branch is dead code. - -**#5 (low) — `removeTypeField` unconditionally strips `type`** -If non-stdio types are now preserved by `addTypeField` (per #2), `removeTypeField` would lose that information when converting back to rulesync format. - -**#6 (low) — Documentation section placement in `file-formats.md`** -The new `.copilot/mcp-config.json` section is inserted before the general "optional override keys" paragraph about hooks, which may cause confusion. - -**#7 (low) — PR description test counts are misleading** -The counts (30 + 45 = 74) refer to total tests in each file, not newly added tests. - -**#8 (low) — Missing negative test for command-less servers with unknown fields** -No test verifies that a server with unknown fields but missing `command` still throws an error correctly. - -### Security Review Findings - -**#9 (low) — Theoretical prototype pollution risk in object spread** -Spreading raw `serverRecord` before Zod-validated `parsed` has a theoretical prototype pollution concern. In practice, JS object literal spread does not trigger this, and `parsed` overrides critical fields. Actual risk is minimal. - -**#10 (low) — Type assertion bypasses defense-in-depth** -The `as` casts suppress TypeScript type checking in the data transformation code, weakening a defense-in-depth layer. - -**#11 (low) — No sanitization of `command`/`args` fields** -Expected behavior for a config file generator operating on user-controlled local files. Consistent with all other tool targets in the codebase. - -**No malicious code, backdoors, obfuscated payloads, or suspicious dependency additions were detected.** - -## Solution / Next Steps - -1. **Address #2 first (highest impact):** Decide whether Copilot CLI supports only stdio or multiple transport types. If stdio-only, revert to hard-coded `type: "stdio"`. If multiple types, make `command` validation conditional on `type`. -2. **Simplify #1:** Spread only `parsed` (not raw `server`) to eliminate redundant type assertions. -3. **Verify #3:** Check `rules-processor.ts` and `additionalConvention` for the new `copilotcli` target. -4. **Clean up #4:** Remove the dead `if (global)` branch or add a TODO explaining the placeholder. -5. **Add test for #8:** Add a negative test case for servers with unknown fields but no `command`. -6. **Fix #6:** Adjust section placement in `file-formats.md` to avoid confusion with the hooks paragraph. diff --git a/.pr-1408.diff b/.pr-1408.diff deleted file mode 100644 index da7cbef29..000000000 --- a/.pr-1408.diff +++ /dev/null @@ -1,307 +0,0 @@ -diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts -index 93ef8170..d5b956b9 100644 ---- a/src/features/mcp/copilotcli-mcp.test.ts -+++ b/src/features/mcp/copilotcli-mcp.test.ts -@@ -391,6 +391,60 @@ describe("CopilotcliMcp", () => { - }, - }); - }); -+ -+ it("should force stdio type even when another type is provided", async () => { -+ const inputMcpServers = { -+ "typed-server": { -+ type: "http" as const, -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }; -+ const rulesyncMcp = new RulesyncMcp({ -+ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, -+ relativeFilePath: "mcp.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ -+ rulesyncMcp, -+ }); -+ -+ expect(copilotCliMcp.getJson()).toEqual({ -+ mcpServers: { -+ "typed-server": { -+ type: "stdio", -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }, -+ }); -+ }); -+ -+ it("should throw error when server has unknown fields but no command", async () => { -+ const inputMcpServers = { -+ "unknown-fields-no-command": { -+ url: "http://localhost:3000/mcp", -+ headers: { -+ Authorization: "Bearer test-token", -+ }, -+ unknown_field: "value", -+ }, -+ }; -+ const rulesyncMcp = new RulesyncMcp({ -+ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, -+ relativeFilePath: "mcp.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ await expect( -+ CopilotcliMcp.fromRulesyncMcp({ -+ rulesyncMcp, -+ }), -+ ).rejects.toThrow('MCP server "unknown-fields-no-command" is missing a command'); -+ }); - }); - - describe("toRulesyncMcp", () => { -@@ -500,6 +554,36 @@ describe("CopilotcliMcp", () => { - $schema: RULESYNC_MCP_SCHEMA_URL, - }); - }); -+ -+ it("should preserve non-stdio type when converting back to RulesyncMcp", () => { -+ const inputMcpServers = { -+ "http-server": { -+ type: "http" as const, -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }; -+ const copilotCliMcp = new CopilotcliMcp({ -+ relativeDirPath: ".copilot", -+ relativeFilePath: "mcp-config.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ const rulesyncMcp = copilotCliMcp.toRulesyncMcp(); -+ -+ expect(rulesyncMcp.getJson()).toEqual({ -+ mcpServers: { -+ "http-server": { -+ type: "http", -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }, -+ $schema: RULESYNC_MCP_SCHEMA_URL, -+ }); -+ }); - }); - - describe("validate", () => { -diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts -index ff9753db..26371028 100644 ---- a/src/features/mcp/copilotcli-mcp.ts -+++ b/src/features/mcp/copilotcli-mcp.ts -@@ -1,7 +1,7 @@ - import { join } from "node:path"; - - import { ValidationResult } from "../../types/ai-file.js"; --import { McpServerSchema, type McpServer, type McpServers } from "../../types/mcp.js"; -+import { McpServerSchema, type McpServers } from "../../types/mcp.js"; - import { readFileContentOrNull, readOrInitializeFileContent } from "../../utils/file.js"; - import { RulesyncMcp } from "./rulesync-mcp.js"; - import { -@@ -16,11 +16,11 @@ import { - export type CopilotcliMcpParams = ToolMcpParams; - - type CopilotcliMcpConfig = { -- mcpServers?: Record>; -+ mcpServers?: McpServers; - }; - - /** -- * Adds "type": "stdio" to each MCP server config if not present. -+ * Adds "type": "stdio" to each MCP server config. - * GitHub Copilot CLI requires the "type" field for each server. - * @throws Error if a server doesn't have a command (Copilot CLI stdio servers require a command) - */ -@@ -56,19 +56,12 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] - args = cmdArgs.length > 0 ? [...cmdArgs, ...(parsed.args ?? [])] : parsed.args; - } - -- // Use the parsed object for the base, then override with normalized command/args -- // and ensure type is set to "stdio" if not present. -- // We spread server as well to keep unknown fields as suggested by reviewers. -- // eslint-disable-next-line no-type-assertion/no-type-assertion -- const serverRecord = server as Record; -- // eslint-disable-next-line no-type-assertion/no-type-assertion - result[name] = { -- ...serverRecord, - ...parsed, -- type: parsed.type ?? "stdio", -+ type: "stdio", - command, - ...(args && { args }), -- } as McpServer & Record; -+ }; - } - - return result; -@@ -81,6 +74,11 @@ function removeTypeField(config: CopilotcliMcpConfig): McpServers { - const result: McpServers = {}; - - for (const [name, server] of Object.entries(config.mcpServers ?? {})) { -+ if (server.type !== "stdio") { -+ result[name] = server; -+ continue; -+ } -+ - const { type: _, ...rest } = server; - result[name] = rest; - } -@@ -109,13 +107,7 @@ export class CopilotcliMcp extends ToolMcp { - return !this.global; - } - -- static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { -- if (global) { -- return { -- relativeDirPath: ".copilot", -- relativeFilePath: "mcp-config.json", -- }; -- } -+ static getSettablePaths({ global: _global }: { global?: boolean } = {}): ToolMcpSettablePaths { - return { - relativeDirPath: ".copilot", - relativeFilePath: "mcp-config.json", -diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts -new file mode 100644 -index 00000000..8575be8d ---- /dev/null -+++ b/src/features/rules/copilotcli-rule.ts -@@ -0,0 +1,41 @@ -+import { ToolTarget } from "../../types/tool-targets.js"; -+import { CopilotRule } from "./copilot-rule.js"; -+import { RulesyncRule } from "./rulesync-rule.js"; -+import { -+ ToolRuleForDeletionParams, -+ ToolRuleFromFileParams, -+ ToolRuleFromRulesyncRuleParams, -+} from "./tool-rule.js"; -+ -+export class CopilotcliRule extends CopilotRule { -+ private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { -+ return new CopilotcliRule({ -+ baseDir: copilotRule.getBaseDir(), -+ relativeDirPath: copilotRule.getRelativeDirPath(), -+ relativeFilePath: copilotRule.getRelativeFilePath(), -+ frontmatter: copilotRule.getFrontmatter(), -+ body: copilotRule.getBody(), -+ validate: true, -+ root: copilotRule.isRoot(), -+ }); -+ } -+ -+ static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { -+ return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); -+ } -+ -+ static override async fromFile(params: ToolRuleFromFileParams): Promise { -+ return this.fromCopilotRule(await CopilotRule.fromFile(params)); -+ } -+ -+ static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { -+ return this.fromCopilotRule(CopilotRule.forDeletion(params)); -+ } -+ -+ static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { -+ return this.isTargetedByRulesyncRuleDefault({ -+ rulesyncRule, -+ toolTarget: "copilotcli" satisfies ToolTarget, -+ }); -+ } -+} -diff --git a/src/features/rules/rules-processor.test.ts b/src/features/rules/rules-processor.test.ts -index 60e3da0d..cbab91f5 100644 ---- a/src/features/rules/rules-processor.test.ts -+++ b/src/features/rules/rules-processor.test.ts -@@ -11,6 +11,7 @@ import { AugmentcodeLegacyRule } from "./augmentcode-legacy-rule.js"; - import { ClaudecodeLegacyRule } from "./claudecode-legacy-rule.js"; - import { ClaudecodeRule } from "./claudecode-rule.js"; - import { CopilotRule } from "./copilot-rule.js"; -+import { CopilotcliRule } from "./copilotcli-rule.js"; - import { CursorRule } from "./cursor-rule.js"; - import { OpenCodeRule } from "./opencode-rule.js"; - import { RovodevRule } from "./rovodev-rule.js"; -@@ -186,6 +187,7 @@ describe("RulesProcessor", () => { - it("should correctly validate and filter rules for each supported tool", async () => { - const testCases = [ - { toolTarget: "copilot" as const, ruleClass: CopilotRule }, -+ { toolTarget: "copilotcli" as const, ruleClass: CopilotcliRule }, - { toolTarget: "cursor" as const, ruleClass: CursorRule }, - { toolTarget: "claudecode" as const, ruleClass: ClaudecodeRule }, - { toolTarget: "warp" as const, ruleClass: WarpRule }, -@@ -795,6 +797,7 @@ Content that would fail parsing`; - "claudecode-legacy", - "codexcli", - "copilot", -+ "copilotcli", - "factorydroid", - "geminicli", - "goose", -@@ -825,13 +828,14 @@ Content that would fail parsing`; - expect(globalTargets).toContain("claudecode-legacy"); - expect(globalTargets).toContain("codexcli"); - expect(globalTargets).toContain("copilot"); -+ expect(globalTargets).toContain("copilotcli"); - expect(globalTargets).toContain("factorydroid"); - expect(globalTargets).toContain("geminicli"); - expect(globalTargets).toContain("kilo"); - expect(globalTargets).toContain("goose"); - expect(globalTargets).toContain("opencode"); - expect(globalTargets).toContain("rovodev"); -- expect(globalTargets.length).toBe(10); -+ expect(globalTargets.length).toBe(11); - - // These targets should NOT be in global mode - expect(globalTargets).not.toContain("cursor"); -diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts -index 2f64feb3..2886c85c 100644 ---- a/src/features/rules/rules-processor.ts -+++ b/src/features/rules/rules-processor.ts -@@ -39,6 +39,7 @@ import { ClaudecodeRule } from "./claudecode-rule.js"; - import { ClineRule } from "./cline-rule.js"; - import { CodexcliRule } from "./codexcli-rule.js"; - import { CopilotRule } from "./copilot-rule.js"; -+import { CopilotcliRule } from "./copilotcli-rule.js"; - import { CursorRule } from "./cursor-rule.js"; - import { DeepagentsRule } from "./deepagents-rule.js"; - import { FactorydroidRule } from "./factorydroid-rule.js"; -@@ -74,6 +75,7 @@ const rulesProcessorToolTargets: ToolTarget[] = [ - "cline", - "codexcli", - "copilot", -+ "copilotcli", - "cursor", - "deepagents", - "factorydroid", -@@ -290,6 +292,17 @@ const toolRuleFactories = new Map([ - }, - }, - ], -+ [ -+ "copilotcli", -+ { -+ class: CopilotcliRule, -+ meta: { -+ extension: "md", -+ supportsGlobal: true, -+ ruleDiscoveryMode: "auto", -+ }, -+ }, -+ ], - [ - "cursor", - { diff --git a/.pr-1410.diff b/.pr-1410.diff deleted file mode 100644 index 580b8620f..000000000 --- a/.pr-1410.diff +++ /dev/null @@ -1,361 +0,0 @@ -diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts -index 93ef81703..ef55f7768 100644 ---- a/src/features/mcp/copilotcli-mcp.test.ts -+++ b/src/features/mcp/copilotcli-mcp.test.ts -@@ -364,6 +364,29 @@ describe("CopilotcliMcp", () => { - ).rejects.toThrow('MCP server "no-command-server" is missing a command'); - }); - -+ it("should throw error when stdio server has unknown fields but no command", async () => { -+ const inputMcpServers = { -+ "unknown-fields-no-command": { -+ url: "http://localhost:3000/mcp", -+ headers: { -+ Authorization: "Bearer test-token", -+ }, -+ unknown_field: "value", -+ }, -+ }; -+ const rulesyncMcp = new RulesyncMcp({ -+ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, -+ relativeFilePath: "mcp.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ await expect( -+ CopilotcliMcp.fromRulesyncMcp({ -+ rulesyncMcp, -+ }), -+ ).rejects.toThrow('MCP server "unknown-fields-no-command" is missing a command'); -+ }); -+ - it("should handle command as array and merge remaining elements into args", async () => { - const inputMcpServers = { - "array-command-server": { -@@ -391,6 +414,63 @@ describe("CopilotcliMcp", () => { - }, - }); - }); -+ -+ it("should preserve http and sse servers without requiring command", async () => { -+ const inputMcpServers = { -+ "http-server": { -+ type: "http" as const, -+ url: "http://localhost:3000/mcp", -+ headers: { -+ Authorization: "Bearer token", -+ }, -+ tools: ["search"], -+ }, -+ "sse-server": { -+ type: "sse" as const, -+ url: "http://localhost:4000/sse", -+ headers: { -+ "X-Test": "true", -+ }, -+ }, -+ }; -+ const rulesyncMcp = new RulesyncMcp({ -+ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, -+ relativeFilePath: "mcp.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ -+ rulesyncMcp, -+ }); -+ -+ expect(copilotCliMcp.getJson()).toEqual({ -+ mcpServers: inputMcpServers, -+ }); -+ }); -+ -+ it("should preserve existing non-stdio type when converting", async () => { -+ const inputMcpServers = { -+ "typed-server": { -+ type: "http" as const, -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }; -+ const rulesyncMcp = new RulesyncMcp({ -+ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, -+ relativeFilePath: "mcp.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ const copilotCliMcp = await CopilotcliMcp.fromRulesyncMcp({ -+ rulesyncMcp, -+ }); -+ -+ expect(copilotCliMcp.getJson()).toEqual({ -+ mcpServers: inputMcpServers, -+ }); -+ }); - }); - - describe("toRulesyncMcp", () => { -@@ -500,6 +580,36 @@ describe("CopilotcliMcp", () => { - $schema: RULESYNC_MCP_SCHEMA_URL, - }); - }); -+ -+ it("should preserve non-stdio type when converting back to RulesyncMcp", () => { -+ const inputMcpServers = { -+ "http-server": { -+ type: "http" as const, -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }; -+ const copilotCliMcp = new CopilotcliMcp({ -+ relativeDirPath: ".copilot", -+ relativeFilePath: "mcp-config.json", -+ fileContent: JSON.stringify({ mcpServers: inputMcpServers }), -+ }); -+ -+ const rulesyncMcp = copilotCliMcp.toRulesyncMcp(); -+ -+ expect(rulesyncMcp.getJson()).toEqual({ -+ mcpServers: { -+ "http-server": { -+ type: "http", -+ command: "node", -+ args: ["server.js"], -+ url: "http://localhost:3000/mcp", -+ }, -+ }, -+ $schema: RULESYNC_MCP_SCHEMA_URL, -+ }); -+ }); - }); - - describe("validate", () => { -diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts -index ff9753db8..a448fd96c 100644 ---- a/src/features/mcp/copilotcli-mcp.ts -+++ b/src/features/mcp/copilotcli-mcp.ts -@@ -22,23 +22,29 @@ type CopilotcliMcpConfig = { - /** - * Adds "type": "stdio" to each MCP server config if not present. - * GitHub Copilot CLI requires the "type" field for each server. -- * @throws Error if a server doesn't have a command (Copilot CLI stdio servers require a command) -+ * @throws Error if a stdio server doesn't have a command - */ - function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] { - const result: NonNullable = {}; - - for (const [name, server] of Object.entries(mcpServers)) { -- // Parse and validate the server config - const parsed = McpServerSchema.parse(server); -+ const type = parsed.type ?? "stdio"; -+ -+ if (type !== "stdio") { -+ result[name] = { -+ ...parsed, -+ type, -+ }; -+ continue; -+ } - -- // Copilot CLI stdio servers require a non-empty command - if (!parsed.command) { - throw new Error( - `MCP server "${name}" is missing a command. GitHub Copilot CLI stdio servers require a non-empty command.`, - ); - } - -- // Handle command as string or array - let command: string; - let args: string[] | undefined; - -@@ -46,29 +52,20 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] - command = parsed.command; - args = parsed.args; - } else { -- // command is an array: first element is command, rest are args - const [cmd, ...cmdArgs] = parsed.command; - if (!cmd) { - throw new Error(`MCP server "${name}" has an empty command array.`); - } - command = cmd; -- // Merge command array args with existing args - args = cmdArgs.length > 0 ? [...cmdArgs, ...(parsed.args ?? [])] : parsed.args; - } - -- // Use the parsed object for the base, then override with normalized command/args -- // and ensure type is set to "stdio" if not present. -- // We spread server as well to keep unknown fields as suggested by reviewers. -- // eslint-disable-next-line no-type-assertion/no-type-assertion -- const serverRecord = server as Record; -- // eslint-disable-next-line no-type-assertion/no-type-assertion - result[name] = { -- ...serverRecord, - ...parsed, -- type: parsed.type ?? "stdio", -+ type, - command, - ...(args && { args }), -- } as McpServer & Record; -+ }; - } - - return result; -@@ -81,6 +78,11 @@ function removeTypeField(config: CopilotcliMcpConfig): McpServers { - const result: McpServers = {}; - - for (const [name, server] of Object.entries(config.mcpServers ?? {})) { -+ if (server.type !== "stdio") { -+ result[name] = server; -+ continue; -+ } -+ - const { type: _, ...rest } = server; - result[name] = rest; - } -@@ -109,13 +111,7 @@ export class CopilotcliMcp extends ToolMcp { - return !this.global; - } - -- static getSettablePaths({ global }: { global?: boolean } = {}): ToolMcpSettablePaths { -- if (global) { -- return { -- relativeDirPath: ".copilot", -- relativeFilePath: "mcp-config.json", -- }; -- } -+ static getSettablePaths(_options: { global?: boolean } = {}): ToolMcpSettablePaths { - return { - relativeDirPath: ".copilot", - relativeFilePath: "mcp-config.json", -diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts -new file mode 100644 -index 000000000..8575be8d3 ---- /dev/null -+++ b/src/features/rules/copilotcli-rule.ts -@@ -0,0 +1,41 @@ -+import { ToolTarget } from "../../types/tool-targets.js"; -+import { CopilotRule } from "./copilot-rule.js"; -+import { RulesyncRule } from "./rulesync-rule.js"; -+import { -+ ToolRuleForDeletionParams, -+ ToolRuleFromFileParams, -+ ToolRuleFromRulesyncRuleParams, -+} from "./tool-rule.js"; -+ -+export class CopilotcliRule extends CopilotRule { -+ private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { -+ return new CopilotcliRule({ -+ baseDir: copilotRule.getBaseDir(), -+ relativeDirPath: copilotRule.getRelativeDirPath(), -+ relativeFilePath: copilotRule.getRelativeFilePath(), -+ frontmatter: copilotRule.getFrontmatter(), -+ body: copilotRule.getBody(), -+ validate: true, -+ root: copilotRule.isRoot(), -+ }); -+ } -+ -+ static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { -+ return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); -+ } -+ -+ static override async fromFile(params: ToolRuleFromFileParams): Promise { -+ return this.fromCopilotRule(await CopilotRule.fromFile(params)); -+ } -+ -+ static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { -+ return this.fromCopilotRule(CopilotRule.forDeletion(params)); -+ } -+ -+ static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { -+ return this.isTargetedByRulesyncRuleDefault({ -+ rulesyncRule, -+ toolTarget: "copilotcli" satisfies ToolTarget, -+ }); -+ } -+} -diff --git a/src/features/rules/rules-processor.test.ts b/src/features/rules/rules-processor.test.ts -index 60e3da0d5..cbab91f5d 100644 ---- a/src/features/rules/rules-processor.test.ts -+++ b/src/features/rules/rules-processor.test.ts -@@ -11,6 +11,7 @@ import { AugmentcodeLegacyRule } from "./augmentcode-legacy-rule.js"; - import { ClaudecodeLegacyRule } from "./claudecode-legacy-rule.js"; - import { ClaudecodeRule } from "./claudecode-rule.js"; - import { CopilotRule } from "./copilot-rule.js"; -+import { CopilotcliRule } from "./copilotcli-rule.js"; - import { CursorRule } from "./cursor-rule.js"; - import { OpenCodeRule } from "./opencode-rule.js"; - import { RovodevRule } from "./rovodev-rule.js"; -@@ -186,6 +187,7 @@ describe("RulesProcessor", () => { - it("should correctly validate and filter rules for each supported tool", async () => { - const testCases = [ - { toolTarget: "copilot" as const, ruleClass: CopilotRule }, -+ { toolTarget: "copilotcli" as const, ruleClass: CopilotcliRule }, - { toolTarget: "cursor" as const, ruleClass: CursorRule }, - { toolTarget: "claudecode" as const, ruleClass: ClaudecodeRule }, - { toolTarget: "warp" as const, ruleClass: WarpRule }, -@@ -795,6 +797,7 @@ Content that would fail parsing`; - "claudecode-legacy", - "codexcli", - "copilot", -+ "copilotcli", - "factorydroid", - "geminicli", - "goose", -@@ -825,13 +828,14 @@ Content that would fail parsing`; - expect(globalTargets).toContain("claudecode-legacy"); - expect(globalTargets).toContain("codexcli"); - expect(globalTargets).toContain("copilot"); -+ expect(globalTargets).toContain("copilotcli"); - expect(globalTargets).toContain("factorydroid"); - expect(globalTargets).toContain("geminicli"); - expect(globalTargets).toContain("kilo"); - expect(globalTargets).toContain("goose"); - expect(globalTargets).toContain("opencode"); - expect(globalTargets).toContain("rovodev"); -- expect(globalTargets.length).toBe(10); -+ expect(globalTargets.length).toBe(11); - - // These targets should NOT be in global mode - expect(globalTargets).not.toContain("cursor"); -diff --git a/src/features/rules/rules-processor.ts b/src/features/rules/rules-processor.ts -index 2f64feb30..2886c85c1 100644 ---- a/src/features/rules/rules-processor.ts -+++ b/src/features/rules/rules-processor.ts -@@ -39,6 +39,7 @@ import { ClaudecodeRule } from "./claudecode-rule.js"; - import { ClineRule } from "./cline-rule.js"; - import { CodexcliRule } from "./codexcli-rule.js"; - import { CopilotRule } from "./copilot-rule.js"; -+import { CopilotcliRule } from "./copilotcli-rule.js"; - import { CursorRule } from "./cursor-rule.js"; - import { DeepagentsRule } from "./deepagents-rule.js"; - import { FactorydroidRule } from "./factorydroid-rule.js"; -@@ -74,6 +75,7 @@ const rulesProcessorToolTargets: ToolTarget[] = [ - "cline", - "codexcli", - "copilot", -+ "copilotcli", - "cursor", - "deepagents", - "factorydroid", -@@ -290,6 +292,17 @@ const toolRuleFactories = new Map([ - }, - }, - ], -+ [ -+ "copilotcli", -+ { -+ class: CopilotcliRule, -+ meta: { -+ extension: "md", -+ supportsGlobal: true, -+ ruleDiscoveryMode: "auto", -+ }, -+ }, -+ ], - [ - "cursor", - { diff --git a/README.md b/README.md index 073f52288..2e0d56d57 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ See [Quick Start guide](https://dyoshikawa.github.io/rulesync/getting-started/qu | Gemini CLI | geminicli | ✅ 🌏 | ✅ | ✅ 🌏 | ✅ 🌏 | 🎮 | ✅ 🌏 | ✅ 🌏 | | Goose | goose | ✅ 🌏 | ✅ | | | | | | | GitHub Copilot | copilot | ✅ 🌏 | | ✅ | ✅ | ✅ | ✅ | ✅ | -| GitHub Copilot CLI | copilotcli | | | ✅ 🌏 | | | | | +| GitHub Copilot CLI | copilotcli | ✅ 🌏 | | ✅ 🌏 | | | | | | Cursor | cursor | ✅ | ✅ | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ | | deepagents-cli | deepagents | ✅ | | ✅ 🌏 | | ✅ | ✅ | 🌏 | | Factory Droid | factorydroid | ✅ 🌏 | | ✅ 🌏 | 🎮 | 🎮 | 🎮 | ✅ 🌏 | diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index ec6719a94..9c651951b 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -64,6 +64,15 @@ Example: "type": "stdio", "command": "uvx", "args": ["--from", "git+https://github.com/oraios/serena", "serena", "start-mcp-server"] + }, + "github": { + "type": "http", + "url": "http://localhost:3000/mcp" + }, + "local-dev": { + "type": "local", + "command": "node", + "args": ["scripts/start-local-mcp.js"] } } } @@ -74,7 +83,7 @@ This file is used by the GitHub Copilot CLI for MCP server configuration. Rulesy - **Project mode:** `.copilot/mcp-config.json` (relative to project root) - **Global mode:** `~/.copilot/mcp-config.json` (relative to home directory) -Rulesync ensures that each server entry contains the mandatory `"type": "stdio"` field required by the Copilot CLI. +Rulesync preserves explicit `type` values for `http`, `sse`, and `local` servers. For command-based servers that omit a transport type, Rulesync emits the mandatory `"type": "stdio"` field required by the Copilot CLI. Use optional **override keys** so tool-specific events and config live in one file without leaking to others: `cursor.hooks` for Cursor-only events, `claudecode.hooks` for Claude-only, `opencode.hooks` for OpenCode-only, `copilot.hooks` for GitHub Copilot-only, `geminicli.hooks` for Gemini CLI-only. Events in shared `hooks` that a tool does not support are skipped for that tool (and a warning is logged at generate time). diff --git a/docs/reference/supported-tools.md b/docs/reference/supported-tools.md index 7020334b7..540a1a902 100644 --- a/docs/reference/supported-tools.md +++ b/docs/reference/supported-tools.md @@ -10,7 +10,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Codex CLI | codexcli | ✅ 🌏 | | ✅ 🌏 🔧 | 🌏 | ✅ 🌏 | ✅ 🌏 | | | Gemini CLI | geminicli | ✅ 🌏 | ✅ | ✅ 🌏 | ✅ 🌏 | 🎮 | ✅ 🌏 | ✅ 🌏 | | GitHub Copilot | copilot | ✅ 🌏 | | ✅ | ✅ | ✅ | ✅ | ✅ | -| GitHub Copilot CLI | copilotcli | | | ✅ 🌏 | | | | | +| GitHub Copilot CLI | copilotcli | ✅ 🌏 | | ✅ 🌏 | | | | | | Goose | goose | ✅ 🌏 | ✅ | | | | | | | Cursor | cursor | ✅ | ✅ | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ | | Factory Droid | factorydroid | ✅ 🌏 | | ✅ 🌏 | 🎮 | 🎮 | 🎮 | ✅ 🌏 | diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index ec6719a94..9c651951b 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -64,6 +64,15 @@ Example: "type": "stdio", "command": "uvx", "args": ["--from", "git+https://github.com/oraios/serena", "serena", "start-mcp-server"] + }, + "github": { + "type": "http", + "url": "http://localhost:3000/mcp" + }, + "local-dev": { + "type": "local", + "command": "node", + "args": ["scripts/start-local-mcp.js"] } } } @@ -74,7 +83,7 @@ This file is used by the GitHub Copilot CLI for MCP server configuration. Rulesy - **Project mode:** `.copilot/mcp-config.json` (relative to project root) - **Global mode:** `~/.copilot/mcp-config.json` (relative to home directory) -Rulesync ensures that each server entry contains the mandatory `"type": "stdio"` field required by the Copilot CLI. +Rulesync preserves explicit `type` values for `http`, `sse`, and `local` servers. For command-based servers that omit a transport type, Rulesync emits the mandatory `"type": "stdio"` field required by the Copilot CLI. Use optional **override keys** so tool-specific events and config live in one file without leaking to others: `cursor.hooks` for Cursor-only events, `claudecode.hooks` for Claude-only, `opencode.hooks` for OpenCode-only, `copilot.hooks` for GitHub Copilot-only, `geminicli.hooks` for Gemini CLI-only. Events in shared `hooks` that a tool does not support are skipped for that tool (and a warning is logged at generate time). diff --git a/skills/rulesync/supported-tools.md b/skills/rulesync/supported-tools.md index 7020334b7..540a1a902 100644 --- a/skills/rulesync/supported-tools.md +++ b/skills/rulesync/supported-tools.md @@ -10,7 +10,7 @@ Rulesync supports both **generation** and **import** for All of the major AI cod | Codex CLI | codexcli | ✅ 🌏 | | ✅ 🌏 🔧 | 🌏 | ✅ 🌏 | ✅ 🌏 | | | Gemini CLI | geminicli | ✅ 🌏 | ✅ | ✅ 🌏 | ✅ 🌏 | 🎮 | ✅ 🌏 | ✅ 🌏 | | GitHub Copilot | copilot | ✅ 🌏 | | ✅ | ✅ | ✅ | ✅ | ✅ | -| GitHub Copilot CLI | copilotcli | | | ✅ 🌏 | | | | | +| GitHub Copilot CLI | copilotcli | ✅ 🌏 | | ✅ 🌏 | | | | | | Goose | goose | ✅ 🌏 | ✅ | | | | | | | Cursor | cursor | ✅ | ✅ | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ 🌏 | ✅ | | Factory Droid | factorydroid | ✅ 🌏 | | ✅ 🌏 | 🎮 | 🎮 | 🎮 | ✅ 🌏 | diff --git a/src/features/rules/copilotcli-rule.test.ts b/src/features/rules/copilotcli-rule.test.ts new file mode 100644 index 000000000..98a55c94c --- /dev/null +++ b/src/features/rules/copilotcli-rule.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { setupTestDirectory } from "../../test-utils/test-directories.js"; +import { CopilotRule } from "./copilot-rule.js"; +import { CopilotcliRule } from "./copilotcli-rule.js"; +import { RulesyncRule } from "./rulesync-rule.js"; + +describe("CopilotcliRule", () => { + let testDir: string; + let cleanup: () => Promise; + + beforeEach(async () => { + ({ testDir, cleanup } = await setupTestDirectory()); + vi.spyOn(process, "cwd").mockReturnValue(testDir); + }); + + afterEach(async () => { + await cleanup(); + vi.restoreAllMocks(); + }); + + describe("fromRulesyncRule", () => { + it("should pass validate to the copied Copilot rule", () => { + const rulesyncRule = new RulesyncRule({ + baseDir: testDir, + relativeDirPath: ".rulesync/rules", + relativeFilePath: "test.md", + frontmatter: { + targets: ["copilotcli"], + }, + body: "Test rule content", + }); + + const invalidCopilotRule = Reflect.construct(CopilotRule, [ + { + baseDir: testDir, + relativeDirPath: ".github/instructions", + relativeFilePath: "test.instructions.md", + frontmatter: { + excludeAgent: "invalid-agent", + }, + body: "Test rule content", + validate: false, + }, + ]); + const fromRulesyncRuleSpy = vi + .spyOn(CopilotRule, "fromRulesyncRule") + .mockReturnValue(invalidCopilotRule); + + expect(() => + CopilotcliRule.fromRulesyncRule({ rulesyncRule, validate: false }), + ).not.toThrow(); + expect(fromRulesyncRuleSpy).toHaveBeenCalledWith({ + rulesyncRule, + validate: false, + }); + + expect(() => CopilotcliRule.fromRulesyncRule({ rulesyncRule, validate: true })).toThrow( + "Invalid frontmatter", + ); + }); + }); +}); diff --git a/src/features/rules/copilotcli-rule.ts b/src/features/rules/copilotcli-rule.ts index 8575be8d3..fa1b3d6f1 100644 --- a/src/features/rules/copilotcli-rule.ts +++ b/src/features/rules/copilotcli-rule.ts @@ -8,28 +8,34 @@ import { } from "./tool-rule.js"; export class CopilotcliRule extends CopilotRule { - private static fromCopilotRule(copilotRule: CopilotRule): CopilotcliRule { + private static fromCopilotRule(copilotRule: CopilotRule, validate = true): CopilotcliRule { return new CopilotcliRule({ baseDir: copilotRule.getBaseDir(), relativeDirPath: copilotRule.getRelativeDirPath(), relativeFilePath: copilotRule.getRelativeFilePath(), frontmatter: copilotRule.getFrontmatter(), body: copilotRule.getBody(), - validate: true, + validate, root: copilotRule.isRoot(), }); } - static override fromRulesyncRule(params: ToolRuleFromRulesyncRuleParams): CopilotcliRule { - return this.fromCopilotRule(CopilotRule.fromRulesyncRule(params)); + static override fromRulesyncRule({ + validate = true, + ...rest + }: ToolRuleFromRulesyncRuleParams): CopilotcliRule { + return this.fromCopilotRule(CopilotRule.fromRulesyncRule({ validate, ...rest }), validate); } - static override async fromFile(params: ToolRuleFromFileParams): Promise { - return this.fromCopilotRule(await CopilotRule.fromFile(params)); + static override async fromFile({ + validate = true, + ...rest + }: ToolRuleFromFileParams): Promise { + return this.fromCopilotRule(await CopilotRule.fromFile({ validate, ...rest }), validate); } static override forDeletion(params: ToolRuleForDeletionParams): CopilotcliRule { - return this.fromCopilotRule(CopilotRule.forDeletion(params)); + return this.fromCopilotRule(CopilotRule.forDeletion(params), false); } static override isTargetedByRulesyncRule(rulesyncRule: RulesyncRule): boolean { From e7e3bc3cd802826bda17c88368fa5297e4360780 Mon Sep 17 00:00:00 2001 From: dyoshikawa-claw Date: Tue, 31 Mar 2026 23:18:34 +0900 Subject: [PATCH 4/6] fix: validate remote copilotcli MCP endpoints --- src/features/mcp/copilotcli-mcp.test.ts | 22 ++++++++++++++++++++++ src/features/mcp/copilotcli-mcp.ts | 7 +++++++ 2 files changed, 29 insertions(+) diff --git a/src/features/mcp/copilotcli-mcp.test.ts b/src/features/mcp/copilotcli-mcp.test.ts index 6a6155276..b6912e35d 100644 --- a/src/features/mcp/copilotcli-mcp.test.ts +++ b/src/features/mcp/copilotcli-mcp.test.ts @@ -489,6 +489,28 @@ describe("CopilotcliMcp", () => { }); }); + it("should throw error when remote server has no url or httpUrl", async () => { + const inputMcpServers = { + "remote-server": { + type: "http" as const, + headers: { + Authorization: "Bearer token", + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ mcpServers: inputMcpServers }), + }); + + await expect( + CopilotcliMcp.fromRulesyncMcp({ + rulesyncMcp, + }), + ).rejects.toThrow('MCP server "remote-server" is missing a url or httpUrl'); + }); + it("should require command for local type servers", async () => { const inputMcpServers = { "local-server": { diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts index 0b1cb0feb..62bc91367 100644 --- a/src/features/mcp/copilotcli-mcp.ts +++ b/src/features/mcp/copilotcli-mcp.ts @@ -43,6 +43,7 @@ const resolveCopilotcliServerType = (server: McpServer): CopilotcliServerType => * Adds "type": "stdio" to each MCP server config if not present. * GitHub Copilot CLI requires the "type" field for each server. * @throws Error if a stdio server doesn't have a command + * @throws Error if an http/sse server doesn't have a url or httpUrl */ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] { const result: NonNullable = {}; @@ -52,6 +53,12 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] const type = resolveCopilotcliServerType(parsed); if (isRemoteServerType(type)) { + if (!parsed.url && !parsed.httpUrl) { + throw new Error( + `MCP server "${name}" is missing a url or httpUrl. GitHub Copilot CLI ${type} servers require a non-empty url or httpUrl.`, + ); + } + result[name] = { ...parsed, type, From 140ded95447170e6c3070e76c4452f425c28655d Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Tue, 31 Mar 2026 19:16:14 -0700 Subject: [PATCH 5/6] fix: add local to transport enum for consistency with type enum The transport enum was missing the "local" value that was already added to the type enum, which could cause silent fallback to stdio when transport: "local" was specified. Co-Authored-By: Claude Opus 4.6 --- src/types/mcp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/mcp.ts b/src/types/mcp.ts index 17a8d7aec..9d091ec4a 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -12,7 +12,7 @@ export const McpServerSchema = z.looseObject({ timeout: z.optional(z.number()), trust: z.optional(z.boolean()), cwd: z.optional(z.string()), - transport: z.optional(z.enum(["stdio", "sse", "http"])), + transport: z.optional(z.enum(["local", "stdio", "sse", "http"])), alwaysAllow: z.optional(z.array(z.string())), tools: z.optional(z.array(z.string())), kiroAutoApprove: z.optional(z.array(z.string())), From 7c8289a26e429f36d277740499c679f13f8ab8df Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Tue, 31 Mar 2026 19:39:46 -0700 Subject: [PATCH 6/6] fix: handle transport: local in resolveCopilotcliServerType and use dynamic type in error message - Add transport: local to resolveCopilotcliServerType so it no longer silently falls back to stdio - Use the resolved type variable in the missing-command error message instead of hardcoding stdio - Update JSDoc to reflect the function's actual behavior Co-Authored-By: Claude Opus 4.6 --- src/features/mcp/copilotcli-mcp.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/features/mcp/copilotcli-mcp.ts b/src/features/mcp/copilotcli-mcp.ts index 62bc91367..febeb5a5f 100644 --- a/src/features/mcp/copilotcli-mcp.ts +++ b/src/features/mcp/copilotcli-mcp.ts @@ -32,7 +32,7 @@ const resolveCopilotcliServerType = (server: McpServer): CopilotcliServerType => return server.type; } - if (server.transport === "http" || server.transport === "sse") { + if (server.transport === "http" || server.transport === "sse" || server.transport === "local") { return server.transport; } @@ -40,9 +40,9 @@ const resolveCopilotcliServerType = (server: McpServer): CopilotcliServerType => }; /** - * Adds "type": "stdio" to each MCP server config if not present. + * Resolves and sets the transport type for each MCP server config. * GitHub Copilot CLI requires the "type" field for each server. - * @throws Error if a stdio server doesn't have a command + * @throws Error if a stdio/local server doesn't have a command * @throws Error if an http/sse server doesn't have a url or httpUrl */ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] { @@ -68,7 +68,7 @@ function addTypeField(mcpServers: McpServers): CopilotcliMcpConfig["mcpServers"] if (!parsed.command) { throw new Error( - `MCP server "${name}" is missing a command. GitHub Copilot CLI stdio servers require a non-empty command.`, + `MCP server "${name}" is missing a command. GitHub Copilot CLI ${type} servers require a non-empty command.`, ); }