Isolate embedded CLI versions by directory

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7

Author: SteveSandersonMS

go/README.md

@@ -125,7 +125,7 @@ Resolution and requirements:
   transport when `ClientOptions.Connection` is nil. An explicit connection
   always takes precedence.
 - The entrypoint is resolved with **no `PATH` lookup**: explicit `InProcessConnection.Path`, then `COPILOT_CLI_PATH`, then the bundled embedded CLI. It must be an on-disk path.
-- The runtime library is loaded from next to the resolved entrypoint (the matching versioned `libcopilot_runtime_*` name installed for an embedded CLI, a flat package library name, or the package's `prebuilds/<platform>/runtime.node`). Start fails loudly if it cannot be found.
+- The runtime library is loaded from next to the resolved entrypoint (the natural platform library name installed for an embedded CLI or flat package, or the package's `prebuilds/<platform>/runtime.node`). Embedded CLI versions are isolated in separate cache directories so the CLI and runtime library always remain paired. Start fails loudly if the library cannot be found.
 - The runtime library is loaded once per process; loading a different library path in the same process is an error.
 
 The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`):

go/embeddedcli/installer.go

@@ -5,9 +5,9 @@ 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 cache directory. Version is used to suffix the installed binary and
-// runtime-library names so multiple versions can coexist. License, when
-// provided, is written next to the installed binary.
+// system cache directory. When Version is set, the CLI and runtime library are
+// installed into a version-specific child directory so multiple versions can
+// coexist. License, when provided, is written next to the installed binary.
 type Config = embeddedcli.Config
 
 // Setup sets the embedded GitHub Copilot CLI install configuration.

go/internal/embeddedcli/embeddedcli.go

@@ -18,14 +18,14 @@ 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 cache directory. Version is used to suffix the installed binary and
-// runtime-library names so multiple versions can coexist. License, when
-// provided, is written next to the installed binary.
+// system cache directory. When Version is set, the CLI is installed into a
+// version-specific child directory so multiple versions can coexist. License,
+// when provided, is written next to the installed binary.
 //
 // RuntimeLib and RuntimeLibHash are optional: when set, the native in-process
-// runtime library (cdylib) is installed next to the CLI binary with the same
-// version suffix so the in-process (FFI) transport can load it. They are omitted
-// for CLI packages that do not ship the native runtime.
+// runtime library (cdylib) is installed next to the CLI binary so the in-process
+// (FFI) transport can load it. They are omitted for CLI packages that do not
+// ship the native runtime.
 type Config struct {
 	Cli     io.Reader
 	CliHash []byte
@@ -123,25 +123,24 @@ func install() (path string) {
 }
 
 func installAt(installDir string) (string, error) {
-	if err := os.MkdirAll(installDir, 0755); err != nil {
-		return "", fmt.Errorf("creating install directory: %w", err)
-	}
 	version := sanitizeVersion(config.Version)
-	lockName := ".copilot-cli.lock"
 	if version != "" {
-		lockName = fmt.Sprintf(".copilot-cli-%s.lock", version)
+		installDir = filepath.Join(installDir, version)
+	}
+	if err := os.MkdirAll(installDir, 0755); err != nil {
+		return "", fmt.Errorf("creating install directory: %w", err)
 	}
 
 	// Best effort to prevent concurrent installs.
-	if release, _ := flock.Acquire(filepath.Join(installDir, lockName)); release != nil {
+	if release, _ := flock.Acquire(filepath.Join(installDir, ".copilot-cli.lock")); release != nil {
 		defer release()
 	}
 
 	binaryName := "copilot"
 	if runtime.GOOS == "windows" {
 		binaryName += ".exe"
 	}
-	finalPath := versionedBinaryPath(installDir, binaryName, version)
+	finalPath := filepath.Join(installDir, binaryName)
 
 	if _, err := os.Stat(finalPath); err == nil {
 		existingHash, err := hashFile(finalPath)
@@ -151,6 +150,13 @@ func installAt(installDir string) (string, error) {
 		if !bytes.Equal(existingHash, config.CliHash) {
 			return "", fmt.Errorf("existing binary hash mismatch")
 		}
+		if config.RuntimeLib != nil {
+			libPath, err := installRuntimeLib(installDir)
+			if err != nil {
+				return "", err
+			}
+			runtimeLibPath = libPath
+		}
 		return finalPath, nil
 	}
 
@@ -175,11 +181,10 @@ func installAt(installDir string) (string, error) {
 		}
 	}
 
-	// Install the native in-process runtime library (if bundled) next to the CLI
-	// binary with the same version suffix. Fail closed on any hash mismatch —
-	// never place unverified native code.
+	// Install the native in-process runtime library (if bundled) next to the CLI.
+	// Fail closed on any hash mismatch; never place unverified native code.
 	if config.RuntimeLib != nil {
-		libPath, err := installRuntimeLib(installDir, version)
+		libPath, err := installRuntimeLib(installDir)
 		if err != nil {
 			return "", err
 		}
@@ -189,14 +194,14 @@ func installAt(installDir string) (string, error) {
 	return finalPath, nil
 }
 
-// installRuntimeLib writes the embedded runtime cdylib into installDir under a
-// versioned platform file name, verifying its SHA-256. It is idempotent: an
+// installRuntimeLib writes the embedded runtime cdylib into installDir under its
+// natural platform file name, verifying its SHA-256. It is idempotent: an
 // existing file with a matching hash is reused; a mismatch is a hard error.
-func installRuntimeLib(installDir, version string) (string, error) {
+func installRuntimeLib(installDir string) (string, error) {
 	if len(config.RuntimeLibHash) != sha256.Size {
 		return "", fmt.Errorf("RuntimeLibHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(config.RuntimeLibHash))
 	}
-	libPath := versionedRuntimeLibPath(installDir, naturalRuntimeLibName(), version)
+	libPath := filepath.Join(installDir, naturalRuntimeLibName())
 
 	if _, err := os.Stat(libPath); err == nil {
 		existingHash, err := hashFile(libPath)
@@ -252,27 +257,6 @@ func naturalRuntimeLibName() string {
 	}
 }
 
-// versionedBinaryPath builds the unpacked binary filename with an optional version suffix.
-func versionedBinaryPath(dir, binaryName, version string) string {
-	if version == "" {
-		return filepath.Join(dir, binaryName)
-	}
-	base := strings.TrimSuffix(binaryName, filepath.Ext(binaryName))
-	ext := filepath.Ext(binaryName)
-	return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext))
-}
-
-// versionedRuntimeLibPath builds the runtime-library filename with the same
-// optional version suffix as the embedded CLI.
-func versionedRuntimeLibPath(dir, libraryName, version string) string {
-	if version == "" {
-		return filepath.Join(dir, libraryName)
-	}
-	base := strings.TrimSuffix(libraryName, filepath.Ext(libraryName))
-	ext := filepath.Ext(libraryName)
-	return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext))
-}
-
 // sanitizeVersion makes a version string safe for filenames.
 func sanitizeVersion(version string) string {
 	if version == "" {
@@ -293,7 +277,11 @@ func sanitizeVersion(version string) string {
 			b.WriteRune('_')
 		}
 	}
-	return b.String()
+	sanitized := b.String()
+	if sanitized == "." || sanitized == ".." {
+		return strings.Repeat("_", len(sanitized))
+	}
+	return sanitized
 }
 
 // hashFile returns the SHA-256 hash of a file on disk.

go/internal/embeddedcli/embeddedcli_test.go

@@ -66,7 +66,7 @@ func TestInstallAtWritesBinaryAndLicense(t *testing.T) {
 
 	path := Path()
 
-	expectedPath := versionedBinaryPath(tempDir, binaryNameForOS(), "1.2.3")
+	expectedPath := filepath.Join(tempDir, "1.2.3", binaryNameForOS())
 	if path != expectedPath {
 		t.Fatalf("unexpected path: got %q want %q", path, expectedPath)
 	}
@@ -100,7 +100,7 @@ func TestInstallAtWritesBinaryAndLicense(t *testing.T) {
 func TestInstallAtExistingBinaryHashMismatch(t *testing.T) {
 	resetGlobals()
 	tempDir := t.TempDir()
-	binaryPath := versionedBinaryPath(tempDir, binaryNameForOS(), "")
+	binaryPath := filepath.Join(tempDir, binaryNameForOS())
 	if err := os.MkdirAll(filepath.Dir(binaryPath), 0755); err != nil {
 		t.Fatalf("mkdir: %v", err)
 	}
@@ -121,18 +121,15 @@ func TestInstallAtExistingBinaryHashMismatch(t *testing.T) {
 }
 
 func TestSanitizeVersion(t *testing.T) {
-	got := sanitizeVersion("v1.2.3+build/abc")
-	want := "v1.2.3_build_abc"
-	if got != want {
-		t.Fatalf("sanitizeVersion() = %q want %q", got, want)
-	}
-}
-
-func TestVersionedBinaryPath(t *testing.T) {
-	got := versionedBinaryPath("/tmp", "copilot.exe", "1.0.0")
-	want := filepath.Join("/tmp", "copilot_1.0.0.exe")
-	if got != want {
-		t.Fatalf("versionedBinaryPath() = %q want %q", got, want)
+	tests := map[string]string{
+		"v1.2.3+build/abc": "v1.2.3_build_abc",
+		".":                "_",
+		"..":               "__",
+	}
+	for input, want := range tests {
+		if got := sanitizeVersion(input); got != want {
+			t.Errorf("sanitizeVersion(%q) = %q want %q", input, got, want)
+		}
 	}
 }
 
@@ -168,10 +165,58 @@ func TestInstallAtAllowsMultipleRuntimeVersions(t *testing.T) {
 	if runtime1 == runtime2 {
 		t.Fatalf("Expected versioned runtime paths to differ, got %q", runtime1)
 	}
+	if got, want := filepath.Base(cli1), binaryNameForOS(); got != want {
+		t.Fatalf("First CLI filename = %q, want %q", got, want)
+	}
+	if got, want := filepath.Base(runtime1), naturalRuntimeLibName(); got != want {
+		t.Fatalf("First runtime filename = %q, want %q", got, want)
+	}
+	if got, want := filepath.Base(filepath.Dir(cli1)), "1.0.0"; got != want {
+		t.Fatalf("First CLI version directory = %q, want %q", got, want)
+	}
+	if filepath.Dir(cli1) != filepath.Dir(runtime1) {
+		t.Fatalf("CLI and runtime were installed in different directories: %q and %q", cli1, runtime1)
+	}
 	if got, err := os.ReadFile(runtime1); err != nil || string(got) != "runtime-one" {
 		t.Fatalf("Unexpected first runtime: content=%q err=%v", got, err)
 	}
 	if got, err := os.ReadFile(runtime2); err != nil || string(got) != "runtime-two" {
 		t.Fatalf("Unexpected second runtime: content=%q err=%v", got, err)
 	}
 }
+
+func TestInstallAtExistingBinaryInstallsMissingRuntime(t *testing.T) {
+	resetGlobals()
+	tempDir := t.TempDir()
+	versionDir := filepath.Join(tempDir, "1.2.3")
+	if err := os.MkdirAll(versionDir, 0755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	cliContent := []byte("cli")
+	cliPath := filepath.Join(versionDir, binaryNameForOS())
+	if err := os.WriteFile(cliPath, cliContent, 0755); err != nil {
+		t.Fatalf("write CLI: %v", err)
+	}
+	cliHash := sha256.Sum256(cliContent)
+	runtimeContent := []byte("runtime")
+	runtimeHash := sha256.Sum256(runtimeContent)
+	config = Config{
+		Cli:            bytes.NewReader(cliContent),
+		CliHash:        cliHash[:],
+		RuntimeLib:     bytes.NewReader(runtimeContent),
+		RuntimeLibHash: runtimeHash[:],
+		Version:        "1.2.3",
+	}
+
+	gotCLIPath, err := installAt(tempDir)
+	if err != nil {
+		t.Fatalf("installAt(): %v", err)
+	}
+	if gotCLIPath != cliPath {
+		t.Fatalf("installAt() = %q, want %q", gotCLIPath, cliPath)
+	}
+	if got, err := os.ReadFile(filepath.Join(versionDir, naturalRuntimeLibName())); err != nil || string(got) != "runtime" {
+		t.Fatalf("Unexpected runtime: content=%q err=%v", got, err)
+	}
+}

go/internal/ffihost/resolve.go

@@ -62,28 +62,17 @@ func PrebuildsFolder() string {
 // ResolveLibraryPath resolves the native runtime library next to the given CLI
 // entrypoint. It checks, in order:
 //
-//  1. A versioned platform library name matching a versioned embedded CLI.
-//  2. The natural platform library name next to the CLI (flat package layout).
-//  3. prebuilds/<platform>/runtime.node next to the CLI (dev/package layout).
+//  1. The natural platform library name next to the CLI (bundled/flat layout).
+//  2. prebuilds/<platform>/runtime.node next to the CLI (dev/package layout).
 //
-// Versioned embedded CLIs require their exactly matching versioned library.
-// Unversioned entrypoints fall back to the flat and package layouts.
+// It returns an error when neither exists.
 func ResolveLibraryPath(cliEntrypoint string) (string, error) {
 	abs, err := filepath.Abs(cliEntrypoint)
 	if err != nil {
 		abs = cliEntrypoint
 	}
 	dir := filepath.Dir(abs)
 
-	if versioned := versionedLibraryPathForEntrypoint(abs); versioned != "" {
-		if fileExists(versioned) {
-			return versioned, nil
-		}
-		return "", fmt.Errorf(
-			"in-process FFI runtime library matching versioned CLI %q not found at %q",
-			abs, versioned)
-	}
-
 	flat := filepath.Join(dir, NaturalLibraryName())
 	if fileExists(flat) {
 		return flat, nil
@@ -97,30 +86,11 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) {
 	}
 
 	return "", fmt.Errorf(
-		"in-process FFI runtime library not found next to %q (looked for a matching versioned library, %q, and prebuilds/%s/runtime.node); "+
+		"in-process FFI runtime library not found next to %q (looked for %q and prebuilds/%s/runtime.node); "+
 			"use a runtime package that ships the native library",
 		abs, NaturalLibraryName(), PrebuildsFolder())
 }
 
-func versionedLibraryPathForEntrypoint(cliEntrypoint string) string {
-	name := filepath.Base(cliEntrypoint)
-	stem := name
-	if strings.HasSuffix(strings.ToLower(stem), ".exe") {
-		stem = stem[:len(stem)-len(".exe")]
-	}
-	version, ok := strings.CutPrefix(stem, "copilot_")
-	if !ok || version == "" {
-		return ""
-	}
-
-	libraryName := NaturalLibraryName()
-	libraryStem := strings.TrimSuffix(libraryName, filepath.Ext(libraryName))
-	return filepath.Join(
-		filepath.Dir(cliEntrypoint),
-		libraryStem+"_"+version+filepath.Ext(libraryName),
-	)
-}
-
 func fileExists(path string) bool {
 	info, err := os.Stat(path)
 	return err == nil && !info.IsDir()