Merge pull request 'feat(run): single-loop max-steps salvage + enforce MaxToolCalls' (#25) from feat/single-loop-salvage-max-tool-calls into main

Two gaps surfaced by a live fan-out smoke in a downstream host (mort):

1) A single-loop run that exhausts its step budget (ErrMaxSteps / ErrToolLoop)
   returned empty Output + a hard error, discarding all the reasoning it had
   produced — unlike the phased path, which already salvages a partial
   transcript and continues. Add opt-in single-loop salvage
   (Defaults.SalvageSingleLoopMaxSteps, default off): on budget exhaustion with
   empty Output, reconstruct a best-effort answer from the step narration
   (reusing salvagePhaseTranscript) and downgrade the run to a successful
   partial result. Off by default so a structured-output host keeps its clean
   hard error; an interactive host opts in.

2) RunnableAgent carried no tool-call ceiling — MaxIterations bounds STEPS, but
   one step can dispatch several tool calls, so a host's "max tool calls" cap
   had nowhere to land. Add RunnableAgent.MaxToolCalls (<=0 = unlimited,
   backward-compatible): the step observer counts executed calls and, once the
   cap is reached, stepCeilingOption forces the loop to exit; the resulting
   ErrMaxSteps is relabeled to the new ErrMaxToolCalls sentinel. Enforced
   single-loop only for now (phased is a follow-up). A cap hit is budget
   exhaustion, so salvage recovers its partial reasoning too.

isPhaseBudgetExhaustion becomes the shared isBudgetExhaustion (now also matching
ErrMaxToolCalls) so the phased and single-loop paths never drift, and the
RunStateAccessor now reports the real tool-call cap (was hardcoded 0).

Additive + backward-compatible: new optional DTO field, new opt-in Defaults
flag, new exported sentinel; unlimited/off reproduce prior behavior exactly. No
new dependencies (go mod tidy clean). Tests cover salvage on/off/no-prose, cap
enforcement + sentinel, cap+salvage, unlimited no-op, and cap-overrides-critic.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q1eJh5NVR11z6RzyxLANMK

run/agent.go

@@ -37,6 +37,15 @@ type RunnableAgent struct {
 	MaxIterations int
 	MaxRuntime    time.Duration
 
+	// MaxToolCalls caps the TOTAL number of tool calls across the whole run,
+	// independent of the step ceiling (one step can carry several parallel tool
+	// calls). 0 (or negative) = unlimited — the backward-compatible default, so
+	// a run that never sets it is unaffected. Enforced at step granularity: the
+	// loop stops after the step in which the cumulative tool-call count reaches
+	// the cap, surfacing ErrMaxToolCalls. Single-loop runs only today (phased
+	// pipelines are a follow-up).
+	MaxToolCalls int
+
 	// LowLevelTools are tool-registry names the run may call directly.
 	// SkillPalette / SubAgentPalette name saved skills / sub-agents exposed as
 	// skill__<name> / agent__<name> delegation tools, resolved through

run/critic.go

@@ -110,17 +110,35 @@ func (b *criticBinding) recordToolStart(name, args string) {
 	}
 }
 
-// maxStepsOption returns the agent step-ceiling Option. With no critic it's a
-// fixed WithMaxSteps(base); with a critic it's a DYNAMIC WithMaxStepsFunc that
-// polls the handle each step (so the critic can raise a long run's budget),
-// falling back to base when the handle defers (MaxSteps() <= 0).
-func (b *criticBinding) maxStepsOption(base int) agent.Option {
-	if b == nil {
-		return agent.WithMaxSteps(base)
-	}
+// stepCeilingOption returns the agent step-ceiling Option, folding the run's
+// tool-call cap into the (optionally critic-driven) step ceiling. Priority:
+//
+//  1. tool-call cap hit (maxCalls > 0 && *toolCalls >= maxCalls): return the
+//     completed-step count so majordomo's `stepIdx < ceiling` check fails on the
+//     next iteration and the loop exits (the executor relabels the resulting
+//     ErrMaxSteps to ErrMaxToolCalls). Never returns <= 0 — majordomo would fall
+//     back to its own static ceiling — so a cap hit before any step completes
+//     clamps to 1.
+//  2. the critic's DYNAMIC ceiling, when a critic is bound and raises a long
+//     run's budget mid-flight (b.h.MaxSteps() > 0).
+//  3. the static base (MaxIterations).
+//
+// With maxCalls <= 0 and no critic this is exactly the old WithMaxSteps(base):
+// an unlimited-tool-call run is a true no-op. nil-safe on the binding (b == nil
+// skips the critic branch). toolCalls/steps are read live from the step
+// observer's counters (same goroutine as this poll — see executor.go).
+func (b *criticBinding) stepCeilingOption(base, maxCalls int, toolCalls, steps *int) agent.Option {
 	return agent.WithMaxStepsFunc(func() int {
-		if n := b.h.MaxSteps(); n > 0 {
-			return n
+		if maxCalls > 0 && toolCalls != nil && *toolCalls >= maxCalls {
+			if steps != nil && *steps > 0 {
+				return *steps
+			}
+			return 1
+		}
+		if b != nil {
+			if n := b.h.MaxSteps(); n > 0 {
+				return n
+			}
 		}
 		return base
 	})

run/executor.go

@@ -41,6 +41,17 @@ type Defaults struct {
 	// absolute max. Never shorter than the run's MaxRuntime. Non-critic runs ignore
 	// it (they keep the literal MaxRuntime kill).
 	CriticAbsoluteMax time.Duration
+
+	// SalvageSingleLoopMaxSteps opts a single-loop run into partial-transcript
+	// salvage on budget exhaustion (ErrMaxSteps / ErrToolLoop / ErrMaxToolCalls):
+	// instead of returning empty Output + a hard error and discarding the
+	// reasoning it produced, the executor reconstructs a best-effort answer from
+	// the step narration (like the phased path) and downgrades the run to a
+	// successful partial result. Default false (off): a structured-output host
+	// (e.g. a bounded review swarm) keeps the clean hard error, where partial
+	// prose masquerading as a valid answer would be worse than a loud failure. A
+	// host that wants the reasoning recovered (interactive research) sets it true.
+	SalvageSingleLoopMaxSteps bool
 }
 
 func (d Defaults) withFallbacks() Defaults {
@@ -159,6 +170,7 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 	if maxRuntime <= 0 {
 		maxRuntime = e.cfg.Defaults.MaxRuntime
 	}
+	maxCalls := ra.MaxToolCalls // <= 0 => unlimited (no tool-call ceiling)
 
 	// Budget gate (pre-run): a rejected run makes no model call.
 	if e.cfg.Ports.Budget != nil {
@@ -203,7 +215,7 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 		rec = e.cfg.Ports.Audit.StartRun(ctx, info)
 	}
 	if rec != nil {
-		stateAcc = NewRunStateAccessor(rec, maxIter, 0, started)
+		stateAcc = NewRunStateAccessor(rec, maxIter, maxCalls, started)
 		inv.RunState = stateAcc
 	}
 
@@ -372,6 +384,14 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 	// calls with their executed results PAIRWISE — a result without a matching
 	// call (or a call without a result) is skipped rather than recorded as an
 	// empty-name "ghost" step.
+	// Tool-call budget accounting (single-loop enforcement, RunnableAgent.
+	// MaxToolCalls). The step observer runs on majordomo's Run goroutine — the
+	// SAME goroutine that reads the step ceiling — so plain ints suffice (no
+	// atomics). stepCeilingOption reads these to force the loop to exit once the
+	// cap is reached; the caller then relabels the resulting ErrMaxSteps to
+	// ErrMaxToolCalls. maxCalls <= 0 (unlimited) leaves this inert.
+	var toolCallsSeen, stepsSeen int
+	var capHit bool
 	emitter := newStepEmitter(inv.OnStep)
 	stepObserver := func(s agent.Step) {
 		if stateAcc != nil {
@@ -398,6 +418,21 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 				rec.OnTool(call, r.Content)
 			}
 		}
+		// Tool-call budget: accumulate executed calls and note the crossing once.
+		// Only single-loop runs enforce the cap (phased pipelines are a follow-up),
+		// so gate the crossing detection on Phases to avoid a misleading audit
+		// event on a phased run that merely happens to carry a cap.
+		toolCallsSeen += n
+		stepsSeen = s.Index + 1
+		if len(ra.Phases) == 0 && maxCalls > 0 && toolCallsSeen >= maxCalls && !capHit {
+			capHit = true
+			if rec != nil {
+				rec.LogEvent("max_tool_calls_reached", map[string]any{
+					"max_tool_calls": maxCalls,
+					"tool_calls":     toolCallsSeen,
+				})
+			}
+		}
 	}
 
 	// Shared agent options used by BOTH the single-loop path and every phase: the
@@ -480,7 +515,7 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 		}
 		opts := append([]agent.Option{
 			agent.WithToolbox(toolbox),
-			critic.maxStepsOption(maxIter),
+			critic.stepCeilingOption(maxIter, maxCalls, &toolCallsSeen, &stepsSeen),
 			agent.WithStepObserver(obs),
 		}, sharedOpts...)
 		ag := agent.New(model, e.systemPrompt(ra), opts...)
@@ -492,6 +527,15 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 			runRes, runErr = runAgent(runCtx, ag, input, inv.Images, agent.WithSteer(steer))
 		}
 
+		// A tool-call-cap exit surfaces from majordomo as ErrMaxSteps (we drove
+		// the step ceiling down to force the loop to stop). Relabel it so status,
+		// salvage, and audit report the real cause. A capped step always carries
+		// tool calls (a final-answer turn has zero), so capHit + ErrMaxSteps
+		// pinpoints a cap-triggered exit.
+		if capHit && errors.Is(runErr, agent.ErrMaxSteps) {
+			runErr = fmt.Errorf("%w (max %d)", ErrMaxToolCalls, maxCalls)
+		}
+
 		// ResultSchema (optional): validate the final answer and let the model
 		// self-repair on the same conversation before the host sees it. Runs
 		// BEFORE FinalGuard so the delivery check inspects the answer that will
@@ -570,6 +614,27 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 				}
 			}
 		}
+
+		// Single-loop budget-exhaustion salvage (opt-in). A loop that hit its
+		// step/tool-call budget without composing a final answer returns empty
+		// Output + a hard error, discarding the reasoning it DID produce. Mirror
+		// the phased path: reconstruct a best-effort answer from the step
+		// narration and downgrade the hard error to a successful partial result.
+		// This runs only after a budget error — ResultSchema + FinalGuard above
+		// are gated on runErr == nil, so neither fired. Gated on the host default
+		// so a structured-output host keeps its clean hard error.
+		if e.cfg.Defaults.SalvageSingleLoopMaxSteps &&
+			isBudgetExhaustion(runErr) && runRes != nil &&
+			strings.TrimSpace(runRes.Output) == "" {
+			if salvaged := salvagePhaseTranscript(runRes); salvaged != "" {
+				runRes.Output = salvaged +
+					"\n\n(Note: reached its step budget before composing a final answer; the above is its partial reasoning.)"
+				if rec != nil {
+					rec.LogEvent("single_loop_salvaged", map[string]any{"error": runErr.Error()})
+				}
+				runErr = nil // downgrade: status becomes "ok" and Output is delivered
+			}
+		}
 	} else {
 		// Multi-phase pipeline: each phase runs its own prompt/tier/tools/step-cap
 		// sequentially, threading outputs through {{.<PhaseName>}} templates. The

run/phases.go

@@ -156,7 +156,7 @@ func (e *Executor) runPhases(runCtx context.Context, ra RunnableAgent, deps phas
 					deps.rec.LogEvent("phase_failed_optional", map[string]any{"phase": phase.Name, "error": err.Error()})
 				}
 
-			case isPhaseBudgetExhaustion(err) && (!isLast || trimmed != ""):
+			case isBudgetExhaustion(err) && (!isLast || trimmed != ""):
 				// Soft stop: the phase ran out of its step/tool budget before
 				// composing a final answer. Not fatal — it did real work (runOnePhase
 				// salvaged its partial transcript into output), and aborting would
@@ -248,7 +248,7 @@ func (e *Executor) runOnePhase(runCtx context.Context, ra RunnableAgent, deps ph
 	}
 	// Budget/guard exhaustion leaves a usable partial transcript but an empty
 	// final answer; salvage the narrated work so the pipeline can carry it forward.
-	if runErr != nil && isPhaseBudgetExhaustion(runErr) {
+	if runErr != nil && isBudgetExhaustion(runErr) {
 		if salvaged := salvagePhaseTranscript(res); salvaged != "" {
 			output = salvaged
 		}
@@ -278,11 +278,15 @@ func (e *Executor) phaseModel(ctx context.Context, deps phaseDeps, ra RunnableAg
 	return modelCtx, m
 }
 
-// isPhaseBudgetExhaustion reports whether err is a soft budget/guard stop (the
-// loop hit its step cap or tripped a tool-error guard) — which leaves a usable
-// partial transcript — as opposed to a hard error (cancellation, model failure).
-func isPhaseBudgetExhaustion(err error) bool {
-	return errors.Is(err, agent.ErrMaxSteps) || errors.Is(err, agent.ErrToolLoop)
+// isBudgetExhaustion reports whether err is a soft budget/guard stop (the loop
+// hit its step cap, its tool-call cap, or tripped a tool-error guard) — which
+// leaves a usable partial transcript — as opposed to a hard error (cancellation,
+// model failure). Shared by the phased pipeline and the single-loop salvage in
+// executor.go, so the two paths never drift on what counts as exhaustion.
+func isBudgetExhaustion(err error) bool {
+	return errors.Is(err, agent.ErrMaxSteps) ||
+		errors.Is(err, agent.ErrToolLoop) ||
+		errors.Is(err, ErrMaxToolCalls)
 }
 
 // maxSalvageBytes bounds a salvaged partial transcript so a long phase's narrated

run/ports.go

@@ -16,6 +16,14 @@ import (
 // kill via KillCause(); the executor wraps that reason with this sentinel.
 var ErrCriticKill = errors.New("run: critic killed the run")
 
+// ErrMaxToolCalls is the cause when a run reaches its RunnableAgent.MaxToolCalls
+// budget before composing a final answer. It surfaces from majordomo as
+// ErrMaxSteps (the kernel forces the step ceiling down to stop the loop) and is
+// relabeled to this sentinel so audit + status report the real cause. Treated
+// as budget exhaustion by isBudgetExhaustion, so the single-loop salvage path
+// recovers a partial answer from it when enabled.
+var ErrMaxToolCalls = errors.New("run: max tool calls reached without a final answer")
+
 // Ports are the host seams the run executor consumes. Every field is nil-safe:
 // a light host passes the zero Ports and gets a bounded, in-memory run with no
 // persistence, audit, budget, critic, delegation, or delivery — which is

run/salvage_test.go

@@ -0,0 +1,250 @@
+package run_test
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strings"
+	"sync"
+	"testing"
+
+	"gitea.stevedudenhoeffer.com/steve/majordomo/agent"
+	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
+	"gitea.stevedudenhoeffer.com/steve/majordomo/provider/fake"
+
+	"gitea.stevedudenhoeffer.com/steve/executus/run"
+	"gitea.stevedudenhoeffer.com/steve/executus/tool"
+)
+
+// salvageNoopTool is an always-succeeds tool injected via Invocation.ExtraTools
+// so a scripted tool-call loop advances (and counts a real tool call) without
+// tripping the tool-error guard.
+func salvageNoopTool() []llm.Tool {
+	return []llm.Tool{{
+		Name:        "noop",
+		Description: "no-op",
+		Handler:     func(context.Context, json.RawMessage) (any, error) { return "ok", nil },
+	}}
+}
+
+// narratingLoop returns a fake provider whose every reply NARRATES a step and
+// calls noop with DISTINCT args (so the same-call-repeat guard never trips), so
+// the loop never finalizes and runs until its step / tool-call budget. Each
+// step's narration is recoverable by the salvage path.
+func narratingLoop() *fake.Provider {
+	var mu sync.Mutex
+	var n int
+	return fake.New("fake", fake.WithDefault(func(_ string, _ llm.Request) fake.Step {
+		mu.Lock()
+		n++
+		step := n
+		mu.Unlock()
+		return fake.ReplyWith(llm.Response{
+			Parts: []llm.Part{llm.Text(fmt.Sprintf("reasoning %d: partial finding %d", step, step))},
+			ToolCalls: []llm.ToolCall{{
+				ID:        fmt.Sprintf("c%d", step),
+				Name:      "noop",
+				Arguments: []byte(fmt.Sprintf(`{"n":%d}`, step)),
+			}},
+		})
+	}))
+}
+
+// silentLoop is like narratingLoop but writes NO prose — so there is nothing for
+// the salvage path to recover.
+func silentLoop() *fake.Provider {
+	var mu sync.Mutex
+	var n int
+	return fake.New("fake", fake.WithDefault(func(_ string, _ llm.Request) fake.Step {
+		mu.Lock()
+		n++
+		step := n
+		mu.Unlock()
+		return fake.ReplyWith(llm.Response{
+			ToolCalls: []llm.ToolCall{{
+				ID:        fmt.Sprintf("c%d", step),
+				Name:      "noop",
+				Arguments: []byte(fmt.Sprintf(`{"n":%d}`, step)),
+			}},
+		})
+	}))
+}
+
+func modelsFor(m llm.Model) run.ModelResolver {
+	return func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, m, nil }
+}
+
+// TestSalvageSingleLoop_OnRecoversPartialReasoning: with the salvage default ON,
+// a single-loop run that exhausts its step budget without a final answer is
+// downgraded to a successful PARTIAL result carrying its step narration, instead
+// of returning empty output + a hard ErrMaxSteps.
+func TestSalvageSingleLoop_OnRecoversPartialReasoning(t *testing.T) {
+	fp := narratingLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		Defaults: run.Defaults{SalvageSingleLoopMaxSteps: true},
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "researcher", ModelTier: "m", MaxIterations: 3},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"do research")
+
+	if res.Err != nil {
+		t.Fatalf("salvage ON: budget exhaustion should downgrade to success; got err %v", res.Err)
+	}
+	if !strings.Contains(res.Output, "partial finding") {
+		t.Errorf("salvaged output should contain the step narration; got %q", res.Output)
+	}
+	if !strings.Contains(res.Output, "step budget") {
+		t.Errorf("salvaged output should carry the partial-answer note; got %q", res.Output)
+	}
+}
+
+// TestSalvageSingleLoop_OffKeepsHardError: with salvage OFF (the zero-value
+// default), the same run returns empty output + a hard ErrMaxSteps — the
+// structured-output host's clean failure.
+func TestSalvageSingleLoop_OffKeepsHardError(t *testing.T) {
+	fp := narratingLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		// Defaults zero => SalvageSingleLoopMaxSteps false.
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 3},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if !errors.Is(res.Err, agent.ErrMaxSteps) {
+		t.Fatalf("salvage OFF: expected ErrMaxSteps, got %v", res.Err)
+	}
+	if strings.TrimSpace(res.Output) != "" {
+		t.Errorf("salvage OFF: output should be empty; got %q", res.Output)
+	}
+}
+
+// TestSalvageSingleLoop_NoProseStillErrors: salvage recovers nothing when the
+// loop wrote no prose, so a hard error stands even with salvage ON.
+func TestSalvageSingleLoop_NoProseStillErrors(t *testing.T) {
+	fp := silentLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		Defaults: run.Defaults{SalvageSingleLoopMaxSteps: true},
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 3},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if !errors.Is(res.Err, agent.ErrMaxSteps) {
+		t.Fatalf("no prose to salvage: ErrMaxSteps should stand, got %v", res.Err)
+	}
+	if strings.TrimSpace(res.Output) != "" {
+		t.Errorf("output should be empty; got %q", res.Output)
+	}
+}
+
+// TestMaxToolCalls_ForcesExitAndSurfacesSentinel: a run capped at 2 tool calls
+// stops after the 2nd (well before its high step ceiling) and surfaces the
+// distinct ErrMaxToolCalls sentinel.
+func TestMaxToolCalls_ForcesExitAndSurfacesSentinel(t *testing.T) {
+	fp := narratingLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		// salvage OFF so the raw sentinel is observable.
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 50, MaxToolCalls: 2},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if !errors.Is(res.Err, run.ErrMaxToolCalls) {
+		t.Fatalf("expected ErrMaxToolCalls, got %v", res.Err)
+	}
+	// It must NOT read as a plain step-budget exhaustion.
+	if !strings.Contains(res.Err.Error(), "max tool calls") {
+		t.Errorf("error should name the tool-call cap; got %q", res.Err.Error())
+	}
+}
+
+// TestMaxToolCalls_SalvagedWhenEnabled: a tool-call-cap exit is treated as budget
+// exhaustion, so with salvage ON it recovers the partial reasoning too.
+func TestMaxToolCalls_SalvagedWhenEnabled(t *testing.T) {
+	fp := narratingLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		Defaults: run.Defaults{SalvageSingleLoopMaxSteps: true},
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 50, MaxToolCalls: 2},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if res.Err != nil {
+		t.Fatalf("cap + salvage: expected downgraded success, got %v", res.Err)
+	}
+	if !strings.Contains(res.Output, "partial finding") {
+		t.Errorf("expected salvaged narration, got %q", res.Output)
+	}
+}
+
+// TestMaxToolCalls_ZeroIsUnbounded: MaxToolCalls <= 0 imposes no ceiling — a run
+// that makes several tool calls then finalizes completes normally (regression:
+// unlimited must be a true no-op).
+func TestMaxToolCalls_ZeroIsUnbounded(t *testing.T) {
+	fp := fake.New("fake")
+	fp.Enqueue("m",
+		fake.ReplyWith(llm.Response{ToolCalls: []llm.ToolCall{{ID: "c1", Name: "noop", Arguments: []byte(`{"n":1}`)}}}),
+		fake.ReplyWith(llm.Response{ToolCalls: []llm.ToolCall{{ID: "c2", Name: "noop", Arguments: []byte(`{"n":2}`)}}}),
+		fake.ReplyWith(llm.Response{ToolCalls: []llm.ToolCall{{ID: "c3", Name: "noop", Arguments: []byte(`{"n":3}`)}}}),
+		fake.Reply("final answer after 3 tools"),
+	)
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 10, MaxToolCalls: 0},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if res.Err != nil {
+		t.Fatalf("unbounded run should complete: %v", res.Err)
+	}
+	if res.Output != "final answer after 3 tools" {
+		t.Errorf("output = %q, want the final answer", res.Output)
+	}
+}
+
+// TestMaxToolCalls_CapOverridesCriticRaise: the tool-call cap is a HARD budget —
+// it stops the run even when a critic is willing to raise the step ceiling far
+// past it. Also proves stepCeilingOption composes with a critic.
+func TestMaxToolCalls_CapOverridesCriticRaise(t *testing.T) {
+	h := &fakeCriticHandle{maxSteps: 50}
+	fp := narratingLoop()
+	m, _ := fp.Model("m")
+	ex := run.New(run.Config{
+		Registry: tool.NewRegistry(),
+		Models:   modelsFor(m),
+		Ports:    run.Ports{Critic: &fakeCritic{h: h}},
+	})
+	res := ex.Run(context.Background(),
+		run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 1, MaxToolCalls: 2, Critic: run.CriticConfig{Enabled: true}},
+		tool.Invocation{RunID: "r", ExtraTools: salvageNoopTool()},
+		"go")
+
+	if !errors.Is(res.Err, run.ErrMaxToolCalls) {
+		t.Fatalf("the tool-call cap must override the critic's raised ceiling; got %v", res.Err)
+	}
+}