fix(ci): Prevent duplicate recurring CI failure issues - #19804
fix(ci): Prevent duplicate recurring CI failure issues#19804Ankit Jain (radical) wants to merge 14 commits into
Conversation
8c7bee6 to
b034867
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19804Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19804" |
This comment has been minimized.
This comment has been minimized.
b034867 to
952a3f1
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds deterministic CI-failure cause resolution to prevent duplicate memory records/issues and improve job attribution.
Changes:
- Canonicalizes proposed cause IDs using prior causes, test names, aliases, retry patterns, and matchers.
- Serializes publication and records per-cause job associations.
- Adds resolver, workflow, and retry-pattern coverage.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/analyze-ci-failure-cause-resolver.js |
Implements cause canonicalization and attribution. |
.github/workflows/analyze-ci-failure.md |
Integrates resolver and serializes publication. |
.github/workflows/analyze-ci-failure.lock.yml |
Updates generated workflow. |
.github/workflows/auto-rerun-transient-ci-failures.js |
Validates optional canonical cause IDs. |
eng/test-retry-patterns.json |
Assigns the Windows initialization cause ID. |
docs/ci/auto-rerun-transient-ci-failures.md |
Documents causeId usage. |
AnalyzeCiFailureCauseResolverTests.cs |
Adds resolver and workflow tests. |
analyze-ci-failure-cause-resolver.harness.js |
Provides the Node test harness. |
AutoRerunTransientCiFailuresTests.cs |
Tests retry-pattern validation. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d591fdc to
68cf3ee
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
.github/workflows/analyze-ci-failure-cause-resolver.js:416
- This combines output evidence from all jobs with
jobNames.some(...), so a two-field job pattern can match across different jobs. For example, a cause spanning Windows and Linux is canonicalized by a{ jobName: "windows", output: "LINUX_ONLY_TOKEN" }rule when only the Linux test contains that token. Job-level retry rules require both matchers to hold for the same job; build evidence per job and accept a rule only when one job satisfies all of its matchers, otherwise unrelated causes can be merged under one canonical ID.
.filter(pattern => !pattern.output || matchesConfiguredPattern(pattern.output, evidence))
.filter(pattern => !pattern.jobName || jobNames.some(jobName => matchesConfiguredPattern(pattern.jobName, jobName)))
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Alias-chain handling and failed canonical updates can still leave duplicate issues unresolved or incorrectly closed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.github/workflows/analyze-ci-failure-cause-resolver.js:207
- Transitive alias records are discovered in
canonicalPriorRecords, but their IDs are not added to the emittedaliasesunless this is the special same-test normalization path. For a chain such asleaf -> intermediate -> root, resolvingleafemits onlyleaf; an existing issue whose marker isintermediateis therefore invisible to reconciliation and can remain as a duplicate. Include every record in the canonical family (and its marker aliases) in this list.
const aliases = unique([
...(canonicalPriorCause?.aliases ?? []),
...(proposedAlias && proposedPriorCause.id !== canonicalId ? [proposedPriorCause.id] : []),
...(priorCauseId && priorCauseId !== canonicalId ? [priorCauseId] : []),
...(supersededPriorCause
? [supersededPriorCause.id, ...(supersededPriorCause.aliases ?? [])]
: []),
...normalizedLegacyRecords.map(record => record.id),
...compatibleNormalizedRecords.flatMap(record => [
record.id,
...(record.aliases ?? []),
]),
...sameTestExtraAliases,
]).filter(alias => alias !== canonicalId);
.github/workflows/analyze-ci-failure-cause-issues.js:508
- Returning no update after this render failure does not stop duplicate reconciliation. In the post-create relist case, the newly created issue contains the occurrence, but the planner still comments on and closes it while the selected older canonical never receives the occurrence. Keep duplicate closure disabled/aborted when the canonical occurrence update cannot be produced, so the only issue containing the run is not closed as successfully reconciled.
core.warning(
`Issue #${issue.number} has an unsupported occurrence section: ${error.message}. Skipping occurrence update.`);
return issue.state === 'closed' ? [{ type: 'reopen' }] : [];
- Files reviewed: 17/18 changed files
- Comments generated: 1
- Review effort level: Balanced
Compatible normalized-ID convergence remapped an entire legacy alias family, but only exempted the root from the normalized collision guard. A transitive alias that normalized to the same safe ID still aborted the resolver. Mark every remapped family member as compatible. The guard still requires each alias to normalize exactly to its canonical target, so unrelated and mismatched-type collisions remain fail-closed. Extend the filesystem regression with a colliding transitive alias and verify that metadata, occurrences, the physical canonical file, and replay idempotency are preserved. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Canonical-update failures can still close duplicates, and selected canonical families can omit reverse aliases and create duplicate issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/tracking-issue.js:97
- This sentence is grammatically broken: “
isMatchingIssueis an Identity extension points…”. Rewrite it as a single description of the identity extension points.
.github/workflows/analyze-ci-failure-cause-issues.js:508
- If occurrence rendering fails here, returning no canonical action still allows the reconciliation plan to comment on and close every duplicate. In the post-create race covered by
CreateRelistDoesNotReceiptOccurrenceWhenOlderCanonicalCannotBeUpdated, this closes the newly created issue—the only issue containing the new occurrence—even though the older canonical was not updated. Abort reconciliation or explicitly suppress duplicate mutations when the canonical occurrence cannot be written, so the operation remains resumable without partially reconciling duplicates.
core.warning(
`Issue #${issue.number} has an unsupported occurrence section: ${error.message}. Skipping occurrence update.`);
return issue.state === 'closed' ? [{ type: 'reopen' }] : [];
- Files reviewed: 17/18 changed files
- Comments generated: 1
- Review effort level: Balanced
Historical reverse aliases were not included in ordinary cause resolution, so an existing issue discoverable only through an older marker could be duplicated. A malformed canonical occurrence section could also allow duplicate comments and closures before the canonical issue was safely updated. Main breakage issues still exposed agent-proposed titles and diagnostics. Merge the finalized lower-layer validation and publisher contracts while retaining the JavaScript issue lifecycle implementation. Fold the complete compatible alias family into issue discovery, and suppress duplicate reconciliation when the canonical occurrence update cannot be rendered while allowing unrelated causes to continue. Render and migrate main breakage issues from trusted run context while preserving operator notes and occurrence history. Tests cover reverse alias ordering, create/relist races, malformed occurrence sections, and main issue migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
|
[automated] Addressed the two suppressed findings from the latest Copilot review in 5dc0d1b. If a canonical occurrence section cannot be rendered safely, publication now warns and suppresses duplicate comments and closures for that cause. This prevents a create/relist race from closing the newly created issue that contains the only published occurrence, while unrelated causes can continue. The malformed identity-callback comment in |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Main-breakage fallback updates can incorrectly record unpublished occurrences, while migration errors can abort unrelated cause publication.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 17/18 changed files
- Comments generated: 2
- Review effort level: Balanced
Main issue migration failures could abort publication after the occurrence body was already rendered, preventing later causes from being processed. A title-only fallback update was also treated as a successful occurrence publication, permanently suppressing the missing row on later retries. Keep the bounded occurrence update when main issue migration cannot safely rewrite legacy content, while suppressing duplicate reconciliation for that cause. Persist the durable publication receipt only when the canonical issue was created, already contained the occurrence, or accepted a body update that contains it. Tests cover malformed main issue migration, duplicate safety, later-cause continuation, and title-only fallback retries. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🟡 Changes recommended
Unsanitized multiline job names can create malformed occurrence rows that are subsequently treated as published.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 17/18 changed files
- Comments generated: 1
- Review effort level: Balanced
Workflow job names can contain line breaks and control characters. The validation step sanitized only a temporary copy, so the resolver could pass the original API value to the issue renderer and split a managed occurrence row across multiple lines. Apply the existing single-line, 500-character contract before escaping and rendering every job name. This keeps new and updated occurrence rows parseable and preserves replay idempotence. Tests cover creation, updates, control characters, marker-like text, and repeated publication. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
🔵 Needs a closer look
Retry matching can misassociate multi-job causes, and malformed occurrence histories can create false publication receipts.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
.github/workflows/analyze-ci-failure-cause-issues.js:446
- This inference also succeeds for an empty or duplicate rendered row set because
every([])is true and repeated IDs are never rejected. For example, a body sayingShowing 0 most recent of 3 occurrences.(or containing run 100 twice) with three stored occurrences causes any omitted run to be markedissue_published, so it will never be appended after the body is repaired. Require at least one rendered row and distinct rendered run IDs before using the total-count migration heuristic.
.github/workflows/analyze-ci-failure-cause-resolver.js:633 - Retry-rule fields are no longer evaluated against one failed job as the retry configuration requires.
outputis matched against aggregate agent-authored cause/test text, whilejobNameis matched independently against any trusted job name. For a multi-job cause, one job can satisfyjobNameand another job's diagnostics can satisfyoutput; the configuredcauseIdis then applied to the whole cause even though no job actually matched the rule. It also lets an agent-suppliederror_patternmanufacture the output match. Evaluate each rule against the trusted log/evidence for each referenced job and accept it only when the same job satisfies every configured field.
- Files reviewed: 17/18 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Malformed occurrence histories could mark omitted runs as published when no distinct visible rows proved they had reached the canonical issue. Retry-pattern cause IDs could also be assigned from agent-authored text or from job-name and output matches belonging to different jobs. Require legacy publication inference to use a non-empty, distinct, and internally consistent visible occurrence set. Preserve a bounded trusted log tail for each successfully downloaded failed job, and require one referenced job to satisfy every configured retry-pattern field. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68afc81c-eaf6-4e13-a0e8-9528d8824b0c
|
[automated] Addressed both suppressed findings in 13f3c7d.
The affected five-class Infrastructure test matrix passes 481/481, and the generated workflow compiles successfully. |
Tests selector1 / 99 PR test projects · 0 PR jobs · 0 advisory-only targets, from 17 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
Same-type current causes can incorrectly merge when an unrelated proposal collides with a trusted canonical ID.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 17/18 changed files
- Comments generated: 1
- Review effort level: Balanced
| const existing = normalizedById.get(canonicalId); | ||
| if (existing && existing.type !== normalizedCause.type) { | ||
| throw new Error( | ||
| `Canonical cause '${canonicalId}' cannot merge current causes with types ` + | ||
| `'${existing.type}' and '${normalizedCause.type}'.`); | ||
| } | ||
| normalizedById.set( | ||
| canonicalId, | ||
| existing ? mergeCurrentCauses(existing, normalizedCause) : normalizedCause); |
[automated] The same recurring CI failure could split across multiple memory records and GitHub issues. Repeated occurrences such as:
could receive different proposed cause IDs, lose a historical issue association, or mutate duplicate issues before the canonical issue was updated safely.
Root cause: Agent-proposed IDs were treated as stable identities before comparison with historical records, and issue reconciliation was embedded in producer-specific workflow code. Same-test records, reverse aliases, trusted job evidence, and occurrence history could therefore diverge across runs.
The fix: Resolve proposed causes deterministically against canonical IDs, aliases, normalized test names, retry patterns backed by trusted per-job logs, and trusted failed-job attribution before persistence. Compatible historical same-test records converge on the oldest canonical root, and a sole prior-backed owner absorbs fresh same-test proposals while preserving aliases, issue URLs, and complete reference remapping.
Retry-pattern cause IDs are applied only when one referenced job satisfies every configured field against its trusted bounded log tail. Agent-authored cause text cannot manufacture an output match, fields cannot be combined across jobs, and patterns requiring output fail closed when the job log is unavailable.
Ordinary resolution exposes every compatible reverse and transitive canonical-family ID and stored alias. Existing issues identified only by historical markers therefore remain discoverable, regardless of input order.
When a legacy root normalizes to an existing compatible canonical ID, the existing safe file remains the physical canonical. The legacy root and its transitive aliases are rewritten as aliases without overwriting their occurrence or issue metadata, while unrelated and mismatched-type collisions remain fail-closed.
The checked-in
tracking-issue.jsplanner/executor owns exact-marker lookup, oldest-canonical selection, post-create relisting, comment hydration, occurrence deduplication, reopening, and optional duplicate closure. Canonical actions execute before duplicate comments or closes. If an existing canonical occurrence section cannot be rendered safely, duplicate reconciliation is suppressed for that cause while unrelated causes continue, preventing partial or destructive mutation.Cause publication renders agent-derived fields as literal Markdown, retains a managed rolling occurrence section within GitHub's issue-body budget, and exposes triggering-merge context only when candidate history is complete. Main-breakage titles and diagnostics are rendered from trusted run context, and existing issues are migrated while preserving operator notes and occurrence history.
Occurrence delimiters and rows are recognized only as complete standalone lines, so marker or run-link text inside diagnostics remains inert and run IDs are compared exactly. Historical publication inference additionally requires a non-empty, distinct visible row set whose declared count matches the parsed rows. Job names use the existing single-line, 500-character sanitization contract before Markdown table rendering. The obsolete shell issue renderer and its duplicate lifecycle contract are removed.
Successful issue publication is recorded on the durable occurrence and resolved across canonical and transitive alias records. Replaying a run whose row was trimmed remains idempotent without treating memory persistence before a failed issue mutation as successful publication. Post-create relisting records a receipt only when the selected canonical issue already contains the occurrence, accepts the update, or remains the newly created issue.
The issue-body budget blocks only new issue creation; existing issues still receive bounded occurrence updates. Run-scoped persistence can continue when a subject pull request is closed or locked, while PR comments and pull-request reruns recheck that the PR is open and unlocked immediately before mutation.
Regression coverage: The five affected Infrastructure test classes pass 481 focused cases covering canonical identity, historical convergence, reverse alias discovery, authoritative/fresh coalescing, normalized-ID and transitive-alias collisions, trusted attribution and job-log matching, candidate collection, persistence, rendering, main issue migration, standalone marker and row parsing, exact occurrence IDs, malformed historical occurrence counts, action ordering, post-create relisting, fault injection, duplicate handling, publication retries, and replay idempotence.
Fixes #19578