From 6f0001d04c0974c79dbcdd1829f3e2c1a6bc96b0 Mon Sep 17 00:00:00 2001 From: Molecule AI Infra-SRE Date: Mon, 11 May 2026 18:32:05 +0000 Subject: [PATCH 1/2] fix(provisioner): fail-fast pre-flight check for docker+git in local-build mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before reaching the clone/build cold path, check that both `docker` and `git` are on PATH. Previously, a missing `docker` would produce a cryptic "exec: docker: executable file not found" from deep inside the docker-has-tag or docker-build call. Now the error surfaces immediately with: local-build: "docker" not found on PATH — local-build mode requires both docker and git; either install them, or set MOLECULE_IMAGE_REGISTRY so local-build is bypassed The check runs before the cache-hit fast path too, since docker is used for image inspect + tag even on a cache hit. Adds checkTool seam to LocalBuildOptions so tests can inject a stub (no-op in makeTestOpts; two new tests exercise the missing-tool path). Fixes issue #529 option B. Co-Authored-By: Claude Opus 4.7 --- .../internal/provisioner/localbuild.go | 35 +++++++++++++- .../internal/provisioner/localbuild_test.go | 48 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/workspace-server/internal/provisioner/localbuild.go b/workspace-server/internal/provisioner/localbuild.go index 9f1fcf5d..a651b4a0 100644 --- a/workspace-server/internal/provisioner/localbuild.go +++ b/workspace-server/internal/provisioner/localbuild.go @@ -109,13 +109,16 @@ type LocalBuildOptions struct { // http.DefaultClient with a 30s timeout. HTTPClient *http.Client - // remoteHeadSha + dockerBuild + gitClone are seams for tests; if - // nil, the production implementations are used. + // remoteHeadSha + dockerBuild + gitClone + checkTool are seams for tests; + // if nil, the production implementations are used. remoteHeadSha func(ctx context.Context, opts *LocalBuildOptions, runtime string) (string, error) gitClone func(ctx context.Context, opts *LocalBuildOptions, runtime, dest string) error dockerBuild func(ctx context.Context, opts *LocalBuildOptions, contextDir, tag string) error dockerHasTag func(ctx context.Context, tag string) (bool, error) dockerTag func(ctx context.Context, src, dst string) error + // checkTool validates that the named binary is on PATH. nil = production + // LookPath check; tests override to skip or mock. + checkTool func(tool string) error } func newDefaultLocalBuildOptions() *LocalBuildOptions { @@ -182,6 +185,20 @@ func EnsureLocalImage(ctx context.Context, runtime string) (string, error) { // production code. var ensureLocalImageHook = EnsureLocalImage +// checkToolOnPath verifies tool is on PATH and returns its absolute path, +// or a descriptive error. Used for pre-flight validation before the +// clone/build cold path. +func checkToolOnPath(tool string) (string, error) { + path, err := exec.LookPath(tool) + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + return "", fmt.Errorf("local-build: %q not found on PATH — local-build mode requires both docker and git; either install them, or set MOLECULE_IMAGE_REGISTRY so local-build is bypassed", tool) + } + return "", fmt.Errorf("local-build: LookPath(%q) failed: %w", tool, err) + } + return path, nil +} + func ensureLocalImageWithOpts(ctx context.Context, runtime string, opts *LocalBuildOptions) (string, error) { if !IsKnownRuntime(runtime) { return "", fmt.Errorf("local-build: refusing to build unknown runtime %q (must be one of %v)", runtime, knownRuntimes) @@ -191,6 +208,20 @@ func ensureLocalImageWithOpts(ctx context.Context, runtime string, opts *LocalBu lock.Lock() defer lock.Unlock() + // Pre-flight: both docker and git are required even on the cache-hit + // path (docker is used for image inspect + tag). Fail fast with a clear + // message rather than a cryptic "exec: docker: executable file not found". + checkFn := opts.checkTool + if checkFn == nil { + checkFn = checkToolOnPath + } + if err := checkFn("docker"); err != nil { + return "", fmt.Errorf("local-build: %w; set MOLECULE_IMAGE_REGISTRY to bypass local-build mode", err) + } + if err := checkFn("git"); err != nil { + return "", fmt.Errorf("local-build: %w; set MOLECULE_IMAGE_REGISTRY to bypass local-build mode", err) + } + // 1. HEAD lookup → cache key. headFn := opts.remoteHeadSha if headFn == nil { diff --git a/workspace-server/internal/provisioner/localbuild_test.go b/workspace-server/internal/provisioner/localbuild_test.go index 1a169592..df804821 100644 --- a/workspace-server/internal/provisioner/localbuild_test.go +++ b/workspace-server/internal/provisioner/localbuild_test.go @@ -43,6 +43,10 @@ func makeTestOpts(t *testing.T) *LocalBuildOptions { dockerTag: func(ctx context.Context, src, dst string) error { return nil }, + // checkTool: skip the real LookPath in tests (docker/git may not be on PATH + // in the CI environment). Tests that exercise tool-not-found behaviour + // override this stub explicitly. + checkTool: func(tool string) error { return nil }, } } @@ -87,6 +91,50 @@ func TestEnsureLocalImage_CacheHit(t *testing.T) { } } +// TestEnsureLocalImage_MissingTool_Docker — pre-flight catches a missing +// docker binary before any cryptic exec-not-found error propagates up. +// The error must mention both the missing tool and the escape-hatch hint. +func TestEnsureLocalImage_MissingTool_Docker(t *testing.T) { + opts := makeTestOpts(t) + opts.checkTool = func(tool string) error { + if tool == "docker" { + return errors.New(`"docker" not found on PATH`) + } + return nil + } + _, err := ensureLocalImageWithOpts(context.Background(), "claude-code", opts) + if err == nil { + t.Fatalf("expected error for missing docker") + } + if !strings.Contains(err.Error(), "docker") { + t.Errorf("error = %v, want one mentioning docker", err) + } + if !strings.Contains(err.Error(), "MOLECULE_IMAGE_REGISTRY") { + t.Errorf("error = %v, want one mentioning MOLECULE_IMAGE_REGISTRY", err) + } +} + +// TestEnsureLocalImage_MissingTool_Git — same for a missing git binary. +func TestEnsureLocalImage_MissingTool_Git(t *testing.T) { + opts := makeTestOpts(t) + opts.checkTool = func(tool string) error { + if tool == "git" { + return errors.New(`"git" not found on PATH`) + } + return nil + } + _, err := ensureLocalImageWithOpts(context.Background(), "claude-code", opts) + if err == nil { + t.Fatalf("expected error for missing git") + } + if !strings.Contains(err.Error(), "git") { + t.Errorf("error = %v, want one mentioning git", err) + } + if !strings.Contains(err.Error(), "MOLECULE_IMAGE_REGISTRY") { + t.Errorf("error = %v, want one mentioning MOLECULE_IMAGE_REGISTRY", err) + } +} + // TestEnsureLocalImage_UnknownRuntime — the allowlist guard rejects // arbitrary runtime names before any network or filesystem call. func TestEnsureLocalImage_UnknownRuntime(t *testing.T) { From b95a20bb9ebcb35048116da20df33d84f4cbe5be Mon Sep 17 00:00:00 2001 From: Molecule AI Infra-SRE Date: Mon, 11 May 2026 18:45:39 +0000 Subject: [PATCH 2/2] fix(provisioner): fix type mismatch in checkTool seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checkToolOnPath must match the checkTool func(tool string) error signature in LocalBuildOptions — Go does not allow assigning a function with (string, error) returns to a func(string) error variable. Co-Authored-By: Claude Opus 4.7 --- workspace-server/internal/provisioner/localbuild.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/workspace-server/internal/provisioner/localbuild.go b/workspace-server/internal/provisioner/localbuild.go index a651b4a0..2a19feae 100644 --- a/workspace-server/internal/provisioner/localbuild.go +++ b/workspace-server/internal/provisioner/localbuild.go @@ -185,18 +185,19 @@ func EnsureLocalImage(ctx context.Context, runtime string) (string, error) { // production code. var ensureLocalImageHook = EnsureLocalImage -// checkToolOnPath verifies tool is on PATH and returns its absolute path, -// or a descriptive error. Used for pre-flight validation before the +// checkToolOnPath verifies tool is on PATH and returns an error with a +// descriptive message if missing. Used for pre-flight validation before the // clone/build cold path. -func checkToolOnPath(tool string) (string, error) { +func checkToolOnPath(tool string) error { path, err := exec.LookPath(tool) if err != nil { if errors.Is(err, exec.ErrNotFound) { - return "", fmt.Errorf("local-build: %q not found on PATH — local-build mode requires both docker and git; either install them, or set MOLECULE_IMAGE_REGISTRY so local-build is bypassed", tool) + return fmt.Errorf("%q not found on PATH — local-build mode requires both docker and git; either install them, or set MOLECULE_IMAGE_REGISTRY so local-build is bypassed", tool) } - return "", fmt.Errorf("local-build: LookPath(%q) failed: %w", tool, err) + return fmt.Errorf("LookPath(%q) failed: %w", tool, err) } - return path, nil + log.Printf("local-build: pre-flight OK (%s=%s)", tool, path) + return nil } func ensureLocalImageWithOpts(ctx context.Context, runtime string, opts *LocalBuildOptions) (string, error) {