fix(ci): Reconcile duplicate automated failure issues - #19805
fix(ci): Reconcile duplicate automated failure issues#19805Ankit Jain (radical) wants to merge 11 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 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
cd9dd14 to
924fcf2
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/tracking-issue.js:474
reconcileRunstill performs occurrence dedup as a non-atomic list-comments-then-create-comment sequence. Two callers for the same run can both read no<!-- run:... -->marker and both post it; the post-create relist only converges issue selection, not comment insertion. This is reachable because manual dispatches use a per-run concurrency group and the standalone tool can overlap. Serialize reconciliation by stable failure identity or add cross-process idempotency for occurrence recording so concurrent reporters cannot duplicate the same occurrence.
async function reconcileRun(
transport,
core,
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
This comment has been minimized.
This comment has been minimized.
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
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
docs/ci/specialized-test-failure-issues.md:92
- This overstates reconciliation for closed duplicates:
planIssueReconciliationskips a duplicate when its state is alreadyclosed, so it is neither linked nor rewritten tonot_planned. Qualify this as “newer open exact duplicates,” matching the implementation and the other updated CI docs.
plus a local exact-marker filter, mirroring the scanner. The oldest match is
canonical even when closed; newer exact duplicates are linked and closed as
`not_planned`. The markers use a distinct
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/create-failing-test-issue.yml:37
- This workflow is triggered for every new issue/PR comment, and workflow-level concurrency is acquired before the job-level
/create-issuecondition is evaluated. The constantqueue: maxgroup therefore lets ordinary public comments fill or delay the shared queue, potentially canceling or starving legitimate commands once the 100-entry queue is full. Move this concurrency block ontocreate_failing_test_issueso only command jobs participate in serialization.
concurrency:
# Issue comments cannot atomically reserve occurrence markers, so serialize writers.
group: create-failing-test-issue
cancel-in-progress: false
queue: max
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
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.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/monitor-scheduled-workflows.js:257
- After a create race, the executor's relist is internal to
reconcileRun; this outerissuesarray remains the original no-match snapshot. If the polling window contains another failure (for example, the hourlydeployment-cleanup.ymlcan have several), the next iteration passes the same empty snapshot and creates another throwaway duplicate before reconciling again. Propagate the relisted inventory back to this loop, or avoid supplying the cached inventory for live reconciliation so subsequent failures select the canonical issue directly.
const result = await tracking.reconcileRun(transport, core, {
label, labels: issueLabels, marker, title: buildIssueTitle(wf.name),
runId: latest.id,
buildBody: () => issueBody,
comment, issues,
closeDuplicates: true,
| else | ||
| { | ||
| Log($"Updated existing issue #{result.Number}: {result.Url}"); | ||
| } |
[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.