[WRONG BRANCH] Fix: defuse bot-authored mentions in translated text after punctuation/Markdown - #84
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
📝 WalkthroughWalkthroughThe issue translation sanitizer now defuses mention-shaped tokens after additional punctuation and Markdown boundaries while preserving email addresses, scopes, decorators, and other mid-token at-signs. Tests cover the expanded boundary cases. ChangesMention sanitization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/issue-translation.cjs:
- Around line 810-813: The replacement in sanitizeTranslationBody must preserve
bare scoped package names while still handling actual mentions. Add
context-aware matching around the regex so `@scope/package` and `@org/team` remain
unchanged, while npm:`@scope/package` is handled correctly, and add tests covering
all three forms.
In @.github/scripts/issue-translation.test.cjs:
- Around line 1171-1178: Extend the test around sanitizeTranslationBody to
assert that user@example.com and npm:`@scope` remain unchanged, and add a
slash-bearing scoped-package input such as npm:`@scope/package` if that form is
supported by the contract. Verify each preserved token explicitly in the output
while retaining the existing defused-mention assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 273445e1-da4d-4cb6-8505-bf06b3411f23
📒 Files selected for processing (2)
.github/scripts/issue-translation.cjs.github/scripts/issue-translation.test.cjs
| .replace( | ||
| /(^|[\s(])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,38})(?:\/[A-Za-z0-9._-]+)?)/g, | ||
| /(^|[^A-Za-z0-9._%+:-])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,38})(?:\/[A-Za-z0-9._-]+)?)/g, | ||
| "$1@\u200b$2", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
node <<'NODE'
const mention = /(^|[^A-Za-z0-9._%+:-])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,38})(?:\/[A-Za-z0-9._-]+)?)/g;
for (const value of ["`@scope/package`", "npm:`@scope/package`", "`@org/team`"]) {
console.log(`${value} => ${value.replace(mention, "$1@\u200b$2")}`);
}
NODERepository: luvs01/opencodex
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target context ---'
sed -n '760,835p' .github/scripts/issue-translation.cjs
printf '%s\n' '--- related tests and references ---'
rg -n -C 3 'scope/package|npm:`@scope`|org/team|zero.?width|issue-translation|mention' .github test tests 2>/dev/null || trueRepository: luvs01/opencodex
Length of output: 39724
🌐 Web query:
GitHub username mention syntax colon immediately before @user notification
💡 Result:
No, a colon is not required before a GitHub @mention to trigger a notification [1][2]. The standard syntax for mentioning a user or team on GitHub is simply the @ symbol followed immediately by their username or team name (e.g., @username) [1][3]. Placing a colon immediately before the @ symbol is not part of the standard @mention syntax and would not be interpreted as a user mention [1][4]. While colons are used in GitHub Flavored Markdown for other features—such as creating emoji (e.g.,:smile:)—they are not used for user or team mentions [4]. When you type @, GitHub provides an autocomplete list of users and teams that filters as you type [1][2]. Selecting a user or team from this list ensures the correct syntax is used to trigger a notification [1]. Notifications for @mentions occur when a user is mentioned in contexts such as issues, pull requests, and discussions, provided the user has read access to the repository [1][5][6].
Citations:
- 1: https://github.com/github/docs/blob/main/content/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax.md
- 2: https://docs.github.com/en/enterprise-server@3.0/articles/basic-writing-and-formatting-syntax
- 3: https://www.markdownlang.com/advanced/github.html
- 4: https://docs.github.com/github/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax
- 5: https://github.blog/news-insights/mention-somebody-they-re-notified/
- 6: https://docs.github.com/en/subscriptions-and-notifications/get-started/configuring-notifications
Preserve bare scoped package names
At .github/scripts/issue-translation.cjs:810-813, the regex rewrites bare @scope/package tokens, although sanitizeTranslationBody states that scopes must remain unchanged. @scope/package and @org/team have the same lexical form. Add context-aware handling and tests for bare scopes, npm:@scope/package``, and @org/team.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/issue-translation.cjs around lines 810 - 813, The
replacement in sanitizeTranslationBody must preserve bare scoped package names
while still handling actual mentions. Add context-aware matching around the
regex so `@scope/package` and `@org/team` remain unchanged, while npm:`@scope/package`
is handled correctly, and add tests covering all three forms.
Source: Path instructions
| it("defuses mention-shaped tokens at Markdown and punctuation boundaries", () => { | ||
| const out = sanitizeTranslationBody( | ||
| "see @octocat, comma,@team, [@user], >@org/team, user@example.com, npm:@scope", | ||
| ); | ||
| assert.match(out, /@\u200boctocat/); | ||
| assert.match(out, /,@\u200bteam/); | ||
| assert.match(out, /\[@\u200buser\]/); | ||
| assert.match(out, />@\u200borg\/team/); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the preserved forms.
The test input includes user@example.com and npm:@scope``, but the test only asserts the defused mentions. A regression that inserts a zero-width character into either preserved token would still pass.
Add preservation assertions, and add a slash-bearing scoped-package case if bare npm scopes are part of the contract.
Proposed test additions
assert.match(out, /@\u200boctocat/);
assert.match(out, /,@\u200bteam/);
assert.match(out, /\[@\u200buser\]/);
assert.match(out, />@\u200borg\/team/);
+ assert.match(out, /user@example\.com/);
+ assert.match(out, /npm:`@scope/`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("defuses mention-shaped tokens at Markdown and punctuation boundaries", () => { | |
| const out = sanitizeTranslationBody( | |
| "see @octocat, comma,@team, [@user], >@org/team, user@example.com, npm:@scope", | |
| ); | |
| assert.match(out, /@\u200boctocat/); | |
| assert.match(out, /,@\u200bteam/); | |
| assert.match(out, /\[@\u200buser\]/); | |
| assert.match(out, />@\u200borg\/team/); | |
| it("defuses mention-shaped tokens at Markdown and punctuation boundaries", () => { | |
| const out = sanitizeTranslationBody( | |
| "see `@octocat`, comma,`@team`, [`@user`], >`@org/team`, user@example.com, npm:`@scope`", | |
| ); | |
| assert.match(out, /@\u200boctocat/); | |
| assert.match(out, /,@\u200bteam/); | |
| assert.match(out, /\[@\u200buser\]/); | |
| assert.match(out, />@\u200borg\/team/); | |
| assert.match(out, /user@example\.com/); | |
| assert.match(out, /npm:`@scope/`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/issue-translation.test.cjs around lines 1171 - 1178, Extend
the test around sanitizeTranslationBody to assert that user@example.com and
npm:`@scope` remain unchanged, and add a slash-bearing scoped-package input such
as npm:`@scope/package` if that form is supported by the contract. Verify each
preserved token explicitly in the output while retaining the existing
defused-mention assertions.
The six open js/polynomial-redos alerts I listed included #84, which had already been fixed fourteen minutes before I wrote the list, and omitted #50, which is open. Still six, one member wrong. The live set is #83, #60, #53, #52, #51, #50, all from 2026-08-12/13 - so the scoped claim holds: this campaign introduced exactly one and fixed exactly that one. And the one it introduced has now closed. Alert #87 reads fixed as of 03:17:06Z from the rescan of main. The earlier text predicted that and declined to claim it in advance, which turned out to be the right call.
1 participant
Motivation
(so mentions following punctuation or Markdown delimiters are neutralized.detected_languagebookkeeping path in the control flow was reviewed and the change focuses on the translated-body sanitization where the regression existed.Description
sanitizeTranslationBodyso mentions are defused when preceded by punctuation or Markdown boundaries by replacing/(^|[\s(])@.../gwith/(^|[^A-Za-z0-9._%+:-])@.../gin.github/scripts/issue-translation.cjs..github/scripts/issue-translation.test.cjsto cover mentions after commas, brackets, and blockquote delimiters as well as org/team mentions, while asserting emails andnpm:@scoperemain unchanged.Testing
node --test .github/scripts/issue-translation.test.cjsand the translation-related tests passed (68 tests in that suite succeeded).bun run typecheckandbun run privacy:scanand both checks completed successfully.bun run test/bun run prepush; the translation unit changes did not cause regressions, but the full-suite run hit unrelated timeouts/failures intests/combo-management-api.test.tsso the prepush full-suite step could not be completed end-to-end in this environment.Summary by CodeRabbit