Skip to content

refactor(terminal): decouple terminal input from JSON RPC path#4696

Merged
coffeedeveloper merged 1 commit intomainfrom
fix/optimize_terminal
Jan 28, 2026
Merged

refactor(terminal): decouple terminal input from JSON RPC path#4696
coffeedeveloper merged 1 commit intomainfrom
fix/optimize_terminal

Conversation

@coffeedeveloper
Copy link
Member

@coffeedeveloper coffeedeveloper commented Jan 27, 2026

Types

  • 🎉 New Features
  • 🐛 Bug Fixes
  • 📚 Documentation Changes
  • 💄 Code Style Changes
  • 💄 Style Changes
  • 🪚 Refactors
  • 🚀 Performance Improvements
  • 🏗️ Build System
  • ⏱ Tests
  • 🧹 Chores
  • Other Changes

Background

Terminal input was flowing through the same JSON RPC path as resize/other messages, causing per‑keystroke JSON.stringify/parse overhead and coupling input with output handling under high output load.

Solution

Introduce a dedicated raw input RPC method for terminal input. The browser now sends input via input(id, data) and resize via its own method, avoiding JSON serialization on every keystroke. The node side handles raw input directly and keeps a backward‑compatible fallback for legacy JSON messages.

Changelog

  • Added input(id, data) to ITerminalServiceClient for raw terminal input.
  • Browser terminal service now uses raw input RPC and direct resize calls.
  • Node terminal service client handles raw input and keeps legacy JSON handling in onMessage.
  • Updated related tests and mocks to support the new input method.

Summary by CodeRabbit

发布说明

  • 重构

    • 简化了终端输入和大小调整的内部实现机制,采用更直接的远程调用方式,提升代码效率。
  • 测试

    • 更新所有终端相关测试中的mock实现,确保与新的操作接口兼容。

✏️ Tip: You can customize this high-level summary in your review settings.

@opensumi opensumi bot added the ⚙️ refactor Refactor code label Jan 27, 2026
@coffeedeveloper
Copy link
Member Author

/next

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

总体说明

该变更为终端服务客户端接口添加了 input() 方法,并更新了多个测试和实现文件以支持该新方法。同时,浏览器终端服务的内部消息传递机制从使用私有包装器改为直接RPC调用。

变更详情

文件组 / 文件 变更摘要
测试stub方法添加
packages/extension/__tests__/hosted/api/vscode/ext.host.terminal.test.ts, packages/terminal-next/__tests__/browser/inject.ts, packages/terminal-next/__tests__/browser/terminal.service.test.ts, packages/terminal-next/__tests__/node/terminal.reconnect.test.ts
ITerminalServicePathMockTerminalClient 的mock实现添加了 input() 和/或 resize() 方法作为无操作stub
接口定义更新
packages/terminal-next/src/common/pty.ts
ITerminalServiceClient 接口中添加了新的 input(id: string, data: string): void 方法
客户端实现改动
packages/terminal-next/src/node/terminal.service.client.ts
新增 input() 方法实现,并增强了 onMessage 处理逻辑以支持JSON解析失败的降级路径
测试用例更新
packages/terminal-next/__tests__/node/terminal.service.client.test.ts
将测试中的 onMessage() 调用替换为新的 input() 方法调用
浏览器服务重构
packages/terminal-next/src/browser/terminal.service.ts
移除了私有计数器和消息包装器函数,改为直接使用RPC调用 serviceClientRPC.input()serviceClientRPC.resize()

预估代码审查工作量

🎯 3 (Moderate) | ⏱️ ~20 minutes

建议审阅者

  • Ricbet
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately describes the main refactoring objective: decoupling terminal input from the JSON RPC path to improve performance by avoiding JSON serialization overhead.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/optimize_terminal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/terminal-next/src/node/terminal.service.client.ts`:
- Around line 157-169: The onMessage handler currently parses msg and may pass
undefined into resize or terminalService.onMessage; update onMessage to validate
parsed payload fields before calling downstream: after JSON.parse(msg) check
that method is a string, that for method === 'resize' params is an object with
numeric rows and cols (or coerce/default to safe values) before calling
this.resize(id, rows, cols), and for other methods ensure data is not undefined
(fallback to original msg or a safe default) before calling
this.terminalService.onMessage(id, data); also keep the catch fallback for
legacy/raw messages. This change affects the onMessage method and calls to
this.resize and this.terminalService.onMessage.

@opensumi
Copy link
Contributor

opensumi bot commented Jan 27, 2026

🎉 PR Next publish successful!

3.9.1-next-1769515889.0

@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

❌ Patch coverage is 18.18182% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.88%. Comparing base (e176173) to head (5d55eb7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../terminal-next/src/node/terminal.service.client.ts 22.22% 6 Missing and 1 partial ⚠️
...ages/terminal-next/src/browser/terminal.service.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4696      +/-   ##
==========================================
- Coverage   52.88%   52.88%   -0.01%     
==========================================
  Files        1686     1686              
  Lines      104888   104888              
  Branches    22954    22787     -167     
==========================================
- Hits        55475    55471       -4     
- Misses      41054    41058       +4     
  Partials     8359     8359              
Flag Coverage Δ
jsdom 48.17% <0.00%> (-0.01%) ⬇️
node 12.25% <18.18%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@Ricbet Ricbet left a comment

Choose a reason for hiding this comment

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

LGTM

@coffeedeveloper coffeedeveloper merged commit b0d6ed3 into main Jan 28, 2026
15 checks passed
@coffeedeveloper coffeedeveloper deleted the fix/optimize_terminal branch January 28, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚙️ refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants