Repository navigation
Review board: free-form comments and Request fixes action - #5322
Merged
Merged
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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
Contributor
Author
|
No code change is needed for this automated status notice. The high-risk tier is expected for changes to the review-board extension and command-router workflow; merging still requires the stated maintainer approval. |
Contributor
Author
|
No code change is needed for this automated reputation notice. It is informational and asks maintainers to review the contributor; it does not identify an actionable issue in the PR changes. |
Contributor
Author
|
This earlier comment addressed both automated notices together. To make the outcome explicit per notice: neither requires a code change; the submission risk tier and contributor reputation result remain for maintainers to evaluate. |
Track outstanding fixes requests separately from closed decisions, make status polling best-effort, and guard conflicting actions until the router confirms the label transition. Keep previews consistent with submitted commands and preserve edits made while actions are in flight. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
awaiting-automation
and removed
ready-for-review
github-actions
Bot
added
ready-for-review
Use the same complete command-token boundary as the board preview so command names with hyphenated suffixes are not partially stripped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
awaiting-automation
and removed
ready-for-review
github-actions
Bot
added
ready-for-review
|
|
||
| async resolvePendingFixesRequest(number, status) { | ||
| const item = this.state.items[String(number)]; | ||
| if (!item?.pendingFixesRequest || !(status.labels ?? []).includes("requires-submitter-fixes")) return null; |
Comment on lines
+79
to
+80
| <button type="button" class="btn btn-comment" id="decision-comment-post">Comment</button> | ||
| <button type="button" class="btn btn-warning" id="decision-request-fixes">Request fixes</button> |
Comment on lines
+148
to
+162
| .btn-comment.confirming { | ||
| border-color: transparent; | ||
| background: var(--blue); | ||
| color: var(--color-white, #fff); | ||
| } | ||
|
|
||
| .btn-warning { | ||
| color: var(--orange); | ||
| } | ||
|
|
||
| .btn-warning.confirming { | ||
| border-color: transparent; | ||
| background: var(--orange); | ||
| color: var(--color-white, #fff); | ||
| } |
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.