Repository navigation
Review board: free-form comments and Request fixes action - #5322
Open
aaronpowell wants to merge 2 commits into
Open
aaronpowell wants to merge 2 commits into
aaronpowell wants to merge 2 commits into
Conversation
Adds a Comment panel to the External Plugin Review Board drawer with a two-click confirm. Bodies must be non-empty and cannot start with '/' so they can't trigger the command router. Posts via the existing gh.postComment helper and per-issue in-flight guard, then reloads the Comments tab. Co-authored-by: Copilot App <[email protected]>
Adds a /request-fixes <reason> maintainer command to the external plugin command router, and footer Comment and Request fixes buttons that reuse the shared comment box. Co-authored-by: Copilot App <[email protected]>
github-actions
Bot
added
workflow
Contributor
🚦 Submission status: 👀 Ready for reviewRisk tier: Why this tier
Automated checks
Review
Commands
Updated for |
Contributor
|
🟡 Contributor Reputation Check: MEDIUM risk
Maintainers: please review this contributor before merging. |
github-actions
Bot
added
needs-review:MEDIUM
Contributor
Author
|
Re: submission status (6357357119): no change needed. The Re: contributor reputation check (6357361613): no change needed. This is an informational automated check with nothing actionable in the code; leaving the contributor review to the maintainers. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unsent drafts can be lost, and outstanding fixes requests can permit conflicting decisions.
5 open issues
What changed in this PR
Extends the External Plugin Review Board so maintainers can discuss submissions and request corrections without rejecting them.
Changes:
- Adds confirmed Comment and Request fixes actions using the shared textarea.
- Introduces maintainer-only
/request-fixesrouting to update labels and record requested changes. - Documents the workflow and adds parser, validation, and UI coverage.
| File | Description |
|---|---|
eng/external-plugin-review-board.test.mjs |
Extends pending-decision button checks. |
eng/external-plugin-review-board-ui.test.mjs |
Tests availability when fixes are already required. |
eng/external-plugin-intake.test.mjs |
Tests command parsing and reason handling. |
eng/external-plugin-intake.mjs |
Adds request-fixes constants and parser. |
eng/board-preview.test.mjs |
Covers confirmation, posting, refresh, and failure flows. |
CONTRIBUTING.md |
Documents the maintainer request-fixes command. |
.github/workflows/external-plugin-command-router.yml |
Authorizes commands, updates labels, and posts feedback. |
.github/extensions/external-plugin-review-board/README.md |
Documents new actions and validation commands. |
.github/extensions/external-plugin-review-board/public/styles.css |
Styles the new action and confirmation states. |
.github/extensions/external-plugin-review-board/public/index.html |
Adds footer controls and shared-input guidance. |
.github/extensions/external-plugin-review-board/public/app.js |
Implements previews, confirmation, posting, and comments refresh. |
.github/extensions/external-plugin-review-board/lib/github.test.mjs |
Tests comment validation and command formatting. |
.github/extensions/external-plugin-review-board/lib/github.mjs |
Adds comment validation and request-fixes formatting. |
.github/extensions/external-plugin-review-board/extension.mjs |
Adds posting routes and label-confirmation polling. |
Review effort: Max
Comment on lines
+216
to
+224
| for (const delay of [1000, 2000, 4000]) { | ||
| await sleep(delay); | ||
| const latest = await gh.getIssueStatus(store.state.repo, number); | ||
| if (latest.labels.includes("requires-submitter-fixes")) { | ||
| await refresh(); | ||
| return { number, commentUrl, pending: false }; | ||
| } | ||
| } | ||
| return { number, commentUrl, pending: true }; |
Comment on lines
+224
to
+226
| return { number, commentUrl, pending: true }; | ||
| } finally { | ||
| decisionsInFlight.delete(number); |
| function commentBody(kind) { | ||
| const text = ui.comment.value.trim().replace(/^\/(approve|reject)\b\s*/i, ""); | ||
| if (kind === "comment") return ui.comment.value.trim(); | ||
| const text = reasonText(); |
Comment on lines
+623
to
+630
| const hasReason = Boolean(reasonText()); | ||
| ui.reject.disabled = !hasReason; | ||
| ui.reject.title = hasReason ? "" : "Reject requires a reason."; | ||
| ui.requestFixes.disabled = !hasReason; | ||
| ui.requestFixes.title = hasReason ? "" : "Request fixes requires a description of what to change."; | ||
| const commentProblem = freeformCommentError(ui.comment.value.trim()); | ||
| ui.commentPost.disabled = Boolean(commentProblem); | ||
| ui.commentPost.title = commentProblem ?? ""; |
Comment on lines
+693
to
+711
| const result = await api("POST", "/api/comment", { number, body: ui.comment.value.trim() }); | ||
| toast(`Posted comment on #${number}`, { link: result.commentUrl }); | ||
| stagedComments.set(number, ""); | ||
| if (selected === number) { | ||
| ui.comment.value = ""; | ||
| activeTab = "comments"; | ||
| renderDrawer(); | ||
| } | ||
| await loadDetail(number, { force: true }); | ||
| } else if (kind === "request-fixes") { | ||
| const result = await api("POST", "/api/request-fixes", { number, comment: ui.comment.value }); | ||
| toast( | ||
| result.pending | ||
| ? `Posted /request-fixes on #${number}; waiting for the workflow to update labels.` | ||
| : `Requested fixes on #${number}`, | ||
| { link: result.commentUrl }, | ||
| ); | ||
| stagedComments.delete(number); | ||
| if (selected === number) closeDrawer(); |
This branch was successfully deployed
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
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.
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.mainbranch for this pull request.Description
Maintainers using the External Plugin Review Board could only act on submissions through structured commands (
/approve,/reject, re-review, bucket moves). There was no way to ask the submitter a question, and no action that sends a submission back for fixes.This PR adds two footer actions next to Approve and Reject. Both use the same comment box.
/are rejected on both the client and the server, so a comment can't trigger the command router. After posting, the drawer switches to the Comments tab and reloads it./request-fixes <reason>, which requires a reason. The button is disabled while a decision is pending or once the issue already hasrequires-submitter-fixes./request-fixes <reason>inexternal-plugin-command-router.yml:external-pluginissues that are not yet approved or rejected.requires-submitter-fixesand posts a marker comment listing the requested fixes./rerun-intake, intake runs again as usual.parseRequestFixesCommandineng/external-plugin-intake.mjs.CONTRIBUTING.mdand the extension README are updated.Type of Contribution
/request-fixesmaintainer command in the external plugin command router.Additional Notes
main, so the Request fixes button only takes effect after this merges. Before that, the command comment is posted but nothing moves the issue.Validation:
node --test .github/extensions/external-plugin-review-board/lib/github.test.mjs eng/board-preview.test.mjs eng/external-plugin-review-board-ui.test.mjs eng/external-plugin-review-board-state.test.mjs eng/board-bulk-actions.test.mjs eng/board-column-order.test.mjs eng/external-plugin-review-board.test.mjs eng/external-plugin-intake-state.test.mjs: passed (98/98)node --experimental-vm-modules --test eng/board-state-storage.test.mjs: passed (11/11)node --test --test-name-pattern=parseRequestFixesCommand eng/external-plugin-intake.test.mjs: passed (3/3)By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.