Skip to content

test(ci): derive the merge_group subscription floor from REQUIRED_CONTEXTS#6187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yinlianghui-tw merged 1 commit into from
Aug 25, 2026
Merged

test(ci): derive the merge_group subscription floor from REQUIRED_CONTEXTS #6187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions content/docs/guide/ci-cd-pipeline.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -81,11 +81,14 @@ checks it requires are green **on that rebuilt commit**. Those runs are a distin
`merge_group`, on a throwaway `gh-readonly-queue/**` branch — a workflow that does not subscribe
to that event simply does not run there.

Which workflows subscribe is deliberately not listed here. `MUST_SUBSCRIBE_MERGE_GROUP` in
`scripts/__tests__/merge-queue-reporting.test.ts` is the maintained list, and the only copy
anything reads — it records why each entry is on it, and an assertion fails when one of them drops
the trigger. A copy of it on this page would be right the day it was written and quietly wrong
after the next subscriber landed, which is exactly what this paragraph used to do
Which workflows subscribe is deliberately not listed here, and is not maintained by hand anywhere
either: `scripts/__tests__/merge-queue-reporting.test.ts` derives the floor from
`REQUIRED_CONTEXTS` — every workflow producing a check that list declares blocking must subscribe,
and an assertion fails the moment one of them does not. `MUST_SUBSCRIBE_MERGE_GROUP` in the same
file records *why* particular members are requirable; a further assertion holds it to being a
subset of the derived floor, so the two cannot drift apart. A copy of the list on this page would
be right the day it was written and quietly wrong after the next subscriber landed, which is
exactly what this paragraph used to do
([#4154](https://github.com/objectstack-ai/objectui/issues/4154)). What is worth knowing here is
the rule that decides membership, not the instances: a gate that carries no path filter reports on
every pull request and is therefore requirable — and a requirable context that skips the queue
Expand DownExpand Up@@ -115,11 +118,14 @@ queued PR burns an hour and fails, with nothing red to point at.

Two things follow for anyone editing this directory:

- **A workflow producing a context that could ever be required must subscribe `merge_group`**,
and takes an entry in `MUST_SUBSCRIBE_MERGE_GROUP` (above) naming the context it produces. That
entry is what fails the build if the workflow later drops the trigger; nothing derives the set,
because "may this context be required?" is a property of the repository's settings, which no
test here can read.
- **A workflow producing a context that could ever be required must subscribe `merge_group`.**
Nothing has to be added to a list for that to be enforced: name the context in
`REQUIRED_CONTEXTS` (`scripts/dependabot-merge-gate.mjs`), which is where this repository already
writes down that a check is blocking and reports on every pull request, and the workflow is
inside the derived floor from that moment. "May this context be required?" is still a property of
the repository's settings that no test here can read — `REQUIRED_CONTEXTS` is a human's answer to
it, and deriving from that answer beats writing it down a second time and watching the copies
drift ([#6160](https://github.com/objectstack-ai/objectui/issues/6160)).
- **Some contexts can never be required, structurally**, and no amount of triggering changes
that. Each line below is blocked by a *different* property, which is why they are all worth
reading; they are examples rather than a census, so a further workflow carrying any of these
Expand Down
93 changes: 11 additions & 82 deletions scripts/__tests__/dependabot-merge-gate.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,6 +13,12 @@ import {
renderVerdict,
waitForGate,
} from '../dependabot-merge-gate.mjs';
import {
type Workflow,
producedCheckNames,
pullRequestTrigger,
readWorkflows,
} from './workflow-checks.js';

/**
* objectui#4973 — `dependabot-auto-merge.yml` merged a pull request 8m20s before
Expand DownExpand Up@@ -406,91 +412,14 @@ describe('the refusal is legible', () => {
});

// ── The buckets versus the workflows that actually produce the checks ────────

type Workflow = {
file: string;
text: string;
/** Lines with comments stripped, so prose mentioning `pull_request:` cannot count. */
lines: string[];
};

function readWorkflows(): Workflow[] {
return fs
.readdirSync(workflowDir)
.filter((file) => file.endsWith('.yml') || file.endsWith('.yaml'))
.map((file) => {
const text = fs.readFileSync(path.join(workflowDir, file), 'utf8');
return { file, text, lines: text.split('\n').filter((line) => !/^\s*#/.test(line)) };
});
}

/** The `on:` block of a workflow, comment lines already stripped. */
function triggerBlock(workflow: Workflow): string[] {
const start = workflow.lines.findIndex((line) => /^on:/.test(line));
if (start === -1) return [];
const rest = workflow.lines.slice(start + 1);
const end = rest.findIndex((line) => /^[A-Za-z]/.test(line));
return end === -1 ? rest : rest.slice(0, end);
}

/**
* Does this workflow subscribe `pull_request`, and does that subscription carry
* a path filter? `pull_request_target` deliberately does not count:
* `cross-repo-issue-closer.yml` uses it with `types: [closed]`, so it acts after
* a merge and has no verdict to contribute to one.
*/
function pullRequestTrigger(workflow: Workflow): { subscribes: boolean; filtered: boolean } {
const block = triggerBlock(workflow);
const start = block.findIndex((line) => /^ {2}pull_request:\s*$/.test(line));
if (start === -1) return { subscribes: false, filtered: false };

const rest = block.slice(start + 1);
const end = rest.findIndex((line) => /^ {2}\S/.test(line));
const sub = end === -1 ? rest : rest.slice(0, end);
return { subscribes: true, filtered: sub.some((line) => /^ {4}paths(-ignore)?:/.test(line)) };
}

/**
* The check names a workflow's jobs appear under. A job's `name:` if it has one,
* else its id (`labeler.yml`'s job is simply `label`), with `matrix.shard`
* expanded. Every job of a subscribing workflow produces a check run on a pull
* request — including one skipped by a job-level `if:`, which reports
* `conclusion=skipped` rather than not existing.
*/
function checkNames(workflow: Workflow): string[] {
const jobsAt = workflow.lines.findIndex((line) => /^jobs:\s*$/.test(line));
if (jobsAt === -1) return [];
const body = workflow.lines.slice(jobsAt + 1);

const starts: number[] = [];
body.forEach((line, index) => {
if (/^ {2}[A-Za-z0-9_-]+:\s*$/.test(line)) starts.push(index);
});

return starts.flatMap((start, i) => {
const block = body.slice(start, starts[i + 1] ?? body.length);
const id = block[0].trim().replace(/:$/, '');
const named = block.find((line) => /^ {4}name:/.test(line));
const name = named ? named.replace(/^ {4}name:\s*/, '').trim() : id;

const shards = block.find((line) => /^ {8}shard: \[/.test(line));
if (!shards || !name.includes('matrix.shard')) return [name];

return (shards.match(/\[(.*)\]/)?.[1] ?? '')
.split(',')
.map((shard) => shard.trim())
.filter(Boolean)
.map((shard) => name.replace(/\$\{\{\s*matrix\.shard\s*\}\}/g, shard));
});
}
//
// The parser these assertions run on now lives in `./workflow-checks.ts`, so
// that `merge-queue-reporting.test.ts` can derive its `merge_group` floor from
// the same answer instead of parsing the workflows a second time (objectui#6160).

describe('the declared buckets partition what a pull request actually produces', () => {
const workflows = readWorkflows().filter((workflow) => pullRequestTrigger(workflow).subscribes);

const produced = new Map<string, string>();
for (const workflow of workflows) {
for (const name of checkNames(workflow)) produced.set(name, workflow.file);
}
const produced = producedCheckNames();

it('found the workflows and the shard matrix (the parser still parses)', () => {
expect(workflows.map((w) => w.file)).toContain('ci.yml');
Expand Down
186 changes: 180 additions & 6 deletions scripts/__tests__/merge-queue-reporting.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,6 +3,9 @@ import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';

import { REQUIRED_CONTEXTS } from '../dependabot-merge-gate.mjs';
import { producedCheckNames, readWorkflows, subscribesMergeGroup } from './workflow-checks.js';

/**
* objectui#3523 — the merge queue was enforced and validated nothing.
*
Expand DownExpand Up@@ -51,11 +54,27 @@ const doc = fs.readFileSync(path.join(repoRoot, DOC), 'utf8');
/**
* `filename -> why this workflow must subscribe merge_group`.
*
* A hand-maintained list, and it has to be: "may this context be required?" is a
* property of the repository's settings, not of the YAML, so nothing mechanical
* can derive the set. What IS mechanical is the honesty check below — an entry
* naming a workflow that no longer exists fails, so the list cannot rot into a
* comfortable fiction the way a stale count does (#3261).
* Hand-maintained, and it carries the REASONS only — since objectui#6160 it no
* longer decides the MEMBERSHIP. `DERIVED_MERGE_GROUP_FLOOR` below does that,
* out of `REQUIRED_CONTEXTS`.
*
* The header this replaced said the set could not be derived at all, because
* "may this context be required?" is a property of the repository's settings and
* not of the YAML. That premise is still true and the conclusion no longer
* follows: `REQUIRED_CONTEXTS` in `scripts/dependabot-merge-gate.mjs` is already
* a human's written-down answer to exactly that question, so deriving from it
* mechanises nothing — it stops one judgement being written down twice in two
* files that then drift apart. Which is what happened: the map named six
* workflows while eight produced an unfiltered blocking check, and by the time
* anyone acted on that it was seven against ten, because PR #6159 added its own
* entry by hand without closing the class (objectui#6160).
*
* Two mechanical honesty checks keep what is left of it true: an entry naming a
* workflow that no longer exists fails, so the list cannot rot into a
* comfortable fiction the way a stale count does (#3261); and an entry naming a
* workflow that produces no required context fails, so this map can only ever be
* a SUBSET of the derived floor and the derivation cannot silently narrow past
* it.
*/
const MUST_SUBSCRIBE_MERGE_GROUP = new Map<string, string>([
['ci.yml', 'produces Type Check, Build & E2E, Test (shard N/4) and Changeset Fixed Group Check'],
Expand DownExpand Up@@ -83,6 +102,70 @@ const MUST_SUBSCRIBE_MERGE_GROUP = new Map<string, string>([
],
]);

/**
* The floor, DERIVED — objectui#6160.
*
* ## What it is protecting
*
* `main` sits behind an enforced merge queue (#3243). A context the repository
* REQUIRES, produced by a workflow that does not subscribe `merge_group`, never
* reports on the queue build — and a required check that never reports does not
* FAIL the queue, it STALLS it until the ruleset's 60-minute status-check
* timeout. That is the whole reason a floor exists here at all: objectui#3523,
* where nothing subscribed, the queue's required set could therefore only be
* empty, and #3503 / #3510 / #3516 merged on 2026-08-07 with `Type Check` at
* `conclusion=failure`.
*
* ## Why it derives instead of being listed
*
* `REQUIRED_CONTEXTS` is the other place this repository writes down "this check
* is blocking and reports on every pull request". Every name in it is a check a
* maintainer may put in the required set, so every workflow producing one is
* exactly a workflow that must subscribe `merge_group`. Reading the membership
* off that list instead of off a second hand-kept one means a gate added to
* `REQUIRED_CONTEXTS` is inside this floor the moment it is added, with nobody
* having to remember a second file.
*
* The check names come from `REQUIRED_CONTEXTS`; the name -> workflow-file
* mapping is `producedCheckNames()`, the same parser
* `dependabot-merge-gate.test.ts` partitions its buckets with. One parser, so
* the two files cannot disagree about which workflow produces what.
*
* ## The precondition, and why it is pinned rather than assumed
*
* A derived floor is an improvement only while it is a SUPERSET of what the hand
* map named. A derivation that quietly narrows would read as more coverage while
* asserting less — this card's own defect, one level up. So the containment is
* itself an assertion below ("loses nothing the hand-maintained map named"), and
* so is the resolvability of every `REQUIRED_CONTEXTS` name: a renamed check that
* resolves to no workflow drops that workflow out of the floor silently, which is
* the same narrowing wearing a different hat.
*
* Measured on this tree when the derivation landed: every one of the map's
* entries produces a `REQUIRED_CONTEXTS` check (nothing is lost) and every
* workflow producing one subscribes `merge_group` (the floor is satisfiable
* today, and no workflow YAML needed changing — the missing thing was the
* assertion, not the trigger).
*/
function deriveMergeGroupFloor(): { byWorkflow: Map<string, string[]>; unresolved: string[] } {
const produced = producedCheckNames();
const byWorkflow = new Map<string, string[]>();
const unresolved: string[] = [];

for (const context of REQUIRED_CONTEXTS) {
const file = produced.get(context);
if (!file) {
unresolved.push(context);
continue;
}
byWorkflow.set(file, [...(byWorkflow.get(file) ?? []), context]);
}

return { byWorkflow, unresolved };
}

const DERIVED_MERGE_GROUP_FLOOR = deriveMergeGroupFloor();

/** Workflows whose path filtering had to move from the trigger into the jobs. */
const FILTER_MOVED_INTO_JOBS = ['ci.yml', 'lint.yml'];

Expand DownExpand Up@@ -212,6 +295,85 @@ describe('every requirable context reports on a merge-queue build (#3523 step 1)
});
});

describe('the merge_group floor derives itself from REQUIRED_CONTEXTS (#6160)', () => {
const { byWorkflow, unresolved } = DERIVED_MERGE_GROUP_FLOOR;

it('resolves every required context to the workflow that produces it', () => {
// Non-vacuity guard, and it comes first because the two assertions after it
// are both quantified over `byWorkflow`: a derivation that resolved nothing
// would make them pass while asserting nothing at all — green because
// nothing was produced, the failure mode this whole file is about.
expect(
unresolved,
`These names are in \`REQUIRED_CONTEXTS\` but no \`pull_request\` workflow produces a check ` +
`by that name:\n` +
unresolved.map((name) => ` - ${name}`).join('\n') +
`\n\nEach one is a workflow silently dropped OUT of the merge_group floor below — the ` +
`derivation cannot require a subscription of a workflow it failed to identify. Usually a ` +
`renamed job: fix the name on whichever side is stale. (\`dependabot-merge-gate.test.ts\` ` +
`fails on the same drift from the other direction — there it means the gate waits forever ` +
`for a context nothing creates.)`,
).toEqual([]);

expect(
byWorkflow.size,
`The derivation resolved no workflows at all, so the floor below is empty and asserts ` +
`nothing. Either \`REQUIRED_CONTEXTS\` was emptied or \`producedCheckNames()\` stopped ` +
`parsing \`.github/workflows\`.`,
).toBeGreaterThan(0);
});

it('requires merge_group of every workflow that produces a required context', () => {
const workflows = new Map(readWorkflows().map((workflow) => [workflow.file, workflow]));

const missing = [...byWorkflow.keys()].filter((file) => {
const workflow = workflows.get(file);
return !workflow || !subscribesMergeGroup(workflow);
});

expect(
missing,
`These workflows produce a context \`REQUIRED_CONTEXTS\` declares blocking, but do not ` +
`subscribe \`merge_group\`:\n` +
missing.map((f) => ` - ${f} (produces ${byWorkflow.get(f)?.join(', ')})`).join('\n') +
`\n\nThe merge queue is ENFORCED here (#3243). A required context that does not report on ` +
`a queue build does not fail the queue, it STALLS it until the ruleset's 60-minute ` +
`status-check timeout — every queued pull request burns an hour and fails with nothing ` +
`red to point at. Add \`merge_group:\` to the workflow's \`on:\` block; it is a pure ` +
`addition and changes nothing about pull requests (objectui#3523 step 1, where the ` +
`opposite state let #3503 / #3510 / #3516 merge with \`Type Check\` at ` +
`conclusion=failure).\n\nIf the context genuinely cannot be required, it does not belong ` +
`in \`REQUIRED_CONTEXTS\` — move it to \`OPTIONAL_CONTEXTS\` or \`NOT_A_GATE\` in ` +
`\`scripts/dependabot-merge-gate.mjs\` with the reason, rather than weakening this floor.`,
).toEqual([]);
});

it('loses nothing the hand-maintained map named', () => {
// The precondition of the whole derivation, pinned rather than assumed: the
// derived floor must CONTAIN what the hand-maintained map named, or swapping
// one for the other would look like more coverage while asserting less.
//
// It fails in the direction that matters. Adding a workflow to the map that
// produces no `REQUIRED_CONTEXTS` check reads as "the map now covers
// something the derivation does not" — which is the two declarations
// crossing, and is exactly what may not happen quietly.
const lost = [...MUST_SUBSCRIBE_MERGE_GROUP.keys()].filter((file) => !byWorkflow.has(file));

expect(
lost,
`MUST_SUBSCRIBE_MERGE_GROUP names these workflows, but none of them produces a check listed ` +
`in \`REQUIRED_CONTEXTS\`:\n` +
lost.map((f) => ` - ${f} (${MUST_SUBSCRIBE_MERGE_GROUP.get(f)})`).join('\n') +
`\n\nSince objectui#6160 the floor is DERIVED from \`REQUIRED_CONTEXTS\`, and this map ` +
`carries only the reasons. So the two declarations have just crossed: either the check ` +
`belongs in \`REQUIRED_CONTEXTS\` (add it there — that is what puts the workflow inside ` +
`the floor), or it is not requirable and the entry here is claiming a floor nothing ` +
`enforces (drop it). Leaving it is the one option that is not available: it would be a ` +
`hand-maintained membership again, the thing that fell behind twice before anyone noticed.`,
).toEqual([]);
});
});

describe('every context reports on every pull request (#3523 step 2)', () => {
it.each(FILTER_MOVED_INTO_JOBS)('%s filters no pull request at the trigger', (file) => {
const pullRequest = nestedBlock(topLevelBlock(withoutComments(read(file)), 'on'), 'pull_request');
Expand DownExpand Up@@ -441,11 +603,23 @@ const namesWorkflow = (text: string, file: string): boolean =>
new RegExp(`(?<![\\w./-])${file.replace(/\./g, '\\.')}`).test(text);

describe('ci-cd-pipeline.md does not keep its own copy of the subscriber list (#4154)', () => {
/**
* Every workflow the page may not enumerate: the map's entries AND the derived
* floor's, which since objectui#6160 is the larger set. Scanning only the map
* would have let the page name the subscribers the map does not — the same
* drift this describe exists to stop, hiding in the gap the map had already
* fallen behind by.
*/
const CURRENT_SUBSCRIBERS = new Set([
...MUST_SUBSCRIBE_MERGE_GROUP.keys(),
...DERIVED_MERGE_GROUP_FLOOR.byWorkflow.keys(),
]);

it('names no current subscriber outside the dated #3523 paragraph', () => {
const offenders: string[] = [];
for (const paragraph of paragraphsOf(mergeQueueSection())) {
const dated = paragraph.includes(HISTORY_ANCHOR);
for (const file of MUST_SUBSCRIBE_MERGE_GROUP.keys()) {
for (const file of CURRENT_SUBSCRIBERS) {
if (!namesWorkflow(paragraph, file)) continue;
if (dated && HISTORY_NON_SUBSCRIBERS.includes(file)) continue;
offenders.push(`${file} — in ${dated ? 'the dated #3523 paragraph' : 'a live paragraph'}: "${paragraph.split('\n')[0].slice(0, 72)}…"`);
Expand Down
Loading
Loading