Repository navigation
fix(console): bound the long-name check so joiner runs can't hang the Agents list - #2926
Conversation
… Agents list The joiner alternative of UNBROKEN_RUN matched any character before a word joiner or zero-width joiner, overlapping the plain-character, hyphen and wide-space/dash alternatives (CLOSING lists the joiners) and letting a combining mark start a run character. Each overlap doubled the ways to split a run, so a failing test of a joiner-run name backtracked exponentially: 10.7 s for one 210-character name, about 2.5 s per search keystroke in the console (finding 1999). Restrict the joiner alternative to ideographs and emoji, the only characters it added. The rule is now linear (under 1 ms on the bughunt payloads, 1,050 characters included). A combining mark before a joiner no longer counts as a character of its own, which matches Chromium's measured layout. Tests: a conformance test bounds the check at 50 ms on worst-case joiner names (10.7 s on main); a browser test times search keystrokes with joiner-run Agents listed (5 s per keystroke on main); two measured rows pin the combining-mark case.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Internal review of the bound: a hyphen-minus before an Arabic-Indic digit, with no letter or digit before it, still matched two alternatives (bounded, about 2^7 per start, but against the rule's own comment). The "-5" alternative now leaves those digits to the line above, and the conformance test times that shape too.
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs maintainer review before merge. What this changesThis PR removes overlapping regex alternatives that make certain Agent names stall list rendering and search. Example: Search for an Agent named "Joiners " followed by nine space-separated groups of 20 U+2060 word joiners, totaling 196 characters.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness✅ Ready for maintainer review This PR addresses a current Agents-list freeze, resolves both prior findings, and introduces no further identified code defect. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe console classifies each displayed Agent name during list rendering and synchronous search updates, then applies a CSS class when the name needs additional wrapping. flowchart LR
A[Agent names] --> B[Agents list render]
C[Search input] --> B
B --> D[Unbroken run classification]
D --> E[Wrapped name link]
Technical reviewBest possible solution: Keep the focused classifier repair and its browser and adversarial-name regression coverage. Do we have a high-confidence way to reproduce the issue? Current main contains overlapping alternatives inside repeated matching, and the contributor supplies a supported 196-character browser scenario with before-fix measurements; no target code was executed during this review. Is this the best way to solve the issue? The repair removes the matching ambiguity at its existing owner while retaining Chromium-based wrapping behavior, including the previously missed ideographic combining mark. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against b10157ca98de. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (2 earlier review cycles)Reviewed October 19, 2026, 7:32 AM ET / 11:32 UTC (Revision 3). |
…through Review request: Pages/Agents gains a 196-character name of word-joiner runs, found by a search, with steps to type the search one letter at a time.
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:bug
…lternative U+16FE4 is both Ideographic and a combining mark, so it could start the ideograph-before-joiner alternative and also ride a previous character's trailing marks. A name of 'a' + 18 such marks + U+2060, repeated, took 3.6 s to classify. Skip marks in that alternative; a scan of every code point now finds no character that matches two alternatives and no mark that starts one. Chromium measures the two new positive-control names as runs of 2 and 21, which the rule now matches.
|
Free's clanker here. Addressed the U+16FE4 finding in 2bfd012: the ideograph-before-joiner alternative now skips combining marks, and a scan of every code point finds no remaining overlap or mark-started alternative. The payload from the review (and a 10-segment version) now classifies in a few ms, with conformance rows pinning it (Chromium measures runs of 2 and 21, which the rule matches). @clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
clawsweeper:intake:bug
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Problem
The long-name rule from #2916 (
UNBROKEN_RUNinapps/controller/src/console/agents/list.mjs) backtracks exponentially on names built from word-joiner (U+2060) or zero-width-joiner (U+200D) runs (finding 1999, medium).CLOSINGlists the joiners. It also let a combining mark start a run character of its own.{21,}attempt explored about 2^20 paths. One check of a 210-character name took 10.7 s in Node.Change
[${IDEOGRAPHIC}](?=\p{M}*[\u2060\u200d])). Those are the only characters it added; every other character a joiner holds is already counted by another alternative. No character now matches two alternatives, and no combining mark can start one, so the check is linear."e\u0301\u2060" x 10was marked on main, but Chromium lays it out as a run of 20. Two measured rows pin this.hasUnbrokenRun(name), so a Node test can time the rule directly.(?!\p{M})). A scan of every code point, with each neighbour shape the rule looks at, finds no character that matches two alternatives and no combining mark that starts one."-5"alternative now leaves those digits to the line above it. Marking is unchanged.Verification
New
tests/conformance/console-agent-name-runs.test.mjs(mapped to Checks and Conformance 1):a+ 18 U+16FE4 + U+2060, six times, took 3.65 s; now 4 ms.New browser test
Agents list search stays responsive with joiner-run names(console-agents-list-return.test.mjs): two 196-character joiner-run Agents are listed, and each search keystroke must finish its synchronous re-render in under 250 ms. On main the first keystroke blocked for 4,971 ms.Agents list marks names with no break opportunity...gains two measured rows (combining mark before a joiner, run of 20 unmarked and 21 marked). On main it fails ("marked"). All 32 fix(console): match Chromium's break points for long Agent names #2916 rows still pass, and each is still checked against Chromium's own layout.Pinned at 2 CPUs (
taskset -c 12,13) on head 82c4317:console-agents-list-return.test.mjs, 19/19.Old-versus-new differential on 40,000 random short names over the rule's special characters: no difference outside the combining-mark case.
Random and periodic 200-character names under the new rule: the slowest took 4 ms (JIT warm-up).
Prettier and ESLint are clean on the touched files.
run-tests.mjs auditreports no issues.Conflicts with open PRs (3-way
git merge-treeagainst this head, compared withorigin/main): no new conflict hunk.list.mjs): the same 2 hunks as against main.checks-baseline-1.json): auto-merge cleanly.Storybook: new story Pages/Agents → Long name of word-joiner runs (
agentsLongNameJoiners), a 196-character joiner-run name found by a search, with a step to type the search one letter at a time. On the fixture frame, each keystroke's synchronous handling took 0.4-0.6 ms at 1440 and 390.UX follow-up to bughunt-127 (finding 1999). Ordinary names look the same.
Risks
Low. The change narrows two regex alternatives and adds tests. The only behaviour change is the combining-mark-before-joiner case, and that now matches Chromium.