Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/scripts/issue-translation.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -805,9 +805,10 @@ function sanitizeTranslationBody(raw, maxChars = 60000) {
.split(MARKER).join("")
.split(END_MARKER).join("")
.replace(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/g, "")
// Defuse pings only: @login / @org/team — not emails, scopes, or decorators.
// Defuse pings at Markdown/punctuation boundaries, but not emails, scopes,
// or other mid-token at-signs.
.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",
)
Comment on lines 810 to 813

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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")}`);
}
NODE

Repository: 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 || true

Repository: 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:


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

.trim()
Expand Down
9 changes: 7 additions & 2 deletions .github/scripts/issue-translation.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1168,9 +1168,14 @@ describe("bot-owned control state", () => {
assert.equal(decision.reason, "rate_limited_interval");
});

it("defuses mention-shaped tokens without rewriting emails or mid-token at-signs", () => {
const out = sanitizeTranslationBody("see @octocat and user@example.com and npm:@scope");
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/);
Comment on lines +1171 to +1178

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.

Suggested change
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.

assert.ok(out.includes("user@example.com"));
assert.ok(out.includes("npm:@scope"));
});
Expand Down
Loading