fix: stop spawning a child process per file, and bound the codex version probe

internal/agents/codex/adapter.go

@@ -13,6 +13,7 @@
 package codex
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -21,10 +22,12 @@ import (
 	"reflect"
 	"slices"
 	"strings"
+	"time"
 
 	"github.com/zzet/gortex/internal/agents"
 	"github.com/zzet/gortex/internal/agents/internalutil"
 	"github.com/zzet/gortex/internal/mcp"
+	"github.com/zzet/gortex/internal/platform"
 	"github.com/zzet/gortex/internal/version"
 )
 
@@ -400,13 +403,27 @@ func codexStartupTimeoutAtLeast(value any, minimum int) bool {
 	}
 }
 
+// codexVersionProbeTimeout bounds the `codex --version` probe. A version
+// banner is instant; anything slower is a wedged binary, and without a
+// deadline the probe blocks the install indefinitely and leaks a child
+// process that never exits. Failing the probe is cheap — the caller reads
+// an error as "unknown install" and keeps direct tool exposure on.
+// Overridable so the timeout test does not have to sleep for the real one.
+var codexVersionProbeTimeout = 5 * time.Second
+
 // codexVersionOutput is a seam for hermetic adapter tests.
 var codexVersionOutput = func() ([]byte, error) {
 	path, err := exec.LookPath("codex")
 	if err != nil {
 		return nil, err
 	}
-	return exec.Command(path, "--version").Output()
+	ctx, cancel := context.WithTimeout(context.Background(), codexVersionProbeTimeout)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, path, "--version")
+	// The probe also runs from surfaces with no console of their own; without
+	// this, Windows hands the child a fresh console window.
+	platform.ConfigureBackgroundCommand(cmd)
+	return cmd.Output()
 }
 
 func codexSupportsDirectToolNamespaces() (supported bool, detectedVersion string) {

internal/agents/codex/adapter_version_probe_test.go

@@ -0,0 +1,63 @@
+package codex
+
+import (
+	"os"
+	"path/filepath"
+	"runtime"
+	"testing"
+	"time"
+)
+
+// TestCodexVersionOutput_TimesOut pins the deadline on the `codex --version`
+// probe. A wedged binary previously blocked the caller forever and left a
+// child process behind that never exited; the probe must give up and report
+// an error instead.
+func TestCodexVersionOutput_TimesOut(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("fake binary is a POSIX shell script")
+	}
+
+	dir := t.TempDir()
+	fake := filepath.Join(dir, "codex")
+	if err := os.WriteFile(fake, []byte("#!/bin/sh\nsleep 60\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", dir)
+
+	original := codexVersionProbeTimeout
+	codexVersionProbeTimeout = 100 * time.Millisecond
+	t.Cleanup(func() { codexVersionProbeTimeout = original })
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := codexVersionOutput()
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("hung probe returned success; want a timeout error")
+		}
+	case <-time.After(10 * time.Second):
+		t.Fatal("probe did not honour its deadline")
+	}
+}
+
+// TestCodexSupportsDirectToolNamespaces_TreatsProbeFailureAsCurrent documents
+// the posture the timeout relies on: a probe that fails for any reason —
+// including the new deadline — leaves direct tool exposure enabled rather
+// than silently downgrading the install.
+func TestCodexSupportsDirectToolNamespaces_TreatsProbeFailureAsCurrent(t *testing.T) {
+	original := codexVersionOutput
+	codexVersionOutput = func() ([]byte, error) { return nil, os.ErrDeadlineExceeded }
+	t.Cleanup(func() { codexVersionOutput = original })
+
+	supported, detected := codexSupportsDirectToolNamespaces()
+	if !supported {
+		t.Error("probe failure disabled direct tool namespaces")
+	}
+	if detected != "" {
+		t.Errorf("detected version = %q, want empty on probe failure", detected)
+	}
+}

internal/churn/churn.go

@@ -31,7 +31,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"os/exec"
+	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -378,9 +378,6 @@ func stripRepoPrefix(filePath, repoRoot string) string {
 	if !strings.Contains(filePath, "/") {
 		return filePath
 	}
-	if _, err := exec.LookPath("git"); err != nil {
-		return filePath
-	}
 	abs := filepath.Join(repoRoot, filePath)
 	if fileExists(abs) {
 		return filePath
@@ -394,9 +391,16 @@ func stripRepoPrefix(filePath, repoRoot string) string {
 	return filePath
 }
 
+// fileExists is split out so tests can stub it. os.Stat follows symlinks,
+// and Mode().IsRegular matches the previous `test -f` semantics without
+// spawning a process for every lookup. Shelling out here cost one child
+// process per indexed file on every enrichment pass, and was outright
+// wrong on Windows, where there is no `test` executable: every lookup
+// failed, so multi-repo paths were never stripped and the enricher
+// silently produced no churn data.
 var fileExists = func(path string) bool {
-	cmd := exec.Command("test", "-f", path)
-	return cmd.Run() == nil
+	info, err := os.Stat(path)
+	return err == nil && info.Mode().IsRegular()
 }
 
 // runGit shells out and returns trimmed stdout, or "" on error. Used

internal/churn/churn_test.go

@@ -203,3 +203,61 @@ func currentBranch(t *testing.T, dir string) string {
 	}
 	return strings.TrimSpace(string(out))
 }
+
+// TestStripRepoPrefix_NeedsNoExternalBinaries pins the fix for the
+// subprocess fan-out: resolving whether a path exists is a filesystem
+// question, and answering it must not depend on anything being on PATH.
+// The previous implementation gated on `exec.LookPath("git")` and then
+// shelled out to `test -f` once per indexed file, so on a host where
+// neither resolves — every Windows box, since there is no `test`
+// executable — the prefix was never stripped and the enricher silently
+// produced no churn data.
+func TestStripRepoPrefix_NeedsNoExternalBinaries(t *testing.T) {
+	root := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(root, "internal"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	target := filepath.Join(root, "internal", "foo.go")
+	if err := os.WriteFile(target, []byte("package internal\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	// Nothing is resolvable on PATH — the old implementation bailed here.
+	t.Setenv("PATH", "")
+
+	if got := stripRepoPrefix("myrepo/internal/foo.go", root); got != "internal/foo.go" {
+		t.Errorf("prefixed path: got %q, want %q", got, "internal/foo.go")
+	}
+	if got := stripRepoPrefix("internal/foo.go", root); got != "internal/foo.go" {
+		t.Errorf("repo-relative path: got %q, want %q", got, "internal/foo.go")
+	}
+	// A path that resolves under neither spelling is returned untouched.
+	if got := stripRepoPrefix("myrepo/internal/absent.go", root); got != "myrepo/internal/absent.go" {
+		t.Errorf("unresolvable path: got %q, want it unchanged", got)
+	}
+}
+
+// TestFileExists_MatchesTestF keeps the os.Stat replacement honest about
+// the `test -f` semantics it replaced: regular files only, symlinks
+// followed, directories and missing paths rejected.
+func TestFileExists_MatchesTestF(t *testing.T) {
+	dir := t.TempDir()
+	regular := filepath.Join(dir, "regular")
+	if err := os.WriteFile(regular, []byte("data"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if !fileExists(regular) {
+		t.Error("regular file reported missing")
+	}
+	if fileExists(dir) {
+		t.Error("directory reported as a regular file")
+	}
+	if fileExists(filepath.Join(dir, "missing")) {
+		t.Error("missing path reported as a regular file")
+	}
+
+	link := filepath.Join(dir, "regular-link")
+	if err := os.Symlink(regular, link); err == nil && !fileExists(link) {
+		t.Error("symlink to a regular file reported missing")
+	}
+}