Skip to content

fix(runway): reject merged GitHub pulls listed after unmerged ones - #836

Open
DivyanshGarg380 wants to merge 7 commits into
uber:mainfrom
DivyanshGarg380:fix/runway-github-merged-pr-order
Open

DivyanshGarg380 wants to merge 7 commits into
uber:mainfrom
DivyanshGarg380:fix/runway-github-merged-pr-order

Conversation

@DivyanshGarg380

Copy link
Copy Markdown
Contributor

Why?

validateStepIsGitHubStack contains an ineffective ordering check. It checks the prefix of pull requests returned by unmergedPulls, rather than validating the suffix containing unmerged pull requests.

Consequently, a merged pull request listed after an unmerged pull request can bypass validation and reach the GitHub stack lookup path.

What?

  • Validate the suffix returned by unmergedPulls and reject any already-merged pull request found after the first unmerged pull request.
  • Return merger.ErrInvalidRequest for invalid pull request ordering.
  • Add regression tests covering valid ordering and merged pull requests appearing after unmerged ones.

Test Plan

  • go test ./runway/extension/merger/github -run TestValidateStepIsGitHubStack_MergedMustPrecedeUnmerged -count=1
  • go test ./runway/extension/merger/github -count=1
  • python .\tool\bazel --output_user_root=C:\bazel test //runway/extension/merger/github:all

All tests passed.

Issue

Closes #835

@DivyanshGarg380

Copy link
Copy Markdown
Contributor Author

Hi @behinddwalls, just following up on this PR. Would appreciate a review when you get a chance.
Thanks!

This branch has not been deployed

No deployments
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.

[Bug] Fix merged pull request ordering validation in GitHub merger

1 participant