Repository navigation
Plan validation self-clears: the checks table ships pre-filled ✅, and three surfaces disagree on what it contains #1801
Description
Queued for tomorrow. Filed while blocked on #1789's merge. No code, no prompt edits, no PR — this is analysis plus a proposal. Do not start work until the current wave lands.
Related: #1784 (Owner/Agent binding leak) · Evidence: https://github.com/bradygaster/squad/issues/1784#issuecomment-5367625542 (Sims) · Same class as #1766, #1778
The observation
E3's implementation plan emitted its own validation as:
| Agent assignments valid (cast Names) | ✅ (lead, lead, devrel) |
It asserted that three role strings are cast Names. The validation passed on precisely the input it exists to reject.
Context that makes this worse than a one-off: correct binding has a zero success rate on that fixture. No roster name (keaton, mcmanus, fenster, hockney, kint) has ever been minted; all three tokens named in the prohibitions (lead, devrel, reviewer) have. The self-check has reported green across a population of runs in which the underlying behavior was never once correct.
Correcting the premise — it is not invented, which is worse
The working theory was that the model invented the row and cleared itself on it. I checked the full prompt surface rather than inheriting the grep, and that is not what happened.
Agent assignments valid is a specified row. It lives in workflows/shared/squad-planning-ontology.md:258 — imported at squad.md:38, and five skills are told "The planning ontology is imported — follow its schemas directly." A squad.md-only grep misses it.
Here is the specification, verbatim (ontology §3.6):
### Checks
| Check | Status | Detail |
|-------|--------|--------|
| All tasks map to an epic | ✅ | — |
| No circular dependencies | ✅ | — |
| Sizing within bounds (no >XL) | ✅ | — |
| Agent assignments valid | ✅ | — |
| Scope boundary respected | ✅ | — |
| <check> | ❌ | <diagnostic> |Every status cell ships pre-filled ✅. The model was not asked to determine a verdict — it was handed a table where the verdict was already PASS and asked to reproduce it.
This is #1784's mechanism, pointed at the verdict instead of the value. A concrete literal in the prompt gets copied verbatim; that is the finding #1784 established. Here the salient literal is ✅.
The ontology's own convention proves the row is malformed. Look at §3.5 directly above it — <PR-sized task title>, <squad member>, <risk>, <mitigation strategy> — angle brackets mean "you fill this in." §3.6's own last row obeys it: | <check> | ❌ | <diagnostic> |. The five rows above it break the convention and hardcode the answer.
Not invention. A spec that ships pre-cleared — reproducible by design, on every run, in every downstream repo.
Scope: three surfaces, none of them agreeing
| # | Surface | Row set | Status |
|---|---|---|---|
| 1 | Validation Pre-check in the implementation plan (squad.md:937) |
None. Named in a structure list; zero rows specified. | Fully emergent — and this is the artifact E3's leaked row lived in |
| 2 | Ontology §3.6 Validation Result (squad-planning-ontology.md:245-264) |
5 literal names, pre-filled ✅ |
Specified and pre-cleared |
| 3 | /squad plan validate Step 2 (squad.md:962-977) |
Numbered checks 1–9 (1–10 after #1789) | Specified — but does not correspond to §3.6's five names |
Surface 1 has no rows, so the model reached for surface 2, which is pre-cleared. Surface 3 — the one we think of as "the validator" — was never consulted, and its check names do not even match. The ambiguity resolved toward the pre-filled template, which is the outcome you would predict.
Does #1789's Check 10 close this? No.
Asked to be adversarial about my own fix. It does not, for four reasons, and I do not want this recorded as closed by #1789:
- Wrong artifact. E3's self-cleared row was emitted by
squad-plan-implementationinto the implementation plan. Check 10 lives insquad-plan-validateStep 2. Different skill, different artifact —squad-plan-implementationnever reads Step 2's table. - Nothing forces Check 10 to appear. Step 3 specifies "Results table (Check|Status|Details)" with no required row set. A run that silently omits row 10 emits a structurally valid
RESULT: PASS. A validator that can be omitted is not a validator — that framing is correct and I concede it. - The pre-cleared template survives untouched. fix(squad.md): stop seeding Owner/Agent binding with the tokens it forbids #1789 did not modify the ontology.
| Agent assignments valid | ✅ | — |is still in the prompt surface, still shipping a literal✅. My PR left the actual proximate cause in place. - Same category as the rule that failed. Check 10 is an instruction to a model. The binding rule was an instruction present at seven sites with a zero success rate. There is no basis for a stronger prior on Check 10.
#1789 remains a net improvement — Step 2 previously had no roster check at all — but I overstated its reach. It addresses one of three surfaces, and not the one that produced this artifact.
Proposal: deterministic enforcement (scoping only — do not implement from this)
The binding constraint: the Squad workflow holds issues: read. It cannot write labels. All mutation flows through gh-aw safe-outputs, processed by generated .lock.yml steps that are DO-NOT-EDIT. So enforcement cannot live inside the agent job — anything there is still the model grading itself. It must be a separate job reading an emitted artifact.
Option B — artifact lint on the implementation comment ⭐ recommended
Trigger on issue_comment.created. If the comment carries squad_artifact: implementation, parse the Agent column from the markdown table, parse the Name column from .squad/team.md, fail on any value not in the roster set (or @copilot).
- Permissions:
issues: readonly — no new write surface. - Cost: one workflow plus a parser. The parser is the risk: regex over model-authored markdown, so table drift breaks it. Mitigated by the table shape being specified in ontology §3.5, with
template-sync.test.tsas precedent for gating drift. - Must fail closed. Zero parsed rows on an
implementationartifact = failure, not pass. A malformed table that reads as "no violations" is the exact false-green class we are trying to kill; a checker that can be silenced by bad input reproduces this very bug. - Blind spot: catches the Agent column only. Per Sims' scope bound, epic labels are minted at
activatefrom a program artifact that carries no owner assignment — so this cannot see every label leak. - Why this one: it catches the defect at emission, before
activateapplies anything, and it inspects the same column E4 inspects.
Option A — post-emission label reconciler (defense in depth)
Separate workflow with issues: write. On issues.labeled, strip any squad:{x} whose x is absent from the roster, apply squad:needs-owner, comment with the roster.
- Corrective, not preventive — the bad label exists briefly.
- Requires a new
issues: writegrant that the Squad workflow deliberately does not hold. That is a real security surface and needs RETRO's review, not mine. - Catches leaks that never passed through an implementation artifact.
Option C — inject the roster as data (complement only, ❌ not a fix)
A workflow step reads .squad/team.md and interpolates the parsed Name list into the prompt.
Flagging this explicitly because it is the tempting cheap option. It improves the input; it does not validate the output. Shipping C and calling #1784 retired would be this failure mode one more time. It is a complement to B, never a substitute.
Recommendation: B retires the issue, A as defense in depth, C optional.
How this gets tested (both bars applied)
The bars: a test must be able to fail against the pre-fix state, and a test asserting prompt text is present can never prove the prompt is obeyed — that is the trap that let #1778 ship the very defect it was meant to catch.
Option B escapes that trap structurally: the checker is code, and the test executes code against data. There is no prompt in the loop.
- Red against recorded production input. We now hold E3's actual emitted table. Fixture it — Agent column
lead,lead,devrel; rosterkeaton/mcmanus/fenster/hockney/kint— and assert the checker fails. That is a real red built from observed fact, not a constructed hypothetical. - Green on a clean artifact. Same table with roster names → passes.
- Fail-closed. Malformed or empty table on an
implementationartifact → fails. Explicitly asserted, because vacuous-pass is the bug.
Separately testable sub-fix, available immediately
Assert that no schema template in the ontology ships a literal verdict token in a cell the model is supposed to determine. §3.6 currently has five, so this goes red today and green once the ✅s become <status>. Small, mechanical, no model in the loop, and it kills the proximate cause of this artifact.
Second-order risk
An invented — or worse, pre-cleared — validation row is more dangerous than no validation at all, because it manufactures false confidence in exactly the artifact a reviewer reaches for first. This is tonight's seventh instance of asserting on a reporter rather than an artifact, and the most dangerous, because here the reporter is the system's own quality claim.
Acceptance criteria
- Ontology §3.6 status cells use placeholder syntax, not a literal verdict; a test enforces no literal verdict tokens in determinable schema cells
- The implementation plan's
Validation Pre-checkhas a specified row set, or is removed — an unspecified self-report section is not retained - Surfaces 2 and 3 are reconciled to one check vocabulary, or their relationship is made explicit
- A deterministic, non-model checker compares the emitted
Agentcolumn against.squad/team.md'sNamecolumn - That checker fails closed on unparseable input, with an explicit test
- Red proven against E3's recorded artifact, not a constructed example
- No acceptance test asserts that prompt text is present
Activity
github-actions commentedon Aug 21, 2026
📋 Assigned to Procedures (Prompt Engineer)
Issue: #1801 — Plan validation self-clears: the checks table ships pre-filled ✅, and three surfaces disagree on what it contains
Procedures will pick this up in the next Copilot session.
bradygaster commentedon Aug 21, 2026
Sequencing decision (2026-08-21) — sub-fix ships AFTER E4, as a separable first commit
Recorded so the next person to pick this up does not ship the sub-fix early and burn E4's attribution.
Commit 1 — the
✅ → <status>sub-fix (separable, no dependencies)Scope:
workflows/shared/squad-planning-ontology.md§3.6 only. Replace the five literal✅status cells with placeholder syntax matching the section's own convention (<status>), so the schema stops shipping a pre-cleared verdict. Add the accompanying test — no schema template in the ontology ships a literal verdict token in a cell the model is supposed to determine.This lands on its own. It needs none of Option B's infrastructure — no new workflow, no parser, no permissions review. It is red today against five rows and green immediately after. Do not bundle it into the Option B PR.
Commit 2+ — Option B, fail-closed
The artifact lint described in the body. Separate PR, separate review, and it is the commit that actually retires #1784.
Why the sub-fix waits for E4
E4 is a single-variable test. Its entire value is attribution — it is the first opportunity to observe whether #1789's prompt change is obeyed at runtime, against a defect with a zero success rate across every observation held. A second prompt change to a file the fixture also imports gives any E4 failure two candidate causes and costs us the one clean measurement available.
Two things were checked rather than assumed before deferring:
$E4_STARTcutoff, and theAgentcolumn itself. So the pre-cleared✅cannot contaminate E4's verdict. Had E4 read that table, this would be a blocker and the sub-fix would ship now.shared/squad-planning-ontology.md, 16,420 bytes, byte-identical on both sides. It is the one file that is not drifting, and the two that are drifting are mid-sync. Changing a third underneath that work turns a bounded operation into a moving target.Standing caveats carried forward
RESULT: PASS.Failure class, restated
This was previously recorded as "the model invented a validation row and cleared itself on it." That framing is wrong, and the correction makes it worse:
The original framing made this a model failure: rare, hard to reproduce, arguably a fluke. The correct framing makes it a spec failure: deterministic, reproducible by construction, and shipped into every downstream repo that imports the ontology. It is also #1784's own mechanism aimed at the verdict rather than the value — the same salient-literal capture, a different cell. Two instances of one mechanism is a pattern, not a coincidence.
bradygaster commentedon Aug 23, 2026
1. Owner & rationale
Owner:
squad:procedures. The defect lives in the planning ontology and validation prompt surfaces; Procedures owns prompt contracts and response schemas.Labels reconciled as:
gh-aw,type:bug,priority:p1,wave:1-next,squad:procedures,triaged,go:spec-ready. I removed the legacybuglabel because this issue now uses thetype:*taxonomy.2. Files in scope
Verified existing paths:
workflows/shared/squad-planning-ontology.mdworkflows/squad.mdtest/gh-aw-quality.test.tstest/template-sync.test.ts3. Reproduction / measurement
This command was execution-tested and uses no output suppression:
It currently prints the five pre-cleared validation rows, including
| Agent assignments valid | ✅ | — |.4. Verification bar
Two bars must go red on today's state and green after the fix: (1) no schema/template cell that the model is supposed to determine may ship a literal pass/fail verdict, and (2) a deterministic checker using E3's recorded bad implementation table (
lead,lead,devrel) against the fixture roster (Keaton,McManus,Fenster,Hockney,Kint) must fail closed, including on empty or unparseable tables.5. Traps
Cross-cutting call with #1812: keep the issues independent. This one fixes the validator's self-clearing surface; #1812 fixes activation's false roster provenance. They are the same false-green class, but not the same stage or code path.
Cross-cutting call with #1757: do not absorb this into #1757. Re-scope #1757 to the later adversarial-quality pass after this deterministic false-green bug is fixed; #1757 should not own literal verdict tokens, fail-closed parsing, or E3 regression fixtures.
Do not accept a test that only asserts prompt text exists. The check must execute code against emitted artifact text and fail on today's bad artifact.
6. Out of scope
Do not implement Fact Checker/Devil's Advocate validation here, redesign the whole validation workflow, or add write permissions. Keep this to deterministic validation-template and emitted-artifact gates.
7. Session-stall guard
fix(gh-aw): stop the validation schema from shipping its own verdict (#…
bradygaster commentedon Aug 24, 2026
Design note from the first full E4 run — one approach is now ruled out
Running E4 to full activation on
octodemo/aspiregregator-squad-e2eproduced the two real defects this issue anticipated (#1859 agent mis-binding, #1860 summary reporting labels it never applied), plus the artifacts a deterministic checker would have to parse. Two findings change the shape of the fix.1. Title matching is infeasible — this rules out the naive parser
The obvious Option B implementation is: read the accepted plan's task table, read the created issues, match them by title, compare agents. Issue titles are shortened and reworded during activation, so that join does not exist:
Persist `LastFailureAt`/`FailureCount`/`LastFailureReason` on source (Orleans ids 4/5/6, per D2)ISourceProvider/`ISourceGrain```The second is materially reworded, not just truncated. Fuzzy matching here would produce a checker whose false-negative rate is unknown and unmeasurable — worse than no checker, because it would be trusted.
2. The checker should not parse prose at all
The
activatedartifact currently carries:{"origin_issue":5,"phases":[],"schema_version":"1","squad_artifact":"activated"}Extending it with a
bindingsarray — task # -> issue # -> agent — gives the checker a machine-readable join and removes prose parsing from the critical path entirely. The activation step already knows every one of those three values atcreate-issuetime; nothing needs to be re-derived.3. What it must assert
squad:{agent}label actually on the issue equals that task's ownAgentcell.Agentvalues — one distinct value mints that label, two or more resolve to baresquad. Verified consistent for epics 1.1, 1.2, 1.3, 2.2, 2.3 on the fixture; 2.1 was the lone drift and is exactly gh-aw:plan activatebinds a task'ssquad:{agent}label from its epic owner, not the task'sAgentcell #1859.activatedartifact is a failure, not a pass.4. Parser contract gotcha
The implementation-plan table (ontology §3.5,
| # | Title | Size | Depends On | Agent | Epic |) repeats its header once per rollout wave. A parser that assumes a single table silently reads only the first wave — a partial pass that looks clean.5. Why this is still needed after #1863
#1863 disambiguates the prompt so
{agent}is bound to each issue's own plan row, and adds a correspondence step to the Label Pre-flight gate. That addresses the mechanism behind #1859/#1860, and it is mutation-tested.But it is still an instruction to a model, verified by asserting prompt text. The central finding stands: TG-2 and Check 10 were both green while #1859 was live. Membership was certified; correspondence was never checked by anything deterministic. Until something outside the model compares the label on the issue against the row that assigned it, a correct-looking run and a wrong one remain indistinguishable from the outside.
Repo precedent for the shape:
scripts/check-squad-leakage.mjs+ a plain (non-agentic) workflow onissue_comment.created, mirroringsquad-label-enforce.yml. #1859 and #1860 are its first two test cases.fix(gh-aw): bind squad:{agent} labels to each issue's own plan row (#…
fix(gh-aw): enforce activation agent bindings (github#1865)