Fix long-path planning lifecycle defects + guard empty dispatch probe (squad.md) - #1778
Conversation
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>
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>
🟡 Impact Analysis — PR #1778Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
squad-state (1 file)
tests (2 files)
This report is generated automatically for every PR. See #733 for details. |
|
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 2 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 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, +463 −62)
| File | +/− |
|---|---|
.squad/agents/procedures/history.md |
+14 −0 |
test/gh-aw-plan-lifecycle.test.ts |
+319 −0 |
test/gh-aw-quality.test.ts |
+8 −4 |
workflows/squad.md |
+122 −58 |
Total: +463 −62
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
There was a problem hiding this comment.
Pull request overview
This PR updates the workflows/squad.md workflow contract to repair long-path planning lifecycle defects (accept routing, validate ordering, and implement leaf-task dispatch), harden workflow_dispatch activation against empty dispatch probes, and strengthen research output requirements via a structural contract. It also adds/updates contract tests to enforce these behaviors going forward.
Changes:
- Add a mandatory workflow_dispatch activation guard to halt empty/invalid dispatch probes without creating junk issues.
- Fix long-path planning lifecycle behaviors (accept routing, validate-before-accept ordering, and leaf-task-only implement dispatch) and tighten Owner/Agent cast-name binding.
- Add new lifecycle contract tests and update quality tests to assert the new guard behavior.
Show a summary per file
| File | Description |
|---|---|
| workflows/squad.md | Updates workflow contract text to fix long-path lifecycle defects, add workflow_dispatch activation guard, and replace research length floor with a structural schema. |
| test/gh-aw-quality.test.ts | Updates assertions to require the new guarded-halt behavior (warning + no junk issue) for missing workflow_dispatch inputs. |
| test/gh-aw-plan-lifecycle.test.ts | Adds new contract tests for lifecycle ordering, accept routing, leaf-task dispatch, cast-name binding, and structural research requirements. |
| .squad/agents/procedures/history.md | Documents Procedures’ work completed for the associated issues and PR. |
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
| function skillBlock(markdown: string, name: string): string { | ||
| const start = markdown.indexOf(`## skill: \`${name}\``); | ||
| if (start === -1) throw new Error(`skill block "${name}" not found`); | ||
| const rest = markdown.slice(start + 1); | ||
| const nextIdx = rest.indexOf('\n## skill: `'); | ||
| return nextIdx === -1 ? markdown.slice(start) : rest.slice(0, nextIdx); | ||
| } |
…1787) The implement-worker refilled a freed dispatch slot by dispatching `/squad implement` against the completing task's *immediate parent epic*. In a three-level tree (root -> epics -> leaf tasks) that scopes the refill to one epic: when Epic A drains, the worker finds nothing, exits green, and Epic B's unstarted leaf tasks are never reached. Slots sit idle with no error surfaced -- the same silent-no-op signature as #1772. Walk the parent chain to the root issue and dispatch that instead. squad.md's implement mode already descends the full sub-tree recursively (#1778), so root-scoped dispatch covers every sibling epic without any new traversal code in the worker. Budgets are untouched: dispatch-workflow max stays at 2 and the 3-slot concurrency cap in squad.md's Epic Dispatch still governs selection. The bug was traversal scope, not budget, and raising max would widen blast radius on the empty-probe path closed in #1778. Leaf-only rule hardened, not regressed. Root-scoped refill descends past drained-but-still-open epics, which have no *open* sub-issues and so passed squad.md's previous leaf test -- reviving the #1758 defect 2 shape at the end of an epic's life. Leafness is now "no sub-issues at all" plus not labeled epic/initiative. Testing: test/gh-aw-implement-workflow.test.ts gains a traversal simulator rather than a wording assertion (#1784 showed prompt-text presence cannot prove prompt obedience). It parses the shipped dispatch payload, binds its issue_number placeholder to the traversal it names, and runs that traversal over a two-sibling-epic fixture using squad.md's own leaf rule and slot budget. Verified red against the pre-fix workflows: 3 failures, including "refill must scan from the root, not the drained parent epic: expected 110 to be 100". Closes #1779 Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
…rbids (#1789) Live experiment E3 showed /squad plan activate minting `squad:lead` x3 and `squad:devrel` on a fixture whose roster contains neither role. The fixture had the #1778 fix deployed and a valid roster, so the model had the rule and the data and disobeyed both. `devrel` could not have come from that roster — it was lifted verbatim out of the parenthetical that forbids it. Naming a forbidden value concretely makes it more likely to be emitted, not less: the token is salient, the negation is not. Rewrites all six binding sites in workflows/squad.md as positive requirements — what the value MUST be (a verbatim `Name` cell from the `## Members` table) and the `@copilot` fallback — with no enumerated counter-examples. Prohibition is now stated abstractly ("no other column supplies a valid owner"). The plan and implementation rules also instruct the model to write out the roster it read before assigning, so the binding is grounded rather than recalled. Also makes the roster real at the two sites that previously rubber-stamped it: `/squad plan validate` gains Check 10, a Critical-severity roster check whose sole source of truth is the `Name` column, and the activate skill gains a pre-flight gate that refuses to mint a `squad:{agent}` label from a non-roster value. Testing: #1778's detector asserted the prompt *contains* the rule, so it passed while the system was broken. That assertion is replaced with its falsifiable inverse — the binding blocks must contain no backticked code span naming a Role (derived from team.md's `Role` column, not hardcoded). Verified by restoring the pre-fix squad.md: 8 tests fail. gh-aw strict-compile gate passes. Closes #1784 Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6
Working as Procedures (Prompt Engineer). All changes are confined to
workflows/squad.mdplus its contract tests — nopackages/*/src/touched, so no changeset is required.These gate tomorrow's (2026-08-21) full-day E2E series; #1758 in particular gates the long-path scenario.
#1758 — long path repaired in three places
Every touched stage is in the "NEVER EXERCISED" bucket, so each fix is grounded in a readable source of truth rather than inference.
squad-plan-accept). Step 1 hard-failed "No plan found" on a missingplanartifact before the routing note could run. Rewrote it as "Find Plan and Route":program/implementationartifacts route to Accept Scope → Accept Implementation → Activate; "No plan found" only when none ofprogram/implementation/planexist.squad-implement-worker.md).workflows/shared/squad-planning-ontology.md:48-87) is the authoritative state machine and sequencesprogram → implementation → validate → accept scope → accept implementation → activate.squad.md'snext=hints had drifted; re-synced them to the ontology so validate precedes BOTH accept steps. Repair the long program path so it runs end to end #1758 ships fully, not partial.#1759 — Owner/Agent emit cast Names, not Role strings
Added an explicit Owner/Agent binding rule (resolve to the
Namecolumn of.squad/team.md, never a Role string) at every emission site (squad-plan Step 1/Step 3; squad-plan-implementation Step 2/3/4), and madesquad:{owner}label minting use the lowercased cast Name, forbiddingsquad:lead.#1756 — structural research contract replaces the ≥200-char floor
Replaced the length floor (which a whitespace blob or truncated error passes) with a structural contract: required sections (Evidence table, Goals, Non-goals, Load-bearing assumptions, Open decisions, Acceptance framing) +
Rntraceability IDs + one citation token per evidence row, enforced by the MANDATORY verify step. Structural half only — the "well-formatted bad plan should FAIL" taste judgment (#1757) stays deferred; it can't be asserted cheaply without a curated corpus.Defense-in-depth: guard empty
workflow_dispatchprobe (paired with #1772 / PR #1777)EECOM's PR #1777 raises the worker's
dispatch-workflowmaxso the real relay dispatch survives the LLM's empty probe — but the probe still reachessquad.mdwith empty inputs.squad.mdpreviously handled empty inputs by creating an issue, which is the exact junk-issue generator behind fixture issues #12 and #14. Replaced both issue-minting paths with a MANDATORY activation guard: an empty-command probe (or a command with no resolvableissue_number) emits a single::warning::log annotation and STOPS — no issue, no comment, no skill. The guard also holds if the LLM ever emits a third dispatch entry, which would defeat themaxbump alone.Tests
test/gh-aw-plan-lifecycle.test.ts(structural contract assertions, one criterion per component), including a role-string-leak detector for #1759 that parses a plan's Owner column againstteam.mdand flags Role strings (lead) while passing cast Names (Procedures). Added guard assertions and updated thegh-aw-qualityissue_number contract test to expect the guarded halt. Targeted suites: 113 passed / 13 skipped. Pre-existing unrelated Windows-only failures (scheduler, patch-esm-imports, packaging smoke) confirmed present without these changes.npm run buildpasses.Closes #1758
Closes #1759
Closes #1756
Refs #1772, #1777