copilot feedback

Author: qmuntal

go/README.md

@@ -79,7 +79,7 @@ Follow these steps to embed the CLI:
 1. Run `go get -tool github.com/github/copilot-sdk/go/cmd/bundler`. This is a one-time setup step per project.
 2. Run `go tool bundler` in your build environment just before building your application.
 
-That's it! When your application calls `copilot.NewClient` without a `CLIPath` nor the `COPILOT_CLI_PATH` environment variable, the SDK will automatically install the embedded CLI to a temporary directory and use it for all operations. Subsequent calls to `copilot.NewClient`, even from different processes, will reuse the same installed CLI binary.
+That's it! When your application calls `copilot.NewClient` without a `CLIPath` nor the `COPILOT_CLI_PATH` environment variable, the SDK will automatically install the embedded CLI to a cache directory and use it for all operations.
 
 ## API Reference
 

go/client.go

@@ -103,7 +103,7 @@ type Client struct {
 //	})
 func NewClient(options *ClientOptions) *Client {
 	opts := ClientOptions{
-		CLIPath:  "copilot",
+		CLIPath:  "",
 		Cwd:      "",
 		Port:     0,
 		LogLevel: "info",
@@ -183,9 +183,6 @@ func NewClient(options *ClientOptions) *Client {
 	// Check environment variable for CLI path
 	if cliPath := os.Getenv("COPILOT_CLI_PATH"); cliPath != "" {
 		opts.CLIPath = cliPath
-	} else if embeddedPath := embeddedcli.Path(); embeddedPath != "" && opts.CLIPath == "copilot" {
-		// Use the unpacked embedded CLI if available and no custom path was set
-		opts.CLIPath = embeddedPath
 	}
 
 	client.options = opts
@@ -1317,6 +1314,15 @@ func (c *Client) verifyProtocolVersion(ctx context.Context) error {
 // This spawns the CLI server as a subprocess using the configured transport
 // mode (stdio or TCP).
 func (c *Client) startCLIServer(ctx context.Context) error {
+	cliPath := c.options.CLIPath
+	if cliPath == "" {
+		// If no CLI path is provided, attempt to use the embedded CLI if available
+		cliPath = embeddedcli.Path()
+	}
+	if cliPath == "" {
+		// Default to "copilot" in PATH if no embedded CLI is available and no custom path is set
+		cliPath = "copilot"
+	}
 	args := []string{"--headless", "--no-auto-update", "--log-level", c.options.LogLevel}
 
 	// Choose transport mode
@@ -1343,10 +1349,10 @@ func (c *Client) startCLIServer(ctx context.Context) error {
 
 	// If CLIPath is a .js file, run it with node
 	// Note we can't rely on the shebang as Windows doesn't support it
-	command := c.options.CLIPath
-	if strings.HasSuffix(c.options.CLIPath, ".js") {
+	command := cliPath
+	if strings.HasSuffix(cliPath, ".js") {
 		command = "node"
-		args = append([]string{c.options.CLIPath}, args...)
+		args = append([]string{cliPath}, args...)
 	}
 
 	c.process = exec.CommandContext(ctx, command, args...)

go/cmd/bundler/main.go

@@ -406,7 +406,10 @@ func downloadCLIBinary(npmPlatform, binaryName, cliVersion, destDir string) (str
 		}
 	}
 
-	stat, _ := os.Stat(binaryPath)
+	stat, err := os.Stat(binaryPath)
+	if err != nil {
+		return "", fmt.Errorf("failed to stat binary: %w", err)
+	}
 	sizeMB := float64(stat.Size()) / 1024 / 1024
 	fmt.Printf("Downloaded %s (%.1f MB)\n", binaryName, sizeMB)
 

go/embeddedcli/installer.go

@@ -5,7 +5,7 @@ import "github.com/github/copilot-sdk/go/internal/embeddedcli"
 // Config defines the inputs used to install and locate the embedded Copilot CLI.
 //
 // Cli and CliHash are required. If Dir is empty, the CLI is installed into the
-// system temp directory. Version is used to suffix the installed binary name to
+// system cache directory. Version is used to suffix the installed binary name to
 // allow multiple versions to coexist. License, when provided, is written next
 // to the installed binary.
 type Config = embeddedcli.Config

go/internal/embeddedcli/embeddedcli.go

@@ -18,7 +18,7 @@ import (
 // Config defines the inputs used to install and locate the embedded Copilot CLI.
 //
 // Cli and CliHash are required. If Dir is empty, the CLI is installed into the
-// system temp directory. Version is used to suffix the installed binary name to
+// system cache directory. Version is used to suffix the installed binary name to
 // allow multiple versions to coexist. License, when provided, is written next
 // to the installed binary.
 type Config struct {
@@ -35,6 +35,9 @@ func Setup(cfg Config) {
 	if cfg.Cli == nil {
 		panic("Cli reader is required")
 	}
+	if len(cfg.CliHash) != sha256.Size {
+		panic(fmt.Sprintf("CliHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(cfg.CliHash)))
+	}
 	setupMu.Lock()
 	defer setupMu.Unlock()
 	if setupDone {
@@ -81,7 +84,12 @@ func install() (path string) {
 	}
 	installDir := config.Dir
 	if installDir == "" {
-		installDir = os.TempDir()
+		var err error
+		if installDir, err = os.UserCacheDir(); err != nil {
+			// Fall back to temp dir if UserCacheDir is unavailable
+			installDir = os.TempDir()
+		}
+		installDir = filepath.Join(installDir, "copilot-sdk")
 	}
 	path, err := installAt(installDir)
 	if err != nil {
@@ -131,6 +139,9 @@ func installAt(installDir string) (string, error) {
 	if err1 := f.Close(); err1 != nil && err == nil {
 		err = err1
 	}
+	if closer, ok := config.Cli.(io.Closer); ok {
+		closer.Close()
+	}
 	if err != nil {
 		return "", fmt.Errorf("writing binary file: %w", err)
 	}

go/internal/flock/flock.go

@@ -5,7 +5,7 @@ import "os"
 // Acquire opens (or creates) the lock file at path and blocks until the lock is acquired.
 // It returns a release function to unlock and close the file.
 func Acquire(path string) (func() error, error) {
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_TRUNC, 0644)
+	f, err := os.OpenFile(path, os.O_CREATE, 0644)
 	if err != nil {
 		return nil, err
 	}