From c4b2e42092c0e58d03d6ec8f68101cf7c681e451 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Thu, 7 May 2026 22:57:50 +0200 Subject: [PATCH 1/8] =?UTF-8?q?feat(binary):=20onPost=20hook=20for=20binar?= =?UTF-8?q?ies=20=E2=80=94=20runs=20after=20install/update?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a per-binary post-install/update hook: a shell command that runs after a successful download, receiving B_EVENT (install|update), B_NAME, B_VERSION, and B_FILE as env vars. Non-zero exit is surfaced as a warning, not a fatal error. Skipped when the binary didn't change. b.yaml: binaries: github.com/arg-sh/argsh: onPost: argsh builtin ${B_EVENT} CLI: b install --add github.com/arg-sh/argsh --on-post 'argsh builtin ${B_EVENT}' Implementation: - LocalBinary gains OnPost (yaml:"onPost") — round-trips via BinaryList Marshal/Unmarshal. - managedKey updated so SaveConfig preserves the field but can also remove it when the user deletes it from config. - RunHook helper in pkg/binary/hook.go — exec.Command with the four B_ env vars set, runs in the project root directory. - install.go / update.go call RunHook after a successful EnsureBinary / DownloadBinary, with event="install" or "update" respectively. - --on-post flag on 'b install' feeds into addToConfig. Tests: - TestRunHook_{SetsEnvVars,EmptyIsNoOp,NonZeroExitReturnsError,RunsInDir} - TestBinaryListMarshalYAML_OnPostRoundTrip - TestAddToConfig_WithOnPost Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/binary/hook.go | 28 +++++++++++++++++++ pkg/binary/hook_test.go | 57 +++++++++++++++++++++++++++++++++++++++ pkg/binary/types.go | 6 +++++ pkg/cli/cli_extra_test.go | 30 +++++++++++++++++++++ pkg/cli/install.go | 16 +++++++++++ pkg/cli/shared.go | 9 +++++++ pkg/cli/update.go | 7 +++++ pkg/state/types.go | 5 ++++ pkg/state/types_test.go | 26 ++++++++++++++++++ pkg/state/yamlmerge.go | 2 +- 10 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 pkg/binary/hook.go create mode 100644 pkg/binary/hook_test.go diff --git a/pkg/binary/hook.go b/pkg/binary/hook.go new file mode 100644 index 0000000..448bfd3 --- /dev/null +++ b/pkg/binary/hook.go @@ -0,0 +1,28 @@ +package binary + +import ( + "fmt" + "os" + "os/exec" +) + +// RunHook executes a shell command with B_EVENT, B_NAME, B_VERSION, B_FILE +// env vars set. dir is the working directory (project root). Returns nil +// on success, the exec error on failure. Callers decide whether to treat +// the error as fatal or as a warning. +func RunHook(command, dir, event, name, version, file string, stdout, stderr *os.File) error { + if command == "" { + return nil + } + cmd := exec.Command("sh", "-c", command) + cmd.Dir = dir + cmd.Stdout = stdout + cmd.Stderr = stderr + cmd.Env = append(os.Environ(), + fmt.Sprintf("B_EVENT=%s", event), + fmt.Sprintf("B_NAME=%s", name), + fmt.Sprintf("B_VERSION=%s", version), + fmt.Sprintf("B_FILE=%s", file), + ) + return cmd.Run() +} diff --git a/pkg/binary/hook_test.go b/pkg/binary/hook_test.go new file mode 100644 index 0000000..e813301 --- /dev/null +++ b/pkg/binary/hook_test.go @@ -0,0 +1,57 @@ +package binary + +import ( + "os" + "path/filepath" + "testing" +) + +func TestRunHook_SetsEnvVars(t *testing.T) { + tmp := t.TempDir() + out := filepath.Join(tmp, "env.txt") + + // The hook writes the four B_ env vars to a file so we can inspect them. + cmd := `printf "%s\n%s\n%s\n%s" "$B_EVENT" "$B_NAME" "$B_VERSION" "$B_FILE" > ` + out + if err := RunHook(cmd, tmp, "install", "kubectl", "v1.28.0", "/usr/local/bin/kubectl", os.Stdout, os.Stderr); err != nil { + t.Fatalf("RunHook: %v", err) + } + + data, err := os.ReadFile(out) + if err != nil { + t.Fatalf("read: %v", err) + } + lines := string(data) + want := "install\nkubectl\nv1.28.0\n/usr/local/bin/kubectl" + if lines != want { + t.Errorf("env vars:\ngot: %q\nwant: %q", lines, want) + } +} + +func TestRunHook_EmptyIsNoOp(t *testing.T) { + if err := RunHook("", t.TempDir(), "install", "x", "v1", "/x", os.Stdout, os.Stderr); err != nil { + t.Errorf("empty hook should be no-op, got: %v", err) + } +} + +func TestRunHook_NonZeroExitReturnsError(t *testing.T) { + if err := RunHook("exit 1", t.TempDir(), "update", "x", "v1", "/x", os.Stdout, os.Stderr); err == nil { + t.Error("expected error on non-zero exit") + } +} + +func TestRunHook_RunsInDir(t *testing.T) { + tmp := t.TempDir() + out := filepath.Join(tmp, "pwd.txt") + cmd := "pwd > " + out + if err := RunHook(cmd, tmp, "install", "x", "v1", "/x", os.Stdout, os.Stderr); err != nil { + t.Fatalf("RunHook: %v", err) + } + data, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + got := string(data) + if got[:len(got)-1] != tmp { // trim trailing newline + t.Errorf("hook ran in %q, want %q", got, tmp) + } +} diff --git a/pkg/binary/types.go b/pkg/binary/types.go index e4af004..cd7017f 100644 --- a/pkg/binary/types.go +++ b/pkg/binary/types.go @@ -51,6 +51,7 @@ type Binary struct { AssetFilter string `json:"-"` // glob pattern to filter release assets (e.g. "argsh-so-*") SelectAsset SelectAssetFunc `json:"-"` // interactive asset selector for ambiguous matches ResolvedAsset *provider.Asset `json:"-"` // pre-resolved asset (skips matching during download) + OnPost string `json:"-"` // shell command to run after successful install/update } type LocalBinary struct { @@ -64,6 +65,11 @@ type LocalBinary struct { Alias string `json:"alias,omitempty"` // Asset is a glob pattern to filter release assets (e.g. "argsh-so-*") Asset string `json:"asset,omitempty"` + // OnPost is a shell command run after a successful install/update of this + // binary. Receives B_EVENT (install|update), B_NAME, B_VERSION, B_FILE. + // Skipped when the binary didn't change or on --dry-run. Non-zero exit is + // surfaced as a warning, not a fatal error. + OnPost string `json:"onPost,omitempty" yaml:"onPost,omitempty"` // IsProviderRef is true when Name is a provider ref (e.g. github.com/derailed/k9s) IsProviderRef bool `json:"-" yaml:"-"` } diff --git a/pkg/cli/cli_extra_test.go b/pkg/cli/cli_extra_test.go index 84110b3..96750ee 100644 --- a/pkg/cli/cli_extra_test.go +++ b/pkg/cli/cli_extra_test.go @@ -1374,6 +1374,36 @@ func TestAddToConfig_WithAlias(t *testing.T) { } } +func TestAddToConfig_WithOnPost(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, ".bin", "b.yaml") + + o := &InstallOptions{ + SharedOptions: &SharedOptions{ + IO: &streams.IO{Out: &bytes.Buffer{}, ErrOut: &bytes.Buffer{}}, + ConfigPath: configPath, + loadedConfigPath: configPath, + Config: &state.State{}, + }, + OnPost: "argsh builtin ${B_EVENT}", + } + + binaries := []*binary.Binary{ + {Name: "argsh", AutoDetect: true, ProviderRef: "github.com/arg-sh/argsh"}, + } + if err := o.addToConfig(binaries); err != nil { + t.Fatalf("addToConfig() error = %v", err) + } + + cfg, _ := state.LoadConfigFromPath(configPath) + if len(cfg.Binaries) != 1 { + t.Fatalf("got %d binaries, want 1", len(cfg.Binaries)) + } + if cfg.Binaries[0].OnPost != "argsh builtin ${B_EVENT}" { + t.Errorf("onPost = %q, want %q", cfg.Binaries[0].OnPost, "argsh builtin ${B_EVENT}") + } +} + // --- addEnvToConfig tests --- func TestAddEnvToConfig_New(t *testing.T) { diff --git a/pkg/cli/install.go b/pkg/cli/install.go index a65cac4..f323c03 100644 --- a/pkg/cli/install.go +++ b/pkg/cli/install.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "os" "sort" "strings" "sync" @@ -40,6 +41,7 @@ type InstallOptions struct { Fix bool // Pin version in b.yaml Alias string // Alias for the binary Asset string // Asset filter glob pattern + OnPost string // Shell command to run after install/update specifiedBinaries []*binary.Binary // Binaries specified on command line envInstalls []envInstall // SCP-style env installs configEnvRefs []string // env refs to sync from config @@ -93,6 +95,7 @@ func NewInstallCmd(shared *SharedOptions) *cobra.Command { cmd.Flags().BoolVar(&o.Fix, "fix", false, "Pin the specified version in b.yaml") cmd.Flags().StringVar(&o.Alias, "alias", "", "Alias for the binary") cmd.Flags().StringVar(&o.Asset, "asset", "", "Glob pattern to filter release assets (e.g. \"argsh-so-*\")") + cmd.Flags().StringVar(&o.OnPost, "on-post", "", "Shell command to run after install/update (saved to b.yaml with --add)") return cmd } @@ -149,6 +152,9 @@ func (o *InstallOptions) Complete(args []string) error { if o.Asset != "" { b.AssetFilter = o.Asset } + if o.OnPost != "" { + b.OnPost = o.OnPost + } o.specifiedBinaries = append(o.specifiedBinaries, b) } @@ -258,6 +264,13 @@ func (o *InstallOptions) installBinaries(binaries []*binary.Binary) error { err = b.EnsureBinary(false) // Don't update, just ensure } + // Run onPost hook after a successful download. + if err == nil && b.OnPost != "" { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "install", b.Name, b.Version, b.BinaryPath(), os.Stdout, os.Stderr); hookErr != nil { + fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) + } + } + name := b.Name if b.Alias != "" { name = b.Alias + " (" + color.New(color.FgYellow).Sprint(b.Name) + ")" @@ -331,6 +344,9 @@ func (o *InstallOptions) addToConfig(binaries []*binary.Binary) error { if b.AssetFilter != "" { entry.Asset = b.AssetFilter } + if o.OnPost != "" { + entry.OnPost = o.OnPost + } config.Binaries = append(config.Binaries, entry) } } diff --git a/pkg/cli/shared.go b/pkg/cli/shared.go index b741498..aee8037 100644 --- a/pkg/cli/shared.go +++ b/pkg/cli/shared.go @@ -105,6 +105,9 @@ func (o *SharedOptions) resolveBinary(lb *binary.LocalBinary) (*binary.Binary, b if lb.File != "" { b.File = lb.File } + if lb.OnPost != "" { + b.OnPost = lb.OnPost + } } return b, ok @@ -162,6 +165,9 @@ func (o *SharedOptions) GetBinary(name string) (*binary.Binary, bool) { if configEntry.Asset != "" { b.AssetFilter = configEntry.Asset } + if configEntry.OnPost != "" { + b.OnPost = configEntry.OnPost + } } return b, true } @@ -200,6 +206,9 @@ func (o *SharedOptions) GetBinariesFromConfig() []*binary.Binary { if lb.Asset != "" { b.AssetFilter = lb.Asset } + if lb.OnPost != "" { + b.OnPost = lb.OnPost + } result = append(result, b) } else if b, ok := o.resolveBinary(lb); ok { result = append(result, b) diff --git a/pkg/cli/update.go b/pkg/cli/update.go index 8ef89aa..2b81804 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -927,6 +927,13 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { downloadFailed[b.Name] = attempted && err != nil outcomeMu.Unlock() + // Run onPost hook after a successful update. + if err == nil && b.OnPost != "" { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "update", b.Name, b.Version, b.BinaryPath(), os.Stdout, os.Stderr); hookErr != nil { + fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) + } + } + doneLabel := name + " updated" if b.Alias != "" { doneLabel = b.Alias + " (" + color.New(color.FgYellow).Sprint(b.Name) + ") updated" diff --git a/pkg/state/types.go b/pkg/state/types.go index c944799..71f8790 100644 --- a/pkg/state/types.go +++ b/pkg/state/types.go @@ -372,6 +372,11 @@ func (list *BinaryList) MarshalYAML() (interface{}, error) { config["asset"] = b.Asset } + // Add post-install/update hook + if b.OnPost != "" { + config["onPost"] = b.OnPost + } + // If we have any configuration, use it; otherwise use empty struct if len(config) > 0 { result[b.Name] = config diff --git a/pkg/state/types_test.go b/pkg/state/types_test.go index 869acf3..5d6c118 100644 --- a/pkg/state/types_test.go +++ b/pkg/state/types_test.go @@ -563,6 +563,32 @@ func TestBinaryListMarshalYAML_WithAsset(t *testing.T) { } } +func TestBinaryListMarshalYAML_OnPostRoundTrip(t *testing.T) { + list := BinaryList{ + {Name: "github.com/arg-sh/argsh", OnPost: "argsh builtin ${B_EVENT}"}, + } + data, err := yaml.Marshal(&list) + if err != nil { + t.Fatalf("marshal: %v", err) + } + s := string(data) + if !strings.Contains(s, "onPost:") { + t.Errorf("marshal output missing onPost field:\n%s", s) + } + + // Unmarshal and verify round-trip. + var list2 BinaryList + if err := yaml.Unmarshal(data, &list2); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if len(list2) != 1 { + t.Fatalf("got %d binaries, want 1", len(list2)) + } + if list2[0].OnPost != "argsh builtin ${B_EVENT}" { + t.Errorf("onPost = %q, want %q", list2[0].OnPost, "argsh builtin ${B_EVENT}") + } +} + func TestBinaryListUnmarshalYAML_NilBinary(t *testing.T) { input := ` terraform: diff --git a/pkg/state/yamlmerge.go b/pkg/state/yamlmerge.go index d16dfb9..aac6e6e 100644 --- a/pkg/state/yamlmerge.go +++ b/pkg/state/yamlmerge.go @@ -37,7 +37,7 @@ func managedKey(path []string, key string) bool { case "binaries": // Matches BinaryList.MarshalYAML. switch key { - case "version", "enforced", "alias", "file", "asset": + case "version", "enforced", "alias", "file", "asset", "onPost": return true } return false From 0337f1776498c979bc70b491aeddfdc3d054d038 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 09:40:43 +0200 Subject: [PATCH 2/8] =?UTF-8?q?fix(binary):=20review=20round=201=20?= =?UTF-8?q?=E2=80=94=20hook=20only=20on=20actual=20download=20+=20io.Write?= =?UTF-8?q?r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 1: 1. RunHook signature changed from *os.File to io.Writer so callers can route through the CLI's IO streams (respecting --quiet, output capture). nil defaults to io.Discard. Added docstring noting hooks are POSIX-only (sh -c), consistent with existing env hooks. 2. install.go: track wasMissing before EnsureBinary(false) so the hook only fires when a download actually happened. Force mode always counts as downloaded. Streams routed through o.IO. 3. update.go: track 'downloaded' per branch — DownloadBinary branches set it on err==nil; EnsureBinary compares pre/post SHA to detect whether the file actually changed. Hook gated behind '!o.effectiveDryRun()' for --dry-run/--plan-json respect. Streams routed through o.IO. 4. Types.go docstring updated to match actual guarantees: only fires when on-disk binary changed, not on no-op skip or dry-run. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/binary/hook.go | 20 ++++++++++++++++---- pkg/binary/hook_test.go | 32 ++++++++++++++++++++++++-------- pkg/binary/types.go | 9 +++++---- pkg/cli/install.go | 12 ++++++++---- pkg/cli/update.go | 16 +++++++++++++--- 5 files changed, 66 insertions(+), 23 deletions(-) diff --git a/pkg/binary/hook.go b/pkg/binary/hook.go index 448bfd3..fe5ac71 100644 --- a/pkg/binary/hook.go +++ b/pkg/binary/hook.go @@ -2,18 +2,30 @@ package binary import ( "fmt" + "io" "os" "os/exec" ) // RunHook executes a shell command with B_EVENT, B_NAME, B_VERSION, B_FILE -// env vars set. dir is the working directory (project root). Returns nil -// on success, the exec error on failure. Callers decide whether to treat -// the error as fatal or as a warning. -func RunHook(command, dir, event, name, version, file string, stdout, stderr *os.File) error { +// env vars set. dir is the working directory (project root). stdout/stderr +// accept io.Writer so callers can route through the CLI's IO streams +// (respecting --quiet / output capture). nil writers default to io.Discard. +// Returns nil on success, the exec error on failure. Callers decide whether +// to treat the error as fatal or as a warning. +// +// The command runs via "sh -c" — hooks are POSIX-only. This is consistent +// with the existing env onPreSync/onPostSync hooks in pkg/env/env.go. +func RunHook(command, dir, event, name, version, file string, stdout, stderr io.Writer) error { if command == "" { return nil } + if stdout == nil { + stdout = io.Discard + } + if stderr == nil { + stderr = io.Discard + } cmd := exec.Command("sh", "-c", command) cmd.Dir = dir cmd.Stdout = stdout diff --git a/pkg/binary/hook_test.go b/pkg/binary/hook_test.go index e813301..67b12f6 100644 --- a/pkg/binary/hook_test.go +++ b/pkg/binary/hook_test.go @@ -1,6 +1,7 @@ package binary import ( + "bytes" "os" "path/filepath" "testing" @@ -10,7 +11,6 @@ func TestRunHook_SetsEnvVars(t *testing.T) { tmp := t.TempDir() out := filepath.Join(tmp, "env.txt") - // The hook writes the four B_ env vars to a file so we can inspect them. cmd := `printf "%s\n%s\n%s\n%s" "$B_EVENT" "$B_NAME" "$B_VERSION" "$B_FILE" > ` + out if err := RunHook(cmd, tmp, "install", "kubectl", "v1.28.0", "/usr/local/bin/kubectl", os.Stdout, os.Stderr); err != nil { t.Fatalf("RunHook: %v", err) @@ -20,21 +20,20 @@ func TestRunHook_SetsEnvVars(t *testing.T) { if err != nil { t.Fatalf("read: %v", err) } - lines := string(data) want := "install\nkubectl\nv1.28.0\n/usr/local/bin/kubectl" - if lines != want { - t.Errorf("env vars:\ngot: %q\nwant: %q", lines, want) + if string(data) != want { + t.Errorf("env vars:\ngot: %q\nwant: %q", data, want) } } func TestRunHook_EmptyIsNoOp(t *testing.T) { - if err := RunHook("", t.TempDir(), "install", "x", "v1", "/x", os.Stdout, os.Stderr); err != nil { + if err := RunHook("", t.TempDir(), "install", "x", "v1", "/x", nil, nil); err != nil { t.Errorf("empty hook should be no-op, got: %v", err) } } func TestRunHook_NonZeroExitReturnsError(t *testing.T) { - if err := RunHook("exit 1", t.TempDir(), "update", "x", "v1", "/x", os.Stdout, os.Stderr); err == nil { + if err := RunHook("exit 1", t.TempDir(), "update", "x", "v1", "/x", nil, nil); err == nil { t.Error("expected error on non-zero exit") } } @@ -43,7 +42,7 @@ func TestRunHook_RunsInDir(t *testing.T) { tmp := t.TempDir() out := filepath.Join(tmp, "pwd.txt") cmd := "pwd > " + out - if err := RunHook(cmd, tmp, "install", "x", "v1", "/x", os.Stdout, os.Stderr); err != nil { + if err := RunHook(cmd, tmp, "install", "x", "v1", "/x", nil, nil); err != nil { t.Fatalf("RunHook: %v", err) } data, err := os.ReadFile(out) @@ -51,7 +50,24 @@ func TestRunHook_RunsInDir(t *testing.T) { t.Fatal(err) } got := string(data) - if got[:len(got)-1] != tmp { // trim trailing newline + if got[:len(got)-1] != tmp { t.Errorf("hook ran in %q, want %q", got, tmp) } } + +func TestRunHook_WritesToProvidedStreams(t *testing.T) { + var out bytes.Buffer + if err := RunHook("echo hello", t.TempDir(), "install", "x", "v1", "/x", &out, nil); err != nil { + t.Fatalf("RunHook: %v", err) + } + if out.String() != "hello\n" { + t.Errorf("stdout = %q, want %q", out.String(), "hello\n") + } +} + +func TestRunHook_NilWritersDefaultToDiscard(t *testing.T) { + // Should not panic with nil writers. + if err := RunHook("echo ok", t.TempDir(), "install", "x", "v1", "/x", nil, nil); err != nil { + t.Errorf("unexpected error: %v", err) + } +} diff --git a/pkg/binary/types.go b/pkg/binary/types.go index cd7017f..0b8c0ed 100644 --- a/pkg/binary/types.go +++ b/pkg/binary/types.go @@ -65,10 +65,11 @@ type LocalBinary struct { Alias string `json:"alias,omitempty"` // Asset is a glob pattern to filter release assets (e.g. "argsh-so-*") Asset string `json:"asset,omitempty"` - // OnPost is a shell command run after a successful install/update of this - // binary. Receives B_EVENT (install|update), B_NAME, B_VERSION, B_FILE. - // Skipped when the binary didn't change or on --dry-run. Non-zero exit is - // surfaced as a warning, not a fatal error. + // OnPost is a shell command (POSIX, via "sh -c") run after a successful + // install/update of this binary. Receives B_EVENT (install|update), + // B_NAME, B_VERSION, B_FILE. Only runs when the on-disk binary actually + // changed — skipped on no-op installs, digest-match skips, and + // --dry-run. Non-zero exit is surfaced as a warning, not a fatal error. OnPost string `json:"onPost,omitempty" yaml:"onPost,omitempty"` // IsProviderRef is true when Name is a provider ref (e.g. github.com/derailed/k9s) IsProviderRef bool `json:"-" yaml:"-"` diff --git a/pkg/cli/install.go b/pkg/cli/install.go index f323c03..3f2b5b4 100644 --- a/pkg/cli/install.go +++ b/pkg/cli/install.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "os" "sort" "strings" "sync" @@ -257,16 +256,21 @@ func (o *InstallOptions) installBinaries(binaries []*binary.Binary) error { b.Tracker = tracker b.Writer = pw + // Track whether a download actually happened so we only run + // the onPost hook when the binary changed — not on a no-op + // "already exists" skip from EnsureBinary(false). + wasMissing := !b.BinaryExists() var err error if o.Force { err = b.DownloadBinary() } else { err = b.EnsureBinary(false) // Don't update, just ensure } + downloaded := err == nil && (o.Force || wasMissing) - // Run onPost hook after a successful download. - if err == nil && b.OnPost != "" { - if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "install", b.Name, b.Version, b.BinaryPath(), os.Stdout, os.Stderr); hookErr != nil { + // Run onPost hook only when a download actually happened. + if downloaded && b.OnPost != "" { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "install", b.Name, b.Version, b.BinaryPath(), o.IO.Out, o.IO.ErrOut); hookErr != nil { fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) } } diff --git a/pkg/cli/update.go b/pkg/cli/update.go index 2b81804..207712a 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -895,10 +895,12 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { var err error attempted := false + downloaded := false switch { case o.Force: attempted = true err = b.DownloadBinary() + downloaded = err == nil case digestMatchesLock(b, lk, freshDigests[b.Name]) && b.BinaryExists(): // Manifest digest matches the locked one AND the binary // is actually on disk: upstream hasn't moved since the @@ -914,22 +916,30 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { // otherwise keep the old binary for mutable tags like 'cli'. attempted = true err = b.DownloadBinary() + downloaded = err == nil default: // EnsureBinary's internal skip check may or may not // download; treat it as "attempted" only on error so a // failed preset update doesn't poison the lock either. + preSHA := "" + if h, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { + preSHA = h + } err = b.EnsureBinary(true) if err != nil { attempted = true + } else if postSHA, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { + downloaded = preSHA != postSHA } } outcomeMu.Lock() downloadFailed[b.Name] = attempted && err != nil outcomeMu.Unlock() - // Run onPost hook after a successful update. - if err == nil && b.OnPost != "" { - if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "update", b.Name, b.Version, b.BinaryPath(), os.Stdout, os.Stderr); hookErr != nil { + // Run onPost hook only when a download actually changed the + // binary, and not in dry-run mode. + if downloaded && b.OnPost != "" && !o.effectiveDryRun() { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "update", b.Name, b.Version, b.BinaryPath(), o.IO.Out, o.IO.ErrOut); hookErr != nil { fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) } } From b676747f1d037e322197917b7818e08d792dd1a4 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 09:45:55 +0200 Subject: [PATCH 3/8] fix(binary): macOS symlink in TestRunHook_RunsInDir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS /var → /private/var symlink causes t.TempDir() to return the unresolved path while pwd in the hook resolves it. Use filepath.EvalSymlinks + strings.TrimSpace for the comparison. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/binary/hook_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/binary/hook_test.go b/pkg/binary/hook_test.go index 67b12f6..c79bf9a 100644 --- a/pkg/binary/hook_test.go +++ b/pkg/binary/hook_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "strings" "testing" ) @@ -40,6 +41,12 @@ func TestRunHook_NonZeroExitReturnsError(t *testing.T) { func TestRunHook_RunsInDir(t *testing.T) { tmp := t.TempDir() + // Resolve symlinks so the comparison works on macOS where + // /var → /private/var and t.TempDir() returns the unresolved path. + realTmp, err := filepath.EvalSymlinks(tmp) + if err != nil { + t.Fatal(err) + } out := filepath.Join(tmp, "pwd.txt") cmd := "pwd > " + out if err := RunHook(cmd, tmp, "install", "x", "v1", "/x", nil, nil); err != nil { @@ -49,9 +56,9 @@ func TestRunHook_RunsInDir(t *testing.T) { if err != nil { t.Fatal(err) } - got := string(data) - if got[:len(got)-1] != tmp { - t.Errorf("hook ran in %q, want %q", got, tmp) + got := strings.TrimSpace(string(data)) + if got != realTmp { + t.Errorf("hook ran in %q, want %q", got, realTmp) } } From ed6a126348270d3c8afaa1d0b8fe5f757546f962 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 09:50:56 +0200 Subject: [PATCH 4/8] =?UTF-8?q?fix(binary):=20round=202=20=E2=80=94=20filt?= =?UTF-8?q?er=20parent=20B=5F=20env,=20use=20b.OnPost=20in=20addToConfig?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. RunHook now filters existing B_EVENT/B_NAME/B_VERSION/B_FILE from os.Environ() before appending the new values, so platform-dependent duplicate-env-key precedence can't make the hook observe a stale value from the parent process. 2. addToConfig was persisting o.OnPost (the CLI flag) instead of b.OnPost (the per-binary value). Changed to b.OnPost for consistency with asset/version/alias which all read from the Binary struct. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/binary/hook.go | 21 ++++++++++++++++++++- pkg/cli/cli_extra_test.go | 2 +- pkg/cli/install.go | 4 ++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/pkg/binary/hook.go b/pkg/binary/hook.go index fe5ac71..a4a6e40 100644 --- a/pkg/binary/hook.go +++ b/pkg/binary/hook.go @@ -30,11 +30,30 @@ func RunHook(command, dir, event, name, version, file string, stdout, stderr io. cmd.Dir = dir cmd.Stdout = stdout cmd.Stderr = stderr - cmd.Env = append(os.Environ(), + // Build env from parent process, filtering out any existing B_ vars + // so our values take guaranteed precedence regardless of platform. + hookVars := map[string]bool{ + "B_EVENT=": true, "B_NAME=": true, "B_VERSION=": true, "B_FILE=": true, + } + env := make([]string, 0, len(os.Environ())+4) + for _, e := range os.Environ() { + skip := false + for prefix := range hookVars { + if len(e) >= len(prefix) && e[:len(prefix)] == prefix { + skip = true + break + } + } + if !skip { + env = append(env, e) + } + } + env = append(env, fmt.Sprintf("B_EVENT=%s", event), fmt.Sprintf("B_NAME=%s", name), fmt.Sprintf("B_VERSION=%s", version), fmt.Sprintf("B_FILE=%s", file), ) + cmd.Env = env return cmd.Run() } diff --git a/pkg/cli/cli_extra_test.go b/pkg/cli/cli_extra_test.go index 96750ee..22948ad 100644 --- a/pkg/cli/cli_extra_test.go +++ b/pkg/cli/cli_extra_test.go @@ -1389,7 +1389,7 @@ func TestAddToConfig_WithOnPost(t *testing.T) { } binaries := []*binary.Binary{ - {Name: "argsh", AutoDetect: true, ProviderRef: "github.com/arg-sh/argsh"}, + {Name: "argsh", AutoDetect: true, ProviderRef: "github.com/arg-sh/argsh", OnPost: "argsh builtin ${B_EVENT}"}, } if err := o.addToConfig(binaries); err != nil { t.Fatalf("addToConfig() error = %v", err) diff --git a/pkg/cli/install.go b/pkg/cli/install.go index 3f2b5b4..e654387 100644 --- a/pkg/cli/install.go +++ b/pkg/cli/install.go @@ -348,8 +348,8 @@ func (o *InstallOptions) addToConfig(binaries []*binary.Binary) error { if b.AssetFilter != "" { entry.Asset = b.AssetFilter } - if o.OnPost != "" { - entry.OnPost = o.OnPost + if b.OnPost != "" { + entry.OnPost = b.OnPost } config.Binaries = append(config.Binaries, entry) } From e32275e00d9d088e8e1c3a54cbeac353c85f3735 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 09:58:33 +0200 Subject: [PATCH 5/8] =?UTF-8?q?perf(update):=20round=203=20=E2=80=94=20ski?= =?UTF-8?q?p=20SHA=20hash=20when=20no=20hook=20+=20rename=20shadow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Only compute pre/post SHA256 in the EnsureBinary branch when b.OnPost is non-empty and not dry-run — avoids O(file-size) hashing on every update for binaries without hooks. 2. Renamed local 'preSHA' → 'beforeHash' to avoid shadowing the outer preSHA map used by refreshLockDigests. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/cli/update.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/cli/update.go b/pkg/cli/update.go index 207712a..e5b5c09 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -921,15 +921,21 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { // EnsureBinary's internal skip check may or may not // download; treat it as "attempted" only on error so a // failed preset update doesn't poison the lock either. - preSHA := "" - if h, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { - preSHA = h + // Only hash pre/post when a hook might run — avoids + // O(file-size) work when there's no onPost hook. + var beforeHash string + if b.OnPost != "" && !o.effectiveDryRun() { + if h, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { + beforeHash = h + } } err = b.EnsureBinary(true) if err != nil { attempted = true - } else if postSHA, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { - downloaded = preSHA != postSHA + } else if beforeHash != "" { + if afterHash, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { + downloaded = beforeHash != afterHash + } } } outcomeMu.Lock() From fa49f4f17475927df24a62fe8128bc3a89dbadce Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 10:05:59 +0200 Subject: [PATCH 6/8] =?UTF-8?q?fix(update):=20round=204=20=E2=80=94=20hand?= =?UTF-8?q?le=20missing=20binary=20+=20tighten=20env=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. EnsureBinary branch now tracks wasMissing before hashing. If the binary was deleted from disk, the pre-SHA would fail and downloaded would stay false, silently skipping the hook. Now wasMissing=true sets downloaded=true after a successful EnsureBinary. 2. Tightened RunHook env-filter comment to say "the four hook-specific variables" not "any existing B_ vars". Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/binary/hook.go | 5 +++-- pkg/cli/update.go | 13 ++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/binary/hook.go b/pkg/binary/hook.go index a4a6e40..c57ed62 100644 --- a/pkg/binary/hook.go +++ b/pkg/binary/hook.go @@ -30,8 +30,9 @@ func RunHook(command, dir, event, name, version, file string, stdout, stderr io. cmd.Dir = dir cmd.Stdout = stdout cmd.Stderr = stderr - // Build env from parent process, filtering out any existing B_ vars - // so our values take guaranteed precedence regardless of platform. + // Build env from parent process, filtering out the four hook-specific + // variables (B_EVENT, B_NAME, B_VERSION, B_FILE) so our values take + // guaranteed precedence regardless of platform. hookVars := map[string]bool{ "B_EVENT=": true, "B_NAME=": true, "B_VERSION=": true, "B_FILE=": true, } diff --git a/pkg/cli/update.go b/pkg/cli/update.go index e5b5c09..ff1f75e 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -921,10 +921,15 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { // EnsureBinary's internal skip check may or may not // download; treat it as "attempted" only on error so a // failed preset update doesn't poison the lock either. - // Only hash pre/post when a hook might run — avoids - // O(file-size) work when there's no onPost hook. + // Detect whether a download happened via two signals: + // - binary was missing → any successful EnsureBinary + // must have downloaded it + // - binary existed → compare SHA before/after + // Only compute hashes when a hook might run — avoids + // O(file-size) work for binaries without hooks. + wasMissing := !b.BinaryExists() var beforeHash string - if b.OnPost != "" && !o.effectiveDryRun() { + if !wasMissing && b.OnPost != "" && !o.effectiveDryRun() { if h, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { beforeHash = h } @@ -932,6 +937,8 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { err = b.EnsureBinary(true) if err != nil { attempted = true + } else if wasMissing { + downloaded = true } else if beforeHash != "" { if afterHash, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { downloaded = beforeHash != afterHash From d39d004816a2b3eb9b28355ebb3a31eab2c3bbbd Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 10:14:26 +0200 Subject: [PATCH 7/8] =?UTF-8?q?fix(update):=20round=205=20=E2=80=94=20best?= =?UTF-8?q?-effort=20hook=20on=20hash=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If SHA256File fails before or after EnsureBinary (e.g. transient I/O error), assume the binary changed and run the hook. Better to run it unnecessarily than silently skip due to an unrelated file-read error. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/cli/update.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/cli/update.go b/pkg/cli/update.go index ff1f75e..1268fc7 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -929,18 +929,23 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { // O(file-size) work for binaries without hooks. wasMissing := !b.BinaryExists() var beforeHash string - if !wasMissing && b.OnPost != "" && !o.effectiveDryRun() { - if h, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { - beforeHash = h - } + needHookCheck := b.OnPost != "" && !o.effectiveDryRun() + if !wasMissing && needHookCheck { + beforeHash, _ = lock.SHA256File(b.BinaryPath()) } err = b.EnsureBinary(true) if err != nil { attempted = true } else if wasMissing { downloaded = true - } else if beforeHash != "" { - if afterHash, shaErr := lock.SHA256File(b.BinaryPath()); shaErr == nil { + } else if needHookCheck { + // If we can't hash, assume the file changed — better + // to run the hook unnecessarily than silently skip it + // due to an unrelated I/O error. + afterHash, shaErr := lock.SHA256File(b.BinaryPath()) + if shaErr != nil || beforeHash == "" { + downloaded = true + } else { downloaded = beforeHash != afterHash } } From ff44eb3677b6eb2d4acf95c06ba5d0d790c522d9 Mon Sep 17 00:00:00 2001 From: Jan Guth Date: Fri, 8 May 2026 10:39:42 +0200 Subject: [PATCH 8/8] =?UTF-8?q?fix(binary):=20round=206=20=E2=80=94=20rout?= =?UTF-8?q?e=20hook=20stdout=20to=20ErrOut=20to=20avoid=20progress=20corru?= =?UTF-8?q?ption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook stdout was routed to o.IO.Out while the progress writer renders to the same stream concurrently. Any hook output would interleave with progress bars and corrupt the terminal display. Route both hook stdout and stderr to o.IO.ErrOut so hook output stays clean and separate from progress rendering. This is consistent with how hook warnings are already printed. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/cli/install.go | 2 +- pkg/cli/update.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cli/install.go b/pkg/cli/install.go index e654387..837f4f2 100644 --- a/pkg/cli/install.go +++ b/pkg/cli/install.go @@ -270,7 +270,7 @@ func (o *InstallOptions) installBinaries(binaries []*binary.Binary) error { // Run onPost hook only when a download actually happened. if downloaded && b.OnPost != "" { - if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "install", b.Name, b.Version, b.BinaryPath(), o.IO.Out, o.IO.ErrOut); hookErr != nil { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "install", b.Name, b.Version, b.BinaryPath(), o.IO.ErrOut, o.IO.ErrOut); hookErr != nil { fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) } } diff --git a/pkg/cli/update.go b/pkg/cli/update.go index 1268fc7..c5cff4e 100644 --- a/pkg/cli/update.go +++ b/pkg/cli/update.go @@ -957,7 +957,7 @@ func (o *UpdateOptions) updateBinaries(binaries []*binary.Binary) error { // Run onPost hook only when a download actually changed the // binary, and not in dry-run mode. if downloaded && b.OnPost != "" && !o.effectiveDryRun() { - if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "update", b.Name, b.Version, b.BinaryPath(), o.IO.Out, o.IO.ErrOut); hookErr != nil { + if hookErr := binary.RunHook(b.OnPost, o.ProjectRoot(), "update", b.Name, b.Version, b.BinaryPath(), o.IO.ErrOut, o.IO.ErrOut); hookErr != nil { fmt.Fprintf(o.IO.ErrOut, "Warning: onPost hook for %s failed: %v\n", b.Name, hookErr) } }