fix(engine): resolve @-links per scope, and settle the remaining ADK gaps

docs/adk-20-upgrade.md

@@ -27,9 +27,9 @@ purpose: **a row is only "done" when a production caller reaches it.**
 | Per-node retry | `workflow.NodeConfig.RetryConfig` | **done** — replaced the hand-rolled retry loop |
 | Step context isolation | `llmagent.IncludeContentsNone` | **done** |
 | Self-healing tool errors | `plugin/retryandreflect` | **done** — tools return Go errors, so `OnToolErrorCallback` fires |
-| Parameter injection | `plugin/functioncallmodifier` | **inert** — see Gaps |
-| Subagent delegation | `tool/agenttool` | **not wired** — see Gaps |
-| Human-in-the-loop | `tool/toolconfirmation` | **not wired** — see Gaps |
+| Parameter injection | `plugin/functioncallmodifier` | **not needed** — `description` is a static params field |
+| Subagent delegation | `tool/agenttool` | **rejected** — see Deliberate departures |
+| Human-in-the-loop | `tool/toolconfirmation` | **deferred** — needs turn suspend/resume |
 | Artifacts | `artifact.Service`, `loadartifactstool` | **dropped** — see Gaps |
 | Dynamic instructions | `util/instructionutil` | **deliberately not used** — see Gaps |
 | Parallel / fan-out | `JoinNode`, `NodeConfig.ParallelWorker` | **planned** |
@@ -88,45 +88,53 @@ so a chain that died at step 1 of 5 rendered 5/5 green.
 
 ---
 
-## Gaps
-
-Recorded so the next reader does not mistake an import for an
-integration.
-
-**`plugin/functioncallmodifier`** is registered with a predicate that
-always returns `false` (`pkg/engine/plugins.go`), so it never applies.
-PR #132 disabled it to fix a proto validation error; the manual JSON
-schema surgery it was meant to replace is still in
-`pkg/tools/bridge.go`.
-
-**`tool/agenttool`** — `BuildResearchSubagent`, `BuildNamedSubagent`,
-`BuildResearchAgentTool`, and `BuildNamedAgentTool` have no production
-callers. The `task` tool still spawns a nested `engine.Run`.
-
-**`tool/toolconfirmation`** — no tool declares `RequireConfirmation` or a
-`ConfirmationProvider`, so ADK never emits `adk_request_confirmation` and
-the handling in `run.go` / `agent_run.go` is unreachable. Approval is the
-in-tool blocking path in `pkg/tools/env.go`.
-
-**Artifacts** were dropped rather than wired. ADK ships only
-`InMemoryService` and `gcsartifact`; ask's was rebuilt per turn and
-nothing ever saved to it, so `loadartifactstool` could only return empty
-while costing tokens on every request. Node outputs cover step handoff
-and `pkg/memory` covers durable state.
-
-**`util/instructionutil`** is deliberately not used. ask's instruction
-text is user documentation inlined verbatim, not a template — see the
-comment on `BuildInstructionProvider`.
-
----
+## Deliberate departures
+
+Recorded so nobody "finishes the migration" by wiring one of these.
+
+**`plugin/functioncallmodifier` — removed, not needed.** 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 shipped 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; that is a one-time build at construction.
+
+**`tool/agenttool` — rejected for the task tool.** `agent_tool.go`
+builds its own runner with a hardcoded config: no `PluginConfig`, so a
+subagent would lose `retryandreflect`, and `MemoryService:
+memory.InMemoryService()`, so it would lose 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. The four
+builders added for this migration had no production callers and are
+deleted.
+
+**`tool/toolconfirmation` — deferred.** It emits an
+`adk_request_confirmation` function 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 is `ToolEnv.ApprovalDenied`,
+which blocks on the TUI modal and returns the denial inline.
+`IsConfirmationCall` / `UnwrapConfirmationCall` stay wired in the event
+loops because an MCP server can declare confirmation on its own tools.
+
+**`util/instructionutil` — not used.** ask's instruction text is user
+documentation inlined verbatim, not a template. See the comment on
+`BuildInstructionProvider`.
+
+**Artifacts — dropped.** ADK ships only `InMemoryService` and
+`gcsartifact`; ask's was rebuilt per turn and nothing ever saved to it.
+Node outputs cover step handoff and `pkg/memory` covers durable state.
 
 ## Planned
 
-**Parallel / fan-out.** A `parallel` step kind alongside `loop`, compiled
-to fan-out edges plus a `JoinNode`, with `NodeConfig.ParallelWorker` for
-list-typed inputs. Needs builder UI and a store schema addition.
+See [follow-ups.md](follow-ups.md). Two items:
 
-**Pause / resume and HITL.** `workflow.Persistence` plus
-`Workflow.Resume`, and `NewRequestInputEvent` routed to ask's question
-modal — replacing today's behaviour where workflow tabs auto-decline
-every prompt.
+- **Pause and resume** — non-blocking approvals and resumable workflows.
+  One project, not two: both need the ability to pause a turn and pick it
+  up later. Covers `workflow.Persistence`, `Workflow.Resume`,
+  `NewRequestInputEvent`, and `tool/toolconfirmation`.
+- **Parallel / fan-out** — a `parallel` step kind using `JoinNode` and
+  `NodeConfig.ParallelWorker`. Independent of the above.

docs/follow-ups.md

@@ -0,0 +1,80 @@
+# Follow-ups
+
+Work that is deliberately not done yet, with the reasoning, so it can be
+picked up without re-deriving it. Written 2026-08-21, after the ADK
+migration audit.
+
+---
+
+## 1. Pause and resume — the big one
+
+**The problem today.** When the agent wants to write a file or run a
+command, ask pops a modal and the agent freezes. You have to be at that
+machine, at that moment. Walk away and it sits there. Close the app and
+the work is gone.
+
+Worse: workflow tabs **auto-deny every approval**, because no human is
+attached to answer. That is why workflows only really work for
+pre-cleared operations.
+
+**What changes.** The agent stops cleanly, saves where it was, and the
+approval becomes a message instead of a blocking popup. You answer
+whenever. The run picks up where it left off.
+
+**What it unlocks, in rough order of value:**
+
+1. **Workflows that can ask.** A pipeline that hits an approval pauses
+   and waits instead of being denied. This is the difference between
+   "workflows only do pre-cleared work" and "workflows do real work."
+2. **Approvals that survive a restart.** Close the laptop, come back,
+   approve, continue.
+3. **Approve from somewhere else.** Once an approval is data rather than
+   a modal, it can go to a phone, a web page, Slack. Prerequisite for ask
+   running anywhere but the terminal in front of you.
+4. **Long-running agents.** Something that runs for an hour, hits one
+   decision point, parks, waits.
+
+**Why it is one project, not two.** Resumable workflows and
+non-blocking approvals need the same machinery: the ability to pause a
+turn and pick it up later. Doing either gets most of the other.
+
+**The ADK pieces that map to it:** `workflow.Persistence` and
+`Workflow.Resume` for the workflow half, `NewRequestInputEvent` for
+surfacing the prompt, and `tool/toolconfirmation` for approvals. ask
+currently uses none of them, and `ToolEnv.ApprovalDenied` blocks on the
+modal instead.
+
+**Size.** Not small. ask has no suspend/resume path for a chat turn at
+all; that is the work.
+
+---
+
+## 2. Parallel / fan-out
+
+Independent of the above. Add a `parallel` step kind alongside `loop` in
+a workflow definition: fan several steps out at once, join their results,
+continue.
+
+The graph engine already supports it — `JoinNode` for the join,
+`NodeConfig.ParallelWorker` for running a node once per item of a list
+input. The work is the workflow schema, the builder UI, and the
+compiler mapping. It needs nothing from follow-up 1.
+
+---
+
+## 3. Agent discoverability (maybe)
+
+**The symptom to watch for:** the model not using agents that are
+defined. Today there is one `task` tool and the model names which agent
+it wants as a parameter; the available agents are described in prose in
+the system prompt.
+
+ADK's `tool/agenttool` would make every agent its own tool, which models
+pick from more reliably than from a prose list. It was rejected —
+see "Deliberate departures" in `adk-20-upgrade.md` — because it would
+cost every agent's definition on every request, and because subagents
+would lose memory access, tool-error recovery, and background execution.
+
+If this turns out to be a real complaint, cheaper fixes first: better
+trigger descriptions on agent definitions, or surfacing agents through
+the same registry search that MCP tools use.

pkg/engine/interaction.go

@@ -114,6 +114,19 @@ func (h HeadlessInteractionHandler) RequestSudoPassword(ctx context.Context, tab
 	return SudoPasswordResponse{Cancelled: true}, nil
 }
 
+// Approval is NOT ADK's tool/toolconfirmation flow, deliberately.
+//
+// That flow emits an adk_request_confirmation function call, pauses the
+// run, and resumes when a function response arrives. ask has no
+// suspend/resume path for a chat turn, so adopting it means building
+// one. Instead a mutating tool calls ToolEnv.ApprovalDenied, which blocks
+// on the TUI modal and returns the denial message inline.
+//
+// The two unwrap helpers below stay because an MCP server can declare
+// confirmation on its own tools; if ADK ever emits the call, the event
+// loops in run.go and agent_run.go render the inner intent rather than
+// the wrapper.
+
 // IsConfirmationCall reports whether a function call is an ADK tool confirmation request.
 func IsConfirmationCall(fc *genai.FunctionCall) bool {
 	if fc == nil {
@@ -126,10 +139,3 @@ func IsConfirmationCall(fc *genai.FunctionCall) bool {
 func UnwrapConfirmationCall(fc *genai.FunctionCall) (*genai.FunctionCall, error) {
 	return toolconfirmation.OriginalCallFrom(fc)
 }
-
-// FormatConfirmationResponse constructs the standard ADK confirmation response payload.
-func FormatConfirmationResponse(confirmed bool) map[string]any {
-	return map[string]any{
-		"confirmed": confirmed,
-	}
-}

pkg/engine/interaction_test.go

@@ -52,14 +52,4 @@ func TestInteraction_ConfirmationHelpers(t *testing.T) {
 		t.Errorf("expected unwrapped command 'rm -rf tmp', got %v", origCall.Args["command"])
 	}
 
-	// FormatConfirmationResponse
-	respTrue := FormatConfirmationResponse(true)
-	if confirmed, ok := respTrue["confirmed"].(bool); !ok || !confirmed {
-		t.Errorf("expected confirmed: true in response, got %v", respTrue)
-	}
-
-	respFalse := FormatConfirmationResponse(false)
-	if confirmed, ok := respFalse["confirmed"].(bool); !ok || confirmed {
-		t.Errorf("expected confirmed: false in response, got %v", respFalse)
-	}
 }

pkg/engine/plugins.go

@@ -2,44 +2,37 @@ package engine
 
 import (
 	"google.golang.org/adk/v2/plugin"
-	"google.golang.org/adk/v2/plugin/functioncallmodifier"
 	"google.golang.org/adk/v2/plugin/retryandreflect"
-	"google.golang.org/genai"
 )
 
 // DefaultPlugins returns the standard set of ADK plugins configured for ask.
+//
+// functioncallmodifier is deliberately absent. It exists to inject
+// synthetic arguments into tool declarations at request time; ask needed
+// that for the required `description` phrase, which is now a real field
+// on every native tool's params struct, so there is nothing left to
+// inject. It was registered here with a predicate that always returned
+// false ever since PR #132 disabled it to stop it clobbering
+// ParametersJsonSchema — a plugin that could never fire.
+//
+// Bridge tools (linear_*, workflow_*) still have `description` added to
+// their input schema in pkg/tools/bridge.go, because their input types
+// come from the MCP handler cores and do not carry the field. That is a
+// one-time schema build at construction, not per-request AST surgery.
 func DefaultPlugins() []*plugin.Plugin {
 	var plugins []*plugin.Plugin
 
-	// 1. Retry and reflect plugin for automated in-turn tool error self-healing.
+	// Retry and reflect: when a tool returns a Go error, hand the model
+	// corrective guidance and let it retry in the same turn instead of
+	// surrendering. Every ask tool reports failure as a real error, which
+	// is what OnToolErrorCallback keys on.
 	if retryPlugin, err := NewRetryAndReflectPlugin(2); err == nil && retryPlugin != nil {
 		plugins = append(plugins, retryPlugin)
 	}
 
-	// 2. Function call modifier plugin for parameter injection when configured.
-	// Defaults to inactive so native tools using functiontool.New with ParametersJsonSchema
-	// are not corrupted by the plugin's decl.Parameters initialization.
-	if modPlugin, err := NewFunctionCallModifierPlugin(FunctionCallModifierOptions{
-		Predicate: func(toolName string) bool {
-			return false
-		},
-	}); err == nil && modPlugin != nil {
-		plugins = append(plugins, modPlugin)
-	}
-
 	return plugins
 }
 
-func isCoreCodingTool(toolName string) bool {
-	switch toolName {
-	case "read", "write", "edit", "glob", "grep", "ls", "bash", "job_output", "job_kill",
-		"fetch", "todos", "ask_user_question", "end_turn", "web_search":
-		return true
-	default:
-		return false
-	}
-}
-
 // NewRetryAndReflectPlugin creates an ADK retryandreflect plugin with the specified max retries.
 func NewRetryAndReflectPlugin(maxRetries int) (*plugin.Plugin, error) {
 	if maxRetries <= 0 {
@@ -50,26 +43,3 @@ func NewRetryAndReflectPlugin(maxRetries int) (*plugin.Plugin, error) {
 		retryandreflect.WithTrackingScope(retryandreflect.Invocation),
 	)
 }
-
-// FunctionCallModifierOptions defines configuration for the functioncallmodifier plugin.
-type FunctionCallModifierOptions struct {
-	Predicate           func(toolName string) bool
-	Args                map[string]*genai.Schema
-	OverrideDescription func(originalDescription string) string
-}
-
-// NewFunctionCallModifierPlugin creates an ADK functioncallmodifier plugin with safety guards.
-func NewFunctionCallModifierPlugin(opts FunctionCallModifierOptions) (*plugin.Plugin, error) {
-	pred := opts.Predicate
-	if pred == nil {
-		pred = func(toolName string) bool {
-			return len(opts.Args) > 0 || opts.OverrideDescription != nil
-		}
-	}
-	cfg := functioncallmodifier.FunctionCallModifierConfig{
-		Predicate:           pred,
-		Args:                opts.Args,
-		OverrideDescription: opts.OverrideDescription,
-	}
-	return functioncallmodifier.NewPlugin(cfg)
-}