dispatch-workflow: untyped dispatch_workflow tool accepts empty args and displaces the typed per-workflow tool

[gregsmi]

Summary

With a dispatch-workflow safe-output, gh-aw registers two MCP tools: the typed per-workflow tool (e.g. my_fixer, with a real schema derived from the target's workflow_dispatch inputs) and an untyped catch-all dispatch_workflow. Three behaviours compound into a hard run failure:

1. The injected <safe-output-tools> prompt manifest advertises only dispatch_workflow and omits the typed tool — so the agent is steered to the untyped one.
2. dispatch_workflow's MCP schema is empty and unvalidated, so a {} call is accepted and answered with a success message, recording a real safe-output item.
3. max: truncation keeps the first item, so when the agent notices its mistake and re-calls the typed tool correctly, the malformed item wins and the good one is discarded.

Net effect: an agent that self-corrects still fails the run.

Reproduction

Frontmatter:

safe-outputs:
  dispatch-workflow:
    workflows: [my-fixer]
    max: 1

Have the agent call dispatch_workflow with {} (e.g. to probe the schema), then call the typed my_fixer tool correctly.

Observed:

• The MCP server replies to the empty call with Safe-job 'dispatch_workflow' executed successfully with arguments: {}.
• safeoutputs.jsonl records a bare {"type":"dispatch_workflow"} item, followed by the correct typed item.
• Truncation to max: 1 keeps the bare item and drops the correct one.
• The safe_outputs job then fails: POST /repos/.../actions/workflows/my-fixer.lock.yml/dispatches - 422 → Failed to dispatch workflow "my-fixer": Required input 'pr_number' not provided.

Note the 422 names the target workflow, so workflow_name was resolved despite being absent from the item — presumably defaulted from the single entry in workflows:. GH_AW_VALIDATION_JSON marks workflow_name as required: true for dispatch_workflow, yet the bare item still reached the dispatch step, so that validation appears not to be enforced on this path (or is satisfied by the defaulting). Either way the empty call should not have become a dispatch item.

Root cause: the manifest omission is a systematic gap

generateDynamicTools (pkg/workflow/safe_outputs_tools_generation.go) generates typed MCP tools for six categories:

• SafeOutputs.Jobs
• SafeOutputs.Scripts
• SafeOutputs.Actions
• SafeOutputs.DispatchWorkflow
• SafeOutputs.DispatchRepository
• SafeOutputs.CallWorkflow

But buildSafeOutputsSections — which produces the <safe-output-tools> Tools: line the agent actually reads — only enumerates the first three. TestBuildSafeOutputsSectionsCustomToolsConsistency in pkg/workflow/safe_outputs_prompt_tools_test.go asserts exactly that set, and its doc comment states the intent:

verifies that every custom tool type registered in the runtime configuration has a corresponding entry in the compiled <safe-output-tools> prompt block — preventing silent drift.

So the invariant is already established and deliberately tested; dispatch_workflow, dispatch_repository, and call_workflow are simply outside its coverage.

dispatch-workflow is the worst of the three, because the static safe_outputs_tools.json also contributes a generic dispatch_workflow entry, enabled whenever the config key is present. So for this category the manifest doesn't merely omit the typed tool — it advertises an untyped one in its place. That is what actively steers the agent to the unvalidated call rather than merely leaving the right one undiscovered.

The two registrations, as they appear in a compiled lock:

{ "name": "my_fixer", "inputSchema": { "type": "object", "required": ["pr_number"], "properties": { ... } } }

versus dispatch_workflow, registered as a "Custom safe-job" with "properties": {}, "additionalProperties": true, and nothing required.

The same lock file contains, several hundred lines earlier, the prompt chunk the agent reads:

<safe-output-tools>
Tools: add_comment, create_pull_request_review_comment(max:15), add_labels, remove_labels, dispatch_workflow, missing_tool, missing_data, noop

my_fixer is registered and callable, but absent from that list. A single generated file both registers the typed tool and tells the agent it doesn't exist.

This repo's own workflows are exposed to the same path: squad.md, squad-implement-worker.md, and the smoke-copilot* workflows all configure dispatch-workflow:, so their agents see a Tools: line naming dispatch_workflow while the typed squad_implement_worker / haiku_printer tools go unadvertised.

Impact

The failure is not self-healing. In a review-loop setup where a merge gate requires the critic run to succeed, this fails the gate closed and strands the PR until a human dispatches the downstream workflow by hand.

It is also probabilistic — it depends on the agent choosing to probe — so it presents as a rare, hard-to-reproduce flake rather than a deterministic bug.

Suggested fixes

1. Close the manifest gap. Extend buildSafeOutputsSections to enumerate all six dynamic tool categories, and widen TestBuildSafeOutputsSectionsCustomToolsConsistency to assert over them, so the existing anti-drift invariant covers the remaining three. Additionally, suppress the generic dispatch_workflow from the manifest when typed per-workflow tools exist, so the agent isn't pointed at the unvalidated tool.
2. Reject empty-argument calls on custom safe-job tools. Built-ins already carry an explicit anti-probing guard in their descriptions (e.g. add_comment's "WRITE-ONCE: do NOT call this tool with empty or placeholder arguments to probe"). Custom safe-jobs get neither the guard nor schema enforcement.
3. Make max: truncation prefer schema-valid items. Dropping a well-formed item in favour of a malformed one converts a recoverable agent mistake into a run failure. Validating before truncating, or preferring valid items when truncating, would make this class of error self-correcting.

Any one of the three breaks the chain; (1) is the most targeted and (3) is the most general.

Environment

Reproduced on gh-aw v0.86.2. The dual registration, the empty schema, and the manifest omission are byte-identical in locks compiled with v0.83.4 and v0.85.4, so this is longstanding behaviour rather than a recent regression.