Skip to content

ci: guard the testsuite workflow contract - #1044

Merged
castrojo merged 2 commits into
projectbluefin:testingfrom
Danathar:fix/stable-promotion-contract
Aug 12, 2026
Merged

ci: guard the testsuite workflow contract#1044
castrojo merged 2 commits into
projectbluefin:testingfrom
Danathar:fix/stable-promotion-contract

Conversation

@Danathar

@Danathar Danathar commented Aug 8, 2026

Copy link
Copy Markdown

What does this change?

Add a repository-level regression check for the Bluefin testsuite workflow contract. It requires the canonical wrapper to use the managed @v1 workflow and test_ref: v1, and rejects direct testsuite E2E calls from other Bluefin workflows. PR validation runs the check.

Why?

Issue #929's latest release run executed projectbluefin/testsuite/.github/workflows/e2e.yml@ee2a5b9 while checking out test_ref: v1. The stale nested workflow caused the release-gate smoke failures; the owner-side fix is tracked in projectbluefin/actions#409. Bluefin already uses the managed wrapper after #1012, so this change prevents that contract from regressing in this repository.

Validation

  • python3 scripts/check-testsuite-workflow-ref.py
  • actionlint .github/workflows/*.yml
  • just check
  • git diff --check
  • Python AST parse
  • pre-commit was unavailable in the environment (command not found)

Refs #929

@Danathar
Danathar marked this pull request as ready for review August 8, 2026 00:46
@Danathar
Danathar requested a review from a team as a code owner August 8, 2026 00:46

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

Verified the guard actually enforces the contract it describes. I ran scripts/check-testsuite-workflow-ref.py from this branch against the current tree (passes) and mutation-tested three violations — all are caught with exit 1:

  1. run-testsuite.yml ref changed @v1@ee2a5b9 (the exact #929 regression) → 'must contain exactly one direct testsuite workflow reference at @v1'
  2. test_ref: v1test_ref: main → 'must pass exactly one test_ref: v1'
  3. A new workflow calling projectbluefin/testsuite/.github/workflows/e2e.yml directly → rejected, told to use the local wrapper

Current state on main: run-testsuite.yml:24 is the only direct testsuite reference and passes test_ref: v1, so the check is green today. Wiring it as a step in the validate job of pr-validation.yml (after checkout) is the right place. The comment fix at pr-validation.yml:234 ('SHA' → 'workflow ref') matches the post-#1012 @v1 reality, and the docs/skills/ci/SKILL.md addition keeps the skill doc in sync per repo policy.

Minor (non-blocking): the regex requires the ref to be the last token before any # comment, which is the house style, so that's fine.

CI note: PR Validation run 31229938653 is action_required — awaiting maintainer approval to run; nothing has failed.

@Danathar

Copy link
Copy Markdown
Author

Thanks for mutation-testing it rather than just reading it — the three violations you ran are exactly the cases the guard exists for, and #929 being one of them is the one I most wanted covered.

On the minor note: agreed, and leaving it as-is. To be precise about what the pattern does, since "last token before any #" could be read as stricter than it is — @([^\s#]+) stops at whitespace or #, so a trailing comment is fine either way:

uses: projectbluefin/testsuite/.github/workflows/e2e.yml@v1        # pinned  -> captures "v1"
uses: projectbluefin/testsuite/.github/workflows/e2e.yml@v1# pinned          -> captures "v1"

# is excluded from the capture deliberately: without it, @v1 # pinned would capture v1 but @v1# pinned would capture v1#, and the ref comparison would fail with a confusing message about a ref nobody wrote. What it genuinely will not match is a ref containing #, which git does not permit anyway.

No changes pushed — your read matches the intent and the house style, so there is nothing here I want to loosen.

On the CI note: action_required is the maintainer-approval gate for a fork PR, not a failure. Nothing has run yet, so whenever you or another maintainer approves the workflow run it should go green — the check passes against the current tree, as you confirmed locally.


🐝 Hive Agent: contributor | SHA: 8cc2b09

Danathar and others added 2 commits August 10, 2026 00:30
Assisted-by: GPT-5 via GitHub Copilot

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The validate job was failing: `validate-docs` caps a SKILL.md at 180 lines,
ci/SKILL.md already sits at exactly 180 on testing, and this branch's
four-line bullet pushed it to 184.

Rework rather than trim to fit. The testsuite-contract bullet collapses to
two lines, the neighbouring @v1 bullet tightens from six to four, and the
detail those lose -- that run-testsuite.yml is the single permitted direct
caller, the two invariants the script enforces, the projectbluefin#929 freeze it prevents,
and why config:best-practices makes the Renovate exclusion necessary -- moves
into references/workflow-map.md. That is the progressive-disclosure split the
skill docs already use, and it is why the cap exists.

ci/SKILL.md is back to 180. Verified locally: validate-docs.py reports
"documentation ok: 13 skills, 41 Markdown files", and
check-testsuite-workflow-ref.py still passes against the current tree.

Also rebased onto testing; the branch was 16 commits behind. No conflicts,
and pr-validation.yml did not change in that range.
@Danathar
Danathar force-pushed the fix/stable-promotion-contract branch from 8c8a57f to fe9dd2b Compare August 10, 2026 00:32
@Danathar

Copy link
Copy Markdown
Author

Thanks again for the mutation testing — heads up that I've force-pushed twice since your approval, so it may need a re-look.

validate was failing, and it wasn't the guard. Your note said run 31229938653 was action_required; it has since run and gone red. The cause is unrelated to the check's logic:

Validate agent documentation....Failed
- hook id: validate-docs
ERROR: docs/skills/ci/SKILL.md exceeds 180 lines

validate-docs caps a SKILL.md at 180 lines. ci/SKILL.md is at exactly 180 on testing, so the four-line bullet this PR added took it to 184. Any addition at all would have broken it.

Fixed by rework, not by trimming to fit. The testsuite-contract bullet is now two lines and the neighbouring @v1 bullet tightens from six to four. What those lose — that run-testsuite.yml is the single permitted direct caller, the two invariants the script enforces, the #929 freeze it prevents, and why config:best-practices makes the Renovate exclusion necessary — moved into references/workflow-map.md, along with a copy-pasteable invocation. That is the progressive-disclosure split the skill docs already use, and presumably why the cap exists. ci/SKILL.md is back to exactly 180.

Verified locally:

$ python3 .github/scripts/validate-docs.py
documentation ok: 13 skills, 41 Markdown files
$ python3 scripts/check-testsuite-workflow-ref.py
Testsuite workflow contract passed: canonical wrapper uses @v1 with test_ref: v1

Also rebased onto testing — the branch was 16 commits behind. No conflicts, and pr-validation.yml did not change in that range, so the wiring you reviewed is untouched. scripts/check-testsuite-workflow-ref.py is byte-identical to what you mutation-tested; the only changes since your review are the two docs files.


🐝 Hive Agent: contributor | SHA: fe9dd2b

@castrojo
castrojo merged commit 15f50b2 into projectbluefin:testing Aug 12, 2026
5 checks passed
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.

2 participants