diff --git a/.changeset/patch-stop-404-discussion-retries.md b/.changeset/patch-stop-404-discussion-retries.md new file mode 100644 index 0000000000..e879009549 --- /dev/null +++ b/.changeset/patch-stop-404-discussion-retries.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Handle 404s from `add_comment` by warning instead of retrying the request as a discussion so deleted targets are skipped cleanly. diff --git a/Dockerfile b/Dockerfile index 14bd3f16b0..8eeccbd37d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,7 +32,8 @@ WORKDIR /workspace ENTRYPOINT ["gh-aw"] # Default command runs MCP server -CMD ["mcp-server", "--cmd", "gh-aw"] +# Note: Binary path detection is automatic via os.Executable() +CMD ["mcp-server"] # Metadata labels LABEL org.opencontainers.image.source="https://github.com/github/gh-aw" diff --git a/pkg/cli/mcp_server.go b/pkg/cli/mcp_server.go index fc954e7a46..a29ea9782a 100644 --- a/pkg/cli/mcp_server.go +++ b/pkg/cli/mcp_server.go @@ -108,6 +108,18 @@ func runMCPServer(port int, cmdPath string) error { mcpLog.Print("Starting MCP server with stdio transport") } + // Determine, log, and validate the binary path only if --cmd flag is not provided + // When --cmd is provided, the user explicitly specified the binary path to use + if cmdPath == "" { + // Attempt to detect the binary path and assign it to cmdPath + // This ensures createMCPServer receives the actual binary path instead of falling back to "gh aw" + detectedPath, err := logAndValidateBinaryPath() + if err == nil && detectedPath != "" { + cmdPath = detectedPath + mcpLog.Printf("Using detected binary path: %s", cmdPath) + } + } + // Log current working directory if cwd, err := os.Getwd(); err == nil { mcpLog.Printf("Current working directory: %s", cwd) diff --git a/pkg/cli/mcp_validation.go b/pkg/cli/mcp_validation.go index aacad6d0a2..9aeeacad0f 100644 --- a/pkg/cli/mcp_validation.go +++ b/pkg/cli/mcp_validation.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "time" @@ -19,6 +20,60 @@ import ( var mcpValidationLog = logger.New("cli:mcp_validation") +// GetBinaryPath returns the path to the currently running gh-aw binary. +// This is used by the MCP server to determine where the gh-aw binary is located +// when launching itself with different arguments. +// +// Returns the absolute path to the binary, or an error if the path cannot be determined. +func GetBinaryPath() (string, error) { + // Get the path to the currently running executable + exePath, err := os.Executable() + if err != nil { + return "", fmt.Errorf("failed to get executable path: %w", err) + } + + // Resolve any symlinks to get the actual binary path + // This is important because gh extensions are typically symlinked + // Note: EvalSymlinks already returns an absolute path + resolvedPath, err := filepath.EvalSymlinks(exePath) + if err != nil { + // If we can't resolve symlinks, use the original path + mcpValidationLog.Printf("Warning: failed to resolve symlinks for %s: %v", exePath, err) + return exePath, nil + } + + return resolvedPath, nil +} + +// logAndValidateBinaryPath determines the binary path, logs it, and validates it exists. +// Returns the detected binary path and an error if the path cannot be determined or if the file doesn't exist. +// This is a helper used by both runMCPServer and validateMCPServerConfiguration. +func logAndValidateBinaryPath() (string, error) { + binaryPath, err := GetBinaryPath() + if err != nil { + mcpValidationLog.Printf("Warning: failed to get binary path: %v", err) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: failed to get binary path: %v", err))) + return "", err + } + + // Check if the binary file exists + if _, err := os.Stat(binaryPath); err != nil { + if os.IsNotExist(err) { + mcpValidationLog.Printf("ERROR: binary file does not exist at path: %s", binaryPath) + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("ERROR: binary file does not exist at path: %s", binaryPath))) + return "", fmt.Errorf("binary file does not exist at path: %s", binaryPath) + } + mcpValidationLog.Printf("Warning: failed to stat binary file at %s: %v", binaryPath, err) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Warning: failed to stat binary file at %s: %v", binaryPath, err))) + return "", err + } + + // Log the binary path for debugging + mcpValidationLog.Printf("gh-aw binary path: %s", binaryPath) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("gh-aw binary path: %s", binaryPath))) + return binaryPath, nil +} + // validateServerSecrets checks if required environment variables/secrets are available func validateServerSecrets(config parser.MCPServerConfig, verbose bool, useActionsSecrets bool) error { mcpValidationLog.Printf("Validating server secrets: server=%s, type=%s, useActionsSecrets=%v", config.Name, config.Type, useActionsSecrets) @@ -169,6 +224,17 @@ func validateServerSecrets(config parser.MCPServerConfig, verbose bool, useActio func validateMCPServerConfiguration(cmdPath string) error { mcpValidationLog.Printf("Validating MCP server configuration: cmdPath=%s", cmdPath) + // Determine, log, and validate the binary path only if --cmd flag is not provided + // When --cmd is provided, the user explicitly specified the binary path to use + if cmdPath == "" { + // Attempt to detect the binary path and assign it to cmdPath + // This ensures the validation uses the actual binary path instead of falling back to "gh aw" + detectedPath, err := logAndValidateBinaryPath() + if err == nil && detectedPath != "" { + cmdPath = detectedPath + } + } + // Try to run the status command to verify CLI is working ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() diff --git a/pkg/cli/mcp_validation_test.go b/pkg/cli/mcp_validation_test.go new file mode 100644 index 0000000000..e30daf724b --- /dev/null +++ b/pkg/cli/mcp_validation_test.go @@ -0,0 +1,61 @@ +//go:build !integration + +package cli + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetBinaryPath(t *testing.T) { + t.Run("returns non-empty path", func(t *testing.T) { + path, err := GetBinaryPath() + require.NoError(t, err, "Should get binary path without error") + assert.NotEmpty(t, path, "Binary path should not be empty") + }) + + t.Run("returns absolute path", func(t *testing.T) { + path, err := GetBinaryPath() + require.NoError(t, err, "Should get binary path without error") + assert.True(t, filepath.IsAbs(path), "Binary path should be absolute") + }) + + t.Run("returned path exists", func(t *testing.T) { + path, err := GetBinaryPath() + require.NoError(t, err, "Should get binary path without error") + + // Check if the file exists + info, err := os.Stat(path) + require.NoError(t, err, "Binary file should exist at the returned path") + assert.False(t, info.IsDir(), "Binary path should not be a directory") + }) + + t.Run("path ends with executable name", func(t *testing.T) { + path, err := GetBinaryPath() + require.NoError(t, err, "Should get binary path without error") + + // The path should end with a reasonable executable name + // During tests, it might be a test binary name + base := filepath.Base(path) + assert.NotEmpty(t, base, "Binary path should have a base name") + // Don't check for specific name as it could be the test binary + }) + + t.Run("resolves symlinks", func(t *testing.T) { + path, err := GetBinaryPath() + require.NoError(t, err, "Should get binary path without error") + + // The path should be the resolved path, not a symlink + // We can verify this by checking that EvalSymlinks returns the same path + resolved, err := filepath.EvalSymlinks(path) + if err == nil { + // If we can resolve symlinks, the path should already be resolved + assert.Equal(t, path, resolved, "Path should already be resolved (no symlinks)") + } + // If EvalSymlinks fails, that's OK - the original path is still valid + }) +} diff --git a/pkg/workflow/mcp_config_builtin.go b/pkg/workflow/mcp_config_builtin.go index 88749737ab..10753adee7 100644 --- a/pkg/workflow/mcp_config_builtin.go +++ b/pkg/workflow/mcp_config_builtin.go @@ -185,7 +185,8 @@ func renderAgenticWorkflowsMCPConfigWithOptions(yaml *strings.Builder, isLast bo if actionMode.IsDev() { // Dev mode: Use locally built Docker image which includes gh-aw binary and gh CLI - // The Dockerfile sets ENTRYPOINT ["gh-aw"] and CMD ["mcp-server", "--cmd", "gh-aw"] + // The Dockerfile sets ENTRYPOINT ["gh-aw"] and CMD ["mcp-server"] + // Binary path is automatically detected via os.Executable() // So we don't need to specify entrypoint or entrypointArgs containerImage = constants.DevModeGhAwImage entrypoint = "" // Use container's default entrypoint