From 68814755323c73ce50d4b7ba7991df7503e7e653 Mon Sep 17 00:00:00 2001 From: gatsby74 <166927047+gatsby74@users.noreply.github.com> Date: Fri, 26 Jun 2026 03:36:37 +0200 Subject: [PATCH] fix(ports): stop cursor-move redraws corrupting advertised dev-server hosts (#6371) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ports): stop cursor-move redraws corrupting advertised dev-server hosts The Live Ports panel showed `localhst:5199` (and opened a broken URL with the missing `o`) for a dev server that printed `http://localhost:5199`. AdvertisedUrlWatcher cleans raw PTY bytes with stripTerminalControls, which *deletes* ANSI/cursor sequences instead of emulating them. CLIs that redraw a line differentially step the cursor forward over characters already on screen (`ESC[1C`) rather than reprinting them; a real terminal renders the underlying glyph, but deleting the move splices the two text runs together and drops the skipped cell — `http://localh` + `ESC[1C` + `st` collapses to `localhst`. `new URL()` then accepts the corrupted, plausible-looking host verbatim, so it gets cached, displayed, copied, and opened. Neutralize horizontal cursor moves (forward/back `C`/`D`, absolute column `G`, position `H`/`f`) to a space before the CSI delete pass, so the URL candidate matcher breaks at the seam and can't fuse a corrupted hostname. Worst case we skip that frame and fall back to the kernel bind, which is the safe default. Co-Authored-By: Claude Opus 4.8 (1M context) * docs(ports): document stripTerminalControls Add a docstring to the exported helper to clear CodeRabbit's docstring coverage warning and explain why horizontal cursor moves are neutralized. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(ports): invalidate cursor-redrawn URL candidates Co-authored-by: Orca --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Jinwoo-H Co-authored-by: Orca --- src/main/ports/advertised-url-watcher.test.ts | 24 +++++++++++++++++++ src/main/ports/advertised-url-watcher.ts | 9 +++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/main/ports/advertised-url-watcher.test.ts b/src/main/ports/advertised-url-watcher.test.ts index d7de883a8ab..d46b8268fab 100644 --- a/src/main/ports/advertised-url-watcher.test.ts +++ b/src/main/ports/advertised-url-watcher.test.ts @@ -30,6 +30,15 @@ describe('stripTerminalControls', () => { it('drops non-printable bytes but keeps whitespace and printable ASCII', () => { expect(stripTerminalControls('a\x00b\x08c\td')).toBe('abc\td') }) + + it('turns horizontal cursor moves into a URL-invalid guard', () => { + // A differential redraw steps the cursor over the on-screen `o` instead of + // reprinting it. Deleting the move would splice `localh` + `st` into `localhst`. + expect(stripTerminalControls('http://localh\x1b[1Cst:5199/')).toBe('http://localh[st:5199/') + // Absolute column (G) and position (H) moves are neutralized the same way. + expect(stripTerminalControls('localh\x1b[8Gst')).toBe('localh[st') + expect(stripTerminalControls('localh\x1b[1;9Hst')).toBe('localh[st') + }) }) describe('extractUrlCandidates', () => { @@ -57,6 +66,21 @@ describe('extractUrlCandidates', () => { expect(extractUrlCandidates('example.com:3001')).toHaveLength(0) }) + it('does not capture a corrupted host from a cursor-skip redraw', () => { + // Regression: a CLI redrawing `http://localhost:5199/` via a cursor-forward + // over the already-drawn `o` must not yield the plausible-but-wrong + // `localhst` host that `new URL()` would otherwise accept verbatim. + const cleaned = stripTerminalControls(' > Local: http://localh\x1b[1Cst:5199/\r\n') + const urls = extractUrlCandidates(cleaned) + expect(urls).toHaveLength(0) + }) + + it('does not cache a partial default-port URL from a cursor-skip redraw', () => { + const watcher = bindFresh() + watcher.ingest(PTY, 'Local: http://localh\x1b[1Cst/\n') + expect(watcher.lookup(WORKTREE, 80)).toBeUndefined() + }) + it('handles multiple URLs in one line', () => { const urls = extractUrlCandidates( 'Local: http://localhost:3001/ Network: https://custom:3001/' diff --git a/src/main/ports/advertised-url-watcher.ts b/src/main/ports/advertised-url-watcher.ts index 804237e3fd6..b5a9876b55d 100644 --- a/src/main/ports/advertised-url-watcher.ts +++ b/src/main/ports/advertised-url-watcher.ts @@ -21,9 +21,13 @@ const MAX_PENDING_ENTRIES = 32 const MAX_CACHE_ENTRIES = 256 const URL_CANDIDATE_LIMIT = 2048 -// ANSI/OSC strippers mirror normalizeTerminalChunk in -// src/main/runtime/orca-runtime.ts so the two stay in lockstep. +// ANSI/OSC strippers mirror the runtime normalizer, with URL-specific cursor +// move handling below to avoid fusing text that a real terminal would skip. const OSC_PATTERN = /\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)/g +// Why: cursor moves in differential redraws can skip cells that are already on +// screen. Replacing them with a URL-invalid guard skips the damaged candidate. +const CURSOR_MOVE_PATTERN = /\x1b\[[0-?]*[ -/]*[CDGHf]/g +const CURSOR_MOVE_URL_GUARD = '[' const CSI_PATTERN = /\x1b\[[0-?]*[ -/]*[@-~]/g const SINGLE_ESC_PATTERN = /\x1b[@-_]/g const CONTROL_PATTERN = /[\x00-\x08\x0b-\x1f\x7f]/g @@ -117,6 +121,7 @@ export function stripTerminalControls(text: string): string { .replace(/\r\n/g, '\n') .replace(/\r/g, '\n') .replace(OSC_PATTERN, '') + .replace(CURSOR_MOVE_PATTERN, CURSOR_MOVE_URL_GUARD) .replace(CSI_PATTERN, '') .replace(SINGLE_ESC_PATTERN, '') .replace(CONTROL_PATTERN, '')