googleapis / googleapis/release-please

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

Open
#2,883 0 comments 0 reactions 0 assignees View on GitHub
priority: p2 type: bug
Dominant language
TypeScript
Stars
7.5k
Forks
588
Avg merge
12h 16m
Merged PRs (30d)
7

Description

### 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](https://github.com/googleapis/release-please/blob/main/src/manifest.ts#L671-L673):

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

The generator gets its commits from GitHub's GraphQL `history` connection with no `orderBy` — [src/github.ts#L254](https://github.com/googleapis/release-please/blob/main/src/github.ts#L254):

```graphql
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 `) 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 ..` by hand.

Contributor guide

Open the contributing guide

Research direction

Start in src/manifest.ts around the release-commit stopping condition and src/github.ts around the GraphQL history query. Reproduce the release-pr --dry-run scenario with a release PR merged before another PR, then compare the walk with git log ... Done means unreleased commits are not silently omitted, with the behavior covered by an appropriate regression test.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, github, graphql, typescript
Domain
api, release
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.