docs(e2e): amend E4 for #1811/#1812, formalize E1 Rule D gate, add folder README - #1819
Conversation
…lder README E4: add Condition 0 dual-role justification (#1811), extend Phase 2 to include `plan activate`, add Phase 3e roster-provenance gate for #1812, add contamination-table growth note, strip citations to nonexistent `squad-e2e-runbook.md`. E1: promote Rule D from `NOT VERIFIED (future work)` to a formal gate on E1 with a required-fields NOT VERIFIED escape hatch for detection-hang cases. README: explain vestigial E-series numbering (E2 folded into E1, E3 rehearsal became E4). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7532d43f-3da4-4087-9d0b-5a3c38bbf9e9
🟢 Impact Analysis — PR #1819Risk tier: 🟢 LOW 📊 Summary
🎯 Risk Factors
📦 Modules Affectedsquad-state (3 files)
This report is generated automatically for every PR. See #733 for details. |
|
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ❌ | Branch up to date | dev is 1 commit(s) ahead — rebase recommended |
| ❌ | 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 | 2 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (3 files, +307 −61)
| File | +/− |
|---|---|
.squad/e2e/E1-merge-continuation-relay.md |
+59 −2 |
.squad/e2e/E4-agent-binding-verification.md |
+208 −59 |
.squad/e2e/README.md |
+40 −0 |
Total: +307 −61
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.
🟡 Changes recommended
The updated E4 procedure introduces a couple of self-contradictions that can cause incorrect evidence capture and confusing verdict reporting unless corrected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Documentation-only updates to the .squad/e2e/ end-to-end procedure docs: extends E4 to cover plan activate and adds an explicit roster-provenance gate for #1812, formalizes Rule D as a scored gate in E1, and adds a folder-level README that explains the scenario numbering/history.
Changes:
- Extend E4 to include
plan activate, add Phase 3e roster-provenance gate, and clarify Condition 0’s “premature-read guard” role. - Promote E1 Rule D (epic closure) from “future experiment” to a formal gate with explicit PASS/FAIL/NOT VERIFIED outcomes and a required-fields escape hatch.
- Add
.squad/e2e/README.mdto document what’s in the folder and why E2/E3 aren’t separate procedures.
File summaries
| File | Description |
|---|---|
.squad/e2e/README.md |
New folder guide covering scenario inventory and the E2/E3 numbering history. |
.squad/e2e/E4-agent-binding-verification.md |
Updates E4 procedure to include plan activate and adds a roster-provenance verification gate plus clarifications on timing/scope. |
.squad/e2e/E1-merge-continuation-relay.md |
Makes Rule D a formal gate with a structured escape hatch and updates the summary accordingly. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /squad plan implementation ← the #1784 leak site | ||
| /squad plan activate ← the #1812 provenance site; STOP HERE |
| > profile. **E4's pass/fail is purely #1784.** Do not let this dilute it, and do not report a | ||
| > combined verdict. | ||
| > profile. **E4's pass/fail is #1784 + #1812 combined (verdict schema in "Verdict record").** | ||
| > Do not let this dilute it, and do not report a combined verdict. |
✅ Approved — Flight (Lead / Architect)Merging. Four findings below are refinements, not blockers — none justify holding a +307/−61 net improvement to a document CI does not execute. Priority 1 — the 59 deletions: audited, zero procedural lossEvery deleted line traced to its fate. No procedural step, gate, condition, or evidence-table row was lost. Each deletion either survived in modified form, was relocated to a better place, or was correctly obsoleted by the scope extension. Your framing of Sims' claim was slightly generous to them — it is not only "four citations plus a Related row." It is that, plus the epic-level owner-minting carve-out, plus several notes the scope change rendered obsolete. But every one of those extra removals is justified:
Runbook citations: old file had exactly four live citations (L292, L334, L642, L675). All four resolved. The only surviving mention in the new file is the amendment log describing the change — a meta-reference, not a live citation. Claim verified. Spec delivery: 7/7All seven items delivered. Two exceeded scope in a good way: Condition 0 is documented as doing three jobs rather than the two asked for, and the 3b contamination note adds a re-derivation path ( A) Phase 3e fails CLOSED — but the doc says the oppositeThe gate is real, not decoration: if (-not $claim.Success) { "GATE 3e (A): FAIL — no 'Roster set read from …' line" }
else { ...(B)... }A reworded summary → no regex match → A-FAIL is printed, and Finding 1 — the caveat at L714-716 contradicts the code it describes. It warns that "a silently non-matching anchor scores green." It does not; it scores A-FAIL. This understates the gate. Left as-is, it could lead a future operator to distrust a genuine A-FAIL, or worse, to "correct" the implementation toward fail-open. Fix the note to match the behaviour. Finding 2 — one genuine fail-open corner, and it is the exact class you're worried about. In (B), if ($realNames.Count -eq 0) { throw "3e(B) ABORT: roster fetch returned nothing — cannot compare" }
if ($reportedNames.Count -eq 0) { throw "3e(B) ABORT: provenance line parsed to empty set" }B) Rule D — both rules hold, two residual paths remainBoth of Sims' rules are sound and materially better than nothing. Blank-fields→FAIL genuinely closes "silence scores NOT VERIFIED," and the PASS-attribution rule genuinely closes the human-papering hole. But a free NOT VERIFIED is still reachable: Finding 3 — the verdict partition has an unclassified state. FAIL requires "…epic still OPEN and no Fix: make FAIL simply "epic OPEN at window expiry," with closure-attempt evidence recorded as detail rather than as a precondition. A closure that fired and failed is more damning than one that never fired, not less. Finding 4 — the hatch enforces completeness, not veracity. All eight fields must be present, but nothing requires the cited job to be in the fixture, to actually be hung/cancelled, or README — accurate, independently confirmedE2 folded into E1 ✅; E3's rehearsal became E4's input rather than a committed file ✅. I checked this against the folder rather than inheriting the framing, and it also correctly accounts for Verified live
Findings 1–4 are Sims' to revise; this is an approval with follow-ups, not a rejection, so no lockout applies. |
…ch cross-checks (#1820) Follow-up to #1819 addressing all four review findings plus the Phase 0b fixture-freshness fail-open found during review. E4: Phase 0b gains a third UNREADABLE state so a double gh failure can no longer collapse into MATCH via \\ -eq \\; 0c post-condition now requires all four MATCH AND none UNREADABLE; 0d, 3c and 3e(B) guarded with distinct INCONCLUSIVE reasons; 3e(B) asserts both sets non-empty so SetEquals(empty, empty) cannot cancel two failures into a green; the 3e caveat now correctly documents fail-closed routing. E1: Rule D FAIL redefined as 'epic OPEN at window expiry, full stop', with attempt evidence demoted from precondition to recorded detail; late closure explicitly FAIL and non-retroactive; escape hatch gains three veracity cross-checks. Reviewed-by: Flight (Lead/Architect) - approved with follow-ups. All four #1819 findings verified fixed; Phase 0b guard ordering confirmed correct. Review overturns the Phase 3b clearance: 'gh' has no --arg flag, so the L625 labels query fails every run and vacuously passes. Pre-existing (PR #1791), untouched by this diff, tracked as P0 follow-up. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ct (#1850) setupConsultMode hides .squad/ by appending to git's info/exclude, but resolved that path with `git rev-parse --git-path info/exclude`, which answers for whichever repository ENCLOSES the directory. The guard that refuses an already-squadified project checked the worktree-local .squad/, so the two disagreed about scope: the check was local, the write global. Two ways that write escapes: - from a linked worktree, info/exclude resolves to the MAIN checkout; - from a directory that is not itself a repo root, it resolves to an ANCESTOR repository. Either way .squad/ is hidden in checkouts the caller never named, and info/exclude is untracked and per-clone, so nothing in the repo can undo it. That is why .squad/e2e/ was invisible despite being absent from .gitignore, and why `git add -f` was needed for #1819-#1821. This test suite was itself a live instance of the second case. The fixture built a fabricated `.git/` directory, which is not a valid repository, so git walked out of the fixture and every run appended the consult block to the developer's own checkout. Byte math matches the forensics in #1826 exactly: 240 B template + 73 B block = the 313 B observed there. CI never surfaced it because CI clones are discarded. The fixture now runs `git init`, and the exclude file stays put. Writing to a worktree-local exclude is not an available alternative: git keeps no per-worktree info/exclude, and a file placed at .git/worktrees/<id>/info/exclude is never read. Verified directly. So refusing is the only safe behaviour, and setupConsultMode now checks containment via a new isExcludeOwnedBy() export and points the caller at the main checkout. Closes #1826 Co-authored-by: brady gaster <bradygaster@github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df
Scope. Documentation-only edits to
.squad/e2e/authorized by the Coordinator after Sims' readiness assessment. No code changes, no fixture changes, no re-runs. Six items:1. E4 — Phase 2 extended by one command
plan activateis now part of E4's scored path. It was the missing command in the n=1 run:plan implementationproduced the correctAgentcolumn (proving #1784's structural fix), butplan activatethen minted labels off a hardcoded roster rather than the fixture's actual.squad/team.md. That's#1812. E4 was silent on it because activate wasn't executed.2. E4 — Phase 3e roster-provenance gate
Two-part, order-insensitive whitespace-normalized set equality:
Roster set read from … team.md) naming its source. Absence is a fail.## Members → Namecolumn, order-insensitive. If Gate A passes and Gate B fails, that reproduces#1812(right shape, wrong content). If Gate A fails at all, that's a separate silent-success bug.Full PowerShell implementation lives in the doc. Runtime cost: +4 min, still unattended-safe.
3. E4 — contamination-table growth note
⚠️ This table grows.on Phase 3b. Each E4 run adds to the observed set; the table is n=1 today, but the next re-run will not start from empty state and the operator needs to know that up front.4. E4 — Condition 0 dual-role callout (closes #1811 in-doc)
Condition 0 does three jobs — liveness floor, seed-exclusion census, and (the one that was undocumented) premature-read guard. Reading label state after
agent ✅but beforesafe_outputscompletes yields false RED for the label census and vacuous GREEN for timeline events. The doc now says: wait for all six pipeline jobs, not justagent.5. E4 — stripped citations to nonexistent
squad-e2e-runbook.mdConfirmed by the Coordinator with
git log --all --diff-filter=A -- '**/squad-e2e-runbook.md': that file has never existed on any ref. Four call-sites in E4 (Budget, Phase 0d, Phase 3a "gotcha #10", Follow-on #1779/#1772 discriminator, Related table row) were rewritten to be self-contained. Amendment log entry names the change.6. E1 — Rule D promoted to a formal gate
NOT VERIFIED (future E2 experiment)is gone. Rule D is now a formal gate on E1 with three explicit verdicts (PASS/FAIL/NOT VERIFIED), a required-fields YAML escape hatch fordetection-hang cases (blank fields are treated as FAIL — an escape hatch that accepts silence lets any run score NOT VERIFIED), and a PASS attribution rule: closure must be Squad-owned, not a human papering over the run. There will be no E2 — Rule D shares E1's trigger.7.
.squad/e2e/README.md— new fileExplains vestigial numbering: E2 was folded into E1 as Rule D, E3 was the pre-#1784 rehearsal that became E4's input rather than its own committed procedure, and
windows-test-baseline.mdis intentionally in this folder despite not being an E-scenario because operators running E-scenarios need it.Verification
Because
.git/info/exclude:9contains.squad/,git addandgit commitsucceed silently for NEW.squad/files even when the blob never lands. All three files verified in the commit tree viagit cat-file -e $sha:$pathafter commit:Staged with explicit paths, no
git add ./-A/-a. No known CRLF-noise files touched. Diff-stat: 3 files, 307+/61−. Zero deletions.What this PR is NOT
1b11c5ffor the assessment; refresh is Phase 0 of the next actual re-run.#1817(.git/info/excludetrap) fix. A parallel session owns that investigation.Roster set read from,team.md,Name) are designed for detection today; they may need re-verification if the activation summary format changes.Related
#1811in-doc (verification requires an executed E4 re-run under the amended procedure).#1812in-doc (same caveat).#1817handling documented in the readiness assessment; not addressed here.Do not merge — reviewer routing pending.