Skip to content

fix: silence xterm.js parser diagnostics leaking from headless shell terminals#7663

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
mvanhorn:fix/7631-shell-headless-terminal-loglevel
Jul 25, 2026
Merged

fix: silence xterm.js parser diagnostics leaking from headless shell terminals#7663
wenshao merged 1 commit into
QwenLM:mainfrom
mvanhorn:fix/7631-shell-headless-terminal-loglevel

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

What this PR does

Sets logLevel: 'off' on the two headless @xterm/headless Terminal instances in packages/core/src/services/shellExecutionService.ts — the replay terminal used to reserialize captured output and the live PTY terminal used during shell execution — and adds a regression test. A brief inline comment at each construction site records why the log level is disabled so it is not dropped in a later edit.

Why it's needed

When a shell command emits an incomplete or invalid ANSI escape sequence — for example an ESC byte followed by 0xB0, which is common with GBK/GB2312 (non‑UTF‑8) output — xterm.js's default LogService writes an xterm.js: Parsing error: {...} line to console.error. The parser recovers gracefully with no data loss and continues processing subsequent bytes, so the message is purely diagnostic. Because these two terminals run inside the shell execution service only to capture and serialize output, that benign diagnostic escapes to stderr and surfaces as alarming, repeated noise for anything that forwards child‑process stderr verbatim (the case reported in #7631). In a normal interactive TUI the same message is swallowed and never seen. These terminals never render diagnostics to a user, so turning their internal logging off is the appropriate scope for silencing the noise without changing any captured output.

Reviewer Test Plan

This is a small, non‑user‑visible change (it suppresses an internal stderr diagnostic), so the plan below is a focused unit test plus a direct before/after probe of the library behavior rather than a TUI screenshot.

How to verify

Run the focused unit tests:

cd packages/core && npx vitest run src/services/shellExecutionService.test.ts

All tests pass, including the new case suppresses parser diagnostics for malformed PTY output, which feeds an ESC + 0xB0 sequence followed by normal text through the shell execution path with a console.error spy, then asserts that no Parsing error line is emitted while the captured output still contains the trailing text (proving the parser still recovers).

The underlying behavior can also be confirmed directly against @xterm/headless: constructing a Terminal and writing "\xb0recovered" emits one console.error containing Parsing error by default and zero once logLevel: 'off' is set; in both cases the buffer still reads recovered.

Evidence (Before & After)

Not user‑visible (this suppresses an internal stderr diagnostic, not TUI output). Verified via the new unit test and a direct @xterm/headless probe:

  • Before ({ allowProposedApi: true }): writing \xb0recovered → 1 console.error call containing Parsing error; buffer line = recovered.
  • After ({ allowProposedApi: true, logLevel: 'off' }): same write → 0 console.error calls; buffer line = recovered.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Unit tests only (vitest); no runtime sandbox needed.

Risk & Scope

  • Main risk or tradeoff: internal xterm.js log output from these two headless terminals is now suppressed entirely. They are used only for output capture and serialization and never surface diagnostics to the user, so this removes noise without hiding anything a user would otherwise see.
  • Not validated / out of scope: does not change encoding/charset detection and does not touch AcpBridge stderr forwarding; it only sets the log level on the two headless terminals.
  • Breaking changes / migration notes: none.

Linked Issues

Closes #7631

中文说明

packages/core/src/services/shellExecutionService.ts 中的两个 @xterm/headless 无头 Terminal 实例上设置 logLevel: 'off'——一个是用于重新序列化已捕获输出的 replay 终端,另一个是 shell 执行期间使用的实时 PTY 终端——并新增一个回归测试。每个构造处都加了一行简短注释,说明为何关闭日志级别,以免日后被误删。

当 shell 命令输出不完整或无效的 ANSI 转义序列时(例如 ESC 字节后跟 0xB0,这在 GBK/GB2312 等非 UTF‑8 输出中很常见),xterm.js 默认的 LogService 会向 console.error 写入一行 xterm.js: Parsing error: {...}。解析器会自动恢复且不丢失数据,因此该信息纯属诊断性质。由于这两个终端仅用于在 shell 执行服务内部捕获和序列化输出,该诊断信息会外泄到 stderr,对任何原样转发子进程 stderr 的场景(即 #7631 报告的情况)表现为反复出现、令人担忧的噪声;而在正常的交互式 TUI 中同样的信息会被吞掉、从不显示。这些终端从不向用户呈现诊断信息,因此关闭其内部日志是在不改变任何已捕获输出的前提下消除该噪声的恰当范围。

验证:cd packages/core && npx vitest run src/services/shellExecutionService.test.ts,全部测试通过,包括新增用例 suppresses parser diagnostics for malformed PTY output。修复前:写入 \xb0recovered 会产生一次包含 Parsing errorconsole.error;修复后:不再产生该调用,且缓冲区仍为 recovered。仅在 macOS 上本地验证(单元测试)。

AI was used for assistance.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments above for the result.

Qwen Triage 已完成 —— 查看运行。结果见上方各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug with evidence — #7631 reports repeated xterm.js: Parsing error noise on stderr when shell output contains incomplete ANSI sequences (code 176 / 0xB0, consistent with GBK/GB2312 output). The issue is labeled type/bug and welcome-pr. This is a real user-facing problem, not theoretical hardening.

Direction: aligned. The headless terminals in shellExecutionService.ts exist only to capture and serialize output — they never render diagnostics to a user. Silencing their internal log is the right scope. No auth/sandbox/model/telemetry/public-contract surface is touched.

Size: 2 production logic lines (two logLevel: 'off' additions) + 2 comment lines + 26 test lines. Core path (packages/core/src/services/), but trivially small — no maintainer escalation needed.

Approach: the scope feels right. Two Terminal construction sites, one option added to each, one regression test. No unrelated changes, no drive-by refactors. I checked — these are the only two production new Terminal() calls in the codebase, so coverage is complete.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,有证据——#7631 报告了 shell 输出包含不完整 ANSI 转义序列(code 176 / 0xB0,与 GBK/GB2312 输出一致)时 stderr 上反复出现 xterm.js: Parsing error 噪声。该 issue 已标记 type/bugwelcome-pr。这是真实的用户可见问题,不是理论性加固。

方向:对齐。shellExecutionService.ts 中的无头终端仅用于捕获和序列化输出——从不向用户呈现诊断信息。关闭其内部日志是正确的范围。未触及 auth/sandbox/model/telemetry/公共契约。

规模:2 行生产逻辑(两处 logLevel: 'off')+ 2 行注释 + 26 行测试。触及核心路径(packages/core/src/services/),但极其微小——无需维护者关注。

方案:范围合理。两个 Terminal 构造点各加一个选项,一个回归测试。无无关改动,无顺手重构。已确认这是代码库中仅有的两个生产 new Terminal() 调用,覆盖完整。

进入代码审查 🔍

Qwen Code · qwen3.8-max-preview

Reviewed at 10d50eb68f99856637b07034c0ffcd01a2f2449b · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: given "xterm.js parser diagnostics leak to stderr from headless terminals," I would find every production new Terminal() using @xterm/headless, add logLevel: 'off' (these terminals only capture/serialize output and never display diagnostics), and add a regression test feeding malformed ANSI to confirm the noise is gone while output capture still works.

Comparison with the diff: the PR does exactly this. Two Terminal construction sites in shellExecutionService.ts (the replay terminal at ~L395 and the live PTY terminal at ~L1488), one logLevel: 'off' option added to each with a brief why-comment, and one regression test. These are the only two production new Terminal() calls in the codebase, so coverage is complete.

No correctness bugs, security issues, or regressions found. Specifics:

  • logLevel: 'off' is a valid ITerminalOptions property in @xterm/headless 5.5.0 — the parser still recovers from malformed sequences, only the diagnostic console.error is suppressed.
  • The test is well-structured: spies on console.error, feeds \u001b\xb0 (ESC + 0xB0, the exact byte from [AcpBridge] xterm.js: Parsing error #7631) followed by 'recovered', then asserts both that no Parsing error was emitted and that the output still contains 'recovered' — proving the parser recovers and data is not lost. The try/finally spy restore is good hygiene.
  • The inline comments ("This headless terminal only captures output, so suppress parser diagnostics") explain a non-obvious constraint — appropriate per project conventions.
  • No unrelated changes, no scope creep.

CI Test Evidence

Check Status Conclusion
precheck-pr / precheck completed ✅ success
Classify PR completed ✅ success
label completed ✅ success
Test (ubuntu-latest, Node 22.x) in_progress
Test (windows-latest, Node 22.x) completed skipped
Test (macos-latest, Node 22.x) completed skipped
Integration Tests (CLI, No Sandbox) completed skipped

CI still running at review time: Test (ubuntu-latest, Node 22.x) was still in_progress after ~10 minutes of polling. No failures observed in any completed check. Windows/macOS tests and integration tests are skipped (typical for fork PRs pending approval).

Not verified: live TUI behavior — this change suppresses an internal stderr diagnostic, not user-visible TUI output, so tmux real-scenario testing is not applicable. A maintainer can trigger the isolated @qwen-code /tmux job if desired.

中文说明

代码审查

独立方案: 针对"xterm.js 解析器诊断信息从无头终端泄漏到 stderr"的问题,我会找到所有使用 @xterm/headless 的生产 new Terminal() 调用,添加 logLevel: 'off'(这些终端仅用于捕获/序列化输出,从不显示诊断信息),并添加一个回归测试,输入畸形 ANSI 序列以确认噪声消失且输出捕获仍正常工作。

与 diff 的对比: PR 正是这样做的。shellExecutionService.ts 中两个 Terminal 构造点(~L395 的 replay 终端和 ~L1488 的实时 PTY 终端),各添加一个 logLevel: 'off' 选项和简短的 why 注释,加一个回归测试。这是代码库中仅有的两个生产 new Terminal() 调用,覆盖完整。

未发现正确性 bug、安全问题或回归。logLevel: 'off'@xterm/headless 5.5.0 的有效 ITerminalOptions 属性——解析器仍会从畸形序列中恢复,只是抑制了诊断性的 console.error。测试结构良好:监听 console.error,输入 \u001b\xb0(ESC + 0xB0,即 #7631 中的确切字节)后跟 'recovered',然后断言未发出 Parsing error 且输出仍包含 'recovered'——证明解析器恢复且数据未丢失。

CI 测试证据

审查时 CI 仍在运行:Test (ubuntu-latest, Node 22.x) 在约 10 分钟的轮询后仍为 in_progress。所有已完成的检查中未观察到失败。Windows/macOS 测试和集成测试已跳过(fork PR 待批准的典型情况)。

未验证:实时 TUI 行为——此更改抑制的是内部 stderr 诊断,而非用户可见的 TUI 输出,因此 tmux 真实场景测试不适用。

Qwen Code · qwen3.8-max-preview

Reviewed at 10d50eb68f99856637b07034c0ffcd01a2f2449b · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation.

This is exactly the kind of PR I like to see: a real user-reported bug (#7631, with concrete error output showing the 0xB0 parsing error), a two-line fix that addresses it at the right level, and a solid regression test that proves both the noise is gone and the parser still recovers.

My independent proposal was identical to what the PR does — add logLevel: 'off' to the two headless Terminal constructions and test with the exact malformed byte from the issue. The PR matches this exactly, with no extra scope, no drive-by changes, and appropriate why-comments at each site.

The only open item is that Test (ubuntu-latest, Node 22.x) was still running at review time. The precheck (lint, typecheck, build) passed, and the change is mechanically trivial — one option added to two existing object literals. No concerns about the test outcome.

If I had to maintain this in six months, I'd thank the author: the comments explain why the log level is off, the test documents the exact failure mode, and the diff is small enough to revert in seconds if ever needed.

中文说明

置信度:5/5 —— 每个阶段都很干净;毫不犹豫地合并。

这正是我希望看到的 PR:一个真实的用户报告 bug(#7631,有具体的错误输出显示 0xB0 解析错误),一个在正确层面解决问题的两行修复,以及一个可靠的回归测试,证明噪声消失且解析器仍能恢复。

我的独立方案与 PR 完全一致——在两个无头 Terminal 构造处添加 logLevel: 'off',并用 issue 中的确切畸形字节进行测试。PR 完全匹配,没有额外范围、没有顺手改动,每个构造点都有恰当的 why 注释。

唯一的开放项是审查时 Test (ubuntu-latest, Node 22.x) 仍在运行。预检(lint、typecheck、build)已通过,且更改在机制上极为简单——向两个现有对象字面量各添加一个选项。对测试结果没有顾虑。

Qwen Code · qwen3.8-max-preview

Reviewed at 10d50eb68f99856637b07034c0ffcd01a2f2449b · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Maintainer verification — built and tested locally against real runtimes

I verified this PR locally with no mocks: real @lydell/node-pty, real @xterm/headless 5.5.0, and the real AcpBridge class, in two isolated worktrees (base = 4c2b481 merge-base, PR head = 10d50eb), each with its own node_modules and its own build. Verdict: the fix is correct, minimally scoped, and complete. Recommend merge.


1. The reported failure reproduces end-to-end, and the PR eliminates it

AcpBridge.start() spawns the agent child and forwards its stderr verbatim (AcpBridge.ts:119-122, process.stderr.write("[AcpBridge] " + sanitizeLogText(msg))). I pointed a real AcpBridge at a child that runs one real shell command through ShellExecutionService — nothing else stubbed — and let both sides run identically.

real AcpBridge E2E

  • base: 1717 bytes of stderr, 2 [AcpBridge] xterm.js: Parsing error diagnostics — same fields and same escaped-newline rendering as [AcpBridge] xterm.js: Parsing error #7631 (code: 176, currentState: 1; position differs only because my repro puts the ESC at a different offset).
  • PR head: 0 diagnostics, 93 bytes of stderr.
  • Captured output identical on both sides: "recovered", rawOutput byte-identical (1bc2b07265636f76657265640d0a).

Both byte patterns from the issue reproduce: ESC + UTF-8 ° (0xC2 0xB0code: 176, the exact code in the report) and raw GBK ESC 0xB0 0xA1 (→ code: 65533 after replacement-char decoding). Both are silenced; both still capture recovered, confirming the parser recovers with no data loss.

2. Controlled experiments: both edits and the test are load-bearing

controlled experiments

Per-site (E2E): patching only the replay terminal (L392) leaves 1 diagnostic; patching only the live PTY terminal (L1486) leaves 1; both → 0. Neither line is redundant — both construction sites fire on a single ordinary command.

Per-site (unit test): running the PR's it("suppresses parser diagnostics for malformed PTY output") against base source patched one site at a time, it is red in all three partial states and green only with both lines present. The regression test genuinely guards both sites, so a future refactor dropping either one turns CI red. Full-file A/B: PR source 133 passed; base source + PR tests 1 failed | 132 passed — exactly 1/133 discriminates (AssertionError: expected true to be false, shellExecutionService.test.ts:466).

Amplification: 606 bytes of shell output containing 200 malformed sequences produced 307,484 bytes of stderr on base (400 diagnostics, ~507× amplification) versus 0 on the PR, with rawOutput byte-identical on both sides. This is not a cosmetic-only issue for anything that persists or ships child stderr.

3. Scope: nothing else is hidden, nothing else needs patching

Scanning the shipped @xterm/headless 5.5.0 bundle for LogService call sites: exactly one error-level call ("Parsing error: " — the target of this PR), one warn-level call ("writeSync is unreliable", unreachable here — the service only calls terminal.write(); the sole writeSync hits in packages/core are fs.writeSync), and 14 debug-level calls that were already suppressed at xterm's default logLevel: "info". So 'off' suppresses precisely the one diagnostic and nothing a user could otherwise see. 'off' is a valid member of the exported LogLevel union.

These are also the only two new Terminal() sites in packages/.

4. Static checks and suites

Check Result
tsc --noEmit (packages/core)
eslint (both changed files)
prettier --check
shellExecutionService.test.ts ✅ 133/133
full packages/core suite ✅ 17,572 passed / 6 failed
CI Test (ubuntu-latest, Node 22.x) ✅ (32m34s)

The 6 local failures are not attributable to this PR — the identical 6 fail on the base worktree (web-fetch and shell-ast-parser-lazy fail on Could not resolve "turndown", a gap in my cloned node_modules; plus the known memoryLifecycle integration artifact). CI is green.

Notes (non-blocking)

  • Windows. [AcpBridge] xterm.js: Parsing error #7631 was reported on win32 10.0.26200; I verified on macOS only. The mechanism is platform-independent — logLevel is a pure-JS xterm option evaluated before any platform code — so I see no reason for divergence, but flagging that the reporter's exact platform is unverified.
  • integration-tests/. Two further non-test new Terminal() constructions exist outside packages/ (integration-tests/terminal-capture/terminal-capture.ts:302, integration-tests/interactive/interactive-session.ts:96). Neither ships (files: ["dist"]) and diagnostics are arguably useful in a test harness — correctly left alone, noting it only so the "only two sites" claim is precisely scoped.
中文说明

维护者验证 —— 本地构建并针对真实运行时测试

我在本地以完全无 mock 的方式验证了本 PR:真实的 @lydell/node-pty、真实的 @xterm/headless 5.5.0、以及真实的 AcpBridge 类,在两个相互隔离的 worktree 中进行(base = 4c2b481 合并基点,PR head = 10d50eb),各自拥有独立的 node_modules 和独立构建产物。结论:修复正确、范围最小且完整,建议合并。

1. 端到端复现了报告的故障,本 PR 将其消除

AcpBridge.start() 会 spawn agent 子进程并原样转发其 stderr(AcpBridge.ts:119-122process.stderr.write("[AcpBridge] " + sanitizeLogText(msg)))。我让真实的 AcpBridge 指向一个仅通过 ShellExecutionService 执行一条真实 shell 命令的子进程——其余一律不打桩——两侧以完全相同的方式运行。

  • base:1717 字节 stderr,2[AcpBridge] xterm.js: Parsing error 诊断信息——字段与转义换行的呈现方式与 [AcpBridge] xterm.js: Parsing error #7631 一致(code: 176, currentState: 1position 不同仅因为我的复现把 ESC 放在了不同偏移量上)。
  • PR head0 条诊断,93 字节 stderr。
  • 两侧捕获的输出完全一致:"recovered"rawOutput 逐字节相同(1bc2b07265636f76657265640d0a)。

issue 中的两种字节模式都能复现:ESC + UTF-8 °0xC2 0xB0code: 176,与报告中的 code 完全一致)以及原始 GBK 字节 ESC 0xB0 0xA1(经替换字符解码后 → code: 65533)。两者都被消除,且都仍然捕获到 recovered,证明解析器可自行恢复、不丢数据。

2. 对照实验:两处改动与新增测试都是「承重」的

按站点(E2E): 只修 replay 终端(L392)仍剩 1 条诊断;只修实时 PTY 终端(L1486)仍剩 1 条;两处都修 → 0 条。两行都不冗余——一条普通命令就会同时触发两个构造点。

按站点(单元测试): 把 PR 的 it("suppresses parser diagnostics for malformed PTY output") 跑在「base 源码 + 逐个打补丁」的组合上,结果是三种部分状态全红,只有两行都在时才变绿。也就是说该回归测试确实同时守住了两个站点,未来若重构删掉任意一行,CI 都会变红。整文件 A/B:PR 源码 133 passed;base 源码 + PR 测试 1 failed | 132 passed——恰好 1/133 具备判别力(AssertionError: expected true to be falseshellExecutionService.test.ts:466)。

噪声放大: 606 字节、含 200 个畸形序列的 shell 输出,在 base 上产生 307,484 字节 stderr(400 条诊断,约 507 倍放大),在 PR 上为 0,且两侧 rawOutput 逐字节相同。对任何会持久化或传输子进程 stderr 的场景而言,这不只是观感问题。

3. 范围:没有隐藏其他信息,也没有遗漏其他站点

扫描随包发布的 @xterm/headless 5.5.0 bundle 中的 LogService 调用点:恰好 1error 级("Parsing error: ",即本 PR 的目标)、1warn 级("writeSync is unreliable",此处不可达——该服务只调用 terminal.write()packages/core 中所有 writeSync 命中均为 fs.writeSync)、以及 14 处 debug 级(在 xterm 默认 logLevel: "info" 下本就已被抑制)。因此 'off' 精确地只抑制了这一条诊断,不会隐藏任何用户本可看到的信息。'off' 是导出的 LogLevel 联合类型的合法成员。

这两处也是 packages/ 内仅有的 new Terminal() 站点。

4. 静态检查与测试套件

检查项 结果
tsc --noEmit(packages/core)
eslint(两个改动文件)
prettier --check
shellExecutionService.test.ts ✅ 133/133
packages/core 全量套件 ✅ 17,572 通过 / 6 失败
CI Test (ubuntu-latest, Node 22.x) ✅(32m34s)

这 6 个本地失败与本 PR 无关——在 base worktree 上同样是这 6 个失败(web-fetchshell-ast-parser-lazyCould not resolve "turndown" 失败,是我克隆的 node_modules 缺失依赖所致;另有已知的 memoryLifecycle 集成测试环境产物)。CI 为绿。

说明(不阻塞合并)

  • Windows。 [AcpBridge] xterm.js: Parsing error #7631 报告于 win32 10.0.26200,而我仅在 macOS 上验证。该机制与平台无关——logLevel 是纯 JS 的 xterm 选项,在任何平台相关代码之前生效——因此我认为不会有差异,但仍如实说明报告者的具体平台未经验证。
  • integration-tests/ packages/ 之外还有两处非测试的 new Terminal() 构造(integration-tests/terminal-capture/terminal-capture.ts:302integration-tests/interactive/interactive-session.ts:96)。二者都不随包发布(files: ["dist"]),且在测试工具中保留诊断信息反而有用——保持不动是正确的,此处提及只是为了让「仅两处站点」的说法范围更精确。

@wenshao
wenshao added this pull request to the merge queue Jul 25, 2026
Merged via the queue into QwenLM:main with commit a4d8845 Jul 25, 2026
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AcpBridge] xterm.js: Parsing error

3 participants