feat(hooks): support terminal sequence notifications#4895
Conversation
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual, macOS tested noted. On direction: this is squarely on the hooks/events roadmap. Terminal notification hooks need a safe way to request native terminal notifications without opening an escape-sequence injection vector — that's a real problem for hook authors who want parity with terminal-native behavior. The allowlist approach (BEL + OSC 0/1/2/9/99/777, reject everything else) is the right security model for this. No direct Claude Code CHANGELOG reference, but the area is clearly relevant to the hooks architecture. On approach: the scope feels right. The layering is clean — types → validation → aggregation → emission → ACP forwarding — and each layer has a single responsibility. The parser rejects invalid input entirely rather than attempting partial sanitization, which is the correct choice for escape-sequence handling. The 769 additions across 17 files is reasonable for a feature that touches hook output types, aggregation, validation, TUI emission, ACP session forwarding, and HTTP bridge translation. One thing worth noting: the Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必填章节齐全,双语,已标注 macOS 测试。 方向:这个改动完全在 hooks/events roadmap 范围内。Terminal notification hooks 需要一种安全方式来请求终端原生通知,同时不能打开 escape sequence 注入的口子——这对 hook 作者来说是个真实问题。白名单方案(BEL + OSC 0/1/2/9/99/777,其余全部拒绝)是正确的安全模型。Claude Code CHANGELOG 中没有直接参考,但该领域与 hooks 架构明确相关。 方案:范围合理。分层清晰——types → validation → aggregation → emission → ACP forwarding——每一层职责单一。Parser 对非法输入整体拒绝,而不是尝试部分清洗,这对 escape sequence 处理来说是正确的选择。17 个文件 769 行新增,对于一个涉及 hook output 类型、聚合、校验、TUI 发射、ACP session 转发和 HTTP bridge 转换的功能来说是合理的。一个小问题:aggregator 中的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewIndependent proposal: to add terminal notification sequences to hooks, I'd: (1) add No critical blockers found. The implementation is clean and security-conscious:
Minor observations (not blockers):
Integration TestingUnit Tests (local)All 435 tests pass across the changed files:
End-to-End Parse + Emit PipelineRan a custom integration script against the built CIAll checks green: CodeQL, Lint, Test (macOS/Linux/Windows). Note on TUI testingThis feature is hook-driven — the terminal sequence is emitted only when a hook returns 中文说明代码审查独立方案:要给 hooks 添加终端通知序列,我会:(1) 在 未发现 critical blocker。实现干净且安全意识强:
小问题(非 blocker):
集成测试所有 435 个单测通过。端到端 parse+emit 管道 17 项集成测试全绿。CI 全平台通过。 该功能是 hook 驱动的,没有直接的 CLI 命令可以触发。单测和集成测试覆盖了从 hook output 创建到聚合、校验、发射和 ACP 转发的完整链路。 — Qwen Code · qwen3.7-max |
|
Stepping back: this PR does exactly what I would have done. The layering is clean (types → validation → aggregation → emission → ACP forwarding), the security model is the right one for escape-sequence handling (allowlist, reject-invalid-entirely, no partial sanitization), and the test coverage is thorough — 435 unit tests plus 17 end-to-end integration assertions, all green. CI is clean on all three platforms. The scope is proportional to the feature: it touches hook output types, aggregation, a new validation/parser module, TUI emission, ACP session forwarding, and HTTP bridge translation. That's six distinct integration points, each with targeted tests. Nothing feels over-engineered or speculative. The before/after screenshots in the PR body demonstrate the window-title-change use case working on macOS. The only minor concern is that testing was macOS-only (Linux and Windows marked as "not tested"), but the code itself is platform-agnostic — it writes escape sequences through the existing I'd maintain this happily. The parser is straightforward, the allowlist is explicit and easy to extend, and the ACP forwarding follows the same pattern as the existing Approving. ✅ 中文说明回顾整体:这个 PR 的做法和我自己的方案一致。分层清晰(types → validation → aggregation → emission → ACP forwarding),安全模型对 escape sequence 处理来说是正确的(白名单、整体拒绝、不做部分清洗),测试覆盖充分——435 个单测 + 17 个端到端集成断言,全绿。CI 三个平台全通过。 范围与功能匹配:涉及 hook output 类型、聚合、新的校验/parser 模块、TUI 发射、ACP session 转发和 HTTP bridge 翻译。六个不同的集成点,每个都有针对性测试。没有过度工程化或投机性的代码。 PR 中的 before/after 截图展示了 macOS 上的窗口标题修改用例。唯一的小顾虑是测试仅在 macOS 上进行(Linux 和 Windows 标注为"未测试"),但代码本身是平台无关的——通过现有的 这个代码我会乐意维护。Parser 直截了当,白名单明确且易于扩展,ACP 转发遵循与现有 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
This PR adds support for hook-provided terminal notification sequences. Hooks can now return a terminal sequence payload, Qwen Code preserves it through hook output aggregation, validates it against a strict allowlist, and emits it through the active terminal notification path. The same payload is forwarded through ACP session notifications and converted to session-scoped server-sent events for clients that consume ACP sessions.
Why it's needed
Terminal notification hooks need a safe way to request terminal-native notifications such as BEL, window title updates, iTerm2 notifications, Kitty notifications, and Ghostty notifications without allowing arbitrary escape-sequence injection. This gives hook authors parity with terminal notification behavior while keeping emission centralized, validated, and compatible with tmux and screen passthrough.
Reviewer Test Plan
How to verify
Confirm that a hook output containing an allowed terminal sequence is preserved by hook output creation and aggregation, emitted by the terminal notification writer, forwarded by ACP session notification hooks, and translated by the HTTP ACP bridge into a terminal sequence SSE frame. Confirm that invalid terminal sequences are rejected and do not write to the terminal. The local verification covered targeted unit tests for core hook output handling, terminal sequence parsing and emission, TUI notification hook consumption, ACP session forwarding, and HTTP ACP bridge forwarding, plus package type checking and root build/typecheck.
Evidence (Before & After)
Change Terminal Title:
hook script:
hook config:
Before:

After:

Tested on
Environment (optional)
Local macOS development environment with Node.js v22.22.1. Verification used unit tests, TypeScript type checking, and repository build/typecheck. No sandbox integration test was required for this hook output and notification forwarding change.
Risk & Scope
Linked Issues
#4882
中文说明
这个 PR 做了什么
这个 PR 增加了 hook 返回终端通知序列的能力。Hook 现在可以返回 terminal sequence payload,Qwen Code 会在 hook output 聚合过程中保留它,对它进行严格白名单校验,并通过当前终端通知链路发射。同一 payload 也会通过 ACP session notification 转发,并由 HTTP ACP bridge 转换成 session 级别的 SSE 事件,供 ACP 客户端消费。
为什么需要
Terminal notification hooks 需要一种安全方式来请求终端原生通知,例如 BEL、窗口标题更新、iTerm2 notification、Kitty notification 和 Ghostty notification,同时不能允许任意 escape sequence 注入。这个改动让 hook 作者可以使用与终端通知一致的能力,同时把发射逻辑集中在受控、可校验、兼容 tmux/screen passthrough 的路径中。
Reviewer Test Plan
如何验证
确认包含允许的 terminal sequence 的 hook output 会被 hook output creation 和 aggregation 保留,会被 terminal notification writer 发射,会被 ACP session notification hook 转发,并会被 HTTP ACP bridge 转换成 terminal sequence SSE frame。确认非法 terminal sequence 会被拒绝,并且不会写入终端。本地验证覆盖了 core hook output 处理、terminal sequence 解析和发射、TUI notification hook 消费、ACP session 转发、HTTP ACP bridge 转发的定向单测,以及 package type checking 和根级 build/typecheck。
证据(Before & After)
Before:
After:
Tested on
环境(可选)
本地 macOS 开发环境,Node.js v22.22.1。验证使用了单元测试、TypeScript 类型检查和仓库 build/typecheck。该改动属于 hook output 与 notification forwarding 链路,不需要 sandbox integration test。
风险与范围
关联 Issue
#4882