CI: one workflow and a merge gate that verifies queue failures in the same run (shadow) by mohityadav766 · Pull Request #35465 · open-metadata/OpenMetadata

mohityadav766 · 2026-10-14T14:37:21Z

Describe your changes:

Fixes #ISSUE

One CI workflow per event that builds once and stops at the first static failure, and a
merge gate that decides queue entries from the run's own evidence: a failing test is
re-run on the entry and on its parent commit in the same run, and the entry is ejected
only when the failure is its own. Network blips and other infrastructure failures are
re-run instead of ejecting the PR, with a bounded AI triage for failures no signature
recognises. Everything ships off: merging it changes nothing until an admin turns the
shadow on, in which ci.yml runs beside today's workflows and the gate posts
merge-gate (shadow), which no ruleset requires. Nothing changes for the live queue until
the cutover in docs/ci.md.

Why: 32.4% of 1,405 queue entries (2026-09-29 → 10-13) had a failed required check and
merged PRs needed 2.86 entries on average; recurring flakes (TestCaseResourceIT bulk
tests: 28 PRs) and broken-main failures ejected PRs that did not cause them, and nothing
acted on them.

Type of change:

New feature

High-level design:

Read docs/ci.md first; the decisions are in docs/decisions/2026-10-13-* (five records).

Pipeline (ci.yml): plan → static checks ∥ builds → suites → verify / * → ci-summary.

As today: a PR runs the static checks and the unit tests (Java, Jest, Python unit); the IT lanes, Python integration and Playwright run in the merge queue. A guard test pins it.
Static checks gate every suite (needs:); a PR run cancels itself on a static failure, a queue run never does.
static / ui and unit / java call ui-checkstyle.yml and openmetadata-service-unit-tests.yml, so the gate and the old required checks cannot drift; Playwright and Python call their reusables.
The Java reactor is built once and shared by the ten IT lanes and verification. Jest runs in 3 shards.
Shadow runs are sampled (CI_SHADOW_PR_PERCENT / CI_SHADOW_MQ_PERCENT); a run outside the sample gets no gate status.

Merge gate (merge-gate.yml, merge_gate*.py, .github/merge-gate/policy.json)

Runs main's code on every finished CI run, plus a 10-minute sweep for missed deliveries; decides only finished runs; App-pinned status in enforce mode.
Pure decision in merge_gate_policy.py: deterministic and static failures fail; failing tests go to verification (3× entry, 3× parent, classified flaky / pre-existing / introduced / ambiguous); infrastructure failures re-run only the failed jobs; budgets cap re-runs.
Network blips: signatures in annotations and in the failed step's log; on PR runs too; strict (CI-touching) entries still re-run blips. An unrecognised failure can get one bounded AI triage (ci_ai/triage.py) that must quote the log line; it can only add a re-run, never a pass (property-tested).
Flake registry and ledger (merge_gate_registry.py): every verified test is recorded; forgiven failures become flaky-test / broken-main issues; a parent control recorded for the same test, failure and parent is reused by later entries on that parent (same-run ADR v3).

AI roles (advisory, capped, off by default): ejection explanations, /ci-fix, conflict resolution (mechanical resolvers first), blip triage. Kill switches: CI_AI_MODE, CI_AI_TRIAGE_MODE. The conflict and fix workflows run in main's context, so they never run the code they edit: the agent only edits files and cannot reach the tooling or .git later steps use, publish re-checks the result with main's code (a conflict resolution must equal a fresh git merge-tree outside the conflicted files), and the CI of the PR it lands in tests it.

Merging changes nothing until it is turned on. With the repository variables unset, every ci.yml run stops after plan (both sampling shares default to 0), the merge gate starts no job (MERGE_GATE_MODE unset; a dispatch still decides one run), and the conflict resolver starts none (CI_CONFLICT_MODE defaults to off). An admin starts the shadow with MERGE_GATE_MODE=shadow and CI_SHADOW_PR_PERCENT / CI_SHADOW_MQ_PERCENT (start around 10, ramp while watching runner queue time).

Rollout: shadow ≥ 2 weeks, compare with ci_metrics.py compare, then one cutover PR to main that deletes or strips the replaced workflows (never gh workflow disable: release-branch rulesets still require those checks).

Workflow changes to existing files (defaults keep today's behaviour): ui-checkstyle.yml and openmetadata-service-unit-tests.yml gain workflow_call; playwright-e2e-reusable.yml and py-tests-shared.yml gain inputs, including a concurrency-group suffix so a call from ci.yml never shares a group with the workflow's own run (sharing the spaCy job's group could have cancelled py-tests' job and failed py-tests-status); backport-automerge.yaml gets a job-level if that only skips branches its own gate rejects. classify_test_outcome.py additionally lists tests that passed only on a re-run (flakyTests).

Overlap: #34944 (#34945–#34948) changes some of the same workflows (Playwright pipeline, IT lanes, quarantine/flaky reporting). Needs coordinating before either lands.

Tests:

Use cases covered

A static failure starts no suite; a PR run cancels itself, a queue run skips through needs:.
A queue entry whose IT test fails only with this entry's code is ejected; a flaky or broken-on-main test is forgiven and filed.
A network blip in any job is re-run (PR and queue); an unrecognised failure is triaged, never forgiven by AI.
A purely mechanical conflict resolution (locale JSON) re-queues itself once the PR's checks pass; anything a model edited, or that needs regenerating, waits for a person.

Unit tests

.github/scripts/tests/ and .github/scripts/test_*.py (gate policy, controller against a fake GitHub, evidence, verification, registry, triage, conflicts, publishing, metrics, local runner): 875 passing. CI leaves out the two whole-repository Java impact-map audits: map ownership is checked before the PR (Fixes #34815: Make agents own new code and ITs in the Java impact map before the PR #34984), not by CI.
Guard tests for the decisions: static checks gate every suite; a PR runs unit tests and the queue adds integration and Playwright; no job is named merge-gate; called workflows get the permissions their jobs declare and never share a concurrency group with their own runs; every paths filter can list a PR's files; no artifact path contains ..; the conflict and fix workflows never run the code they check out; only the triage adapter reaches a model; a blip read can only add a re-run.

Backend integration tests

Not applicable (no backend API changes).

Ingestion integration tests

Not applicable (no ingestion changes).

Playwright (UI) tests

Not applicable (no UI changes).

Manual testing performed

Locally with act (.github/scripts/ci_local.py, documented in docs/ci.md):

ci_local.py scenarios: the real job graph of ci.yml and merge-gate.yml in 20 scenarios (PR, draft, static failure, IT lane failure, build failure, manual run, gate triggers, and a PR, a queue entry, the gate and its sweep with the variables unset, where nothing runs): 20/20. Four deliberately broken copies of ci.yml each fail a scenario.
Real jobs: plan, static / json-yaml, static / ci-scripts, static / python pass.
A real network blip (job network cut): the gate's own code decides "re-run plan, infrastructure".
The Java queue path for real (ci_local.py job verify-java: reactor build, the retry-queue lane on Postgres + OpenSearch via Testcontainers, verification, the gate's decision):
- a regression the entry introduces: entry 3/3 failed, parent rebuilt, parent 0/3 → FAIL (introduced);
- the same regression already on the base commit: entry 3/3, parent 1/1 → PASS (pre-existing, filed as broken-main);
- a test that fails on its first run only: entry 0/1 → PASS (flaky, filed).
Conflict resolver on real repo files: locale merged, generated taken from main, Java/migration/workflow left for AI or a person; a resolution dropping a side is caught.

Found and fixed by these runs:

verify / java-it wrote under github.workspace/.., which upload-artifact rejects: verification would never have reached the gate (guard test added).
failsafe re-runs a failing IT after the harness has shut the server and pool down, so an in-JVM re-run always errors: verification would have read a flaky test as 3/3 failed and ejected it. Every attempt is now a fresh JVM, and the lanes no longer pass rerunFailingTestsCount.
unnamed action steps and Testcontainers launch failures were not recognised as infrastructure.

Review rounds

ab5ca3b: ci.yml's python job grants pull-requests: read, which the paths filter in py-tests-shared.yml needs on pull_request (without it every Python job of a sampled PR run would have been skipped); a guard requires it of every job that reaches a paths filter.

c380d5c: merging changes nothing until it is turned on (sampling shares 0, gate off while MERGE_GATE_MODE is unset, conflict resolver off); the spaCy concurrency collision above is fixed and guarded; the Redis IT workflow keeps its queue trigger until the cutover.

8b93c6b, 25d6280: the gate decides only ci.yml runs whose jobs ran (one that could not start fails; one awaiting approval or cancelled before any job began gets no verdict) and re-runs several jobs together; each failure is answered by its own job's verification across re-run attempts; a parent failure counts only when it is the entry's own failure; the ledger keys records by job and keeps nothing from strict or undecided entries; the impact-map link applies to the ambiguous tie-break.

Round 2 (f4bff18): the sweep lists queue runs separately and first, oldest first, and its limit of 50 counts only runs that reach a verdict (runs with a final status, outside the sample, superseded or out of the queue no longer crowd out an entry near its timeout); a listing past its page budget no longer fails the whole sweep. The merge-gate record (v3) now states the sweep's window, order and limit, and that a pending status, including a held entry's, is decided again.

Round 1 (5a393c6):

gitar: the sweep listed runs created in the last 30 minutes, so it never saw a finished queue run; it now decides runs that finished 15–60 minutes ago whose head has no gate status or only a pending one. Fixing it showed the gate never checked that a run had finished: a run still going (a dispatch, or a re-run starting under the sweep) would have read as passed. It now decides finished runs only (tests fail without the check).
gitar: a dispatched dry run no longer writes the ledger or flake issues; the ledger's parent-control reuse is now stated in the same-run record (v3), and the record job writes with the gate App token.
CodeQL: the conflict and fix workflows' verify jobs ran PR code (fork code included) in main's cache scope, and /ci-resolve was a comment-triggered checkout (TOCTOU). Both are gone; the agent can no longer reach the trusted tooling or .git; nothing runs inside a PR checkout (a PR's secrets.py would have been imported by a heredoc step); a symlinked locale file can no longer redirect a write. Also a linear test-id regex and escaped invisible-character classes.
CodeQL alerts 2428–2431 are left open: they trace ci.yml's workflow_dispatch into the reusables' github.event.pull_request.head.sha checkout, which is empty on a dispatch, so the dispatched branch is checked out, never a PR head. The other ten alerts on ui-checkstyle.yml / py-tests-shared.yml are open on main already.
The seven CI-script tests that failed on main: four went stale when the code changed (two when Playwright moved to the queue in ci: Python integration tests and Playwright E2E only in the merge queue #34740) and now match today's CI; one exposed a real bug (an unreadable first attempt counted as a missing report, now unknown); the two impact-map audits are left out of CI, as above.

Admin actions before enforce mode: docs/ci.md. Draft PRs skip most CI.

UI screen recording:

Not applicable — CI workflows, scripts and docs only; no product behaviour changes.

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes <issue-number>: <short explanation> (pending the issue; Fixes #ISSUE above is a placeholder)
I have commented on my code, particularly in hard-to-understand areas.
I have added tests around the new logic.
Decision check: docs/decisions/2026-10-13-* (five records; four revised 2026-10-14).

🤖 Generated with Claude Code

… same run ci.yml runs once per PR head and per queue entry: static checks gate every suite, the Java reactor is built once for the IT lanes and their verification, and UI checkstyle and the service unit tests run as the workflows they already are. merge-gate.yml turns each finished run into the merge-gate status. A failing test is re-run on the entry and on its parent commit in the same run, and the entry is ejected only when the failure is its own. Network blips and other infrastructure failures are re-run instead (known signatures first, then a bounded triage call that can only add a re-run), and every forgiven failure becomes a tracked flaky-test or broken-main issue. Ships in shadow: merge-gate (shadow) is required by nothing until the cutover in docs/ci.md. Design records: docs/decisions/2026-10-13-*.

- Merge gate: the sweep looks back six hours and decides runs that finished 15 to 60 minutes ago whose head has no gate status or only a pending one (a re-run leaves it pending). A run still going is never decided: its unfinished jobs would read as passed. - merge-gate.yml: a dispatched dry run records nothing; the record job writes the ledger with the gate App token from the ci-ai environment. - Same-run ADR v3: a parent control recorded for the same test, failure and parent commit stands in for running it again. - Conflict and fix workflows run no PR code: their verify jobs ran it in main's cache scope (CodeQL cache poisoning), and /ci-resolve is gone (TOCTOU). Tooling and .git stay out of the agent's reach, no step runs inside a PR checkout, and publish re-merges with git merge-tree and refuses edits outside the conflict. The PR's own CI tests what they push; taken generated files and lockfiles wait for a person. - CodeQL: a linear test-id regex, escaped invisible-character classes. - CI Scripts Tests run on workflow changes and leave out the two whole-repository Java impact-map audits (ownership is checked before the PR, #34984). Four stale tests follow today's CI: PRs run unit tests, the merge queue runs integration and Playwright, now pinned by a guard test. An unreadable first Playwright attempt counts as unknown, not as a missing report.

…dicts The sweep read one listing of every CI run, newest first, and cut it at 50 runs before filtering. Runs that already had a final status, or would never get one (outside the sample, superseded, out of the queue), could push the oldest queue runs, the ones nearest their check timeout, past the cut; and a listing past its page budget failed the whole sweep. Queue runs now have their own listing (6 hours) and PR runs theirs (3); each is sorted oldest first, queue runs before PR runs, and the limit of 50 counts only runs that reach a verdict. A listing that cannot be read is reported and the other is still swept. The merge-gate ADR (v3) records the window, the order and the limit, that a pending status counts as undecided, and that a held entry is decided again on each sweep in the window.

github-actions · 2026-10-14T18:57:18Z

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

…wn job The merge gate passed runs in which no job ran, lost the evidence of jobs a re-run left alone, and could forgive a regression on a parent control that failed for another reason. - merge_gate.py decides only ci.yml runs whose jobs ran: a run that could not start fails; one awaiting approval or cancelled before any job began gets no verdict. Several jobs that need a re-run go through rerun-failed-jobs, since GitHub refuses a targeted re-run once another has started a new attempt. - merge_gate_evidence.py reads each failed job's evidence from the execution that wrote it (GitHub copies the jobs a re-run leaves alone into the new attempt with their original start time) and answers a failure only with the verification of its own job, run no earlier than the attempt that saw it. - verify_failures.py verifies each engine and lane separately, reads each lane's newest evidence, and counts a parent failure only when it is the entry's failure (the same fingerprint). - merge_gate_registry.py keys ledger records and parent-control lookups by job and records nothing from strict entries or runs the gate did not decide. - The ambiguous-failure tie-break applies the impact-map link the record promised, with plan_local_java_tests.py's static rules.

A re-run leaves the lanes it did not re-run with an earlier attempt's evidence. Downloading only the current attempt's left a re-run verify job with nothing to verify for them, so the entry was ejected as unverified. verify_failures.py select keeps each lane's newest outcome.

- py-tests-shared.yml: the spaCy job's concurrency group takes the caller's suffix, and ci.yml passes one. GitHub keeps one pending job per group and cancels the one before it, so ci.yml's shadow run, a third caller beside py-tests and py-tests-postgres, could cancel one of their spaCy jobs for the same PR or queue entry and fail the required py-tests-status. A guard test now requires every concurrency group in a workflow ci.yml calls to take the caller's suffix. - Off by default: both shadow sampling shares default to 0, the merge gate starts no job while MERGE_GATE_MODE is unset (a dispatch still decides one run), and CI_CONFLICT_MODE defaults to off. Local scenarios cover each of them left unset. - The Redis IT workflow keeps its merge_group trigger until the cutover. - Merge-gate ADR v4; docs/ci.md says what merging changes and how to turn the shadow on.

py-tests-shared.yml's changes job runs dorny/paths-filter on pull_request, which lists the PR's files through the API and needs pull-requests: read. ci.yml's python job granted only the workflow's contents: read, so on a sampled PR run the filter would fail and every Python job behind it would be skipped. The job now grants pull-requests: read, and a guard test requires it of every ci.yml job that reaches a paths filter.

gitar-bot · 2026-10-15T13:36:38Z

Code Review ✅ Approved 6 closed / 6 findings

🔴 High risk · When enabled, the new merge gate controls queue eligibility; an incorrect verdict could block valid PRs or allow failing changes to merge.

Adds a single CI workflow with a merge gate that verifies queue failures in the same run, shipped off by default. Open findings: none. Resolved this session: the sweep now lists queue runs first so long-running runs are not dropped, dry-run dispatches no longer write the ledger or flake issues, parent-control reuse matches the same-run ADR, and the python job in ci.yml now grants pull-requests: read so paths-filter can list a PR's files, guarded by test_every_paths_filter_can_list_a_prs_files.

✅ 6 closed

✅ Bug: Sweep's 30-minute created>= window misses every long queue run

📄 .github/scripts/merge_gate.py:359-373 📄 .github/workflows/merge-gate.yml:267
cmd_sweep lists runs with ?status=completed&created=>=now-30min, which filters on when a run was created, not when it finished. Queue runs build the reactor and then run the IT lanes, Playwright and verification, so they take well over 30 minutes. By the time one completes, its created_at is already outside the window. A missed workflow_run delivery for a merge-group run is the exact case the sweep exists for (merge-gate.yml:16-17), and the sweep never sees that run, so the entry waits for the 240-minute queue timeout. There is a second gap: after a RERUN the gate posts a pending status, and the sweep skips any head with a status in this context. If the delivery for the re-run attempt is missed, that run is also never decided. Fix: filter on updated_at or use a window longer than the longest CI run. Also re-decide heads whose only gate status is pending.

✅ Bug: A dry-run dispatch still writes the ledger and flake issues

📄 .github/workflows/merge-gate.yml:30-33 📄 .github/workflows/merge-gate.yml:100 📄 .github/workflows/merge-gate.yml:173 📄 .github/workflows/merge-gate.yml:206-213
The dry_run input of workflow_dispatch defaults to true and is described as "Decide and report only; post nothing". It only passes --dry-run to merge_gate.py in the decide step. The record job still runs for every successful workflow_dispatch (line 173) and calls merge_gate_registry.py record with MERGE_GATE_REGISTRY taken from the repo variable. The decision payload does not carry the dry-run flag either. Once the registry is set to on, a manual dry-run decision appends records to ledger.jsonl and creates or reopens flaky-test/broken-main issues. Later entries then reuse those parent controls from the ledger. Fix: skip record when inputs.dry_run is true, or pass --dry-run to the registry in that case.

✅ Quality: Reusing parent controls from the ledger contradicts the same-run ADR

📄 docs/ci.md:113-121 📄 .github/scripts/merge_gate_registry.py:15-23 📄 .github/scripts/merge_gate_registry.py:105-112 📄 docs/decisions/2026-10-13-a-queue-failure-is-forgiven-only-when-the-same-run-proves-it-is-not-the-entrys.md
ADR:2026-10-13-a-queue-failure-is-forgiven-only-when-the-same-run-proves-it-is-not-the-entrys (Accepted) says a failure is forgiven only when "the same run proves" it. It also says failing tests are re-run "in the same run with the run's own builds … then on the parent commit". This PR adds something different: entries on the same parent "reuse a recorded parent control instead of rebuilding the parent" (docs/ci.md:115; ledger reuse in merge_gate_registry.py, read by verification). In that case a pre-existing forgiveness rests on a result produced by another run and stored on a branch that docs/ci.md:120 admits can be forged. Either drop the reuse, or revise the ADR and add a record that states this rule: "A parent-control result recorded on the ci/merge-gate ledger for the same parent SHA may stand in for running the parent control in this run; the ledger keeps 30 days, 5,000 records and 2 MB, and only the gate App may push to it." That record should also give the ledger's branch and line format, which verification and the gate both depend on.

✅ Edge Case: Sweep caps drop the long-running queue runs first under load

📄 .github/scripts/merge_gate.py:366-380
The runs API returns runs newest-created first. cmd_sweep reads at most 3 pages (300 runs) created in the last 6 hours, then walks only finished[:SWEEP_LIMIT]. Already-decided runs count toward that 50, and so do runs that never get a status: sampled-out runs, superseded PRs and entries that have left the queue. So once more than 50 ci.yml runs finish in the 45-minute window, or more than 300 are created in 6 hours, the runs created earliest are cut off. Those are the long queue runs this change exists to catch, so a missed delivery for them still waits for the queue timeout. A fix: drop runs whose latest gate status is already final before applying the cap, sort what is left by created_at or updated_at oldest first, and list event=merge_group runs in a separate query so PR traffic cannot use up the page budget.

✅ Quality: Sweep window and 'pending means undecided' rule are not in any ADR

📄 .github/scripts/merge_gate.py:366-380 📄 .github/scripts/merge_gate.py:426-427 📄 docs/ci.md:48-51
This change sets new judgment-call values for the sweep: it considers runs that finished 15 to 60 minutes ago (--grace-minutes 15, --minutes 60), created within SWEEP_LOOKBACK = 6h. It also adopts a correctness rule: a head whose latest gate status is pending counts as undecided and gets decided again. docs/ci.md describes this, but no record in docs/decisions/ does. ADR:2026-10-13-merge-queue-entries-are-decided-by-the-merge-gate-status only mentions a 10-minute sweep. The record would say: "The sweep decides a finished CI run created within 6 hours that finished 15–60 minutes ago (the grace equals the decide job's timeout, so a delivered decision is never raced), when the head's latest gate status is missing or pending." The record should also note that a held (PENDING) entry is therefore decided again on every sweep in that window.

...and 1 more closed from earlier reviews

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

`Compact`
`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

sonarqubecloud · 2026-10-15T14:12:56Z

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mohityadav766 added 2 commits October 14, 2026 20:04

Merge remote-tracking branch 'origin/main' into ci/merge-gate

355057e

github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Oct 14, 2026

github-advanced-security Bot found potential problems Oct 14, 2026

View reviewed changes

Comment thread .github/scripts/tests/test_merge_gate_registry.py Fixed

Comment thread .github/scripts/ci_ai/bundle.py Fixed

Comment thread .github/scripts/ci_ai/bundle.py Fixed

Comment thread .github/scripts/ci_ai/sanitize.py Fixed

Comment thread .github/scripts/ci_ai/sanitize.py Fixed