From 153730f56675b1fd1687215dfc814d085276081d Mon Sep 17 00:00:00 2001 From: brady gaster Date: Fri, 21 Aug 2026 01:15:35 -0700 Subject: [PATCH] fix(squad.md): stop seeding Owner/Agent binding with the tokens it forbids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6 --- test/gh-aw-plan-lifecycle.test.ts | 114 +++++++++++++++++++++++++++--- workflows/squad.md | 87 ++++++++++++++++++----- 2 files changed, 176 insertions(+), 25 deletions(-) diff --git a/test/gh-aw-plan-lifecycle.test.ts b/test/gh-aw-plan-lifecycle.test.ts index 8366e9870..084d02b1f 100644 --- a/test/gh-aw-plan-lifecycle.test.ts +++ b/test/gh-aw-plan-lifecycle.test.ts @@ -72,9 +72,44 @@ function isRoleStringLeak(ownerCell: string): boolean { } // --------------------------------------------------------------------------- -// #1759 — Owner/Agent columns must resolve to cast Names, never Role strings +// #1759 / #1784 — Owner/Agent columns must resolve to cast Names, and the +// binding sites must not seed the model with the tokens they forbid. // --------------------------------------------------------------------------- +/** + * #1784: the binding rules used to enumerate their own counter-examples — + * `` never a Role string (`Lead`, `DevRel`) `` — and live run E3 showed the model + * emitting `lead`/`devrel` verbatim, including `devrel` for a roster that has no + * DevRel role. The concrete token is salient; the negation is not. + * + * A backticked code span is the most copyable form a token can take in the + * prompt, so the binding blocks must contain none that name a Role rather than a + * Name. Roles come from team.md's `Role` column, so this stays honest as the + * roster changes; the extra literals are the values E3 actually leaked. + */ +const FORBIDDEN_TOKENS = [ + ...castRoles().map(r => r.toLowerCase()), + 'devrel', + 'reviewer', +]; + +/** Backticked code spans in `text`, lowercased. */ +function codeSpans(text: string): string[] { + return [...text.matchAll(/`([^`\n]+)`/g)].map(m => m[1].trim().toLowerCase()); +} + +/** + * Code spans in `text` that name a Role (or a known leaked value), in bare or + * `squad:`-prefixed form. These are the tokens #1784 proved get copied out. + */ +function leakedRoleTokens(text: string): string[] { + return codeSpans(text).filter(span => { + const bare = span.startsWith('squad:') ? span.slice('squad:'.length) : span; + if (NAMES_LC.has(bare)) return false; // a cast Name is a legitimate value + return FORBIDDEN_TOKENS.includes(bare); + }); +} + describe('#1759: Owner/Agent bind to the cast Name column', () => { it('team.md exposes distinct Name and Role columns to bind against', () => { expect(NAMES).toContain('Procedures'); @@ -114,23 +149,86 @@ describe('#1759: Owner/Agent bind to the cast Name column', () => { const block = skillBlock(squad, 'squad-plan'); expect(block).toMatch(/Owner\/Agent binding rule/i); expect(block).toContain('`Name` column'); - // Explicitly forbids the leaking values named in #1759. - expect(block).toMatch(/never a Role string/i); - expect(block).toMatch(/`lead`, `devrel`, `reviewer`/); + expect(block).toContain('@copilot'); }); it('squad-plan-accept mints squad:{owner} from the cast Name, not a role', () => { const block = skillBlock(squad, 'squad-plan-accept'); - expect(block).toContain('cast **Name** lowercased'); - expect(block).toContain('squad:flight'); - expect(block).toMatch(/never mint a role-derived label such as `squad:lead`/); + expect(block).toContain('`Name` column'); + expect(block).toContain('`squad:{owner}`'); }); it('squad-plan-implementation binds the Agent column to the cast Name', () => { const block = skillBlock(squad, 'squad-plan-implementation'); expect(block).toMatch(/Agent binding rule/i); expect(block).toContain('`Name` column'); - expect(block).toMatch(/agent validity \(every `Agent` resolves to a `Name` row/); + expect(block).toMatch(/appears verbatim in the `Name` column/); + }); +}); + +describe('#1784: binding sites never name the tokens they forbid', () => { + it('the leak detector recognizes a prohibition that enumerates Role tokens', () => { + // The pre-fix text from workflows/squad.md:913 — this MUST be flagged, or + // the assertions below prove nothing. + const preFix = + 'every `Agent` value MUST be a cast **Name** from the `Name` column of ' + + '`.squad/team.md`, never a Role string (`Lead`, `DevRel`) or lowercased ' + + 'role (`lead`, `reviewer`).'; + expect(leakedRoleTokens(preFix)).toEqual( + expect.arrayContaining(['lead', 'devrel', 'reviewer']) + ); + + // The pre-fix label text from workflows/squad.md:730. + const preFixLabel = + 'never mint a role-derived label such as `squad:lead` or `squad:reviewer`.'; + expect(leakedRoleTokens(preFixLabel)).toEqual( + expect.arrayContaining(['squad:lead', 'squad:reviewer']) + ); + + // A positive-only rule carries no forbidden token. + const postFix = + 'Every `Agent` value MUST appear verbatim in the `Name` column of the ' + + '`## Members` table in `.squad/team.md`, or be `@copilot`.'; + expect(leakedRoleTokens(postFix)).toEqual([]); + }); + + for (const skill of [ + 'squad-plan', + 'squad-plan-accept', + 'squad-plan-implementation', + 'squad-plan-validate', + 'squad-plan-activate', + ]) { + it(`${skill} states the binding positively, without naming a Role token`, () => { + expect(leakedRoleTokens(skillBlock(squad, skill))).toEqual([]); + }); + } +}); + +// --------------------------------------------------------------------------- +// #1784 — /squad plan validate must fail a non-roster Owner/Agent value +// --------------------------------------------------------------------------- + +describe('#1784: squad-plan-validate treats the Name column as sole truth', () => { + const block = skillBlock(squad, 'squad-plan-validate'); + + it('declares a roster-binding check in the checks table', () => { + expect(block).toMatch(/Non-roster owner\/agent/i); + }); + + it('scores a non-roster value as Critical, not a warning', () => { + const severity = block.match(/^Severity:.*$/m)?.[0] ?? ''; + // Check 10 is the roster check; it must sit on the Critical side. + expect(severity).toMatch(/Critical[^.]*\b10\b/); + expect(severity).not.toMatch(/Warning[^.]*\b10\b/); + }); + + it('binds the check to the Name column and forbids attesting validity otherwise', () => { + const check = block.match(/###### Check 10[\s\S]*?(?=\n##### )/)?.[0] ?? ''; + expect(check).not.toEqual(''); + expect(check).toContain('`Name` column'); + expect(check).toMatch(/Critical/); + expect(check).toMatch(/Never report a value as a\s+valid roster name unless/); }); }); diff --git a/workflows/squad.md b/workflows/squad.md index d281513c4..0daed6a2a 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -661,15 +661,27 @@ Decompose issue into sub-issues as a comment. Does NOT create issues. Works on o 1. Read issue body (the epic/brief). 2. Find latest `research` artifact comment for this issue. If found, use as primary context. If not, do lightweight repo analysis. -3. Read `.squad/team.md` if it exists. **Owner/Agent binding rule:** every - `Owner` and `Agent` value MUST be a cast **Name** taken verbatim from the - `## Members` table's `Name` column of `.squad/team.md` (e.g. `Flight`, - `Procedures`, `EECOM`) — never a Role string (`Lead`, `Prompt Engineer`, - `DevRel`) and never a lowercased role (`lead`, `devrel`, `reviewer`). Map each - work item's domain to an owner via `.squad/routing.md`, then resolve that - owner to its exact `Name`. If no cast member fits, use `@copilot`. This - binding governs every `Owner`/`Agent` column and every `squad:{owner}` label - emitted downstream. +3. Read `.squad/team.md` if it exists. **Owner/Agent binding rule:** + + a. Locate the `## Members` table in **this repository's** `.squad/team.md`. + Read its `Name` column and build the **allowed-owner set**: every `Name` + cell value copied verbatim, plus `@copilot`. + b. Before assigning any owner, write the allowed-owner set out explicitly in + your working notes, so the binding is grounded in the roster you actually + read rather than recalled. Use only values read from this repository's + `.squad/team.md` — never a name carried over from another roster, an + example, or a previous task. + c. Every `Owner` and `Agent` value you emit MUST be a member of the + allowed-owner set, character-for-character as it appears in the `Name` + column. The `Name` column is the sole source of these values; no other + column of `.squad/team.md` — the `Role` column included — supplies a valid + owner, in any casing. + d. Map each work item's domain to a member via `.squad/routing.md`, then + resolve that member to its exact `Name` cell value. If no cast member fits, + use `@copilot`. + + This binding governs every `Owner`/`Agent` column and every `squad:{owner}` + label emitted downstream. 4. Text after `/squad plan` = planning guidance. ##### Step 2: Decompose @@ -682,7 +694,7 @@ Break into discrete work items. **Minimum 3 items** unless genuinely atomic (exp Structure: `## 📋 Squad Plan — {Title}` → reference line → Phase tables (# | Title | Owner | Size | Depends On) → Details per item (Scope, Acceptance criteria, Notes) → Dependency Graph → Execution Notes → Next Steps (`/squad plan accept`, `/squad plan accept phase 1`, `/squad plan revise`, `/squad plan`). -The `Owner` column MUST be a cast **Name** per the Owner/Agent binding rule (Step 1) — a value from the `Name` column of `.squad/team.md`, never a Role string. +Every `Owner` cell MUST be a member of the allowed-owner set built in Step 1 — a verbatim value from the `Name` column of the `## Members` table in `.squad/team.md`, or `@copilot`. Re-check each `Owner` cell against that column before posting; a value that does not appear there is invalid and must be re-resolved. Do NOT create issues. @@ -727,7 +739,7 @@ If plan has phases: Root → Phase issues → Task issues. Flat plan: tasks dire For each work item, `create-issue`: - Title: work item title -- Labels: `squad` (color `9B8FCC`), `squad:{owner}` (color `9B8FCC`), where `{owner}` is the work item's cast **Name** lowercased (e.g. Owner `Flight` → `squad:flight`). It MUST resolve to a `Name` row in `.squad/team.md`; never mint a role-derived label such as `squad:lead` or `squad:reviewer`. +- Labels: `squad` (color `9B8FCC`), plus `squad:{owner}` (color `9B8FCC`), where `{owner}` is the work item's `Owner` value lowercased. Before minting each `squad:{owner}` label, confirm the `Owner` value appears verbatim in the `Name` column of the `## Members` table in `.squad/team.md`. The `Name` column is the only permitted source for this label — no other column of `.squad/team.md` supplies one. If the `Owner` value is not present in that column, do not mint the label: re-resolve the owner against the `Name` column, or fall back to `@copilot` and apply only the `squad` label. - Body: scope, acceptance criteria, context (parent, phase, size, depends on, owner), notes, footer - Parent: phase issue (hierarchical) or root (flat) - Size: set Project field if available, else body `**Size:**` line @@ -910,13 +922,13 @@ Search in order: `scope-accepted` artifact (use as authoritative) → `program` Per task specify: Title, Scope (files/modules/APIs), Acceptance criteria, Size (XS <1h, S 1-3h, M 3-8h, L 1-2d; max per policy default L), Dependencies (task numbers), Agent, Rollout notes. -**Agent binding rule:** every `Agent` value MUST be a cast **Name** from the `Name` column of `.squad/team.md` (resolved via `.squad/routing.md`), never a Role string (`Lead`, `DevRel`) or lowercased role (`lead`, `reviewer`). If no cast member fits, use `@copilot`. +**Agent binding rule:** before assigning any agent, read the `## Members` table in **this repository's** `.squad/team.md` and list its `Name` column values verbatim in your working notes. That list, plus `@copilot`, is the complete set of permitted `Agent` values — ground your assignments in the roster you just read rather than recalling one. Every `Agent` value MUST be one of those values, character-for-character. Resolve each task's domain to a member via `.squad/routing.md`, then emit that member's exact `Name` cell value. The `Name` column is the sole source of `Agent` values; no other column of `.squad/team.md` — the `Role` column included — supplies a valid one, in any casing. If no cast member fits, use `@copilot`. Rules: no task > max_task_size. DAG only. Every task traces to program item. Every epic has ≥1 task. Vertical slices. Group into phases by dependency order (Phase 1 = no deps). ##### Step 3: Validate Structure -Check: sizes ≤ L, no cycles, traceability, coverage, agent validity (every `Agent` resolves to a `Name` row in `.squad/team.md`, never a Role string). Fix before posting. +Check: sizes ≤ L, no cycles, traceability, coverage, agent validity (every `Agent` value appears verbatim in the `Name` column of the `## Members` table in `.squad/team.md`, or is `@copilot`). Fix before posting. ##### Step 4: Post Implementation Plan @@ -924,7 +936,7 @@ Check: sizes ≤ L, no cycles, traceability, coverage, agent validity (every `Ag Structure: `## 🔧 Squad Implementation Plan` → Program ref → Phase tables (Title|Size|Depends On|Agent|Epic) → Details per task (Scope, Acceptance criteria, Dependencies, Rollout, Traces to) → Dependency Graph → Sizing Summary table → Validation Pre-check → Next: `/squad plan validate`. -The `Agent` column MUST be a cast **Name** per the Agent binding rule (Step 2) — a value from the `Name` column of `.squad/team.md`, never a Role string. +Every `Agent` cell MUST be a permitted value per the Agent binding rule (Step 2) — a verbatim value from the `Name` column of the `## Members` table in `.squad/team.md`, or `@copilot`. Re-check each `Agent` cell against that column before posting. ##### Step 5: Update Lifecycle @@ -960,8 +972,31 @@ Find the latest `program`, `implementation`, and `triage` artifacts. At minimum | 7 | Incomplete metadata | Both | Missing sizes/agents/criteria | | 8 | Orphaned items | Both | Triage items not in program | | 9 | Milestone gaps | Program | Epics not in any milestone | - -Severity: ❌ Critical (blocks acceptance): 1–6, 8. ⚠️ Warning: 7, 9, borderline 5. +| 10 | Non-roster owner/agent | Both | An `Owner`/`Agent` value is absent from the roster (see below) | + +Severity: ❌ Critical (blocks acceptance): 1–6, 8, 10. ⚠️ Warning: 7, 9, borderline 5. + +###### Check 10 — roster binding (the `Name` column is the sole source of truth) + +Run this check mechanically. Do not judge whether a value "looks like" a +teammate, and do not accept a value because it seems plausible or was used +earlier in the plan. + +1. Read the `## Members` table in **this repository's** `.squad/team.md`. Collect + its `Name` column cell values verbatim into the **roster set**. If + `.squad/team.md` is missing or its `## Members` table has no data rows, report + Check 10 as ❌ Critical and stop — a plan cannot bind owners without a roster. +2. Quote the roster set in the validation output, so the reader can see exactly + which values were treated as valid. +3. For every `Owner` cell in the program artifact and every `Agent` cell in the + implementation artifact: the value is valid **only** if it matches a roster-set + entry ignoring case, or is exactly `@copilot`. Every other value — including + any value drawn from a different column of `.squad/team.md`, such as the + `Role` column — is invalid. +4. Every invalid value is a ❌ **Critical** finding, which makes the run + `RESULT: FAIL`. Report each one as: the artifact, the row, the offending + value, and the roster set it must be drawn from. Never report a value as a + valid roster name unless you found it in the `Name` column in step 1. ##### Step 3: Post Result @@ -1086,7 +1121,25 @@ Count expected issues before starting. If total > 50: recommend phased activatio ##### Label Pre-flight -Before the first `create-issue`, verify labels `squad` and any `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. +**Roster binding gate — run this before any `create-issue` call.** + +1. Read the `## Members` table in **this repository's** `.squad/team.md` and + collect its `Name` column cell values verbatim into the **roster set**. +2. List the roster set in the activation summary, so the labels applied can be + traced back to the roster that was actually read. +3. For every `Agent` value in the implementation plan, confirm it matches a + roster-set entry ignoring case, or is exactly `@copilot`. The `Name` column is + the only permitted source; a value taken from any other column of + `.squad/team.md`, such as the `Role` column, does not qualify. +4. A value that fails step 3 MUST NOT be turned into a `squad:{agent}` label. + Apply only the `squad` label for that issue and record the value in the + activation summary under a `Non-roster agent values` heading, naming the + roster set it should have been drawn from. + +`{agent}` in `squad:{agent}` is a roster-set value lowercased — never anything +else. + +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. ##### Transient Failure Handling