fix(dispatch): raise max to 2 and add schema static gate (#1772) - #1777
Merged
Conversation
- workflows/squad-implement-worker.md: raise dispatch-workflow max from 1 to 2
so the real dispatch survives an LLM probe. With max:1, the empty probe consumed
the only slot and the real payload was silently discarded (confirmed 3 live runs
in aspiregregator-squad-e2e). Raising to 2 gives the real dispatch a second slot.
- scripts/check-workflow-input-interpolation.mjs: extend static gate with
checkDispatchWorkflowSchemas() that validates every JSON code block adjacent to a
dispatch_workflow reference. Catches: missing workflow_name (failure shape 3 from
run 32316227601), missing inputs object (top-level keys are silently dropped by
gh-aw), missing inputs.issue_number, and top-level command/issue_number.
Supports SQUAD_GATE_SCAN_OVERRIDE env var for fixture-based testing.
- test/gh-aw-quality.test.ts: add 6 tests
- 'worker dispatch-workflow max is at least 2': fails against old max:1 state,
passes after fix. This is the structural test the prompt-wording approach lacked.
- 4 gate failure tests: missing workflow_name, top-level inputs, missing
issue_number, valid schema passes.
- Regression guard: gate passes against current workflow files.
Closes #1772. Supersedes failed prompt-only approach in #1766.
Coordination needed: Procedures to add squad.md empty-command guard (separate PR) —
when workflow_dispatch fires with no command/issue_number, squad should detect and
report failure immediately rather than running in undefined mode.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
🟡 Impact Analysis — PR #1777Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
scripts (1 file)
squad-state (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | 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 merge conflicts | No merge conflicts |
| ❌ | Copilot threads resolved | 1 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (4 files, +386 −7)
| File | +/− |
|---|---|
.squad/agents/eecom/history.md |
+10 −0 |
scripts/check-workflow-input-interpolation.mjs |
+164 −6 |
test/gh-aw-quality.test.ts |
+211 −0 |
workflows/squad-implement-worker.md |
+1 −1 |
Total: +386 −7
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the merge-continuation relay by preventing dispatch_workflow schema probes from silently consuming the only safe-output slot, and adds a CI/static validation gate to catch malformed dispatch_workflow JSON examples in workflow prompts.
Changes:
- Raise
dispatch-workflow.maxfrom 1 → 2 inworkflows/squad-implement-worker.mdso a probe can’t discard the real dispatch payload. - Add a static gate in
scripts/check-workflow-input-interpolation.mjsto validatedispatch_workflowJSON example blocks adjacent todispatch_workflowreferences. - Add/extend Vitest coverage to structurally assert the
max >= 2contract and validate the new schema gate behavior (including fixture-based failure cases).
Show a summary per file
| File | Description |
|---|---|
| workflows/squad-implement-worker.md | Raises dispatch-workflow.max to 2 to avoid first-wins probe suppression. |
| test/gh-aw-quality.test.ts | Adds structural test for max >= 2 and a suite covering the new dispatch schema gate. |
| scripts/check-workflow-input-interpolation.mjs | Implements checkDispatchWorkflowSchemas() and adds scan override support for fixtures. |
| .squad/agents/eecom/history.md | Records the investigation summary and fix details for #1772. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+1134
to
+1152
| function runGate(scanDir: string): { exitCode: number; stderr: string; stdout: string } { | ||
| // The script resolves SCAN_DIRS relative to repo root via import.meta.url. | ||
| // We invoke it with a patched environment variable so the fixture directory is | ||
| // scanned instead of the real workflows directory. | ||
| const result = spawnSync( | ||
| process.execPath, | ||
| [scriptPath], | ||
| { | ||
| env: { ...process.env, SQUAD_GATE_SCAN_OVERRIDE: scanDir }, | ||
| encoding: 'utf8', | ||
| cwd: process.cwd(), | ||
| } | ||
| ); | ||
| return { | ||
| exitCode: result.status ?? 1, | ||
| stderr: result.stderr ?? '', | ||
| stdout: result.stdout ?? '', | ||
| }; | ||
| } |
bradygaster
added a commit
that referenced
this pull request
Aug 20, 2026
… (squad.md) (#1778) * Fix long-path planning lifecycle defects in squad.md Working as Procedures (Prompt Engineer) Repairs three defects in workflows/squad.md that gate the long-path end-to-end scenario, all grounded in readable sources of truth rather than inference: - #1758.1: squad-plan-accept Step 1 rewritten as "Find Plan and Route" so program/implementation artifacts route to Accept Scope -> Accept Implementation -> Activate before the "No plan found" hard-fail. The routing note is no longer dead code. - #1758.2: Implement descends the sub-issue hierarchy recursively and dispatches only leaf tasks; intermediate epics are never handed to a worker. Preserves the 3-slot dispatch cap. - #1758.3: next-hints re-synced to the authoritative ontology (shared/squad-planning-ontology.md) so validate precedes BOTH accept steps: program -> implementation -> validate -> accept scope -> accept implementation -> activate. - #1759: Owner/Agent fields bind to the team.md Name column (never a Role string) at every emission site; squad:{owner} labels mint from the lowercased cast Name, forbidding squad:lead. - #1756: research artifact's >=200-char floor replaced with a structural contract (Evidence table, Goals, Non-goals, Load-bearing assumptions, Open decisions, Acceptance framing + Rn IDs + one citation token per evidence row), enforced by the MANDATORY verify step. Structural half only; the taste-judgment half (#1757) stays deferred. Adds test/gh-aw-plan-lifecycle.test.ts (18 assertions), including a role-string-leak detector that catches a Role string reaching an Owner column. No packages/*/src touched, so no changeset required. Closes #1758 Closes #1759 Closes #1756 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Guard empty workflow_dispatch probe against junk-issue creation Working as Procedures (Prompt Engineer) Defense-in-depth paired with EECOM's dispatch-workflow max fix (PR #1777). The worker relay fires an empty workflow_dispatch probe before its real dispatch; PR #1777 raises the worker's dispatch-workflow max so the real dispatch survives, but the probe still arrives at squad.md with empty inputs. squad.md previously handled empty inputs by CREATING an issue (titled "Squad workflow dispatch missing command/issue_number") — that is the exact junk-issue generator behind fixture issues #12 and #14, which destroys signal during the E2E series. Replace both issue-minting paths with a MANDATORY activation guard at the top of the workflow_dispatch path: - Empty/missing command on workflow_dispatch: emit a single ::warning:: log annotation and STOP — no issue, no comment, no skill. - Non-empty command naming an issue-bound mode but no issue_number: emit a ::warning:: annotation and STOP — no issue. The guard makes the surviving probe harmless and visible (a run-log annotation) instead of silently minting junk. It also holds if the LLM ever emits a third dispatch entry, which would defeat the max bump alone. Adds guard assertions to gh-aw-plan-lifecycle.test.ts and updates the gh-aw-quality issue_number contract test to expect the guarded halt instead of the removed issue-creation behavior. No packages/*/src touched. Refs #1772, PR #1777 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bradygaster
pushed a commit
that referenced
this pull request
Aug 23, 2026
FIDO's BLOCKING review on #1832: the /squad parser hardening was tested in isolation and broke its callers. workflow_dispatch delivers a BARE command -- squad-implement-worker.md relays {"command": "implement"}, and squad.md's own input schema documents cast/implement/connect org/repo, none carrying a /squad prefix. PC-1 returned NO_COMMAND for every one of those, routing a structurally valid run into PC-3 and hard-failing it. Manual dispatch and the autonomous implement relay worked before the hardening and did not after. Add Step PC-0, which normalizes the dispatch input before PC-1 sees it: trim, leave an existing /squad prefix alone (idempotent), otherwise prepend one, and emit EMPTY_DISPATCH for empty input so the activation guard halts it silently instead of PC-3 commenting and failing (PR #1777, junk issues #12/#14). PC-1 is deliberately NOT loosened. The paths are asymmetric: on the comment path a missing token IS the error condition, so loosening would reopen #1824. A regression test guards that wrong fix; mutation proves the guard fires. Also resolve the text contradiction that made the parser nondeterministic -- one passage said a dispatched value skips parsing, another marked PC-1 MANDATORY over it. Choosing a source now never skips parsing. Fix the greedy sub(/^.*\/squad/,"") that stripped through the LAST token on a line, resolving "/squad cast, then /squad status" to `status` -- a different mode than requested, silently. Use match() with substr($0, RSTART+RLENGTH). Record PC-3's "exit non-zero" as a known limitation: it is a prompt directive, not enforced code. Steps 1-3 are load-bearing because their output survives the run whatever exit status the agent chooses. Paste RETRO's shell input security contract as normative parser requirements (env: channel, four forbidden anti-patterns, six per-hop rules). Hop 1 -- compiler-channel compliance -- is UNMEASURED here: this repo ships no compiled gh-aw output. The verification gate is tracked in #1834 and explicitly not built in this PR; the text says so, so a declared requirement cannot be mistaken for an enforced one. Tests: 15 -> 27 in gh-aw-command-parse. Three mutations each turn the suite red AND name the offending input: PC-0 pass-through, greedy sub() restored, PC-1 loosened. All four gh-aw suites green, including strict gh aw compile and the prompt budget guard. Refs #1824, #1834 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bradygaster
added a commit
that referenced
this pull request
Aug 23, 2026
* fix(gh-aw): fail loudly when /squad parses no command `/squad cast` silently no-opped with a green check unless the command started the issue body. `Parse Command` said "strip /squad prefix" — a position-0 assumption — and had no failure branch, so a run that cast nothing was byte-identical to one that worked. That is the first-run path for every new user. Parse Command is now three steps. PC-1 scans the whole body for the first /squad token and prints either the argument text or the NO_COMMAND sentinel. PC-2 routes NO_COMMAND to PC-3 and explicitly forbids falling back to `cast`. PC-3 is mandatory: it greps the body, emits ::error:: with the offending text quoted verbatim, comments on the issue, and exits non-zero. The parser is an LLM prompt, so the parse is declared as concrete shell commands in the markdown and the test extracts and executes those exact commands against real issue bodies — the declared contract and its observed behavior are independent sources. Two mutations were run against the fix. Reintroducing position-0 anchoring was caught by only 1 of 14 assertions: awk `^` anchors per record, so a line-anchored parser still handles a command on its own line after prose, and the headline "prose, blank line, then command" case passed under the reintroduced bug. Indented and mid-sentence cases were added to close that blind spot; the mutation then failed 2 tests naming the input. Replacing PC-3's diagnostic with a generic message — leaving every status signal intact, the shape that slipped through on #1793 — was caught by 3 assertions, because they require the diagnostic to contain the offending text rather than merely to exist. The suite resolves Git Bash rather than skipping on Windows, and asserts a shell was found, so it cannot report green for assertions that never ran. Closes #1824 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(gh-aw): normalize bare workflow_dispatch commands before PC-1 FIDO's BLOCKING review on #1832: the /squad parser hardening was tested in isolation and broke its callers. workflow_dispatch delivers a BARE command -- squad-implement-worker.md relays {"command": "implement"}, and squad.md's own input schema documents cast/implement/connect org/repo, none carrying a /squad prefix. PC-1 returned NO_COMMAND for every one of those, routing a structurally valid run into PC-3 and hard-failing it. Manual dispatch and the autonomous implement relay worked before the hardening and did not after. Add Step PC-0, which normalizes the dispatch input before PC-1 sees it: trim, leave an existing /squad prefix alone (idempotent), otherwise prepend one, and emit EMPTY_DISPATCH for empty input so the activation guard halts it silently instead of PC-3 commenting and failing (PR #1777, junk issues #12/#14). PC-1 is deliberately NOT loosened. The paths are asymmetric: on the comment path a missing token IS the error condition, so loosening would reopen #1824. A regression test guards that wrong fix; mutation proves the guard fires. Also resolve the text contradiction that made the parser nondeterministic -- one passage said a dispatched value skips parsing, another marked PC-1 MANDATORY over it. Choosing a source now never skips parsing. Fix the greedy sub(/^.*\/squad/,"") that stripped through the LAST token on a line, resolving "/squad cast, then /squad status" to `status` -- a different mode than requested, silently. Use match() with substr($0, RSTART+RLENGTH). Record PC-3's "exit non-zero" as a known limitation: it is a prompt directive, not enforced code. Steps 1-3 are load-bearing because their output survives the run whatever exit status the agent chooses. Paste RETRO's shell input security contract as normative parser requirements (env: channel, four forbidden anti-patterns, six per-hop rules). Hop 1 -- compiler-channel compliance -- is UNMEASURED here: this repo ships no compiled gh-aw output. The verification gate is tracked in #1834 and explicitly not built in this PR; the text says so, so a declared requirement cannot be mistaken for an enforced one. Tests: 15 -> 27 in gh-aw-command-parse. Three mutations each turn the suite red AND name the offending input: PC-0 pass-through, greedy sub() restored, PC-1 loosened. All four gh-aw suites green, including strict gh aw compile and the prompt budget guard. Refs #1824, #1834 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This was referenced Aug 23, 2026
bradygaster
added a commit
that referenced
this pull request
Aug 23, 2026
Closes #1835 PC-0 normalized the dispatched command with an awk program gated on NR==1, so a value whose first character is a newline put an EMPTY string on the scanned record: PC0($'\nimplement') returned EMPTY_DISPATCH. That routes to the activation guard, which halts with a ::warning:: and no comment by design (PR #1777, junk issues #12/#14) -- so a structurally valid implement dispatch would halt silently instead of running. Silent loss of a valid command is the same defect class #1824 and #1832 exist to close. Scan the first non-empty line instead: skip blank records, take the first one carrying content, and fall through to EMPTY_DISPATCH via END only when every record was empty or whitespace-only. Genuinely empty input still reaches the silent activation-guard halt, so #1777 stays closed. The one-liner is written without inter-statement spaces deliberately. workflows/squad.md sits ~37 bytes above the 5 KB floor of the 100 KB prompt budget asserted by gh-aw-quality's "reports combined bytes and headroom", so the readable spacing and an explanatory paragraph both overran the gate. The behavior is documented in the tests instead. That headroom is now ~65 bytes and is a blocker for the next change to this file; it needs its own issue. Tests: 27 -> 36 in gh-aw-command-parse. Reverting the awk to NR==1 turns four of the new cases red, and each failure message names the offending input, so a truncating parser cannot pass them. The whitespace-only cases ('', ' ', '\n', '\n\n', ' \n \t \n ') pin EMPTY_DISPATCH so the fix cannot overshoot, and the load-bearing PC-1 cases are untouched -- a bare token still fails on the comment path. Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bradygaster
deleted the
bradygaster-eecom-fix-dispatch-probe-gate-1772
branch
September 9, 2026 19:44
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The merge-continuation relay silently fails. The implement worker emits an empty
dispatch_workflowprobe first, which consumes the only slot (max: 1). The real dispatch is silently discarded. Squad fires with no inputs and creates junk issues.Confirmed three times in
bradygaster/aspiregregator-squad-e2e:32324473906→ Squad run32324738114→ junk issue Add @copilot coding agent as a Squad member type #1232394811753→ Squad run32395290857→ junk issue Add Ralph — built-in work monitor squad member #1432316227601→ wrong dispatch schema (noinputswrapper, noworkflow_name) → Squad ran wrong skillPrior fix #1766 added prompt wording ("NEVER call dispatch_workflow with empty/placeholder arguments"). The wording was present in the live runs and the probe happened anyway. Prompt text is not a reliable gate for LLM runtime behavior.
Fix
1.
workflows/squad-implement-worker.md— raisedispatch-workflow: maxfrom 1 to 2With
max: 2, the real dispatch gets a second slot even when the LLM probes first. The probe fires too (dispatching squad with no inputs), but the real dispatch now succeeds — the relay is no longer silently broken.2.
scripts/check-workflow-input-interpolation.mjs— dispatch schema static gateAdded
checkDispatchWorkflowSchemas()that validates every JSON code block adjacent to adispatch_workflowreference. Catches:workflow_name— confirmed failure shape in run32316227601inputsobject — top-levelcommand/issue_numberare silently dropped by gh-awinputspresent but missingissue_numbercommand/issue_numberkeysThe gate runs in CI at
.github/workflows/squad-workflow-lint.yml:146on every PR that touches workflows.Tests Added (6 new)
worker dispatch-workflow max is at least 2workflow_namecommandinstead ofinputsinputs.issue_numberThe
max >= 2test is the key structural test that the wording-only approach (#1766) lacked: prompt text can be present and the live run still broken; this test directly asserts the frontmatter value that governs runtime behavior. It fails against the oldmax: 1state and passes after the fix.What This Fix Does NOT Cover
The LLM probe behavior is not prevented — that requires gh-aw to validate dispatch payloads before counting them toward
max. The fix reduces the blast radius and catches authoring mistakes.Coordination Required — squad.md empty-command guard
When
workflow_dispatchfires with nocommandand noissue_number(from the probe), squad runs in an undefined mode and creates junk issues because both inputs arerequired: false. Procedures should add an explicit guard at the top of squad.md that detects emptycommand/issue_numberonworkflow_dispatch, logs a visible failure comment, and stops. This is a separate PR and should not block this fix.Supersedes failed prompt-only approach in #1766.
Closes #1772