diff --git a/pkg/cli/copilot_setup.go b/pkg/cli/copilot_setup.go index 3341f14717..6bd22204a1 100644 --- a/pkg/cli/copilot_setup.go +++ b/pkg/cli/copilot_setup.go @@ -137,8 +137,9 @@ func upgradeCopilotSetupSteps(verbose bool, actionMode workflow.ActionMode, vers return ensureCopilotSetupStepsWithUpgrade(verbose, actionMode, version, true) } -// ensureCopilotSetupStepsWithUpgrade creates or updates .github/workflows/copilot-setup-steps.yml -// When upgradeVersion is true, it will update existing actions/setup-cli versions +// ensureCopilotSetupStepsWithUpgrade creates .github/workflows/copilot-setup-steps.yml +// If the file already exists, it renders console instructions instead of editing +// When upgradeVersion is true and called from upgrade command, this is a special case func ensureCopilotSetupStepsWithUpgrade(verbose bool, actionMode workflow.ActionMode, version string, upgradeVersion bool) error { copilotSetupLog.Printf("Creating copilot-setup-steps.yml with action mode: %s, version: %s, upgradeVersion: %v", actionMode, version, upgradeVersion) @@ -168,9 +169,10 @@ func ensureCopilotSetupStepsWithUpgrade(verbose bool, actionMode workflow.Action (strings.Contains(contentStr, "Install gh-aw extension") && strings.Contains(contentStr, "curl -fsSL")) hasActionInstall := strings.Contains(contentStr, "actions/setup-cli") - // If we have an install step and upgradeVersion is true, attempt to upgrade the version + // If we have an install step and upgradeVersion is true, this is from upgrade command + // In this case, we still update the file for backward compatibility if (hasLegacyInstall || hasActionInstall) && upgradeVersion { - copilotSetupLog.Print("Extension install step exists, attempting version upgrade") + copilotSetupLog.Print("Extension install step exists, attempting version upgrade (upgrade command)") // Parse existing workflow var workflow Workflow @@ -209,43 +211,22 @@ func ensureCopilotSetupStepsWithUpgrade(verbose bool, actionMode workflow.Action return nil } + // File exists - render instructions instead of editing if hasLegacyInstall || hasActionInstall { - copilotSetupLog.Print("Extension install step already exists, skipping update") + copilotSetupLog.Print("Extension install step already exists, file is up to date") if verbose { fmt.Fprintf(os.Stderr, "Skipping %s (already has gh-aw extension install step)\n", setupStepsPath) } return nil } - // Parse existing workflow - var workflow Workflow - if err := yaml.Unmarshal(content, &workflow); err != nil { - return fmt.Errorf("failed to parse existing copilot-setup-steps.yml: %w", err) - } - - // Inject the extension install step - copilotSetupLog.Print("Injecting extension install step into existing file") - if err := injectExtensionInstallStep(&workflow, actionMode, version); err != nil { - return fmt.Errorf("failed to inject extension install step: %w", err) - } - - // Marshal back to YAML - updatedContent, err := yaml.Marshal(&workflow) - if err != nil { - return fmt.Errorf("failed to marshal updated workflow: %w", err) - } - - if err := os.WriteFile(setupStepsPath, updatedContent, 0600); err != nil { - return fmt.Errorf("failed to update copilot-setup-steps.yml: %w", err) - } - copilotSetupLog.Printf("Updated file with extension install step: %s", setupStepsPath) - - if verbose { - fmt.Fprintf(os.Stderr, "Updated %s with gh-aw extension install step\n", setupStepsPath) - } + // File exists but needs update - render instructions + copilotSetupLog.Print("File exists without install step, rendering update instructions instead of editing") + renderCopilotSetupUpdateInstructions(setupStepsPath, actionMode, version) return nil } + // File doesn't exist - create it if err := os.WriteFile(setupStepsPath, []byte(generateCopilotSetupStepsYAML(actionMode, version)), 0600); err != nil { return fmt.Errorf("failed to write copilot-setup-steps.yml: %w", err) } @@ -254,6 +235,38 @@ func ensureCopilotSetupStepsWithUpgrade(verbose bool, actionMode workflow.Action return nil } +// renderCopilotSetupUpdateInstructions renders console instructions for updating copilot-setup-steps.yml +func renderCopilotSetupUpdateInstructions(filePath string, actionMode workflow.ActionMode, version string) { + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, "%s %s\n", + "ℹ", + fmt.Sprintf("Existing file detected: %s", filePath)) + fmt.Fprintln(os.Stderr) + fmt.Fprintln(os.Stderr, "To enable GitHub Copilot Agent integration, please add the following steps") + fmt.Fprintln(os.Stderr, "to the 'copilot-setup-steps' job in your .github/workflows/copilot-setup-steps.yml file:") + fmt.Fprintln(os.Stderr) + + // Determine the action reference + actionRef := "@main" + if actionMode.IsRelease() && version != "" && version != "dev" { + actionRef = "@" + version + } + + if actionMode.IsRelease() { + fmt.Fprintln(os.Stderr, " - name: Checkout repository") + fmt.Fprintln(os.Stderr, " uses: actions/checkout@v4") + fmt.Fprintf(os.Stderr, " - name: Install gh-aw extension\n") + fmt.Fprintf(os.Stderr, " uses: github/gh-aw/actions/setup-cli%s\n", actionRef) + fmt.Fprintln(os.Stderr, " with:") + fmt.Fprintf(os.Stderr, " version: %s\n", version) + } else { + fmt.Fprintln(os.Stderr, " - name: Install gh-aw extension") + fmt.Fprintln(os.Stderr, " run: |") + fmt.Fprintln(os.Stderr, " curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash") + } + fmt.Fprintln(os.Stderr) +} + // upgradeSetupCliVersion upgrades the version in existing actions/setup-cli steps // Returns true if any upgrades were made, false otherwise func upgradeSetupCliVersion(workflow *Workflow, actionMode workflow.ActionMode, version string) (bool, error) { diff --git a/pkg/cli/copilot_setup_test.go b/pkg/cli/copilot_setup_test.go index 797a1d5a5e..39e9c9b4bc 100644 --- a/pkg/cli/copilot_setup_test.go +++ b/pkg/cli/copilot_setup_test.go @@ -70,7 +70,7 @@ func TestEnsureCopilotSetupSteps(t *testing.T) { }, }, { - name: "injects extension install into existing workflow", + name: "renders instructions for existing workflow without install step", existingWorkflow: &Workflow{ Name: "Copilot Setup Steps", On: "workflow_dispatch", @@ -93,7 +93,7 @@ func TestEnsureCopilotSetupSteps(t *testing.T) { verbose: false, wantErr: false, validateContent: func(t *testing.T, content []byte) { - // Unmarshal YAML content into Workflow struct for structured validation + // File should NOT be modified - should remain with only 2 steps var wf Workflow if err := yaml.Unmarshal(content, &wf); err != nil { t.Fatalf("Failed to unmarshal workflow YAML: %v", err) @@ -103,13 +103,14 @@ func TestEnsureCopilotSetupSteps(t *testing.T) { t.Fatalf("Expected job 'copilot-setup-steps' not found") } - // Extension install and verify steps should be injected at the beginning - if len(job.Steps) < 3 { - t.Fatalf("Expected at least 3 steps after injection (1 injected + 2 existing), got %d", len(job.Steps)) + // File should remain unchanged with only 2 existing steps + if len(job.Steps) != 2 { + t.Errorf("Expected 2 steps (file should not be modified), got %d", len(job.Steps)) } - if job.Steps[0].Name != "Install gh-aw extension" { - t.Errorf("Expected first step to be 'Install gh-aw extension', got %q", job.Steps[0].Name) + // Verify the install step was NOT injected + if job.Steps[0].Name == "Install gh-aw extension" { + t.Errorf("Expected 'Install gh-aw extension' step to NOT be injected (instructions should be rendered)") } }, }, @@ -729,34 +730,32 @@ jobs: t.Fatalf("Failed to write existing workflow: %v", err) } - // Update with release mode + // Call with release mode - should render instructions instead of modifying testVersion := "v3.0.0" err = ensureCopilotSetupSteps(false, workflow.ActionModeRelease, testVersion) if err != nil { t.Fatalf("ensureCopilotSetupSteps() failed: %v", err) } - // Read updated file + // Read file - should remain unchanged content, err := os.ReadFile(setupStepsPath) if err != nil { - t.Fatalf("Failed to read updated file: %v", err) + t.Fatalf("Failed to read file: %v", err) } contentStr := string(content) - // Verify release mode injection - if !strings.Contains(contentStr, "actions/setup-cli@v3.0.0") { - t.Errorf("Expected injected action with @v3.0.0 tag, got:\n%s", contentStr) + // Verify file was NOT modified - should remain identical to existingContent + if contentStr != existingContent { + t.Errorf("Expected file to remain unchanged (instructions should be rendered instead), got:\n%s", contentStr) } - if !strings.Contains(contentStr, "version: v3.0.0") { - t.Errorf("Expected version: v3.0.0 parameter, got:\n%s", contentStr) - } - if !strings.Contains(contentStr, "actions/checkout@v4") { - t.Errorf("Expected checkout step to be injected") + + // Verify the install step was NOT injected + if strings.Contains(contentStr, "actions/setup-cli") { + t.Errorf("Expected 'actions/setup-cli' to NOT be injected (instructions should be rendered)") } - // Verify original step is preserved - if !strings.Contains(contentStr, "Some other step") { - t.Errorf("Expected original step to be preserved") + if strings.Contains(contentStr, "Install gh-aw extension") { + t.Errorf("Expected 'Install gh-aw extension' step to NOT be injected (instructions should be rendered)") } } @@ -796,26 +795,31 @@ jobs: t.Fatalf("Failed to write existing workflow: %v", err) } - // Update with dev mode + // Call with dev mode - should render instructions instead of modifying err = ensureCopilotSetupSteps(false, workflow.ActionModeDev, "dev") if err != nil { t.Fatalf("ensureCopilotSetupSteps() failed: %v", err) } - // Read updated file + // Read file - should remain unchanged content, err := os.ReadFile(setupStepsPath) if err != nil { - t.Fatalf("Failed to read updated file: %v", err) + t.Fatalf("Failed to read file: %v", err) } contentStr := string(content) - // Verify dev mode injection - if !strings.Contains(contentStr, "curl -fsSL") { - t.Errorf("Expected curl command in dev mode") + // Verify file was NOT modified - should remain identical to existingContent + if contentStr != existingContent { + t.Errorf("Expected file to remain unchanged (instructions should be rendered instead), got:\n%s", contentStr) } - if !strings.Contains(contentStr, "install-gh-aw.sh") { - t.Errorf("Expected install-gh-aw.sh in dev mode") + + // Verify the install step was NOT injected + if strings.Contains(contentStr, "curl -fsSL") { + t.Errorf("Expected 'curl' command to NOT be injected (instructions should be rendered)") + } + if strings.Contains(contentStr, "install-gh-aw.sh") { + t.Errorf("Expected 'install-gh-aw.sh' to NOT be injected (instructions should be rendered)") } if strings.Contains(contentStr, "actions/setup-cli") { t.Errorf("Did not expect actions/setup-cli in dev mode") diff --git a/pkg/cli/init_mcp_test.go b/pkg/cli/init_mcp_test.go index d0322223ca..43130f76af 100644 --- a/pkg/cli/init_mcp_test.go +++ b/pkg/cli/init_mcp_test.go @@ -159,7 +159,7 @@ func TestInitRepository_MCP_Idempotent(t *testing.T) { } } -func TestEnsureMCPConfig_UpdatesExisting(t *testing.T) { +func TestEnsureMCPConfig_RendersInstructions(t *testing.T) { // Create a temporary directory for testing tempDir := testutil.TempDir(t, "test-*") @@ -201,7 +201,7 @@ func TestEnsureMCPConfig_UpdatesExisting(t *testing.T) { t.Fatalf("ensureMCPConfig() returned error: %v", err) } - // Verify the config now contains both servers + // Verify the config was NOT modified (file should remain unchanged) content, err := os.ReadFile(mcpConfigPath) if err != nil { t.Fatalf("Failed to read mcp.json: %v", err) @@ -212,17 +212,18 @@ func TestEnsureMCPConfig_UpdatesExisting(t *testing.T) { t.Fatalf("Failed to parse mcp.json: %v", err) } - // Check both servers exist + // Check that other-server still exists if _, exists := config.Servers["other-server"]; !exists { t.Errorf("Expected existing 'other-server' to be preserved") } - if _, exists := config.Servers["github-agentic-workflows"]; !exists { - t.Errorf("Expected 'github-agentic-workflows' server to be added") + // Check that github-agentic-workflows was NOT added (file should not be modified) + if _, exists := config.Servers["github-agentic-workflows"]; exists { + t.Errorf("Expected 'github-agentic-workflows' server to NOT be added (should render instructions instead)") } } -func TestEnsureCopilotSetupSteps_InjectsStep(t *testing.T) { +func TestEnsureCopilotSetupSteps_RendersInstructions(t *testing.T) { // Create a temporary directory for testing tempDir := testutil.TempDir(t, "test-*") @@ -268,34 +269,22 @@ jobs: t.Fatalf("ensureCopilotSetupSteps() returned error: %v", err) } - // Verify the extension install step was injected + // Verify the file was NOT modified (should render instructions instead) content, err := os.ReadFile(setupStepsPath) if err != nil { t.Fatalf("Failed to read setup steps file: %v", err) } contentStr := string(content) - if !strings.Contains(contentStr, "Install gh-aw extension") { - t.Errorf("Expected extension install step to be injected") - } - if !strings.Contains(contentStr, "install-gh-aw.sh") { - t.Errorf("Expected extension install command to be present with bash script") - } - if !strings.Contains(contentStr, "curl -fsSL") { - t.Errorf("Expected curl command to be present") - } - - // Verify it was injected at the beginning (before other steps) - extensionIndex := strings.Index(contentStr, "Install gh-aw extension") - someStepIndex := strings.Index(contentStr, "Some step") - buildIndex := strings.Index(contentStr, "Build code") - if extensionIndex == -1 || someStepIndex == -1 || buildIndex == -1 { - t.Fatalf("Could not find expected steps in file") + // File should remain unchanged + if contentStr != customContent { + t.Errorf("Expected file to remain unchanged (should render instructions instead of modifying)") } - if extensionIndex >= someStepIndex || someStepIndex >= buildIndex { - t.Errorf("Extension install step not in correct position (should be at beginning, before other steps)") + // Verify extension install step was NOT injected + if strings.Contains(contentStr, "Install gh-aw extension") { + t.Errorf("Expected extension install step to NOT be injected (should render instructions instead)") } } diff --git a/pkg/cli/mcp_config_file.go b/pkg/cli/mcp_config_file.go index 36ff655c20..f6aaba67e1 100644 --- a/pkg/cli/mcp_config_file.go +++ b/pkg/cli/mcp_config_file.go @@ -23,9 +23,10 @@ type MCPConfig struct { Servers map[string]VSCodeMCPServer `json:"servers"` } -// ensureMCPConfig creates or updates .vscode/mcp.json with gh-aw MCP server configuration +// ensureMCPConfig creates .vscode/mcp.json with gh-aw MCP server configuration +// If the file already exists, it renders console instructions instead of editing func ensureMCPConfig(verbose bool) error { - mcpConfigLog.Print("Creating or updating .vscode/mcp.json") + mcpConfigLog.Print("Creating .vscode/mcp.json") // Create .vscode directory if it doesn't exist vscodeDir := ".vscode" @@ -36,18 +37,6 @@ func ensureMCPConfig(verbose bool) error { mcpConfigPath := filepath.Join(vscodeDir, "mcp.json") - // Read existing config if it exists - var config MCPConfig - if data, err := os.ReadFile(mcpConfigPath); err == nil { - mcpConfigLog.Printf("Reading existing config from: %s", mcpConfigPath) - if err := json.Unmarshal(data, &config); err != nil { - return fmt.Errorf("failed to parse existing mcp.json: %w", err) - } - } else { - mcpConfigLog.Print("No existing config found, creating new one") - config.Servers = make(map[string]VSCodeMCPServer) - } - // Add or update gh-aw MCP server configuration ghAwServerName := "github-agentic-workflows" ghAwConfig := VSCodeMCPServer{ @@ -56,22 +45,40 @@ func ensureMCPConfig(verbose bool) error { CWD: "${workspaceFolder}", } - // Check if the server is already configured - if existingConfig, exists := config.Servers[ghAwServerName]; exists { - mcpConfigLog.Printf("Server '%s' already exists in config", ghAwServerName) - // Check if configuration is different - existingJSON, _ := json.Marshal(existingConfig) - newJSON, _ := json.Marshal(ghAwConfig) - if string(existingJSON) == string(newJSON) { - mcpConfigLog.Print("Configuration is identical, skipping update") - if verbose { - fmt.Fprintf(os.Stderr, "MCP server '%s' already configured in %s\n", ghAwServerName, mcpConfigPath) + // Check if file already exists + if data, err := os.ReadFile(mcpConfigPath); err == nil { + mcpConfigLog.Printf("File already exists: %s", mcpConfigPath) + + // Parse existing config + var config MCPConfig + if err := json.Unmarshal(data, &config); err != nil { + return fmt.Errorf("failed to parse existing mcp.json: %w", err) + } + + // Check if the server is already configured correctly + if existingConfig, exists := config.Servers[ghAwServerName]; exists { + existingJSON, _ := json.Marshal(existingConfig) + newJSON, _ := json.Marshal(ghAwConfig) + if string(existingJSON) == string(newJSON) { + mcpConfigLog.Print("Configuration is identical, skipping") + if verbose { + fmt.Fprintf(os.Stderr, "MCP server '%s' already configured in %s\n", ghAwServerName, mcpConfigPath) + } + return nil } - return nil } - mcpConfigLog.Print("Configuration differs, updating") + + // File exists but needs update - render instructions instead of editing + mcpConfigLog.Print("File exists, rendering update instructions instead of editing") + renderMCPConfigUpdateInstructions(mcpConfigPath, ghAwServerName, ghAwConfig) + return nil } + // File doesn't exist - create it + mcpConfigLog.Print("No existing config found, creating new one") + config := MCPConfig{ + Servers: make(map[string]VSCodeMCPServer), + } config.Servers[ghAwServerName] = ghAwConfig // Write config file with proper indentation @@ -83,7 +90,27 @@ func ensureMCPConfig(verbose bool) error { if err := os.WriteFile(mcpConfigPath, data, 0644); err != nil { return fmt.Errorf("failed to write mcp.json: %w", err) } - mcpConfigLog.Printf("Wrote config to: %s", mcpConfigPath) + mcpConfigLog.Printf("Created new file: %s", mcpConfigPath) return nil } + +// renderMCPConfigUpdateInstructions renders console instructions for updating .vscode/mcp.json +func renderMCPConfigUpdateInstructions(filePath, serverName string, serverConfig VSCodeMCPServer) { + fmt.Fprintln(os.Stderr) + fmt.Fprintf(os.Stderr, "%s %s\n", + "ℹ", + fmt.Sprintf("Existing file detected: %s", filePath)) + fmt.Fprintln(os.Stderr) + fmt.Fprintln(os.Stderr, "To enable GitHub Copilot Agent MCP server integration, please add the following") + fmt.Fprintln(os.Stderr, "to the \"servers\" section of your .vscode/mcp.json file:") + fmt.Fprintln(os.Stderr) + + // Generate the JSON to add + serverJSON, _ := json.MarshalIndent(map[string]VSCodeMCPServer{ + serverName: serverConfig, + }, "", " ") + + fmt.Fprintln(os.Stderr, string(serverJSON)) + fmt.Fprintln(os.Stderr) +} diff --git a/pkg/cli/mcp_config_file_test.go b/pkg/cli/mcp_config_file_test.go index 02da491171..220f11a4c5 100644 --- a/pkg/cli/mcp_config_file_test.go +++ b/pkg/cli/mcp_config_file_test.go @@ -43,7 +43,7 @@ func TestEnsureMCPConfig(t *testing.T) { }, }, { - name: "adds to existing config without gh-aw server", + name: "renders instructions for existing config without gh-aw server", existingConfig: &MCPConfig{ Servers: map[string]VSCodeMCPServer{ "other-server": { @@ -55,14 +55,16 @@ func TestEnsureMCPConfig(t *testing.T) { verbose: true, wantErr: false, validateContent: func(t *testing.T, config *MCPConfig) { - if len(config.Servers) != 2 { - t.Errorf("Expected 2 servers, got %d", len(config.Servers)) + // File should NOT be modified - should remain with only 1 server + if len(config.Servers) != 1 { + t.Errorf("Expected 1 server (file should not be modified), got %d", len(config.Servers)) } if _, exists := config.Servers["other-server"]; !exists { t.Error("Expected existing other-server to be preserved") } - if _, exists := config.Servers["github-agentic-workflows"]; !exists { - t.Error("Expected github-agentic-workflows server to be added") + // gh-aw server should NOT be added (instructions rendered instead) + if _, exists := config.Servers["github-agentic-workflows"]; exists { + t.Error("Expected github-agentic-workflows server to NOT be added (instructions should be rendered)") } }, }, @@ -86,7 +88,7 @@ func TestEnsureMCPConfig(t *testing.T) { }, }, { - name: "updates existing config with different settings", + name: "renders instructions for existing config with different settings", existingConfig: &MCPConfig{ Servers: map[string]VSCodeMCPServer{ "github-agentic-workflows": { @@ -98,12 +100,13 @@ func TestEnsureMCPConfig(t *testing.T) { verbose: false, wantErr: false, validateContent: func(t *testing.T, config *MCPConfig) { + // File should NOT be modified - old settings should remain server := config.Servers["github-agentic-workflows"] - if server.Command != "gh" { - t.Errorf("Expected command to be updated to 'gh', got %q", server.Command) + if server.Command != "old-command" { + t.Errorf("Expected command to remain 'old-command' (file should not be modified), got %q", server.Command) } - if server.CWD != "${workspaceFolder}" { - t.Errorf("Expected CWD to be updated, got %q", server.CWD) + if len(server.Args) != 1 || server.Args[0] != "old-arg" { + t.Errorf("Expected args to remain ['old-arg'] (file should not be modified), got %v", server.Args) } }, },