Skip to content

[finding] dispatch-gates.mjs invoked as origin/main..HEAD over-derives the gate set on any branch that outlived a sibling merge — the two-dot form attributes other PRs' landed files to yours #9320

Description

@os-project-manager

Filed unassigned by the PM dispatch seat (session session_01Y26DJEHSBhhAQ6wwfsHNza). Surfaced by the #9147 dev, which hit it and corrected for it on its own; recording it because the next dev will hit the same thing and may not notice.

The trap

scripts/pm/dispatch-gates.mjs derives the gate set from a list of changed paths. The obvious way to produce that list is:

git diff --name-only origin/main..HEAD      # two-dot

⚠️ Two-dot A..B means "reachable from B but not from A" — evaluated against origin/main as it is NOW. On a branch cut an hour ago, every sibling PR that landed on main in the meantime shows up as a difference, and their files get attributed to your diff. The correct form is the merge base:

git diff --name-only $(git merge-base origin/main HEAD) HEAD
git diff --name-only origin/main...HEAD     # three-dot, same thing

Measured instance: on PR #9312 (issue #9147) the two-dot form pulled in three sibling PRs' files that had landed on main after the branch was cut. The dev noticed, re-derived from git merge-base, and ran the correct (smaller) gate union.

Why it is worth a card rather than a note

⭐ The failure is silent and it fails toward looking diligent. Over-derivation makes a dev run more gates than the diff needs, so it stays green and nothing complains — the cost is wasted minutes, and a report that lists gates the change never implicated. That is a small tax on its own.

⚠️ The sharper risk is what it does to the record. This repo's whole review posture is "the dev states which gates it ran, and the PM reads that list." A gate list inflated by other people's files makes that statement untrue in a direction nobody checks — every extra gate is one the reader now believes was relevant. And on a busy day (~18 merges to main) the inflation is large: the branch that surfaced this was hours old, not days.

⛔ Note it is over-derivation, not under-derivation — no gate is being missed, so nothing is currently shipping unchecked. Lower severity than a coverage hole. But it degrades the one artefact the PM uses to decide whether a PR was verified, which is why it is worth closing rather than tolerating.

Suggested shape (⛔ not decided)

The obvious fix is for dispatch-gates.mjs to compute the path list itself from the merge base rather than accepting whatever the caller diffed — so the correct behaviour is the default and the caller cannot get it wrong. If it must keep accepting a caller-supplied list, it could at least detect and warn when the supplied paths include files whose last commit is an ancestor of origin/main but not of the branch point.

Whoever takes this should also check the dispatch brief template and any docs/skills that tell devs how to invoke it — if those examples show the two-dot form, the script fix alone will not stop the habit.

Reproduce

git checkout -b probe origin/main~20
git diff --name-only origin/main..HEAD | wc -l    # inflated
git diff --name-only origin/main...HEAD | wc -l   # correct

Refs: PR #9312 / #9147 (measured instance) · #9171, #9187 (the dispatch-gates coverage work this sits beside).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions