fix(ci): Reconcile duplicate automated failure issues - #19805
Ankit Jain (radical) wants to merge 22 commits into
Conversation
7453c09 to
7de7b1c
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19805Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19805" |
This comment has been minimized.
This comment has been minimized.
7de7b1c to
38950f1
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Centralizes automated failure issue reconciliation to consistently select canonical issues, deduplicate occurrences, close duplicates, and support dry-run behavior.
Changes:
- Adds shared reconciliation and GitHub CLI transports.
- Migrates CI reporters and the C# failing-test tool.
- Expands regression tests and documentation.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.agents/skills/automated-failure-issues/SKILL.md |
Documents reconciliation contracts. |
.github/workflows/create-failing-test-issue-tracking.js |
Adds failing-test reconciliation adapter. |
.github/workflows/create-failing-test-issue.js |
Removes legacy search-query logic. |
.github/workflows/create-failing-test-issue.yml |
Delegates issue lifecycle management. |
.github/workflows/monitor-scheduled-workflows.js |
Uses shared live/dry-run reconciliation. |
.github/workflows/report-ci-failure.js |
Reconciles failure and success paths. |
.github/workflows/report-pipeline-failure.js |
Enables duplicate reconciliation. |
.github/workflows/specialized-test-failure-runner.js |
Enables duplicate reconciliation. |
.github/workflows/tracking-issue-gh-api.js |
Adds gh api transport. |
.github/workflows/tracking-issue.js |
Extends the shared planner and transports. |
docs/ci/ci-failure-issues.md |
Documents CI reconciliation. |
docs/ci/monitor-scheduled-workflows.md |
Documents monitor reconciliation. |
docs/ci/pipeline-failure-issues.md |
Documents pipeline reconciliation. |
docs/ci/specialized-test-failure-issues.md |
Documents specialized-test reconciliation. |
tests/Infrastructure.Tests/CreateFailingTestIssue/CreateFailingTestIssueToolTests.cs |
Expands tool regression coverage. |
tests/Infrastructure.Tests/CreateFailingTestIssue/GitHubCliArgumentTests.cs |
Generalizes CLI argument capture. |
tests/Infrastructure.Tests/WorkflowScripts/CreateFailingTestIssueWorkflowTests.cs |
Tests adapter and transport delegation. |
tests/Infrastructure.Tests/WorkflowScripts/MonitorScheduledWorkflowsIntegrationTests.cs |
Tests monitor duplicate handling. |
tests/Infrastructure.Tests/WorkflowScripts/ReportCiFailureIntegrationTests.cs |
Tests CI duplicate reconciliation. |
tests/Infrastructure.Tests/WorkflowScripts/ReportPipelineFailureIntegrationTests.cs |
Tests pipeline duplicate reconciliation. |
tests/Infrastructure.Tests/WorkflowScripts/SpecializedTestFailureRunnerTests.cs |
Tests specialized-runner reconciliation. |
tests/Infrastructure.Tests/WorkflowScripts/TrackingIssueTests.cs |
Tests shared planner behavior. |
tests/Infrastructure.Tests/WorkflowScripts/create-failing-test-issue.harness.js |
Adds adapter/transport test harnesses. |
tests/Infrastructure.Tests/WorkflowScripts/monitor-scheduled-workflows.integration.harness.js |
Improves issue snapshot simulation. |
tests/Infrastructure.Tests/WorkflowScripts/report-ci-failure.integration.harness.js |
Captures reporter logs. |
tests/Infrastructure.Tests/WorkflowScripts/report-pipeline-failure.integration.harness.js |
Exposes state reasons. |
tests/Infrastructure.Tests/WorkflowScripts/specialized-test-failure-runner.harness.js |
Exposes state reasons. |
tests/Infrastructure.Tests/WorkflowScripts/tracking-issue.harness.js |
Supports new planner options. |
tools/Aspire.TestTools/GitHubCli.cs |
Removes superseded issue mutation methods. |
tools/CreateFailingTestIssue/CreateFailingTestIssue.csproj |
Copies JavaScript adapters to output. |
tools/CreateFailingTestIssue/FailingTestIssueCommand.cs |
Invokes the reconciliation adapter. |
38950f1 to
8f65460
Compare
This comment has been minimized.
This comment has been minimized.
8f65460 to
cd9dd14
Compare
This comment has been minimized.
This comment has been minimized.
|
[automated] Addressed the suppressed polling-window inventory finding from review 5073162001 in 7d80551. The monitor now refreshes the selected transport's all-state issue inventory after each reconciliation, so a creator race cannot leave the next failure using the original empty snapshot. The regression processes two failures after concurrent canonical convergence and verifies one create, closure of the throwaway duplicate, and both run markers on the canonical issue. |
There was a problem hiding this comment.
🟡 Changes recommended
Cause publishing can duplicate an occurrence across matching issues and does not ensure its lookup label.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/analyze-ci-failure-cause-issues.js:164
CAUSE_LABELis the lookup/creation label, butensureCauseLabelsonly createsmain-ci-break; this producer therefore depends on the base label having been provisioned out of band and cannot recover if it is missing. Ensureci-failure-causebefore reconciliation, matching the established producer pattern inreport-ci-failure.js:93,report-pipeline-failure.js:86, andspecialized-test-failure-runner.js:68.
- Files reviewed: 49/50 changed files
- Comments generated: 1
- Review effort level: Balanced
| actionsForCanonical: (issue, { created }) => { | ||
| if (created || hasOccurrence(issue.body, run.runId)) { | ||
| return []; | ||
| } | ||
| return [{ | ||
| type: 'update', | ||
| body: `${(issue.body ?? '').trimEnd()}\n${occurrenceRow(cause, run)}\n`, | ||
| }]; | ||
| }, |
Failed-test summaries could persist agent-provided job labels that did not match trusted failed-job metadata. Cause validation also checked only cause-to-job compatibility, allowing a non-code failed job to remain unrepresented by any matching recurring cause. Keep a failed-test job name only when it exactly matches a trusted failed job. Track cause job IDs by cause type and require every transient, flaky, and main-breakage job to be covered by its corresponding cause, while preserving evidence-backed flaky causes on deterministic jobs. Tests cover exact, forged, and prefix near-match job names; uncovered infrastructure, flaky, and main-breakage jobs; and both same-job mixed scopes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b364edcb-a6a1-48a6-aedb-011cda75c167
Copilot review flagged four issues in the CI-failure analysis workflow: a query-string injection risk in the PR-number lookup, an untested run-attempt-advanced rerun guard, missing job_ids trust/coverage validation on rerun causes, and two rerun-guard branches (dry-run, closed-PR) that the test harness could never exercise. Resolve the PR-number lookup with `gh api --method GET ... -f key=value` instead of a concatenated query string, so branch names containing query delimiters cannot alter the request. Make the rerun test harness honor request-supplied run_attempt, enableRerun, and PR state instead of hardcoding them, so the advanced-attempt, disabled-rerun, and closed-PR skip paths are covered. Validate that each rerun cause's job_ids are positive, unique, and drawn only from the run's trusted failed jobs, and that the union of all causes' job_ids covers every trusted failed job, mirroring the equivalent check already used by the publish-side validator. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b364edcb-a6a1-48a6-aedb-011cda75c167
CI failure analysis trusted agent-proposed cause IDs, so equivalent failures could create separate memory records and GitHub issues. It also attributed every cause occurrence to the first failed job, hiding which job actually produced each cause. Resolve proposed IDs against canonical memory, explicit aliases, configured retry-pattern IDs, and per-cause job metadata before publishing. Serialize publication and persist canonical identities before issue side effects so concurrent runs cannot create duplicates. Preserve the lower layer's trusted run scope and exact failed-job-set validation, including main-repository-breakage causes. Split PR comments from publication to stay below GitHub's expression-size limit without masking resolver failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
Recurring failure issue publication was embedded in workflow shell, which made canonical selection, race convergence, and duplicate handling hard to reuse or exercise outside the live workflow. Alias redirects and job attribution could also lose the issue or trusted job identity. Move deterministic reconciliation into a pure planner with an injectable transport executor, and make recordRun compose through it. Add a cause-specific adapter for trusted rendering and memory links while the workflow only validates inputs and orchestrates persistence. Resolve cause aliases and job names against trusted data, preserve existing issue identities across redirects, and cover create races, reopen and update behavior, duplicate closure, dry-run parity, and workflow delegation in the Infrastructure test harnesses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
Canonical cause reconciliation could trust agent-authored job names, accept invalid marker IDs, collide aliases after normalization, or associate a cause with a job whose classification contradicted its type. REST-shaped comment counts, replayed runs, and duplicate-exempt issues also exposed gaps in deterministic issue selection and idempotence. Validate canonical IDs and trusted job attribution at each publication and rerun boundary. Hydrate issue comments before marker planning, fail closed on conflicting identities, and preserve aliases and migrations across normalized retries and replay. Coalesce fresh flaky-test causes with the same normalized test identity, remap every analysis reference deterministically, and reopen only the canonical non-exempt issue when a new occurrence is recorded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
Duplicate issue mutations could run before the canonical issue update completed, leaving partially reconciled state when a canonical action failed. Compatible same-test failures could also remain split across historical roots or fresh proposals. Run canonical actions before duplicate notices and closures. Converge compatible historical roots on the oldest canonical cause, preserve their issue markers and aliases, and absorb fresh same-test proposals into their authoritative owner. Keep cause attribution aligned with trusted job classifications while allowing a flaky cause on a deterministic job only when validated flaky-test evidence names that same trusted job. Cover failure-and-resume ordering, deterministic historical convergence, authoritative absorption, alias conflicts, canonical issue selection, and the same-job mixed contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b364edcb-a6a1-48a6-aedb-011cda75c167
Automated workflows could split one failure across multiple issues when creators raced or a run replayed. Lookup depended on mutable issue state and broad matching, so comments and closure state could diverge. Reconcile strongly consistent all-state label listings with exact versioned markers, oldest-issue canonicalization, cross-duplicate run deduplication, and opt-in idempotent duplicate closure. Preserve force-created exemptions and stable normalized failing-test identities. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Automated failure reporters could split a replayed run across duplicate issues because producers used independent Search and direct-create paths whose results depended on mutable issue state. Route failing-test creation, trusted close, and dry-run through the shared tracking planner, executor, and transports. With duplicate reconciliation enabled, exact versioned all-state markers keep the oldest match canonical, deduplicate run comments, and close newer matches as not_planned. Preserve force-new exemptions and the stable normalized XxHash3 identity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
The scheduled monitor could report an opted-out canonical issue as closed when reconciliation only closed a newer duplicate. It treated any close action in the shared plan as a canonical close and updated the wrong cached issue. Track applied close actions by issue number and report a canonical close only when that issue was actually closed. Strengthen adapter coverage for label ordering and the existing-label response. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Dry-run monitoring cloned the comments field returned by the issue-list API as though it contained comment bodies, but Octokit exposes an integer count. Labeled issues with existing comments therefore failed before reconciliation could hydrate their comments through the comments endpoint. Treat only comment arrays as hydrated and defer REST-shaped counts to the shared transport. Normalize missing canonical issue numbers to null and log an explicit no-op so green runs without a matching issue never report #undefined. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Scheduled-workflow success previews only reported a canonical close even when shared reconciliation also closed newer duplicate issues. If the canonical issue was already closed, the dry run emitted no actionable line for the duplicate closure. Report each planned duplicate closure with the same wording used by failure previews, then retain the canonical close output. This keeps dry-run output faithful to the shared plan without mutating GitHub state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Shared reconciliation classified every applied close action as a duplicate closure, including a canonical issue closed as completed. Success dry-run previews therefore emitted a self-referential not_planned duplicate line before the correct canonical close. Exclude the canonical issue number from duplicatesClosed and pin that contract in the shared engine and monitor integration tests. Consumers now receive only actual duplicate closures and previews match live behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Dry-run-created issues lost their synthetic placeholder identity, so later preview actions exposed issue #0. Trusted success reconciliation also used the watchdog's run-start snapshot, allowing an intervening user closure or removed autoclose stamp to be overwritten. Moving reconciliation from GitHubCli to the Node adapter also dropped the five-minute process ceiling, allowing a hung adapter to block indefinitely. Keep dry-run placeholders non-overridable, re-list through the selected transport immediately before success planning, and restore a linked five-minute adapter budget with process-tree cleanup. Document the accepted paginated JSON shapes and trusted-close refresh behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6a91e656-e33e-4d36-8b47-eed90ad7d144
The automated-failure skill required a test-driven-development sub-skill that is not provided by the repository. Agents without that external skill could not satisfy the dependency even though this skill already defines a stricter test-first contract. Point behavior changes to the local Required Tests section so the checked-in skill remains portable while preserving its existing coverage requirements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Concurrent failing-test issue requests can read the same comment inventory before either writes, causing both to append the same run marker. Replayed runs can also reopen a canonical issue and close duplicates while standalone diagnostics report only that the occurrence was skipped. Serialize workflow reconciliation through one non-canceling retained queue. Log reconciliation mutations independently from occurrence deduplication, document the standalone Node.js dependency, and clarify when scheduled success handling refreshes issue inventory. Tests pin the workflow serialization contract and the skipped-run reconciliation diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
CreateFailingTestIssue is built and exercised by Infrastructure.Tests, but the dependency is invisible to the project graph because the tests launch a separate tool build rather than referencing its project. Tool- only changes therefore selected no regression tests. Route the tool directory explicitly to Infrastructure.Tests and pin the real trigger map to that exact consumer without falling back to ALL. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Every created issue comment entered the workflow-wide reconciliation queue before command eligibility and repository permission checks. That let unrelated traffic consume the bounded queue even though only authorized /create-issue commands mutate failing-test issues. Move eligibility and authorization into an ungrouped gate job, then serialize only the downstream reconciliation job. Keep the constant group so eligible writers cannot race occurrence-marker updates. Post-create convergence can also retain the newly created issue while closing a newer concurrent duplicate. Report those duplicate closures independently, and do not claim a run was recorded when the occurrence already existed on the duplicate. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3388c16-c87a-485b-8a46-37c99401e014
Replayed failing-test runs that made no mutation still reported an existing issue as updated. The scheduled-workflow monitor also retained its pre-reconciliation issue snapshot, so a creator race followed by a second failure in the same polling window could create another throwaway duplicate. Report a no-op replay as finding the existing issue and reserve the updated message for an applied comment, reopen, or duplicate closure. Refresh the selected transport's all-state inventory after each reconciliation so live and dry-run processing share canonical issue state across the polling window. Tests cover open and closed no-op replays, mutating skipped reconciliation, and two failures after concurrent creator convergence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b364edcb-a6a1-48a6-aedb-011cda75c167
7d80551 to
8ee51b4
Compare
Tests selector1 / 99 PR test projects · 0 PR jobs · 0 advisory-only targets, from 29 changed files. Selected PR test projects (1 / 99)
Selected PR jobs (0)none Advisory workflow impact (0)none How these were chosen — grouped by what changed📄 📄 📄 📄 📄 📄 📄 📄 📄 📄 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 📄 📄 Job reasonsnone Selection computed for commit |
There was a problem hiding this comment.
🟡 Changes recommended
Auto-close can permanently lose its success comment when the comment mutation fails after closure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 35/35 changed files
- Comments generated: 2
- Review effort level: Balanced
| ? [ | ||
| { type: 'close', stateReason: 'completed' }, | ||
| { type: 'comment', body: closeComment }, | ||
| ] |
| { type: 'close', stateReason: 'completed' }, | ||
| { | ||
| type: 'comment', | ||
| body: `CI is green again on \`${ref}\` ([run #${run.runNumber}](${run.runUrl})). Closing automatically.`, | ||
| }, |
[automated] Concurrent reporters and replayed runs could split one automated failure across multiple GitHub issues or record the same occurrence more than once. Canonical selection and duplicate handling depended on which mutable issue a workflow or the C# failing-test tool observed first.
Root cause: Failure producers implemented their own lookup, comment, reopen, close, and dry-run behavior. Issue-list responses expose comment counts rather than comment bodies, creation races were not followed by a canonical relist, and close decisions could use stale issue state.
The fix: Route CI, pipeline, specialized-test, scheduled-workflow, and failing-test producers through one exact-marker reconciliation planner and shared live/dry-run transports. All-state label listings keep the oldest exact match canonical, hydrate comments before marker decisions, relist after creation so races converge, deduplicate occurrence comments across every match, and optionally close newer duplicates as
not_planned.Force-created issues remain exempt from duplicate reconciliation. Trusted auto-close policy is evaluated against refreshed state, and dry-run reports the same canonical and duplicate actions without mutating GitHub. Missing matches remain explicit no-ops rather than producing
#undefinedoutput.The C# failing-test command now delegates issue lifecycle to the checked-in adapter while retaining its normalized XxHash3 identity contract. The repository skill and CI documentation define the shared identity, migration, replay, duplicate, and trusted-close contracts for future producers.
Regression coverage: Eight affected Infrastructure test classes pass 159 focused cases covering exact identity, REST comment hydration, create races, replay idempotence, force-new exemptions, canonical and duplicate closure results, dry-run parity, trusted auto-close, GitHub API transport behavior, and the C# adapter.