fix(workflows): interpolate dispatch inputs so merge continuation resolves the epic - #1741
Conversation
🏗️ Architectural Review
Automated architectural review — informational only. |
🟡 Impact Analysis — PR #1741Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedci-workflows (1 file)
root (2 files)
scripts (1 file)
This report is generated automatically for every PR. See #733 for details. |
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 4 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | 0 active Copilot thread(s) resolved (1 outdated skipped) |
| ✅ | CI passing | All checks passing |
Files Changed (4 files, +203 −12)
| File | +/− |
|---|---|
.github/workflows/squad-workflow-lint.yml |
+9 −0 |
scripts/check-workflow-input-interpolation.mjs |
+139 −0 |
workflows/squad-implement-worker.md |
+14 −1 |
workflows/squad.md |
+41 −11 |
Total: +203 −12
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
…olves the epic The PR-merged epic continuation chain fired correctly end-to-end but died on the last hop. squad.md described its workflow_dispatch inputs in prose -- backtick-quoted expression names, never real interpolations -- so the dispatched issue number was delivered to Actions but never rendered into the agent's context. The run-name interpolated fine, so the Actions tab showed a healthy "Squad -- implement" run while the agent saw no command and emitted noop. noop is configured report-as-issue: false, so the epic went silent with no next step for the user. - Interpolate the dispatched command and issue number in Trigger Context, with an explicit resolution order and a rule against no-oping a non-empty dispatch. - Require every Implement run against an epic to end with a status comment, covering the dispatched, blocked, slots-full, and complete cases. - Add a lint guard for bare github.event.inputs.* references in prompt bodies. Scoped to inputs only: comment.body and issue.body carry untrusted text and are intentionally left un-interpolated to avoid a prompt-injection vector. Closes #1740 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
b0ea81a to
512853b
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a gh-aw prompt rendering gap where workflow_dispatch inputs were previously referenced only as prose in workflows/squad.md, causing merge-driven epic continuation runs to silently no-op because the agent never received the dispatched command/issue_number. It also adds a CI guard to prevent this regression class from recurring.
Changes:
- Update
workflows/squad.mdto interpolategithub.event.inputs.commandandgithub.event.inputs.issue_numberinto the prompt body and clarify command/issue resolution order (including a “no noop on dispatch” rule). - Strengthen Implement-mode guidance to always post a terminal-status comment on epics (never silently exit via
noop). - Add a Node-based lint script and wire it into the workflow lint job to fail on bare
github.event.inputs.*references in Markdown prompt bodies.
Show a summary per file
| File | Description |
|---|---|
| workflows/squad.md | Interpolates dispatch inputs into Trigger Context and tightens Implement-mode “always comment” behavior to avoid silent continuation failures. |
| scripts/check-workflow-input-interpolation.mjs | New repo guard that scans prompt Markdown bodies and fails on prose-only github.event.inputs.* references. |
| .github/workflows/squad-workflow-lint.yml | Runs the new interpolation guard in CI when workflows/prompt sources or the guard script change. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
…ill bodies Two rationales that were implicit and would be easy to "helpfully" undo: - Widening to all github.event.* would flag comment.body/issue.body, whose correct form IS a bare prose reference -- they carry attacker-controlled text that gh-aw sanitizes via trigger context, so interpolating them would splice untrusted input into the prompt ahead of the sanitizer. - Scanning whole files (skill blocks included) is load-bearing now that mode playbooks are restored in isolation: a skill body needing a dispatch input must interpolate it itself rather than assume Trigger Context ran first. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
The router's Implement mode now always leaves a visible next step, but the worker's merge-continuation path still emitted `noop` when a merged pull request's issue had no parent epic. `noop` is not reported as a comment, so the run ended with no signal on the pull request -- the same "merge happened, nothing visible to do next" failure the dispatch fix addresses, one hop later. Observed live: merging a Squad implementation pull request produced a successful worker run that logged "report-as-issue is disabled, skipping no-op message posting" and surfaced nothing. The worker already declares `add-comment` with `target: "*"`, so it had the capability and simply was not required to use it. Refs #1740 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
The header claimed the script uses only fs and path; it also imports fileURLToPath from node:url. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
Closes #1740
What broke
The PR-merged → continue-implementation chain already existed and fired correctly end-to-end — it just died on the last hop, silently.
53498appears nowhere in that run's agent log.Root cause
workflows/squad.mdonly described itsworkflow_dispatchinputs in prose — backtick-quoted expression names, never${{ }}interpolations. The values were delivered to Actions but never rendered into the agent's context.The frontmatter
run-namedoes interpolate, which is why the Actions tab showed a healthySquad — implementrun while the agent saw nothing.This was drift, not a novel mistake.
workflows/squad-implement-worker.mdalready had it right — 7/7github.event.inputs.*references interpolated (5/5 in the prompt body, plusrun-nameandconcurrency). Two files that should have followed the same rule diverged, and only one of them was correct. The worker is the reference implementation.Compounding it:
noopis configuredreport-as-issue: false, so the failed continuation posted zero comments. The epic went quiet with no next step — exactly what surfaced on github/gh-aw#53498.How the interpolation actually works — please read before extending this pattern
It is tempting to assume GitHub Actions interpolates the prompt. It does not, and the real mechanism constrains what is safe to add here.
The compiled
squad.lock.ymlcontains zero prompt text. The body is pulled in at runtime:The file is read off disk inside the job, so Actions never sees its
${{ }}. Instead gh-aw's ownruntime_import.cjsresolves them — and it does not neutralize what it dislikes, it throws:Here is the part worth knowing:
github.event.inputs.commandis not in that module'sALLOWED_EXPRESSIONSlist. It is accepted anyway, viaisSafeExpression()'s property-path check, and then resolved throughcontext.payload.inputs(populated forworkflow_dispatch).So interpolating a dispatch input in a prompt body is safe because of a code path, not because of the allow-list — reading the list alone would suggest this change hard-fails the workflow. It was verified by executing
isSafeExpression()against the real module, and by running the whole 150 KB file throughprocessExpressions()without a throw.Practical consequence: anyone extending this pattern to a different expression should execute
isSafeExpression()against it first. A wrong guess fails the run at runtime, not at compile time.Changes
1. Interpolate, don't describe (
workflows/squad.md)Trigger Context now emits real values for the dispatched command and issue number, with an explicit resolution order for each and a rule against no-oping a non-empty dispatch.
2. Always leave a visible next step (
workflows/squad.md)Every Implement run against an epic must end with a comment on that epic, covering all four terminal cases: children dispatched, all blocked, all slots occupied, or implementation complete.
noopis explicitly banned for Implement runs.3. Lint guard (
scripts/check-workflow-input-interpolation.mjs+ CI step)Fails on bare
github.event.inputs.*references in agentic prompt bodies. This bug class is invisible toactionlint(which only reads YAML) and invisible at runtime (the run reports success), so it recurs silently without a gate.Measured against
devbefore this fix, the gate finds 4 bare references, all inworkflows/squad.md(L135, L140, L141, L486) — and zero insquad-implement-worker.md, which quantifies the drift above.4. Same rule for the worker's merge continuation (
workflows/squad-implement-worker.md)Added after a live end-to-end run showed change 2 fixed only half the problem.
The worker's post-merge path still emitted
noopwhen a merged pull request's issue had no parent epic — andnoopis not reported as a comment, so a merged PR ended with no signal about what happens next. That is the same defect as #53498, one hop further down the chain: change 2 covers the router's epic runs, this covers the worker's per-PR runs.Observed live: merging a Squad implementation pull request produced a green worker run whose log read
report-as-issue is disabled, skipping no-op message posting. Nothing was posted anywhere a human would look. The worker already declaresadd-commentwithtarget: "*"— it had the capability all along and simply wasn't required to use it. Both terminal cases now end in a comment: epic resolved and children queued, or issue is standalone and nothing further queued.The head-branch guard case needs no rule — it's a workflow-level
if:, so the agent never runs at all for non-Squad merges. Verified live when a mergedsquad/cast-*PR correctly produced a skipped run.Two scoping decisions worth reviewing
Scoped to
inputs.*, not allgithub.event.*. An earlier draft flagged everything and immediately caughtcomment.bodyandissue.body. Those must stay un-interpolated: they carry attacker-controlled text that gh-aw delivers through sanitized trigger context, so interpolating them would splice untrusted input into the prompt ahead of the sanitizer. There, a bare prose reference is the correct form. The gate was working; the tempting fix was wrong. That reasoning is now a comment directly above the filter so the next reader isn't tempted to "fix" it.Scans whole files, including
## skill:blocks. This is load-bearing after #1739. One of the four bare references ondev(L486) sits inside thesquad-implementskill block — a gate that only walked ambient prose would have missed a live instance of the exact bug on day one.It also enforces something the skill refactor made structural: skill bodies are restored in isolation with no guarantee about what precedes them, so a skill needing a dispatch input must interpolate it itself rather than assume
## Trigger Contextalready resolved it into scope. The gate turns that from a convention into a checked property.Validation
Reproduced the failure and the fix offline, against production gh-aw code. Downloaded
github/gh-aw-actionsat the exact SHA the lock file pins (6aab9e5b5c91c615506061f09bedd81a23babe3c, v0.86.2) and ran its realruntime_import.cjsover the fullsquad.mdbody with a simulatedworkflow_dispatchpayload of{command: "implement", issue_number: "53498"}:dev(pre-fix)53498reaches the prompt`github.event.inputs.issue_number``53498`The pre-fix column is run
32082667356'snoop, reproduced deterministically without dispatching anything.Also verified:
gh aw compile --strictagainst a scratch checkout: 2 workflows, 2 succeeded. The compiler lifts both inputs intoGH_AW_GITHUB_EVENT_INPUTS_*env vars across all three prompt steps.processExpressions()run does not throw — no unauthorized expression anywhere in the merged file.39f7a2f(perf(workflows): load shipped mode playbooks as inline skills #1739, mode playbooks → inline skills). Clean rebase, but placement was verified by listing H2 boundaries rather than trusting the diff: both implement hunks land at L504 / L553, inside## skill: \squad-implement`` (spans 489–569). Git will reattach a hunk across a skill boundary and still rebase cleanly.##headings added inside a skill block — edits are bold lead-ins and bullets, so nothing implicitly closes it.test/gh-aw-quality.test.ts: 79 passed, 13 skipped — includingdiscards no content during extraction,loses no bytes overall, ambient-prompt-under-40 KB, and the strict-compile contract test.github.event.inputs.command, while ignoring the interpolated frontmatter in the same fixture.packages/*/src/changes, so no changeset required.Live end-to-end run (
bradygaster/aspiregregator-squad-test, a real .NET/Aspire/Orleans repo) exercised change 1 in production rather than in simulation:/squad implementon an issue produced the visible router status comment — "Dispatched the implementation worker for this issue (feat: GitHub Issues intake, PRD mode, and human team members #2)" — which is precisely the behavior this PR adds. Pre-fix, that run would have no-op'd silently.Note on ordering
Once this lands, gh-aw needs its
squad.mdrefreshed from this source before pelikhan's epic will resume — the fix has to reach the consumer repo to unblock #53498.