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
251 lines
8.7 KiB
Go
251 lines
8.7 KiB
Go
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)
|
|
}
|
|
}
|