Flaky-test workflow: reassure on empty PR list + drop local reproduction (quarantine-first) - #13938
Merged
Conversation
Run 26883332898 (otherwise a clean two-run-model success: quarantined 4 pre-existing tracked tests in one PR, 2.85M effective tokens, no firewall blocks) wasted ~8 tool calls when the Step 5 dedup query 'gh pr list --label flaky-test --json number,body' correctly returned an empty array. With no flaky-test PR in flight, [] is the expected result, but the agent read it as a broken command -- re-running it, inspecting the file with xxd/wc, toggling 2>/dev/null, and finally switching to gh api -- partly because open flaky-test *issues* (the tracking issues) did exist and it conflated issues with PRs. Add a note to Step 5 that an empty [] is the normal case (no cross-run duplicates to skip), must not be treated as a tool failure, and that 'gh pr list --label flaky-test' lists only PRs -- so returning [] while flaky-test-labeled issues exist is correct, not a bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the flaky-test-detector agent workflow instructions to explicitly document that Step 5’s cross-run dedup query can legitimately return an empty JSON array ([]) when there are no open flaky-test PRs, and that this should not be treated as a tool failure.
Changes:
- Add explicit guidance that
[]fromgh pr list --label flaky-test --json ...is the expected common case. - Clarify that
gh pr listonly returns pull requests, so it can be empty even whenflaky-test-labeled issues exist. - Instruct the agent not to waste tool calls re-running/validating the empty result via
xxd/wc/stderr tweaks/gh api.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/flaky-test-detector.agent.md | Adds Step 5 documentation clarifying empty PR dedup output ([]) is normal and should not trigger retries or alternative queries. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…build validation Local reproduction on the single Linux/.NET agent runner is unreliable: it cannot run Windows-only or net472-only tests, and isolated single-method loops miss the ordering/shared-state/parallel-contention flakes that dominate. The scheduled quarantine pipeline (AzDO def 344) already re-runs quarantined tests on Windows/Linux/macOS twice daily to gather repeat-failure signal over time, so in-workflow reproduction is redundant. - Remove the up-front whole-repo build, the parallel 25x reproduction screening, and the determinism-fix authoring (old Steps 6/7a/7b/7c). - New flakes are now simply quarantined with [ActiveIssue]; backlog tests still flaking in def 344 stay quarantined (no local fix attempt); only consistently-green backlog tests are un-quarantined. - Move ./build.sh to a post-edit validation step that only checks the [ActiveIssue] edits compile; on an environmental/network build failure the mechanical edits are low-risk so the draft PR is opened anyway (its CI is the first real compile), while a genuine compile error reverts the edit. - Strengthen the Step 8 diff gate to require each changed path be a selected test's source file (the workflow never edits product code now). - Lower timeout 120->60 min and refresh frontmatter description/comments; recompile the lock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ViktorHofer
changed the title
Adds a new scheduled gh-aw workflow (flaky-test-fixer) that proposes test-only fixes for tests that are already quarantined ([ActiveIssue]) but still flaking in the quarantine pipeline (AzDO definition 344). It diagnoses root causes from accumulated def-344 failure evidence (no local reproduction), keeps the [ActiveIssue] in place, and opens one individual draft PR per confidently-fixable test. The separate detector workflow un-quarantines once def 344 proves the test green. Also adds a gated -IncludeErrorDetails switch to Get-FlakyTests.ps1 that emits per-test errorSamples (distinct failure signatures with message + stack excerpts) for diagnosis. The switch is additive and default-off, so the detector's output shape is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Run the auto-fixer at 12:38 UTC, ~1 hour after the detector's 11:38 UTC run, so it sees the detector's latest quarantine/un-quarantine state before proposing fixes. Use an explicit cron (not 'daily') so the time stays fixed and is not re-scattered on recompile. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename the detector workflow "Flaky Test Triage and Fix" -> "Flaky Test Triage" since authoring fixes moved to the separate flaky-test-fixer workflow. Update the body to state the detector only quarantines/ un-quarantines and to reference flaky-test-fixer.agent.md as the place a determinism fix is proposed (from accumulated def-344 evidence). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable the quarantine re-validation pipeline (def 344) on pull requests targeting main, in addition to its twice-daily schedule. This lets a fix PR's effect on the quarantined tests be validated directly on the PR (the quarantined tests are excluded from normal CI). Still never runs on CI pushes (trigger: none). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that the quarantine pipeline (def 344) also runs on PRs targeting main, a PR build runs the quarantined tests against unmerged changes and can pass incidentally (or via an in-flight fix). Counting those greens toward un-quarantine would re-enable a test on main before the responsible change is actually merged, re-disrupting CI. Get-FlakyTests.ps1 -IncludePassed now tags each passed observation with its source type and bases the un-quarantine signal fields (distinctBuilds/buildIds/distinctDays/legs/tfms, totalPassed) on scheduled-main (rolling) observations ONLY. PR greens are surfaced as a separate informational prDistinctBuilds; a test seen green solely on PR builds is dropped from passedTests so it neither drives an un-quarantine nor looks 'trending green' to the fixer. Body-only doc updates in both workflows clarify that passedTests green counts are scheduled-main only (runtime-imported; no recompile needed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The quarantine pipeline now triggers on main's rolling (batched) builds for fresh per-commit signal, keeps a once-daily schedule as a fallback for quiet periods, and continues to run on PRs targeting main. Consumers already treat rolling and scheduled main runs identically (reasons batchedCI/individualCI/ schedule), so existing scheduled data stays valid. Updated detector and fixer body wording and Get-FlakyTests.ps1 comments to describe the un-quarantine signal as main-branch (rolling + scheduled) rather than scheduled-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… fix PR The auto-fixer now has two tiers. By default it keeps the [ActiveIssue] quarantine as before (candidate fix validated by def 344 after merge). When confidence is very high -- a single dominant signature explaining essentially all current def-344 failures, a deterministic textbook root cause fully removed by a minimal test-only change, full-scope PR-CI coverage, and a cleanly removable attribute (new Step 5b) -- it also deletes the single [ActiveIssue] line in the same file, so normal PR CI runs the test as additional pre-merge validation. The PR body states which variant applies; the tracking issue is still never closed. Updated the frontmatter description, intro invariant, overall shape, Step 5 forbidden list, new Step 5b, Step 6, the PR-body caveat (conditional) and markers, and the Important section accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…idence un-quarantine Make the quality/understanding bar explicit in Step 5b: the agent must be able to state the exact causal chain from its change to why the dominant failure signature can no longer occur, target that mechanism (not a symptom or mask), and preserve the test's coverage. A category-match or a 'should help' rationale is not sufficient to also remove the [ActiveIssue]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ViktorHofer
added a commit
that referenced
this pull request
Jun 5, 2026
…es section from PR body (#13958) Follow-up to #13938, addressing two pieces of feedback on how the flaky-test **detector** opens its combined quarantine/un-quarantine PR. ### 1. Open quarantine/un-quarantine PRs ready-for-review (not draft) The detector previously opened its PR as a draft (`draft: true`). dnceng AzDO build validation does not run on draft PRs, so CI only kicked in once a maintainer marked the PR ready for review (observed on #13952). Since these PRs are mechanical `[ActiveIssue]` add/removes that benefit from immediate CI validation, switch to `draft: false` so CI runs as soon as the PR is opened. ### 2. Don't list newly-filed tracking issues in the PR body On an un-quarantine PR (#13952), the agent appended a "New flaky test issues filed this run" section referencing freshly-filed issues (e.g. #13949-#13951). Those issues are **not** acted on by the PR (they only become quarantine-eligible on a later run), so the `#number` references created a misleading issue<->PR cross-link. Step 8 now explicitly instructs the agent to reference only the issues for tests it actually quarantines or un-quarantines in that PR. Frontmatter changed (`draft` + `description`), so the lock file was regenerated; cron is preserved at `38 11`. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 6, 2026
This was referenced Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improvements to the scheduled Flaky Test Triage and Fix agentic workflow, batched onto one PR.
1. Empty open-flaky-PR list is normal (original)
Reassure the agent that
gh pr list --label flaky-testreturning[]is the expected case (no flaky-test PR in flight), not a tool failure to re-investigate.2. Stop local flaky-test reproduction — quarantine-first
Local reproduction on the single Linux/.NET agent runner is structurally unreliable: it cannot run Windows-only or
net472-only tests, and isolated single-method loops miss the ordering/shared-state/parallel-contention flakes that dominate. The scheduled quarantine pipeline (AzDO definition 344,azure-pipelines/quarantine.yml) already re-runs quarantined tests on Windows/Linux/macOS twice daily to gather repeat-failure signal over time, so in-workflow reproduction is redundant.[ActiveIssue]. Backlog tests still flaking in def 344 stay quarantined (no local fix attempt). Only consistently-green backlog tests are un-quarantined../build.shmoves to a post-edit validation step that only confirms the[ActiveIssue]edits compile. On an environmental/network build failure the mechanical edits are low-risk, so the draft PR is opened anyway (its CI is the first real compile); a genuineerror CS...reverts the offending edit.timeout-minutes120 → 60, refresh the frontmatter description/comments, and recompile the lock.3. Add a separate evidence-based auto-fixer workflow
A new scheduled workflow
.github/workflows/flaky-test-fixer.agent.md(compiled to…/flaky-test-fixer.agent.lock.yml) complements the detector. Where the detector quarantines/un-quarantines, the fixer proposes test-only fixes for tests that are already[ActiveIssue]-quarantined but still flaking in def 344.[ActiveIssue]in place (the PR is a candidate fix; def 344 validates it onmainover the following days, then the detector un-quarantines once green). It never closes the tracking issue (Tracked by #N, neverFixes)..csfile.allowed-filesis scoped to test projects (*UnitTests*/*.Tests) and.github/**is excluded, so product code can never be touched. A forbidden-changes list blocks weakening assertions, sleep/retry/timeout-bump-only fixes, swallowing exceptions, etc.flaky-testPRs (marker + issue number + edited file) so it never collides with the detector or its own prior runs. The whole repo is built once (./build.sh) to validate the fixes compile before opening PRs.Supporting change:
Get-FlakyTests.ps1gains a gated, default-off-IncludeErrorDetailsswitch that emits per-testerrorSamples(distinct failure signatures with truncated message + source-frame-preferring stack excerpt) for diagnosis. Additive only — the detector's output shape is unchanged.