Skip to content

Fix merged state in pull request list output - #3468

Open
po-et wants to merge 3 commits into
github:mainfrom
po-et:fix/list-pr-merged-state
Open

po-et wants to merge 3 commits into
github:mainfrom
po-et:fix/list-pr-merged-state

Conversation

@po-et

@po-et po-et commented Oct 9, 2026 •

Copy link
Copy Markdown

Summary

Correct list_pull_requests output for merged pull requests when the REST list endpoint omits the merged boolean. Use a nonzero merged_at as a fallback while preserving an explicit boolean.

Why

An agent listing closed PRs currently sees merged: false even for merged changes, which can corrupt contribution summaries. The regression reproduces this from raw REST JSON at current main.

Fixes #3467

What changed

  • Fall back to the merge timestamp only when the API omits merged.
  • Cover merged, closed unmerged, open, explicit true/false, and zero timestamp entries, with and without field selection.
  • Verify legacy text output and modern MCP structured output.

MCP impact

  • Tool schema or behavior changed

The existing merged output now correctly describes list entries. No input or output fields are added.

Prompts tested (tool changes only)

Regression calls use list_pull_requests with owner, repo, and state: "all", both with all fields and with fields: ["number", "merged", "merged_at"]. No natural-language model prompts were run.

Security / limits

  • No security or limits impact

The conversion uses data already returned by the API and adds no requests.

Tool renaming

  • I am not renaming tools as part of this PR

Lint & tests

  • Repository-pinned golangci-lint 2.14.0 reports 0 issues
  • Full go test -race -count=1 -json ./... passed on Go 1.26.8 / macOS arm64
  • go vet ./..., go build ./..., go mod tidy -diff and documentation generation passed
  • Node 26.10.0 UI typechecking, 11 UI tests and all four embedded Apps builds passed

The original regression fails before the fix and passes after it. The branch now normally merges main e3809fe91c7546633241006bf0b91f23b2595e28, preserving the original history. The two-file contribution patch is byte-identical to the reviewed patch. The full race run produced 8,392 test/subtest pass events, not 8,392 independent test cases. An independent targeted race rerun of the merged-state regression also passed

npm audit is not clean. The unchanged upstream lockfile has one existing advisory affecting three development-dependency package entries. This PR does not upgrade that dependency chain. The audit result is separate from the passing builds and tests

No other-platform local tests, live GitHub/provider calls, browser visual review or natural-language model prompts were run. Local results do not imply upstream CI success or approval of the new head

Docs

  • Not needed

This corrects an existing field without changing the tool schema.

@po-et
po-et requested a review from a team as a code owner October 9, 2026 03:31

@IrynaKulakova IrynaKulakova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 4dbf51a. No actionable findings: the merged_at fallback preserves explicit merged values and is shared by text and structured output. Local lint and tests could not run because the installed Go binaries fail with "Bad CPU type in executable"; this approval is based on code inspection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

list_pull_requests reports merged pull requests as merged: false

3 participants