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..1abd60846 --- /dev/null +++ b/scripts/check-workflow-input-interpolation.mjs @@ -0,0 +1,139 @@ +#!/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 -- 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, url). + +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', +]; + +// 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; + +/** 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-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 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.