Skip to content

Commit walk is chronological, not topological, so commits on the release branch can be silently omitted from a release #2883

Description

@MGrin

What happened

A fix: commit that was on main and not reachable from the previous release tag was not included in the release PR, and nothing reported it. Two of four releasable commits were listed. There was no error, no warning at default verbosity, and CI was green.

Why

Manifest.buildPullRequests walks the release branch through a commit generator and stops as soon as it has seen the previous release's SHA — src/manifest.ts#L671-L673:

} else if (!needsBootstrap && releaseCommitsFound >= expectedShas) {
  // found enough commits
  break;
}

The generator gets its commits from GitHub's GraphQL history connection with no orderBysrc/github.ts#L254:

history(first: $num, after: $cursor) { … }

history defaults to committer date, descending. So the walk is chronological, not topological, and "I have reached the last release's commit" is not the same statement as "everything after this point is already released".

A branch cut before a release PR merged and merged after it has commits whose committer dates predate the release commit. They therefore sort behind the stop point and are never visited — even though they are on the release branch and are not reachable from the release tag.

Reproduction

  1. Open PR A. Let its commits be committed at T0.
  2. release-please opens a release PR. Merge it at T1 > T0. The release commit's committer date is T1.
  3. Merge PR A at T2 > T1. Its branch commits keep their T0 committer dates.
  4. release-please runs. The walk sees the release commit at T1 before it reaches PR A's commits at T0, hits the break, and never reads them.

Measured on release-please 17.11.1 against a real repository with release-pr --dry-run:

walk position commit committed
8 merge of the PR from step 3 08:39:00Z
9 previous release commit — the walk stops here 08:26:51Z
15 fix(...) from step 1 08:12:49Z
16 fix(...) from step 1 08:12:49Z

Splitting 8 commits by path. Two of the four releasable commits reached the changelog.

Why this is not a configuration mistake

  • bootstrap-sha self-disables: needsBootstrap is false once a matching tag exists, so that branch of the condition never fires.
  • last-release-sha does not widen the walk — the count-based break above is a sibling else if and fires regardless.
  • No config key widens the walk past a matched release SHA.

Suggested fix

The stop condition wants to be topological — "this commit is an ancestor of the last release" rather than "this commit is the last release". Two shapes that would work:

  • keep walking past the release SHA until the set of unreleased commits is exhausted, i.e. treat the release tag as an exclusion (main --not <tag>) rather than as a stopping point; or
  • ask GitHub for the comparison directly (GET /repos/{owner}/{repo}/compare/{tag}...{branch}), which is topological by construction.

Either removes the failure entirely.

A cheaper mitigation, if the walk shape has to stay: warn when the walk terminates on the release SHA while commits remain that are not reachable from the release tag.

Impact

The trigger is routine — it fires whenever a release PR merges while another pull request is open. The failure is silent in both directions: the release notes are simply short, and the only way to find it is to diff the release PR against git log <tag>..<branch> by hand.

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions