googleapis / googleapis/release-please
Manifest mode: release-time body re-parse decodes `<`/`>` and silently drops the component after a section with an escaped `<word>`
- Dominant language
- TypeScript
- Stars
- 7.5k
- Forks
- 588
- Avg merge
- 12h 16m
- Merged PRs (30d)
- 7
Description
Related to #1659 (fixed by #1661) and #2801, but a different route in: here `htmlEscape` **did** escape the commit subject, the stored PR body is correct, and the component is still silently skipped at release time — because release-please parses the merged PR body twice and the first pass decodes its own escaping.
#### Environment details
- OS: macOS 15 (also observed on `ubuntu-latest` in GitHub Actions)
- Node.js version: v24.13.0
- npm version: 11
- `release-please` version: 17.6.0 (via `googleapis/release-please-action@v5.0.0`); reproduced identically on 17.11.2 with `node-html-parser` 6.1.13
- Manifest mode, `separate-pull-requests: false`, ~27 packages, `include-component-in-tag` on
#### What happens
A commit with subject `feat(ci): Record the registry digest and claim :v only on main` touched two packages. `htmlEscape` in `src/changelog-notes/default.ts` correctly wrote `:v<version>` into both CHANGELOGs and both `` sections of the release PR body (verified in the stored body via the API). The PR merged. On the merge run release-please logged, for the second of those two packages only:
```
✔ Building release for path: services/tezos-indexer
✔ Pull request contains releases, but not for component: tezos-indexer
...
⚠ Expected 27 releases, only found 26
⚠ Missing 1 paths: services/tezos-indexer
```
and the run succeeded. Result: manifest, `Cargo.toml` and CHANGELOG at the new version, no tag, no GitHub release. The *first* of the two packages was released, but its GitHub release body contains every later section of the PR as plain text (the swallowed sections' summaries and notes), with `:v` rendered as `:v only on main`. The next run then finds no release for the skipped path, regenerates its notes from the whole commit window and links its CHANGELOG at a tag that never existed.
#### Root cause
1. `Manifest.findMergedReleasePullRequests` ([manifest.ts#L1146-L1173](https://github.com/googleapis/release-please/blob/v17.11.2/src/manifest.ts#L1146-L1173)) runs `pullRequestOverflowHandler.parseOverflow(pullRequest)` → `PullRequestBody.parse(body)` and then yields `{...pullRequest, body: pullRequestBody.toString()}`.
2. `extractMultipleReleases` ([pull-request-body.ts#L124-L146](https://github.com/googleapis/release-please/blob/v17.11.2/src/util/pull-request-body.ts#L124-L146)) takes each section's notes as `detail.textContent.trim()`. `node-html-parser`'s `textContent` **decodes HTML entities**, so `<version>` becomes `` in `releaseData[i].notes`.
3. `PullRequestBody.notes()` / `toString()` ([pull-request-body.ts#L68](https://github.com/googleapis/release-please/blob/v17.11.2/src/util/pull-request-body.ts#L68)) re-emits those notes verbatim, without re-escaping. The re-serialised body now contains a raw `` inside each affected section.
4. Every `Strategy.buildRelease` ([base.ts#L647](https://github.com/googleapis/release-please/blob/v17.11.2/src/strategies/base.ts#L647)) parses that re-serialised body. `node-html-parser` opens `` as an unknown element; when the section's `` arrives it does not match and `version` is not in `kElementsClosedByClosing`, so the closing tag is silently dropped (the `// Use aggressive strategy to handle unmatching markups.` → `break` branch). The still-open stack at the end of input is `[details(a), version, details(b), version]`; `parse()`'s repair loop removes each unclosed element and hoists its children into the parent, which **deletes `details(b)`** (its `` becomes stray text inside `details(a)`) and keeps only the outermost `details(a)`. `getElementsByTagName('details')` therefore no longer returns `pkg-b`.
5. `buildRelease` logs `Pull request contains releases, but not for component: …` at **info** ([base.ts#L685](https://github.com/googleapis/release-please/blob/v17.11.2/src/strategies/base.ts#L685)) and returns.
The condition is: two or more sections whose decoded notes contain `<` + letter (one commit touching two packages is the natural way to get there). The first such section survives (it is the outermost and the repair loop skips the last element under root); every later such section is lost. Sections without the token that sit between or after them survive, because they closed properly and are merely nested. That is why the same PR body can lose one package on one merge and none on the next, depending on section order.
#2801's minimal reproduction calls `PullRequestBody.parse` once and shows the escaped form works — which is exactly why this route is invisible to that test: it needs the round-trip, `parse(parse(body).toString())`.
#### Steps to reproduce
1. `npm install --ignore-scripts --save-exact release-please@17.11.2`
2. Run the script below (it only exercises `PullRequestBody`; no GitHub calls):
```js
const {PullRequestBody} = require('release-please/build/src/util/pull-request-body.js');
// A release PR body exactly as release-please writes it when one commit
// (subject: "feat: claim :v only on main") touches two packages:
// htmlEscape has done its job, both sections carry the ESCAPED form.
const body = [
':robot: I have created a release *beep* *boop*',
'---',
'',
'pkg-a: 1.0.1\n\n### Features\n\n* claim :v<version> only on main\n',
'',
'pkg-b: 2.0.1\n\n### Features\n\n* claim :v<version> only on main\n',
'',
'pkg-c: 3.0.1\n\n### Features\n\n* unrelated\n',
'',
'---',
'footer',
].join('\n');
const components = (prBody) => prBody.releaseData.map((d) => d.component);
// Pass 1: Manifest.findMergedReleasePullRequests -> pullRequestOverflowHandler.parseOverflow
const pass1 = PullRequestBody.parse(body);
console.log('pass 1:', components(pass1));
console.log('pass 1 pkg-a notes still escaped?', pass1.releaseData[0].notes.includes('<version>'));
// Manifest then yields {...pullRequest, body: pullRequestBody.toString()} ...
const reserialised = pass1.toString();
console.log('re-serialised body contains a raw ?', reserialised.includes(''));
// ... and every Strategy.buildRelease parses THAT.
const pass2 = PullRequestBody.parse(reserialised);
console.log('pass 2:', components(pass2));
console.log('pass 2 pkg-a notes:', JSON.stringify(pass2.releaseData[0].notes));
```
Output, identical on 17.6.0 and 17.11.2:
```text
pass 1: [ 'pkg-a', 'pkg-b', 'pkg-c' ]
pass 1 pkg-a notes still escaped? false
re-serialised body contains a raw ? true
pass 2: [ 'pkg-a', 'pkg-c' ]
pass 2 pkg-a notes: "### Features\n\n* claim :v only on main\n\n\npkg-b: 2.0.1\n\n### Features\n\n* claim :v only on main\n\n\npkg-c: 3.0.1\n\n### Features\n\n* unrelated"
```
`pkg-b` is gone from pass 2, and `pkg-a`'s notes (which become its GitHub release body) now contain `pkg-b`'s and `pkg-c`'s sections as text.
#### Suggested fix
Any one of these closes the route; the first two are small:
- In `extractMultipleReleases`, read the notes with `detail.innerHTML` (or `rawText`) rather than `textContent`, so the escaping release-please wrote survives the round-trip. Because the entities are release-please's own, the notes are already in the form `toString()` needs.
- Or re-apply `htmlEscape` (the same one as `changelog-notes/default.ts`, ideally without its inline-code exemption for this purpose) in `PullRequestBody.notes()` when re-serialising.
- Longer term, make `extractMultipleReleases` not depend on a lenient HTML DOM at all — e.g. split on the `…` / `` markers release-please itself emits — so no text inside a section can affect how sibling sections are found. That would also cover #2801 and the ``-in-inline-code crash in #2884.
Independently of the parser: `Pull request contains releases, but not for component` is logged at `info` while the release PR is being marked `autorelease: tagged` and the run succeeds. When the body *does* contain a section for that component (the summary text is right there in a sibling's notes), this is a lost release, and a `warn`/`error` would have made it visible.
#### Real-world case
In a 27-package manifest repo, the release PR merge released 19 of 20 sections; the one dropped was the second of the two packages carrying the escaped subject above. We now run a post-merge step that diffs the manifest against the merge commit's parent and creates any bumped component's missing release, and a commitlint rule that keeps `<` + letter out of commit subjects and `BREAKING CHANGE` notes (the latter are not escaped at all by `default.ts`, so that is a third way into the same parser path).
Contributor guide
Research direction
Start with src/util/pull-request-body.ts, especially extractMultipleReleases and the notes/toString path, then trace the second parse from Manifest.findMergedReleasePullRequests in src/manifest.ts and the release strategy entry point in src/strategies/base.ts. Run the supplied parse(parse(body).toString()) reproduction. Done means all three components remain discoverable and escaped notes stay intact after the round trip.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- release, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100