Skip to content

ci(compatibility): review PR breaks and acknowledge release risks - #4488

Merged
SDAChess merged 5 commits into
mainfrom
ci/4487-compatibility-pr-reviews/SDAChess
Oct 13, 2026
Merged

SDAChess merged 5 commits into
mainfrom
ci/4487-compatibility-pr-reviews/SDAChess

Conversation

@SDAChess

@SDAChess SDAChess commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Summary

Run the advisory compatibility reviewer in Branch Checks and post a fresh PR comment whenever it detects breaking changes. Both branch checks and tagged-release qualification inspect code first, then use read-only gh lookups to identify the introducing PR and report whether its specific risk was acknowledged. Make reports concise, with concrete changes, consumer impact, actions, and PR links.

Related Issue

Closes #4487

The author has verified ADMIN access; the linked issue currently has no state:accepted label.

Changes

  • Review the prospective merge tree in Branch Checks and publish a new comment for each run with findings. Clean runs post nothing; closed PRs and outdated heads are skipped.
  • Enable read-only GitHub CLI lookups in the reviewer, retain read-only filesystem access, and show ACKED, NOT ACKED, or UNKNOWN without overriding release policy.
  • Shorten readable findings and collapse review metadata; update the CI documentation.

Testing

  • Checks appropriate to the affected code and behavior pass: actionlint on the three affected workflows, Ruff lint and formatting on the Python helpers, Python/JSON syntax checks, and git diff --check.
  • Unit tests added/updated (not applicable; no new test files requested).
  • E2E tests added/updated (not applicable).

The live compatibility review job passed for 1ec632b85. Its retained report was complete across all ten surfaces with zero findings; the comment publisher correctly skipped posting. Tagged qualification and publication of a findings comment have not been exercised. The local pre-commit hook was bypassed after its mise-managed Python and Node installations failed on NixOS; the focused checks above passed independently.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (not applicable)

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
@github-actions

Copy link
Copy Markdown

Signed-off-by: Simon Scatton <sscatton@nvidia.com>
@SDAChess
SDAChess enabled auto-merge October 13, 2026 16:59
Signed-off-by: Simon Scatton <sscatton@nvidia.com>
Signed-off-by: Simon Scatton <sscatton@nvidia.com>

@matthewgrossman matthewgrossman 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.

Three concerns from the compatibility workflow review. Local Ruff, Python/JSON syntax, and diff whitespace checks passed; the latest live reviewer run had no findings and therefore did not exercise comment publication.

Comment thread tasks/scripts/codex_compatibility.py
Comment thread .github/workflows/branch-checks.yml Outdated
Comment thread .github/workflows/branch-checks.yml Outdated
Signed-off-by: Simon Scatton <sscatton@nvidia.com>

@purp purp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🚢

@SDAChess
SDAChess disabled auto-merge October 13, 2026 19:13
@SDAChess
SDAChess added this pull request to the merge queue Oct 13, 2026
Merged via the queue into main with commit d3e6a35 Oct 13, 2026
81 checks passed
@SDAChess
SDAChess deleted the ci/4487-compatibility-pr-reviews/SDAChess branch October 13, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: report PR compatibility breaks and qualification acknowledgements

3 participants