From 512853b29689f67baef914ed0825315077455dc6 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Mon, 17 Aug 2026 18:27:35 -0700 Subject: [PATCH 1/4] fix(workflows): interpolate dispatch inputs so merge continuation resolves 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 --- .github/workflows/squad-workflow-lint.yml | 9 ++ .../check-workflow-input-interpolation.mjs | 128 ++++++++++++++++++ workflows/squad.md | 52 +++++-- 3 files changed, 178 insertions(+), 11 deletions(-) create mode 100644 scripts/check-workflow-input-interpolation.mjs diff --git a/.github/workflows/squad-workflow-lint.yml b/.github/workflows/squad-workflow-lint.yml index 2083ee801..5701d9731 100644 --- a/.github/workflows/squad-workflow-lint.yml +++ b/.github/workflows/squad-workflow-lint.yml @@ -18,6 +18,7 @@ on: - '.squad-templates/workflows/**' - 'templates/workflows/**' - 'workflows/**' + - 'scripts/check-workflow-input-interpolation.mjs' push: branches: [dev, main] paths: @@ -27,6 +28,7 @@ on: - '.squad-templates/workflows/**' - 'templates/workflows/**' - 'workflows/**' + - 'scripts/check-workflow-input-interpolation.mjs' permissions: contents: read @@ -135,3 +137,10 @@ jobs: run: | echo "--- Linting templates/workflows/ (root mirror) ---" actionlint templates/workflows/*.yml + + # actionlint only reads YAML. Agentic workflow prompts are Markdown, where a + # `github.event.inputs.x` reference written as prose is silently inert: the + # dispatched value never reaches the agent, the run still reports success, and + # the workflow no-ops with no visible signal. See #1740. + - name: Check agentic prompt input interpolation + run: node scripts/check-workflow-input-interpolation.mjs diff --git a/scripts/check-workflow-input-interpolation.mjs b/scripts/check-workflow-input-interpolation.mjs new file mode 100644 index 000000000..f33aaf48c --- /dev/null +++ b/scripts/check-workflow-input-interpolation.mjs @@ -0,0 +1,128 @@ +#!/usr/bin/env node +// check-workflow-input-interpolation.mjs -- Catch prose-only workflow_dispatch input +// references in agentic workflow prompt bodies. +// +// An agentic workflow prompt is Markdown that gets rendered into the agent's context. +// Writing `github.event.inputs.command` in backticks names the expression but never +// resolves it, so the agent sees a literal string instead of the dispatched value. +// The workflow then looks healthy from the Actions tab -- run-name and `if:` guards +// interpolate correctly in frontmatter -- while the agent silently no-ops for want of +// a value that was delivered but never rendered. +// +// Rule: inside a prompt body, every `github.event.inputs.*` reference must appear +// within a `${{ ... }}` interpolation. Two deliberate exemptions: +// +// * Frontmatter -- `run-name`, `if`, `concurrency` and friends are evaluated by +// Actions itself, not by the agent. +// * Other `github.event.*` paths (notably `comment.body` and `issue.body`) -- these +// carry untrusted user text. gh-aw supplies them to the agent through sanitized +// trigger context; interpolating them straight into the prompt would be a +// prompt-injection vector, so bare prose references are correct there. +// +// Exit 0 when clean, exit 1 with file:line details otherwise. +// Uses only Node.js built-ins (fs, path). + +import { readFileSync, readdirSync, existsSync } from 'node:fs'; +import { resolve, join, dirname, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = resolve(__dirname, '..'); + +const SCAN_DIRS = [ + 'workflows', + '.squad-templates/workflows', + 'templates/workflows', + 'packages/squad-cli/templates/workflows', + 'packages/squad-sdk/templates/workflows', +]; + +const INPUT_REF = /github\.event\.inputs\.[A-Za-z0-9_]+/g; +const INTERPOLATION = /\$\{\{[^}]*\}\}/g; + +/** Collect .md files from a directory tree, skipping nothing -- these trees are small. */ +function collectMarkdown(dir) { + const found = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + found.push(...collectMarkdown(full)); + } else if (entry.name.endsWith('.md')) { + found.push(full); + } + } + return found; +} + +/** + * Return the 0-based line index where the prompt body starts. + * A leading `---` fence opens YAML frontmatter; the body begins after its closing fence. + * Without frontmatter the whole file is body. + */ +function bodyStartLine(lines) { + if (lines[0]?.trim() !== '---') return 0; + for (let i = 1; i < lines.length; i++) { + if (lines[i].trim() === '---') return i + 1; + } + return 0; +} + +const violations = []; +let scannedFiles = 0; + +for (const relDir of SCAN_DIRS) { + const dir = join(REPO_ROOT, relDir); + if (!existsSync(dir)) continue; + + for (const file of collectMarkdown(dir)) { + scannedFiles++; + const lines = readFileSync(file, 'utf8').split(/\r?\n/); + const start = bodyStartLine(lines); + + for (let i = start; i < lines.length; i++) { + const line = lines[i]; + if (!line.includes('github.event.inputs.')) continue; + + // Blank out every interpolation, then see if any reference survives outside one. + const masked = line.replace(INTERPOLATION, (match) => ' '.repeat(match.length)); + const bare = masked.match(INPUT_REF); + if (!bare) continue; + + for (const ref of bare) { + violations.push({ + file: relative(REPO_ROOT, file).replace(/\\/g, '/'), + line: i + 1, + ref, + text: line.trim(), + }); + } + } + } +} + +if (violations.length === 0) { + console.log( + `Workflow input interpolation check passed: ${scannedFiles} prompt file(s) scanned, no bare github.event.inputs.* references.`, + ); + process.exit(0); +} + +console.error( + `Workflow input interpolation check FAILED: ${violations.length} bare github.event.inputs.* reference(s) in prompt bodies.\n`, +); +console.error( + 'These name an expression without resolving it, so the agent receives the literal text', +); +console.error('instead of the dispatched value -- and silently no-ops.\n'); + +for (const { file, line, ref, text } of violations) { + console.error(` ${file}:${line}`); + console.error(` reference: ${ref}`); + console.error(` line: ${text}\n`); +} + +console.error('To fix: wrap the reference in an interpolation, e.g.'); +console.error(' - **Dispatched command:** `${{ github.event.inputs.command }}`'); +console.error('If the prompt genuinely needs to discuss the input rather than its value,'); +console.error('describe it without the literal `github.event.inputs.` prefix.'); +process.exit(1); diff --git a/workflows/squad.md b/workflows/squad.md index b2c5453bd..8a239bd7d 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -132,14 +132,32 @@ When locating artifacts: ## Trigger Context -Read slash command from: `github.event.comment.body` (issue/PR comment), `github.event.issue.body` (issue body), or `github.event.inputs.command` (workflow_dispatch, default: `cast`). +The dispatch inputs for this run are interpolated below. Treat a non-empty value +as authoritative; an empty value means this run was not started by a workflow +dispatch. -- **Issue body:** `github.event.issue.body` — the full issue description -- **Issue comment:** `github.event.comment.body` — the full comment text -- **PR review comment:** `github.event.comment.body` — the full comment text -- **Workflow dispatch:** `github.event.inputs.command` — manual input (default: `cast`) -- **Manual issue target:** `github.event.inputs.issue_number` — issue number for - `/squad implement` runs started from the Actions tab +- **Dispatched command:** `${{ github.event.inputs.command }}` +- **Dispatched issue number:** `${{ github.event.inputs.issue_number }}` + +Resolve the slash command in this order: + +1. **Dispatched command** (above) — when non-empty, this run is a workflow + dispatch. Use this value as the command and skip the remaining sources. +2. **Issue comment / PR review comment:** `github.event.comment.body` — the full + comment text. +3. **Issue body:** `github.event.issue.body` — the full issue description. +4. Otherwise default to `cast`. + +Resolve the target issue in this order: + +1. **Dispatched issue number** (above) — when non-empty, this is the target + issue, including for merge-driven epic continuations. +2. The triggering issue or pull request number from the event payload. + +**Never emit `noop` when the dispatched command is non-empty.** A workflow +dispatch is always actionable: run the named mode against the dispatched issue +number. If the dispatched command is non-empty but the dispatched issue number +is empty, post a comment naming the missing input rather than exiting silently. The activation job already ran `squad init --preset default`, which produced a generic 5-agent team (lead, reviewer, devrel, security, docs) in `.squad/`. Cast @@ -483,10 +501,10 @@ to this mode so it can automatically refill the parent epic's available slots. ##### Step 1: Validate and Gather Context -1. Resolve the target issue number from `github.event.inputs.issue_number` for a - workflow dispatch, otherwise from the triggering issue. If invoked from a - pull request review comment, explain that `/squad implement` must be run from - the target issue. +1. Resolve the target issue number using the Trigger Context resolution order: + the interpolated dispatched issue number when non-empty, otherwise the + triggering issue. If invoked from a pull request review comment, explain that + `/squad implement` must be run from the target issue. 2. Read the target issue title, body, labels, state, and relevant comments. 3. Find open child issues using native GitHub sub-issue relationships. Also include open issues whose body contains a @@ -532,6 +550,18 @@ children with existing implementation pull requests, and any ready children deferred because all three slots are occupied. If no child is ready or no slot is available, post the status summary and do not dispatch a workflow. +**Always leave a visible next step.** Every Implement run against an epic ends +with a comment on that epic — never a silent exit. Cover each terminal case: + +- Children dispatched → name them and state how many children remain open. +- All remaining children blocked → name the blocking dependencies. +- All three slots occupied → name the in-flight pull requests. +- No open children left → state that the epic's implementation is complete. + +Never emit `noop` for an Implement run. `noop` is not reported as a comment, so +it strands the epic with no signal about what to do next — the exact failure +this procedure exists to prevent. + After each implementation pull request merges, this workflow runs again and fills newly available slots. Continue until the epic has no open children. `/squad implement` remains available as a manual recovery command. From 11fcbed781dd2e07f41caa70bab980ba65b4f2a2 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Mon, 17 Aug 2026 18:34:27 -0700 Subject: [PATCH 2/4] docs(scripts): record why the gate is scoped to inputs.* and scans skill 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 --- .../check-workflow-input-interpolation.mjs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/scripts/check-workflow-input-interpolation.mjs b/scripts/check-workflow-input-interpolation.mjs index f33aaf48c..d8de4171f 100644 --- a/scripts/check-workflow-input-interpolation.mjs +++ b/scripts/check-workflow-input-interpolation.mjs @@ -14,10 +14,14 @@ // // * Frontmatter -- `run-name`, `if`, `concurrency` and friends are evaluated by // Actions itself, not by the agent. -// * Other `github.event.*` paths (notably `comment.body` and `issue.body`) -- these -// carry untrusted user text. gh-aw supplies them to the agent through sanitized -// trigger context; interpolating them straight into the prompt would be a -// prompt-injection vector, so bare prose references are correct there. +// * Other `github.event.*` paths -- see the INPUT_REF comment below. +// +// This scans whole files, including `## skill:` blocks, and that matters: mode +// playbooks are now extracted into inline skills and restored in isolation, with no +// guarantee about what precedes them. A skill body that needs a dispatch input must +// interpolate it itself -- it cannot lean on `## Trigger Context` having resolved the +// value into scope. Scanning skill bodies turns that from a convention into a +// structural guarantee. // // Exit 0 when clean, exit 1 with file:line details otherwise. // Uses only Node.js built-ins (fs, path). @@ -37,6 +41,13 @@ const SCAN_DIRS = [ 'packages/squad-sdk/templates/workflows', ]; +// Scoped to `inputs.*` deliberately. Do NOT widen this to all `github.event.*`: +// `comment.body` and `issue.body` are attacker-controlled. gh-aw delivers them to the +// agent through sanitized trigger context, so interpolating them here would splice +// untrusted text straight into the prompt ahead of the sanitizer -- a prompt-injection +// vector. A bare prose reference to those paths is CORRECT and must not be "fixed". +// An earlier draft of this check flagged them; the gate was right and the tempting fix +// was wrong. const INPUT_REF = /github\.event\.inputs\.[A-Za-z0-9_]+/g; const INTERPOLATION = /\$\{\{[^}]*\}\}/g; From 885a15a42b93d98ab29edab8ce16a6ab213a4a0f Mon Sep 17 00:00:00 2001 From: brady gaster Date: Tue, 18 Aug 2026 01:13:04 -0700 Subject: [PATCH 3/4] fix(workflows): comment instead of silent noop on merge continuation 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 --- workflows/squad-implement-worker.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/workflows/squad-implement-worker.md b/workflows/squad-implement-worker.md index fb9e81272..9fe81ffeb 100644 --- a/workflows/squad-implement-worker.md +++ b/workflows/squad-implement-worker.md @@ -142,7 +142,8 @@ For a merged pull request: `squad/implement-{issue-number}-` head branch. 2. Read the child issue and resolve its parent epic using the native parent relationship, falling back to its `Parent: #N` body line. -3. If no parent epic exists, emit `noop` and stop. +3. If no parent epic exists, comment on the merged pull request saying its issue + is standalone and that no further work was queued, then stop. 4. Call the workflow-specific `squad` safe-output tool exactly once: ```json @@ -155,6 +156,18 @@ For a merged pull request: Never call the generic `dispatch_workflow` tool. Never edit files or create a pull request in this mode. Stop after the `squad` workflow is dispatched. +**Always leave a visible next step.** Every merge continuation ends with a +comment — never a silent exit. Cover both terminal cases: + +- Parent epic resolved → name the epic and state that its next children were + queued. +- No parent epic → state that the pull request's issue is standalone and that + nothing further was queued. + +Never emit `noop` for a merge continuation. `noop` is not reported as a comment, +so it strands a merged pull request with no signal about what happens next — the +exact failure this procedure exists to prevent. + The remaining instructions apply only to `workflow_dispatch`. ## Gather Context From 934ee8b1fb40941b55d95843c8243c3da0239bbb Mon Sep 17 00:00:00 2001 From: brady gaster Date: Tue, 18 Aug 2026 08:54:33 -0700 Subject: [PATCH 4/4] docs(scripts): note node:url in the built-ins comment 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 --- scripts/check-workflow-input-interpolation.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-workflow-input-interpolation.mjs b/scripts/check-workflow-input-interpolation.mjs index d8de4171f..1abd60846 100644 --- a/scripts/check-workflow-input-interpolation.mjs +++ b/scripts/check-workflow-input-interpolation.mjs @@ -24,7 +24,7 @@ // structural guarantee. // // Exit 0 when clean, exit 1 with file:line details otherwise. -// Uses only Node.js built-ins (fs, path). +// Uses only Node.js built-ins (fs, path, url). import { readFileSync, readdirSync, existsSync } from 'node:fs'; import { resolve, join, dirname, relative } from 'node:path';