From 526104b3151f7f783a6c8d7f7341de792f53eae7 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Mon, 24 Aug 2026 15:03:54 -0700 Subject: [PATCH] fix(gh-aw): bind squad:{agent} labels to each issue's own plan row Activation minted a certified-but-wrong label and then reported labels it never applied. Both survived every existing guard because the guards check membership, and these were failures of correspondence. Team Guard TG-2 certifies the vocabulary of names that may become a `squad:{agent}` label; validation Check 10 asserts every plan `Agent` is a member of it. Neither asks whether the label on task N is the agent the plan assigned to task N. A label can be simultaneously certified and wrong. Measured on octodemo/aspiregregator-squad-e2e (run 32778953402, TG-2 green, Check 10 green, 12/12 values certified): #1859 Task 6 is assigned to McManus; issue #17 was created with squad:kint, the owner of its parent epic. 11 of 12 bindings were correct, and the twelfth was invisible because `kint` is a valid roster name. #1860 The summary reported `squad:kint` on epic #6 and `squad:kint / squad:mcmanus` on epic #7. Both carry only `squad`. The multi-owner epic was also never recorded under `Non-roster agent values`, so a run that dropped two bindings read as clean. The cause was ambiguity, not disobedience. Both create-issue steps said `Labels: squad, squad:{agent}` without ever binding `{agent}` to a source -- unlike the `plan implement` path at L928, which qualifies it. Reading tasks grouped under an epic, inheriting the epic's agent is the natural resolution. - Task rule now names the task's own `Agent` cell, keyed by the plan row whose `#` matches, and prohibits epic inheritance and carry-forward by name. - Epic rule now derives from the epic's own tasks: one distinct agent mints that label, two or more resolve to bare `squad` plus a `Non-roster agent values` entry. An epic has no Agent column, so any other reading is a guess. - Label Pre-flight gains a correspondence step stating that membership across the run is not evidence, and a reporting step requiring the summary to name a label only after that issue's create-issue call returned carrying it. Tests assert prompt text, which is weaker than the executed-shell assertions in gh-aw-activate-roster-binding.test.ts; deterministic post-activation enforcement remains #1801. They are mutation-checked: restoring either rule to its previous wording fails two of them. Closes #1859 Closes #1860 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df --- ...gh-aw-agent-binding-correspondence.test.ts | 184 ++++++++++++++++++ workflows/squad.md | 17 +- 2 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 test/gh-aw-agent-binding-correspondence.test.ts diff --git a/test/gh-aw-agent-binding-correspondence.test.ts b/test/gh-aw-agent-binding-correspondence.test.ts new file mode 100644 index 000000000..798f1db9e --- /dev/null +++ b/test/gh-aw-agent-binding-correspondence.test.ts @@ -0,0 +1,184 @@ +/** + * Agent-binding correspondence in `workflows/squad.md` (#1859, #1860). + * + * Two defects survived every guard the workflow already had, because those guards + * verify **membership** and the defects were failures of **correspondence**. + * + * Team Guard TG-2 certifies a vocabulary — the set of names that may become a + * `squad:{agent}` label — and validation Check 10 asserts every `Agent` value is a + * member of it. Neither asks whether the label applied to task *N* is the agent the + * plan assigned to task *N*. **A label can be simultaneously certified and wrong.** + * + * Measured on `octodemo/aspiregregator-squad-e2e` (run 32778953402, 12/12 values + * certified, TG-2 green, Check 10 green): + * + * #1859 — the accepted plan assigns task 6 to `McManus`. Issue #17 was created + * with `squad:kint`, the owner of its parent epic (2.1). 11 of 12 correct, + * and the one failure was invisible to every membership check because + * `kint` is a perfectly valid roster name. + * + * #1860 — the activation summary reported `— squad:kint` on epic #6 and + * `squad:kint / squad:mcmanus` on epic #7. Both issues carry `[squad]` + * only. The summary also omitted the `Non-roster agent values` heading + * that the multi-owner epic required, so a run that silently dropped two + * bindings read as clean. + * + * The proximate cause was ambiguity, not disobedience: the epic and task rules both + * said `Labels: squad, squad:{agent}` and never bound `{agent}` to a source. Reading + * tasks grouped under an epic, inheriting the epic's agent is the natural resolution. + * + * These tests lock the disambiguation in place. They assert prompt text rather than + * behavior, which is a weaker instrument than the executed-shell assertions in + * `gh-aw-activate-roster-binding.test.ts` — deterministic post-activation enforcement + * is #1801. What they do buy is that the specific ambiguity which produced #1859 + * cannot silently return, and every failure names the rule that went missing. + */ + +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const SQUAD_WORKFLOW = join(process.cwd(), 'workflows', 'squad.md'); +const workflow = readFileSync(SQUAD_WORKFLOW, 'utf8').replace(/\r\n/g, '\n'); + +/** + * Whitespace-collapsed view for prose assertions. + * + * Rules that span two or three wrapped lines are otherwise unmatchable, and a test + * that fails when a maintainer rewraps a paragraph teaches people to delete it. + * Matching meaning, not layout. + */ +const prose = workflow.replace(/\s+/g, ' '); + +/** + * Return the single line declaring labels for a `create-issue` step. + * + * Anchored on the step heading so a rule added to the *other* step cannot satisfy + * an assertion about this one — the two rules are near-identical in wording and + * that is precisely how the epic's agent leaked onto tasks in the first place. + */ +function labelLineAfter(heading: string): string { + const at = workflow.indexOf(heading); + expect(at, `"${heading}" is missing from workflows/squad.md`).toBeGreaterThan(-1); + + const line = workflow + .slice(at) + .split('\n') + .find(l => /^-\s+Labels:/.test(l.trim())); + + expect(line, `no "- Labels:" line follows "${heading}" in workflows/squad.md`).toBeDefined(); + return line ?? ''; +} + +const EPIC_HEADING = '**2b. Create Epic Issues:**'; +const TASK_HEADING = '**2c. Create Task Issues:**'; + +describe('gh-aw: agent-binding correspondence (#1859, #1860)', () => { + it('finds both create-issue label rules, so the assertions below are not vacuous', () => { + // Without this, a renamed heading would make every test here pass by finding + // nothing to object to — the same empty-set failure mode that let #1822 through. + expect(labelLineAfter(EPIC_HEADING).length).toBeGreaterThan(0); + expect(labelLineAfter(TASK_HEADING).length).toBeGreaterThan(0); + }); + + it('binds a task label to that task\'s own Agent cell', () => { + const rule = labelLineAfter(TASK_HEADING); + + expect( + /own\s+`Agent`\s+cell/i.test(rule), + `The task label rule must name the task's OWN Agent cell as the source. ` + + `Left unbound it reads as "some agent", and the model resolves it from the ` + + `surrounding epic — which is #1859 exactly. Found:\n${rule}`, + ).toBe(true); + + expect( + /matches this task/i.test(rule), + `The task label rule must say which plan row to read — the one whose "#" ` + + `matches this task. Found:\n${rule}`, + ).toBe(true); + }); + + it('forbids inheriting the epic\'s agent or carrying the previous task\'s forward', () => { + const rule = labelLineAfter(TASK_HEADING); + + expect( + /never inherit the parent epic/i.test(rule), + `#1859's observed failure was inheritance from the parent epic. The rule must ` + + `prohibit it by name; a positive instruction alone was already present and ` + + `did not prevent it. Found:\n${rule}`, + ).toBe(true); + + expect( + /carry the previous task/i.test(rule), + `Sequential create-issue calls make carry-forward the other natural drift. ` + + `Found:\n${rule}`, + ).toBe(true); + }); + + it('derives an epic label from its own tasks and refuses to guess for multi-owner epics', () => { + const rule = labelLineAfter(EPIC_HEADING); + + expect( + /`Epic`\s+cell\s+names\s+this\s+epic/i.test(rule), + `An epic has no Agent column of its own; its label is only well-defined as the ` + + `agents of the tasks that name it. Epic 2.1 carried squad:kint while its only ` + + `task belonged to McManus. Found:\n${rule}`, + ).toBe(true); + + expect( + /two or more/i.test(rule) && /only\s+`squad`/i.test(rule), + `A multi-owner epic has no single correct label. Epic 1.2 spanned Kint and ` + + `McManus; the summary printed "squad:kint / squad:mcmanus", which is not a ` + + `label. The rule must resolve to bare "squad". Found:\n${rule}`, + ).toBe(true); + + expect( + /never\s+choose\s+one\s+of\s+several/i.test(rule), + `Picking an arbitrary owner is the failure that looks most like success — the ` + + `label is certified, present, and wrong. Found:\n${rule}`, + ).toBe(true); + }); + + it('states that membership is not correspondence', () => { + expect( + /simultaneously certified and wrong/i.test(prose), + `The roster-binding gate must say why TG-2 and Check 10 passing is not ` + + `evidence that the binding is right. Both were green for #1859. Without ` + + `this, a future reader reasonably concludes the gate already covers it.`, + ).toBe(true); + + expect( + /membership across the run is not evidence/i.test(prose), + `The gate must require per-issue verification. "Every value is a roster name" ` + + `is true of a run in which every label is on the wrong issue.`, + ).toBe(true); + }); + + it('requires the summary to report labels applied, not intended', () => { + expect( + /only after that issue's `create-issue` call returned successfully carrying it/i.test(prose), + `#1860: the summary attributed squad:kint to epic #6, which never received it. ` + + `The summary must be a record of what happened, not a restatement of the plan.`, + ).toBe(true); + + expect( + /Omitting the heading while omitting the label/i.test(prose), + `The two omissions compound: dropping a label AND its "Non-roster agent values" ` + + `entry produces a summary indistinguishable from a clean run. The gate must ` + + `name that combination, since each omission alone looks benign.`, + ).toBe(true); + }); + + it('keeps the Non-roster agent values heading required, not conditional on taste', () => { + const at = prose.indexOf('Non-roster agent values'); + expect(at, '`Non-roster agent values` heading rule is missing').toBeGreaterThan(-1); + + // The heading is referenced in more than one rule; require that at least one + // occurrence is stated as mandatory. Epic 1.2 was a legitimate multi-owner case + // that correctly received no agent label — and was never reported. + expect( + /`Non-roster agent values` heading is \*\*required\*\*/i.test(prose), + 'A "should" here yields silence in exactly the multi-owner case that needs it.', + ).toBe(true); + }); +}); diff --git a/workflows/squad.md b/workflows/squad.md index 75693e1b7..fd54a74b7 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -1327,6 +1327,19 @@ Count expected issues before starting. If total > 50: recommend phased activatio `squad:{agent}` label MUST be applied across the created issues. Zero labels on a plan with roster owners is a binding failure, not a pass — report it, don't proceed silently. +7. **Correspondence — the label must match *this* issue's own row.** Steps 4-6 certify the + *vocabulary*: that each value is a roster Name. They do not check that the right issue + received the right value. **A label can be simultaneously certified and wrong.** Before + each `create-issue` call, re-read the agent from that issue's own source — a task's own + `Agent` cell, an epic's derived task-set — and never from the row above it, the parent + epic, or the previous call. Verify per issue; membership across the run is not evidence. +8. **Report what was applied, not what was intended.** The activation summary may name a + `squad:{agent}` label for an issue only after that issue's `create-issue` call returned + successfully carrying it. Never state a label that was skipped, omitted, deferred, or + assumed. Whenever an `Agent` value did not become a label — multi-owner epic, uncertified + name, unavailable label — the `Non-roster agent values` heading is **required**, and must + name the value and the issue it applied to. Omitting the heading while omitting the label + reports a clean run that did not happen. Then verify labels `squad` and each roster-bound `squad:{agent}` exist. If missing, record them in the activation summary as a prerequisite gap (label creation requires `issues: write` + `create-label` safe-output — not configured in this workflow). Continue activation — `create-issue` will apply any existing labels normally; unavailable labels are omitted and reported, not silently applied. @@ -1346,7 +1359,7 @@ Root → Epics → Tasks. Phase-specific: filter to matching phase heading. **2b. Create Epic Issues:** `create-issue` per epic (dedup by title `[Epic] {name}` if already exists from prior phase). - Title: `[Epic] {name}` -- Labels: `squad` (0075ca), `squad:{agent}` (e4e669) +- Labels: `squad` (0075ca), `squad:{agent}` (e4e669) where `{agent}` is **derived from this epic's own tasks**: collect the `Agent` values of every implementation-plan row whose `Epic` cell names this epic. Exactly one distinct value → mint `squad:{that agent}`. Two or more → multi-owner epic: apply only `squad` and record it under `Non-roster agent values`. Never mint a single agent label for a multi-owner epic, and never choose one of several. - Body: outcome, stories, epic-level acceptance criteria, context (parent, initiative, milestone, deps) - Parent: sub-issue of root intent issue - Milestone: assigned @@ -1360,7 +1373,7 @@ Root → Epics → Tasks. Phase-specific: filter to matching phase heading. > **DO NOT** compose or buffer multiple task bodies before making calls. One compose → one call → one verify, repeated per task. - Title: task title -- Labels: `squad` (0075ca), `squad:{agent}` (e4e669). No `size:*` labels unless policy says so. +- Labels: `squad` (0075ca), `squad:{agent}` (e4e669) where `{agent}` is **this task's own `Agent` cell**, lowercased — read from the implementation-plan row whose `#` matches this task. Never inherit the parent epic's agent, and never carry the previous task's value forward: re-read the `Agent` cell for every task, because consecutive tasks under one epic routinely have different agents. No `size:*` labels unless policy says so. - Body: one sentence describing scope; 1-2 acceptance criteria; one compact context line (parent epic, size, deps) - Parent: sub-issue of EPIC (not root) - Milestone: same as parent epic