fix(skills): retry pip installs without --break-system-packages when unsupported

pip >= 23.0 added the PEP 668 --break-system-packages flag. On older pip
builds (e.g. macOS Command Line Tools Python) and on the pip `list`
subcommand of every version, the flag is rejected with
"no such option: --break-system-packages", breaking skill dependency
install/check paths entirely.

- dep_installer: route pip installs through pipRunInstall, which retries
  once without the flag when pip rejects it (issue #956)
- pip_update_executor: same retry for the upgrade path
- pip_update_checker: drop the flag from `pip3 list --outdated` - list is
  read-only, PEP 668 never applies, and no pip accepts the flag there
- add pip_flags.go helpers + unit tests, and legacy-pip integration tests
  reproducing the original failure

Fixes #956

internal/skills/dep_installer.go

@@ -106,8 +106,9 @@ func InstallSingleDep(ctx context.Context, dep string) (bool, string) {
 			}
 			defer release()
 		}
-		cmd := exec.CommandContext(ctx, "pip3", "install", "--no-cache-dir", "--break-system-packages", pkg)
-		out, err := cmd.CombinedOutput()
+		// pipRunInstall retries without the PEP 668 flag when pip rejects it
+		// (pip < 23.0, see issue #956).
+		out, err := pipRunInstall(ctx, []string{"install", "--no-cache-dir", pipBreakSystemPackagesFlag, pkg})
 		if err != nil {
 			msg := fmt.Sprintf("%s: %v", strings.TrimSpace(string(out)), err)
 			slog.Error("skills: dep install failed", "dep", dep, "error", msg)
@@ -183,8 +184,9 @@ func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string)
 		slog.Info("skills: installing pip packages", "pkgs", pipPkgs)
 		var successful []string
 		for _, pkg := range pipPkgs {
-			cmd := exec.CommandContext(ctx, "pip3", "install", "--no-cache-dir", "--break-system-packages", pkg)
-			if out, err := cmd.CombinedOutput(); err != nil {
+			// pipRunInstall retries without the PEP 668 flag when pip rejects it.
+			out, err := pipRunInstall(ctx, []string{"install", "--no-cache-dir", pipBreakSystemPackagesFlag, pkg})
+			if err != nil {
 				result.Errors = append(result.Errors, fmt.Sprintf("pip %s: %s (%v)", pkg, strings.TrimSpace(string(out)), err))
 				if hint := pipBuildFailHint(pkg, string(out)); hint != "" {
 					slog.Warn("skills: dep install hint", "pkg", pkg, "hint", hint)

internal/skills/pip_flags.go

@@ -0,0 +1,49 @@
+package skills
+
+import (
+	"bytes"
+	"context"
+	"os/exec"
+)
+
+// pipBreakSystemPackagesFlag is the PEP 668 opt-out flag accepted by pip >= 23.0.
+// Older pip builds (and the `list` subcommand on every pip) reject it with
+// "no such option: --break-system-packages", which breaks skill dependency
+// installs on macOS Command Line Tools Python and other legacy environments.
+const pipBreakSystemPackagesFlag = "--break-system-packages"
+
+// pipRejectsBSPFlag reports whether pip's combined output indicates the
+// --break-system-packages flag was rejected as an unknown option. pip < 23.0
+// predates PEP 668 and emits exactly this error (see issue #956).
+func pipRejectsBSPFlag(combinedOut []byte) bool {
+	return bytes.Contains(combinedOut, []byte("no such option: "+pipBreakSystemPackagesFlag))
+}
+
+// dropPipBSPFlag returns args with every --break-system-packages occurrence
+// removed, preserving order of the remaining tokens. Used to retry an install
+// on pip builds that do not support the flag.
+func dropPipBSPFlag(args []string) []string {
+	out := make([]string, 0, len(args))
+	for _, a := range args {
+		if a == pipBreakSystemPackagesFlag {
+			continue
+		}
+		out = append(out, a)
+	}
+	return out
+}
+
+// pipRunInstall runs `pip3 <args...>` where args includes the PEP 668
+// --break-system-packages flag. If pip rejects the flag as an unknown option
+// (pip < 23.0, issue #956), the command is retried once without it. Returns
+// the combined output of the final attempt. Success-path overhead is zero
+// extra subprocesses.
+func pipRunInstall(ctx context.Context, args []string) ([]byte, error) {
+	cmd := exec.CommandContext(ctx, pipBinary, args...)
+	out, err := cmd.CombinedOutput()
+	if err != nil && pipRejectsBSPFlag(out) {
+		cmd = exec.CommandContext(ctx, pipBinary, dropPipBSPFlag(args)...)
+		out, err = cmd.CombinedOutput()
+	}
+	return out, err
+}

internal/skills/pip_flags_test.go

@@ -0,0 +1,166 @@
+package skills
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+)
+
+func TestPipRejectsBSPFlag(t *testing.T) {
+	cases := []struct {
+		name string
+		out  string
+		want bool
+	}{
+		{"exact error", "no such option: --break-system-packages", true},
+		{"error with context", "Usage: pip3 install\n\nno such option: --break-system-packages", true},
+		{"unrelated error", "ERROR: Could not find a version that satisfies the requirement", false},
+		{"empty", "", false},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := pipRejectsBSPFlag([]byte(c.out)); got != c.want {
+				t.Errorf("pipRejectsBSPFlag(%q) = %v, want %v", c.out, got, c.want)
+			}
+		})
+	}
+}
+
+func TestDropPipBSPFlag(t *testing.T) {
+	in := []string{"install", "--upgrade", "--no-cache-dir", "--break-system-packages", "--upgrade-strategy", "only-if-needed", "--pre", "pkg"}
+	got := dropPipBSPFlag(in)
+	want := []string{"install", "--upgrade", "--no-cache-dir", "--upgrade-strategy", "only-if-needed", "--pre", "pkg"}
+	if strings.Join(got, " ") != strings.Join(want, " ") {
+		t.Errorf("dropPipBSPFlag() = %v, want %v", got, want)
+	}
+}
+
+// writeLegacyPipScript writes a fake `pip3` that behaves like pip < 23.0: it
+// rejects --break-system-packages with the exact error from issue #956 and
+// succeeds without it. Every successful non-flag invocation appends its args
+// to argsFile so tests can assert the flag was not passed on the retry.
+func writeLegacyPipScript(t *testing.T, argsFile string) string {
+	t.Helper()
+	dir := t.TempDir()
+	path := filepath.Join(dir, "pip3")
+	body := "#!/bin/sh\n" +
+		"for a in \"$@\"; do\n" +
+		"  if [ \"$a\" = \"--break-system-packages\" ]; then\n" +
+		"    echo \"no such option: --break-system-packages\" >&2\n" +
+		"    exit 2\n" +
+		"  fi\n" +
+		"done\n" +
+		"case \"$1\" in\n" +
+		"  install|cache) echo \"$@\" >> \"" + argsFile + "\"; exit 0 ;;\n" +
+		"esac\n" +
+		"exit 0\n"
+	if runtime.GOOS == "windows" {
+		path += ".cmd"
+		body = "@echo off\r\n" +
+			"setlocal\r\n" +
+			"for %%A in (%*) do (\r\n" +
+			"  if \"%%~A\"==\"--break-system-packages\" (\r\n" +
+			"    echo no such option: --break-system-packages 1>&2\r\n" +
+			"    exit /b 2\r\n" +
+			"  )\r\n" +
+			")\r\n" +
+			"if \"%~1\"==\"install\" (\r\n" +
+			"  echo %* >> \"" + argsFile + "\"\r\n" +
+			"  exit /b 0\r\n" +
+			")\r\n" +
+			"if \"%~1\"==\"cache\" (\r\n" +
+			"  echo %* >> \"" + argsFile + "\"\r\n" +
+			"  exit /b 0\r\n" +
+			")\r\n" +
+			"exit /b 0\r\n"
+	}
+	if err := os.WriteFile(path, []byte(body), 0o755); err != nil {
+		t.Fatalf("write legacy pip script: %v", err)
+	}
+	return path
+}
+
+// setupLegacyPip points pipBinary/pipLookPath at a legacy pip script that
+// rejects --break-system-packages.
+func setupLegacyPip(t *testing.T, argsFile string) {
+	t.Helper()
+	scriptPath := writeLegacyPipScript(t, argsFile)
+	origBinary := pipBinary
+	origLookPath := pipLookPath
+	pipBinary = scriptPath
+	pipLookPath = func(string) (string, error) { return scriptPath, nil }
+	t.Cleanup(func() {
+		pipBinary = origBinary
+		pipLookPath = origLookPath
+	})
+}
+
+// TestInstallSingleDepPip_LegacyPipNoFlag reproduces issue #956: on a pip that
+// rejects --break-system-packages, installing a pip dep must still succeed via
+// the automatic retry without the flag.
+func TestInstallSingleDepPip_LegacyPipNoFlag(t *testing.T) {
+	argsFile := filepath.Join(t.TempDir(), "captured-args.txt")
+	setupLegacyPip(t, argsFile)
+
+	ok, msg := InstallSingleDep(context.Background(), "pip:requests")
+	if !ok {
+		t.Fatalf("InstallSingleDep failed on legacy pip: %s", msg)
+	}
+
+	captured, err := os.ReadFile(argsFile)
+	if err != nil {
+		t.Fatalf("args file not written: %v", err)
+	}
+	if strings.Contains(string(captured), "--break-system-packages") {
+		t.Fatalf("legacy pip received --break-system-packages: %q", string(captured))
+	}
+}
+
+// TestInstallDepsPip_LegacyPipNoFlag covers the batch path (InstallDeps) with
+// the same legacy pip fixture.
+func TestInstallDepsPip_LegacyPipNoFlag(t *testing.T) {
+	argsFile := filepath.Join(t.TempDir(), "captured-args.txt")
+	setupLegacyPip(t, argsFile)
+
+	res, err := InstallDeps(context.Background(), &SkillManifest{}, []string{"pip:requests", "pip:numpy"})
+	if err != nil {
+		t.Fatalf("InstallDeps returned error: %v", err)
+	}
+	if len(res.Errors) != 0 {
+		t.Fatalf("InstallDeps errors = %v, want none", res.Errors)
+	}
+	if len(res.Pip) != 2 {
+		t.Fatalf("InstallDeps installed = %v, want 2 packages", res.Pip)
+	}
+
+	captured, err := os.ReadFile(argsFile)
+	if err != nil {
+		t.Fatalf("args file not written: %v", err)
+	}
+	if strings.Contains(string(captured), "--break-system-packages") {
+		t.Fatalf("legacy pip received --break-system-packages: %q", string(captured))
+	}
+}
+
+// TestPipUpdateExecutor_LegacyPipNoFlag verifies the upgrade path also retries
+// without the flag on legacy pip.
+func TestPipUpdateExecutor_LegacyPipNoFlag(t *testing.T) {
+	argsFile := filepath.Join(t.TempDir(), "captured-args.txt")
+	setupLegacyPip(t, argsFile)
+
+	e := NewPipUpdateExecutor()
+	if err := e.Update(context.Background(), "requests", "2.31.0", nil); err != nil {
+		t.Fatalf("Update failed on legacy pip: %v", err)
+	}
+
+	captured, err := os.ReadFile(argsFile)
+	if err != nil {
+		t.Fatalf("args file not written: %v", err)
+	}
+	if strings.Contains(string(captured), "--break-system-packages") {
+		t.Fatalf("legacy pip received --break-system-packages: %q", string(captured))
+	}
+}

internal/skills/pip_update_checker.go

@@ -111,7 +111,11 @@ func (c *PipUpdateChecker) runOutdated(ctx context.Context, includePre bool) ([]
 	cctx, cancel := context.WithTimeout(ctx, 30*time.Second)
 	defer cancel()
 
-	args := []string{"list", "--outdated", "--format", "json", "--break-system-packages"}
+	// `list` is a read-only query — PEP 668 (externally-managed environments)
+	// only constrains commands that write into site-packages, and pip rejects
+	// --break-system-packages on `list` on every version (verified on pip 24).
+	// Passing it here makes the outdated check fail unconditionally.
+	args := []string{"list", "--outdated", "--format", "json"}
 	if includePre {
 		args = append(args, "--pre")
 	}

internal/skills/pip_update_executor.go

@@ -36,9 +36,8 @@ func (e *PipUpdateExecutor) Update(ctx context.Context, name, toVersion string,
 	cctx, cancel := context.WithTimeout(ctx, 5*time.Minute)
 	defer cancel()
 
-	args := []string{
-		"install", "--upgrade",
-		"--no-cache-dir", "--break-system-packages",
+	args := []string{"install", "--upgrade",
+		"--no-cache-dir", pipBreakSystemPackagesFlag,
 		"--upgrade-strategy", "only-if-needed",
 	}
 
@@ -57,18 +56,17 @@ func (e *PipUpdateExecutor) Update(ctx context.Context, name, toVersion string,
 	}
 	args = append(args, name)
 
-	cmd := exec.CommandContext(cctx, pipBinary, args...)
-	cmd.WaitDelay = 2 * time.Second
-	var stdout, stderr bytes.Buffer
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
-
 	start := time.Now()
-	runErr := cmd.Run()
+	stderr, runErr := runPipUpdateCommand(cctx, args)
+	// If pip rejects the PEP 668 flag (pip < 23.0, issue #956), retry once
+	// without it so updates keep working on legacy environments.
+	if runErr != nil && pipRejectsBSPFlag([]byte(stderr)) {
+		stderr, runErr = runPipUpdateCommand(cctx, dropPipBSPFlag(args))
+	}
 	durationMs := time.Since(start).Milliseconds()
 
 	if runErr != nil {
-		sentinel, reason := ClassifyPipStderr(stderr.String())
+		sentinel, reason := ClassifyPipStderr(stderr)
 		if sentinel == nil {
 			sentinel = fmt.Errorf("pip install failed: %w", runErr)
 		}
@@ -91,3 +89,16 @@ func (e *PipUpdateExecutor) Update(ctx context.Context, name, toVersion string,
 		"duration_ms", durationMs)
 	return nil
 }
+
+// runPipUpdateCommand runs `pip3 <args...>` and returns stderr only (it feeds
+// ClassifyPipStderr); stdout is captured and discarded. Command-level
+// WaitDelay mirrors the previous inline exec.
+func runPipUpdateCommand(ctx context.Context, args []string) (stderr string, runErr error) {
+	cmd := exec.CommandContext(ctx, pipBinary, args...)
+	cmd.WaitDelay = 2 * time.Second
+	var outBuf, errBuf bytes.Buffer
+	cmd.Stdout = &outBuf
+	cmd.Stderr = &errBuf
+	runErr = cmd.Run()
+	return errBuf.String(), runErr
+}