Merge pull request 'fix(provisioner): fail-fast pre-flight check for docker+git in local-build mode' (#536) from sre/fix-localbuild-preflight into main

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,21 @@ func EnsureLocalImage(ctx context.Context, runtime string) (string, error) {
 // production code.
 var ensureLocalImageHook = EnsureLocalImage
 
+// 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) error {
+	path, err := exec.LookPath(tool)
+	if err != nil {
+		if errors.Is(err, exec.ErrNotFound) {
+			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("LookPath(%q) failed: %w", tool, err)
+	}
+	log.Printf("local-build: pre-flight OK (%s=%s)", tool, path)
+	return 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 +209,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 {

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) {