feat(forge): GitHub SubmitReview + Provider.SubmitReview method (RIG-2170) - #398
Merged
Merged
Conversation
…2170)
Add the submit-review write path to the GitHub Provider: a new SubmitReview/ReviewCommentInput/SubmittedReview type triple and a SubmitReview method on the Provider interface, implemented for the GitHub client as POST /repos/{repo}/pulls/{number}/reviews through the shared doJSON write helper.
The write-side verdict vocabulary (approve/request_changes/comment) maps to GitHub's reviews-POST event tokens (APPROVE/REQUEST_CHANGES/COMMENT); an unknown verdict is rejected before any wire call, and COMMENT/REQUEST_CHANGES require a non-empty body client-side while APPROVE may be bodyless. Empty inline-comments omit the array rather than sending null.
Widens the Provider interface, so FakeProvider gains the method (the compile-break named in the write-path design A7); no out-of-package Provider implementor exists. Hermetic httptest coverage: request golden, event mapping, empty-comments, unknown-verdict/empty-body no-wire, bodyless approve, off-diff 422, and 403/404 error mapping.
Refs RIG-2208, RIG-2170.
Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
|
Compass engineering docs preview: https://compass-server-rig-2208-subm.compass-eng-docs.pages.dev Deployed from |
…RIG-2208 review) Review low#1: the exported Verdict* consts were unspecced and name-collided with the read-side Review.Verdict field; make them package-private (verdictApprove/verdictRequestChanges/verdictComment) since nothing outside the package references them. Review low#2: presize the review-comments slice to len(in.Comments) to avoid the incremental-append reallocs. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
mattwilkinsonn
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of a stack containing 2 PRs:
mainAdd the submit-review write path to the GitHub Provider: a new SubmitReview/ReviewCommentInput/SubmittedReview type triple and a SubmitReview method on the Provider interface, implemented for the GitHub client as POST /repos/{repo}/pulls/{number}/reviews through the shared doJSON write helper.
The write-side verdict vocabulary (approve/request_changes/comment) maps to GitHub's reviews-POST event tokens (APPROVE/REQUEST_CHANGES/COMMENT); an unknown verdict is rejected before any wire call, and COMMENT/REQUEST_CHANGES require a non-empty body client-side while APPROVE may be bodyless. Empty inline-comments omit the array rather than sending null.
Widens the Provider interface, so FakeProvider gains the method (the compile-break named in the write-path design A7); no out-of-package Provider implementor exists. Hermetic httptest coverage: request golden, event mapping, empty-comments, unknown-verdict/empty-body no-wire, bodyless approve, off-diff 422, and 403/404 error mapping.
Refs RIG-2208, RIG-2170.
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com