feat(workflow): tail-only loop break, final-step outcome report, artifact handoff

CLAUDE.md

@@ -99,7 +99,8 @@ One `package main`, one file per concern.
 | `workflow_store.go`    | Three-scope workflow persistence: user (ask.json) + repo (`<root>/.ask/workflows/*.json`, committed) + global (`~/.config/ask/workflows/*.json`, machine-local, visible from every project); merged global-first listing (personal-wins), ambiguity-strict name resolution, dir sync on save, cross-scope copy. |
 | `workflows_screen.go`  | Workflows builder screen — list/steps/step editor levels with multi-line prompt textarea. `e` on a selected workflow opens that same textarea (workflow-scoped `promptTarget=="description"`) to edit the workflow's free-text Description; it commits to `workflowDef.Description` and shows as the steps-pane subtitle. |
 | `workflows_picker.go`  | Small centred modal popped on `f` (issues) / `Ctrl+F` (chat) to pick which workflow to run. |
-| `pkg/workflow/compile.go` | `CompileWorkflow` — a `Def` becomes an ADK graph: `AgentNode` per step, `loopagent`+`exitlooptool` per loop, `IncludeContentsNone` + `InstructionProvider` per step agent, per-node `RetryConfig`. Returns `Compiled` with the agent-name → step-index map the progress adapter needs. |
+| `pkg/workflow/compile.go` | `CompileWorkflow` — a `Def` becomes an ADK graph: `AgentNode` per step, `loopagent`+`exitlooptool` (tail step only) per loop, `IncludeContentsNone` + `InstructionProvider` per step agent, per-node `RetryConfig`. Hands each step a `StepRole{InLoop, IsTail, IsFinal}` so the `ToolsBuilder` attaches position-dependent tools. Returns `Compiled` with the agent-name → step-index map the progress adapter needs. |
+| `pkg/tools/artifact.go` | `SaveArtifactTool` (writes through `ctx.Artifacts().Save`; ADK ships only `load_artifacts`) and `WorkflowStepTools(env, isFinal)` — save/load on every step for data handoff, `finish_workflow` on the final step for the user-facing outcome report. |
 | `pkg/workflow/progress.go` | `Progress` — ADK events → `RunnerListener` callbacks, driven only by events that actually arrived. |
 | `cmd/ask/workflow_graph.go` | TUI runner: one session for the whole graph, agent swapped for the compiled workflow. |
 | `pkg/engine/workflow_run.go` | Headless runner + `WorkflowGraphAgent` + `IngestWorkflowMemory`. |
@@ -438,14 +439,22 @@ were deleted. Don't add a second one.
 - A top-level agent step becomes an `AgentNode` wrapping an `llmagent`;
   nodes are chained `Start -> n0 -> n1 -> …`.
 - A `kind: "loop"` step becomes an `AgentNode` wrapping ADK's
-  `loopagent`, whose sub-agents are the inner steps, each carrying
-  `exitlooptool`. A step breaks the loop by calling `exit_loop` (it sets
-  `Actions.Escalate`, which is what `loopagent` watches for); otherwise
-  the loop runs to `MaxIterations`. There is no `decision` argument any
-  more — loop control is ADK's tool, not an `end_turn` field.
+  `loopagent`, whose sub-agents are the inner steps. **Only the tail
+  (last) inner step carries `exitlooptool`**, so only it can break the
+  loop early: `exit_loop` sets `Actions.Escalate` (what `loopagent`
+  watches), no other ask tool touches `Actions`, so withholding the tool
+  from the non-tail steps makes an early break structurally impossible
+  for them — this replaces the old `end_turn` decision guard, which
+  caught the violation after the fact and re-prompted. Otherwise the loop
+  runs to `MaxIterations`. There is no `decision` argument — loop control
+  is ADK's tool, not an `end_turn` field.
 - `NodeConfig.RetryConfig` gives per-node retry, replacing the runner's
   hand-rolled `stepErrorRetry` loop.
 
+`CompileWorkflow` hands each step a `StepRole{InLoop, IsTail, IsFinal}`
+via its `ToolsBuilder`, which is what decides the position-dependent
+tools: `IsTail` gates `exit_loop`, `IsFinal` gates `finish_workflow`.
+
 Two `llmagent` settings carry the semantics and MUST NOT be dropped:
 
 - **`IncludeContents: IncludeContentsNone`** is what isolates a step.
@@ -500,8 +509,29 @@ Every step should call `end_turn` once with a `summary` (1-3 sentences);
 it becomes the step's line in the workflow log via `stepSummaryLine`.
 A step that ends without it is NOT re-prompted any more — the whole
 remind/re-prompt machinery is gone — its log line falls back to the first
-line of its own output. `finish_workflow` still reports the run's
-outcome; the runner reads it from `env.PendingFinishData`.
+line of its own output.
+
+`finish_workflow` reports the run's outcome and the artifacts it produced
+(PR links, tickets) to the user. It is attached to the **final step
+only** (`WorkflowStepTools(env, isFinal)` in pkg/tools/artifact.go);
+`Progress` captures the call straight from the event stream
+(progress.go), and the TUI also reads `env.PendingFinishData` as a
+backup. Nothing attaches it otherwise — an earlier regression left it
+attached to no step at all once the graph became one session.
+
+### Passing data between steps (artifacts)
+
+A node's text output is the implicit handoff to the next node. For
+structured data — a plan, a diff, notes — a step calls the native
+`save_artifact` tool; a later step loads it by name with ADK's
+`load_artifacts`. Both are attached to every workflow step by
+`WorkflowStepTools`. This works because the whole graph runs as ONE
+runner invocation, so the runner's `ArtifactService` (set in
+`RunnerBuilder`) spans every step — a save in step 1 is visible to a load
+in step 3. ADK ships `load_artifacts` but no save tool; `SaveArtifactTool`
+(pkg/tools/artifact.go) is the missing half, writing through
+`ctx.Artifacts().Save`. This is the ADK-native replacement for the old
+`ask/plans/` notes directories.
 
 ### Step instruction assembly
 
@@ -524,9 +554,10 @@ node's input, and `IncludeContentsNone` is what lets the step see it.
 
 `ask/plans/` is gone, along with `plans.go`, `clear_plans`, and the
 `RemindFixPlanDir` re-prompt. Step-to-step handoff is the graph's node
-output. Durable "what we learned" goes to `pkg/memory` — `RunWorkflow`
-calls `engine.IngestWorkflowMemory` on a clean finish, which is what the
-notes directories were badly approximating.
+output plus ADK artifacts (see above). Durable "what we learned" goes to
+`pkg/memory` — `RunWorkflow` calls `engine.IngestWorkflowMemory` on a
+clean finish, which is what the notes directories were badly
+approximating.
 
 
 ### Builder screen (`Ctrl+W` / `/workflows`)

cmd/ask/workflow_graph.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/Cidan/ask/pkg/engine"
 	"github.com/Cidan/ask/pkg/providers"
+	"github.com/Cidan/ask/pkg/tools"
 	"github.com/Cidan/ask/pkg/workflow"
 	adkmodel "google.golang.org/adk/v2/model"
 	adktool "google.golang.org/adk/v2/tool"
@@ -127,10 +128,12 @@ func tuiWorkflowCompileConfig(sess *agentSession, def workflow.Def, src workflow
 		ModelBuilder: func(ctx context.Context, step workflow.Step) (adkmodel.LLM, error) {
 			return workflowStepModel(ctx, sess, step)
 		},
-		ToolsBuilder: func(ctx context.Context, step workflow.Step, inLoop bool) ([]adktool.Tool, error) {
-			return engine.AsADKTools(sess.currentTools())
+		ToolsBuilder: func(ctx context.Context, step workflow.Step, role workflow.StepRole) ([]adktool.Tool, error) {
+			list := append([]tools.Tool(nil), sess.currentTools()...)
+			list = append(list, tools.WorkflowStepTools(sess.env, role.IsFinal)...)
+			return engine.AsADKTools(list)
 		},
-		ToolsetsBuilder: func(ctx context.Context, step workflow.Step, inLoop bool) ([]adktool.Toolset, error) {
+		ToolsetsBuilder: func(ctx context.Context, step workflow.Step, role workflow.StepRole) ([]adktool.Toolset, error) {
 			var toolsets []adktool.Toolset
 			if sess.mcp != nil {
 				toolsets = append(toolsets, sess.mcp.Toolsets()...)

pkg/engine/run.go

@@ -105,6 +105,13 @@ type ToolFactoryArgs struct {
 	EventListener         EventListener
 	InteractionHandler    InteractionHandler
 	AttachWebSearch       bool
+	// WorkflowStep attaches the workflow-step tools (save_artifact,
+	// load_artifacts) so a step can pass data to a later one.
+	WorkflowStep bool
+	// WorkflowFinalStep also attaches finish_workflow, which reports the
+	// run's outcome and created artifacts to the user. Only meaningful
+	// with WorkflowStep.
+	WorkflowFinalStep bool
 }
 
 // ToolFactory builds a slice of Tools for an engine turn.

pkg/engine/workflow_artifact_integration_test.go

@@ -0,0 +1,177 @@
+package engine_test
+
+import (
+	"context"
+	"iter"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/Cidan/ask/pkg/config"
+	"github.com/Cidan/ask/pkg/engine"
+	"github.com/Cidan/ask/pkg/providers"
+	// Imported for its init(): registers the tool factory, so workflow
+	// steps get the real core toolset plus save_artifact / load_artifacts.
+	_ "github.com/Cidan/ask/pkg/tools"
+	"github.com/Cidan/ask/pkg/workflow"
+	adkmodel "google.golang.org/adk/v2/model"
+	"google.golang.org/genai"
+)
+
+// scriptedModel drives one step at a time by inspecting the marker in the
+// step's system instruction and counting calls per marker. It is a full
+// model.LLM, not the internal test mock, because this test lives in the
+// external engine_test package to avoid the tools -> engine import cycle.
+type scriptedModel struct {
+	mu    sync.Mutex
+	calls map[string]int
+	// readerHandoff is what the reader saw on its FIRST call — the step
+	// handoff, before any artifact was loaded.
+	readerHandoff string
+	// readerContent captures every request text the reader step saw on
+	// its second call, so the test can assert the artifact crossed over.
+	readerContent []string
+}
+
+func (m *scriptedModel) Name() string { return "scripted" }
+
+func (m *scriptedModel) GenerateContent(_ context.Context, req *adkmodel.LLMRequest, _ bool) iter.Seq2[*adkmodel.LLMResponse, error] {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	sys := systemText(req)
+	switch {
+	case strings.Contains(sys, "SAVE_STEP_MARKER"):
+		n := m.calls["save"]
+		m.calls["save"]++
+		if n == 0 {
+			return one(functionCall("save_artifact", map[string]any{
+				"name":        "plan.md",
+				"content":     "SECRET_PLAN_BODY",
+				"description": "save the plan",
+			}))
+		}
+		return one(finalText("saved the plan"))
+
+	case strings.Contains(sys, "READ_STEP_MARKER"):
+		n := m.calls["read"]
+		m.calls["read"]++
+		if n == 0 {
+			m.readerHandoff = requestText(req)
+			return one(functionCall("load_artifacts", map[string]any{
+				"artifact_names": []any{"plan.md"},
+			}))
+		}
+		// Second call: load_artifacts' ProcessRequest has injected the
+		// artifact content into the request by now.
+		m.readerContent = append(m.readerContent, requestText(req))
+		return one(finalText("read the plan"))
+	}
+	return one(finalText("done"))
+}
+
+func systemText(req *adkmodel.LLMRequest) string {
+	if req == nil || req.Config == nil || req.Config.SystemInstruction == nil {
+		return ""
+	}
+	var b strings.Builder
+	for _, p := range req.Config.SystemInstruction.Parts {
+		if p != nil {
+			b.WriteString(p.Text)
+		}
+	}
+	return b.String()
+}
+
+func requestText(req *adkmodel.LLMRequest) string {
+	var b strings.Builder
+	for _, c := range req.Contents {
+		if c == nil {
+			continue
+		}
+		for _, p := range c.Parts {
+			if p != nil && p.Text != "" {
+				b.WriteString(p.Text)
+				b.WriteByte('\n')
+			}
+		}
+	}
+	return b.String()
+}
+
+func functionCall(name string, args map[string]any) *adkmodel.LLMResponse {
+	return &adkmodel.LLMResponse{
+		Content: &genai.Content{
+			Role:  genai.RoleModel,
+			Parts: []*genai.Part{genai.NewPartFromFunctionCall(name, args)},
+		},
+	}
+}
+
+func finalText(text string) *adkmodel.LLMResponse {
+	return &adkmodel.LLMResponse{
+		Content:      &genai.Content{Role: genai.RoleModel, Parts: []*genai.Part{genai.NewPartFromText(text)}},
+		FinishReason: genai.FinishReasonStop,
+	}
+}
+
+func one(resp *adkmodel.LLMResponse) iter.Seq2[*adkmodel.LLMResponse, error] {
+	return func(yield func(*adkmodel.LLMResponse, error) bool) { yield(resp, nil) }
+}
+
+// TestWorkflowArtifactHandoff is the full round trip: a two-step workflow
+// on the real ADK graph, where step 1 saves an artifact and step 2 loads
+// it. It proves the wiring end to end — the tool factory attaches
+// save_artifact/load_artifacts to every step, the graph runs as one
+// runner invocation so the ArtifactService spans both steps, and the
+// content saved by step 1 reaches step 2's model.
+func TestWorkflowArtifactHandoff(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	tmp := t.TempDir()
+
+	scripted := &scriptedModel{calls: map[string]int{}}
+	origBuilder := engine.ModelBuilder
+	engine.ModelBuilder = func(_ context.Context, _ *providers.AgentProviderSpec, _ config.Config, _ string) (adkmodel.LLM, error) {
+		return scripted, nil
+	}
+	defer func() { engine.ModelBuilder = origBuilder }()
+
+	eng := engine.New(engine.Options{
+		Config:             config.Config{Provider: "vertex"},
+		InteractionHandler: engine.HeadlessInteractionHandler{AutoApproveTools: true},
+	})
+
+	def := workflow.Def{
+		Name: "artifact-handoff",
+		Steps: []workflow.Step{
+			{Name: "saver", Prompt: "SAVE_STEP_MARKER: save the plan as an artifact."},
+			{Name: "reader", Prompt: "READ_STEP_MARKER: load the plan artifact and use it."},
+		},
+	}
+	src := workflow.NewTextSource(1, "Artifact Handoff Source")
+
+	if err := eng.RunWorkflow(context.Background(), tmp, 1, def, src); err != nil {
+		t.Fatalf("RunWorkflow: %v", err)
+	}
+
+	scripted.mu.Lock()
+	defer scripted.mu.Unlock()
+
+	if scripted.calls["save"] == 0 {
+		t.Fatal("the saver step never ran")
+	}
+	if len(scripted.readerContent) == 0 {
+		t.Fatal("the reader step never reached its second call, so load_artifacts never resolved")
+	}
+	// The secret must NOT arrive through the normal step handoff — the
+	// saver's node output is "saved the plan", and IncludeContentsNone
+	// keeps the saver's tool calls out of the reader's context. So its
+	// only path to the reader is load_artifacts.
+	if strings.Contains(scripted.readerHandoff, "SECRET_PLAN_BODY") {
+		t.Fatalf("the secret leaked through the step handoff, not the artifact:\n%s", scripted.readerHandoff)
+	}
+	joined := strings.Join(scripted.readerContent, "\n")
+	if !strings.Contains(joined, "SECRET_PLAN_BODY") {
+		t.Errorf("the artifact saved by step 1 did not reach step 2's model via load_artifacts:\n%s", joined)
+	}
+}

pkg/engine/workflow_run.go

@@ -27,7 +27,7 @@ func WorkflowCompileConfig(e *Engine, cwd string, tabID int, def workflow.Def, s
 		ModelBuilder: func(ctx context.Context, step workflow.Step) (adkmodel.LLM, error) {
 			return buildStepModel(ctx, e, step)
 		},
-		ToolsBuilder: func(ctx context.Context, step workflow.Step, inLoop bool) ([]tool.Tool, error) {
+		ToolsBuilder: func(ctx context.Context, step workflow.Step, role workflow.StepRole) ([]tool.Tool, error) {
 			var agentTools []Tool
 			if tf := GetDefaultToolFactory(); tf != nil {
 				agentTools = tf(ToolFactoryArgs{
@@ -37,11 +37,13 @@ func WorkflowCompileConfig(e *Engine, cwd string, tabID int, def workflow.Def, s
 					EventListener:      e.opts.EventListener,
 					InteractionHandler: e.opts.InteractionHandler,
 					AttachWebSearch:    true,
+					WorkflowStep:       true,
+					WorkflowFinalStep:  role.IsFinal,
 				})
 			}
 			return AsADKTools(agentTools)
 		},
-		ToolsetsBuilder: func(ctx context.Context, step workflow.Step, inLoop bool) ([]tool.Toolset, error) {
+		ToolsetsBuilder: func(ctx context.Context, step workflow.Step, role workflow.StepRole) ([]tool.Toolset, error) {
 			var toolsets []tool.Toolset
 			if skillTS, err := NewSkillToolset(ctx, cwd); err == nil && skillTS != nil {
 				toolsets = append(toolsets, skillTS)

pkg/tools/artifact.go

@@ -0,0 +1,68 @@
+package tools
+
+import (
+	"errors"
+	"strings"
+
+	"google.golang.org/adk/v2/agent"
+	"google.golang.org/adk/v2/tool/loadartifactstool"
+	"google.golang.org/genai"
+)
+
+// SaveArtifactParams is the save_artifact tool's input.
+type SaveArtifactParams struct {
+	Name        string `json:"name" jsonschema:"artifact name, e.g. plan.md — later steps load it by this name"`
+	Content     string `json:"content" jsonschema:"the artifact's text content"`
+	Description string `json:"description" jsonschema:"one short human-readable phrase (under 10 words) telling the user what this call is doing"`
+}
+
+// SaveArtifactResult is the save_artifact tool's response.
+type SaveArtifactResult struct {
+	Name    string `json:"name,omitempty"`
+	Version int64  `json:"version,omitempty" jsonschema:"the saved version number"`
+}
+
+// SaveArtifactTool lets a workflow step hand structured data to a later
+// step. The artifact lives in the run's ArtifactService, which spans the
+// whole graph because the graph runs as one runner invocation, so a step
+// saves plan.md and a downstream step reads it back with load_artifacts.
+//
+// ADK ships load_artifacts but no save tool; this is the missing half.
+func SaveArtifactTool() Tool {
+	return NewTypedTool(
+		"save_artifact",
+		"Save a named artifact (a plan, a diff, notes) so a later workflow step can load it by name. Use this to hand structured output forward rather than restating it.",
+		func(ctx agent.Context, p SaveArtifactParams) (SaveArtifactResult, error) {
+			name := strings.TrimSpace(p.Name)
+			if name == "" {
+				return SaveArtifactResult{}, errors.New("name is required")
+			}
+			arts := ctx.Artifacts()
+			if arts == nil {
+				return SaveArtifactResult{}, errors.New("artifacts are not available in this context")
+			}
+			resp, err := arts.Save(ctx, name, genai.NewPartFromText(p.Content))
+			if err != nil {
+				return SaveArtifactResult{}, err
+			}
+			var version int64
+			if resp != nil {
+				version = resp.Version
+			}
+			return SaveArtifactResult{Name: name, Version: version}, nil
+		},
+	)
+}
+
+// WorkflowStepTools returns the position-dependent tools every workflow
+// step needs: save_artifact and load_artifacts on every step so steps can
+// pass data forward, plus finish_workflow on the final step only, which
+// reports the run's outcome and created artifacts (PR links, tickets) to
+// the user.
+func WorkflowStepTools(env *ToolEnv, isFinal bool) []Tool {
+	out := []Tool{SaveArtifactTool(), loadartifactstool.New()}
+	if isFinal {
+		out = append(out, FinishWorkflowTool(env))
+	}
+	return out
+}