@qwen-code /triage

Maintainer verification — local build + before/after parser harness

Verified 848a9bc against origin/main (f5761ac) on macOS (Darwin arm64, Node v22.22.2). This is a parser-only change on a real input-validation boundary — hook-supplied terminalSequence flows useAttentionNotifications.ts:124 → emitTerminalSequence → parseAllowedTerminalSequences before bytes are written to the terminal — so I drove it two ways: the real unit suites, and a before/after harness running the actual parser from both origin/main and the PR head against an adversarial corpus.

1. Unit tests / typecheck / lint (PR code)

2. Before/after adversarial corpus (23 inputs through both versions)

End-to-end gate, headline case:

emitTerminalSequence("\x1b]9oops\x07")
  before: true   → wrapped + written to the terminal
  after:  false  → rejected, 0 bytes written

Result: 6/6 malformed cases fixed, 11/11 valid preserved, 0 regressions.

Notes for the merge decision

• Regression-safe by construction. The fix only adds input[position] !== ';', i.e. it rejects when the char after the numeric code isn't ;. Every valid allowlisted OSC sequence has ; there, so no valid input can flip — the corpus confirms this empirically (11/11).
• Slightly broader than the summary says, and correctly so. It tightens the boundary for all allowlisted codes, not just OSC 9 — e.g. ESC]0title␇ (no separator) is now rejected too. It also drops the stale OSC 0/1/2 can have just a title with ';' comment, which no longer matched behavior.
• Security relevance. Because the input is hook-controlled, accepting ESC]9<arbitrary>␇ previously let a hook smuggle a payload past the allowlist and have it wrapped + written raw. Tightening the code/payload boundary closes that.

Verdict

Correct, focused, and regression-free from my testing. ✅ No code concerns. The only outstanding item is the triage bot's PR-template formatting request (process, not code).

_{Verified by maintainer @wenshao: local unit suites + a before/after harness running the compiled parser from both origin/main and the PR head (848a9bc) against a 23-input adversarial corpus.}

emitTerminalSequence("\x1b]9oops\x07")
  before: true   → 被包装并写入终端
  after:  false  → 拒绝，写入 0 字节

@qwen-code /triage

Thanks for the PR!

Template looks good ✓

On direction: this is a clean correctness and security fix — the OSC parser was accepting malformed sequences where the numeric code wasn't followed by ;, letting hook-controlled input smuggle arbitrary payloads past the allowlist. Clearly within scope, clearly needed. No CHANGELOG reference needed for a parser bug fix.

On approach: minimal and focused — 2 files, +8/-3. The fix is exactly one condition change (input[position] !== ';'), plus a stale comment cleanup, plus 3 new test assertions. No scope creep, no drive-by refactors. This is how bug fixes should look.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

Code review

The fix is a single condition change in parseOscSequence: after reading the numeric code, the parser now requires input[position] === ';' before proceeding to read the payload. Previously it only checked position >= input.length, so any non-terminator byte after the code was silently treated as the start of the payload.

The stale comment OSC 0/1/2 can have just a title with ';' was correctly removed — it no longer matched behavior.

No correctness bugs, no security holes, no regressions. The fix is structurally safe: every valid allowlisted OSC sequence has ; after the code, so no valid input can be affected. Follows project conventions — no over-abstraction, no unrelated changes.

No blockers found.

Testing

Unit tests (PR code)

Before/after parser verification

=== BEFORE (main branch - bug present) ===
ESC]9oopsBEL (malformed): ACCEPTED    ← bug: should be rejected
ESC]9BEL (no separator): ACCEPTED     ← bug: should be rejected
ESC]9;hiBEL (valid): ACCEPTED
ESC]0;titleBEL (valid): ACCEPTED

=== AFTER (PR fix applied) ===
ESC]9oopsBEL (malformed): REJECTED    ← fixed ✅
ESC]9BEL (no separator): REJECTED     ← fixed ✅
ESC]9;hiBEL (valid): ACCEPTED         ← no regression
ESC]0;titleBEL (valid): ACCEPTED      ← no regression

Vitest (PR code)

 RUN  v3.2.4 packages/cli
      Coverage enabled with v8

 ✓ src/utils/terminalSequence.test.ts (26 tests) 10ms

 Test Files  1 passed (1)
      Tests  26 passed (26)

2 malformed cases fixed, all valid cases preserved, 0 regressions.

=== 修改前（main 分支 - bug 存在） ===
ESC]9oopsBEL（非法）：被接受    ← bug：应被拒绝
ESC]9BEL（无分隔符）：被接受     ← bug：应被拒绝
ESC]9;hiBEL（合法）：被接受
ESC]0;titleBEL（合法）：被接受

=== 修改后（PR 修复） ===
ESC]9oopsBEL（非法）：被拒绝    ← 已修复 ✅
ESC]9BEL（无分隔符）：被拒绝     ← 已修复 ✅
ESC]9;hiBEL（合法）：被接受     ← 无回归
ESC]0;titleBEL（合法）：被接受  ← 无回归

— Qwen Code · qwen3.7-max

This is a textbook bug fix — one condition, one file, one bug. The parser had a gap between its documented contract (ESC ] <code> ; <payload> <terminator>) and its actual behavior (accepting anything after the code as payload). The fix closes that gap with a single character check.

My independent proposal would have been identical: add input[position] !== ';' right after the code-reading loop. The PR matches this exactly, and also correctly removes the stale comment that no longer reflected behavior.

The before/after testing confirms: malformed sequences (ESC]9oopsBEL, ESC]9BEL) go from accepted to rejected, while all valid sequences remain accepted. 26/26 unit tests pass, 24/24 downstream consumer tests pass. Zero regressions by construction — every allowlisted OSC code is followed by ; in valid input.

The security angle is real too: since hooks control terminalSequence input, the old parser let a malicious hook smuggle arbitrary escape payloads past the allowlist. Tightening the boundary closes that path.

Approving. ✅

— Qwen Code · qwen3.7-max