Compare commits

...
Author SHA1 Message Date
Gnanam 5068a3840f fix(doctor): address review — rename shadowing var, align missing-binary message
- Rename the local `runtime` (was shadowing the stdlib `runtime` package used
  elsewhere in doctor.go) to `runtimeVersion`.
- Align the unresolvable-binary FAIL message's remediation with the npm wrapper
  (reinstall, or build from source: <repo url>) so both missing-binary paths
  give consistent guidance.

Addresses CodeRabbit review on #405.
2026-07-03 16:00:59 +05:30
Gnanam 2a5b2af277 fix(doctor): make the runtime check verify the native binary, not always pass
`zero doctor`'s runtime check returned StatusPass unconditionally with a
hardcoded "go" label — it verified nothing (the running binary, a version,
anything), so it was a false-confidence green: "Zero Go runtime is available
(go)" no matter what (#405).

Make it real: resolve the running binary via os.Executable, stat it, and report
the resolved PATH plus the actual Go version — failing loudly if the path can't
be resolved, the file has vanished, or it's a directory. os.Executable/os.Stat
are injectable (Options.Executable/StatExecutable) so the check is testable
without a real on-disk layout.

Note on scope: doctor runs INSIDE the native binary, so it cannot diagnose the
binary being ABSENT at install time — that case is already caught earlier and
loudly by the npm wrapper (bin/zero.js refuses to launch, present since v0.1.0),
which is why the issue's literal "doctor passes while binary missing" repro does
not occur. The real defect this fixes is the tautological check; surfacing the
binary path is the useful part (spotting an npm-installed binary vs a stale or
shadowing copy on PATH) instead of a meaningless pass. The caller now passes the
real runtime.Version() rather than the "go" placeholder.

Tests: present-binary passes and reports the path; missing/inaccessible binary,
unresolvable os.Executable, and a directory path each FAIL (red-green — the old
code passed unconditionally); the default (uninjected) path resolves the real
running binary.

Fixes #405
2026-07-03 07:13:35 +05:30
3 changed files with 137 additions and 7 deletions

No files matched your search

+2 -1
View File
@@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"io"
"runtime"
"strconv"
"strings"
@@ -65,7 +66,7 @@ func runDoctor(args []string, stdout io.Writer, stderr io.Writer, deps appDeps)
report := doctor.Run(doctor.Options{
Now: deps.now,
Runtime: "go",
Runtime: runtime.Version(),
UserConfig: userConfig,
ProjectConfig: projectConfig,
Provider: provider,
+47 -6
View File
@@ -57,6 +57,11 @@ type Options struct {
// LSP-server checks. Nil means exec.LookPath; tests inject a stub so the
// checks are deterministic regardless of the host's installed tooling.
LookupExecutable func(string) (string, error)
// Executable resolves the running Zero binary's path (nil => os.Executable)
// and StatExecutable stats it (nil => os.Stat). Injected so the native-binary
// runtime check is testable without depending on a real on-disk layout.
Executable func() (string, error)
StatExecutable func(string) (os.FileInfo, error)
}
func Run(options Options) Report {
@@ -65,7 +70,7 @@ func Run(options Options) Report {
now = time.Now
}
checks := []Check{
runtimeCheck(options.Runtime),
runtimeCheck(options),
configFilesCheck(options.UserConfig, options.ProjectConfig),
configValidationCheck(options.UserConfig, options.ProjectConfig),
}
@@ -114,12 +119,48 @@ func Format(report Report) string {
return strings.Join(lines, "\n")
}
func runtimeCheck(runtime string) Check {
runtime = strings.TrimSpace(runtime)
if runtime == "" {
runtime = "go"
// runtimeCheck verifies the running Zero native binary is actually present and
// runnable, and reports its resolved path — instead of the former hardcoded
// always-pass, which claimed "Go runtime is available" while checking nothing
// (#405). doctor executes INSIDE the native binary, so this only fails if
// os.Executable can't resolve or the running file has vanished; the missing-
// binary-at-install case is caught earlier by the npm wrapper, which refuses to
// launch without the platform binary. Surfacing the binary PATH is the useful
// part: it lets a user see which binary answered (e.g. an npm-installed one vs a
// stale/shadowing copy on PATH) rather than a meaningless green.
func runtimeCheck(options Options) Check {
runtimeVersion := strings.TrimSpace(options.Runtime)
if runtimeVersion == "" {
runtimeVersion = "go"
}
return check("runtime.go", "Go runtime", StatusPass, fmt.Sprintf("Zero Go runtime is available (%s).", runtime), map[string]any{"runtime": runtime})
details := map[string]any{"runtime": runtimeVersion}
executable := options.Executable
if executable == nil {
executable = os.Executable
}
path, err := executable()
if err != nil {
return check("runtime.go", "Zero native binary", StatusFail,
"Could not resolve the running Zero binary: "+err.Error(), details)
}
details["binaryPath"] = path
stat := options.StatExecutable
if stat == nil {
stat = os.Stat
}
info, err := stat(path)
if err != nil {
return check("runtime.go", "Zero native binary", StatusFail,
fmt.Sprintf("The running Zero binary is not accessible at %s: %s. Reinstall the zero package, or build from source: https://github.com/Gitlawb/zero", path, err.Error()), details)
}
if info.IsDir() {
return check("runtime.go", "Zero native binary", StatusFail,
fmt.Sprintf("The resolved Zero binary path %s is a directory, not an executable.", path), details)
}
return check("runtime.go", "Zero native binary", StatusPass,
fmt.Sprintf("Zero native binary is present and runnable at %s (%s).", path, runtimeVersion), details)
}
func configFilesCheck(userPath string, projectPath string) Check {
+88
View File
@@ -0,0 +1,88 @@
package doctor
import (
"errors"
"io/fs"
"os"
"strings"
"testing"
"time"
)
// fakeFileInfo is a minimal os.FileInfo for the runtime-check stat injection.
type fakeFileInfo struct{ dir bool }
func (f fakeFileInfo) Name() string { return "zero" }
func (f fakeFileInfo) Size() int64 { return 1 }
func (f fakeFileInfo) Mode() fs.FileMode { return 0o755 }
func (f fakeFileInfo) ModTime() time.Time { return time.Unix(0, 0) }
func (f fakeFileInfo) IsDir() bool { return f.dir }
func (f fakeFileInfo) Sys() any { return nil }
func runtimeCheckFor(t *testing.T, exe func() (string, error), stat func(string) (os.FileInfo, error)) Check {
t.Helper()
return runtimeCheck(Options{Runtime: "go1.26", Executable: exe, StatExecutable: stat})
}
// A present, runnable binary passes AND reports its resolved path — the check is
// no longer a hardcoded green; it names which binary answered.
func TestRuntimeCheckPassesAndReportsBinaryPath(t *testing.T) {
c := runtimeCheckFor(t,
func() (string, error) { return "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero", nil },
func(string) (os.FileInfo, error) { return fakeFileInfo{}, nil })
if c.Status != StatusPass {
t.Fatalf("status = %s, want pass", c.Status)
}
if c.Details["binaryPath"] != "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero" {
t.Fatalf("binaryPath = %v, want the resolved path in details", c.Details["binaryPath"])
}
if !strings.Contains(c.Message, "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero") {
t.Fatalf("message should surface the binary path, got %q", c.Message)
}
}
// The vanished-binary case FAILS loudly instead of the old unconditional pass —
// this is the #405 regression: a broken runtime no longer gets a green check.
func TestRuntimeCheckFailsWhenBinaryMissing(t *testing.T) {
c := runtimeCheckFor(t,
func() (string, error) { return "/path/to/zero", nil },
func(string) (os.FileInfo, error) { return nil, os.ErrNotExist })
if c.Status != StatusFail {
t.Fatalf("#405: a missing/inaccessible binary must FAIL, got %s (%q)", c.Status, c.Message)
}
if !strings.Contains(c.Message, "/path/to/zero") {
t.Fatalf("failure message should name the path, got %q", c.Message)
}
}
// os.Executable resolution failure fails loudly too.
func TestRuntimeCheckFailsWhenExecutableUnresolvable(t *testing.T) {
c := runtimeCheckFor(t,
func() (string, error) { return "", errors.New("no executable path") },
func(string) (os.FileInfo, error) { return fakeFileInfo{}, nil })
if c.Status != StatusFail {
t.Fatalf("unresolvable executable must FAIL, got %s", c.Status)
}
}
// A path that resolves to a directory is not a runnable binary → FAIL.
func TestRuntimeCheckFailsWhenPathIsDirectory(t *testing.T) {
c := runtimeCheckFor(t,
func() (string, error) { return "/some/dir", nil },
func(string) (os.FileInfo, error) { return fakeFileInfo{dir: true}, nil })
if c.Status != StatusFail {
t.Fatalf("a directory path must FAIL, got %s", c.Status)
}
}
// Default injection (nil Executable/StatExecutable) resolves the real running
// test binary — proving the production default path works and passes.
func TestRuntimeCheckDefaultsToRealBinary(t *testing.T) {
c := runtimeCheck(Options{})
if c.Status != StatusPass {
t.Fatalf("default runtime check should pass for the running test binary, got %s (%q)", c.Status, c.Message)
}
if strings.TrimSpace(c.Details["binaryPath"].(string)) == "" {
t.Fatal("default runtime check should report the running binary path")
}
}