Skip to content

ci(queue): stop merge-queue entries re-running checks a PR already passed on the same tree (FND-4344) - #4592

Merged
cmgrote merged 2 commits into
mainfrom
FND-4344-queue-head
Oct 23, 2026
Merged

cmgrote merged 2 commits into
mainfrom
FND-4344-queue-head

Conversation

@cmgrote

@cmgrote cmgrote commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

What

There are two changes, and the second depends on the first:

  1. queue_tree_diff.py now reads the PR head from refs/pull/<N>/head and adds a pr_head output.
  2. New queue-proof job in tests-reusable.yaml. It runs on merge_group only, on ubuntu-slim, with contents: read and statuses: read. When a release PR's /gate-check already passed on the exact tree being queued, its Unit/Integration re-run in the queue is skipped.

1. Queue head lookup fix

GitHub names a queue branch gh-readonly-queue/<base>/pr-<N>-<sha>. That SHA is the commit the entry was built on, the queue commit's parent. It is not the PR head. I checked 7/7 recent queue entries on a connector repo and 1 on this repo: the ref SHA equalled parents[0] of the queue commit every time.

The script read that SHA as the PR head, so it compared each queue commit with its own base. identical was never true, and the FND-3321 skips for Pre-commit and Conformance Gate never fired. For example, a release PR whose queue tree was byte-identical to its head still re-ran both.

The head now comes from refs/pull/<N>/head, which needs only contents: read, the permission every queue-diff job already has.

Behaviour change for every PR, not just releases: Pre-commit and Conformance Gate now skip on a queue entry whose tree equals the PR head and whose head already contains the base. This is what FND-3321 documented, and it is the normal case for an up-to-date PR.

2. Release-PR skip in tests-reusable

A release PR's full suite already ran on its head: /gate-check dispatches tests.yaml there, and it refuses a branch behind its base. Real data from a connector with a queue: 16/16 recent bump PRs queued an identical, up-to-date tree, and each queue entry re-ran ~8 billed minutes of Tests jobs.

queue-proof works in three steps:

  1. It runs queue_tree_diff.py, the same step as the other queue-diff jobs, pinned by a test.
  2. Only when identical=true does it run the new queue_gate_check_proof.py. That script reads the newest gate-check status that github-actions[bot] posted on that exact PR head, using the same creator rule as Release Gate (FND-3947).
  3. It sets proven=true only on success.

On a proven entry, Unit, Detect integration suite, Detect hosted api member, the scorecard and Tests Gate skip at job level. Integration, api-member and dataforge wake/pause skip because their roots did. Every job keeps its static name, so tests / Tests Gate resolves as skipped, which passes the required context. That is the FND-3320/FND-3947 pattern.

What still runs everything

  • A base that moved, or a batched group: the tree differs.
  • A head pushed after queueing.
  • No gate-check success from the bot on the head.
  • Any git or API error.
  • queue-proof skipped (every non-queue event) or failed: its output is empty.

Single-leg retry is unchanged. It happens on the PR's dispatched run (FND-4331), before the queue.

Tests

  • test_queue_tree_diff.py now builds the real ref shape: base SHA in the name, head at refs/pull/42/head. New tests:
    • the compared head is the PR's, not the ref SHA;
    • a head pushed after queueing is not identical;
    • an unresolvable head fails safe.
  • test_queue_gate_check_proof.py covers the script's trust rules: bot only, newest row wins, a re-run that cleared a failure counts, and every fail-safe path. It also simulates a whole run of the reusable job by job through the real if: expressions, with GitHub's implicit success() on needs:
    • a proven entry runs only queue-proof;
    • unproven, empty or failed answers run the tiers;
    • PR, push and dispatch never skip.
  • Red/green: the new head-lookup tests fail against the previous script, and the run simulation fails against the previous workflow. Both pass here.
  • Two pinned tests now assert the property instead of the old literal condition:
    • test_the_gate_job_still_always_reports asserted if: always(). The gate now skips on exactly one input, proven == 'true', which only a merge_group run can produce. FND-2167's hazard was a skipped suite on a PR commit overriding a real failure. A queue commit belongs to one entry, so no other suite shares its SHA (ci.md, "Newest suite wins").
    • test_integration_always_runs_off_the_pr_path now passes the needs context that detect-integration reads.
  • 2033 passed across every test file that reads tests-reusable.yaml, plus the queue tests.

Rollout and cost

Consumers call these reusables at @main, so no resync is needed. Callers already grant statuses: write, because Tests Gate needs it, so statuses: read is not clamped.

Expected on a release queue entry: about 11 → 3 billed min. Tests drops from ~8 to 1 (queue-proof), and Pre-commit plus Conformance (~3) go to 0, while the three decision jobs stay. I'll post the before/after from the next real release on FND-4344.

Review notes

This is a security-relevant change to scripts and workflows that decide whether required checks skip. A wrong identical=true or proven=true would green a required check on a tree that was not tested. Guards:

  • an exact tree match, with the base already in the head;
  • a status from github-actions[bot] only, read on the same SHA whose tree was compared;
  • every error path ends in "run everything".

🤖 Generated with Claude Code

https://claude.ai/code/session_017gdsZwMP78agPsUHUV3ksA

…queue branch name (FND-4344)

GitHub names a queue branch gh-readonly-queue/<base>/pr-<N>-<sha>, where the
SHA is the commit the entry was built on (the queue commit's parent), not the
PR head. queue_tree_diff.py read it as the head, so it compared every queue
commit with its own base: `identical` was never true and the FND-3321
Pre-commit and Conformance Gate skips never fired.

Fetch the head from refs/pull/<N>/head instead (contents: read only) and
expose it as a `pr_head` output for a later step that trusts a verdict on
that commit. Every failure path still runs every check.

Claude-Session: https://claude.ai/code/session_017gdsZwMP78agPsUHUV3ksA
@linear

linear Bot commented Oct 23, 2026

Copy link
Copy Markdown

FND-4344

@cmgrote

cmgrote commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

/lens

@cmgrote cmgrote changed the title fix(ci): read the merge-queue PR head from refs/pull/N/head, not the queue branch name (FND-4344) ci(queue): stop merge-queue entries re-running checks a PR already passed on the same tree (FND-4344) Oct 23, 2026
@atlan-app-fleet

Copy link
Copy Markdown
Contributor

lens summary · updated after round 1 (first review)

✅ Ready to merge — every finding is resolved

Open findings: 🔴 0 critical · 🟠 0 high · 🟡 0 medium · ⚪ 0 low (nit)

lens reads this PR as — The merge-queue branch-name SHA identifies the queue entry’s base parent, not the PR head, so the shared diff logic compared against the wrong commit and failed to recognize identical trees. The PR also aims to let release queue entries reuse a successful /gate-check verdict when the queued tree is the tested PR head. How: The diff driver now extracts the PR number, fetches refs/pull/<N>/head, compares the queue tree and base ancestry with that head, and outputs the exact pr_head. The PR also adds a status-backed proof job in tests-reusable.yaml that uses this comparison and the head’s gate-check status to skip test work for proven queue entries.

Approach check — worth a second look (advisory, does not block):

  • The reusable-workflow optimization is included despite being described as a follow-up (A1) — The review note says the queue-proof job is “Stacked next,” while .github/workflows/tests-reusable.yaml already adds it and gates Unit, Integration, and Tests Gate on its output. This is a caller-observable change to the reusable workflow consumed by connector repositories (for example, packages/conformance/conformance/bootstrap/templates/tests.yaml); consumers in other repositories were not checked. Instead: Keep queue-proof for the stated follow-up, or make it an explicit behavior change in this PR and review its impact on downstream reusable-workflow callers.
🕘 Round history (1)
round run commits new resolved blocking open cost log
1 first review 2eaa597..11eac02 0 0 0 $0.053 run

round 1 · $0.053 of $1.00 · 18 model calls · 0 failed requests · 76% prompt cache hits · gpt-6-luna

@atlan-ci atlan-ci 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.

lens: ready to merge — every finding at every level is resolved (round 1).

@cmgrote
cmgrote added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 8dba895 Oct 23, 2026
29 of 30 checks passed
@cmgrote
cmgrote deleted the FND-4344-queue-head branch October 23, 2026 16:57
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