Add deterministic test quarantine KBE shadow evaluator - #69021
Closed
PureWeen wants to merge 10 commits into
Closed
Conversation
Add a deterministic, read-only evaluator and versioned schemas for validating Runtime-style Known Build Error signatures against hash-pinned quarantine evidence. The evaluator remains fail-closed and never authorizes repository mutations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d5905c51-843f-4d8c-9852-cbbcf77d7723
… KBE
Extends the previously unused shadow evaluator into a working, read-only
shadow feature:
- Collect-TestQuarantineKbeEvidence.ps1: deterministic collector for one
open, canonical test-failure quarantine issue. Parses the issue body,
resolves Azure DevOps build metadata, performs a capped supplementary
recurrence/negative-evidence scan, records Build Analysis check-run
snapshots as corroborating-only evidence, runs a categorized duplicate
KBE/fix-PR search, and emits either a candidate conforming to the
existing (unmodified) candidate schema or a structured "incomplete"
outcome. Never infers a pass, recurrence, or signature from missing or
expired evidence; supports an offline -FixtureRoot mode for fully
deterministic testing.
- test-quarantine-kbe-shadow-dossier.schema.json: new, independently
versioned envelope for the collector's provenance output. Does not
modify or compete with the existing candidate/receipt schemas.
- New-TestQuarantineKbeSummary.ps1: renders a human-readable Markdown
summary of a dossier/receipt pair.
- fixtures/{68724,68947,68945}: compact, sanitized, offline fixtures and
golden expected-dossier.json files for the three documented pilots,
reproducing their real outcomes end-to-end through the unmodified
evaluator.
- Test-Collect-TestQuarantineKbeEvidence.ps1: deterministic, offline test
suite covering the three pilots plus edge cases (closed issue, missing
label, exact Build Analysis test/known-issue match).
- test-quarantine-kbe-shadow.yml: maintainer-triggered workflow_dispatch
workflow with a fork guard, per-issue concurrency, pinned actions,
GITHUB_TOKEN-only auth, and least-privilege permissions
(contents/issues/pull-requests/checks: read). Never mutates issues,
labels, comments, PRs, branches, or files; uploads short-retention
(7 day) dossier/candidate/receipt/summary/evidence artifacts only.
- test-quarantine-kbe-shadow-tests.yml: path-triggered CI running both
PowerShell test suites.
- README.md: documents the trust boundary, Build Analysis vs. raw
evidence, workflow inputs/outputs, retention, non-authorization status,
the path to eventually extracting the deterministic collector already
embedded in test-quarantine.md, and measurable promotion gates.
Does not modify test-quarantine.md, test-quarantine.lock.yml, or any
production quarantine/unquarantine behavior. Does not add a fixer
workflow.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
test-quarantine-kbe-shadow.yml interpolated the workflow_dispatch
`issue_number` and `signature` inputs directly into `run:` script bodies
via `${{ inputs.issue_number }}` / `${{ inputs.signature }}`. A signature
value containing a quote, backtick, or newline could execute arbitrary
commands on the runner.
- Add a "Validate and normalize inputs" step that reads issue_number via
a step `env:` binding, validates it against ^[1-9][0-9]*$, and persists
it to $GITHUB_ENV as ISSUE_NUMBER for later steps.
- Pass signature through a step `env:` binding (SIGNATURE_INPUT) instead
of embedding it in the script; read both via $env:* inside PowerShell,
which never re-parses an environment variable's value as script text.
- Replace every `${{ github.workspace }}` reference inside `run:` bodies
with $env:GITHUB_WORKSPACE (a builtin runner variable) so no `${{ }}`
expression appears inside a run: script body at all.
- Use the validated env.ISSUE_NUMBER (not the raw input) in the uploaded
artifact's name, a non-run: context that Actions substitutes
structurally rather than via a shell.
- Apply the same $env:GITHUB_WORKSPACE fix to
test-quarantine-kbe-shadow-tests.yml.
- Add Test-WorkflowScriptInjectionSafety.ps1: a static, offline test that
parses every run: block in both workflow files and fails if any
contains a `${{ ... }}` GitHub Actions expression. Wired into the CI
tests workflow and documented in README.md.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…llector Independent review found these high-confidence blockers in the read-only collector; all are fixed and covered by new/updated fixtures: 1. GitHub auth: Get-GitHubIssue/Get-CheckRunsForSha/Get-DuplicateSearch sent a literal placeholder Authorization header instead of a real bearer token. Consolidated into one Get-GitHubHeaders helper that sends a real "Bearer $GitHubToken" and logs (non-blocking) the authenticated X-RateLimit-Remaining/-Limit headers after each GitHub call. 2. Recurrence build filter: Azure DevOps' resultFilter does not support a comma-separated multi-value combination (verified live). Query resultFilter=failed and resultFilter=partiallySucceeded separately and merge/dedupe by build id via a new, directly unit-tested Merge-AzdoBuildLists function -- ASP.NET Core failures routinely land in a partiallySucceeded build (aspnetcore#68947's own cited build 1551326 is one, confirmed live). 3. Live VSTMR shape: resultsbyBuild summary rows carry only identity + outcome for ordinary xUnit tests (verified live -- no comment/errorMessage/stackTrace field). Added Get-VstmrSummaryRows (identity/outcome) and Get-VstmrDetail (the detailed per-result endpoint, the authoritative errorMessage/stackTrace source). Helix coordinates are recorded only when a work item's own crash pseudo-test row happens to carry them; otherwise helix_unavailable is recorded explicitly and VSTMR detail text remains authoritative on its own. Dropped the previous (unrealistic) direct Helix console-log fetch path entirely. 4. Multi-test identity: '## Failing Test(s)' can name more than one concrete identity (aspnetcore#68724 names a base test and its server-execution override; live data shows only the override actually failed). The collector now fails closed (multiple-test-identities-unresolved) unless exactly one identity is named, documented as an accepted one-issue/one-root-cause simplification. dotnet#68724's fixture/golden updated to exercise this. 5. Canonical issue validation: the 'test-failure' label alone is not proof of quarantine automation. Now also requires the trusted '<!-- gh-aw-workflow-id: test-quarantine -->' / '<!-- gh-aw-workflow-call-id: dotnet/aspnetcore/test-quarantine -->' marker in the issue body. Added a negative fixture/test. 6. Build Analysis flag precision: exact_test_referenced now requires the full fully-qualified name (a new short_name_referenced field records a bare-method-name-only match separately, non-authoritative). known_issue_referenced now requires a concrete issue number/URL near the phrase (known_issue_numbers records them); a generic heading/table label reading "Known Issue(s)" with no reference no longer sets it. Added fixtures for both precision gates. 7. Duplicate search completeness: recently-closed-kbe/recently-merged-fix-pr queries now carry an explicit 90-day closed:>=/merged:>= window. Search-GitHubIssues paginates up to 300 results and marks a query complete only when incomplete_results=false AND every matching item (per total_count) was actually retrieved. 8. Duplicate search validation: every search hit is fetched and required to contain the exact fully-qualified test name before being treated as a validated existing-kbe/existing-fix-pr reference; otherwise it is recorded as an unvalidated_candidate (new dossier-only field) and never sets duplicate_check.status. Added a negative fixture (same short method name, different test). 9. Repository ref binding: repository_ref.commit_sha is the checked-out HEAD (required by the unmodified evaluator's own self-consistency check), but branch="main" is now only ever asserted after confirming, via a trusted GET /repos/dotnet/aspnetcore/commits/main response, that the checkout actually is main's tip (repository-ref-not-main fails closed otherwise). Recorded in a new provenance.repository_ref_verification block. Added a mismatch fixture. 10. Platform/configuration: previously hardcoded to Linux/Release for every log. Now parsed from the authoritative VSTMR TestRun name (e.g. "Quarantine-Mono-Linux-Release-xunit"); records the literal "unknown" when no recognized token is present, never a fabricated default. 11. Evidence capping: raw evidence text is now constructed with the failed/passed-test marker line and the (already-matched) signature always first, so an 12KB cap can only ever truncate the tail of a long stack trace, never the lines the evaluator's association window needs. 12. Signature matching: replaced `-notlike`/`-like` (which misinterprets literal `*`, `?`, `[` as wildcards) with ordinal, case-sensitive substring containment everywhere a signature is matched against raw evidence text. Added a wildcard-signature fixture with a decoy build that would have spuriously matched under -like semantics. Also fixes a handful of PowerShell pitfalls found while implementing the above: `return @()` collapsing to $null when directly assigned (fixed by consistently wrapping call sites with @(...) rather than mixing conventions), `.PSObject.Properties.Name -contains` throwing under Set-StrictMode for zero-property objects (replaced with a safe Test-HasProperty helper), and `[Parameter(Mandatory=$true)]` collection/null parameters rejecting legitimately-empty/null arguments (added AllowEmptyCollection()/AllowNull() where needed). Does not modify test-quarantine.md, test-quarantine.lock.yml, or any production quarantine/unquarantine behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
Author
|
Closing this in favor of #69039. This PR was a read-only shadow evaluator that tried to reconstruct quarantine evidence retroactively from Azure DevOps. It never got there. All three of its real fixtures (#68724, #68945, #68947) ended up That result is worth something though. It is a fairly strong argument that reconstructing this evidence after the fact does not work, because by the time anyone looks, AzDO test results have aged out and what is left is not enough to prove intermittency in a specific leg. It also drifted into being a second Azure DevOps client, which is the thing we said up front we did not want. So #69039 takes the opposite approach: capture the real error signature at the moment the quarantine issue is created, when the collector already has the failure text in hand. No new AzDO calls, no second client. The useful rules from this work are being carried forward rather than dropped: the build gating (definition 83/87, exact |
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.
Summary
Adds a deterministic, read-only shadow collector and evaluator for deciding whether an ASP.NET Core test-quarantine issue has enough evidence for a Runtime-style Known Build Error signature.
The implementation validates versioned candidate/dossier/receipt schemas, canonical workflow issue provenance, immutable main-dispatch identity, SHA-256-pinned evidence, strict Azure DevOps build provenance, recurrence across distinct builds, Build Insights KBE
ErrorMessage/ErrorPatternsemantics, failed-test association, interleaved pass evidence, exact TestRun-family identity, and duplicate KBE coverage.Evidence model
Build Insightsand app slugbuild-insightswhen app metadata is present.Safety boundary
This PR does not change the production quarantine workflow or enable an automated fix workflow. The shadow path performs no remote writes and always emits:
evidence_provenance_verified: falseeligible_for_kbe_enrichment: falsehuman_review_required: truezero_remote_writes: trueFix-PR search hits remain unvalidated until closing-link and changed-file relevance can be proven.
Pilot results
incompletebecause the issue names multiple distinct test identities.incompletebecause its only authoritative pass predates both collected failures rather than being interleaved.incompletebecause only one failure result remains retrievable and its pass is not interleaved around two failures.Validation
Offline collector/evaluator regressions cover hostile pull-request environment values, Build Insights exact name/app selection, current captured payload shape, historical absence, wrong-name/wrong-app rejection, nullable dashboard URL, TestRun-family normalization, strict failure/pass/failure ordering, environment matching, duplicate compatibility, and script-injection safety. CI also runs actionlint and schema parsing.