Literal "null" is rendered in the pull request timeline after CI finishes
type/bug
## What to build
Leaving a pull request page open while its CI is still running can render the literal text `null` in the conversation timeline, and the merge box disappears until the page is reloaded.
Root cause:
- While the PR status is pending/checking, the merge box is refreshed every 5s by `GET /{owner}/{repo}/pulls/{index}/merge_box`, and the response is applied with `createElementFromHTML(await resp.text())` followed by `el.replaceWith(newEl)`.
- `createElementFromHTML()` returns `div.firstChild`, which is `null` for an empty/whitespace body, while its declared return type claims a non-null element.
- `ParentNode.replaceWith()` accepts `(Node or DOMString)`, so `null` is converted to the string `"null"`: the merge box element is destroyed and a literal `null` text node is inserted into the timeline. Because the element is gone, `data-global-init` never re-registers the refresh timer, so the merge box never comes back without a full page reload.
- The fragment can legitimately be an empty `200` response: the whole merge box template is wrapped in `{{if $data.ShowMergeBox}}`, and `ShowMergeBox` is false for a merged PR whose head branch is not deletable (branch deleted after merge, no delete permission, or the session expired so the poll became anonymous). So when a PR is merged while the page is polling - for example auto-merge firing once CI turns green - the next refresh returns nothing and the timeline shows `null`.
- The same symptom can be produced by any template execution error in this fragment, because the page renderer writes the HTTP status before executing the template, so a render failure still returns `200` with an empty body.
Fix direction:
- The merge box fragment endpoint must never return an empty body: render a minimal empty `.pull-merge-box` placeholder element when there is no merge box to show, so a refresh keeps replacing exactly one element and simply stops reloading.
- Make the empty/invalid fragment unrepresentable on the frontend too: `createElementFromHTML()` must express that it can return nothing, and the merge box refresh must bail out instead of injecting `null`.
## Acceptance criteria
- [ ] `GET /{owner}/{repo}/pulls/{index}/merge_box` always responds with exactly one `.pull-merge-box` element, including for a merged pull request whose head branch no longer exists.
- [ ] The rendered pull request page looks unchanged for merged pull requests that currently show no merge box.
- [ ] A merge box refresh that receives an empty or non-element response leaves the timeline untouched and never inserts a `null` text node.
- [ ] `createElementFromHTML()` no longer claims a non-null return value, and callers that can receive nothing handle it.
- [ ] Unit test covers the empty-fragment refresh path, and an integration test covers the endpoint always returning a merge box element.
## Blocked by
- None - can start immediately.
---
Generated by Codet
1 条评论