Skip to content

Literal "null" is rendered in the pull request timeline after CI finishes #38738

Description

@lunny

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions