fix(cli): stop bare-URL hyperlinks at full-width CJK punctuation - #8755
Conversation
The bare-URL linkifier used https?://\S+ in both the React inline and ANSI table renderers; \S swallows full-width punctuation that Chinese prose glues onto URLs (https://x.com(2 …), turning the OSC 8 target into a 404. Share a BARE_URL_PATTERN that stops at CJK/full-width punctuation and extend trimTrailingUrlPunctuation to full-width trailing punctuation. Resolves #8750.
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Gate: pass — moving on to code review. (Re-run after two
Moving on to code review. 🔍 中文说明准入:通过,进入代码审查。(本轮为 re-run:经过两轮
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewNo blocking findings on the current head. I wrote down my own proposal before reading the diff: a trailing trimmer cannot fix this bug — in
Non-blocking, for a follow-up: the doc comment enumerates 々 (U+3005) and 〇 (U+3007) as deliberately kept but omits 〆 (U+3006), which is kept and tested ( TestingEvidence carried: the PR's own CI check-runs on the reviewed commit, fetched via API (unattended run — PR code not built or executed here).
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Two notes on that table. The macOS/Windows test legs and Sandboxed verification would settle the one remaining gap: Not verified: Windows/Linux runtime behavior (author marked 中文说明代码审查当前 head 无阻塞项。读 diff 前先写了独立方案:尾部裁剪修不了这个 bug——
非阻塞跟进项:文档注释列举了刻意保留的 々(U+3005)与 〇(U+3007),却漏了同样保留且有测试的 〆(U+3006)( 测试本次证据:通过 API 拉取的该 commit 自身 CI 结果(无人值守运行,未在此构建或执行 PR 代码)。表中 macOS/Windows 测试腿与 沙箱验证可以补上最后一个缺口: 未验证:Windows/Linux 运行时行为(作者标记 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every stage on the current head; the missing point is two cosmetic follow-ups (doc comment omits 〆, drive-by comment re-wrap), neither blocking. Stepping back: this head is the fix I would have written. A trailing trimmer cannot reach a balanced One thread to be aware of: the standing 中文说明置信度:4/5 —— 当前 head 各阶段全部干净;扣掉的一分是两个外观性跟进项(文档注释漏了 〆、顺手重排的注释),均不阻塞。 复盘:这个 head 正是我会写的修法。尾部裁剪够不到配平的 一个需要留意的线头:PR 上仍挂着的 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
Addressed the actionable review feedback in
Verified with 186 focused OSC 8 and renderer tests, ESLint, Prettier, and diff checks. Intentionally not changed: extracting the duplicated composite tokenizer regex, or broadening the fix to Small Form Variants, Vertical Forms, CJK Compatibility Forms, and wave-dash code points. Those are refactoring / pre-existing residue outside issue #8750's reported punctuation case and would expand the matching policy. The red No screenshot was attached because the change is hyperlink-target byte behavior; a rendered screenshot cannot prove the OSC 8 target. 中文说明已在 验证通过:186 个 OSC 8 与 renderer 定向测试、ESLint、Prettier 和 diff 检查。 未采纳组合 tokenizer regex 抽象,也未把本次修复扩大到 Small Form Variants、Vertical Forms、CJK Compatibility Forms 和 wave-dash 码点;这些属于 issue #8750 报告范围之外的重构或既有残留,会扩大匹配策略。 红色 未附截图:本轮改变的是 OSC 8 超链接目标字节,截图无法证明目标值。 |
|
Updated the PR description to match the repository template without changing code. The English scope, evidence, environment, risk, and Verified by read-back: all required template headings are present in both language sections and the issue link remains intact. Intentionally not changed: implementation, tests, branch head, or CI. Pending: automatic review. 中文摘要仅修正 PR 描述,没有修改代码。保留英文范围、证据、环境、风险和 |
|
Closeout for
中文摘要裸 URL 现在会保留构词字符 U+3006 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
Handled the current feedback at Changed:
Verified:
Intentionally not changed:
Pending:
中文摘要已保留合法 IRI 中的短横线、弯引号与省略号,并补充聚焦回归;89/89 通过。未扩展到 tokenizer 架构或正则重构,等待 exact-head CI 与人工复审。 |
* fix(ci): route workflow label mutations through REST `gh pr edit` cannot mutate anything on this repository: its GraphQL lookup requests repository.pullRequest.projectCards, and with Projects (classic) attached GitHub returns the deprecation as an error, so the command exits 1 before applying the change. Reproduced from a live clone against PR QwenLM#8755 — the error names the field outright. Three workflows carried label mutations through it: - pr-self-report-label.yml: every add/remove arm failed — 43 straight run failures from 2026-08-04 on; the green runs were all the nothing-to-do arm. Self-reported PRs (like QwenLM#8755, whose author also opened QwenLM#8750) never got the label. - qwen-autofix.yml: the `@qwen-code /takeover` and `/takeover stop` COMMAND paths never toggled the label — only the UI label events worked, so the command was dead weight wearing an ack. - repo-hygiene.yml: the add was `|| echo`-guarded, so it never failed the run — it just never labeled anything, while the fallback message blamed a label that exists. All five sites now use the REST issues/labels endpoints, which never touch that query. Two traps handled on the way: - Every label involved contains a slash, and in the DELETE the label is a PATH SEGMENT — unencoded it 404s. Encoded via jq @uri, and the tests assert the literal %2F because a real jq runs in the replay. - The REST add auto-creates a missing label, which repo-hygiene explicitly promises never to do — that site gets an existence probe first, and its misdiagnosing fallback message is corrected. Verified live on QwenLM#8755 before editing anything: the exact gh pr edit call fails with the projectCards error; REST POST applies the label (backfilling the one it was owed), DELETE with %2F removes it. Tests: the stub-driven replays for both the self-report step and the takeover toggle now pin the full REST method + path (encoding included), and a repo-wide guard bans `gh pr edit --add-label/ --remove-label` in every workflow so the class cannot return. Mutation-tested, 6 of 6 caught: each of the five sites reverted to gh pr edit, and the DELETE stripped of its encoding. * fix(ci): harden REST label mutation steps per review (QwenLM#8761) * fix(ci): pin REST label failure policies per review (QwenLM#8761) Review round for the REST migration: - The DELETE arms tolerated EVERY failure (`|| true`), masking 403/5xx/network errors behind a green run and a false "removed" log. They now tolerate only the documented 404 race — any other failure emits a ::warning:: while keeping the step green (pr-self-report-label) and the release ack alive (qwen-autofix). - Neither replay harness could make a `gh api` call fail, so both failure policies were unpinned. They gain failure knobs (knob value on stderr like a real gh HTTP error) and now pin: 404 race silent, other DELETE failures warned, POST loud. The toggle replay also moves to -eo pipefail like the runner's bash default, reproducing the step's real failure semantics. - The jq stub enforced only the --arg shape; it now also enforces the `$l|@uri` program, so a filter mutation fails the suite instead of riding the stub's unconditional percent-encoding. - The gh-pr-edit guard misfired on comments and miscounted lines after joining continuations: comments are stripped before matching, and offenders are reported at the physical line where the (possibly wrapped) command starts. Mutation-tested with 8 probes, all caught: blanket || true on either DELETE, || true on either POST, dropped |@uri, a comment quoting the ban (stays green), an executable and a wrapped violation (both red, correct line). * Address review round 3: close the guard evasions, convert the release path Four round-3 findings, each reproduced before fixing, plus the release path the round-1 scope note deferred. - The ban guard now scans what bash executes, not the YAML surface: the decoded run: values of every parsed workflow, whole-line comments stripped, continuations joined the way bash joins them (backslash- newline removed, nothing inserted), matched whitespace-tolerantly. All three reproduced evasions — a # inside a quoted string eating the trailing backslash, wraps inside the command prefix or a flag token, and folded scalars — are fixture-pinned. Offenders report as file » job » step; line numbers stopped meaning anything after joins. - classify-release-notes.mjs mutates labels through REST now, and the guard grew an argv-form scan over .github/scripts/*.mjs that flags the old file (negative-controlled) — the release path was the last gh pr edit label site, failing silently behind continue-on-error. - JQ_STUB enforces the full invocation: -rn (with -r alone real jq evaluates zero inputs and prints nothing), the binding name l (real jq exits 3 on $l undefined), and the program. Either reproduced mutation previously expanded the substitution empty, sent the DELETE to …/labels/ with no name segment, and the 404 tolerance swallowed it. - The takeover engage POST gets the idempotent create its siblings carry, pinned to the label's real color (1D76DB): the REST add would re-create a deleted label silently with a random color. - runToggle captures writes on throw, and the engage-failure assertion now pins the ORDER its comment claims: a failing apply must leave no "takeover-ack engaged" in the captured writes — the bare toThrow passed even with the ack moved above the POST (reproduced). - The two REMOVE_ERR DELETE idioms are drift-pinned byte-identical modulo the label variable, the honest substitute for sharing shell across workflow files. Mutation-tested, 6 of 6 caught: the evadable regex restored, -rn and the binding name mutated in the workflow, the create dropped, the ack posted before the POST, and the old .mjs flagged by the new scan. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 1312 passed · 0 failed · 1312 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:1312 通过 · 0 失败 · 1312 总计 Verification reportPR 8755 — fix(cli): stop bare-URL hyperlinks at full-width CJK punctuationVerdict: 中文摘要
Central claim + A/BCentral claim: a bare URL glued to full-width CJK punctuation ( Harness:
4/4 discriminating cells flip broken→fixed; 6/6 retention cells identical. Base S2 failure diff quotes the exact swallowed target ( Secondary claims verified:
Findings (non-blocking)
Not covered
MethodologyEnvironment: Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Local UI verification: PASS
The verdict below is based on the visible color boundary in the screenshots, not merely on obtaining two screenshots or on the intended code change. No dependency install, build, test suite, or browser-based terminal capture was run. Before — blue link incorrectly includes After — blue link stops at 中文结论:修改前蓝色链接范围吞入了 |
…rm exclusions Co-Authored-By: wenshao <wenshao@users.noreply.github.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
[Critical] R4-3 (Critical, probe-verified): Re-check of @wenshao's standing unresolved correctness thread (comment 3748885014, InlineMarkdownRenderer.tsx:42): still stands at the current head. After the new early bare-URL stop, the tail after a truncated URL is re-tokenized by the earlier INLINE_MARKDOWN_REGEX alternatives: for input 见 https://a.com/x(说明_1)和 https://b.com/y_。 the combined regex yields tokens https://a.com/x + _1)和 https://b.com/y_ — the second URL gets no OSC 8 envelope (not clickable) and both underscores are deleted from the visible text. Pre-PR \S+ kept the second URL linkified. TableRenderer.tsx:54 builds the identical regex and fails the same way. Not re-posted inline — the full claim and discussion already live in that thread; this entry keeps the verdict accounting for it.
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
[Critical] R4-3 (Critical, probe-verified): Re-check of @wenshao's standing unresolved correctness thread (comment 3748885014, InlineMarkdownRenderer.tsx:42): still stands at the current head. After the new early bare-URL stop, the tail after a truncated URL is re-tokenized by the earlier INLINE_MARKDOWN_REGEX alternatives: for input 见 https://a.com/x(说明_1)和 https://b.com/y_。 the combined regex yields tokens https://a.com/x + _1)和 https://b.com/y_ — the second URL gets no OSC 8 envelope (not clickable) and both underscores are deleted from the visible text. Pre-PR \S+ kept the second URL linkified. TableRenderer.tsx:54 builds the identical regex and fails the same way. Not re-posted inline — the full claim and discussion already live in that thread; this entry keeps the verdict accounting for it.
— qwen3.8-max via Qwen Code /review (v0.21.8)
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Thanks for iterating through the review rounds — the typographic punctuation fix, the CJK compat/vertical form additions, and the doc comment corrections are solid improvements. However, there is one correctness regression that should be resolved before merge.
Summary
| Severity | Count | Status |
|---|---|---|
| 🔴 P0 Critical | 1 | Must fix |
| 🟡 P2 Medium | 1 | Fix or defer |
| 🔵 P3 Low | 2 | Optional |
P0 Critical — Re-tokenization regression
After a bare URL is correctly truncated at CJK punctuation, the remaining tail is re-parsed by the earlier alternatives in INLINE_MARKDOWN_REGEX. Since BARE_URL_PATTERN is last in the alternation, the _.*?_ / \*.*?\* branches can now match across prose that follows a truncated URL — a path that \S+ never exposed.
Repro: 见 https://a.com/x(说明_1)和 https://b.com/y_。
- Pre-PR (
\S+): One giant bare URL run covering everything (broken target, but at least the second URL was link-colored inside it). - Post-PR (this branch):
https://a.com/xextracted correctly, then_1)和 https://b.com/y_matched as italic — underscores silently deleted from visible text,https://b.com/yreceives no OSC 8 envelope at all.
Both InlineMarkdownRenderer.tsx:41 and TableRenderer.tsx:53 build the identical regex and exhibit the same behavior.
P2 — U+00B7 · and U+2027 ‧ still missing
The exclusion set added \uFE10-\uFE1F / \uFE30-\uFE6F in the latest commit, and that works. But U+00B7 (Chinese 间隔号 / middle dot) and U+2027 (hyphenation point, common in TW text) are not covered — https://x.com/a·说明 and https://x.com/a‧說明 still swallow the trailing prose into the URL match.
P3 — Other observations
- Duplicated
INLINE_MARKDOWN_REGEXbetween the two renderers — the fix had to be applied in two places. Exporting the assembled regex fromosc8.ts(whereMD_LINK_PATTERNalready lives) would make future changes single-file. - Policy split between
BARE_URL_PATTERN(regex-level) andtrimTrailingUrlPunctuation(post-match) could be consolidated, but this is a design comment for a follow-up; the current separation works.
What's been addressed well
- ✅ Typographic dashes/quotes (
\u2013-\u2015,\u2018-\u201F,\u2026) removed from exclusion (commit01a37f346) — Wikipedia en-dash URLs work. - ✅ CJK Compatibility Forms and Vertical Forms added (commit
0aa4fdc8f). - ✅ NBSP redundancy removed.
- ✅ Doc comment now correctly lists U+3006 〆.
- ✅ CI green, 198 targeted tests pass,
tscclean.
See inline comments for the specific lines.
| @@ -38,7 +39,7 @@ const UNDERLINE_TAG_START_LENGTH = 3; // For "<u>" | |||
| const UNDERLINE_TAG_END_LENGTH = 4; // For "</u>" | |||
| const INLINE_MARKDOWN_REGEX = new RegExp( | |||
| String.raw`(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|${MD_LINK_PATTERN}|` + | |||
There was a problem hiding this comment.
[P0 Critical] Re-tokenization: after BARE_URL_PATTERN (last in the alternation) truncates a URL at CJK punctuation, the tail is re-scanned by the earlier branches. Input 见 https://a.com/x(说明_1)和 https://b.com/y_。 produces tokens ["https://a.com/x", "_1)和 https://b.com/y_"] — the second URL loses its OSC 8 envelope, and the underscores are silently deleted from visible text.
Pre-PR \S+ kept both URLs inside one giant (broken) link; now the second URL is plain text with no clickable target at all.
One possible direction: add a negative lookbehind so _.*?_ / \*.*?\* / backtick spans cannot start inside text that was already part of a URL run — or ensure a bare-URL match followed by CJK punctuation is followed by a non-capturing word-boundary reset.
| @@ -50,7 +51,7 @@ const SAFETY_MARGIN = 4; | |||
|
|
|||
| const INLINE_MARKDOWN_REGEX = new RegExp( | |||
| String.raw`(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|${MD_LINK_PATTERN}|` + | |||
There was a problem hiding this comment.
Same re-tokenization issue as InlineMarkdownRenderer.tsx:41 — this is the identical INLINE_MARKDOWN_REGEX. See that thread for the repro and discussion.
|
Formal review submitted: REQUEST_CHANGES (review #4898282561). |
|
Closeout: fixed the cross-URL underscore/emphasis collision in both renderers and kept the visible trailing underscore while excluding it from the OSC 8 target before full-width punctuation. Verified 104 focused tests. Intentionally not changed: U+00B7/U+2027 and broader non-ASCII typography expansion after repeated review rounds. All active threads are closed; exact-head CI/review is pending. 中文摘要已修复两个渲染器中的跨 URL 下划线误解析,104 个测试通过;未扩展到 U+00B7/U+2027 等额外标点,线程已清零,等待 CI/评审。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR #8755 (fixes #8750) replaces the duplicated b...: none — all planned checks completed within budget.; Context: PR #8755 (fixes #8750) replaces the duplicated b...: none significant. What I did not do: full test suite run (unnecessary), read every consumer of RenderInline (sampled). I'll state explicitly that I did not chec….
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):Context: PR #8755 (fixes #8750) replaces the duplicated b...:none — all planned checks completed within budget.;Context: PR #8755 (fixes #8750) replaces the duplicated b...:none significant. What I did not do: full test suite run (unnecessary), read every consumer of RenderInline (sampled). I'll state explicitly that I did not chec…。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| * (U+FE10–U+FE1F). ASCII and other typographic punctuation stays matched and | ||
| * is left to `trimTrailingUrlPunctuation`. | ||
| */ | ||
| export const BARE_URL_PATTERN = String.raw`https?:\/\/[^\s\u3001-\u3004\u3008-\u303f\uff01-\uff0f\uff1a-\uff20\uff3b-\uff40\uff5b-\uff65\ufe10-\ufe1f\ufe30-\ufe6f]+`; |
There was a problem hiding this comment.
[Critical] R5-1: The exclusion band \u3008-\u303f sweeps over word-forming code points that the pattern's own doc comment ("Raw CJK ideographs … stay in the match because they are word-forming IRI characters") and the R1-1/R2-1 carve-outs for U+3005/3006/3007 require to stay matched: U+303C 〼 (General_Category Lo — same category as deliberately retained 〆), U+303B 〻 (Lm, same as 々), U+3031–U+3035 (Lm vertical kana repeat marks), U+3021–U+3029 and U+3038–U+303A (Nl Hangzhou numerals, same as 〇); U+FE33/U+FE34 (Pc, presentation forms of _) inside the \ufe30-\ufe6f band are also excluded while ASCII _ stays matched. — Failure scenario: probe against the committed pattern: new RegExp(BARE_URL_PATTERN).exec('https://ja.example.com/〼道と〻次') returns only https://ja.example.com/ — the match stops mid-word at 〼, so the OSC 8 target becomes the bare host root and the click 404s while the visible text shows the full run; same truncation for 〳〴〵 (U+3031–35), 〡〢 (U+3021–22), 〸〹 (U+3038–39) and a︳b/a︴b (U+FE33/FE34). Regression versus pre-PR \S+, and the same defect class R1-1/R2-1 caught for 々〆〇 (R2-1's parenthetical named 〻/〼 as the same class; they were never carved out).
// In BARE_URL_PATTERN, replace `\u3008-\u303f` with the word-forming carve-out
// \u3008-\u3020\u302e-\u3030\u3036-\u3037\u303d-\u303f
// and drop \ufe33\ufe34 from the FE band (\ufe30-\ufe32\ufe35-\ufe6f).
// Mirror the identical change into the `_`-trim guard's character class in
// trimTrailingUrlPunctuation so the two literals stay in sync (see R5-5),
// and add keep-tests for 〼/〻 next to the existing 々〆〇 ones.中文说明
[严重] R5-1:排除区间 \u3008-\u303f 扫过了模式自身文档注释("原始 CJK 表意文字……作为构词 IRI 字符保留在匹配中")以及 R1-1/R2-1 为 U+3005/3006/3007 做的挖空所要求保留的构词码点:U+303C 〼(General_Category Lo——与刻意保留的 〆 同类)、U+303B 〻(Lm,与 々 同类)、U+3031–U+3035(Lm,竖排假名重复标记)、U+3021–U+3029 与 U+3038–U+303A(Nl,苏州码子,与 〇 同类);\ufe30-\ufe6f 区间内的 U+FE33/U+FE34(Pc,_ 的展示形式)也被排除,而 ASCII _ 却保留在匹配中。— 失效场景:对已提交的模式做探测:new RegExp(BARE_URL_PATTERN).exec('https://ja.example.com/〼道と〻次') 只返回 https://ja.example.com/ ——匹配在 〼 处从词中间断开,OSC 8 目标变成裸主机根路径,点击即 404,而可见文本显示的是完整串;〳〴〵(U+3031–35)、〡〢(U+3021–22)、〸〹(U+3038–39)以及 a︳b/a︴b(U+FE33/FE34)同样被截断。相对 PR 之前的 \S+ 这是回退,与 R1-1/R2-1 在 々〆〇 上抓到的是同一缺陷类别(R2-1 的括号附注当时就点名 〻/〼 属于同一类,但从未挖空)。修复需同步两处字符类字面量(见 R5-5),并为 〼/〻 增加保留用例。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| String.raw`(\*\*.*?\*\*|\*.*?\*|(?<![\w\u3400-\u9fff])_.*?_(?![\w\u3400-\u9fff])|~~.*?~~|${MD_LINK_PATTERN}|` + | ||
| String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`, |
There was a problem hiding this comment.
[Critical] R5-2: The new emphasis closing-lookahead turns INLINE_MARKDOWN_REGEX quadratic on inputs with many underscores that pass the lookbehind but whose every closing candidate fails the lookahead (e.g. _a _b _c …): the lazy .*? extends to end-of-string for each candidate and the engine restarts that O(n) scan from every eligible _. — Failure scenario: benchmarked on the committed combined regex — " _a".repeat(N): 9 kB = 17.7 ms (old pattern 0.31 ms), 30 kB = 182.6 ms (old 0.77 ms), 90 kB = 1753 ms (old 1.39 ms); ~×10 time per ×3 length = quadratic. This regex runs on the chat display path and is re-executed as messages stream in, so a code-heavy message listing underscore-prefixed identifiers compounds into multi-second UI freezes; pre-PR the same input scanned in ~1 ms. Identical copy at TableRenderer.tsx:53-54.
// Bound the span so a failed attempt cannot rescan the whole tail — e.g.
// forbid '_' inside the emphasis span:
// (?<![\w\u3400-\u9fff])_[^_]*_(?![\w\u3400-\u9fff])
// (decide the interior-'_' tradeoff explicitly — `_a_b_` no longer matches),
// or validate boundaries in code rather than in-pattern. Apply in BOTH
// renderers, and add a regression check that the scan stays linear on
// '_a _b _c…'-style input.中文说明
[严重] R5-2:新的强调闭合前瞻使 INLINE_MARKDOWN_REGEX 在"许多下划线都能通过环视后顾、但每个闭合候选又都被前瞻拒绝"的输入(如 _a _b _c …)上退化为平方复杂度:懒惰 .*? 对每个候选都延伸到字符串末尾,引擎又从每个合格的 _ 重启这次 O(n) 扫描。— 失效场景:在已提交的组合正则上实测——" _a".repeat(N):9 kB = 17.7 ms(旧模式 0.31 ms),30 kB = 182.6 ms(旧 0.77 ms),90 kB = 1753 ms(旧 1.39 ms);长度 ×3 耗时约 ×10,即平方级。该正则运行在聊天展示路径上并随流式消息增长反复执行,代码密集、列举下划线开头标识符的消息会叠加成数秒的 UI 冻结;PR 之前同样输入只需约 1 ms。TableRenderer.tsx:53-54 有完全相同的副本。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| String.raw`(\*\*.*?\*\*|\*.*?\*|(?<![\w\u3400-\u9fff])_.*?_(?![\w\u3400-\u9fff])|~~.*?~~|${MD_LINK_PATTERN}|` + | ||
| String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`, |
There was a problem hiding this comment.
[Critical] R5-11: The new emphasis guard makes __init__-style dunder identifiers match as ONE _…_ token: the inner __ closers are rejected (the char after the second _ is a word char), so the lazy match extends to the final _, and the italic branch renders fullMatch.slice(1, -1) — two underscores vanish from the visible text. — Failure scenario: probe through the real renderers at this commit: Python 的 __init__ 方法 renders Python 的 _init_ 方法; __init__ is special renders _init_ is special; see __main__, ok renders see _main_ , ok — in BOTH the React renderer and the ANSI table renderer. Pre-PR, _.*?_ matched the two inner __ pairs (length 2, rejected by the length > 2 italic check) and the output stayed byte-intact (flip arm verified). Unbackticked Python dunders are routine in the dev-chat output this renderer serves; no test pins this case. Identical defect at TableRenderer.tsx:53-54 (an older unrelated comment occupies that line, so this one is anchored here).
// Preserve `__` pairing semantics — e.g. change the emphasis alternative to
// (?<![\w\u3400-\u9fff])_(?!_).*?_(?![\w\u3400-\u9fff])
// or reject '_'-delimited matches whose content itself starts/ends with '_'
// in the italic branch. Apply in BOTH renderers; add a regression test
// rendering 'Python 的 __init__ 方法' and asserting '__init__' stays intact.中文说明
[严重] R5-11:新的强调守卫让 __init__ 这类双下划线标识符整体匹配为一个 _…_ 记号:内部的 __ 闭合被拒绝(第二个 _ 之后是单词字符),懒惰匹配延伸到最后一个 _,斜体分支随后渲染 fullMatch.slice(1, -1) ——可见文本丢失两个下划线。— 失效场景:在本 commit 上用真实渲染器探测:Python 的 __init__ 方法 渲染为 Python 的 _init_ 方法;__init__ is special 渲染为 _init_ is special;see __main__, ok 渲染为 see _main_ , ok ——React 渲染器与 ANSI 表格渲染器均如此。PR 之前 _.*?_ 匹配到的是内部两个 __ 对(长度 2,被 length > 2 的斜体检查拒绝),输出逐字节完好(翻转对照已验证)。未加反引号的 Python dunder 在该渲染器承载的开发对话输出中非常常见,而当前没有任何测试钉住此场景。TableRenderer.tsx:53-54 存在同样缺陷(该行已有一条不相关的旧评论,因此本条锚定在此处)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| String.raw`(\*\*.*?\*\*|\*.*?\*|(?<![\w\u3400-\u9fff])_.*?_(?![\w\u3400-\u9fff])|~~.*?~~|${MD_LINK_PATTERN}|` + | ||
| String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`, |
There was a problem hiding this comment.
[Suggestion] R5-4: The emphasis guard word class [\w\u3400-\u9fff] covers only ASCII word chars + CJK Unified Ideographs (incl. Ext A): hiragana/katakana, hangul, CJK Compatibility Ideographs (U+F900–FAFF) and astral CJK (the regex is compiled without the u flag, so the lookarounds see lone surrogates) still allow the cross-URL underscore-emphasis swallow this PR fixes — the fix is script-partial against its own test shape. Not a regression (pre-PR unguarded _.*?_ behaved identically for these scripts). — Concrete cost: probe with the committed combined regex: カタカナ_説明 https://a.com/doc_。 → single token _説明 https://a.com/doc_ (URL never linkified); (せつめい_1)和 https://b.com/y_。 → _1)和 https://b.com/y_; \uF900_https://b.com/y_。 → _https://b.com/y_; hangul likewise — the exact shape the new tests assert is fixed, reproduced with sibling scripts. Identical at TableRenderer.tsx:53.
// Widen the guard to the word-forming scripts the URL pattern keeps, e.g.
// (?<![\p{L}\p{N}_])_.*?_(?![\p{L}\p{N}_]) // with the `u` flag (covers astral)
// or extend the explicit ranges (U+3040–30FF, U+AC00–D7AF, U+F900–FAFF),
// in BOTH renderers. If the narrow scope is deliberate, say so in a comment.中文说明
[建议] R5-4:强调守卫的单词类 [\w\u3400-\u9fff] 只覆盖 ASCII 单词字符 + CJK 统一表意文字(含扩展 A):平假名/片假名、谚文、CJK 兼容表意文字(U+F900–FAFF)以及增补平面 CJK(正则未带 u 标志编译,环视看到的是孤立代理项)仍然允许本 PR 所修复的"跨 URL 下划线强调吞并"——相对其自身测试形状,该修复按文字范围是不完整的。这并非回退(PR 之前未设防的 _.*?_ 对这些文字的行为相同)。— 具体代价:用已提交的组合正则探测:カタカナ_説明 https://a.com/doc_。 → 单一记号 _説明 https://a.com/doc_(URL 不再被链接化);(せつめい_1)和 https://b.com/y_。 → _1)和 https://b.com/y_;\uF900_https://b.com/y_。 → _https://b.com/y_;谚文同理——正是新测试断言已修复的形状,在兄弟文字上复现。TableRenderer.tsx:53 同样如此。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| /[\u3001-\u3004\u3008-\u303f\uff01-\uff0f\uff1a-\uff20\uff3b-\uff40\uff5b-\uff65\ufe10-\ufe1f\ufe30-\ufe6f]/.test( | ||
| nextCharacter, |
There was a problem hiding this comment.
[Suggestion] R5-5: The ten-range CJK/full-width stop-punctuation class is written as two independent literals in this same file — the _-trim guard here and the negated class in BARE_URL_PATTERN at line 204. They are semantically coupled (the guard must fire exactly where the pattern stops) but nothing enforces that, and no test pins them together (the trim describe block calls the function one-arg only). — Concrete cost: this PR's own review history iterated these exact ranges repeatedly (restoring 々〆〇, adding Small Form Variants and Vertical Forms); a future one-sided edit makes a URL ending in _ before a newly added stop char keep the _ in the OSC 8 target (or strips _ where the pattern no longer stops), silently reintroducing the wrong-click-target class of #8750 with green tests.
// Extract the ranges once:
// const CJK_URL_STOP_SOURCE = String.raw`\u3001-\u3004\u3008-\u303f\uff01-\uff0f\uff1a-\uff20\uff3b-\uff40\uff5b-\uff65\ufe10-\ufe1f\ufe30-\ufe6f`;
// then build BARE_URL_PATTERN as
// String.raw`https?:\/\/[^\s${CJK_URL_STOP_SOURCE}]+`
// and the trim check as a module-level
// new RegExp(`[${CJK_URL_STOP_SOURCE}]`)中文说明
[建议] R5-5:十段 CJK/全角终止标点字符类在同一文件中写成了两个独立字面量——此处的 _ 裁剪守卫与第 204 行 BARE_URL_PATTERN 中的取反字符类。二者语义耦合(守卫必须在模式停止之处恰好触发),但没有任何机制强制一致,也没有测试把它们钉在一起(trim 的 describe 块仍只用单参调用)。— 具体代价:本 PR 自己的评审历史就反复迭代过这些区间(恢复 々〆〇、补充小变体与竖排形式);未来单边修改会让"以 _ 结尾、后接新增终止字符"的 URL 在 OSC 8 目标中保留 _(或在模式已不再停止之处剥掉 _),在测试全绿的情况下悄悄重新引入 #8750 的错误点击目标类别。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| String.raw`(\*\*.*?\*\*|\*.*?\*|(?<![\w\u3400-\u9fff])_.*?_(?![\w\u3400-\u9fff])|~~.*?~~|${MD_LINK_PATTERN}|` + | ||
| String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`, |
There was a problem hiding this comment.
[Suggestion] R5-8: The new emphasis lookarounds are tested only for their CJK half — deleting \w from both guard classes survives the entire suite (the only snake_case content in either suite sits inside backticks and never reaches the emphasis alternative). — Concrete cost: mutation-tested at this commit: removing \w from both guard classes leaves 200/200 green; the real untested consequence is de-linkification — the mutated regex on see foo_https://x.com_ bar yields token _https://x.com_ (URL never linkified), whereas the committed lookbehind rejects and https://x.com_ is linkified. (Visible-text corruption does not occur — the runtime italic guards catch that.) Identical at TableRenderer.tsx:53.
// Add one case per renderer, e.g.:
// render 'see foo_https://x.com_ bar' with hyperlinks enabled and
// assert the OSC 8 envelope wraps 'https://x.com_'.中文说明
[建议] R5-8:新的强调环视只测试了 CJK 一半——把 \w 从两个守卫类中删掉后整个测试套件仍然通过(两个套件中唯一的 snake_case 内容都在反引号内,永远到不了强调分支)。— 具体代价:在本 commit 上做变异测试:从两个守卫类中移除 \w,200/200 依旧全绿;真正未被测试的后果是链接化失效——变异后的正则对 see foo_https://x.com_ bar 产生记号 _https://x.com_(URL 不再被链接化),而已提交的环视后顾会拒绝该匹配,使 https://x.com_ 正常链接化。(可见文本损坏不会发生——运行期斜体守卫会拦截。)TableRenderer.tsx:53 同样如此。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| * (U+FE10–U+FE1F). ASCII and other typographic punctuation stays matched and | ||
| * is left to `trimTrailingUrlPunctuation`. | ||
| */ | ||
| export const BARE_URL_PATTERN = String.raw`https?:\/\/[^\s\u3001-\u3004\u3008-\u303f\uff01-\uff0f\uff1a-\uff20\uff3b-\uff40\uff5b-\uff65\ufe10-\ufe1f\ufe30-\ufe6f]+`; |
There was a problem hiding this comment.
[Suggestion] R5-9: Three exclusion bands of BARE_URL_PATTERN have no test character at all: \u3008-\u303f (「」《》〈〉…), \uff3b-\uff40 (full-width [\]^_`), and \uff5b-\uff65 ({|}~…・) — tests exercise only U+3001–3002, FF01/FF08/FF0C/FF1A/FF1F, FE12 and FE56/FE58/FE59. — Concrete cost: mutation-tested per band at this commit: deleting \u3008-\u303f passes the entire 200-test suite, yet 参考https://x.com「文档」 would again swallow 「文档」 into the OSC 8 target — the exact #8750 bug class; deleting \uff3b-\uff40 or \uff5b-\uff65 likewise survives with the probe flipping (a_b/a~b swallowed). All three bands are load-bearing and unpinned.
// Add it.each rows such as:
// ['https://example.com/page「引用」', 'https://example.com/page'],
// ['https://example.com/page_後続', 'https://example.com/page'],
// ['https://example.com/page|注', 'https://example.com/page'],中文说明
[建议] R5-9:BARE_URL_PATTERN 有三个排除区间完全没有测试字符覆盖:\u3008-\u303f(「」《》〈〉等)、\uff3b-\uff40(全角 [\]^_`)、\uff5b-\uff65({|}~…・)——现有测试只覆盖 U+3001–3002、FF01/FF08/FF0C/FF1A/FF1F、FE12 与 FE56/FE58/FE59。— 具体代价:在本 commit 上逐区间变异测试:删除 \u3008-\u303f 后整个 200 测试套件全部通过,但 参考https://x.com「文档」 会再次把 「文档」 吞进 OSC 8 目标——正是 #8750 的缺陷类别;删除 \uff3b-\uff40 或 \uff5b-\uff65 同样全绿且探测翻转(a_b/a~b 被吞)。三个区间都是承重的,却没有测试钉住。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
Changed: preserve word-forming CJK/vertical URL characters, make underscore emphasis scanning linear, and keep dunder identifiers visible in both renderers. Verified: 205 focused tests, ESLint, Prettier, and a 9/30/90 kB linear-scaling probe (0.071/0.122/0.291 ms) passed. CLI package typecheck was blocked by unrelated unbuilt workspace artifacts. Intentionally not changed: additional late-round edge-case and mutation-only coverage expansion. Pending: exact-head CI and automatic review. 中文摘要已修复 CJK URL 截断、下划线正则平方退化和 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR #8755 (fixes #8750) replaces the duplicated b...: none — all planned checks completed within budget (~15 tool calls).; Context: PR #8755 (fixes #8750) replaces the duplicated b...: none serious. Haven't fully read TableRenderer.tsx lines 1–258 (imports/constants) and InlineMarkdownRenderer.tsx lines 320–347 (tail of getPlainTextLength) — b…; Context: PR #8755 (fixes #8750) replaces the duplicated b...: none material — did I verify everything I set out to verify? One thing I didn't do: trace the full OSC envelope against a terminal for astral-plane URLs / did n…; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within budget.; Context: PR #8755 (fixes #8750) replaces the duplicated b...: running the three test files under vitest (no node_modules in the review worktree; all verification above is static).
中文说明
已审查。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):Context: PR #8755 (fixes #8750) replaces the duplicated b...:none — all planned checks completed within budget (~15 tool calls).;Context: PR #8755 (fixes #8750) replaces the duplicated b...:none serious. Haven't fully read TableRenderer.tsx lines 1–258 (imports/constants) and InlineMarkdownRenderer.tsx lines 320–347 (tail of getPlainTextLength) — b…;Context: PR #8755 (fixes #8750) replaces the duplicated b...:none material — did I verify everything I set out to verify? One thing I didn't do: trace the full OSC envelope against a terminal for astral-plane URLs / did n…;You are review agent reverse-audit — Reverse audit agen...:none — all checks above were completed within budget.;Context: PR #8755 (fixes #8750) replaces the duplicated b...:running the three test files under vitest (no node_modules in the review worktree; all verification above is static)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
fixed
|
Released in v0.21.10. |





What this PR does
This replaces the duplicated bare-URL matcher in the React inline markdown renderer and ANSI table renderer with one shared boundary that stops at CJK / full-width punctuation while preserving word-forming CJK IRI characters such as
〆. The existing ASCII trailing-punctuation behavior remains unchanged.Why it's needed
Chinese prose routinely glues full-width punctuation directly onto URLs —
https://x.com(2 commits),见 https://x.com。— and\Smatches full-width characters, so the bare-URL linkifier swallowed the punctuation plus everything up to the next ASCII space into both the visible hyperlink region and the OSC 8 click target; clicking opened a 404. Reported in #8750 with a real-session screenshot (underline ran past the URL into(2).Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/ui/utils/osc8.test.ts src/ui/utils/InlineMarkdownRenderer.test.tsx src/ui/utils/TableRenderer.test.tsx— confirm the shared matcher stops before glued-on punctuation such as(and。, preserves word-forming CJK IRI characters such as〆, and keeps both renderers aligned.FORCE_HYPERLINK=1), have the model emitPR:https://github.com/QwenLM/qwen-code/pull/8742(2 commits,等 CI)as a bare URL, and confirm the clickable region / OSC 8 target is exactly the URL.Evidence (Before & After)
Real TTY sessions driven via
script(1)withFORCE_HYPERLINK=1; model asked to echoPR:https://github.com/QwenLM/qwen-code/pull/8742(2 commits,等 CI)verbatim. OSC 8 targets extracted from the terminal log:9e8745349):https://github.com/QwenLM/qwen-code/pull/8742andhttps://github.com/QwenLM/qwen-code/pull/8742(2— the reply line's hyperlink target swallowed(2, the broken click target from bug(cli): bare-URL hyperlink swallows trailing full-width/CJK punctuation into the link target #8750.https://github.com/QwenLM/qwen-code/pull/8742;(2 commits,等 CI)renders as plain text after the link.Tested on
Environment (optional)
Node v22.22.0,
npm run build, CLI driven in ascript(1)pty withFORCE_HYPERLINK=1.Risk & Scope
Linked Issues
Resolves #8750
中文说明
这个 PR 做了什么
把 React 行内 Markdown 渲染器和 ANSI 表格渲染器里重复的裸 URL 匹配替换为一个共享边界,使匹配在 CJK / 全角标点处停止,同时保留像
〆这样的构词 CJK IRI 字符。现有 ASCII 尾部标点处理保持不变。为什么需要
中文文本常把全角标点直接贴在 URL 后面,例如
https://x.com(2 commits)、见 https://x.com。。由于\S会匹配全角字符,旧的裸 URL 链接化会把标点及其后直到下一个 ASCII 空格的内容一起吞进可见的下划线区域和 OSC 8 点击目标,点击后会打开 404。#8750 的真实会话截图中,下划线已越过 URL 并包含(2。Reviewer 测试计划
如何验证
cd packages/cli && npx vitest run src/ui/utils/osc8.test.ts src/ui/utils/InlineMarkdownRenderer.test.tsx src/ui/utils/TableRenderer.test.tsx。确认共享匹配会在(、。等粘连标点前停止,保留〆等构词 CJK IRI 字符,并让两个渲染器保持一致。FORCE_HYPERLINK=1),让模型以裸 URL 形式输出PR:https://github.com/QwenLM/qwen-code/pull/8742(2 commits,等 CI),确认可点击区域和 OSC 8 目标严格等于 URL。证据(修改前后)
使用
script(1)和FORCE_HYPERLINK=1驱动真实 TTY,会话中要求模型原样回显PR:https://github.com/QwenLM/qwen-code/pull/8742(2 commits,等 CI),并从终端日志中提取 OSC 8 目标:9e8745349):同时出现https://github.com/QwenLM/qwen-code/pull/8742和https://github.com/QwenLM/qwen-code/pull/8742(2;回复行的链接目标错误地吞入了(2,即 bug(cli): bare-URL hyperlink swallows trailing full-width/CJK punctuation into the link target #8750 中的坏链接。https://github.com/QwenLM/qwen-code/pull/8742;(2 commits,等 CI)在链接后作为普通文本渲染。测试平台
环境(可选)
Node v22.22.0,执行
npm run build,并在script(1)pty 中设置FORCE_HYPERLINK=1驱动 CLI。风险与范围
关联 Issue
Resolves #8750