Merge pull request #6 from sahil87/260516-perf-drop-brew-from-shell-init-version

perf: drop brew probe from shell-init and version

Author: sahil-noon

docs/memory/cli/commands.md

@@ -75,7 +75,7 @@ Roster invariants:
 | `main.go` | Entry point, version variable, `translateExit`, `errSilent`, `errExitCode`. |
 | `root.go` | `newRootCmd()` — cobra root with three subcommands wired in. |
 | `tools.go` | `Tool` struct, `Roster`, `formulaPrefix`, `shellPlaceholder`. |
-| `brew.go` | Shared brew helpers used by every subcommand: `hasBrew`, `isInstalled`, `brewBinary`, `brewMissingHint`, `installBrewMissingHint`, `shllFormula`. See [update](update.md) for details. |
+| `brew.go` | Shared brew helpers used by the brew-coupled subcommands (`install`, `update`): `hasBrew`, `isInstalled`, `brewBinary`, `brewMissingHint`, `installBrewMissingHint`, `shllFormula`. `shell-init` and `version` are install-mechanism agnostic and do NOT consult brew — they detect runnable tools via `proc.ErrNotFound` from the sub-tool invocation itself. See [update](update.md) for details. |
 | `install.go` | `newInstallCmd()` + `runInstall`. See [install](install.md). |
 | `update.go` | `newUpdateCmd()` + `runUpdate`. See [update](update.md). |
 | `shell_init.go` | `newShellInitCmd()` + `runShellInit`. See [shell-init](shell-init.md). |

docs/memory/cli/shell-init.md

@@ -2,7 +2,7 @@
 
 `shll shell-init <shell>` — emits a single concatenated shell-init blob composed from every installed roster tool with shell integration.
 
-Source: `src/cmd/shll/shell_init.go`. Uses the shared brew helpers in `src/cmd/shll/brew.go` and the `Roster` from `src/cmd/shll/tools.go`.
+Source: `src/cmd/shll/shell_init.go`. Uses the `Roster` from `src/cmd/shll/tools.go`. Does **not** call brew — install-state is detected via `proc.ErrNotFound` from the sub-tool invocation itself (i.e. "is the binary on PATH?"), which is install-mechanism agnostic (brew, from-source via `just install`, etc.).
 
 ## Usage
 
@@ -15,18 +15,18 @@ A single eval line replaces what would otherwise be N per-tool eval lines (today
 
 ## Behavior contract
 
-`runShellInit(ctx, shell, stdout, stderr)` (`src/cmd/shll/shell_init.go:65`) is the implementation seam. The cobra `RunE` wrapper handles argument validation before delegating:
+`runShellInit(ctx, shell, stdout, stderr)` (`src/cmd/shll/shell_init.go:74`) is the implementation seam. The cobra `RunE` wrapper handles argument validation before delegating:
 
 1. **Missing shell argument.** No positional → return `errExitCode{code: 2, msg: "shll shell-init: missing shell. Supported: zsh, bash"}`. Exit code: **2**. stdout: empty.
 
 2. **Unsupported shell.** Argument is not in `supportedShells = []string{"zsh", "bash"}` → return `errExitCode{code: 2, msg: ...}`. Exit code: **2**. stdout: empty.
 
 3. **Composition loop.** For each tool in `Roster` (in order):
    - Skip if `len(tool.ShellInit) == 0` (tool has no shell integration).
-   - Skip silently if `!isInstalled(ctx, tool.Formula)` — Constitution V (graceful degradation).
-   - Otherwise build argv via `substituteShell(tool.ShellInit, shell)` (replaces every `"<shell>"` token with the user-supplied shell name).
+   - Build argv via `substituteShell(tool.ShellInit, shell)` (replaces every `"<shell>"` token with the user-supplied shell name).
    - Run `proc.Run(ctx, argv[0], argv[1:]...)` (capture transport).
-   - On error: write `shll shell-init: <tool>: <err>` to stderr, set `anyFailed = true`, **and skip this tool's stdout** (eval-safety — failing tool's partial output never reaches stdout). Continue with the next tool.
+   - On `proc.ErrNotFound` (binary not on PATH): skip silently — Constitution V (graceful degradation). Install-mechanism agnostic: any source-built or non-brew install of the tool participates as long as its binary is on PATH.
+   - On any other error: write `shll shell-init: <tool>: <err>` to stderr, set `anyFailed = true`, **and skip this tool's stdout** (eval-safety — failing tool's partial output never reaches stdout). Continue with the next tool.
    - On success: write the captured stdout bytes verbatim to the output writer.
 
 4. **Final exit.** If `anyFailed`, return `errSilent` (exit 1). Else return nil (exit 0).
@@ -51,7 +51,7 @@ Output is concatenated in `Roster` order. This is deterministic (Spec: Compositi
 
 ## Argv substitution
 
-`substituteShell(argv, shell)` (`src/cmd/shll/shell_init.go:98`) replaces every literal `<shell>` token with `shell`, returning a fresh slice (does not mutate the roster):
+`substituteShell(argv, shell)` (`src/cmd/shll/shell_init.go:107`) replaces every literal `<shell>` token with `shell`, returning a fresh slice (does not mutate the roster):
 
 | Tool | Roster argv | After substitution (zsh) |
 |------|-------------|--------------------------|
@@ -97,7 +97,7 @@ Covered scenarios:
 ## Cross-references
 
 - Roster definition and `<shell>` placeholder: [cli/commands](commands.md#hardcoded-tool-roster).
-- Subprocess wrapper conventions: [internal/proc](../internal/proc.md).
-- Brew detection (`isInstalled`): [cli/update](update.md#detection).
+- Subprocess wrapper conventions: [internal/proc](../internal/proc.md) — including `proc.ErrNotFound` semantics.
+- Brew detection (`isInstalled`) — used by `install` and `update` only, not here: [cli/update](update.md#detection).
 - Rc-file installer: [cli/shell-install](shell-install.md) — wraps `shll shell-init <shell>` in an `eval` line and writes it to the user's rc file (idempotent install / `--print` dry-run / `--uninstall` removal).
-- Constitution V (Graceful Degradation) — uninstalled tools omitted silently.
+- Constitution V (Graceful Degradation) — tools not on PATH are omitted silently.

docs/memory/cli/version.md

@@ -30,15 +30,16 @@ idea      not installed
 3. For each tool in `Roster` (in order), write `<tool.Name>\t<toolVersion(ctx, tool)>\n`.
 4. `w.Flush()` — propagates any write error up.
 
-`toolVersion(ctx, tool)` (`src/cmd/shll/version.go:68`) is the per-tool resolver:
+`toolVersion(ctx, tool)` (`src/cmd/shll/version.go:73`) is the per-tool resolver:
 
-1. If `!isInstalled(ctx, tool.Formula)` → return `notInstalledLabel = "not installed"`.
-2. Else create `subCtx, cancel := context.WithTimeout(ctx, versionTimeout)`. Defer cancel.
-3. Run `proc.Run(subCtx, tool.Name, "--version")` (capture transport).
-4. On any error (transport error, exit non-zero, deadline exceeded) → return `notInstalledLabel`.
-5. On success → return `normalizeVersion(string(out))`.
+1. Create `subCtx, cancel := context.WithTimeout(ctx, versionTimeout)`. Defer cancel.
+2. Run `proc.Run(subCtx, tool.Name, "--version")` (capture transport).
+3. On any error (`proc.ErrNotFound` for missing binary, exit non-zero, deadline exceeded, etc.) → return `notInstalledLabel = "not installed"`.
+4. On success → return `normalizeVersion(string(out))`.
 
-`normalizeVersion(raw string) string` (`src/cmd/shll/version.go:93`) is the single point of normalization shared by the shll row and every roster row. It is purely shape-based — there is no per-tool branching — so independent upstream `--version` standardization (e.g., tu/rk/fab-kit cleaning up their own output in parallel) is absorbed without shll code changes.
+"Installed" is detected via `proc.ErrNotFound` (binary not on PATH) rather than a brew probe — install-mechanism agnostic, and saves ~400ms per tool (no Homebrew/Ruby startup tax).
+
+`normalizeVersion(raw string) string` (`src/cmd/shll/version.go:95`) is the single point of normalization shared by the shll row and every roster row. It is purely shape-based — there is no per-tool branching — so independent upstream `--version` standardization (e.g., tu/rk/fab-kit cleaning up their own output in parallel) is absorbed without shll code changes.
 
 The normalization pipeline runs in this order on the input:
 
@@ -71,7 +72,7 @@ Properties (Design Decision #5):
 - 2s is generous (typical `--version` runs in well under 100ms).
 - Bounds worst-case `shll version` runtime to `len(Roster) * versionTimeout` ≈ 12 seconds even if every tool hangs.
 - A timeout is treated as "not installed" — we don't differentiate hung-but-installed from missing in the output. The user gets a usable table either way.
-- The deadline applies only to the `--version` invocation, not to the `brew list` probe. The probe runs against the parent ctx (typically unbounded for the CLI invocation).
+- The deadline applies only to the `--version` invocation. There is no separate install probe — installation is inferred from `proc.ErrNotFound` returned by the same `--version` call.
 
 `TestVersion_TimeoutHandling` simulates the timeout path by having the fake runner return `context.DeadlineExceeded` immediately for the targeted tool (no real wall-clock wait), then asserts the row reads `not installed` and that the test's elapsed time stays under `versionTimeout`.
 
@@ -109,6 +110,6 @@ Unit scenarios pinning the normalization contract (12 cases, all named `TestNorm
 
 ## Cross-references
 
-- Subprocess wrapper conventions: [internal/proc](../internal/proc.md).
+- Subprocess wrapper conventions: [internal/proc](../internal/proc.md) — including `proc.ErrNotFound` semantics.
 - Roster definition: [cli/commands](commands.md#hardcoded-tool-roster).
-- Brew detection (`isInstalled`): [cli/update](update.md#detection).
+- Brew detection (`isInstalled`) — used by `install` and `update` only, not here: [cli/update](update.md#detection).

src/cmd/shll/shell_init.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 
@@ -64,6 +65,11 @@ func isSupportedShell(shell string) bool {
 //     any sub-tool error output. Eval-safety only applies to stdout.
 //   - exit code is non-zero if any sub-tool's shell-init failed.
 //
+// "Installed" here means "runnable on PATH" — we attempt the sub-tool's
+// shell-init and treat proc.ErrNotFound (binary missing) as graceful skip
+// per Constitution V. This is independent of the install mechanism (brew,
+// from-source, etc.), so source-built tools are not silently dropped.
+//
 // Order is roster order (deterministic — spec requirement).
 func runShellInit(ctx context.Context, shell string, stdout, stderr io.Writer) error {
 	if ctx == nil {
@@ -74,13 +80,13 @@ func runShellInit(ctx context.Context, shell string, stdout, stderr io.Writer) e
 		if len(tool.ShellInit) == 0 {
 			continue
 		}
-		if !isInstalled(ctx, tool.Formula) {
-			// Graceful degradation — uninstalled means no output, no error.
-			continue
-		}
 		argv := substituteShell(tool.ShellInit, shell)
 		out, err := proc.Run(ctx, argv[0], argv[1:]...)
 		if err != nil {
+			if errors.Is(err, proc.ErrNotFound) {
+				// Graceful degradation — binary not on PATH means no output, no error.
+				continue
+			}
 			fmt.Fprintf(stderr, "shll shell-init: %s: %v\n", tool.Name, err)
 			anyFailed = true
 			continue

src/cmd/shll/shell_init_test.go

@@ -11,17 +11,24 @@ import (
 )
 
 // shellInitFakeBuilder constructs a fakeRunner that simulates per-tool installation
-// state and shell-init outputs. installedFormulas selects which roster formulas
-// are present; outputs maps tool argv (joined by space) to stdout, missing entries
-// produce empty stdout success.
+// state and shell-init outputs.
+//
+// installedFormulas selects which roster formulas are "installed" — for a tool
+// whose formula is absent from this map, the fake returns proc.ErrNotFound from
+// its `<tool> shell-init <shell>` invocation, mirroring real exec.LookPath
+// behavior when the binary is missing from PATH. For installed tools, outputs
+// supplies the canned stdout (missing entries default to empty stdout success).
 func shellInitFake(installedFormulas map[string]bool, outputs map[string]string, errors map[string]error) *fakeRunner {
+	// Map binary name → formula so the fake can resolve "is this tool's binary
+	// on the simulated PATH?" from the Roster.
+	formulaByName := map[string]string{}
+	for _, t := range Roster {
+		formulaByName[t.Name] = t.Formula
+	}
 	return &fakeRunner{respond: func(req proc.Request) proc.Result {
-		if req.Name == brewBinary && len(req.Args) >= 4 && req.Args[0] == "list" {
-			formula := req.Args[3]
-			if installedFormulas[formula] {
-				return proc.Result{Stdout: []byte(formula + " 1.0.0\n")}
-			}
-			return proc.Result{Err: stdErr("not installed")}
+		// Simulate ErrNotFound for tools whose formula isn't installed.
+		if formula, ok := formulaByName[req.Name]; ok && !installedFormulas[formula] {
+			return proc.Result{Err: proc.ErrNotFound}
 		}
 		key := strings.Join(append([]string{req.Name}, req.Args...), " ")
 		if e, ok := errors[key]; ok {

src/cmd/shll/version.go

@@ -62,13 +62,15 @@ func runVersion(ctx context.Context, stdout io.Writer) error {
 }
 
 // toolVersion returns the version string for a single roster tool, or
-// notInstalledLabel on any failure (binary missing, brew reports
-// not-installed, --version errors, or timeout). The returned string never
-// contains internal newlines — only the first non-empty line is reported.
+// notInstalledLabel on any failure (binary missing from PATH, --version
+// errors, or timeout). The returned string never contains internal newlines —
+// only the first non-empty line is reported.
+//
+// "Installed" here means "runnable on PATH" — proc.Run returns proc.ErrNotFound
+// when the binary is missing, and any other failure mode (non-zero exit,
+// timeout, etc.) falls under the same notInstalledLabel branch. This is
+// install-mechanism agnostic (brew, from-source, etc.).
 func toolVersion(ctx context.Context, tool Tool) string {
-	if !isInstalled(ctx, tool.Formula) {
-		return notInstalledLabel
-	}
 	subCtx, cancel := context.WithTimeout(ctx, versionTimeout)
 	defer cancel()
 	out, err := proc.Run(subCtx, tool.Name, "--version")

src/cmd/shll/version_test.go

@@ -3,7 +3,6 @@ package main
 import (
 	"bytes"
 	"context"
-	"errors"
 	"strings"
 	"testing"
 	"time"
@@ -12,15 +11,18 @@ import (
 )
 
 // versionFake constructs a fakeRunner that simulates per-tool installation and
-// version output.
+// version output. For a tool whose formula is absent from installedFormulas,
+// the fake returns proc.ErrNotFound from `<tool> --version`, mirroring real
+// exec.LookPath behavior when the binary is missing from PATH.
 func versionFake(installedFormulas map[string]bool, versions map[string]string) *fakeRunner {
+	formulaByName := map[string]string{}
+	for _, t := range Roster {
+		formulaByName[t.Name] = t.Formula
+	}
 	return &fakeRunner{respond: func(req proc.Request) proc.Result {
-		if req.Name == brewBinary && len(req.Args) >= 4 && req.Args[0] == "list" {
-			formula := req.Args[3]
-			if installedFormulas[formula] {
-				return proc.Result{Stdout: []byte(formula + " 1.0.0\n")}
-			}
-			return proc.Result{Err: errors.New("not installed")}
+		// Simulate ErrNotFound for tools whose formula isn't installed.
+		if formula, ok := formulaByName[req.Name]; ok && !installedFormulas[formula] {
+			return proc.Result{Err: proc.ErrNotFound}
 		}
 		// Match per-tool --version invocations: req.Name is the tool name,
 		// args[0] is "--version".