Harden test-quarantine workflow guardrails (B1/B2/B3)#67764
Conversation
Fixes three bad behaviors observed in recent test-quarantine runs: - B1: stray commits from one candidate leaking into a sibling PR's branch. Adds Step 3.0 "Branch hygiene" requiring each PR to start from a clean main (clean the tree, then git checkout -f main) and a pre-submit git log main..HEAD / git diff main...HEAD self-check. - B2: re-quarantining a test whose prior attempt a maintainer already rejected by closing the PR. The closed_quarantine_prs pre-activation feed now emits closed_at/closed_by/trusted_closed; the Important-Rules and Case B logic impose a date cutoff (count only failures after a trusted close for quarantine/re-quarantine; blanket-skip for unquarantine). trusted_closed is derived from token-safe signals (non-author human closer of a bot PR, public dotnet org membership, or a trusted comment). - B3: unquarantining a permanently re-quarantined test. Adds a mandatory pre-PR diff self-check that aborts if any removed QuarantinedTest attribute (method-, class-, or assembly-level) references an issue in requarantine_issue_numbers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6695e66-ef29-4591-8ca6-fc494164b1ca
There was a problem hiding this comment.
Pull request overview
This PR hardens the test-quarantine agentic workflow guardrails to prevent (1) cross-PR branch contamination, (2) re-proposing quarantines/unquarantines after a trusted maintainer has already closed a prior attempt, and (3) unquarantining tests that are permanently barred due to a prior re-quarantine.
Changes:
- Extend the “closed test-quarantine PRs” pre-activation feed to include
closed_at,closed_by, and a token-safetrusted_closedsignal for applying a cutoff based on trusted closes. - Update the workflow guidance to apply a “failures after cutoff only” rule for quarantine/re-quarantine, and to skip unquarantines entirely when a cutoff-establishing prior close exists.
- Add explicit “Branch hygiene” and a mandatory pre-PR diff gate to prevent unquarantining tests whose issue is in
requarantine_issue_numbers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/test-quarantine.md | Updates workflow spec: richer closed-PR metadata/trust signals, cutoff-based rules, branch-hygiene instructions, and a mandatory unquarantine re-quarantine diff gate. |
| .github/workflows/test-quarantine.lock.yml | Regenerated compiled workflow reflecting the updated spec (no action SHA changes). |
| ``` | ||
| git checkout -- . | ||
| git reset --hard HEAD | ||
| git checkout -f main | ||
| ``` |
There was a problem hiding this comment.
Good catch — applied in 628b604. Added git clean -fd to the branch-hygiene sequence (before switching back to main) so untracked files/directories left by a prior candidate can't leak into the next PR. I intentionally omitted -x so gitignored build artifacts are preserved.
…ates Addresses PR review feedback: reset/checkout leave untracked files in place, so a stray file created by a prior candidate could be added into the next PR. Add `git clean -fd` before switching back to main; omit -x so gitignored build artifacts are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c6695e66-ef29-4591-8ca6-fc494164b1ca
Fixes three bad behaviors observed in recent
test-quarantineruns. All changes are to the workflow spec (test-quarantine.md) and its compiledtest-quarantine.lock.yml; both were reviewed to consensus by two code-review agents on different models.B1 — Stray commits leaking across sibling PRs
A change from one candidate could leak into another candidate's PR branch (e.g. #67753 carried an unrelated WebSocketsWorks re-quarantine + revert).
Adds Step 3.0 "Branch hygiene": every PR must start from a clean
main— clean the working tree/index first, thengit checkout -f main(documented thatgit checkout mainbefore cleanup aborts on a dirty tree and would silently strand the agent on the prior branch). Adds a pre-submitgit log main..HEAD/git diff main...HEADself-check so committed stray changes are caught, not just working-tree ones.B2 — Re-quarantining a test a maintainer already rejected
Run 29245029267 re-quarantined WebSocketsWorks despite a maintainer having closed a prior attempt (#67443) asking it not to retry without fresh evidence.
The
closed_quarantine_prspre-activation feed now emitsclosed_at,closed_by, and a precomputedtrusted_closed. The Important-Rules and Case B logic now impose a date cutoff instead of trying to interpret a conditional English comment:startedUtcis strictly after the latest cutoff-establishing close; defer if too few remain.trusted_closedis derived only from token-safe signals: a non-author human closing this bot-authored PR (which necessarily holds triage/write permission — covers public and private org members with zero extra API calls), publicdotnetorg membership, or a trusted-author comment. No fix grows a pre-activation output past the per-env-var limit; the feed still self-caps and fails closed.B3 — Unquarantining a permanently re-quarantined test
Run 29149418882 opened #67740 to unquarantine a test whose issue (#66119) is permanently re-quarantined.
Adds a mandatory pre-PR diff self-check before every unquarantine: for every removed
QuarantinedTestattribute referencing an issue URL — method-, class-, or[assembly: ...]-level — extract the issue number and abort if it appears inrequarantine_issue_numbers. Fails closed if that list is missing.Nothing merges an unrelated file; only the two workflow files change.
gh aw compileis clean (0 errors/warnings) and the pinned action SHAs are unchanged.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com