diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..d924ccb --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,50 @@ +name: Test + +# The release workflow already runs `go test ./...`, but only on a tag — so a +# broken test first surfaced during a release, with the tag already pushed. +# This moves the same gate to the point where it can still cheaply say no. + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +# A force-push supersedes the previous run; keep the newest, drop the rest. +# main is exempt: every commit there should keep a result of its own. +concurrency: + group: test-${{ github.ref }} + cancel-in-progress: ${{ github.ref != 'refs/heads/main' }} + +jobs: + test: + name: vet + test + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + # Formatting first: it is the cheapest signal and the one most likely to + # differ between a contributor's editor and the repo. + - name: Check formatting + run: | + unformatted=$(gofmt -l .) + if [ -n "$unformatted" ]; then + echo "::error::gofmt would rewrite these files; run 'gofmt -w .'" + echo "$unformatted" + exit 1 + fi + + - name: Vet + run: go vet ./... + + - name: Test + run: go test ./... diff --git a/cmd/doctor.go b/cmd/doctor.go index 3c3d104..4bc85b6 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -173,10 +173,11 @@ func convergeMCPForAgent(baseDir string, agent *agents.Agent) (bool, error) { switch agent.ID { case agents.Cursor: return agents.ConvergeCursorMCPJSON(path) - case agents.ClaudeCode, agents.GeminiCLI, agents.RooCode: + // Copilot rides the standard shape: its only project-level MCP source is + // the workspace-root .mcp.json, the same file (and key, and entry) as + // claude-code — see copilotMCPPath. + case agents.ClaudeCode, agents.GeminiCLI, agents.RooCode, agents.Copilot: return agents.ConvergeStandardMCPJSON(path) - case agents.Copilot: - return agents.ConvergeVSCodeMCPJSON(path) default: return false, agent.WriteMCPConfig(baseDir) } diff --git a/cmd/hooks_claude_code.go b/cmd/hooks_claude_code.go index 4b4423b..f3c95c6 100644 --- a/cmd/hooks_claude_code.go +++ b/cmd/hooks_claude_code.go @@ -49,6 +49,28 @@ type hookOutput struct { SystemMessage string `json:"systemMessage,omitempty"` } +// copilotHookOutput is Copilot's native sessionStart response: a bare +// top-level additionalContext, no wrapper and no systemMessage slot +// (GitHub's hooks-configuration reference). Copilot's PascalCase +// "Claude-compatible" mode changes only the INPUT payload shape — the output +// schema is the native one either way — so the hooks config we install +// (camelCase "sessionStart") and this writer must agree, or the context is +// parsed as nothing and the session starts blind. +type copilotHookOutput struct { + AdditionalContext string `json:"additionalContext,omitempty"` +} + +// sessionOutputShape selects how a host wants session-start context framed. +type sessionOutputShape int + +const ( + // shapeClaudeCompat — {"hookSpecificOutput": {...}, "systemMessage": ...}. + // Claude Code's native schema, which Cursor and Gemini CLI also accept. + shapeClaudeCompat sessionOutputShape = iota + // shapeCopilotNative — {"additionalContext": "..."}. + shapeCopilotNative +) + // resolveBaseDir returns the base directory from hook input, falling back to cwd. func resolveBaseDir(input *hookInput) (string, error) { if input.CWD != "" { @@ -82,13 +104,15 @@ func newHooksClaudeCodeCmd(version string) *cobra.Command { } func newSessionStartCmd(version string) *cobra.Command { - return newSessionStartHookCmd("session-start", "Handle SessionStart hook event", version) + return newSessionStartHookCmd("session-start", "Handle SessionStart hook event", version, shapeClaudeCompat) } // --- Hook command factories --- -// newSessionStartHookCmd creates a session-start hook command (shared across agents). -func newSessionStartHookCmd(use, short, version string) *cobra.Command { +// newSessionStartHookCmd creates a session-start hook command (shared across +// agents). The shape argument is per-host and NOT cosmetic: a host that does +// not recognize the wrapper reads no context at all. +func newSessionStartHookCmd(use, short, version string, shape sessionOutputShape) *cobra.Command { return &cobra.Command{ Use: use, Short: short, @@ -104,7 +128,7 @@ func newSessionStartHookCmd(use, short, version string) *cobra.Command { if err != nil { return err } - out, err := handleSessionStartDeduped(baseDir, version, input.dedupKey(), defaultSessionStampDir()) + out, err := handleSessionStartDeduped(baseDir, version, input.dedupKey(), defaultSessionStampDir(), shape) if err != nil { return err } @@ -208,7 +232,7 @@ func sweepExpiredStamps(stampDir string) { // An empty key or stampDir fails open (always emit); a suppressed repeat // returns empty output with nil error — hosts must never see a failing hook // here, just silence. -func handleSessionStartDeduped(baseDir, version, key, stampDir string) ([]byte, error) { +func handleSessionStartDeduped(baseDir, version, key, stampDir string, shape sessionOutputShape) ([]byte, error) { dedup := key != "" && stampDir != "" if dedup { key += "\x00" + baseDir @@ -216,7 +240,7 @@ func handleSessionStartDeduped(baseDir, version, key, stampDir string) ([]byte, return nil, nil } } - out, err := handleSessionStart(baseDir, version) + out, err := handleSessionStart(baseDir, version, shape) if err != nil { if dedup { // Don't let a failed build suppress the retry within the window. @@ -229,8 +253,14 @@ func handleSessionStartDeduped(baseDir, version, key, stampDir string) ([]byte, // --- Session Start Handler (Claude Code adapter) --- -func handleSessionStart(baseDir, version string) ([]byte, error) { +func handleSessionStart(baseDir, version string, shape sessionOutputShape) ([]byte, error) { ctx, docCount := buildSessionContext(baseDir) + if shape == shapeCopilotNative { + // No systemMessage slot on this host, so the connected line is dropped + // rather than smuggled into additionalContext — it is a cosmetic banner + // for the user, not context for the model. + return json.Marshal(copilotHookOutput{AdditionalContext: ctx}) + } output := hookOutput{ HookSpecificOutput: map[string]any{ "hookEventName": "SessionStart", diff --git a/cmd/hooks_claude_code_test.go b/cmd/hooks_claude_code_test.go index f001109..cb8825d 100644 --- a/cmd/hooks_claude_code_test.go +++ b/cmd/hooks_claude_code_test.go @@ -47,7 +47,7 @@ func TestHandleSessionStart_WithDocuments(t *testing.T) { t.Fatal(err) } - data, err := handleSessionStart(base, "v0.1.0") + data, err := handleSessionStart(base, "v0.1.0", shapeClaudeCompat) if err != nil { t.Fatalf("handleSessionStart: %v", err) } @@ -86,7 +86,7 @@ func TestHandleSessionStart_Empty(t *testing.T) { t.Parallel() base := setupArchcoreDir(t) - data, err := handleSessionStart(base, "v0.1.0") + data, err := handleSessionStart(base, "v0.1.0", shapeClaudeCompat) if err != nil { t.Fatalf("handleSessionStart: %v", err) } diff --git a/cmd/hooks_copilot.go b/cmd/hooks_copilot.go index ed3a626..d827d98 100644 --- a/cmd/hooks_copilot.go +++ b/cmd/hooks_copilot.go @@ -10,7 +10,7 @@ func newHooksCopilotCmd(version string) *cobra.Command { Short: "Handle GitHub Copilot hook events", } cmd.AddCommand( - newSessionStartHookCmd("session-start", "Handle Copilot SessionStart hook event", version), + newSessionStartHookCmd("session-start", "Handle Copilot SessionStart hook event", version, shapeCopilotNative), ) return cmd } diff --git a/cmd/hooks_copilot_test.go b/cmd/hooks_copilot_test.go index ad65798..431d892 100644 --- a/cmd/hooks_copilot_test.go +++ b/cmd/hooks_copilot_test.go @@ -17,17 +17,84 @@ func TestRunHooksInstallForAgent_CopilotAlsoInstallsMCP(t *testing.T) { t.Fatalf("runHooksInstallForAgent: %v", err) } - // .vscode/mcp.json should exist with "servers" key. - data, err := os.ReadFile(filepath.Join(base, ".vscode", "mcp.json")) + // The workspace-root .mcp.json should exist with the "mcpServers" key — + // the only project-level MCP source Copilot CLI reads. + data, err := os.ReadFile(filepath.Join(base, ".mcp.json")) if err != nil { - t.Fatalf("ReadFile .vscode/mcp.json: %v", err) + t.Fatalf("ReadFile .mcp.json: %v", err) } var raw map[string]json.RawMessage if err := json.Unmarshal(data, &raw); err != nil { t.Fatalf("Unmarshal: %v", err) } - if _, ok := raw["servers"]; !ok { - t.Error("missing 'servers' in .vscode/mcp.json") + if _, ok := raw["mcpServers"]; !ok { + t.Error("missing 'mcpServers' in .mcp.json") + } +} + +// TestHandleSessionStart_CopilotNativeShape pins the output schema divergence. +// The hooks config we install registers camelCase "sessionStart" — Copilot's +// NATIVE format — whose documented output is a bare top-level +// additionalContext. Emitting Claude's hookSpecificOutput wrapper here parses +// as nothing: the hook exits 0, the user sees no error, and the session simply +// starts with no archcore context. That silence is why this is a test and not +// a comment. +func TestHandleSessionStart_CopilotNativeShape(t *testing.T) { + t.Parallel() + base := setupArchcoreDir(t) + + out, err := handleSessionStart(base, "v0.0.0-test", shapeCopilotNative) + if err != nil { + t.Fatalf("handleSessionStart: %v", err) + } + + var raw map[string]json.RawMessage + if err := json.Unmarshal(out, &raw); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if _, ok := raw["hookSpecificOutput"]; ok { + t.Error("hookSpecificOutput is Claude's wrapper — Copilot ignores it entirely") + } + if _, ok := raw["systemMessage"]; ok { + t.Error("Copilot's sessionStart schema has no systemMessage slot") + } + ctx, ok := raw["additionalContext"] + if !ok { + t.Fatal("missing top-level additionalContext") + } + var s string + if err := json.Unmarshal(ctx, &s); err != nil { + t.Fatalf("additionalContext must be a JSON string: %v", err) + } + if s == "" { + t.Error("additionalContext is empty — no context would reach the model") + } +} + +// The other hosts must keep the Claude-compatible wrapper: same context, same +// helper, different frame. A refactor that collapsed both onto one shape would +// break whichever host it did not pick. +func TestHandleSessionStart_ClaudeCompatShapeUnchanged(t *testing.T) { + t.Parallel() + base := setupArchcoreDir(t) + + out, err := handleSessionStart(base, "v0.0.0-test", shapeClaudeCompat) + if err != nil { + t.Fatalf("handleSessionStart: %v", err) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(out, &raw); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if _, ok := raw["additionalContext"]; ok { + t.Error("top-level additionalContext is Copilot's shape, not Claude's") + } + var wrapper map[string]json.RawMessage + if err := json.Unmarshal(raw["hookSpecificOutput"], &wrapper); err != nil { + t.Fatalf("Unmarshal hookSpecificOutput: %v", err) + } + if _, ok := wrapper["additionalContext"]; !ok { + t.Error("missing additionalContext inside hookSpecificOutput") } } diff --git a/cmd/hooks_cursor.go b/cmd/hooks_cursor.go index d255fb1..f91ab54 100644 --- a/cmd/hooks_cursor.go +++ b/cmd/hooks_cursor.go @@ -10,7 +10,7 @@ func newHooksCursorCmd(version string) *cobra.Command { Short: "Handle Cursor hook events", } cmd.AddCommand( - newSessionStartHookCmd("session-start", "Handle Cursor SessionStart hook event", version), + newSessionStartHookCmd("session-start", "Handle Cursor SessionStart hook event", version, shapeClaudeCompat), ) return cmd } diff --git a/cmd/hooks_gemini_cli.go b/cmd/hooks_gemini_cli.go index 6fe1a64..93e8d36 100644 --- a/cmd/hooks_gemini_cli.go +++ b/cmd/hooks_gemini_cli.go @@ -10,7 +10,7 @@ func newHooksGeminiCLICmd(version string) *cobra.Command { Short: "Handle Gemini CLI hook events", } cmd.AddCommand( - newSessionStartHookCmd("session-start", "Handle Gemini CLI SessionStart hook event", version), + newSessionStartHookCmd("session-start", "Handle Gemini CLI SessionStart hook event", version, shapeClaudeCompat), ) return cmd } diff --git a/cmd/hooks_session_dedup_spec_test.go b/cmd/hooks_session_dedup_spec_test.go index 7e477fa..260516c 100644 --- a/cmd/hooks_session_dedup_spec_test.go +++ b/cmd/hooks_session_dedup_spec_test.go @@ -27,7 +27,7 @@ func TestHandleSessionStart_DedupesBySessionID_Spec(t *testing.T) { stampDir := t.TempDir() // 1. First call for session "s1": full context emitted. - first, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir) + first, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("first call: %v", err) } @@ -43,7 +43,7 @@ func TestHandleSessionStart_DedupesBySessionID_Spec(t *testing.T) { // 2. Repeat for the SAME session: empty output, nil error — hosts must // never see a failing hook here, just silence. - second, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir) + second, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("repeat call must not error: %v", err) } @@ -52,7 +52,7 @@ func TestHandleSessionStart_DedupesBySessionID_Spec(t *testing.T) { } // 3. A DIFFERENT session emits again — dedup is per-session, not global. - other, err := handleSessionStartDeduped(base, "v0.0.0-test", "s2", stampDir) + other, err := handleSessionStartDeduped(base, "v0.0.0-test", "s2", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("other-session call: %v", err) } @@ -67,7 +67,7 @@ func TestHandleSessionStart_EmptyKeyFailsOpen(t *testing.T) { stampDir := t.TempDir() for i := 1; i <= 2; i++ { - out, err := handleSessionStartDeduped(base, "v0.0.0-test", "", stampDir) + out, err := handleSessionStartDeduped(base, "v0.0.0-test", "", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("call %d: %v", i, err) } @@ -89,7 +89,7 @@ func TestHandleSessionStart_UnwritableStampDirFailsOpen(t *testing.T) { stampDir := filepath.Join(parent, "stamps") for i := 1; i <= 2; i++ { - out, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir) + out, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("call %d must stay exit-0 with unwritable stamp dir: %v", i, err) } @@ -104,7 +104,7 @@ func TestHandleSessionStart_ExpiredStampReEmits(t *testing.T) { base := setupArchcoreDir(t) stampDir := t.TempDir() - if _, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir); err != nil { + if _, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat); err != nil { t.Fatal(err) } // Age the stamp beyond the window: the next call must emit again. The @@ -115,7 +115,7 @@ func TestHandleSessionStart_ExpiredStampReEmits(t *testing.T) { t.Fatalf("aging stamp: %v", err) } - out, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir) + out, err := handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatal(err) } @@ -141,7 +141,7 @@ func TestHandleSessionStart_ConcurrentDoubleFire_ExactlyOneEmits(t *testing.T) { for i := range n { wg.Go(func() { <-start - outs[i], errs[i] = handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir) + outs[i], errs[i] = handleSessionStartDeduped(base, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) }) } close(start) @@ -170,7 +170,7 @@ func TestHandleSessionStart_SameSessionDifferentProjectsBothEmit(t *testing.T) { baseB := setupArchcoreDir(t) stampDir := t.TempDir() - outA, err := handleSessionStartDeduped(baseA, "v0.0.0-test", "s1", stampDir) + outA, err := handleSessionStartDeduped(baseA, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("project A: %v", err) } @@ -178,7 +178,7 @@ func TestHandleSessionStart_SameSessionDifferentProjectsBothEmit(t *testing.T) { t.Fatal("project A must emit") } - outB, err := handleSessionStartDeduped(baseB, "v0.0.0-test", "s1", stampDir) + outB, err := handleSessionStartDeduped(baseB, "v0.0.0-test", "s1", stampDir, shapeClaudeCompat) if err != nil { t.Fatalf("project B: %v", err) } diff --git a/cmd/init.go b/cmd/init.go index 4f572a3..865c1c8 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -175,7 +175,7 @@ func newInitCmd() *cobra.Command { } cmd.Flags().StringArrayVar(&agentFlags, "agent", nil, - "non-interactive: initialize and install hooks + MCP config + usage hint for the given agent id (repeatable; e.g. claude-code, cursor, codex-cli)") + "non-interactive: initialize and install hooks + MCP config + usage hint for the given agent id (repeatable; e.g. claude-code, cursor, codex-cli, copilot)") cmd.Flags().StringVar(&projectFlag, "project", "", "project root to initialize (default: current directory; env: ARCHCORE_PROJECT_ROOT)") return cmd diff --git a/internal/agents/copilot.go b/internal/agents/copilot.go index e961bbd..f20117f 100644 --- a/internal/agents/copilot.go +++ b/internal/agents/copilot.go @@ -2,7 +2,20 @@ package agents import "path/filepath" -var copilotMCPPath = filepath.Join(".vscode", "mcp.json") +// copilotMCPPath is the workspace-root .mcp.json — the ONLY project-level MCP +// source Copilot CLI reads. It is deliberately the same file claude-code +// writes: both hosts key on "mcpServers" and accept a bare {command, args} +// stdio entry, so one file serves both and writeMCPConfig merges idempotently. +// +// Not .vscode/mcp.json (what this agent wrote before): Copilot CLI dropped +// that source in v1.0.37 (github/copilot-cli#3019), so it is dead config for +// the CLI and belongs to VS Code alone — a surface copilot-adapter-design.adr +// explicitly scopes out. Not .github/mcp.json either: the config-dir docs list +// it, but it has never been read as a workspace source +// (github/copilot-cli#1886, still open; confirmed by the maintainer closing +// #1291). Copilot CLI discovers .mcp.json from the working directory up to the +// git root, so a repo-root file covers monorepo layouts too. +const copilotMCPPath = ".mcp.json" func copilotAgent() *Agent { return &Agent{ @@ -12,7 +25,7 @@ func copilotAgent() *Agent { return filepath.Join(baseDir, copilotMCPPath) }, WriteMCPConfig: func(baseDir string) error { - return WriteVSCodeMCPJSON(filepath.Join(baseDir, copilotMCPPath)) + return WriteStandardMCPJSON(filepath.Join(baseDir, copilotMCPPath)) }, DetectFn: func(baseDir string) bool { return fileExists(filepath.Join(baseDir, ".github", "copilot-instructions.md")) diff --git a/internal/agents/copilot_test.go b/internal/agents/copilot_test.go index 6c4ea4d..2a94a59 100644 --- a/internal/agents/copilot_test.go +++ b/internal/agents/copilot_test.go @@ -7,8 +7,9 @@ import ( ) // TestCopilotAgent_MCPWiring pins the registry closures for Copilot: a typo in -// the .vscode/mcp.json wiring would otherwise ship green because only the -// shared helper was tested, not the per-agent hookup. +// the .mcp.json wiring would otherwise ship green because only the shared +// helper was tested, not the per-agent hookup. The path itself (and the two +// wrong paths it must never write) is covered in mcp_helpers_test.go. func TestCopilotAgent_MCPWiring(t *testing.T) { t.Parallel() base := t.TempDir() @@ -17,7 +18,7 @@ func TestCopilotAgent_MCPWiring(t *testing.T) { t.Fatal("copilot agent not registered") } - wantPath := filepath.Join(base, ".vscode", "mcp.json") + wantPath := filepath.Join(base, ".mcp.json") if got := a.MCPConfigPath(base); got != wantPath { t.Errorf("MCPConfigPath = %q, want %q", got, wantPath) } diff --git a/internal/agents/mcp_helpers.go b/internal/agents/mcp_helpers.go index 0039e21..f328ec6 100644 --- a/internal/agents/mcp_helpers.go +++ b/internal/agents/mcp_helpers.go @@ -16,29 +16,6 @@ type mcpServerEntry struct { Args []string `json:"args"` } -// vscodeMCPEntry represents an MCP server entry in VS Code format (used by Copilot). -type vscodeMCPEntry struct { - Type string `json:"type"` - Command string `json:"command"` - Args []string `json:"args"` -} - -// corruptPolicy controls what happens when the target file exists but is not -// valid strict JSON. -type corruptPolicy int - -const ( - // corruptBackupAndReset backs the original up as .bak with a visible - // warning and starts fresh (backup-invalid-configs.adr.md). The install - // aborts if the backup itself cannot be written. - corruptBackupAndReset corruptPolicy = iota - // corruptSkipInstall leaves the file untouched and prints manual install - // instructions. Used for JSONC-capable targets (.vscode/mcp.json), where - // "invalid strict JSON" is usually a perfectly valid JSONC config whose - // other MCP servers must not be silently replaced. - corruptSkipInstall -) - // mcpWriteMode controls behavior when an archcore entry already exists. type mcpWriteMode int @@ -56,35 +33,21 @@ const ( // It merges an "archcore" entry under serversKey, round-tripping everything // else (unknown keys, other servers, key order) as opaque RawMessage, and // writes atomically. Returns whether the file was changed. -func writeMCPConfig(filePath, serversKey string, entry any, policy corruptPolicy, mode mcpWriteMode) (bool, error) { +func writeMCPConfig(filePath, serversKey string, entry any, mode mcpWriteMode) (bool, error) { dir := filepath.Dir(filePath) if err := os.MkdirAll(dir, 0o755); err != nil { return false, fmt.Errorf("creating directory %s: %w", dir, err) } - var doc *jsonfile.Doc - switch policy { - case corruptBackupAndReset: - var backedUp bool - var err error - doc, backedUp, err = jsonfile.ReadOrBackup(filePath) - if err != nil { - return false, err - } - if backedUp { - fmt.Println(display.WarnLine(fmt.Sprintf("Corrupted %s backed up, starting fresh", filePath))) - } - default: // corruptSkipInstall - var err error - doc, err = jsonfile.Read(filePath) - if err != nil { - entryJSON, _ := json.Marshal(entry) - fmt.Println(display.WarnLine(fmt.Sprintf( - "%s is not valid strict JSON (VS Code configs may contain comments) — left untouched", filePath))) - fmt.Println(display.HintLine(fmt.Sprintf( - "Add archcore manually under %q: \"archcore\": %s", serversKey, entryJSON))) - return false, nil - } + // A target that exists but is not valid strict JSON is backed up as .bak + // with a visible warning and rewritten fresh (backup-invalid-configs.adr). + // The install aborts if the backup itself cannot be written. + doc, backedUp, err := jsonfile.ReadOrBackup(filePath) + if err != nil { + return false, err + } + if backedUp { + fmt.Println(display.WarnLine(fmt.Sprintf("Corrupted %s backed up, starting fresh", filePath))) } servers := jsonfile.NewDoc() @@ -175,49 +138,32 @@ var ( Command: "archcore", Args: []string{"mcp", "--project", "${workspaceFolder}"}, } - copilotMCPEntry = vscodeMCPEntry{ - Type: "stdio", - Command: "archcore", - Args: []string{"mcp"}, - } ) // WriteStandardMCPJSON writes or merges an archcore entry into a standard // mcpServers JSON config file (used by Claude Code, Gemini CLI, Roo Code). func WriteStandardMCPJSON(filePath string) error { - _, err := writeMCPConfig(filePath, "mcpServers", standardMCPEntry, corruptBackupAndReset, mcpKeepExisting) + _, err := writeMCPConfig(filePath, "mcpServers", standardMCPEntry, mcpKeepExisting) return err } // WriteCursorMCPJSON writes or merges an archcore entry into Cursor's // .cursor/mcp.json (see cursorMCPEntry for why its args differ). func WriteCursorMCPJSON(filePath string) error { - _, err := writeMCPConfig(filePath, "mcpServers", cursorMCPEntry, corruptBackupAndReset, mcpKeepExisting) + _, err := writeMCPConfig(filePath, "mcpServers", cursorMCPEntry, mcpKeepExisting) return err } -// WriteVSCodeMCPJSON writes or merges an archcore entry into a VS Code-style -// MCP config file (uses "servers" key + "type": "stdio"), used by GitHub Copilot. -func WriteVSCodeMCPJSON(filePath string) error { - _, err := writeMCPConfig(filePath, "servers", copilotMCPEntry, corruptSkipInstall, mcpKeepExisting) - return err -} - -// ConvergeStandardMCPJSON, ConvergeCursorMCPJSON, and ConvergeVSCodeMCPJSON -// are the doctor --fix counterparts of the writers above: an existing -// archcore entry that drifted from the desired shape (e.g. a Cursor config -// written before --project ${workspaceFolder} existed) is updated in place. -// Foreign servers and unknown keys stay untouched. They report whether the -// file changed. +// ConvergeStandardMCPJSON and ConvergeCursorMCPJSON are the doctor --fix +// counterparts of the writers above: an existing archcore entry that drifted +// from the desired shape (e.g. a Cursor config written before +// --project ${workspaceFolder} existed) is updated in place. Foreign servers +// and unknown keys stay untouched. They report whether the file changed. func ConvergeStandardMCPJSON(filePath string) (bool, error) { - return writeMCPConfig(filePath, "mcpServers", standardMCPEntry, corruptBackupAndReset, mcpConverge) + return writeMCPConfig(filePath, "mcpServers", standardMCPEntry, mcpConverge) } func ConvergeCursorMCPJSON(filePath string) (bool, error) { - return writeMCPConfig(filePath, "mcpServers", cursorMCPEntry, corruptBackupAndReset, mcpConverge) -} - -func ConvergeVSCodeMCPJSON(filePath string) (bool, error) { - return writeMCPConfig(filePath, "servers", copilotMCPEntry, corruptSkipInstall, mcpConverge) + return writeMCPConfig(filePath, "mcpServers", cursorMCPEntry, mcpConverge) } diff --git a/internal/agents/mcp_helpers_test.go b/internal/agents/mcp_helpers_test.go index 27843b7..4462380 100644 --- a/internal/agents/mcp_helpers_test.go +++ b/internal/agents/mcp_helpers_test.go @@ -170,134 +170,6 @@ func TestWriteStandardMCPJSON_CreatesDirs(t *testing.T) { } } -func TestWriteVSCodeMCPJSON_NewFile(t *testing.T) { - t.Parallel() - base := t.TempDir() - filePath := filepath.Join(base, ".vscode", "mcp.json") - - if err := WriteVSCodeMCPJSON(filePath); err != nil { - t.Fatalf("WriteVSCodeMCPJSON: %v", err) - } - - data, err := os.ReadFile(filePath) - if err != nil { - t.Fatalf("ReadFile: %v", err) - } - - var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - - var servers map[string]json.RawMessage - if err := json.Unmarshal(raw["servers"], &servers); err != nil { - t.Fatalf("Unmarshal servers: %v", err) - } - - if _, ok := servers["archcore"]; !ok { - t.Error("missing 'archcore' in servers") - } - - var entry struct { - Type string `json:"type"` - Command string `json:"command"` - Args []string `json:"args"` - } - if err := json.Unmarshal(servers["archcore"], &entry); err != nil { - t.Fatalf("Unmarshal entry: %v", err) - } - if entry.Type != "stdio" { - t.Errorf("type = %q, want %q", entry.Type, "stdio") - } - if entry.Command != "archcore" { - t.Errorf("command = %q, want %q", entry.Command, "archcore") - } - if len(entry.Args) != 1 || entry.Args[0] != "mcp" { - t.Errorf("args = %v, want [mcp]", entry.Args) - } -} - -func TestWriteVSCodeMCPJSON_Idempotent(t *testing.T) { - t.Parallel() - base := t.TempDir() - filePath := filepath.Join(base, ".vscode", "mcp.json") - - if err := WriteVSCodeMCPJSON(filePath); err != nil { - t.Fatalf("first call: %v", err) - } - if err := WriteVSCodeMCPJSON(filePath); err != nil { - t.Fatalf("second call: %v", err) - } - - data, err := os.ReadFile(filePath) - if err != nil { - t.Fatalf("ReadFile: %v", err) - } - var raw map[string]json.RawMessage - if err := json.Unmarshal(data, &raw); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - var servers map[string]json.RawMessage - if err := json.Unmarshal(raw["servers"], &servers); err != nil { - t.Fatalf("Unmarshal servers: %v", err) - } - - if len(servers) != 1 { - t.Errorf("expected 1 server entry, got %d", len(servers)) - } -} - -func TestWriteVSCodeMCPJSON_MergesExisting(t *testing.T) { - t.Parallel() - base := t.TempDir() - filePath := filepath.Join(base, ".vscode", "mcp.json") - - if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - - existing := map[string]any{ - "servers": map[string]any{ - "other-tool": map[string]any{ - "type": "stdio", - "command": "other-tool", - "args": []string{"serve"}, - }, - }, - } - data, err := json.MarshalIndent(existing, "", " ") - if err != nil { - t.Fatalf("MarshalIndent: %v", err) - } - if err := os.WriteFile(filePath, data, 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - if err := WriteVSCodeMCPJSON(filePath); err != nil { - t.Fatalf("WriteVSCodeMCPJSON: %v", err) - } - - result, err2 := os.ReadFile(filePath) - if err2 != nil { - t.Fatalf("ReadFile: %v", err2) - } - var raw map[string]json.RawMessage - if err := json.Unmarshal(result, &raw); err != nil { - t.Fatalf("Unmarshal: %v", err) - } - var servers map[string]json.RawMessage - if err := json.Unmarshal(raw["servers"], &servers); err != nil { - t.Fatalf("Unmarshal servers: %v", err) - } - - if _, ok := servers["other-tool"]; !ok { - t.Error("existing 'other-tool' was lost during merge") - } - if _, ok := servers["archcore"]; !ok { - t.Error("missing 'archcore' after install") - } -} - func TestWriteStandardMCPJSON_InvalidJSON(t *testing.T) { t.Parallel() base := t.TempDir() @@ -338,37 +210,6 @@ func TestWriteStandardMCPJSON_InvalidJSON(t *testing.T) { } } -// TestWriteVSCodeMCPJSON_JSONCLeftUntouched pins the corruptSkipInstall policy -// for VS Code targets: a JSONC file (valid for VS Code, invalid strict JSON) -// must not be backed up or replaced — the user's other MCP servers survive and -// the install degrades to manual instructions with a nil error. -func TestWriteVSCodeMCPJSON_JSONCLeftUntouched(t *testing.T) { - t.Parallel() - base := t.TempDir() - filePath := filepath.Join(base, ".vscode", "mcp.json") - if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil { - t.Fatal(err) - } - original := "// user comment\n{\"servers\": {\"other\": {\"type\": \"stdio\", \"command\": \"other\"}}}\n" - if err := os.WriteFile(filePath, []byte(original), 0o644); err != nil { - t.Fatal(err) - } - - if err := WriteVSCodeMCPJSON(filePath); err != nil { - t.Fatalf("JSONC file must not fail the install: %v", err) - } - data, err := os.ReadFile(filePath) - if err != nil { - t.Fatal(err) - } - if string(data) != original { - t.Errorf("JSONC file must be left byte-identical:\ngot:\n%s\nwant:\n%s", data, original) - } - if _, err := os.Stat(filePath + ".bak"); !os.IsNotExist(err) { - t.Error("no .bak may be created for a JSONC file") - } -} - // TestWriteStandardMCPJSON_NullServersSection pins the `"mcpServers": null` // handling (previously a nil-map assignment panic). func TestWriteStandardMCPJSON_NullServersSection(t *testing.T) { @@ -448,3 +289,101 @@ func TestWriteStandardMCPJSON_BackupWriteFailureAborts(t *testing.T) { t.Error("original corrupted file must stay untouched when backup fails") } } + +// --- Copilot MCP path ------------------------------------------------------- +// +// Copilot CLI reads exactly one project-level MCP source: the workspace-root +// .mcp.json, keyed "mcpServers". These tests pin that, because the two paths +// this agent must NOT use are both plausible-looking and both wrong: +// .vscode/mcp.json (dropped upstream in v1.0.37, github/copilot-cli#3019) and +// .github/mcp.json (listed in GitHub's config-dir docs but never read — +// github/copilot-cli#1886). Either one silently yields a Copilot session with +// no archcore tools, which is invisible until a user trips it. + +func TestCopilotAgent_WritesWorkspaceRootMCPJSON(t *testing.T) { + t.Parallel() + base := t.TempDir() + agent := copilotAgent() + + want := filepath.Join(base, ".mcp.json") + if got := agent.MCPConfigPath(base); got != want { + t.Errorf("MCPConfigPath = %q, want %q", got, want) + } + if err := agent.WriteMCPConfig(base); err != nil { + t.Fatalf("WriteMCPConfig: %v", err) + } + + for _, dead := range []string{ + filepath.Join(base, ".vscode", "mcp.json"), + filepath.Join(base, ".github", "mcp.json"), + } { + if _, err := os.Stat(dead); !os.IsNotExist(err) { + t.Errorf("%s must not be written — Copilot CLI does not read it", dead) + } + } + + data, err := os.ReadFile(want) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + if _, ok := raw["servers"]; ok { + t.Error(`"servers" is the VS Code key; Copilot CLI reads "mcpServers"`) + } + var servers map[string]json.RawMessage + if err := json.Unmarshal(raw["mcpServers"], &servers); err != nil { + t.Fatalf("Unmarshal mcpServers: %v", err) + } + var entry struct { + Command string `json:"command"` + Args []string `json:"args"` + } + if err := json.Unmarshal(servers["archcore"], &entry); err != nil { + t.Fatalf("Unmarshal entry: %v", err) + } + if entry.Command != "archcore" || len(entry.Args) != 1 || entry.Args[0] != "mcp" { + t.Errorf("entry = %+v, want {archcore [mcp]}", entry) + } +} + +// Copilot and claude-code deliberately target the SAME file. Wiring both must +// converge on one archcore entry, in either order — if it ever duplicated or +// fought, a repo wired for both hosts would flip shape on every init. +func TestCopilotAndClaudeCodeShareOneMCPEntry(t *testing.T) { + t.Parallel() + for _, order := range [][]*Agent{ + {copilotAgent(), claudeCodeAgent()}, + {claudeCodeAgent(), copilotAgent()}, + } { + base := t.TempDir() + if order[0].MCPConfigPath(base) != order[1].MCPConfigPath(base) { + t.Fatalf("expected a shared MCP path, got %q and %q", + order[0].MCPConfigPath(base), order[1].MCPConfigPath(base)) + } + for _, a := range order { + if err := a.WriteMCPConfig(base); err != nil { + t.Fatalf("%s WriteMCPConfig: %v", a.ID, err) + } + } + + data, err := os.ReadFile(filepath.Join(base, ".mcp.json")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + var servers map[string]json.RawMessage + if err := json.Unmarshal(raw["mcpServers"], &servers); err != nil { + t.Fatalf("Unmarshal mcpServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("%s then %s: expected 1 server entry, got %d", + order[0].ID, order[1].ID, len(servers)) + } + } +} diff --git a/internal/agents/opencode.go b/internal/agents/opencode.go index ba1c281..68edfa2 100644 --- a/internal/agents/opencode.go +++ b/internal/agents/opencode.go @@ -34,6 +34,6 @@ func writeOpenCodeMCPConfig(baseDir string) error { _, err := writeMCPConfig(filepath.Join(baseDir, "opencode.json"), "mcp", openCodeMCPEntry{ Type: "local", Command: []string{"archcore", "mcp"}, - }, corruptBackupAndReset, mcpKeepExisting) + }, mcpKeepExisting) return err }