feat(run): critic owns the deadline — MaxRuntime becomes the soft trigger
When a run enables the critic (Ports.Critic set + RunnableAgent.Critic.Enabled), the kernel no longer hard-caps it at MaxRuntime. MaxRuntime becomes the SOFT trigger (passed to startCritic, used by the host critic as its wake + the base for its extendable backstop); the critic's deadline-watch is the real hard cancel. This restores mort's old agentexec two-tier timeout semantics — a slow-but-progressing run (e.g. a parent agent blocked on a 30-min animate render) is given room up to the critic's backstop instead of being killed at the nominal MaxRuntime. Specifics: - run/executor.go: the WithTimeout(MaxRuntime) is now conditional. Non-critic runs keep the literal MaxRuntime kill (→ "timeout"). Critic-owned runs get a GENEROUS WithTimeout at the new Defaults.CriticAbsoluteMax (default 6h) as a failsafe ceiling only — it never fires before the critic's backstop, and it guarantees a broken/nil host handle can't run unbounded. - run/critic.go: startCritic takes the resolved MaxRuntime as the soft trigger (falling back to Defaults.CriticSoftTimeout, then 90s), instead of always using the global CriticSoftTimeout. - Defaults.CriticAbsoluteMax added (withFallbacks default 6h). - Tests: non-critic dies at MaxRuntime; critic-owned survives past it; soft trigger == MaxRuntime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jo75sqmeVPgFUWZQBn179X
This commit is contained in:
+10
-2
@@ -31,12 +31,20 @@ type criticBinding struct {
|
|||||||
// "killed"); when the backstop simply expired, it is context.DeadlineExceeded (→
|
// "killed"); when the backstop simply expired, it is context.DeadlineExceeded (→
|
||||||
// "timeout"). Returns (nil, no-op stop) when there is no critic. The caller MUST
|
// "timeout"). Returns (nil, no-op stop) when there is no critic. The caller MUST
|
||||||
// defer the returned stop.
|
// defer the returned stop.
|
||||||
func (e *Executor) startCritic(runCtx context.Context, cancelCause context.CancelCauseFunc, ra RunnableAgent, info RunInfo) (*criticBinding, func()) {
|
//
|
||||||
|
// softTrigger is the run's resolved MaxRuntime: for a critic-owned run MaxRuntime
|
||||||
|
// is the soft wake (mort's two-tier semantics — the critic first reviews once the
|
||||||
|
// run exceeds its nominal budget, and its backstop = softTrigger × multiplier).
|
||||||
|
// It falls back to the configured CriticSoftTimeout when the run set no MaxRuntime.
|
||||||
|
func (e *Executor) startCritic(runCtx context.Context, cancelCause context.CancelCauseFunc, ra RunnableAgent, info RunInfo, softTrigger time.Duration) (*criticBinding, func()) {
|
||||||
noop := func() {}
|
noop := func() {}
|
||||||
if e.cfg.Ports.Critic == nil || !ra.Critic.Enabled {
|
if e.cfg.Ports.Critic == nil || !ra.Critic.Enabled {
|
||||||
return nil, noop
|
return nil, noop
|
||||||
}
|
}
|
||||||
soft := e.cfg.Defaults.CriticSoftTimeout
|
soft := softTrigger
|
||||||
|
if soft <= 0 {
|
||||||
|
soft = e.cfg.Defaults.CriticSoftTimeout
|
||||||
|
}
|
||||||
if soft <= 0 {
|
if soft <= 0 {
|
||||||
soft = 90 * time.Second // defensive: withFallbacks normally guarantees >0
|
soft = 90 * time.Second // defensive: withFallbacks normally guarantees >0
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,131 @@
|
|||||||
|
package run_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"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"
|
||||||
|
)
|
||||||
|
|
||||||
|
// slowToolInvocation builds an Invocation whose session factory adds a "slow"
|
||||||
|
// tool that sleeps for d (respecting ctx). The model script calls it once, then
|
||||||
|
// answers — so the run's wall-clock is dominated by d, letting a test set a tiny
|
||||||
|
// MaxRuntime and observe whether MaxRuntime hard-cancels the run.
|
||||||
|
func slowToolInvocation(runID string, d time.Duration) tool.Invocation {
|
||||||
|
slow := llm.DefineTool("slow", "sleeps for a while",
|
||||||
|
func(ctx context.Context, _ struct{}) (any, error) {
|
||||||
|
select {
|
||||||
|
case <-time.After(d):
|
||||||
|
return "ok", nil
|
||||||
|
case <-ctx.Done():
|
||||||
|
return nil, ctx.Err()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
return tool.Invocation{
|
||||||
|
RunID: runID,
|
||||||
|
SessionToolFactory: func(_ tool.AgentSession) tool.SessionTools {
|
||||||
|
return tool.SessionTools{Tools: []llm.Tool{slow}}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func slowModel() llm.Model {
|
||||||
|
fp := fake.New("fake")
|
||||||
|
fp.Enqueue("m",
|
||||||
|
fake.ReplyWith(llm.Response{ToolCalls: []llm.ToolCall{{ID: "c1", Name: "slow", Arguments: []byte(`{}`)}}}),
|
||||||
|
fake.Reply("done"),
|
||||||
|
)
|
||||||
|
m, _ := fp.Model("m")
|
||||||
|
return m
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestNoCritic_MaxRuntimeIsHardCap: the legacy contract is preserved — without a
|
||||||
|
// critic, MaxRuntime is a literal WithTimeout that kills a run whose work outlasts
|
||||||
|
// it. The slow tool (200ms) outlasts MaxRuntime (20ms), so runCtx cancels mid-tool
|
||||||
|
// and the run ends in error (timeout).
|
||||||
|
func TestNoCritic_MaxRuntimeIsHardCap(t *testing.T) {
|
||||||
|
m := slowModel()
|
||||||
|
ex := run.New(run.Config{
|
||||||
|
Registry: tool.NewRegistry(),
|
||||||
|
Models: func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, m, nil },
|
||||||
|
})
|
||||||
|
res := ex.Run(context.Background(),
|
||||||
|
run.RunnableAgent{Name: "x", ModelTier: "m", MaxIterations: 5, MaxRuntime: 20 * time.Millisecond},
|
||||||
|
slowToolInvocation("r", 200*time.Millisecond), "go")
|
||||||
|
if res.Err == nil {
|
||||||
|
t.Fatalf("non-critic run should hard-timeout at MaxRuntime; got output=%q err=nil", res.Output)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCriticOwnsDeadline_SurvivesPastMaxRuntime: the fix — when the critic owns the
|
||||||
|
// deadline (Ports.Critic set + Critic.Enabled), MaxRuntime becomes the SOFT trigger
|
||||||
|
// and is NOT a hard cap. The fake critic exposes no hard deadline (Deadline()==zero,
|
||||||
|
// no kill), so the only hard ceiling is CriticAbsoluteMax (10s here). The slow tool
|
||||||
|
// (200ms) outlasts the tiny MaxRuntime (20ms) but the run completes — proving the
|
||||||
|
// old agentexec two-tier semantics are restored.
|
||||||
|
func TestCriticOwnsDeadline_SurvivesPastMaxRuntime(t *testing.T) {
|
||||||
|
m := slowModel()
|
||||||
|
h := &fakeCriticHandle{} // Deadline()==zero → no hard deadline, no kill
|
||||||
|
ex := run.New(run.Config{
|
||||||
|
Registry: tool.NewRegistry(),
|
||||||
|
Models: func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, m, nil },
|
||||||
|
Ports: run.Ports{Critic: &fakeCritic{h: h}},
|
||||||
|
Defaults: run.Defaults{CriticAbsoluteMax: 10 * time.Second},
|
||||||
|
})
|
||||||
|
res := ex.Run(context.Background(),
|
||||||
|
run.RunnableAgent{Name: "watched", ModelTier: "m", MaxIterations: 5, MaxRuntime: 20 * time.Millisecond,
|
||||||
|
Critic: run.CriticConfig{Enabled: true}},
|
||||||
|
slowToolInvocation("r", 200*time.Millisecond), "go")
|
||||||
|
if res.Err != nil {
|
||||||
|
t.Fatalf("critic-owned run must survive past MaxRuntime (soft trigger); got err=%v", res.Err)
|
||||||
|
}
|
||||||
|
if res.Output != "done" {
|
||||||
|
t.Errorf("output = %q, want %q", res.Output, "done")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// capturingCritic records the soft trigger the executor passes to Monitor.
|
||||||
|
type capturingCritic struct {
|
||||||
|
mu sync.Mutex
|
||||||
|
soft time.Duration
|
||||||
|
h run.CriticHandle
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *capturingCritic) Monitor(_ context.Context, _ run.RunInfo, soft time.Duration) run.CriticHandle {
|
||||||
|
c.mu.Lock()
|
||||||
|
c.soft = soft
|
||||||
|
c.mu.Unlock()
|
||||||
|
return c.h
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCriticSoftTriggerIsMaxRuntime: the soft trigger handed to the host critic is
|
||||||
|
// the run's resolved MaxRuntime (mort's two-tier model — the critic first wakes once
|
||||||
|
// the run exceeds its nominal budget), NOT the global Defaults.CriticSoftTimeout.
|
||||||
|
func TestCriticSoftTriggerIsMaxRuntime(t *testing.T) {
|
||||||
|
fp := fake.New("fake")
|
||||||
|
fp.Enqueue("m", fake.Reply("done"))
|
||||||
|
m, _ := fp.Model("m")
|
||||||
|
cc := &capturingCritic{h: &fakeCriticHandle{}}
|
||||||
|
ex := run.New(run.Config{
|
||||||
|
Registry: tool.NewRegistry(),
|
||||||
|
Models: func(ctx context.Context, _ string) (context.Context, llm.Model, error) { return ctx, m, nil },
|
||||||
|
Ports: run.Ports{Critic: cc},
|
||||||
|
Defaults: run.Defaults{CriticSoftTimeout: 90 * time.Second}, // distinct from MaxRuntime below
|
||||||
|
})
|
||||||
|
const wantSoft = 7 * time.Minute
|
||||||
|
ex.Run(context.Background(),
|
||||||
|
run.RunnableAgent{Name: "x", ModelTier: "m", MaxRuntime: wantSoft, Critic: run.CriticConfig{Enabled: true}},
|
||||||
|
tool.Invocation{RunID: "r"}, "go")
|
||||||
|
cc.mu.Lock()
|
||||||
|
got := cc.soft
|
||||||
|
cc.mu.Unlock()
|
||||||
|
if got != wantSoft {
|
||||||
|
t.Errorf("soft trigger = %v, want the agent's MaxRuntime %v (not Defaults.CriticSoftTimeout)", got, wantSoft)
|
||||||
|
}
|
||||||
|
}
|
||||||
+45
-15
@@ -30,6 +30,13 @@ type Defaults struct {
|
|||||||
MaxSameToolCallRepeats int // retry-storm guard; default 3
|
MaxSameToolCallRepeats int // retry-storm guard; default 3
|
||||||
CompactionThresholdRatio float64 // fraction of model context to compact at; default 0.7
|
CompactionThresholdRatio float64 // fraction of model context to compact at; default 0.7
|
||||||
CriticSoftTimeout time.Duration // idle window before the critic wakes; default 90s
|
CriticSoftTimeout time.Duration // idle window before the critic wakes; default 90s
|
||||||
|
// CriticAbsoluteMax is the failsafe wall-clock ceiling for a critic-OWNED run
|
||||||
|
// (Ports.Critic set AND the agent enables it). For such a run MaxRuntime is the
|
||||||
|
// SOFT trigger, not a hard cap, and the critic's extendable backstop is the
|
||||||
|
// normal deadline — so this ceiling only fires if the critic never acts (a
|
||||||
|
// broken/nil host handle). Default 6h; never shorter than the run's MaxRuntime.
|
||||||
|
// Non-critic runs ignore it (they keep the literal MaxRuntime kill).
|
||||||
|
CriticAbsoluteMax time.Duration
|
||||||
}
|
}
|
||||||
|
|
||||||
func (d Defaults) withFallbacks() Defaults {
|
func (d Defaults) withFallbacks() Defaults {
|
||||||
@@ -54,6 +61,9 @@ func (d Defaults) withFallbacks() Defaults {
|
|||||||
if d.CriticSoftTimeout <= 0 {
|
if d.CriticSoftTimeout <= 0 {
|
||||||
d.CriticSoftTimeout = 90 * time.Second
|
d.CriticSoftTimeout = 90 * time.Second
|
||||||
}
|
}
|
||||||
|
if d.CriticAbsoluteMax <= 0 {
|
||||||
|
d.CriticAbsoluteMax = 6 * time.Hour
|
||||||
|
}
|
||||||
return d
|
return d
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -265,18 +275,36 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
|
|||||||
postRun = st.PostRun
|
postRun = st.PostRun
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run context: bound by MaxRuntime, detached from the caller's deadline so a
|
// Run context: detached from the caller's deadline so a lane/queue wait doesn't
|
||||||
// lane/queue wait doesn't eat the run budget (mort's V10 lesson). Caller
|
// eat the run budget (mort's V10 lesson). Caller cancellation still propagates
|
||||||
// cancellation still propagates via MergeCancellation. Created BEFORE the
|
// via MergeCancellation. Created BEFORE the step observer so the observer
|
||||||
// step observer so the observer forwards the merged run context (not a
|
// forwards the merged run context (not a possibly-cancelled caller ctx) to
|
||||||
// possibly-cancelled caller ctx) to OnStep consumers.
|
// OnStep consumers.
|
||||||
// MaxRuntime stays a WithTimeout so its DeadlineExceeded propagates through the
|
//
|
||||||
// child chain (→ "timeout"), preserving the run's-own-timeout vs caller-cancel
|
// Two-tier timeout: who owns the hard deadline depends on the critic.
|
||||||
// distinction. A NESTED cause-carrying layer lets a critic kill surface as a
|
// - NO critic (the default): MaxRuntime is a literal WithTimeout. Its
|
||||||
// distinct "killed" without disturbing that: only an ErrCriticKill cause is
|
// DeadlineExceeded propagates through the child chain (→ "timeout"),
|
||||||
// consulted in statusFor; a generic run error or a caller cancel is classified
|
// preserving the run's-own-timeout vs caller-cancel distinction.
|
||||||
// by the run error itself.
|
// - critic OWNS the deadline (Ports.Critic set + ra.Critic.Enabled):
|
||||||
timeoutCtx, cancelTimeout := context.WithTimeout(context.WithoutCancel(ctx), maxRuntime)
|
// MaxRuntime becomes the SOFT trigger (passed to startCritic), and the
|
||||||
|
// critic's extendable backstop — watched in startCritic, which cancels via
|
||||||
|
// cancelCause — is the real deadline. A slow-but-progressing run is given
|
||||||
|
// room up to the backstop; only a stalled one is killed. We still wrap a
|
||||||
|
// GENEROUS WithTimeout at CriticAbsoluteMax so a broken/nil critic handle
|
||||||
|
// can't run unbounded; that ceiling never fires before the critic's backstop.
|
||||||
|
// A NESTED cause-carrying layer (cancelCause) lets a critic kill surface as a
|
||||||
|
// distinct "killed": only an ErrCriticKill cause is consulted in statusFor; a
|
||||||
|
// generic run error, a backstop expiry, or a caller cancel is classified by the
|
||||||
|
// run error itself.
|
||||||
|
criticOwnsDeadline := e.cfg.Ports.Critic != nil && ra.Critic.Enabled
|
||||||
|
hardCap := maxRuntime
|
||||||
|
if criticOwnsDeadline {
|
||||||
|
hardCap = e.cfg.Defaults.CriticAbsoluteMax
|
||||||
|
if hardCap < maxRuntime {
|
||||||
|
hardCap = maxRuntime // the failsafe ceiling is never shorter than the nominal budget
|
||||||
|
}
|
||||||
|
}
|
||||||
|
timeoutCtx, cancelTimeout := context.WithTimeout(context.WithoutCancel(ctx), hardCap)
|
||||||
defer cancelTimeout()
|
defer cancelTimeout()
|
||||||
runCtx, cancelCause := context.WithCancelCause(timeoutCtx)
|
runCtx, cancelCause := context.WithCancelCause(timeoutCtx)
|
||||||
defer cancelCause(nil)
|
defer cancelCause(nil)
|
||||||
@@ -287,9 +315,11 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
|
|||||||
checkpointCause = func() error { return context.Cause(runCtx) }
|
checkpointCause = func() error { return context.Cause(runCtx) }
|
||||||
|
|
||||||
// Critic (optional): monitors the run for a stall, can nudge/extend/kill via
|
// Critic (optional): monitors the run for a stall, can nudge/extend/kill via
|
||||||
// its host Escalator. Its hard deadline is bound to runCtx (cancel on pass).
|
// its host Escalator. When it owns the deadline, MaxRuntime is its soft trigger
|
||||||
// nil-safe: no-op when no critic is configured or the agent doesn't enable it.
|
// (so a slow-but-progressing run survives past it); its extendable backstop is
|
||||||
critic, stopCritic := e.startCritic(runCtx, cancelCause, ra, info)
|
// bound to runCtx (cancel on pass). nil-safe: no-op when no critic is configured
|
||||||
|
// or the agent doesn't enable it.
|
||||||
|
critic, stopCritic := e.startCritic(runCtx, cancelCause, ra, info, maxRuntime)
|
||||||
defer stopCritic()
|
defer stopCritic()
|
||||||
|
|
||||||
// Step instrumentation: accumulate Result.Steps + fire inv.OnStep, feed the
|
// Step instrumentation: accumulate Result.Steps + fire inv.OnStep, feed the
|
||||||
|
|||||||
Reference in New Issue
Block a user