ci: advisory hint for stale expected-failure markers (EAI-7478) - #134
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
fredespi
force-pushed
the
fix-xfail-cleanup-gap
branch
from
July 21, 2026 07:37
29a3792 to
e0c9d12
Compare
rominf
approved these changes
Jul 21, 2026
rominf
left a comment
rominf
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the matching logic, workflow wiring, Actions input handling, and the CodeQL remediation. The current head removes the flagged file sink and all checks are green. Non-blocking follow-up: scripts/xfail_expectations_hint.py:224 reads expectations.toml without guarding a missing file, so a PR that renames or removes it would make this advisory job exit 1 despite the script's never-fail contract.
fredespi
force-pushed
the
fix-xfail-cleanup-gap
branch
from
July 21, 2026 13:25
47e74a2 to
04d1389
Compare
A bug fix whose CI lane doesn't exercise the platform an xfail row is keyed to merges green, leaving the stale row to surface later as a mis-attributed XPASS on an unrelated PR. Close that gap at authoring time. - scripts/xfail_expectations_hint.py: parse expectations.toml, collect the ticket ids a PR references (title, body, commit messages) in both the project tracker and GitHub #issue shapes, and warn if any still has a live xfail row. Advisory only: always exits 0. Behavior pinned by --self-test. - ci.yml: wire it as a non-blocking, non-required pull_request job on ubuntu-latest, reusing the pinned checkout action; PR text passed via env (no shell interpolation). - pull_request_template.md: one checkbox reminding authors to clear stale xfail rows for a fixed ticket. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
CodeQL flagged the open() on GITHUB_STEP_SUMMARY as a path-injection sink, and GitHub's default code-scanning setup ignores the inline `# codeql[...]` suppression comment. Drop the file write entirely: emit() now prints the note to stdout and the workflow redirects it to $GITHUB_STEP_SUMMARY, matching the existing convention in ci.yml. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
main() read the expectations matrix unconditionally, so a PR that renames or removes it would raise FileNotFoundError and exit 1 — breaking the script's never-fail advisory contract. Guard the read and exit 0 when the file is gone. Signed-off-by: fredespi <fredrik.espinoza@gmail.com>
fredespi
force-pushed
the
fix-xfail-cleanup-gap
branch
from
July 21, 2026 13:30
04d1389 to
1889e21
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A bug fix whose CI lane doesn't exercise the platform an xfail row is keyed to merges green, leaving the stale
expectations.tomlrow to surface later as a mis-attributed XPASS on an unrelated PR (e.g. #94's vllm xfails resurfaced on #127's GPU lane). This closes that gap at authoring time with a lightweight advisory nudge — no change to the reconciler or theheavyfilter.Changes
scripts/xfail_expectations_hint.py— parsestests/e2e-cucumber/expectations.toml, collects the ticket ids a PR references (title, body, commit messages) in both the project tracker shape (ABC-123) and GitHub#issueshape, and warns via the step summary if any referenced ticket still has a live xfail row. Advisory only: always exits 0. Behavior is pinned by an embedded--self-test(the single source of truth for the rules)..github/workflows/ci.yml— wires it as a non-blocking, non-requiredpull_requestjob onubuntu-latest, reusing the already-pinned checkout action. PR title/body are passed viaenv:(never interpolated into the shell)..github/pull_request_template.md— one ticket-neutral checkbox reminding authors to clear stale xfail rows for a fixed ticket.Test plan
python3 scripts/xfail_expectations_hint.py --self-test— green (7 cases: tracker ref,Fixes #123, bare#123, no-xfail-row, nothing-tracked, URL-path-not-an-issue-ref, refs-from-commits).expectations.toml:--body "Fixes EAI-7052"reproduces the ticket's concrete case (warns onserve-default-engine-inference+serve-default-engine-working-endpoint); an unreferenced ticket is silent; exit code is always 0.ruff checkclean;ci.ymlparses; hawkeye license-header check passes.Why match bare
#123too: a bare#123sometimes means "PR 123", so this can occasionally nudge on an unrelated number — acceptable because the check is advisory. Favor recall: a stray nudge costs a glance; a missed stale xfail costs a mis-attributed red on an unrelated PR.