From 4183f516bf5280da1d8bf5a070aa9a7c008a889b Mon Sep 17 00:00:00 2001 From: yetone Date: Mon, 5 Oct 2026 13:23:57 +0800 Subject: [PATCH] =?UTF-8?q?agent,=20provider:=20a=20CLI=20whose=20--versio?= =?UTF-8?q?n=20fails=20isn't=20run=20again=20until=20it=20changes,=20and?= =?UTF-8?q?=20an=20npm=20CLI's=20version=20is=20read=20from=20its=20packag?= =?UTF-8?q?e=20(#864)=20=E2=80=94=20macOS=20put=20up=20=E3=80=8C=E5=B7=B2?= =?UTF-8?q?=E9=98=BB=E6=AD=A2=E6=81=B6=E6=84=8F=E8=BD=AF=E4=BB=B6=EF=BC=9A?= =?UTF-8?q?=E6=9C=AA=E6=89=93=E5=BC=80=20"codex"=E3=80=8D=20each=20time=20?= =?UTF-8?q?magpie's=20window=20opened:=20the=20codex=20magpie=20finds=20wa?= =?UTF-8?q?s=20asked=20its=20version=20every=20ten=20minutes=20(the=20mode?= =?UTF-8?q?l=20list's=20client=20version)=20and=20again=20with=20the=20Age?= =?UTF-8?q?nts=20page=20(a=20failure=20retried=20after=20ten=20minutes),?= =?UTF-8?q?=20and=20macOS=20stops=20a=20program=20XProtect=20flags=20with?= =?UTF-8?q?=20that=20alert=20on=20every=20start.=20proc.Version=20is=20the?= =?UTF-8?q?=20one=20way=20both=20ask=20now:=20an=20npm=20CLI=20(=E2=80=A6/?= =?UTF-8?q?node=5Fmodules/@openai/codex/bin/codex.js)=20has=20its=20packag?= =?UTF-8?q?e.json's=20version=20read=20and=20isn't=20run=20at=20all;=20any?= =?UTF-8?q?=20other=20is=20run=20with=20--version,=20and=20a=20run=20that?= =?UTF-8?q?=20ran=20and=20said=20nothing,=20or=20couldn't=20start,=20isn't?= =?UTF-8?q?=20made=20again=20until=20the=20binary's=20path,=20size=20or=20?= =?UTF-8?q?mtime=20changes=20(a=20reinstall);=20one=20cut=20off=20by=20the?= =?UTF-8?q?=20timeout=20is.=20A=20CLI=20that=20answered=20is=20asked=20as?= =?UTF-8?q?=20before.=20TestCodexVersionNotRerun=20(a=20codex=20that=20exi?= =?UTF-8?q?ts=20137=20run=20once=20over=20three=20ten-minute=20refreshes;?= =?UTF-8?q?=20an=20npm=20codex's=20version=20read,=20not=20run)=20fails=20?= =?UTF-8?q?on=20the=20old=20code=20(run=203=20times);=20TestVersionRunsOnc?= =?UTF-8?q?e.=20TestUpdateCLI=20and=20the=20agent=20and=20provider=20tests?= =?UTF-8?q?=20pass.=20go=20vet,=20builds=20(darwin,=20GOOS=3Dlinux=20nogui?= =?UTF-8?q?,=20GOOS=3Dwindows)=20and=20go=20test=20-tags=20nogui=20-count?= =?UTF-8?q?=3D1=20./...=20under=20a=20temp=20HOME=20pass=20but=20for=20Tes?= =?UTF-8?q?tClientGoneRecordedAsCanceled,=20which=20passes=20alone=20(-cou?= =?UTF-8?q?nt=3D3).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/agent/cliupdate.go | 15 ++-- internal/proc/version.go | 97 ++++++++++++++++++++++++++ internal/proc/version_test.go | 79 +++++++++++++++++++++ internal/provider/codex_models.go | 12 ++-- internal/provider/codex_models_test.go | 48 +++++++++++++ 5 files changed, 235 insertions(+), 16 deletions(-) create mode 100644 internal/proc/version.go create mode 100644 internal/proc/version_test.go diff --git a/internal/agent/cliupdate.go b/internal/agent/cliupdate.go index 06488ede..4e01d905 100644 --- a/internal/agent/cliupdate.go +++ b/internal/agent/cliupdate.go @@ -403,17 +403,10 @@ func installedVersion(bin string) string { }) } -// runVersion runs bin --version with nothing on its stdin; a var so tests -// can fake it. -var runVersion = func(bin string) string { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - cmd := proc.ProbeContext(ctx, bin, "--version") - cmd.Stdin = nil // /dev/null: one that would ask something gets nothing - cmd.Env = append(os.Environ(), "NO_COLOR=1") - out, _ := cmd.CombinedOutput() - return string(out) -} +// runVersion is what bin says its version is (proc.Version: read from its +// npm package, else bin --version, not run again after it failed); a var so +// tests can fake it. +var runVersion = proc.Version // latestVersion is the newest version where u would update from. func latestVersion(u *updater) string { diff --git a/internal/proc/version.go b/internal/proc/version.go new file mode 100644 index 00000000..a82d650e --- /dev/null +++ b/internal/proc/version.go @@ -0,0 +1,97 @@ +package proc + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "time" +) + +// Version is what the CLI at bin says its version is: the version in the +// package.json of the npm package it is a file of, read rather than run, +// else what bin --version prints (stdout and stderr, ANSI and all), "" when +// it says nothing. A run that fails other than by taking too long isn't +// made again until the binary changes (#864): macOS stops a program XProtect flags with +// a "contains malware" alert on each start, and a CLI asked its version +// every few minutes put one up every time magpie's window opened. An npm +// CLI isn't run at all: its package says the version its --version would. +func Version(bin string) string { + real, err := filepath.EvalSymlinks(bin) + if err != nil { + real = bin + } + st, err := os.Stat(real) + if err != nil { + return "" + } + if v := npmVersion(real); v != "" { + return v + } + stamp := fmt.Sprint(real, st.Size(), st.ModTime().UnixNano()) + versionsFailed.Lock() + failed := versionsFailed.m[bin] == stamp + versionsFailed.Unlock() + if failed { + return "" + } + out, err := runVersion(bin) + // a run cut off by the timeout (a busy machine, a slow first start) is + // made again; one that ran and said nothing, or couldn't start, is not. + // One that answered is asked again as its caller likes: what it says + // may come from a file beside it that an update changes. + if out == "" && !errors.Is(err, context.DeadlineExceeded) { + versionsFailed.Lock() + versionsFailed.m[bin] = stamp + versionsFailed.Unlock() + } + return out +} + +// versionsFailed is, for each CLI whose run failed, the binary it was. +var versionsFailed = struct { + sync.Mutex + m map[string]string +}{m: map[string]string{}} + +// runVersion runs bin --version with nothing on its stdin; a var so tests +// can count the runs. +var runVersion = func(bin string) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := ProbeContext(ctx, bin, "--version") + cmd.Stdin = nil // /dev/null: one that would ask something gets nothing + cmd.Env = append(os.Environ(), "NO_COLOR=1") + out, err := cmd.CombinedOutput() + if ctx.Err() != nil { + return "", ctx.Err() + } + // one that says its version and exits non-zero has still said it + return strings.TrimSpace(string(out)), err +} + +// npmVersion is the version of the npm package the file at real is in +// (…/node_modules/@openai/codex/bin/codex.js is in @openai/codex), "" when +// it is in none. +func npmVersion(real string) string { + for d := filepath.Dir(real); ; d = filepath.Dir(d) { + up := filepath.Dir(d) + if up == d { + return "" + } + if filepath.Base(up) == "node_modules" || strings.HasPrefix(filepath.Base(up), "@") && filepath.Base(filepath.Dir(up)) == "node_modules" { + var p struct { + Version string `json:"version"` + } + b, err := os.ReadFile(filepath.Join(d, "package.json")) + if err != nil || json.Unmarshal(b, &p) != nil { + return "" + } + return p.Version + } + } +} diff --git a/internal/proc/version_test.go b/internal/proc/version_test.go new file mode 100644 index 00000000..f7ec0cd5 --- /dev/null +++ b/internal/proc/version_test.go @@ -0,0 +1,79 @@ +package proc + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// A CLI is asked its version once per binary: one that fails to run (macOS +// stopping it as malware, #864) isn't started again until it changes, one +// cut off by the timeout is, and an npm CLI's is read from its package +// without running it. +func TestVersionRunsOnce(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell scripts") + } + dir := t.TempDir() + runs := filepath.Join(dir, "runs") + bin := filepath.Join(dir, "codex") + write := func(body string) { + if err := os.WriteFile(bin, []byte("#!/bin/sh\necho x >> '"+runs+"'\n"+body), 0o755); err != nil { + t.Fatal(err) + } + } + count := func() int { + b, _ := os.ReadFile(runs) + return strings.Count(string(b), "x") + } + write("exit 9\n") + for range 3 { + if v := Version(bin); v != "" { + t.Fatalf("a failing CLI said %q", v) + } + } + if n := count(); n != 1 { + t.Fatalf("a failing CLI was run %d times", n) + } + // changed (reinstalled), it is asked again + write("echo codex-cli 0.161.1\n") + os.Chtimes(bin, time.Now().Add(time.Minute), time.Now().Add(time.Minute)) + if v := Version(bin); v != "codex-cli 0.161.1" { + t.Fatalf("got %q", v) + } + if n := count(); n != 2 { + t.Fatalf("run %d times", n) + } + + // a timeout is asked again + other := filepath.Join(dir, "slow") + os.WriteFile(other, []byte("#!/bin/sh\n"), 0o755) + old := runVersion + t.Cleanup(func() { runVersion = old }) + asked := 0 + runVersion = func(string) (string, error) { asked++; return "", context.DeadlineExceeded } + Version(other) + Version(other) + if asked != 2 { + t.Fatalf("a timed-out CLI was asked %d times", asked) + } + + // an npm CLI: the package says it, nothing is run + asked = 0 + pkg := filepath.Join(dir, "lib", "node_modules", "@openai", "codex") + os.MkdirAll(filepath.Join(pkg, "bin"), 0o755) + os.WriteFile(filepath.Join(pkg, "package.json"), []byte(`{"name":"@openai/codex","version":"0.162.0"}`), 0o644) + os.WriteFile(filepath.Join(pkg, "bin", "codex.js"), []byte("#!/usr/bin/env node\n"), 0o755) + link := filepath.Join(dir, "bin", "codex") + os.MkdirAll(filepath.Dir(link), 0o755) + if err := os.Symlink(filepath.Join(pkg, "bin", "codex.js"), link); err != nil { + t.Fatal(err) + } + if v := Version(link); v != "0.162.0" || asked != 0 { + t.Fatalf("npm CLI: %q, run %d times", v, asked) + } +} diff --git a/internal/provider/codex_models.go b/internal/provider/codex_models.go index c4203b21..40f0167d 100644 --- a/internal/provider/codex_models.go +++ b/internal/provider/codex_models.go @@ -89,17 +89,19 @@ func codexVersion() string { newer(c.ClientVersion) } if exe := codexExecutable(); exe != "" { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - if out, err := proc.ProbeContext(ctx, exe, "--version").Output(); err == nil { - newer(string(out)) // "codex-cli 0.155.1" - } - cancel() + // "codex-cli 0.155.1"; read from its npm package, and not run + // again once it failed (#864: macOS's malware alert each time) + newer(codexCLIVersion(exe)) } codexVersionCache.v, codexVersionCache.at = v, time.Now() } return newerVersion(codexVersionCache.v, codexSeen.get()) } +// codexCLIVersion is what the codex CLI at exe says its version is; a var +// so tests can fake it. +var codexCLIVersion = proc.Version + // newerVersion is the later of two versions, a when b isn't one. func newerVersion(a, b string) string { if b = claudeSemverRE.FindString(b); b != "" && (a == "" || compareClaudeVersion(b, a) > 0) { diff --git a/internal/provider/codex_models_test.go b/internal/provider/codex_models_test.go index fafea929..00842ec4 100644 --- a/internal/provider/codex_models_test.go +++ b/internal/provider/codex_models_test.go @@ -178,3 +178,51 @@ func TestCodexModelsMaxContext(t *testing.T) { t.Fatalf("%+v", ms) } } + +// A codex CLI that fails to run — macOS stopping it as malware, with an +// alert each time (#864) — isn't run again every ten minutes, and an npm +// install's version is read from its package, not run. +func TestCodexVersionNotRerun(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell scripts") + } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("CODEX_HOME", "") + dir := t.TempDir() + runs := filepath.Join(dir, "runs") + exe := filepath.Join(dir, "codex") + if err := os.WriteFile(exe, []byte("#!/bin/sh\necho x >> '"+runs+"'\nexit 137\n"), 0o755); err != nil { + t.Fatal(err) + } + old := codexExecutable + t.Cleanup(func() { codexExecutable = old; codexVersionCache.at = time.Time{}; codexSeen = seenVersion{} }) + codexSeen = seenVersion{} + codexExecutable = func() string { return exe } + for range 3 { // ten minutes apart + codexVersionCache.at = time.Time{} + if got := codexVersion(); got != codexClientVersion { + t.Fatalf("got %s", got) + } + } + if b, _ := os.ReadFile(runs); strings.Count(string(b), "x") != 1 { + t.Fatalf("the failing codex was run %d times", strings.Count(string(b), "x")) + } + + pkg := filepath.Join(dir, "node_modules", "@openai", "codex") + os.MkdirAll(filepath.Join(pkg, "bin"), 0o755) + os.WriteFile(filepath.Join(pkg, "package.json"), []byte(`{"version":"0.199.0"}`), 0o644) + os.WriteFile(filepath.Join(pkg, "bin", "codex.js"), []byte("#!/bin/sh\necho x >> '"+runs+"'\necho codex-cli 0.199.0\n"), 0o755) + link := filepath.Join(dir, "bin", "codex") + os.MkdirAll(filepath.Dir(link), 0o755) + os.Symlink(filepath.Join(pkg, "bin", "codex.js"), link) + codexExecutable = func() string { return link } + codexVersionCache.at = time.Time{} + if got := codexVersion(); got != "0.199.0" { + t.Fatalf("npm codex: %s", got) + } + if b, _ := os.ReadFile(runs); strings.Count(string(b), "x") != 1 { + t.Fatal("the npm codex was run") + } +}