run: PostRun detached ctx + panic-isolated Cleanup (gadfly #12)

Two convergent gadfly refinements on the PostRun wiring:
- PostRun now runs on detach(ctx), not the caller's ctx — a finished/cancelled
  caller no longer aborts artifact production (3-model: glm-5.2/minimax/deepseek).
- Cleanup is panic-isolated via safeCleanup (recover+log), matching runPostRun, so
  a misbehaving teardown can't clobber an otherwise-successful run (deepseek).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

.gitea/workflows/adversarial-review.yml

@@ -51,13 +51,11 @@ jobs:
           GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
           OLLAMA_CLOUD_API_KEY: ${{ secrets.OLLAMA_CLOUD_API_KEY }}
           # Local Macs, reached through their foreman queues (native Ollama on the
-          # wire). GADFLY_ENDPOINT_M5 registers provider "m5",
-          # each a foreman-preset Ollama client at the secret's URL, of the form:
+                    # each a foreman-preset Ollama client at the secret's URL, of the form:
           #   foreman|https://<foreman-host>|<token>
           # Needs an image with foreman provider-type support (this one). If a Mac
           # is offline that model's comment shows an error and the others still post.
           # (Gitea secrets aren't auto-exposed — map each explicitly.)
-          GADFLY_ENDPOINT_M5: ${{ secrets.GADFLY_ENDPOINT_M5 }}
           # Full fleet: 3 cloud + M1 Pro + M5 Max. The Macs are back so the
           # gadfly-reports scoreboard can quantify whether they earn their keep
           # (they previously took 26–29 min for ZERO real findings — now measured).
@@ -65,8 +63,8 @@ jobs:
           # (ollama-cloud=1) with its 3 lenses concurrent (LENS ollama-cloud=3) so
           # its comment lands sooner; each Mac runs one model, lenses serial (its
           # foreman queue serializes anyway). All three provider lanes run parallel.
-          GADFLY_MODELS: "minimax-m3:cloud,glm-5.2:cloud,glm-5.1:cloud,kimi-k2.7-code:cloud,deepseek-v4-pro:cloud,nemotron-3-super:cloud,gpt-oss:120b-cloud,qwen3-coder:480b-cloud,gemma4:cloud,m5/qwen3.6:35b-mlx"
-          GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3,m5=1"
+          GADFLY_MODELS: "minimax-m3:cloud,glm-5.2:cloud,glm-5.1:cloud,deepseek-v4-pro:cloud,nemotron-3-super:cloud,qwen3-coder:480b-cloud"
+          GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3"
           GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3"
           # Default => the 3-lens suite (security, correctness, error-handling).
           # Set the repo var GADFLY_SPECIALISTS to override (csv / "all" / "auto").

critic/critic.go

@@ -18,6 +18,7 @@ package critic
 
 import (
 	"context"
+	"errors"
 	"log/slog"
 	"math"
 	"sync"
@@ -162,12 +163,15 @@ type handle struct {
 	iterations   int
 	maxSteps     int // current tool-dispatch ceiling (base MaxIterations, raised by RaiseStepsBy)
 	lastTool     string
-	killed       bool // sticky: once an Escalator kills, no later decision un-kills it
+	killed       bool  // sticky: once an Escalator kills, no later decision un-kills it
+	killCause    error // non-nil once killed; surfaced via KillCause for "killed" status
 	stopped      bool
 	stopCh       chan struct{}
 }
 
-func (h *handle) RecordStep(iter int) {
+func (h *handle) RecordStep(iter int, _ *llm.Response) {
+	// This battery's Progress tracks iteration count + activity, not per-step
+	// payload, so the response is unused here; a richer Escalator could record it.
 	h.mu.Lock()
 	h.iterations = iter
 	h.lastActivity = h.now()
@@ -204,6 +208,12 @@ func (h *handle) MaxSteps() int {
 	return h.maxSteps
 }
 
+func (h *handle) KillCause() error {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	return h.killCause
+}
+
 func (h *handle) Stop() {
 	h.mu.Lock()
 	if !h.stopped {
@@ -266,8 +276,13 @@ func (h *handle) tick(ctx context.Context) {
 	}
 	if d.Kill {
 		h.killed = true
-		h.deadline = h.now() // immediate hard deadline → executor cancels
-		return               // ignore any Nudge/ExtendBy paired with a Kill
+		reason := d.KillReason
+		if reason == "" {
+			reason = "critic killed the run"
+		}
+		h.killCause = errors.New(reason) // surfaced via KillCause → "killed" status
+		h.deadline = h.now()             // immediate hard deadline → executor cancels
+		return                           // ignore any Nudge/ExtendBy paired with a Kill
 	}
 	if len(d.Nudge) > 0 {
 		h.steer = append(h.steer, d.Nudge...)

critic/critic_test.go

@@ -51,7 +51,7 @@ func TestMonitorEscalatesOncePerIdlePeriodAndExtends(t *testing.T) {
 		t.Error("deadline should have been extended past the original")
 	}
 	// A fresh step re-arms; another idle period escalates again.
-	h.RecordStep(1)
+	h.RecordStep(1, nil)
 	time.Sleep(60 * time.Millisecond)
 	mu.Lock()
 	c2 := calls

run/critic.go

@@ -2,9 +2,11 @@ package run
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo/agent"
+	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
 )
 
 // criticDeadlineCheck is how often the deadline-watch goroutine polls the
@@ -21,12 +23,15 @@ type criticBinding struct {
 }
 
 // startCritic begins critic monitoring for this run when one is configured and
-// the agent enables it. It launches a goroutine that cancels runCtx (via cancel)
-// the moment the critic's hard deadline passes — the critic may extend that
-// deadline, so a healthy-but-slow run is given room while a hung one is killed.
-// Returns (nil, no-op stop) when there is no critic. The caller MUST defer the
-// returned stop.
-func (e *Executor) startCritic(runCtx context.Context, cancel context.CancelFunc, ra RunnableAgent, info RunInfo) (*criticBinding, func()) {
+// the agent enables it. It launches a goroutine that cancels runCtx (via
+// cancelCause) the moment the critic's hard deadline passes — the critic may
+// extend that deadline, so a healthy-but-slow run is given room while a hung one
+// is killed. When the deadline passes because the critic KILLED the run
+// (KillCause() != nil), the cancellation cause is ErrCriticKill (→ status
+// "killed"); when the backstop simply expired, it is context.DeadlineExceeded (→
+// "timeout"). Returns (nil, no-op stop) when there is no critic. The caller MUST
+// defer the returned stop.
+func (e *Executor) startCritic(runCtx context.Context, cancelCause context.CancelCauseFunc, ra RunnableAgent, info RunInfo) (*criticBinding, func()) {
 	noop := func() {}
 	if e.cfg.Ports.Critic == nil || !ra.Critic.Enabled {
 		return nil, noop
@@ -55,9 +60,14 @@ func (e *Executor) startCritic(runCtx context.Context, cancel context.CancelFunc
 				return
 			case <-t.C:
 				// A zero deadline = no hard cap (not yet set); otherwise cancel
-				// once we're at or past it.
+				// once we're at or past it, distinguishing an explicit kill from a
+				// natural backstop expiry so the run gets the right status.
 				if d := h.Deadline(); !d.IsZero() && !time.Now().Before(d) {
-					cancel()
+					if cause := h.KillCause(); cause != nil {
+						cancelCause(fmt.Errorf("%w: %s", ErrCriticKill, cause.Error()))
+					} else {
+						cancelCause(context.DeadlineExceeded)
+					}
 					return
 				}
 			}
@@ -69,9 +79,9 @@ func (e *Executor) startCritic(runCtx context.Context, cancel context.CancelFunc
 	}
 }
 
-func (b *criticBinding) recordStep(iter int) {
+func (b *criticBinding) recordStep(iter int, resp *llm.Response) {
 	if b != nil {
-		b.h.RecordStep(iter)
+		b.h.RecordStep(iter, resp)
 	}
 }
 
@@ -104,11 +114,11 @@ func (b *criticBinding) maxStepsOption(base int) agent.Option {
 	})
 }
 
-// steerOptions returns the agent RunOptions that drain the critic's steer
-// messages into the loop. Empty when there is no critic.
-func (b *criticBinding) steerOptions() []agent.RunOption {
+// drainSteer returns the critic's queued steer messages (nil-safe), so the
+// executor can merge them with the session steer mailbox into one WithSteer.
+func (b *criticBinding) drainSteer() []llm.Message {
 	if b == nil {
 		return nil
 	}
-	return []agent.RunOption{agent.WithSteer(b.h.Steer)}
+	return b.h.Steer()
 }

run/critic_test.go

@@ -23,10 +23,16 @@ type fakeCriticHandle struct {
 	mu                  sync.Mutex
 	steps, tools, stops int
 	steered             int
-	maxSteps            int // 0 => defer to the run's base MaxIterations
+	maxSteps            int   // 0 => defer to the run's base MaxIterations
+	killCause           error // non-nil simulates a critic kill
 }
 
-func (h *fakeCriticHandle) RecordStep(int) { h.mu.Lock(); h.steps++; h.mu.Unlock() }
+func (h *fakeCriticHandle) RecordStep(int, *llm.Response) { h.mu.Lock(); h.steps++; h.mu.Unlock() }
+func (h *fakeCriticHandle) KillCause() error {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	return h.killCause
+}
 func (h *fakeCriticHandle) RecordToolStart(string, string) {
 	h.mu.Lock()
 	h.tools++

run/executor.go

@@ -101,6 +101,10 @@ type Result struct {
 	Steps  []tool.Step
 	Usage  llm.Usage
 	Err    error
+	// PostRunResult carries artifacts produced by a SessionToolFactory's PostRun
+	// hook (rendered images, files). nil when no factory was set or PostRun
+	// returned nil. The host delivers these (e.g. mort's chat API / Discord).
+	PostRunResult *tool.PostRunResult
 }
 
 // Run executes ra with the given invocation + input and returns the Result. It
@@ -176,6 +180,12 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 		inv.RunState = stateAcc
 	}
 
+	// Steer mailbox: lets session tools (via inv.AttachImages) feed multimodal
+	// messages into the running conversation before its next step. Created BEFORE
+	// the toolbox build so any tool's handler captures the live AttachImages seam.
+	mailbox := &steerMailbox{}
+	inv.AttachImages = (&runSession{mailbox: mailbox}).AttachImages
+
 	// Build the toolbox from the agent's low-level tools.
 	toolbox, err := e.cfg.Registry.Build(ra.LowLevelTools, inv, tool.Visibility("private"), nil)
 	if err != nil {
@@ -192,20 +202,56 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 		return res
 	}
 
+	// Per-invocation ExtraTools + a SessionToolFactory's per-run tools, added on
+	// top of the agent's palette. The factory closes over the live session (the
+	// AttachImages mailbox); its PostRun hook (held for after the run) produces
+	// artifacts attached to res.PostRunResult, and its Cleanup is deferred. All
+	// nil-safe.
+	for _, t := range inv.ExtraTools {
+		if err := toolbox.Add(t); err != nil {
+			res.Err = fmt.Errorf("add extra tool: %w", err)
+			e.finishAudit(ctx, rec, "error", res, started, res.Err)
+			return res
+		}
+	}
+	var postRun func(ctx context.Context, transcript []llm.Message, output string, runErr error) *tool.PostRunResult
+	if inv.SessionToolFactory != nil {
+		st := inv.SessionToolFactory(&runSession{mailbox: mailbox})
+		if st.Cleanup != nil {
+			defer safeCleanup(st.Cleanup) // panic-isolated, like runPostRun
+		}
+		for _, t := range st.Tools {
+			if err := toolbox.Add(t); err != nil {
+				res.Err = fmt.Errorf("add session tool: %w", err)
+				e.finishAudit(ctx, rec, "error", res, started, res.Err)
+				return res
+			}
+		}
+		postRun = st.PostRun
+	}
+
 	// Run context: bound by MaxRuntime, detached from the caller's deadline so a
 	// lane/queue wait doesn't eat the run budget (mort's V10 lesson). Caller
 	// cancellation still propagates via MergeCancellation. Created BEFORE the
 	// step observer so the observer forwards the merged run context (not a
 	// possibly-cancelled caller ctx) to OnStep consumers.
-	runCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), maxRuntime)
-	defer cancel()
+	// MaxRuntime stays a WithTimeout so its DeadlineExceeded propagates through the
+	// child chain (→ "timeout"), preserving the run's-own-timeout vs caller-cancel
+	// distinction. A NESTED cause-carrying layer lets a critic kill surface as a
+	// distinct "killed" without disturbing that: only an ErrCriticKill cause is
+	// consulted in statusFor; a generic run error or a caller cancel is classified
+	// by the run error itself.
+	timeoutCtx, cancelTimeout := context.WithTimeout(context.WithoutCancel(ctx), maxRuntime)
+	defer cancelTimeout()
+	runCtx, cancelCause := context.WithCancelCause(timeoutCtx)
+	defer cancelCause(nil)
 	runCtx, mergeCancel := MergeCancellation(runCtx, ctx)
 	defer mergeCancel()
 
 	// 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).
 	// nil-safe: no-op when no critic is configured or the agent doesn't enable it.
-	critic, stopCritic := e.startCritic(runCtx, cancel, ra, info)
+	critic, stopCritic := e.startCritic(runCtx, cancelCause, ra, info)
 	defer stopCritic()
 
 	// Step instrumentation: accumulate Result.Steps + fire inv.OnStep, feed the
@@ -222,7 +268,7 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 		if rec != nil {
 			rec.OnStep(s.Index, s.Response)
 		}
-		critic.recordStep(s.Index) // keep the critic's activity clock fresh
+		critic.recordStep(s.Index, s.Response) // keep the critic's activity clock fresh + carry the step payload
 		var calls []llm.ToolCall
 		if s.Response != nil {
 			calls = s.Response.ToolCalls
@@ -271,9 +317,12 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 	}
 
 	ag := agent.New(model, e.systemPrompt(ra), opts...)
-	runRes, runErr := ag.Run(runCtx, input, critic.steerOptions()...)
+	// One WithSteer drains BOTH the session mailbox (a tool's AttachImages) and
+	// the critic's nudges before each step.
+	steer := func() []llm.Message { return append(mailbox.drain(), critic.drainSteer()...) }
+	runRes, runErr := ag.Run(runCtx, input, agent.WithSteer(steer))
 
-	status := statusFor(runErr)
+	status := statusFor(runCtx, runErr)
 	if runRes != nil {
 		res.Output = runRes.Output
 		res.Usage = runRes.Usage
@@ -281,6 +330,19 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 	res.Steps = emitter.snapshot()
 	res.Err = runErr
 
+	// PostRun: hand the SessionToolFactory's hook the full transcript (populated
+	// even on partial results) so it can produce artifacts. Best-effort +
+	// panic-isolated — a PostRun failure never fails an otherwise-successful run.
+	if postRun != nil {
+		var transcript []llm.Message
+		if runRes != nil {
+			transcript = runRes.Messages
+		}
+		// Detach from the caller's ctx: a finished/cancelled caller must not abort
+		// artifact production (the hook owns its own bounding, per its contract).
+		res.PostRunResult = runPostRun(detach(ctx), postRun, transcript, res.Output, runErr)
+	}
+
 	e.finishAudit(ctx, rec, status, res, started, runErr)
 	if e.cfg.Ports.Budget != nil {
 		e.cfg.Ports.Budget.Commit(detach(ctx), inv.CallerID, time.Since(started).Seconds())
@@ -289,13 +351,22 @@ func (e *Executor) Run(ctx context.Context, ra RunnableAgent, inv tool.Invocatio
 	return res
 }
 
-// statusFor maps a run error to a RunStats.Status, distinguishing a deadline
-// (timeout) and a cancellation (cancelled — caller cancel or shutdown) from a
-// generic error so audit consumers can tell them apart.
-func statusFor(runErr error) string {
+// statusFor maps a run error to a RunStats.Status, distinguishing a critic kill
+// (killed), a deadline (timeout), and a cancellation (cancelled — caller cancel
+// or shutdown) from a generic error so audit consumers can tell them apart. The
+// run context's cancellation cause carries the distinction (ErrCriticKill /
+// DeadlineExceeded), since ctx.Err() alone only reports Canceled.
+func statusFor(runCtx context.Context, runErr error) string {
 	switch {
 	case runErr == nil:
 		return "ok"
+	// Only the kill is recovered from the cancellation cause — a critic kill
+	// surfaces as a plain Canceled run error, so without this it'd read as
+	// "cancelled". Everything else is classified by the run error itself, so a
+	// genuine run error is never relabeled just because the context was later
+	// cancelled, and a caller cancel/deadline stays "cancelled" (not "timeout").
+	case errors.Is(context.Cause(runCtx), ErrCriticKill):
+		return "killed"
 	case errors.Is(runErr, context.DeadlineExceeded):
 		return "timeout"
 	case errors.Is(runErr, context.Canceled):

run/executor_test.go

@@ -148,20 +148,33 @@ func TestExecutorNilModelNoPanic(t *testing.T) {
 	}
 }
 
-// TestStatusFor maps run errors to RunStats.Status (gadfly F3).
+// TestStatusFor maps run errors + cancellation cause to RunStats.Status (gadfly F3).
 func TestStatusFor(t *testing.T) {
+	bg := context.Background()
+	// A context cancelled with the critic-kill cause: ctx.Err() is Canceled, but
+	// context.Cause carries ErrCriticKill → "killed".
+	killCtx, killCancel := context.WithCancelCause(context.Background())
+	killCancel(fmt.Errorf("%w: hung", ErrCriticKill))
+	// A context cancelled with a non-kill cause must NOT relabel a genuine run
+	// error: a real error stays "error" even though the ctx was later cancelled.
+	cancelledCtx, cc := context.WithCancelCause(context.Background())
+	cc(context.DeadlineExceeded)
 	cases := []struct {
+		ctx  context.Context
 		err  error
 		want string
 	}{
-		{nil, "ok"},
-		{context.DeadlineExceeded, "timeout"},
-		{context.Canceled, "cancelled"},
-		{fmt.Errorf("wrapped: %w", context.DeadlineExceeded), "timeout"},
-		{errors.New("boom"), "error"},
+		{bg, nil, "ok"},
+		{bg, context.DeadlineExceeded, "timeout"},
+		{bg, context.Canceled, "cancelled"},
+		{bg, fmt.Errorf("wrapped: %w", context.DeadlineExceeded), "timeout"},
+		{bg, errors.New("boom"), "error"},
+		{killCtx, context.Canceled, "killed"},
+		{cancelledCtx, errors.New("boom"), "error"},   // generic error not relabeled by cause
+		{cancelledCtx, context.Canceled, "cancelled"}, // caller cancel stays cancelled, not timeout
 	}
 	for _, c := range cases {
-		if got := statusFor(c.err); got != c.want {
+		if got := statusFor(c.ctx, c.err); got != c.want {
 			t.Errorf("statusFor(%v) = %q, want %q", c.err, got, c.want)
 		}
 	}

run/ports.go

@@ -2,6 +2,7 @@ package run
 
 import (
 	"context"
+	"errors"
 	"time"
 
 	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
@@ -9,6 +10,12 @@ import (
 	"gitea.stevedudenhoeffer.com/steve/executus/deliver"
 )
 
+// ErrCriticKill is the cancellation cause the executor stamps on a run the
+// critic kills, so a critic kill surfaces as a distinct "killed" status (vs a
+// backstop "timeout" or a caller "cancelled"). A host CriticHandle signals a
+// kill via KillCause(); the executor wraps that reason with this sentinel.
+var ErrCriticKill = errors.New("run: critic killed the run")
+
 // 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
@@ -123,8 +130,10 @@ type Critic interface {
 // methods (the critic battery's handle guards its state with a mutex).
 type CriticHandle interface {
 	// RecordStep / RecordToolStart keep the critic's activity clock fresh so a
-	// healthy-but-slow run is not mistaken for a hang.
-	RecordStep(iter int)
+	// healthy-but-slow run is not mistaken for a hang. RecordStep also carries the
+	// completed step's model response (nil-safe) so the critic's Trace can show
+	// what the agent actually produced, not just an iteration count.
+	RecordStep(iter int, resp *llm.Response)
 	RecordToolStart(name, args string)
 	// Steer returns any messages the critic wants injected into the loop (a
 	// nudge), drained before each step — matches majordomo agent.WithSteer.
@@ -137,6 +146,12 @@ type CriticHandle interface {
 	// healthy-but-long run's iteration budget mid-flight. Return <= 0 to defer to
 	// the run's base MaxIterations.
 	MaxSteps() int
+	// KillCause returns a non-nil reason iff the critic has decided to KILL this
+	// run (as opposed to letting the hard-deadline backstop expire). The executor
+	// reads it when the deadline passes: non-nil → cancel the run with
+	// ErrCriticKill (status "killed"); nil → the backstop expired naturally
+	// (status "timeout"). Hosts that never distinguish the two may return nil.
+	KillCause() error
 	// Stop ends monitoring when the run finishes.
 	Stop()
 }

run/session.go

@@ -0,0 +1,88 @@
+package run
+
+import (
+	"context"
+	"log/slog"
+	"strings"
+	"sync"
+
+	"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
+
+	"gitea.stevedudenhoeffer.com/steve/executus/tool"
+)
+
+// runPostRun invokes a SessionToolFactory's PostRun hook with panic isolation:
+// a PostRun panic (or a slow artifact build that the hook mishandles) must not
+// fail an otherwise-successful run — artifacts are best-effort, the agent's text
+// output is the source of truth.
+func runPostRun(ctx context.Context,
+	hook func(context.Context, []llm.Message, string, error) *tool.PostRunResult,
+	transcript []llm.Message, output string, runErr error) (prr *tool.PostRunResult) {
+	defer func() {
+		if r := recover(); r != nil {
+			slog.Error("run: PostRun hook panicked; no artifacts produced", "panic", r)
+			prr = nil
+		}
+	}()
+	return hook(ctx, transcript, output, runErr)
+}
+
+// steerMailbox is a thread-safe queue of messages a session tool (via
+// tool.Invocation.AttachImages) wants injected into the agent loop before its
+// next step — the same WithSteer mechanism the critic uses for nudges, exposed
+// to ordinary tools so they can show the model content (e.g. a rendered
+// preview) it must SEE, not just be told about.
+type steerMailbox struct {
+	mu   sync.Mutex
+	msgs []llm.Message
+}
+
+func (m *steerMailbox) push(msg llm.Message) {
+	m.mu.Lock()
+	m.msgs = append(m.msgs, msg)
+	m.mu.Unlock()
+}
+
+// drain returns and clears the queued messages (nil when empty).
+func (m *steerMailbox) drain() []llm.Message {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if len(m.msgs) == 0 {
+		return nil
+	}
+	out := m.msgs
+	m.msgs = nil
+	return out
+}
+
+// runSession implements tool.AgentSession over a steer mailbox: AttachImages
+// queues a user-role multimodal message the agent loop injects before its next
+// step. Replaces legacy agentkit's Agent.AttachImages — majordomo's *agent.Agent
+// is immutable mid-run, so mutation flows through the run-scoped steer mailbox.
+type runSession struct{ mailbox *steerMailbox }
+
+func (s *runSession) AttachImages(text string, images ...llm.ImagePart) {
+	parts := make([]llm.Part, 0, len(images)+1)
+	if strings.TrimSpace(text) != "" {
+		parts = append(parts, llm.Text(text))
+	}
+	for _, img := range images {
+		parts = append(parts, img)
+	}
+	if len(parts) == 0 {
+		return
+	}
+	s.mailbox.push(llm.UserParts(parts...))
+}
+
+// safeCleanup runs a SessionTools.Cleanup with panic isolation, so a misbehaving
+// teardown (temp-dir removal, handle close) can't clobber an otherwise-successful
+// run via the executor's top-level recover.
+func safeCleanup(fn func()) {
+	defer func() {
+		if r := recover(); r != nil {
+			slog.Error("run: session Cleanup panicked", "panic", r)
+		}
+	}()
+	fn()
+}

run/session_test.go

@@ -0,0 +1,94 @@
+package run_test
+
+import (
+	"context"
+	"testing"
+
+	"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"
+)
+
+// TestSessionToolFactoryPostRun: a SessionToolFactory's PostRun hook produces an
+// artifact (from the run output + transcript) that lands on Result.PostRunResult,
+// and its Cleanup is deferred.
+func TestSessionToolFactoryPostRun(t *testing.T) {
+	fp := fake.New("fake")
+	fp.Enqueue("m", fake.Reply("hello artifacts"))
+	m, _ := fp.Model("m")
+
+	cleanupCalled := false
+	inv := tool.Invocation{
+		RunID: "r1",
+		SessionToolFactory: func(_ tool.AgentSession) tool.SessionTools {
+			return tool.SessionTools{
+				PostRun: func(_ context.Context, transcript []llm.Message, output string, _ error) *tool.PostRunResult {
+					return &tool.PostRunResult{
+						Artifacts: []tool.Artifact{{Name: "out.txt", MimeType: "text/plain", Data: []byte(output)}},
+						Metadata:  map[string]any{"transcript_len": len(transcript)},
+					}
+				},
+				Cleanup: func() { cleanupCalled = true },
+			}
+		},
+	}
+	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{ModelTier: "m"}, inv, "go")
+	if res.Err != nil {
+		t.Fatalf("run error: %v", res.Err)
+	}
+	if res.PostRunResult == nil {
+		t.Fatal("Result.PostRunResult is nil — PostRun hook not invoked / not attached")
+	}
+	if n := len(res.PostRunResult.Artifacts); n != 1 {
+		t.Fatalf("artifacts = %d, want 1", n)
+	}
+	a := res.PostRunResult.Artifacts[0]
+	if a.Name != "out.txt" || string(a.Data) != "hello artifacts" {
+		t.Errorf("artifact = {%q, %q}", a.Name, string(a.Data))
+	}
+	if tl, _ := res.PostRunResult.Metadata["transcript_len"].(int); tl < 1 {
+		t.Errorf("transcript not passed to PostRun (len=%d)", tl)
+	}
+	if !cleanupCalled {
+		t.Error("Cleanup was not deferred/called")
+	}
+}
+
+// TestSessionToolFactoryAddsTool: tools the factory returns join the run's
+// toolbox and are callable by the model.
+func TestSessionToolFactoryAddsTool(t *testing.T) {
+	fp := fake.New("fake")
+	fp.Enqueue("m",
+		fake.ReplyWith(llm.Response{ToolCalls: []llm.ToolCall{{ID: "c1", Name: "render", Arguments: []byte(`{}`)}}}),
+		fake.Reply("rendered"),
+	)
+	m, _ := fp.Model("m")
+
+	toolCalled := false
+	renderTool := llm.DefineTool("render", "render a preview",
+		func(_ context.Context, _ struct{}) (any, error) { toolCalled = true; return "ok", nil })
+	inv := tool.Invocation{
+		RunID: "r2",
+		SessionToolFactory: func(_ tool.AgentSession) tool.SessionTools {
+			return tool.SessionTools{Tools: []llm.Tool{renderTool}}
+		},
+	}
+	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{ModelTier: "m", MaxIterations: 5}, inv, "go")
+	if res.Err != nil {
+		t.Fatalf("run error: %v", res.Err)
+	}
+	if !toolCalled {
+		t.Error("session-factory tool was not added to the toolbox / not called")
+	}
+}