feat(workflow): migrate workflow engine, artifacts, and runtime to native ADK 2.0 - #132
Open
Cidan wants to merge 1 commit into
Open
feat(workflow): migrate workflow engine, artifacts, and runtime to native ADK 2.0#132Cidan wants to merge 1 commit into
Cidan wants to merge 1 commit into
Conversation
- Replace legacy hand-rolled workflow state machine and string-concatenation step runner with native ADK 2.0 DAG execution using WorkflowAgent and ADK runner event streaming. - Integrate AgentFactory / WorkflowAgentConfig into coordinator and engine runtimes for dynamic step model, tools, and instruction building. - Replace standaloneAgentContext.Artifacts() stubs with active artifact.Service implementation and expose LoadArtifactsTool on interactive session wire. - Purge hardcoded plugin bypasses (FunctionCallModifier) and dual-state side-channel memory injections (file tool recall footers and static XML memory blocks). - Remove static XML skills system prompt injection in favor of dynamic ADK SkillToolset. - Update workflow runner unit tests and graph test suites to verify genuine ADK DAG orchestration.
This was referenced Aug 21, 2026
Cidan
added a commit
that referenced
this pull request
Aug 21, 2026
…gaps Closes the last items from the ADK audit. Two were real bugs, two were dead code that should not be wired, and one is deferred with a reason. @-links now resolve against the scope that owns the document, not against the project root. An @-link in ~/.claude/CLAUDE.md — the real case is "@RTK.md", which sits next to it — was looked up as <repo>/RTK.md and silently found nothing. It became reachable when PR #133 started loading the user-global scope. AgentContextScopes pairs each search directory with its link root, LoadedContextDoc.Root and Rule.Root carry it, and BuildSystemPrompt groups links by root before resolving. The JIT rule path uses r.Root for the same reason. functioncallmodifier is removed rather than enabled. It injects synthetic arguments into tool declarations at request time; ask needed that for the required `description` phrase, which is now a static field on every native tool's params struct, so there is nothing left to inject. It had been registered with a predicate that always returned false since PR #132. Bridge tools still get `description` added to their input schema in pkg/tools/bridge.go, because their input types come from the MCP handler cores — a one-time build at construction, not per-request surgery. agenttool is rejected, and its four dead builders are deleted. agent_tool.go builds its own runner with a hardcoded config: no PluginConfig, so a subagent loses retryandreflect, and MemoryService: memory.InMemoryService(), so it loses ask's memory. It also produces one tool per agent, replacing task(agent: "foo") with a tool named foo. The task tool's nested engine.Run goes through RunnerBuilder — ask's plugins, memory, and file session service — and keeps the background-job path and the subagent UI events. toolconfirmation is deferred, not forgotten. It emits an adk_request_confirmation call, pauses the run, and resumes on a function response; ask has no suspend/resume path for a chat turn, so adopting it means building one. Approval stays ToolEnv.ApprovalDenied, blocking on the modal. IsConfirmationCall / UnwrapConfirmationCall stay wired because an MCP server can declare confirmation on its own tools; FormatConfirmationResponse had no callers at all and is deleted. One item from the audit was my error and needed no change: the deferred registry is not empty for subagents. The task tool passes the parent session's deferred tools as opts.Tools, so engine.Run never reaches the nil registryFunc. It is nil only on a pure headless engine.Run with no tools supplied, where there genuinely is no registry — MCP is not connected and the linear tools live in cmd/ask. docs/adk-20-upgrade.md gains a "Deliberate departures" section so none of these gets wired later by someone reading an import as an intention. Tests: global-scope @-links resolve under ~/.claude and do not pick up a same-named file in the project; project-scope links still resolve under the project root; every coding tool declares `description` statically, which is what makes functioncallmodifier unnecessary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cidan
added a commit
that referenced
this pull request
Aug 21, 2026
…gaps (#138) Closes the last items from the ADK audit. Two were real bugs, two were dead code that should not be wired, and one is deferred with a reason. @-links now resolve against the scope that owns the document, not against the project root. An @-link in ~/.claude/CLAUDE.md — the real case is "@RTK.md", which sits next to it — was looked up as <repo>/RTK.md and silently found nothing. It became reachable when PR #133 started loading the user-global scope. AgentContextScopes pairs each search directory with its link root, LoadedContextDoc.Root and Rule.Root carry it, and BuildSystemPrompt groups links by root before resolving. The JIT rule path uses r.Root for the same reason. functioncallmodifier is removed rather than enabled. It injects synthetic arguments into tool declarations at request time; ask needed that for the required `description` phrase, which is now a static field on every native tool's params struct, so there is nothing left to inject. It had been registered with a predicate that always returned false since PR #132. Bridge tools still get `description` added to their input schema in pkg/tools/bridge.go, because their input types come from the MCP handler cores — a one-time build at construction, not per-request surgery. agenttool is rejected, and its four dead builders are deleted. agent_tool.go builds its own runner with a hardcoded config: no PluginConfig, so a subagent loses retryandreflect, and MemoryService: memory.InMemoryService(), so it loses ask's memory. It also produces one tool per agent, replacing task(agent: "foo") with a tool named foo. The task tool's nested engine.Run goes through RunnerBuilder — ask's plugins, memory, and file session service — and keeps the background-job path and the subagent UI events. toolconfirmation is deferred, not forgotten. It emits an adk_request_confirmation call, pauses the run, and resumes on a function response; ask has no suspend/resume path for a chat turn, so adopting it means building one. Approval stays ToolEnv.ApprovalDenied, blocking on the modal. IsConfirmationCall / UnwrapConfirmationCall stay wired because an MCP server can declare confirmation on its own tools; FormatConfirmationResponse had no callers at all and is deleted. One item from the audit was my error and needed no change: the deferred registry is not empty for subagents. The task tool passes the parent session's deferred tools as opts.Tools, so engine.Run never reaches the nil registryFunc. It is nil only on a pure headless engine.Run with no tools supplied, where there genuinely is no registry — MCP is not connected and the linear tools live in cmd/ask. docs/adk-20-upgrade.md gains a "Deliberate departures" section so none of these gets wired later by someone reading an import as an intention. Tests: global-scope @-links resolve under ~/.claude and do not pick up a same-named file in the project; project-scope links still resolve under the project root; every coding tool declares `description` statically, which is what makes functioncallmodifier unnecessary. Co-authored-by: Claude Opus 5 (1M context) <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.
Summary
This PR completes the full migration of `ask`'s workflow engine, session artifacts, and agent runtime onto Google ADK 2.0 (`google.golang.org/adk/v2`), eliminating legacy shadow code, hardcoded stubs, and dual-state hand-rolled implementations.
Architecture & Key Changes
Native ADK 2.0 Workflow DAG Engine:
Session Artifacts:
Plugins & Memory Deduplication:
Agent Skills:
Motivation
An audit of the previous modernization PRs against the codebase revealed that several major ADK 2.0 milestones were either faked as isolated test-only shadow code (e.g. `pkg/workflow/graph.go` never being invoked by the actual runtime), bypassed via hardcoded booleans (e.g. `plugins.go:26`), or stubbed with `return nil` (e.g. `Artifacts()`). This PR resolves those discrepancies and ensures that all multi-step workflows, artifacts, skills, and memory interactions run natively through ADK 2.0.
Verification