Skip to content

Fix "Debug artifacts after failure" PR check not checking any artifacts - #4273

Merged
henrymercer merged 1 commit into
mainfrom
henrymercer/fix-debug-artifacts-failure-check
Oct 27, 2026
Merged

henrymercer merged 1 commit into
mainfrom
henrymercer/fix-debug-artifacts-failure-check

Conversation

@henrymercer

Copy link
Copy Markdown
Contributor

The "Check expected artifacts exist" step of the "Debug artifacts after failure" PR check loops over $VERSIONS, but nothing in the workflow sets it. This PR sets VERSIONS to the upload job's matrix versions, as the "Debug artifact upload" check does in debug-artifacts-safe.yml.

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

  • Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

  • Other - The "Debug artifacts after failure" PR check will fail.

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI max review requested due to automatic review settings October 27, 2026 12:27
@github-actions github-actions Bot added the size/XS Should be very easy to review label Oct 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The version list matches the upload matrix and restores existing validation without changing production behavior.

No open issues 🎉

What changed in this PR

Restores artifact validation in the “Debug artifacts after failure” PR check, which previously skipped its validation loop because VERSIONS was unset.

Changes:

  • Defines VERSIONS to match the upload job’s four-version matrix.
  • Enables the existing per-version and per-language artifact checks.
File Description
.github/​workflows/​debug-artifacts-failure-safe.yml Initializes the version list used by the artifact-validation loop.

Review effort: Max

@henrymercer
henrymercer marked this pull request as ready for review October 27, 2026 12:39
@henrymercer
henrymercer requested a review from a team as a code owner October 27, 2026 12:39

@mbg mbg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing this and addressing it! I wonder how this happened...

Just one optional suggestion; happy for this to be merged as-is.

uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2
- name: Check expected artifacts exist
run: |
VERSIONS="stable-v2.20.7 default linked nightly-latest"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: Rather than having this as a local variable here, would it be easier to spot/update if it was in an env object for the step/job/workflow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this would be much easier to spot, and it might increase the chance of the variable being unset again.

It would also be nice to have a single source of truth for the versions, but per https://docs.github.com/en/enterprise-cloud@latest/actions/reference/workflows-and-actions/contexts#context-availability we can't use env to define a matrix and would therefore need another job to determine the matrix.

I'll leave this as is for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to migrate these workflows to pr-checks/checks and generate them with sync.ts? Then we could perhaps move towards a single source of truth for the versions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, now that we have validationJobs, I think it should be possible to autogenerate these workflows.

@henrymercer
henrymercer added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 9be075c Oct 27, 2026
228 checks passed
@henrymercer
henrymercer deleted the henrymercer/fix-debug-artifacts-failure-check branch October 27, 2026 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Should be very easy to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants