Skip to content

Commit 7e8dd9d

Browse files
IEvangelistCopilot
IEvangelist
and
Copilot
authored
Post inline PR suggestions for forbidden words (#1359)
* Post inline PR suggestions for forbidden words 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 <[email protected]> Copilot-Session: d8c9e815-476e-4e77-ba91-eeb1882fdacc * Harden forbidden-words suggestion posting per review 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 <[email protected]> Copilot-Session: d8c9e815-476e-4e77-ba91-eeb1882fdacc * Address forbidden-word workflow review feedback Co-authored-by: Copilot App <[email protected]> * Harden forbidden-word suggestion artifacts Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: David Pine <[email protected]> Co-authored-by: Copilot App <[email protected]>
1 parent 952ef46 commit 7e8dd9d

5 files changed

Lines changed: 472 additions & 34 deletions

File tree

‎.github/forbidden-words.json‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,26 @@
11
{
2-
"$comment": "Forbidden words/phrases checked against newly added lines in pull requests by .github/scripts/check-forbidden-words.sh. Each rule has a regex 'pattern' (extended POSIX, evaluated case-insensitively unless 'caseSensitive' is true) and a 'message' shown when matched. Add 'excludePaths' globs to skip files for a specific rule.",
2+
"$comment": "Forbidden words/phrases checked against newly added lines in pull requests by .github/scripts/check-forbidden-words.sh. Each rule has a regex 'pattern' (extended POSIX/PCRE, evaluated case-insensitively unless 'caseSensitive' is true), a required 'replacement' string, and a 'message' shown when matched. The check still fails when a forbidden phrase is found; in addition, .github/workflows/forbidden-words-suggest.yml posts an inline PR review 'suggestion' that rewrites the offending line by substituting each matched pattern with its 'replacement'. The 'replacement' value is inserted verbatim (matching honors 'caseSensitive', but the replacement text is never case-adjusted and does not support backreferences). Add 'excludePaths' globs to skip files for a specific rule.",
33
"excludePaths": [
44
"**/*.lock.yml",
55
"pnpm-lock.yaml",
66
".github/forbidden-words.json",
7-
".github/scripts/check-forbidden-words.sh"
7+
".github/scripts/check-forbidden-words.sh",
8+
".github/scripts/post-forbidden-word-suggestions.sh"
89
],
910
"rules": [
1011
{
1112
"pattern": "\\.NET Aspire",
13+
"replacement": "Aspire",
1214
"message": "Use \"Aspire\" instead of \".NET Aspire\"."
1315
},
1416
{
1517
"pattern": "dotnet aspire",
18+
"replacement": "Aspire",
1619
"message": "Use \"Aspire\" instead of \"dotnet aspire\"."
1720
},
1821
{
1922
"pattern": "\\bapp host\\b",
23+
"replacement": "AppHost",
2024
"message": "Use \"AppHost\" instead of \"app host\"."
2125
}
2226
]

‎.github/scripts/check-forbidden-words.sh‎

Lines changed: 128 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,22 @@
66
# pre-existing text is never flagged. Rules are defined in
77
# .github/forbidden-words.json.
88
#
9+
# In addition to failing when a forbidden phrase is found, this script records
10+
# a machine-readable list of findings (see FINDINGS_FILE below). Each finding
11+
# includes the corrected line produced by substituting every matched rule's
12+
# 'pattern' with its 'replacement'. A companion workflow
13+
# (.github/workflows/forbidden-words-suggest.yml) turns those findings into
14+
# inline pull request review suggestions.
15+
#
916
# Usage:
1017
# check-forbidden-words.sh <base_sha> <head_sha>
1118
#
1219
# Environment overrides:
13-
# CONFIG_FILE Path to the rules file (default: .github/forbidden-words.json)
20+
# CONFIG_FILE Path to the rules file (default: .github/forbidden-words.json)
21+
# FINDINGS_FILE When set, a JSON document of findings is written here (even
22+
# when violations are present). Shape:
23+
# { "findings": [ { "file", "line", "original",
24+
# "suggestion", "messages", "patterns" } ] }
1425
#
1526
# Exits non-zero when at least one forbidden phrase is found.
1627

@@ -20,23 +31,56 @@ BASE_SHA="${1:?base SHA required}"
2031
HEAD_SHA="${2:?head SHA required}"
2132
MERGE_BASE="$(git merge-base "$BASE_SHA" "$HEAD_SHA")" || { echo "::error::Unable to compute merge-base of $BASE_SHA and $HEAD_SHA"; exit 2; }
2233
CONFIG_FILE="${CONFIG_FILE:-.github/forbidden-words.json}"
34+
FINDINGS_FILE="${FINDINGS_FILE:-}"
2335

2436
if [[ ! -f "$CONFIG_FILE" ]]; then
2537
echo "::error::Config file not found: $CONFIG_FILE"
2638
exit 2
2739
fi
2840

2941
command -v jq >/dev/null 2>&1 || { echo "::error::jq is required"; exit 2; }
42+
command -v perl >/dev/null 2>&1 || { echo "::error::perl is required"; exit 2; }
3043

3144
# Read global exclude globs (newline separated).
3245
mapfile -t GLOBAL_EXCLUDES < <(jq -r '.excludePaths // [] | .[]' "$CONFIG_FILE")
3346

3447
rule_count=$(jq '(.rules // []) | length' "$CONFIG_FILE")
48+
49+
# Emit an empty findings file up front so callers always have a valid document.
50+
write_findings() {
51+
[[ -z "$FINDINGS_FILE" ]] && return 0
52+
if [[ -s "$FINDINGS_TMP" ]]; then
53+
jq -s '{findings: .}' "$FINDINGS_TMP" > "$FINDINGS_FILE"
54+
else
55+
printf '{"findings":[]}\n' > "$FINDINGS_FILE"
56+
fi
57+
}
58+
59+
FINDINGS_TMP="$(mktemp)"
60+
trap 'rm -f "$FINDINGS_TMP"' EXIT
61+
3562
if [[ "$rule_count" -eq 0 ]]; then
3663
echo "No rules defined in $CONFIG_FILE; nothing to check."
64+
write_findings
3765
exit 0
3866
fi
3967

68+
# Every rule must define a non-empty replacement so a suggestion can be built.
69+
missing_replacement=$(jq '[.rules[] | select((.replacement // "") == "")] | length' "$CONFIG_FILE")
70+
if [[ "$missing_replacement" -ne 0 ]]; then
71+
echo "::error::Every rule in $CONFIG_FILE must define a non-empty \"replacement\". Found $missing_replacement rule(s) without one."
72+
# Still emit an (empty) findings document so callers that always consume it
73+
# see a valid artifact, even though we fail the check.
74+
write_findings
75+
exit 2
76+
fi
77+
78+
# Preload rule fields to avoid re-invoking jq for every line.
79+
mapfile -t PATTERNS < <(jq -r '.rules[].pattern' "$CONFIG_FILE")
80+
mapfile -t REPLACEMENTS < <(jq -r '.rules[].replacement' "$CONFIG_FILE")
81+
mapfile -t MESSAGES < <(jq -r '.rules[] | (.message // "")' "$CONFIG_FILE")
82+
mapfile -t CASE_SENSITIVE < <(jq -r '.rules[] | (.caseSensitive // false)' "$CONFIG_FILE")
83+
4084
# Returns 0 if $1 matches any of the shell globs passed as remaining args.
4185
matches_any_glob() {
4286
local path="$1"; shift
@@ -51,6 +95,20 @@ matches_any_glob() {
5195
return 1
5296
}
5397

98+
# Substitutes every occurrence of a pattern with its replacement, inserting the
99+
# replacement verbatim. Perl inserts the value of $r without reprocessing its
100+
# contents, so no backreferences ($1) or escape sequences are interpreted.
101+
# Matching honors the rule's case sensitivity.
102+
apply_replacement() {
103+
CONTENT="$1" PAT="$2" REP="$3" CS="$4" perl -e '
104+
my $c = $ENV{CONTENT};
105+
my $p = $ENV{PAT};
106+
my $r = $ENV{REP};
107+
if ($ENV{CS} eq "true") { $c =~ s/$p/$r/g; } else { $c =~ s/$p/$r/gi; }
108+
print $c;
109+
'
110+
}
111+
54112
violations=0
55113

56114
# List the files changed between base and head (added/copied/modified/renamed).
@@ -64,13 +122,25 @@ for file in "${CHANGED_FILES[@]}"; do
64122
continue
65123
fi
66124

125+
# Precompute which rules apply to this file (per-rule excludePaths).
126+
rule_applies=()
127+
for ((r = 0; r < rule_count; r++)); do
128+
mapfile -t RULE_EXCLUDES < <(jq -r ".rules[$r].excludePaths // [] | .[]" "$CONFIG_FILE")
129+
if ((${#RULE_EXCLUDES[@]} > 0)) && matches_any_glob "$file" "${RULE_EXCLUDES[@]}"; then
130+
rule_applies[$r]=0
131+
else
132+
rule_applies[$r]=1
133+
fi
134+
done
135+
67136
# Collect added lines as "<line_number>:<content>". --unified=0 keeps hunks
68137
# tight so we can track new-file line numbers from the @@ headers.
69138
diff_output=$(git diff --unified=0 "$MERGE_BASE" "$HEAD_SHA" -- "$file")
70139
[[ -z "$diff_output" ]] && continue
71140

72141
added_lines=$(awk '
73142
/^@@/ {
143+
in_hunk = 1
74144
for (i = 1; i <= NF; i++) {
75145
if (substr($i, 1, 1) == "+") {
76146
s = substr($i, 2)
@@ -81,8 +151,7 @@ for file in "${CHANGED_FILES[@]}"; do
81151
}
82152
next
83153
}
84-
/^\+\+\+/ { next }
85-
/^\+/ {
154+
in_hunk && /^\+/ {
86155
content = substr($0, 2)
87156
printf "%d:%s\n", line, content
88157
line++
@@ -91,41 +160,74 @@ for file in "${CHANGED_FILES[@]}"; do
91160

92161
[[ -z "$added_lines" ]] && continue
93162

94-
for ((r = 0; r < rule_count; r++)); do
95-
pattern=$(jq -r ".rules[$r].pattern" "$CONFIG_FILE")
96-
message=$(jq -r ".rules[$r].message // \"\"" "$CONFIG_FILE")
97-
case_sensitive=$(jq -r ".rules[$r].caseSensitive // false" "$CONFIG_FILE")
98-
99-
# Per-rule path excludes.
100-
mapfile -t RULE_EXCLUDES < <(jq -r ".rules[$r].excludePaths // [] | .[]" "$CONFIG_FILE")
101-
if ((${#RULE_EXCLUDES[@]} > 0)) && matches_any_glob "$file" "${RULE_EXCLUDES[@]}"; then
102-
continue
103-
fi
104-
105-
grep_opts=(-P)
106-
if [[ "$case_sensitive" != "true" ]]; then
107-
grep_opts+=(-i)
108-
fi
163+
# Check each added line against every applicable rule, aggregating all matches
164+
# on the line into a single corrected line (at most one suggestion per line).
165+
while IFS= read -r entry; do
166+
[[ -z "$entry" ]] && continue
167+
lineno="${entry%%:*}"
168+
content="${entry#*:}"
169+
170+
corrected="$content"
171+
line_has_match=0
172+
suggestion_available=1
173+
line_messages=()
174+
line_patterns=()
175+
176+
for ((r = 0; r < rule_count; r++)); do
177+
[[ "${rule_applies[$r]}" == "1" ]] || continue
178+
179+
pattern="${PATTERNS[$r]}"
180+
replacement="${REPLACEMENTS[$r]}"
181+
message="${MESSAGES[$r]}"
182+
case_sensitive="${CASE_SENSITIVE[$r]}"
183+
184+
grep_opts=(-P)
185+
if [[ "$case_sensitive" != "true" ]]; then
186+
grep_opts+=(-i)
187+
fi
109188

110-
# Check each added line's content (line number stripped) against the rule.
111-
while IFS= read -r entry; do
112-
[[ -z "$entry" ]] && continue
113-
lineno="${entry%%:*}"
114-
content="${entry#*:}"
115189
if printf '%s' "$content" | grep -q "${grep_opts[@]}" -- "$pattern"; then
190+
line_has_match=1
191+
violations=$((violations + 1))
192+
line_messages+=("$message")
193+
line_patterns+=("$pattern")
194+
116195
printf '::error file=%s,line=%s::Forbidden phrase (/%s/): %s\n' \
117196
"$file" "$lineno" "$pattern" "$message"
118197
printf ' %s:%s: %s\n' "$file" "$lineno" "$content"
119-
violations=$((violations + 1))
198+
199+
if ! updated="$(apply_replacement "$corrected" "$pattern" "$replacement" "$case_sensitive")"; then
200+
suggestion_available=0
201+
echo "::warning file=$file,line=$lineno::Could not build an inline suggestion for this line because the matched pattern is not supported by the replacement engine."
202+
else
203+
corrected="$updated"
204+
fi
120205
fi
121-
done <<< "$added_lines"
122-
done
206+
done
207+
208+
if [[ "$line_has_match" -eq 1 && "$suggestion_available" -eq 1 ]]; then
209+
messages_json=$(printf '%s\n' "${line_messages[@]}" | jq -R . | jq -s .)
210+
patterns_json=$(printf '%s\n' "${line_patterns[@]}" | jq -R . | jq -s .)
211+
jq -cn \
212+
--arg file "$file" \
213+
--argjson line "$lineno" \
214+
--arg original "$content" \
215+
--arg suggestion "$corrected" \
216+
--argjson messages "$messages_json" \
217+
--argjson patterns "$patterns_json" \
218+
'{file: $file, line: $line, original: $original, suggestion: $suggestion, messages: $messages, patterns: $patterns}' \
219+
>> "$FINDINGS_TMP"
220+
fi
221+
done <<< "$added_lines"
123222
done
124223

224+
write_findings
225+
125226
echo
126227
if [[ "$violations" -gt 0 ]]; then
127228
echo "Found $violations forbidden phrase occurrence(s) in added lines."
128229
echo "Update the wording, or adjust .github/forbidden-words.json if a rule is wrong."
230+
echo "Inline suggestions will be posted on the pull request where possible."
129231
exit 1
130232
fi
131233

0 commit comments

Comments
 (0)