fix(ci): compare names one workflow run settles the same way

A `name:` whose only leftover expressions read a `github.` property other
than `github.job` is filled in identically for every job of the run that
publishes it, so two jobs of one workflow carrying it land on the same
check run. Those names now compare against the other jobs of their own
file instead of sitting in the blind-spot bucket. They stay out of the
comparison across files, where two workflows can run on different events
This commit is contained in:
mateo-berri 2026-09-06 02:32:50 -07:00
parent aa2c41f489
commit 5a27e11263
2 changed files with 148 additions and 10 deletions

View file

@ -27,12 +27,16 @@ real `<shard> / Run tests` names rather than staying opaque.
Whatever the sweep cannot work out is left out of the comparison and reported
instead of guessed, because a guess that lands wrong fails a workflow GitHub
would have published perfectly well. A name still holding an expression once the
combination is filled in is one GitHub resolves per job, so it is one of those:
guessing that two jobs sharing such a template clash would fail workflows over a
context this sweep cannot read. A matrix that is itself an expression or that
lists values which are not scalars, an `include` or `exclude` row shaped the same
way, a whole `strategy:` that comes from an expression, and a call this sweep
cannot follow, go in the same bucket. The cost is that a real clash hiding behind
combination is filled in is usually one GitHub resolves per job, so it is one of
those: guessing that two jobs sharing such a template clash would fail workflows
over a context this sweep cannot read. The exception is a name whose leftover
expressions all read a `github.` property other than `github.job`, which one run
fills in the same way for every job in it, so those are compared against the
other jobs of their own workflow and stay out of the comparison across files,
where two workflows can run on different events. A matrix that is itself an
expression or that lists values which are not scalars, an `include` or `exclude`
row shaped the same way, a whole `strategy:` that comes from an expression, and a
call this sweep cannot follow, go in the same bucket. The cost is that a real clash hiding behind
one of them goes unseen, which leaves a merge no worse off than before this check
existed, where the opposite direction would block work that was fine.
@ -64,6 +68,7 @@ MATRIX_REF: Final = re.compile(r"^matrix\.(?P<key>[\w-]+)$")
LITERAL: Final = re.compile(r"^'(?P<text>[^']*)'$")
FORMAT_CALL: Final = re.compile(r"^format\((?P<args>.*)\)$", re.DOTALL)
COMPARISON: Final = re.compile(r"^(?P<left>.+?)\s*(?P<operator>==|!=)\s*(?P<right>.+)$", re.DOTALL)
RUN_WIDE: Final = re.compile(r"^github\.(?!job\b)[\w.]+$")
GITHUB_PLACEHOLDER: Final = re.compile(r"\{\{|\}\}|\{\d+\}")
NO_MATRIX: Final[Mapping[str, str]] = MappingProxyType({})
NO_CALLERS: Final[frozenset[str]] = frozenset()
@ -88,6 +93,7 @@ class Names:
known: tuple[str, ...] = ()
unknown: tuple[str, ...] = ()
local: tuple[str, ...] = ()
class Job(BaseModel):
@ -300,10 +306,17 @@ def comparable(name: str) -> bool:
return EXPRESSION.search(name) is None
def run_wide(name: str) -> bool:
"""A name whose leftover expressions one workflow run fills in the same way for every job in it."""
return all(RUN_WIDE.match(span.group("body").strip()) is not None for span in EXPRESSION.finditer(name))
def settled(names: Sequence[str]) -> Names:
unresolved: Final = tuple(name for name in names if not comparable(name))
return Names(
tuple(name for name in names if comparable(name)),
tuple(f"its name stays `{name}`" for name in names if not comparable(name)),
tuple(f"its name stays `{name}`" for name in unresolved if not run_wide(name)),
tuple(name for name in unresolved if run_wide(name)),
)
@ -340,9 +353,15 @@ def joined(groups: Sequence[Names]) -> Names:
return Names(
tuple(name for group in groups for name in group.known),
tuple(reason for group in groups for reason in group.unknown),
tuple(name for group in groups for name in group.local),
)
def tagged(names: Names) -> tuple[tuple[str, bool], ...]:
"""Each name a job publishes beside whether only its own workflow's run settles it."""
return (*((name, False) for name in names.known), *((name, True) for name in names.local))
def call_blocker(job: Job, workflows: Mapping[str, Workflow], callers: frozenset[str]) -> str | None:
path: Final = callee_path(job)
if path is None:
@ -366,9 +385,15 @@ def job_names(job_id: str, job: Job, workflows: Mapping[str, Workflow], callers:
for callee_id, callee_job in workflows[path].jobs.items()
)
)
composed: Final = tuple(
(f"{prefix} / {suffix}", prefix_local or suffix_local)
for prefix, prefix_local in tagged(prefixes)
for suffix, suffix_local in tagged(suffixes)
)
return Names(
tuple(f"{prefix} / {suffix}" for prefix in prefixes.known for suffix in suffixes.known),
tuple(name for name, is_local in composed if not is_local),
(*prefixes.unknown, *suffixes.unknown),
tuple(name for name, is_local in composed if is_local),
)
@ -432,9 +457,33 @@ def clash(name: str, owners: Sequence[str]) -> str | None:
return None
def local_published(sources: Mapping[str, str]) -> Iterator[tuple[tuple[str, str], str]]:
"""Names their own workflow's run settles, keyed by the file whose run settles them."""
for rel, job_id, names in scanned_jobs(sources):
for name in names.local:
yield (rel, name), f"job `{job_id}`"
def local_clash(rel: str, name: str, owners: Sequence[str]) -> str | None:
"""Why one workflow's own run lands several of its jobs on one check run."""
if len(owners) < 2:
return None
jobs: Final = tuple(dict.fromkeys(owners))
return (
f"`{name}` is published {len(owners)} times inside {rel}, by {', '.join(jobs)}. One run fills that "
f"expression in the same way throughout, so they all land on one check run; make the names differ."
)
def local_clashes(sources: Mapping[str, str]) -> tuple[str, ...]:
grouped: Final = itertools.groupby(sorted(local_published(sources)), key=operator.itemgetter(0))
found: Final = tuple(local_clash(rel, name, tuple(owner for _, owner in pairs)) for (rel, name), pairs in grouped)
return tuple(message for message in found if message is not None)
def collisions(sources: Mapping[str, str]) -> tuple[str, ...]:
found: Final = tuple(clash(name, owners) for name, owners in owners_by_name(sources))
return tuple(message for message in found if message is not None)
return (*(message for message in found if message is not None), *local_clashes(sources))
def workflow_sources() -> Mapping[str, str]:

View file

@ -302,7 +302,7 @@ def test_a_matrix_list_supplies_values_the_same_way_include_rows_do() -> None:
assert "`Analyze (go)` is published by 2 jobs" in found[0]
def test_two_jobs_sharing_a_template_over_the_run_itself_are_reported_rather_than_guessed() -> None:
def test_two_workflows_sharing_a_run_wide_template_are_not_called_a_collision() -> None:
template: Final = (
"on: pull_request\njobs:\n {job}:\n name: ${{{{ github.event_name }}}}-build\n runs-on: ubuntu-latest\n"
)
@ -311,10 +311,99 @@ def test_two_jobs_sharing_a_template_over_the_run_itself_are_reported_rather_tha
"b.yml": template.format(job="two"),
}
assert collisions(sources) == ()
assert blind_spots(sources) == ()
def test_two_jobs_of_one_workflow_sharing_a_run_wide_template_are_a_collision() -> None:
sources: Final = {
"a.yml": (
"on: pull_request\njobs:\n"
" one:\n name: ${{ github.event_name }}-build\n runs-on: ubuntu-latest\n"
" two:\n name: ${{ github.event_name }}-build\n runs-on: ubuntu-latest\n"
),
}
found: Final = collisions(sources)
assert len(found) == 1
assert "is published 2 times inside a.yml, by job `one`, job `two`" in found[0]
assert exit_code(sources) == 1
def test_a_run_wide_template_carrying_a_matrix_value_does_not_collide_inside_one_workflow() -> None:
sources: Final = {
"a.yml": (
"on: pull_request\njobs:\n"
" one:\n name: ${{ github.event_name }}-${{ matrix.shard }}\n runs-on: ubuntu-latest\n"
" strategy:\n matrix:\n shard: [core, extras]\n"
),
}
assert collisions(sources) == ()
assert blind_spots(sources) == ()
def test_a_run_wide_name_repeated_over_a_matrix_by_one_job_is_a_collision() -> None:
sources: Final = {
"a.yml": (
"on: pull_request\njobs:\n"
" one:\n name: ${{ github.event_name }}-build\n runs-on: ubuntu-latest\n"
" strategy:\n matrix:\n shard: [core, extras]\n"
),
}
found: Final = collisions(sources)
assert len(found) == 1
assert "is published 2 times inside a.yml, by job `one`" in found[0]
def test_a_name_reading_the_job_it_sits_in_stays_out_of_the_comparison() -> None:
sources: Final = {
"a.yml": (
"on: pull_request\njobs:\n"
" one:\n name: ${{ github.job }}-build\n runs-on: ubuntu-latest\n"
" two:\n name: ${{ github.job }}-build\n runs-on: ubuntu-latest\n"
),
}
assert collisions(sources) == ()
assert len(blind_spots(sources)) == 2
def test_a_run_wide_caller_name_collides_through_the_workflow_it_calls() -> None:
sources: Final = {
".github/workflows/a.yml": (
"on: pull_request\njobs:\n"
" one:\n name: ${{ github.event_name }}\n uses: ./.github/workflows/c.yml\n"
" two:\n name: ${{ github.event_name }}\n uses: ./.github/workflows/c.yml\n"
),
".github/workflows/c.yml": "on:\n workflow_call:\njobs:\n build:\n runs-on: ubuntu-latest\n",
}
found: Final = collisions(sources)
assert len(found) == 1
assert "github.event_name }} / build` is published 2 times inside .github/workflows/a.yml" in found[0]
def test_a_run_wide_name_inside_a_called_workflow_collides_under_the_caller() -> None:
sources: Final = {
".github/workflows/a.yml": ("on: pull_request\njobs:\n one:\n uses: ./.github/workflows/c.yml\n"),
".github/workflows/c.yml": (
"on:\n workflow_call:\njobs:\n"
" build:\n name: ${{ github.event_name }}\n runs-on: ubuntu-latest\n"
" lint:\n name: ${{ github.event_name }}\n runs-on: ubuntu-latest\n"
),
}
found: Final = collisions(sources)
assert len(found) == 1
assert "`one / ${{ github.event_name }}` is published 2 times inside .github/workflows/a.yml" in found[0]
def test_a_name_reading_the_workflow_it_sits_in_is_not_called_a_collision() -> None:
template: Final = (
"on: pull_request\njobs:\n {job}:\n name: ${{{{ github.workflow }}}} / build\n runs-on: ubuntu-latest\n"