diff --git a/workspace-server/internal/provisioner/localbuild.go b/workspace-server/internal/provisioner/localbuild.go index 9f1fcf5d..2a19feae 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,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 { 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) {