diff --git a/docs/architecture.md b/docs/architecture.md index d223f85..a0258f4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -22,8 +22,8 @@ flowchart TD ws -->|yes| member["run hook per member
(member forge.toml)"] ws -->|no| files["gather files
(staged, or all-tracked for --all-files)"] - files --> pre{"all enabled tool
binaries present?"} - pre -->|no| prefail["fail hook
(install / SKIP_* / disable)"] + files --> pre{"all enabled tools runnable?
host binary on PATH /
container running"} + pre -->|no| prefail["fail hook
(install / start / SKIP_* / disable)"] pre -->|yes| mode{"parallel mode?"} mode -->|no| seq["runHookCfg
(sequential)"] mode -->|yes| par["runHookCfgParallel
(waves by depends_on)"] @@ -57,6 +57,6 @@ flowchart TD - **Tool order** follows declaration order in `forge.toml`. Go maps don't preserve order, so `config.parseHookToolOrder` regex-scans the raw TOML and feeds `OrderedToolNames()` — never rely on map iteration for ordering. - **Backend resolution precedence**: per-tool `backend` → `[execution].default_backend` → DDEV auto-detect (if the container is running) → host. -- **Preflight before execution**: every enabled (non-skipped) tool's binary must resolve before any tool runs; a missing host binary aborts the whole hook. Container backends are assumed to provide their own binaries. +- **Preflight before execution**: every enabled (non-skipped) tool must be runnable before any tool runs — host binaries must resolve on `PATH`, and container (ddev/docker) tools must have a running container (forge never starts one). Any failure aborts the whole hook. Container-running checks are memoized per run. - **Mutations happen only on success**: `restage` and `stage_outputs` run after a tool passes (never in `--check` mode); the run cache is only written for passing tools. - **Config precedence**: `FORGE_CONFIG` → repo `forge.toml`, with the global user config (`~/.config/forge/config.toml`) merged underneath — repo values always win. diff --git a/internal/forge/backend/backend.go b/internal/forge/backend/backend.go index cbf631a..d83d2f9 100644 --- a/internal/forge/backend/backend.go +++ b/internal/forge/backend/backend.go @@ -281,6 +281,33 @@ func (e *BackendAvailabilityError) Error() string { return fmt.Sprintf("tool %q not available via backend %q", e.Tool, e.Backend) } +// PreflightTool reports why a tool cannot run under its backend, or nil if it +// can. For container backends it checks that the container is running (it does +// NOT start it — a hook must not spin up infrastructure); for the host it +// checks that the binary resolves. Container-running checks are memoized, so +// this is cheap even when many tools share a container. +func PreflightTool(repoRoot, resolvedCmd string, b Backend) error { + switch bk := b.(type) { + case *DdevBackend: + name, err := ddevContainerName(repoRoot) + if err != nil { + return fmt.Errorf("ddev backend unavailable: %w", err) + } + if !isDockerContainerRunning(name) { + return fmt.Errorf("ddev container %q is not running — start it with `ddev start`", name) + } + case *DockerBackend: + if !isDockerContainerRunning(bk.container) { + return fmt.Errorf("container %q is not running", bk.container) + } + default: + if !ToolBinaryAvailable(repoRoot, resolvedCmd, b) { + return fmt.Errorf("binary not found: %s", resolvedCmd) + } + } + return nil +} + // ToolBinaryAvailable reports whether the resolved command path is accessible. func ToolBinaryAvailable(repoRoot, resolvedCmd string, backend Backend) bool { if _, isDdev := backend.(*DdevBackend); isDdev { diff --git a/internal/forge/backend/backend_test.go b/internal/forge/backend/backend_test.go index bc97ed9..550c224 100644 --- a/internal/forge/backend/backend_test.go +++ b/internal/forge/backend/backend_test.go @@ -278,6 +278,50 @@ func TestResolveCommandForBackend_SystemType_ReturnsCommandAsIs(t *testing.T) { } } +// ---------- PreflightTool ---------- + +func TestPreflightTool_DockerContainerDown(t *testing.T) { + // A container name that cannot be running (also covers docker-not-installed). + b := &DockerBackend{container: "forge-test-no-such-container-zzz"} + err := PreflightTool(t.TempDir(), "phpstan", b) + if err == nil { + t.Fatal("expected error when the docker container is not running") + } + if !strings.Contains(err.Error(), "not running") { + t.Errorf("error should explain the container is down, got: %v", err) + } +} + +func TestPreflightTool_DdevContainerDown(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, ".ddev"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".ddev", "config.yaml"), []byte("name: myproj\n"), 0o644); err != nil { + t.Fatal(err) + } + err := PreflightTool(dir, "phpstan", &DdevBackend{}) + if err == nil { + t.Fatal("expected error when the ddev container is not running") + } + if !strings.Contains(err.Error(), "ddev container") || !strings.Contains(err.Error(), "ddev start") { + t.Errorf("error should mention the ddev container and how to start it, got: %v", err) + } +} + +func TestPreflightTool_HostBinaryMissing(t *testing.T) { + err := PreflightTool(t.TempDir(), "forge-test-no-such-binary-zzz", &HostBackend{}) + if err == nil || !strings.Contains(err.Error(), "binary not found") { + t.Errorf("expected 'binary not found' error, got: %v", err) + } +} + +func TestPreflightTool_HostBinaryPresent(t *testing.T) { + if err := PreflightTool(t.TempDir(), "echo", &HostBackend{}); err != nil { + t.Errorf("expected nil for a present host binary, got: %v", err) + } +} + // ---------- ToolBinaryAvailable ---------- func TestToolBinaryAvailable_DdevBackend_AlwaysTrue(t *testing.T) { diff --git a/internal/forge/runner/review_fixes_test.go b/internal/forge/runner/review_fixes_test.go index 45c354e..d800fbc 100644 --- a/internal/forge/runner/review_fixes_test.go +++ b/internal/forge/runner/review_fixes_test.go @@ -106,8 +106,8 @@ func TestPreflightFailsOnMissingTool(t *testing.T) { if err == nil { t.Fatal("expected hard failure for a missing tool binary") } - if !contains(err.Error(), "missing tool") || !contains(err.Error(), "ghosttool") { - t.Errorf("error should name the missing tool, got: %v", err) + if !contains(err.Error(), "ghosttool") || !contains(err.Error(), "binary not found") { + t.Errorf("error should name the missing tool and reason, got: %v", err) } } diff --git a/internal/forge/runner/runner.go b/internal/forge/runner/runner.go index e3995bf..d48c5ad 100644 --- a/internal/forge/runner/runner.go +++ b/internal/forge/runner/runner.go @@ -172,7 +172,7 @@ func RunHookWithOptions(hookName string, editFile string, opts RunOptions) error // or disable the tool in forge.toml. func preflightTools(root string, hookCfg config.HookConfig, exec config.ExecutionConfig, toolNames []string) error { allowedGroups := parseAllowedGroups() - var missing []string + var problems []string for _, name := range toolNames { tool := hookCfg.Tools[name] if shouldSkipTool(name) || shouldSkipGroup(tool.Group) { @@ -188,13 +188,13 @@ func preflightTools(root string, hookCfg config.HookConfig, exec config.Executio } b := backend.ResolveBackend(root, tool, exec.DefaultBackend) resolvedCmd := backend.ResolveCommandForBackend(root, tool, b) - if !backend.ToolBinaryAvailable(root, resolvedCmd, b) { - missing = append(missing, fmt.Sprintf("%s (%s)", name, resolvedCmd)) + if err := backend.PreflightTool(root, resolvedCmd, b); err != nil { + problems = append(problems, fmt.Sprintf("%s — %s", name, err)) } } - if len(missing) > 0 { - return fmt.Errorf("missing tool binaries:\n - %s\ninstall them, set SKIP_=1 to skip, or disable the tool in forge.toml", - strings.Join(missing, "\n - ")) + if len(problems) > 0 { + return fmt.Errorf("cannot run hook, some tools are unavailable:\n - %s\ninstall/start them, set SKIP_=1 to skip, or disable the tool in forge.toml", + strings.Join(problems, "\n - ")) } return nil } diff --git a/website/guide/hooks.md b/website/guide/hooks.md index 2de0619..d15e2c2 100644 --- a/website/guide/hooks.md +++ b/website/guide/hooks.md @@ -111,21 +111,26 @@ HOOKS_ONLY=format git commit -m "..." ## Missing tools fail the hook -Before running anything, forge checks that every enabled tool's binary is available. If any is missing, the whole hook aborts **before** a single tool runs — so a mistyped command or an uninstalled linter can never silently pass as "all checks green". +Before running anything, forge checks that every enabled tool can actually run. If any can't, the whole hook aborts **before** a single tool runs — so a mistyped command or an uninstalled linter can never silently pass as "all checks green". ``` -run failed: missing tool binaries: - - eslint (node_modules/.bin/eslint) -install them, set SKIP_=1 to skip, or disable the tool in forge.toml +run failed: cannot run hook, some tools are unavailable: + - eslint — binary not found: node_modules/.bin/eslint +install/start them, set SKIP_=1 to skip, or disable the tool in forge.toml ``` You then have three ways forward: -- **Install** the tool. +- **Install** the tool (or start its container). - **Skip it for one run**: `SKIP_ESLINT=1 git commit …`. - **Disable it**: comment out or remove the tool's block in `forge.toml`. -Tools you've already skipped (`SKIP_*`, `--skip-tool`, a non-matching `HOOKS_ONLY` group) are exempt from the check. Tools running through a DDEV/Docker backend are assumed present in the container — only host binaries are verified. Run `forge doctor` to see availability without triggering a commit. +What's checked: + +- **Host tools** — the binary must resolve on `PATH` (or as a `vendor/bin` / `node_modules` path). +- **DDEV / Docker tools** — the container must be **running**. forge does **not** start it for you (a commit shouldn't spin up infrastructure); it fails fast with `container "…" is not running` so you can `ddev start` and retry. + +Tools you've already skipped (`SKIP_*`, `--skip-tool`, a non-matching `HOOKS_ONLY` group) are exempt. Run `forge doctor` to see availability without triggering a commit. ## See also