feat(workflow): run workflows on ADK's graph engine - #136
Merged
Conversation
Replaces the handwritten workflow state machine with ADK's workflow scheduler, and deletes the two dead parallel implementations that PRs #125 and #130 left behind. Net -2600 lines. A workflow.Def now compiles to a workflow.Workflow (pkg/workflow/compile.go): each top-level agent step is an AgentNode wrapping an llmagent, chained Start -> n0 -> n1; a kind:"loop" step is an AgentNode wrapping ADK's loopagent whose sub-agents carry exitlooptool. Loop control is exit_loop (it sets Actions.Escalate, which loopagent watches) rather than an end_turn `decision` argument, and NodeConfig.RetryConfig replaces the runner's hand-rolled stepErrorRetry loop. Two llmagent settings carry the semantics: - IncludeContentsNone isolates each step. Without it a step inherits the whole session and ADK's ConvertForeignEvent renders every prior step's events as prose — each tool call and each full tool result — so step 3 would carry steps 1 and 2 in full. - InstructionProvider, never Config.Instruction. Step prompts are user-authored and routinely contain braces; ADK interpolates the static field and fails the invocation on the first unknown {name}. *workflow.Workflow is not an agent.Agent (unexported method), so engine.WorkflowGraphAgent wraps it with agent.New(Config{Run: wf.Run}). A run is now ONE agent session for the whole graph rather than one provider session per step: the TUI swaps the session's agent for the compiled graph (cmd/ask/workflow_graph.go), so tool execution, approvals, cost accounting, and cancellation behave exactly as in a chat turn. StepExecutor/ExecuteStep are gone from both coordinators. Progress (pkg/workflow/progress.go) derives step lifecycle from the real event stream. The deleted RunGraph closed out every step that never ran as both started AND done and hardcoded a successful FinishData, so a chain dying at step 1 of 5 rendered 5/5 green; nothing is reported now that did not happen. Notes directories are gone. ask/plans/, plans.go, the clear_plans tool, and the RemindFixPlanDir re-prompt were a hack standing in for step-to-step data passing. Handoff is the graph's node output; durable reasoning goes to pkg/memory, which was already an adkmemory.Service wired as the runner's MemoryService but never fed from a workflow — IngestWorkflowMemory closes that. Also removes load_artifacts from the core wire toolset. ADK ships only InMemoryService and gcsartifact, ask's artifact service was rebuilt per turn, and nothing ever saved to it, so the tool could only ever return empty while costing tokens on every request. Deleted: Runner.Run, RunGraph, both BuildWorkflowAgents, LoopRunFrame, the remind/re-prompt machinery, graph.go, plans.go, and the TUI's vestigial loop cursor. docs/adk-20-upgrade.md is rewritten as an honest status table with a Gaps section, so retryandreflect (registered but unreachable), functioncallmodifier (predicate always false), agenttool (no callers), and toolconfirmation (no tool declares it) are recorded as gaps rather than checked off. Tests: graph compilation (linear, loops, agent-name sanitisation and uniqueness, error paths), progress reporting (ordering, summaries, finish data, loop re-entry, unknown authors, and the fabricated-step regression), and end-to-end engine runs asserting step ordering, that a failed run reports no completed steps, that step 3 receives step 2's handoff but NOT step 1's transcript, and that braces in a step prompt do not fail the run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cidan
added a commit
that referenced
this pull request
Aug 21, 2026
…fact handoff (#141) * feat(workflow): tail-only loop break, final-step outcome report, artifact handoff Restores three workflow behaviors and does each the ADK-native way. 1. Only the tail step of a loop can break out early. exit_loop is now attached to the last inner step only, not every inner step. It sets Actions.Escalate — the only way an ask tool ends a loopagent — and no other tool touches Actions, so withholding it makes an early break structurally impossible for the non-tail steps. This replaces the old end_turn decision guard, which detected the violation after the fact and re-prompted; now it cannot happen. The non-tail loop instruction is reworded to say the step cannot end the loop. 2. finish_workflow is attached again. The tool and the Progress capture both survived #136, but nothing attached the tool to a step once the graph became one session, so a run reported no artifacts. It is now attached to the final step (WorkflowStepTools). This is how the user learns what a run produced — PR links, tickets — so it is not optional. 3. Artifacts pass structured data between steps. Every step gets save_artifact (native, writes through ctx.Artifacts().Save — ADK ships only load_artifacts) and load_artifacts. This works now, where it did not before #136: the whole graph runs as one runner invocation, so the runner's ArtifactService spans every step. This is the ADK-native replacement for the ask/plans notes directories. Mechanics: CompileWorkflow hands each step a StepRole{InLoop, IsTail, IsFinal} through its ToolsBuilder. IsTail gates exit_loop, IsFinal gates finish_workflow. The TUI builder (cmd/ask/workflow_graph.go) appends tools.WorkflowStepTools directly; the headless builder passes WorkflowStep/WorkflowFinalStep flags to the tool factory, which appends them in BuildCoreTools (pkg/engine can't import pkg/tools). Tests: StepRole is computed correctly for linear/loop/final-loop shapes; WorkflowStepTools attaches save+load always and finish only on the final step; save_artifact writes name+content through ctx.Artifacts().Save and errors (a real Go error, so retryandreflect sees it) with no service or no name; the loop instruction tells only the tail step how to break, and only the final step to call finish_workflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(workflow): end-to-end artifact handoff through the real ADK graph A two-step workflow on the real ADK runner: step 1 saves an artifact, step 2 loads it. Proves the whole seam the earlier tests only covered in pieces — 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 saved content reaches step 2's model. Lives in the external engine_test package so it can import pkg/tools (whose init registers the tool factory) without the tools -> engine import cycle. A scripted model.LLM drives one step at a time by the marker in each step's system instruction. The assertion is airtight against a false pass: the secret must be ABSENT from the reader's step handoff (the saver's node output is 'saved the plan', and IncludeContentsNone keeps its tool calls out of the reader's context) and PRESENT only after load_artifacts resolves. A negative control with a mismatched artifact name fails as expected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the handwritten workflow state machine with ADK's workflow scheduler and deletes the two dead parallel implementations. Net −2,600 lines.
The engine
workflow.Def→workflow.Workflow(pkg/workflow/compile.go):AgentNodewrapping anllmagent, chainedStart → n0 → n1 → …kind: "loop"step is anAgentNodewrapping ADK'sloopagent, its sub-agents carryingexitlooptoolexit_loop(it setsActions.Escalate, whichloopagentwatches), not anend_turndecisionargumentNodeConfig.RetryConfigreplaces the runner's hand-rolledstepErrorRetryloopTwo
llmagentsettings carry the semantics, and both are load-bearing:IncludeContents: IncludeContentsNoneis what isolates a step. Without it a step inherits the whole session, and ADK'sConvertForeignEventrenders every prior step's events as prose — each tool call and each full tool result — so step 3 would carry steps 1 and 2 in their entirety.InstructionProvider, neverConfig.Instruction. Step prompts are user-authored and routinely contain braces; ADK interpolates the static field against session state and hard-fails the invocation on the first unknown{name}. (This is why #135 landed first.)*workflow.Workflowis not anagent.Agent— the interface has an unexported method — soengine.WorkflowGraphAgentwraps it viaagent.New(Config{Run: wf.Run}).One session per run
A run is now one agent session for the whole graph, not one provider session per step. The TUI swaps the session's agent for the compiled graph (
cmd/ask/workflow_graph.go), so tool execution, approvals, cost accounting, and cancellation behave exactly as in a chat turn.StepExecutor/ExecuteStepare gone from both coordinators.Honest progress
pkg/workflow/progress.goderives step lifecycle from the real event stream, withCompiled.StepIndexByAgentmapping an event author back to the step the user authored (so a loop's inner agents report against the loop step).The deleted
RunGraphmarked every step that never ran as both started and done, then hardcoded a successfulFinishData— a chain dying at step 1 of 5 rendered 5/5 green.TestProgress_FailureDoesNotFabricateRemainingStepspins that it can't come back.Notes directories → node output + memory
ask/plans/,plans.go, theclear_planstool, and theRemindFixPlanDirre-prompt were a hack standing in for step-to-step data passing. Handoff is now the graph's node output. Durable reasoning goes topkg/memory— already anadkmemory.Servicewired as the runner'sMemoryService, but never fed from a workflow untilIngestWorkflowMemory.Also drops
load_artifactsfrom the core wire toolset: ADK ships onlyInMemoryServiceandgcsartifact, ask's was rebuilt per turn, and nothing ever saved to it — so it could only ever return empty while costing tokens on every request.Deleted
Runner.Run,RunGraph, bothBuildWorkflowAgents,LoopRunFrame, the remind/re-prompt machinery,graph.go,plans.go, and the TUI's vestigial loop cursor.docs/adk-20-upgrade.md
Rewritten as a status table plus a Gaps section, so the four ADK features that merged as unreachable code are recorded rather than checked off:
retryandreflect(registered but can't fire — ask's tools never return a Go error),functioncallmodifier(predicate always returns false since #132),agenttool(no production callers),toolconfirmation(no tool declares it). Each gap names what wiring it would actually take.Tests
pkg/workflow: graph compilation (linear, loops, agent-name sanitisation + uniqueness — ADK rejects"user"and duplicates while step names are free text, error paths), and progress reporting (ordering, summaries, finish data, loop re-entry, unknown authors, fabricated-step regression).pkg/engine, end to end against a mock model:WorkflowDoneIncludeContentsNoneexists for${VAR},{notes_dir?},{Name, Steps}) reach the model verbatim and don't fail the runcmd/ask: a workflow starts exactly one provider session for a three-step graph.make testgreen across all 9 packages.Next
Parallel/fan-out (
JoinNode,NodeConfig.ParallelWorker) and pause/resume + HITL (workflow.Persistence,NewRequestInputEvent→ question modal) now sit on top of this rather than needing a re-architecture.🤖 Generated with Claude Code