Post inline PR suggestions for forbidden words#1359
Conversation
Extend the forbidden-words check so that, in addition to failing the PR when a forbidden phrase is found, it posts an inline GitHub PR review `suggestion` block that rewrites each offending line using the rule's replacement. Suggestions are posted by the Aspire bot GitHub App and are skipped when a comment already exists on that line. - forbidden-words.json: require a `replacement` on every rule; document the new field and suggestion behavior; exclude the new posting script. - check-forbidden-words.sh: validate replacements, aggregate all matches per line into one corrected line, and write a findings JSON document (replacement inserted verbatim; matching still honors caseSensitive). - post-forbidden-word-suggestions.sh (new): turn findings into a batched PR review of suggestion blocks, deduped against existing comments. - forbidden-words.yml: scan with continue-on-error, upload findings + PR metadata as an artifact, then gate/fail on violations. - forbidden-words-suggest.yml (new): workflow_run follow-up that runs in the privileged base context, mints the Aspire bot token, and posts the suggestions using trusted scripts from the default branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d8c9e815-476e-4e77-ba91-eeb1882fdacc
There was a problem hiding this comment.
Pull request overview
This PR extends the existing “Forbidden Words” pull request check to also generate inline PR review suggestions (via a follow-up workflow_run workflow) that rewrite offending lines using rule-defined replacements, while preserving the blocking gate when violations exist.
Changes:
- Updates the
Forbidden Wordsworkflow to run the scan withcontinue-on-error, emit findings + PR metadata, upload them as an artifact, and then fail the check when violations (or scan failures) occur. - Adds a new privileged
workflow_runworkflow that downloads the findings artifact, mints the Aspire bot token, and posts batched inlinesuggestionreview comments. - Enhances the forbidden-words scanning script to require
replacementfor each rule, aggregate matches per line into one corrected suggestion, and optionally write a structuredfindings.json.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/forbidden-words.yml | Writes findings + metadata as an artifact while keeping the check blocking on violations. |
| .github/workflows/forbidden-words-suggest.yml | New workflow_run workflow to post inline PR suggestions using the Aspire bot token. |
| .github/scripts/post-forbidden-word-suggestions.sh | New script to dedupe and post batched suggestion comments (with fallback behavior). |
| .github/scripts/check-forbidden-words.sh | Adds findings output and replacement-based suggestion generation logic. |
| .github/forbidden-words.json | Requires per-rule replacement and documents suggestion behavior; adds excludes for the posting script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Frontend HTML artifact readyThe latest frontend build uploaded the This comment updates automatically when a new frontend build artifact is uploaded. |
Address PR review feedback on the inline-suggestion flow: - forbidden-words-suggest.yml: key concurrency by the source PR (head repo + branch) with cancel-in-progress so rapid pushes can't run concurrently and race the dedupe check. - post-forbidden-word-suggestions.sh: treat the findings artifact as untrusted input and skip quietly when it isn't a valid findings document; skip gracefully (instead of failing the job) when existing PR comments can't be listed, to avoid duplicates and keep the workflow best-effort. - check-forbidden-words.sh: emit an empty findings document on the missing-replacement failure path so callers that always consume findings still get a valid artifact (while still exiting 2). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d8c9e815-476e-4e77-ba91-eeb1882fdacc
| name: Post inline suggestions | ||
| runs-on: ubuntu-latest | ||
| # Only act on runs triggered by pull requests. | ||
| if: ${{ github.event.workflow_run.event == 'pull_request' }} |
There was a problem hiding this comment.
How are we guaranteed to know we can trust the meta.json from the checkout? From what I understand here, we are not locking to the head SHA, so theoretically a fork PR could make changes to the meta.json and make bad suggestions to a different PR.
Before we merge this in, we should make sure we have thought well through this and ensure that we have mitigations to protect against this.
| printf ' %s:%s: %s\n' "$file" "$lineno" "$content" | ||
| violations=$((violations + 1)) | ||
|
|
||
| corrected="$(apply_replacement "$corrected" "$pattern" "$replacement" "$case_sensitive")" |
There was a problem hiding this comment.
NIT: Accrording to the agent, this may backfire if you have a pattern defined which uses special PCRE2 notation that Perl doesn't support (like \p{Xan}) it will crash the run and make it so no suggestions are posted ever. I know this is unlikely since we are mostly just using simple regex and mostly forbidden words, but there may be a cheap way to fix that broken case if you just do something like corrected="$(apply_replacement ... || printf '%s' "$corrected")" which would no longer crash the full run any more.
Summary
Extends the Forbidden Words check so that, in addition to failing a PR when a forbidden phrase is found, it now posts inline PR review
suggestionblocks that rewrite each offending line using the rule''s replacement. Suggestions are posted by the Aspire bot GitHub App (the same app used elsewhere in this repo), and are skipped when a comment already exists on that line, so re-runs don''t pile up duplicates.The blocking gate is preserved: the check still fails when forbidden words are present.
How it works
A secure two-workflow split (so it works for fork PRs, which can''t see secrets on
pull_request):Forbidden Words(pull_request) — scans added lines, writes afindings.json+meta.json, uploads them as an artifact, and fails the check when violations exist. Read-only token, no secrets.Forbidden Words Suggestions(workflow_run) — runs in the privileged base-repo context, mints the Aspire bot token, and posts the inline suggestions. It only reads the artifact JSON as data and runs trusted scripts from the default branch — it never checks out or executes PR code.Note
Because the suggestion workflow runs trusted scripts from the default branch, inline suggestions only start appearing on PRs after this change merges to
main.Changes
.github/forbidden-words.json— every rule now requires areplacement; updated$commentdocs; excluded the new posting script..github/scripts/check-forbidden-words.sh— validates replacements (fails if any rule lacks one), aggregates all matches on a line into a single corrected line, and writes a findings document. The replacement is inserted verbatim (matching still honorscaseSensitive, but replacement text is never case-adjusted and does not support backreferences)..github/scripts/post-forbidden-word-suggestions.sh(new) — turns findings into a single batched PR review ofsuggestionblocks, deduped against existing comments, with a per-comment fallback..github/workflows/forbidden-words.yml— scan (continue-on-error) → upload findings + PR metadata → gate/fail on violations..github/workflows/forbidden-words-suggest.yml(new) —workflow_runfollow-up that posts the suggestions.Validation
bash -nclean; both workflows passactionlint.replacementfailure (exit 2), and verbatim replacement (incl.$/@/\edge cases).pnpm buildnot run (no frontend changes).