Patch Tuesday: Tool aggregation

.repertoire.yaml

@@ -0,0 +1,9 @@
+schema: 1
+tool: https://github.com/phillarmonic/repertoire-ai
+skills:
+    github.com/phillarmonic/ai-skills/common-stubs:
+        scope: global
+    github.com/phillarmonic/ai-skills/repertoire:
+        scope: global
+    github.com/phillarmonic/ai-skills/zensical:
+        scope: global

internal/engine/executor_requires_tools.go

@@ -2,6 +2,7 @@ package engine
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -27,10 +28,50 @@ type versionMismatch struct {
 	constraint     statement.VersionConstraint
 }
 
+// toolFailureKind classifies a failed tool requirement check so the
+// aggregated report can group failures by category.
+type toolFailureKind int
+
+const (
+	toolFailureMissing toolFailureKind = iota
+	toolFailureVersionMismatch
+	toolFailureOther
+)
+
+// toolCheckFailure describes a single failed tool requirement check.
+type toolCheckFailure struct {
+	kind       toolFailureKind
+	tool       string
+	current    string // installed version, for version mismatches
+	constraint string // unmet constraint, for version mismatches
+	// needsAllowFlag marks mismatches where provisioning was refused because
+	// --allow-tool-version-changes was not passed.
+	needsAllowFlag bool
+	err            error // underlying error, for toolFailureOther
+}
+
+// Error renders the failure as a single-line message, matching the wording
+// used before failures were aggregated.
+func (f *toolCheckFailure) Error() string {
+	switch f.kind {
+	case toolFailureMissing:
+		return fmt.Sprintf("required tool '%s' is not installed", f.tool)
+	case toolFailureVersionMismatch:
+		msg := fmt.Sprintf("required tool '%s' version %s does not satisfy constraint %s", f.tool, f.current, f.constraint)
+		if f.needsAllowFlag {
+			msg += "; rerun with --allow-tool-version-changes to allow provisioning to change installed versions"
+		}
+		return msg
+	default:
+		return f.err.Error()
+	}
+}
+
 // Domain: Tool Requirements Execution
 // This file contains the executor for "requires tools:" blocks.
-// Tool requirements are checked eagerly — if a tool is missing or doesn't
-// meet the version constraints, execution fails immediately.
+// Tool requirements are checked eagerly — every tool is checked and all
+// missing tools and unmet version constraints are reported together in a
+// single error, so users can fix everything in one pass.
 
 // executeRequiresTools checks that all required tools are available and meet version constraints.
 func (e *Engine) executeRequiresTools(stmt *statement.RequiresTools, ctx *ExecutionContext) error {
@@ -47,14 +88,24 @@ func (e *Engine) executeRequiresTools(stmt *statement.RequiresTools, ctx *Execut
 
 // checkToolRequirements validates a list of tool requirements against the system.
 // This is shared between task-level execution and project-level startup checks.
+// All tools are checked before returning: failures are aggregated and reported
+// together, grouped by missing tools and unmet version constraints.
 func (e *Engine) checkToolRequirements(detector toolDetector, tools []statement.ToolRequirement, projectCtx *ProjectContext, execCtx *ExecutionContext) error {
+	var failures []toolCheckFailure
+	seen := make(map[string]bool)
 	for _, tool := range tools {
-		if err := e.checkSingleToolRequirement(detector, tool, projectCtx, execCtx); err != nil {
-			return err
+		// Direct requirements precede inherited ones, so the first occurrence
+		// of a tool wins (a direct requirement overrides an inherited one).
+		if seen[tool.Name] {
+			continue
+		}
+		seen[tool.Name] = true
+		if failure := e.checkSingleToolRequirement(detector, tool, projectCtx, execCtx); failure != nil {
+			failures = append(failures, *failure)
 		}
 	}
 
-	return nil
+	return formatToolCheckFailures(failures)
 }
 
 // checkProjectToolRequirements checks project-level tool requirements at startup.
@@ -74,20 +125,26 @@ func (e *Engine) checkProjectToolRequirements(projectCtx *ProjectContext) error
 	return e.checkToolRequirements(e.newToolDetector(), projectCtx.RequiredTools, projectCtx, nil)
 }
 
-func (e *Engine) checkSingleToolRequirement(detector toolDetector, tool statement.ToolRequirement, projectCtx *ProjectContext, execCtx *ExecutionContext) error {
+// checkSingleToolRequirement checks one tool and returns a structured failure
+// (nil when the requirement is satisfied). Auto-provisioning still happens
+// inline per tool; only the reporting is aggregated by the caller.
+func (e *Engine) checkSingleToolRequirement(detector toolDetector, tool statement.ToolRequirement, projectCtx *ProjectContext, execCtx *ExecutionContext) *toolCheckFailure {
 	if !detector.IsToolAvailable(tool.Name) {
 		if tool.AutoProvision {
 			if e.dryRun {
 				_, _ = fmt.Fprintf(e.output, "[DRY RUN] 🔧 Would provision missing tool '%s'\n", tool.Name)
 				return nil
 			}
-			return e.provisionAndRecheck(tool, projectCtx, execCtx, "required tool is not installed")
+			if err := e.provisionAndRecheck(tool, projectCtx, execCtx, "required tool is not installed"); err != nil {
+				return &toolCheckFailure{kind: toolFailureOther, tool: tool.Name, err: err}
+			}
+			return nil
 		}
 		if e.dryRun {
 			_, _ = fmt.Fprintf(e.output, "[DRY RUN] ❌ Required tool '%s' is not installed\n", tool.Name)
 			return nil
 		}
-		return fmt.Errorf("required tool '%s' is not installed", tool.Name)
+		return &toolCheckFailure{kind: toolFailureMissing, tool: tool.Name}
 	}
 
 	currentVersion, mismatch, err := evaluateToolVersion(detector, tool)
@@ -96,32 +153,45 @@ func (e *Engine) checkSingleToolRequirement(detector toolDetector, tool statemen
 			_, _ = fmt.Fprintf(e.output, "[DRY RUN] ⚠️  Could not determine version for '%s'\n", tool.Name)
 			return nil
 		}
-		return err
+		return &toolCheckFailure{kind: toolFailureOther, tool: tool.Name, err: err}
 	}
 
 	if mismatch != nil {
+		constraint := mismatch.constraint.Operator + " " + mismatch.constraint.Version
 		if tool.AutoProvision {
 			if e.dryRun {
-				_, _ = fmt.Fprintf(e.output, "[DRY RUN] 🔧 Would provision '%s' to satisfy %s %s\n",
-					tool.Name, mismatch.constraint.Operator, mismatch.constraint.Version)
+				_, _ = fmt.Fprintf(e.output, "[DRY RUN] 🔧 Would provision '%s' to satisfy %s\n",
+					tool.Name, constraint)
 				return nil
 			}
 			if !e.allowToolVersionChanges {
-				_, _ = fmt.Fprintf(e.output, "⚠️  Tool '%s' version %s does not satisfy %s %s; refusing to change the installed version without --allow-tool-version-changes\n",
-					tool.Name, mismatch.currentVersion, mismatch.constraint.Operator, mismatch.constraint.Version)
-				return fmt.Errorf("required tool '%s' version %s does not satisfy constraint %s %s; rerun with --allow-tool-version-changes to allow provisioning to change installed versions",
-					tool.Name, mismatch.currentVersion, mismatch.constraint.Operator, mismatch.constraint.Version)
+				_, _ = fmt.Fprintf(e.output, "⚠️  Tool '%s' version %s does not satisfy %s; refusing to change the installed version without --allow-tool-version-changes\n",
+					tool.Name, mismatch.currentVersion, constraint)
+				return &toolCheckFailure{
+					kind:           toolFailureVersionMismatch,
+					tool:           tool.Name,
+					current:        mismatch.currentVersion,
+					constraint:     constraint,
+					needsAllowFlag: true,
+				}
 			}
-			return e.provisionAndRecheck(tool, projectCtx, execCtx,
-				fmt.Sprintf("tool version %s does not satisfy %s %s", mismatch.currentVersion, mismatch.constraint.Operator, mismatch.constraint.Version))
+			if err := e.provisionAndRecheck(tool, projectCtx, execCtx,
+				fmt.Sprintf("tool version %s does not satisfy %s", mismatch.currentVersion, constraint)); err != nil {
+				return &toolCheckFailure{kind: toolFailureOther, tool: tool.Name, err: err}
+			}
+			return nil
 		}
 		if e.dryRun {
-			_, _ = fmt.Fprintf(e.output, "[DRY RUN] ❌ Tool '%s' version %s does not satisfy %s %s\n",
-				tool.Name, mismatch.currentVersion, mismatch.constraint.Operator, mismatch.constraint.Version)
+			_, _ = fmt.Fprintf(e.output, "[DRY RUN] ❌ Tool '%s' version %s does not satisfy %s\n",
+				tool.Name, mismatch.currentVersion, constraint)
 			return nil
 		}
-		return fmt.Errorf("required tool '%s' version %s does not satisfy constraint %s %s",
-			tool.Name, mismatch.currentVersion, mismatch.constraint.Operator, mismatch.constraint.Version)
+		return &toolCheckFailure{
+			kind:       toolFailureVersionMismatch,
+			tool:       tool.Name,
+			current:    mismatch.currentVersion,
+			constraint: constraint,
+		}
 	}
 
 	if len(tool.Constraints) > 0 {
@@ -138,6 +208,51 @@ func (e *Engine) checkSingleToolRequirement(detector toolDetector, tool statemen
 	return nil
 }
 
+// formatToolCheckFailures renders all failed tool checks as a single error,
+// grouping missing tools, unmet version constraints, and other failures so
+// users can fix everything in one pass instead of one tool per run.
+func formatToolCheckFailures(failures []toolCheckFailure) error {
+	if len(failures) == 0 {
+		return nil
+	}
+
+	var missing, mismatched, other []string
+	for i := range failures {
+		f := &failures[i]
+		switch f.kind {
+		case toolFailureMissing:
+			missing = append(missing, f.Error())
+		case toolFailureVersionMismatch:
+			mismatched = append(mismatched, f.Error())
+		default:
+			other = append(other, f.err.Error())
+		}
+	}
+
+	var b strings.Builder
+	b.WriteString("tool requirements not met:\n")
+	if len(missing) > 0 {
+		b.WriteString("\nmissing tools:\n")
+		for _, entry := range missing {
+			fmt.Fprintf(&b, "  • %s\n", entry)
+		}
+	}
+	if len(mismatched) > 0 {
+		b.WriteString("\nunmet version requirements:\n")
+		for _, entry := range mismatched {
+			fmt.Fprintf(&b, "  • %s\n", entry)
+		}
+	}
+	if len(other) > 0 {
+		b.WriteString("\nother failures:\n")
+		for _, entry := range other {
+			fmt.Fprintf(&b, "  • %s\n", entry)
+		}
+	}
+
+	return errors.New(strings.TrimRight(b.String(), "\n"))
+}
+
 func evaluateToolVersion(detector toolDetector, tool statement.ToolRequirement) (string, *versionMismatch, error) {
 	if len(tool.Constraints) == 0 {
 		return "", nil, nil
@@ -183,8 +298,8 @@ func (e *Engine) provisionAndRecheck(tool statement.ToolRequirement, projectCtx
 	}
 
 	refreshedDetector := e.newToolDetector()
-	if err := e.checkSingleToolRequirement(refreshedDetector, withoutAutoProvision(tool), projectCtx, execCtx); err != nil {
-		return fmt.Errorf("post-provision check for tool '%s' failed: %w", tool.Name, err)
+	if failure := e.checkSingleToolRequirement(refreshedDetector, withoutAutoProvision(tool), projectCtx, execCtx); failure != nil {
+		return fmt.Errorf("post-provision check for tool '%s' failed: %w", tool.Name, failure)
 	}
 
 	if e.verbose {

internal/engine/executor_requires_tools_test.go

@@ -211,6 +211,82 @@ func TestEngine_checkToolRequirements_PostProvisionRecheckFailure(t *testing.T)
 	}
 }
 
+func TestEngine_checkToolRequirements_AggregatesAllFailures(t *testing.T) {
+	e := NewEngine(io.Discard)
+
+	var checked []string
+	detector := &recordingToolDetector{
+		available: map[string]bool{
+			"missing-one": false,
+			"missing-two": false,
+			"go":          true,
+		},
+		versions: map[string]string{"go": "1.20.5"},
+		checked:  &checked,
+	}
+
+	err := e.checkToolRequirements(
+		detector,
+		[]statement.ToolRequirement{
+			{Name: "missing-one"},
+			{
+				Name: "go",
+				Constraints: []statement.VersionConstraint{
+					{Operator: ">=", Version: "1.21"},
+				},
+			},
+			{Name: "missing-two"},
+		},
+		&ProjectContext{},
+		nil,
+	)
+	if err == nil {
+		t.Fatalf("expected error, got nil")
+	}
+
+	// Every tool must be checked even though earlier ones failed.
+	assertCheckedTools(t, checked, []string{"missing-one", "go", "missing-two"})
+
+	// All failures must be reported together in a single error.
+	msg := err.Error()
+	for _, want := range []string{
+		"missing tools:",
+		"required tool 'missing-one' is not installed",
+		"required tool 'missing-two' is not installed",
+		"unmet version requirements:",
+		"required tool 'go' version 1.20.5 does not satisfy constraint >= 1.21",
+	} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("expected aggregated error containing %q, got:\n%s", want, msg)
+		}
+	}
+}
+
+func TestEngine_checkToolRequirements_DirectRequirementOverridesDuplicate(t *testing.T) {
+	e := NewEngine(io.Discard)
+
+	var checked []string
+	detector := &recordingToolDetector{
+		available: map[string]bool{"shared-tool": true},
+		versions:  map[string]string{"shared-tool": "1.5.0"},
+		checked:   &checked,
+	}
+
+	err := e.checkToolRequirements(
+		detector,
+		[]statement.ToolRequirement{
+			{Name: "shared-tool", Constraints: []statement.VersionConstraint{{Operator: ">=", Version: "1.0"}}},
+			{Name: "shared-tool", Constraints: []statement.VersionConstraint{{Operator: ">=", Version: "2.0"}}},
+		},
+		&ProjectContext{},
+		nil,
+	)
+	if err != nil {
+		t.Fatalf("expected first requirement to win, got error: %v", err)
+	}
+	assertCheckedTools(t, checked, []string{"shared-tool"})
+}
+
 func TestEngine_ExecuteTaskRequiresToolsInheritanceChecksInheritedTools(t *testing.T) {
 	program, err := ParseString(`version: 2.0