Skip to content

fix(squad.md): stop seeding Owner/Agent binding with the tokens it forbids - #1789

Merged
bradygaster merged 1 commit into
devfrom
bradygaster-1784-agent-binding-token-leak
Aug 21, 2026
Merged

fix(squad.md): stop seeding Owner/Agent binding with the tokens it forbids#1789
bradygaster merged 1 commit into
devfrom
bradygaster-1784-agent-binding-token-leak

Conversation

@bradygaster

Copy link
Copy Markdown
Owner

Working as Procedures (Prompt Engineer).

Closes #1784.

The finding

E3 ran the full long planning path against fixture bradygaster/aspiregregator-squad-e2e #16. Activate minted squad:lead x3 and squad:devrel x1.

We ruled out the innocent explanations. The fixture's .github/workflows/squad.md is 55,954 chars — byte-identical in length to dev — and contains every #1778 marker. The fixture has a valid .squad/team.md with a populated ## Members table (Keaton, McManus, Fenster, Hockney, Kint).

There is no DevRel role anywhere in that roster. The model could not have derived devrel from team.md. It lifted both tokens verbatim out of the parenthetical that forbids them:

never a Role string (Lead, DevRel) or lowercased role (lead, reviewer)

squad.md:730 had the same shape — it spelled out `squad:lead` as the bad example, and squad:lead is exactly what shipped.

Naming the forbidden value concretely made it more likely to be emitted. The concrete token is salient; the negation is not.

What changed

Six binding sites in workflows/squad.md, each rewritten as a positive requirement only — what the value MUST be, plus the @copilot fallback, with no enumerated counter-examples. Prohibition is now abstract ("no other column of .squad/team.md supplies a valid owner, in any casing").

Site Before After
squad-plan Step 1 Name verbatim (e.g. `Flight`, `Procedures`, `EECOM`)never a Role string (`Lead`, `Prompt Engineer`, `DevRel`) and never a lowercased role (`lead`, `devrel`, `reviewer`) 4-step procedure: build the allowed-owner set from the Name column, write it out, bind every value to it character-for-character, resolve via routing.md. No tokens named.
squad-plan Step 3 "never a Role string" "a verbatim value from the Name column … or @copilot. Re-check each cell before posting."
squad-plan-accept labels never mint a role-derived label such as `squad:lead` or `squad:reviewer` "confirm the Owner value appears verbatim in the Name column … if not present, do not mint the label"
squad-plan-implementation Step 2 never a Role string (`Lead`, `DevRel`) or lowercased role (`lead`, `reviewer`) "list the Name column values verbatim in your working notes … that list, plus @copilot, is the complete set of permitted values"
squad-plan-implementation Step 3 "never a Role string" "appears verbatim in the Name column … or is @copilot"
squad-plan-implementation Step 4 "never a Role string" "a verbatim value from the Name column … Re-check each cell before posting."

Positive examples were genericized too. (e.g. `Flight`, `Procedures`, `EECOM`) and Owner `Flight` → `squad:flight` were this repo's names appearing in a prompt that runs against other repos' rosters — the same copy-the-salient-token mechanism, pointed at a different target.

Grounding. The plan and implementation rules now instruct the model to write out the roster it read before assigning, so the binding rests on data it just retrieved rather than on recall.

/squad plan validate — Check 10. The validator returned RESULT: PASS, 0 critical / 0 warnings, and explicitly attested that lead/devrel were "valid roster names." It now has a mechanical roster check with the Name column as sole source of truth: build the roster set, quote it in the output, flag every non-matching value as ❌ Critical (which forces RESULT: FAIL), and "never report a value as a valid roster name unless you found it in the Name column." A missing or empty roster is itself Critical.

Activate pre-flight. The skill that actually applies labels now gates on the roster set: a value that fails the check must not become a squad:{agent} label. That issue gets the bare squad label and the value is reported under a Non-roster agent values heading.

Testing — read this part

#1778 shipped a "role-string-leak detector" that passed while the system was broken. It asserted the prompt text contained the rule. A prompt-text-presence test can never verify LLM compliance — the rule was present at 7 sites and was disobeyed at all of them. That is the #1766 failure mode.

Those assertions are deleted, not added to. In their place is the falsifiable inverse: the binding blocks must contain no backticked code span that names a Role. Backticked spans are the most copyable form a token takes in a prompt, and that is precisely the regression being fixed. The forbidden set is derived from team.md's Role column rather than hardcoded, so it stays honest as the roster changes.

The detector is itself tested against the real pre-fix strings from lines 913 and 730, so a broken detector cannot silently pass.

Falsifiability check — I restored the pre-fix workflows/squad.md and re-ran:

× squad-plan-accept mints squad:{owner} from the cast Name, not a role
× squad-plan-implementation binds the Agent column to the cast Name
× squad-plan states the binding positively, without naming a Role token
× squad-plan-accept states the binding positively, without naming a Role token
× squad-plan-implementation states the binding positively, without naming a Role token
× declares a roster-binding check in the checks table
× scores a non-roster value as Critical, not a warning
× binds the check to the Name column and forbids attesting validity otherwise
  Tests  8 failed | 23 passed (31)

Post-fix: 31/31 pass.

⚠️ What I did NOT verify

This fix is not live-verified. No fixture run was performed. Everything above is static analysis of prompt text plus a compile check.

Concretely:

  • I have not confirmed the model now emits cast Names. Only a fixture run through /squad plan activate and an inspection of the labels actually applied can show that. That is a post-merge step and it is the only evidence that closes this issue's success criteria.
  • Check 10 and the activate gate are still instructions to a model, not code. They are strictly stronger than what was there — the validator previously had no roster check at all — but they can be disobeyed exactly as the binding rule was.
  • Success criteria 1 and 2 in Owner/Agent binding fix (#1759) is ineffective live — model copies the forbidden tokens out of the prohibition #1784 ("validated programmatically, not by prompt instruction") are not met by this PR and are deliberately out of scope. This is the low-risk half, shipped ahead of tomorrow's E2E series. The deterministic-enforcement rewrite is follow-up work.

Verification performed

  • npx vitest run test/gh-aw-plan-lifecycle.test.ts — 31/31 pass
  • gh aw compile --strict gate (gh-aw-quality.test.ts > strict-compiles and preserves prompt/config behavior) — passes against the modified workflow
  • npm run build — exit 0
  • npm test — 9 files fail, all pre-existing. Confirmed by stashing both changed files and re-running the same 9 on a clean tree: identical failures (Windows path separators, CLI packaging smoke, scheduler timing, ESM patching).
  • Staged diffstat: 2 files, +176/−25. Deletion check empty.
  • No changeset — no packages/*/src changes.

…rbids

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
Copilot AI lite review requested due to automatic review settings August 21, 2026 08:16
@bradygaster bradygaster added squad:procedures Assigned to Procedures (Prompt Engineer) bug Something isn't working labels Aug 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🟡 Impact Analysis — PR #1789

Risk tier: 🟡 MEDIUM

📊 Summary

Metric Count
Files changed 2
Files added 0
Files modified 2
Files deleted 0
Modules touched 2

🎯 Risk Factors

  • 2 files changed (≤5 → LOW)
  • 2 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)
  • workflows/squad.md
tests (1 file)
  • test/gh-aw-plan-lifecycle.test.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 153730f

PR Scope: 🔧 Infrastructure

⚠️ 2 item(s) to address before review

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 .squad/ or docs/proposals/ files
No merge conflicts No merge conflicts
Copilot threads resolved No Copilot review threads
CI passing 9 check(s) still running

Files Changed (2 files, +176 −25)

File +/−
test/gh-aw-plan-lifecycle.test.ts +106 −8
workflows/squad.md +70 −17

Total: +176 −25


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Squad workflow prompt and its contract tests to reduce “role token leakage” into Owner/Agent bindings by rewriting several binding instructions as positive-only constraints (no enumerated counter-examples), and by adding stronger validator/activation roster-binding guidance checks.

Changes:

  • Rewrites multiple Owner/Agent binding instruction sites in workflows/squad.md to avoid naming forbidden role tokens and to ground bindings in the .squad/team.md ## MembersName column.
  • Adds a new “Check 10” roster-binding procedure to /squad plan validate and a corresponding roster pre-flight gate in /squad plan activate.
  • Replaces the prior “prompt-text presence” assertions with a detector that fails when binding blocks contain backticked role tokens derived from the Role column.
Show a summary per file
File Description
workflows/squad.md Reworks plan/accept/implementation/validate/activate instructions to bind Owner/Agent to the roster Name column using positive-only phrasing; adds Check 10 and an activation roster gate.
test/gh-aw-plan-lifecycle.test.ts Updates lifecycle contract tests with a role-token leak detector for binding blocks and adds assertions for the new validation roster check/severity rules.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

workflows/squad.md:1134

  • The roster gate allows @copilot as a valid Agent (step 3), but step 4 only blocks values that fail step 3 from becoming squad:{agent} labels. That leaves the @copilot case ambiguous (and it also conflicts with the later rule that {agent} must be a roster-set value).

It would be clearer to explicitly say that @copilot must never be turned into a squad:{agent} label, and that only roster names get a squad:{agent} label.

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.
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread workflows/squad.md
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.
Comment thread workflows/squad.md
Comment on lines +666 to +670
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
@bradygaster
bradygaster merged commit a4fb991 into dev Aug 21, 2026
19 checks passed
bradygaster pushed a commit that referenced this pull request Aug 21, 2026
… sync

The fixture had drifted: workflows/squad.md was 4,665 bytes behind dev and
squad-implement-worker.md 1,731 behind, so E4 would have executed the pre-#1789
prompt. Synced and recompiled (fixture commit 1b11c5f); all five prompt sources
now byte-identical to dev by blob SHA.

Phase 0d changes:
- Assert BOTH directions. #1789 replaced the old prohibition rather than appending,
  so "never mint a role-derived" must be ABSENT, not just Check 10 present. An
  addition-only check passes a file carrying both rules at once, which is worse
  than either alone.
- Replace the speculative DevRel/squad:lead probe with the now-known markers
  (Check 10 / sole source of truth / Non-roster), since #1789 has merged.
- Warn explicitly against asserting on squad.lock.yml. The lock does not inline
  the prompt body; it emits {{#runtime-import .github/workflows/squad.md}} resolved
  at runtime, so the markers cannot appear there and checking it returns a
  guaranteed false FAIL. The lock's body_hash is what proves a recompile ingested
  the new source. Each artifact asserted for what it can actually represent.
- Fix the gh api call: add ?ref=main and the -join/-replace needed before
  FromBase64String.

Refs #1784, #1789.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
bradygaster pushed a commit that referenced this pull request Aug 21, 2026
…is stage-local

Two corrections to the E4 result, both measured:

1. REFUTED: the claim that with create-label enabled this run would have
   minted squad:lead again. activate applies what the plan hands it; E3's
   plan column was 'lead, lead, devrel' (exactly its minted set) while E4's
   contains only cast names, so there is no role token to apply. create-label
   is required for the correct behaviour, not what suppressed the defect.
   Condition 2's green is still structural, but the structure is the roster
   mismatch rather than the missing permission.

2. The activate roster read is stage-local and provably capable of being
   correct: plan validate in the same walk, 23 minutes earlier, read the same
   file/section/column correctly and verified all four Agent values verbatim
   via Check 10. Not a wrong path, not a hardcoded default, not a shared
   component - a working reference implementation exists one stage earlier.

Also records that E3's validate enumerated no roster at all (false accept by
omission, not by wrong roster), so #1789 added the roster echo and Check 10 at
validate and left activate untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 34df7b7a-c954-4109-99c7-7c20c72bf2a0
bradygaster added a commit that referenced this pull request Aug 21, 2026
…1813)

* docs(e2e): add E4 agent-binding verification procedure for #1784

Adds .squad/e2e/E4-agent-binding-verification.md: a runnable, not-yet-executed
procedure to verify #1784 (plan activate minting squad:lead/squad:devrel as
owner labels instead of cast names) once Procedures' fix merges.

Targets ~27 min instead of a full ~54 min long-path run by stopping at
`plan implementation`, which is where the Agent-column leak originates
(workflows/squad.md:913) rather than where it surfaces (:730).

Key contents:
- Three-way PASS/PARTIAL/FAIL criterion, stated before the run. squad:devrel is
  dispositive (DevRel appears nowhere in the fixture roster, verified), while
  squad:lead is ambiguous (Keaton's Role column reads "Lead / Architect"). A
  binary criterion would score a real partial fix as FAIL.
- Fixture-refresh phase covering the traps that produce false results: two files
  named squad.md (56,525 vs 6,688 bytes), two independently stale surfaces
  (compiled lock + committed source), GH_AW_INFO_FRONTMATTER_SOURCE being
  provenance only, and PowerShell backtick/ConvertFrom-Json pitfalls.
- Seed issue shaped for >=2 sibling epics per EECOM, with an explicit guard rail
  against contorting the fixture to force that shape.
- #1779 / PR #1787 refill verification kept as a clearly separated follow-on with
  its own criterion, marked not-unattended-safe.

Refs #1784. Deliberately does not close it: this procedure has not been executed
and #1784 must not be marked verified until it runs green post-merge.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* docs(e2e): replace briefed observation with direct artifact read; fix label check

Read run 32433493989 (plan implementation) and 32435055598 (plan activate)
directly rather than relying on the relayed observation. Briefed counts confirmed
exactly, plus four findings — one of which was a defect in this procedure.

Confirmed: Agent column is `lead`, `lead`, `devrel` (lead x2, devrel x1); labels
minted are squad:lead x3 + squad:devrel x1.

New findings:
- Squad's own "Validation Pre-check" printed
  "Agent assignments valid (cast Names) | OK (lead, lead, devrel)" — it passes on
  exactly the input it should reject. Added an explicit instruction not to trust it.
- Zero legitimate cast-name labels have ever been minted (all five roster names
  ABSENT). The binding has never once worked.
- squad:reviewer also leaked, in E1 (#9) — so all three tokens named in the two
  prohibitions have leaked, and nothing outside that set ever has. Added
  squad:reviewer to the criterion as a second dispositive token.
- The plan program artifact contains NO owner assignment, so epic #17's squad:lead
  is minted at plan activate — downstream of where E4 stops. Documented as a scope
  limitation: a green E4 means the Agent column is clean, not that no label leaks.

Procedure defect fixed: Phase 3b checked repo-global label existence, but labels
persist once created and squad:lead/devrel/reviewer already exist from E1 and E3.
That check returns true forever and would have failed a perfect run. Rescoped to
issues created after an $E4_START cutoff, with the pre-existing contamination
tabulated.

Refs #1784.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* docs(e2e): correct Phase 0d to assert the .md, and record the fixture sync

The fixture had drifted: workflows/squad.md was 4,665 bytes behind dev and
squad-implement-worker.md 1,731 behind, so E4 would have executed the pre-#1789
prompt. Synced and recompiled (fixture commit 1b11c5f); all five prompt sources
now byte-identical to dev by blob SHA.

Phase 0d changes:
- Assert BOTH directions. #1789 replaced the old prohibition rather than appending,
  so "never mint a role-derived" must be ABSENT, not just Check 10 present. An
  addition-only check passes a file carrying both rules at once, which is worse
  than either alone.
- Replace the speculative DevRel/squad:lead probe with the now-known markers
  (Check 10 / sole source of truth / Non-roster), since #1789 has merged.
- Warn explicitly against asserting on squad.lock.yml. The lock does not inline
  the prompt body; it emits {{#runtime-import .github/workflows/squad.md}} resolved
  at runtime, so the markers cannot appear there and checking it returns a
  guaranteed false FAIL. The lock's body_hash is what proves a recompile ingested
  the new source. Each artifact asserted for what it can actually represent.
- Fix the gh api call: add ?ref=main and the -join/-replace needed before
  FromBase64String.

Refs #1784, #1789.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* docs(e2e): record E4 agent-binding verification result

E4 executed 2026-08-21 against fixture bradygaster/aspiregregator-squad-e2e
(seed #22). 8 gates, 8 green on first attempt, zero interventions, ~55 min.

Verdict PASS on all four conditions:
- Cond 0: 15 squad-authored issues created by activate (#23-#37)
- Cond 1: Agent column 11/11 verbatim roster names, zero role-derived tokens
- Cond 2: 15 timeline labeled events, all 'squad'; zero unlabeled
- Cond 3: activation summary reports the missing-label prerequisite gap

Same-fixture control: E3 produced 'lead, lead, devrel' on this exact column.

Two qualifications recorded so the result is not overstated:
- The activate-stage roster read still returns the generic uncast vocabulary
  (lead/reviewer/devrel/security/docs) while citing a file whose Name column
  is Keaton/McManus/Fenster/Hockney/Kint. Cond 2's green is structural (no
  create-label configured), not earned. Likely a separate defect from #1784.
- The fixture cannot express a correct owner label at all; closing #1784's
  user-visible symptom needs an issues:write + create-label change.

Also adds a known-gap note that the criterion detects wrong vocabulary, not
wrong assignment, and records the first live #1787 observation: all 15 issues
parent to root, epics have zero sub-issues, so refill draws from the root.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 34df7b7a-c954-4109-99c7-7c20c72bf2a0

* docs(e2e): correct E4 Finding A - create-label claim refuted, defect is stage-local

Two corrections to the E4 result, both measured:

1. REFUTED: the claim that with create-label enabled this run would have
   minted squad:lead again. activate applies what the plan hands it; E3's
   plan column was 'lead, lead, devrel' (exactly its minted set) while E4's
   contains only cast names, so there is no role token to apply. create-label
   is required for the correct behaviour, not what suppressed the defect.
   Condition 2's green is still structural, but the structure is the roster
   mismatch rather than the missing permission.

2. The activate roster read is stage-local and provably capable of being
   correct: plan validate in the same walk, 23 minutes earlier, read the same
   file/section/column correctly and verified all four Agent values verbatim
   via Check 10. Not a wrong path, not a hardcoded default, not a shared
   component - a working reference implementation exists one stage earlier.

Also records that E3's validate enumerated no roster at all (false accept by
omission, not by wrong roster), so #1789 added the roster echo and Check 10 at
validate and left activate untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 34df7b7a-c954-4109-99c7-7c20c72bf2a0

---------

Co-authored-by: brady gaster <bradygaster@github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 34df7b7a-c954-4109-99c7-7c20c72bf2a0
@bradygaster
bradygaster deleted the bradygaster-1784-agent-binding-token-leak branch September 9, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working squad:procedures Assigned to Procedures (Prompt Engineer)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Owner/Agent binding fix (#1759) is ineffective live — model copies the forbidden tokens out of the prohibition

2 participants