fix(cli): optimize large paste performance and add progress indicator - #6506
Conversation
Pasting ~260K characters previously caused the UI to freeze because readline fired a keypress event for every single character. This change intercepts bracketed paste markers at the raw stdin data level, bypassing readline entirely. The paste content is accumulated as Buffer chunks and broadcast as a single event, reducing processing time from seconds to milliseconds. Additionally, a progress bar is shown in the Footer during large pastes. On paste-start, the clipboard size is probed asynchronously via platform clipboard commands (pbpaste/xclip/powershell) to enable an accurate percentage display.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Thanks for the PR, @LaZzyMan!
The paste performance optimization looks like a meaningful improvement — intercepting bracketed paste markers at the raw stdin level and the O(n²) → O(n) buffer fix are both sound approaches.
However, the PR body doesn't follow our pull request template. The current body uses ## Summary and ## Test plan headings, but the template requires:
## What this PR does— prose description of the change## Why it's needed— motivation and user-facing benefit## Reviewer Test Planwith subsections:### How to verify— reproduction steps### Evidence (Before & After)— screenshots, tmux logs, or recordings### Tested on— OS table (macOS/Windows/Linux)### Environment— runtime details
## Risk & Scope— tradeoffs, out-of-scope items, breaking changes## Linked Issues— related issue references<details>中文说明</details>— Chinese translation of the body
Could you update the PR body to follow the template? This helps reviewers evaluate the change efficiently and ensures cross-platform testing is documented.
中文说明
感谢 PR,@LaZzyMan!
粘贴性能优化看起来是有意义的改进——在原始 stdin 层拦截括号粘贴标记以及 O(n²) → O(n) 的缓冲区修复都是合理的方案。
但是 PR 正文没有遵循我们的 PR 模板。当前使用的是 ## Summary 和 ## Test plan 标题,但模板要求:
## What this PR does— 改动的描述## Why it's needed— 动机和用户收益## Reviewer Test Plan— 包含验证步骤、Before/After 证据、测试平台表格等## Risk & Scope— 权衡、超出范围的项目、破坏性变更## Linked Issues— 相关 issue 引用<details>中文说明</details>— 正文的中文翻译
请按照模板更新 PR 正文,方便 reviewer 高效评估。
— Qwen Code · qwen3.7-max
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. |
…oard size probe - Remove unused isPasting/setIsPasting from UIState, UIActions, and AppContainer (Footer reads directly from KeypressContext) - Use shell pipe (pbpaste | wc -c) instead of reading full clipboard content into Node.js memory for size probe - Add xsel and wl-paste fallbacks for Linux
…board fallback - Add cross-chunk tail buffer to handleStdinData so paste markers split across data events (common over SSH/tmux) are correctly reassembled - Set pasteAlreadyFlushed in forceFlushRawPaste and Ctrl+C escape so late paste-end markers don't produce ghost paste events - Fix Linux clipboard size probe: use command -v to check tool existence before piping, since wc -c always exits 0 and masked missing tools
Review round 1 response (6ca48ef)
|
partialMarkerTailLength now requires >= 2 byte match, so a lone ESC (0x1b) — which is the start of every ANSI escape sequence — is no longer held back as a potential paste marker prefix. This fixes ESC key handling in tests and real usage. Snapshot updates are from Footer's new useKeypressContext() hook changing the render tree.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
KeypressContext.tsx:1447 |
Shell &&/` |
|
KeypressContext.tsx:1353,1379,1395 |
buf.subarray() creates views into the parent buffer, not copies — the entire stdin data buffer is retained in memory as long as any subarray view exists in rawPasteChunks |
Use Buffer.from(buf.subarray(...)) to create independent copies |
KeypressContext.tsx:1447 |
exec() callback stale race between rapid pastes — old callback fires during new paste, passes the !rawPasteAccumulating guard, and sets totalBytes from previous clipboard probe |
Add a paste-cycle counter incremented on each paste-start; check it in the callback to discard stale results |
— qwen3.7-max via Qwen Code /review
The previous snapshot update was generated with terminal colors enabled locally, producing ANSI escape codes in snapshots. CI runs without colors, causing mismatch. Restore to the plain-text baseline from main.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
Footer.tsx:49,56 |
Missing i18n key for 'Pasting…' — t() falls back to key string, so non-English locales see untranslated text |
Add 'Pasting…' entry to locale files following existing i18n convention |
KeypressContext.tsx:1438 |
exec() from node:child_process not mocked in tests — real pbpaste/xclip subprocesses spawned on every paste event during testing |
Mock node:child_process in test file: vi.mock('node:child_process', ...) |
KeypressContext.tsx:1350 |
Dead push before rawPasteChunks = [] in Ctrl+C handler — data is pushed then immediately cleared two lines later |
Remove the dead rawPasteChunks.push(...) line, or flush via broadcastPasteFromRaw before clearing |
— qwen3.7-max via Qwen Code /review
…ting path In paste-accumulating mode, a lone ESC at a chunk boundary is the start of paste-end marker, not a user keypress. Use minLen=1 so the ESC byte is held back for reassembly with the next chunk.
🔬 Maintainer verification — real CLI, PTY-driven, base-vs-PR dual runI built both arms from source and drove the real CLI through a PTY ( The core claim holds, and it is a big win. Two things I could not confirm are below, and one of them I think is worth acting on before merge. ✅ The headline: confirmed, ~20× — though not 8 ms260 000-character paste, median of 3 runs each, end-to-end (bytes written → placeholder rendered):
Your ✅ Correctness — everything I could break, I tried
🟡 1 — The percentage progress bar never appears on a headless / SSH / container hostThe PR promises Byte counter only. No bar, no percentage, ever. The cause is that the numerator and the denominator come from different data sources. Two things happen there:
And it is not only a "tool not installed" problem: over SSH, in tmux, in a container, the terminal transmits the client's clipboard while Suggestion: either derive 🟡 2 — The 317-line fast path is effectively untested, and the existing 98 tests cannot see itThis PR adds 0 test files. The 98 passing tests are real, but they were written for the old keypress path. I measured what they actually guard, by mutation:
That second mutation is not academic. Same split-end-marker paste as the ✅ row above, against a bundle built with it:
Real content corruption plus a 15× latency regression, and the suite is completely green. And this is the subtlest mechanism in the PR — your own most recent commit is a fix to it ( A few unit tests driving 🟡 3 — 141 commits behind
|
…l call sites The normal paste-end path called broadcastPasteFromRaw without setting pasteAlreadyFlushed, so a stale paste-end marker leaking to the keypress handler could broadcast a duplicate paste. Move the flag into broadcastPasteFromRaw so every call site (normal completion, idle-timeout flush, cleanup) is covered; drop the now-redundant assignment in forceFlushRawPaste.
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
…7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
|
@qwen-code /retry |
|
🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it. 中文说明🔄 已重新武装 AutoFix。下一次扫描会从头重新读取本 PR 的反馈,轮次计数也已重置。未删除任何内容 —— 本标记使其上方的评估标记失效。 |
…7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
…ow (QwenLM#7565) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. QwenLM#7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of QwenLM#7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (QwenLM#7562) * fix(autofix): also treat a git fetch/clone transport death as infra QwenLM#6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on QwenLM#6506's real composite annotation and a bare-timeout-is-not-rerun guard. * test(autofix): single-source the infra-signature list from the workflow The infra-rerun test re-typed INFRA_FAILURE_SIGNATURES as an inline mirror of the workflow's env value. Two copies that must be hand-synced can drift — the test could keep passing against a stale list while production changed, or vice versa. That is exactly the copy the git-transport follow-up had to remember to update in two places. Extract the list from the workflow source instead, the same extract-from-source idiom the file already uses for NON_BLOCKING_CHECKS, so there is only one copy and drift is impossible. A toContain guard fails loudly if the env is renamed or the regex breaks, rather than letting an empty pattern match every line and silently pass. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (QwenLM#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Could not address the latest feedback automatically (round 1/100). A human should take over this PR. What I found before stopping: address-review blocked: pre-existing environmental
|
) * fix(cli): correct queued message display style and ordering Mid-turn steer messages (user input queued while the model is responding) had two display bugs: 1. They rendered with notification styling (● icon) instead of user-input styling (> prefix) because accept() added them to UI history as MessageType.NOTIFICATION. 2. They appeared below the model's reply because accept() was only called in the finally block after the entire response stream completed, appending the user message after all model response items. Fix: use MessageType.USER with sentToModel: true for steer messages, and settle the steer input on the first stream event (after the user-content push lands but before model-response events are committed to UI history). Pass steer inputs through to recursive sendMessageStream calls so all takeSteerInput paths benefit from early settlement. Add a WeakSet guard to settleSteerInput for idempotency across recursive invocations. * test(core): add ordering test for early steer settlement Verify that accept() is called after the first stream event is pulled but before subsequent events reach the consumer, pinning the settle-before-content timing that ensures queued user messages render above the model's reply. * fix(cli): use sentToModel: false for steer messages, address review - Use sentToModel: false instead of true: steer messages are injected into an existing tool-result turn, not standalone user turns. sentToModel: true would make isRealUserTurn() count them as real turns, inflating the rewind turn index. - Remove unnecessary as HistoryItemWithoutId cast. - Add post-cleanup assertion in ordering test to verify the WeakSet guard prevents double-settlement. * fix(cli): align resumed mid-turn steer display with live session (#7381) Resume path now renders mid_turn_user_message as MessageType.USER with sentToModel: false, matching the live-session styling. Add a comment documenting the intentional sentToModel: false choice. * fix(cli): exclude steer messages from user-turn filters (#7381) Steer messages (sentToModel: false) were counted as real user turns by five downstream consumers that filter on type === 'user' without checking sentToModel, breaking cancel auto-restore, telemetry turn count, prompt recall, away-recap thresholds, and resume collapse boundaries. Add sentToModel !== false guards at each site. * test(cli): add coverage for sentToModel !== false guards (#7381) * test(cli): add coverage for sentToModel !== false guard in input-history filter (#7381) * test(cli): add coverage for sentToModel !== false guard in YOLO turn-count telemetry (#7381) * fix(cli): restore corrupted docs and classify steer items as synthetic (#7381) * fix(docs): restore corrupted autogenerated input names in GitHub Action docs (#7381) * fix(cli): deduplicate findLastUserItemIndex and add steerInput forwarding test (#7381) * fix(cli): keep code-block copy numbering continuous across steer items (#7381) * test(core): add Hook continuation steerInput forwarding test Verify that steerInput is forwarded through the Stop-hook continuation path and settled early on the first content event of the continuation turn, matching the existing Steer continuation coverage. * fix(cli): sync selection test fixtures with ink FrameCell/ReadonlyFrame types (#7381) * fix(core): align cron day wildcard semantics (#7464) Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com> * feat(core): keep completed background agents resident (#7426) * feat(core): keep background agents resident * fix(core): harden background continuation boundaries * docs(core): move per-spawn cleanup comment to subagentDispose The comment describing the per-spawn cleanup (which stays undefined on the fork-resume path) had drifted above the launchModel declaration, where it no longer applied and could mislead readers. Relocate it to the subagentDispose assignment in the non-fork branch it actually documents. * fix(core): close finishing window and release resident on error in background GOAL path - Non-worktree GOAL completion drained the message queue but never called registry.beginFinishing(), unlike the worktree path. A send_message racing the terminal transition could be accepted (status still running, finishingAgents empty) and then orphaned by complete(). Call beginFinishing() after the empty drain to reject the racing message instead. - The completion catch block never reset keepResident, so a throw from patchAgentMeta/registry.complete left the runtime resident but finalized as failed — a zombie that cleanupRuntime never reclaimed. Reset keepResident in the catch so the finally block disposes it. --------- Co-authored-by: Claude <noreply@anthropic.com> * ci(autofix): continue environment-specific fixes (#7444) * ci(autofix): continue environment-specific fixes * docs(autofix): align verification wording * docs(autofix): require bundle before integration tests * docs(autofix): scope surrogate verification rules * docs(autofix): require focused tests before integration checks * docs(autofix): clarify review verification guidance * fix(acp-bridge): close prompt-terminal follow-ups from the PR #7400 self-review (#7453) * fix(acp-bridge): close prompt-terminal follow-ups from PR #7400 self-review Keep a removed RUNNING prompt visible to the teardown flush via a removed flag so its terminal still publishes when the session closes before the agent cooperates; gate broadcastTurnError's session turn-state mutation to running prompts; propagate the typed PromptDeadlineExceededError from the pre-dispatch abort check; document the deadline FIFO-release overlap trade-off, the trailing prompt_cancelled after flush, and the result.then/finally ordering invariant; route the dedup log to the debug channel; drop the prompt-deadline re-export that pulled the bridge into a leaf module. Fixes #7451 * test(acp-bridge): cover promote-then-remove-then-settle duplicate completed guard (#7453) --------- Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> * fix(core): strip Qwen-internal daemon secrets from agent-spawned child env (#7256) * fix(core): strip Qwen-internal daemon secrets from agent-spawned child env Shell subprocesses (and the monitor tool and stdio MCP servers) inherited the full daemon process.env, including QWEN_SERVER_TOKEN (the serve-daemon bearer credential), so an agent-run command like printenv QWEN_SERVER_TOKEN could read an internal secret. Add a shared sanitizeChildEnv() that removes Qwen-internal daemon/server tokens (QWEN_SERVER_TOKEN, QWEN_DAEMON_TOKEN) before spawning, and apply it at the shell child_process + PTY paths, monitor.ts, and the mcp-client stdio transport. The denylist is deliberately narrow: it does NOT strip third-party credentials (GH_TOKEN, AWS_*, NPM_TOKEN, ...) that real shell workflows legitimately inherit -- only Qwen-internal secrets. Exported from the package root so the desktop denylists can consolidate onto it later. Fixes #6601. * test(core): cover daemon-secret stripping on monitor and mcp-client spawn sites * test(core): replace process.env instead of mutating in shell sanitization tests The file restores process.env by reference in afterEach, so in-place key mutations leaked into later tests. Use the replacement pattern already used by setupConflictingPathEnv. * docs(core): align JSDoc @param names with actual function signatures (#7492) Fix 6 instances where JSDoc @param tags had drifted from their corresponding function signatures — parameters were renamed, removed, or undocumented over time but the doc blocks were not updated. Closes #7446 * feat(serve): support forced MCP reconnects (#7488) * feat(serve): support forced MCP reconnects * test(serve): cover forced MCP reconnect options --------- Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com> * fix(cli): insert newline on Shift+Enter and stop streaming thinking-block flicker (#7397) * fix(cli): re-push Kitty keyboard flags onto the alternate screen in VP mode In VP mode the app renders on the alternate screen (`alternateScreen: true`), but the Kitty keyboard progressive-enhancement flags were pushed only once at startup on the main screen. The Kitty spec tracks these flags per screen buffer, so the alternate screen's stack stays empty and the terminal never reports modifiers: Shift+Enter arrives as a bare Enter (submit) or, when the terminal emits an ESC-prefixed variant, as an orphaned Escape that trips the empty-buffer double-Esc rewind prompt — so Shift+Enter can never insert a newline in VP mode even on Kitty-capable terminals (e.g. cmux). Re-push the flags onto the alternate screen right after Ink enters it (Ink writes the enter-alt-screen sequence synchronously inside render(), so the push is correctly ordered). Ink discards the alternate screen and its flag stack on unmount, leaving the startup main-screen push balanced by the existing disableKittyProtocol() on cleanup. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): stabilize streaming thinking block height to stop flicker The pending "Thinking…" block renders the tail of the reasoning stream in a content-sized box. As the model emits paragraph separators, a blank line enters and leaves the tail window (and `trimEnd` drops trailing blanks), so the visible line count oscillates and the block flickers 2→3→5 rows during streaming. Track the tallest height the block has reached for the current thought and never render fewer rows than that (capped at the streaming window size), padding at the top so the newest line stays pinned to the bottom. The tracker resets when streaming ends or when the buffer shrinks (a new thought replaced it), so height is monotonic within a thought without leaking across thoughts. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): decode xterm modifyOtherKeys Shift/Ctrl/Alt+Enter so it inserts a newline Terminals such as Ghostty report Shift+Enter as the xterm modifyOtherKeys sequence `ESC [ 27 ; <mods> ; <key> ~` (e.g. `ESC [ 27 ; 2 ; 13 ~`) when the Kitty keyboard protocol is not negotiated — which is the default, since Kitty detection does not always succeed. Two bugs kept this from inserting a newline: 1. The CSI-u parser read the leading `27` marker as the key code (matching the Escape key code 27) instead of the real key code in the third parameter, so with Kitty enabled Shift+Enter was mistaken for Escape and tripped the double-Esc rewind prompt. 2. The reassembly path that stitches readline's shredded CSI fragments back together was gated behind `kittyProtocolEnabled`, so with Kitty disabled the `ESC [ 27 ; 2 ;` head plus the stray `13~` tail leaked into the composer as literal text and no newline was inserted. Decode the third parameter as the real key code for the `27;…~` form, and route those sequences through the reassembly buffer even when Kitty is off (only the `ESC [ 27` marker opts in, so keys readline already parses cleanly are untouched). Shift/Ctrl/Alt+Enter now insert a newline in both VP and non-VP mode regardless of Kitty negotiation. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): anchor VP viewport to the top until a conversation turn exists On a fresh VP-mode session the virtualized list holds the banner plus startup notices (tips / MOTD / info), so it is longer than one item. Keying the initial scroll anchor off list length alone selected scroll-to-end, which pinned the banner to the bottom of the full-height viewport and left the top half of the screen blank. Anchor to the top until there is an actual conversation turn (a user/user_shell history item or a pending response), then resume scroll-to-end so the latest output stays in view. Startup notices no longer count as content that forces bottom alignment. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): stabilize streaming thinking window against availableTerminalHeight drift The grow-only streaming thinking window still flickered because its line cap was derived from availableTerminalHeight. While a thought streams the terminal keeps constrainHeight on, so availableTerminalHeight (and the derived maxLines) drifts up and down as sibling pending content grows, and the grow-only clamp `min(maxLines, …)` shrank the block whenever it dipped. Use a constant window height (MAX_STREAMING_THINKING_VISUAL_LINES) for the pending window instead. The window is only a few lines, so a fixed cap cannot meaningfully overflow (VP scrolls anyway), and the height stays stable while still growing monotonically within a thought. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * Revert "fix(cli): anchor VP viewport to the top until a conversation turn exists" This reverts commit fbe86a9e159b75ea1f5b689cc327599c9dc91090. * fix(cli): guard modifyOtherKeys detection against keypresses without a sequence The modifyOtherKeys prefix check ran on every keypress, but some synthetic keypresses (and the useKeypress test harness) emit a key with no `sequence`, so `key.sequence.startsWith(...)` threw an unhandled rejection. Use optional chaining so a missing sequence is simply not a modifyOtherKeys start. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): mock pushKittyProtocolFlags in gemini.test.tsx kitty mock The kittyProtocolDetector mock omitted the newly added pushKittyProtocolFlags export. Add it so the mock stays in sync with the real module and a VP-mode startup path exercised through this suite cannot hit an undefined call. Generated with AI Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): open singleton subagent details (#7495) Co-authored-by: ytahdn <ytahdn@gmail.com> * fix(web-shell): avoid redundant git status requests (#7496) Co-authored-by: ytahdn <ytahdn@gmail.com> * fix(agent): ignore empty working_dir placeholders (#7343) * fix(agent): ignore empty working_dir placeholders * test(agent): align empty working_dir expectations * feat(prompts): allow overriding core identity via QWEN_SYSTEM_IDENTITY_MD (#7478) * feat(prompts): update prompts.ts for QWEN_SYSTEM_IDENTITY_MD * feat(prompts): update prompts.test.ts for QWEN_SYSTEM_IDENTITY_MD * fix(prompts): address CR on QWEN_SYSTEM_IDENTITY_MD Keep getDefaultCoreIdentitySentence private, fail loud on path resolution errors, use trimEnd, and resolve identity only on the default-prompt branch. * test(prompts): align identity override tests with CR feedback Sample default identity from live prompt, cover trimEnd trailing whitespace, and assert homedir resolution failures throw. --------- Co-authored-by: 易良 <1204183885@qq.com> * fix(cli): yield to single-slot background agents (#7258) Co-authored-by: hogeheer <267467744+hogeheer499-commits@users.noreply.github.com> * docs(autofix): require evidenced pre-commit verification, not a bare "verified" (#7486) * docs(autofix): require evidenced pre-commit verification, not a bare "verified" The skill already said to run build/typecheck/lint/Vitest before committing, but softly — and #7408 committed a fix with a TS error the gate then rejected while its summary claimed "verified all 3 commits". A self-assessment the gate contradicts wastes a whole round. Strengthens the address-review contract from "run the checks" to: - actually run them, do not assert them from reading the diff; - if typecheck or a touched-package test fails, do NOT commit — treat the feedback as unresolved (failure.md); - end address-summary.md with a `## Verification` section listing each command run and its result; a bare "verified" is not acceptable. The framing is structural, not etiquette: the deterministic gate re-runs the same commands and discards the round on any failure, so skipping them only moves the rejection later. Pinned by a test so it cannot soften back. This is the checkable half of "audit before committing" — the undirected/reverse-audit-until-clean practice does not transfer to an unsupervised agent (no verifiable stopping condition, and it worsens the timeouts seen on large PRs), but "run the gate's own checks first and show the evidence" does. * fix(autofix): clarify Verification section precedes collapsed Chinese translation (#7486) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> * feat(autofix): stop a PR that fails to push for N rounds in a row (#7482) * feat(autofix): stop a PR that fails to push for N rounds in a row Under takeover the round cap is 100, which is right for a PR that needs many PRODUCTIVE rounds. It is wrong for one that fails every round: #6723 ran 7 consecutive failed rounds (3 agent timeouts at 50 min, 4 gate rejections whose fix broke tests) over 8 hours, heading for round 100, because it is a 5700-line, 47-file, 5-day-old PR racing a fast-moving main — every round re-resolves a conflict it cannot finish or that fails the gate. Retrying at the same per-round budget will not converge; a human has to rebase or split it. Adds CONSECUTIVE_FAILURE_CAP (5), distinct from the total round cap. The handoff step already runs only when a round did NOT push, so it counts the unbroken run of prior failure markers — stopping at the first push ("Addressed the latest review feedback") or legitimate no-op ("no changes needed"), either of which proves progress and resets the streak. At the cap it forces the terminal round even under takeover, with a handoff that names the real fix (rebase/split, then /retry). Cause- agnostic: a timeout and a gate rejection both count. * fix(autofix): address review feedback on consecutive-failure circuit breaker (#7482) - Fix misleading comment: the walk is oldest-first (API order) with reset-on-success, not newest-first with early stop - Prefer the already-fetched ic.json over a redundant gh api call, falling back to the API only when the file is missing - Filter eval markers by re-arm window (win=) so pre-re-arm failures do not immediately re-terminate a re-armed PR - Add test coverage for the MARK_ROUND == MAX_ROUNDS guard and for window-scoped streak counting * fix(autofix): exempt transient model errors from consecutive-failure breaker (#7482) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> * feat(core): restore background agent roster (#7459) * feat(core): restore background agent roster * fix(web-shell): add list_agents to TOOL_DISPLAY_NAMES The new list_agents core wire tool was added to core's ToolNames but not to the web-shell TOOL_DISPLAY_NAMES map, causing toolFormatting.drift.test.ts to fail (expected ['list_agents'] to deeply equal []). Add the missing 'ListAgents' display-name entry so the browser panel shows a friendly name instead of the raw wire name and the drift guard passes. * fix(cli): reload old-session background agents on failed resume rollback When /resume fails after core has swapped but before the UI swap, the catch block rolls core back to the old session via startNewSession(oldSessionId). However the forward path already called resetBackgroundStateForSessionSwitch, which cleared the old session's in-memory background agents. The rollback did not reload them, so list_agents returned empty for the old session (whose sidecars are still on disk) until the next process start or successful resume. Reload the old session's paused background agents after rolling core back, so the restored roster matches on-disk state. Placed after startNewSession so the loadPausedBackgroundAgents current-session guard is satisfied; best-effort via .catch so it never blocks the rollback path. * fix(web-shell): add zh translation for list_agents tool name The toolFormatting test 'has a zh translation for every tool in the display-name map' failed with expected ['list_agents'] to deeply equal [] because list_agents was added to TOOL_DISPLAY_NAMES without a matching toolName.list_agents zh-CN entry. Add the translation to restore parity. * fix(cli): resolve CI failures for background-agent roster restore - Add toolDisplayName.ListAgents translations (en, zh, zh-TW, ca) so the new list_agents tool has a zh entry; fixes i18n/index.test.ts. - Add loadPausedBackgroundAgents and consumePendingRecoveredAgentsNotice to the acpAgent worktree test config mock, which loadSession now calls via #restoreBackgroundAgentsOnResume; fixes acpAgent.worktree.test.ts. * refactor(core): extract incompatible-isolation blocked reason to a const Move the incompatible-isolation blocked-reason string out of an inline literal into a module-level INCOMPATIBLE_ISOLATION_BLOCKED_REASON const, matching its four sibling reasons so the text is discoverable by constant-name grep and edited alongside the others. * fix(core): preserve retained activity state on failed agent revive Address review feedback on the background-agent roster restore: - On a failed completed-agent revive, restore UI state with a non-empty guard instead of `??`. Because `restorePausedEntry` resets the paused entry's `recentActivities` to `[]`, the previous `failedEntry?.field ?? completedEntry.field` kept that empty array and dropped the pre-revive snapshot (the UI Progress section rendered empty). Applied consistently to pendingMessages, recentActivities, and pendingApprovals. Add regression coverage for previously untested paths: - failed revive preserves pre-revive recentActivities - terminal-agent cap admits only the newest MAX_RETAINED_TERMINAL_AGENTS completed sidecars on restore - /resume rollback reloads the old session's background agents - headless resume prepends the recovered-agents notice to the prompt * test(cli): cover interrupted-turn continuation not consuming recovered-agents notice Add ACP and headless regression tests asserting an interrupted-turn continuation does not consume the one-shot recovered-agents notice (the !isContinue / !continueInterrupted guards), so it is delivered on the user's next ordinary prompt. Mirrors the existing slash-command coverage. --------- Co-authored-by: Claude <noreply@anthropic.com> * feat(cli): support custom skill directories via settings (#7395) * feat(cli): support custom skill directories via settings (#7394) Add skills.directories setting that accepts an array of additional directory paths to scan for skills (SKILL.md files). Paths support ~ expansion. Directories are scanned recursively at user level, after the default ~/.qwen/skills/ directory. Example settings.json: { "skills": { "directories": ["~/.agent/skills", "~/.claude/skills"] } } Changes: - settingsSchema.ts: add skills.directories array setting - core Config: add customSkillDirs param and getCustomSkillDirs() - SkillManager: append custom dirs to user-level skill base dirs - CLI config: read skills.directories and pass to core Config * fix(cli): regenerate settings schema for skills.directories (#7394) * fix(core): address review feedback for custom skill directories (#7395) - Use optional chaining for getCustomSkillDirs() to prevent TypeError on partial Config mocks (workspace-skill-management, workspace-skills-status) - Reuse expandHomeDir utility instead of inline tilde expansion - Fix inaccurate 'scanned recursively' wording to 'one level deep' - Correct JSDoc: paths are raw, expansion happens in SkillManager - Trim whitespace from custom dir entries in CLI layer - Add tests for custom dir expansion, dedup, and partial config safety * fix(core): address review feedback for custom skill directories (#7395) * fix(core): address review feedback for custom skill directories (#7395) * test(core): add relative path resolution test for custom skill dirs (#7395) * fix(cli): add Array.isArray guard for skills.directories and safe mode test (#7395) * fix(skills): address review feedback on custom skill directories (#7395) - Add bare mode test for skills.directories guard - Include resolved absolute path in relative directory warning - Clarify that dedup applies to default user dirs, not bundled skills - Regenerate settings schema --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> * fix(core): add image modality support for qwen3.8-max and kimi-k3 models (#7491) * fix(core): add image modality support for qwen3.8-max models qwen3.8-max-preview supports image input but was falling through to the catch-all text-only rule because no pattern matched it. This caused the vision bridge to unnecessarily transcribe images via a secondary model instead of sending them directly to the primary model. * fix(core): also add image modality for kimi-k3 Kimi K3 officially supports image + video input but was falling through to the catch-all text-only rule, same issue as qwen3.8-max. * fix(dingtalk): preserve non-bot mention context (#7473) * fix(dingtalk): preserve non-bot mention context * test(dingtalk): cover plural mentions, staffId fallback, and edge cases (#7473) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> * fix(core): harden the usage salvage around session deletion (#7425) Post-merge review follow-ups on #7391 (three findings): - Salvage the archived transcript in the active-branch deletion too: when both copies co-exist (an interrupted archive) and the fresh active transcript carries no telemetry, the archived copy holds the session's usage history and was deleted unsalvaged. The dedup guard makes the extra call a no-op whenever the active copy already wrote. - Enforce the "never blocks deletion" contract at the call site: a salvageUsageBestEffort wrapper catches and warns, so the guarantee is structural rather than an implementation detail of persistUsageBeforeTranscriptDeletion. The new failure-tolerance test (salvage rejects -> deletion still succeeds) fails without the wrapper — the bare await let the rejection escape through removeSessionFiles' rethrowing catch. - Clear the salvage module mock in beforeEach so the wiring test's invocationCallOrder assertions can never read stale calls. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(core): make fork subagents discoverable (#7460) * test(core): cover Shell truncation without an artifact (#7470) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): autofix route checks existing labels on non-trigger label events (#7481) * fix(ci): autofix route checks existing labels on non-trigger label events When triage adds multiple labels in sequence, per-issue concurrency cancels earlier runs. If the last label is not a trigger label (e.g. scope/build-system), the surviving run skips the issue phase even though the issue already has autofix/approved + status/ready-for-agent. Before ignoring a non-trigger label event, check ISSUE_LABELS_JSON for both required labels. If present and the issue is open, proceed with the issue phase. Trust was already established when the trigger labels were applied (both require triage+ permission). * fix(ci): require trusted sender for label fallback * feat(cli): preserve semantic text when copying VP selections (#7286) * docs(cli): define semantic copy fidelity scope * docs(cli): address semantic frame review gaps * docs(cli): preserve soft-wrap source separators * feat(cli): preserve semantic selection copy * fix(cli): address semantic copy review findings * fix(cli): preserve clipped semantic boundaries * fix(cli): limit separator carrier joiner to visible width in wrap metadata The greedy /\s+/ match in wrapTextWithMetadata could capture more source whitespace than the separator carrier row actually consumed (e.g. a tab following a space), causing duplicated whitespace in semantic copy. Limit the match to visibleLine.length characters and add a mixed space/tab regression test. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> * test(core): stub the registry methods agent.ts actually calls (#7538) The shared stubRegistry in agent.test.ts was missing six methods that agent.ts reaches: bridgeApprovalEvents, getQueuedCount, registerResidentAgent, restartCompletedAgent, unregisterResidentAgent and waitForMessages. That is not a benign omission. The background body wraps its work in a try/catch that routes any throw into registry.fail(), so a missing method never surfaces as 'not a function' — it silently converts a successful run into a failed one. On the GOAL completion path unregisterResidentAgent is called immediately before complete(), so the TypeError replaced the completion entirely: registry.fail('fork-...', 'registry2.unregisterResidentAgent is not a function', ...) That is what broke 'runs a non-interactive fork through the background registry' on main. #7460 added the registry.complete assertion, which exposed the incomplete stub — before it, nothing checked whether the background body finished successfully and the TypeError was swallowed. Stub all six with their real return shapes (unregisterResidentAgent returns boolean, bridgeApprovalEvents returns the unsubscribe callback agent.ts later invokes, waitForMessages resolves to a list) and assert registry.fail was not called before asserting completion, so a future gap reports the actual error instead of 'complete: 0 calls'. * perf(startup): lazy-load Google GenAI SDK on first use (#7512) * perf(startup): lazy-load Google GenAI SDK on first use Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#7512) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#7512) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(vscode): use file picker image paths for vision input (#7493) * fix(vscode): use image paths from file picker * fix(vscode): keep image picker paths raw * fix(vscode): resolve image picker paths on submit * fix(vscode): send picked images as vision context * fix(vscode): encode prompt image file URIs * fix(vscode): address image path review comments * test(vscode): cover image file reference edge cases * fix(cli): open the actual serve fallback port (#7501) * fix(cli): open actual serve fallback port * test(cli): match serve URL to fallback listener * docs(cli): clarify serve listen error handling --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(ci): don't let one failing scenario sink the whole visual preview (#7511) The web-shell visuals render runs every screenshot and flow in a single `test:e2e:visuals`, and that step had no `continue-on-error`, while the compose and upload steps had no `if: always()`. So one failing or timing-out scenario failed the job, the artifact was never uploaded, and the publish workflow had nothing to post — the entire preview vanished even when every other scenario passed and its PNG was already on disk. A flow (a long multi-click sequence) is the most fragile scenario kind, so the fragile one silently takes down the deterministic screenshots. PR #7498 hit exactly this: 29 scenarios passed, one new channel-management flow timed out, and the PR got no preview and no comment at all. Make the after-capture step `continue-on-error` so the passing captures survive and the later steps still compose and upload them. The publish job only runs on a `success` conclusion, so the job must stay green — but a masked failure must not read as a clean preview. Ship the step's real `.outcome` (which continue-on-error does NOT mask, unlike `.conclusion`) to the publisher as `render-status.txt`, and have the comment builder use it: an empty preview whose render failed says "one or more scenarios failed to render" and is explicitly NOT the reassuring green check or the coverage-gap prompt (both imply the render ran); a partial preview is labelled partial above the shots that did render. A missing status file (older run) defaults to complete, so this only ever adds a warning, never suppresses a real preview. The failing scenario still needs fixing — it's now surfaced in the comment rather than by silently deleting everyone else's preview. Co-authored-by: wenshao <wenshao@example.com> * feat(web-shell): add selective shadow DOM isolation (#7551) Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> * feat(web-shell): add renderChatHeader slot for custom session header (#7553) * fix(cli): say review coverage gaps in the author's units, not chunk ids (#7550) The posted review body rendered coverage disclosures with the run's own bookkeeping as subjects: bare chunk ids, unsorted, one per subject. On a run that certified nothing (PR #7268) the body enumerated all 49 chunk ids across two sentences while opening with "Reviewed. Suggestions are inline." — the opener certified the exact thing every following sentence took back, and nothing on the PR page maps a chunk id to code. Three changes, all render-time — the structural entries, the caps, the caller-echo dedup and the stderr remediation still key on chunk ids, which is where the id is the selector a reader can act on: - Coverage now returns the plan's chunk→files table (DiffChunk.files was already in the plan JSON; the coverage type slice dropped it). - compose-review renders chunk gaps through describeChunkGap: every planned chunk collapses to "the entire diff", a narrow gap with known files names the files, and anything wider is counted against the plan's total. Applied to the receipt sentence, the uncoverable sentence (bare CLI entries only — caller-authored entries render verbatim) and the grouped per-cause sentences. - The COMMENT opener may no longer say "Reviewed." over a disclosure set that denies it: when no chunk is both covered and undisclosed — or no chunk universe could be read at all — it opens with a zero-certified warning instead. A rewritten launch demonstrably read its chunk, so coverage alone is not the test; certified is covered with no disclosure against it. Co-authored-by: verify <verify@local> * fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal (#7490) * fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal A base/infra failure BEFORE the agent runs was misread as an agent crash and terminated the PR forever. When an early step fails — installing or building the trusted base, checkout, node setup — the `Prepare branch and feedback` step is skipped, so NEWEST is empty, and the report step's "crashed before reading feedback" branch fired: MARK_ROUND=MAX_ROUNDS, terminal, scan skips it on every future tick. Observed: a web-shell TypeScript break on `main` failed `Install dependencies and build` (which builds the trusted base) across a whole scan batch, and SIX healthy PRs were stranded terminal at round=100 in one run — including ones at round 9 and 11 that had nothing to do with the break. `round=100` there is a terminal sentinel, not 100 attempts. NEWEST-empty now splits on steps.prepare.outcome: - 'skipped' (an earlier step failed, the agent never ran) is infra/base and transient: retry with a sentinel ts so the feedback stays live, incrementing the round so a PERSISTENTLY broken base is still bounded and stops at the cap (recoverable with /retry). - 'success'/'failure' (Prepare ran, no feedback produced) is a genuine pre-read agent crash: unchanged terminal behaviour. This is the reverse of the asymmetry #7482 addresses: that bounds a crash AFTER reading that retried forever; this stops a transient failure BEFORE reading from going terminal after one. * docs(autofix): note a pre-Prepare cancel also retries intentionally (#7490) * fix(autofix): also retry a cancelled/empty prepare outcome, not just skipped A previous review comment on this PR noted that a job cancelled before Prepare should retry too. It was right about the intent but the code did not do it: `steps.prepare.outcome` is 'cancelled' for a cancel and '' for a job that stopped before Prepare entered the step context — both DISTINCT from 'skipped', so `== 'skipped'` sent them to the terminal branch, the same over-termination this PR exists to fix. Match on "not a real Prepare run" (`!= 'success' && != 'failure'`) instead, so skipped, cancelled, and empty all retry; only a Prepare that actually ran to a verdict (success/failure) with no feedback stays terminal — the genuine pre-read agent crash. Test extended to drive the cancelled and empty cases (retry) and both real-run outcomes (terminal); mutation-verified that reverting to `== 'skipped'` reddens the cancelled case. * test(autofix): update the pre-read-crash case for the broadened retry The prior commit broadened NEWEST-empty retry to skipped/cancelled/empty but left the older 'replays the handoff decision' test asserting the old terminal behaviour for an unset PREPARE_OUTCOME (which now retries). That test's terminal cases now set PREPARE_OUTCOME=success/failure explicitly — the only outcomes that still terminate — so it exercises the genuine pre-read agent crash rather than the infra/cancel path. * test(autofix): anchor the skipped-Prepare extraction past the CONSEC block CI reddened `retries a skipped-Prepare` after main's consecutive-failure cap (#7482) merged into this branch: that block was inserted between this decision block and the report `{`, and it calls `gh api`. The test's `{`-anchored regex over-captured through it, so the extracted script ran the unstubbed `gh api` and failed. Anchor the end on the same `# Consecutive-failure` comment the sibling gate-crash test already uses, so the extraction stops at this decision block's own closing `fi`. * fix(autofix): exempt skipped-Prepare from the consecutive-failure breaker A broken base build skips Prepare, producing no API error file — so the consecutive-failure breaker ran on the new retry path and, after 5 scans, re-introduced the exact mass-stranding this PR exists to prevent. Exempt pre-agent infra failures (skipped/cancelled/empty outcome) from the breaker, mirroring the transient 429/5xx exemption: same failure class (not the PR's fault, self-heals, hits the whole batch). The round cap + sentinel-ts /retry recovery already bounds a persistently broken base. Also trim "checkout" from the retry headlines (checkout failures do not land in this branch) and hoist the duplicated MARK_TS assignment. * fix(autofix): reset the consecutive-failure streak on prior infra-failure markers The streak walker counted prior infra-failure headlines ("AutoFix could not start —…") as failures, inflating the consecutive-failure count on subsequent rounds. A PR with 3 real agent failures, then 3 rounds of base-build infra failures, then 1 more real failure would trip the cap-5 breaker even though only 4 rounds were the PR's fault. Add the two infra-failure headline patterns as reset strings in the streak walker, alongside the existing push and no-op resets. The genuine agent-crash headline ("AutoFix could not start evaluation —…") is deliberately excluded — it is a real failure and must still count. * fix(autofix): clarify infra-failure headlines and else-branch comment (#7490) Address review nits: the retry headline now mentions cancelled runs, the cap headline says 'reached the round cap' instead of overstating 'could not start for N rounds', the else-branch comment says 'prepare itself crashed' instead of 'agent crash', and the streak-reset pattern is simplified now that both infra headlines share the same prefix. --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> * fix(cli): keep role codenames and brief paths out of the posted review body (#7560) The posted body still carried two operator registers #7550 left in place: roster role subjects rendered their internal codenames ("Agent 1c: Cross-file tracer", "Test coverage matrix (whole-diff)"), and an unread brief's disclosure interpolated its filesystem path. And when verify and the reverse audit failed the same way, the body said it twice, in two near-identical sentences. - Every Brief now carries a publicLabel — the dimension said as what it checks ("the cross-file consistency pass") — and coverage's structural disclosures carry it as publicSubject beside the internal subject, plus a path-free publicReason for unread briefs. The internal label and the path stay on stderr, where they are the selector an operator acts on; every dedup and certification check still keys on the internal subject. - compose-review renders the public fields and groups by the reason the body PRINTS, so two unread briefs share one path-free sentence instead of repeating it per role. - verificationGaps merges verify and reverse-audit failures of the same delivery shape into one sentence with both subjects and both consequences; mixed shapes keep their precise per-role texts, and the per-role rebuild commands stay on stderr either way. Co-authored-by: verify <verify@local> * fix(autofix): retry an agent timeout instead of advancing past its feedback (#7563) A timeout evaluated NOTHING — the agent ran out of budget before finishing, so nothing was committed and the feedback is unaddressed. It was treated as an evaluated verdict (real ts, watermark advances), which strands that feedback: the next scan sees "nothing new" and never retries. Observed on #7471 (round 13/100), a heavily-reviewed 1871-line PR: rounds 11 and 13 timed out, but round 12 pushed — so a timeout is transient far more often than not, and advancing past it left the round-13 feedback unhandled. run-agent.mjs now drops an `agent-timeout` signal on result.timedOut, and the handoff routes it like a pre-verdict crash: sentinel ts (feedback stays live) and a retry, with a headline that names the real fix at the cap (split the PR or raise the budget). A PR that PERSISTENTLY times out is bounded by the round cap and the consecutive-failure cap, so this cannot loop forever — it just stops treating a one-off budget blip as a verdict. The loop guard stays terminal (a tool-call loop is a real defect, not a budget blip). An API error still routes to its own model-key handoff; the timeout signal is written only when NOT an API error. Co-authored-by: wenshao <wenshao@example.com> * feat(serve): add workspace-level generation (#7552) * feat(serve): add workspace-level generation * docs(serve): document workspace generation capability * fix(serve): align workspace generation contracts --------- Co-authored-by: ytahdn <ytahdn@gmail.com> * ci: matrix ECS runner update + sudo install + repository_dispatch trigger (#7513) * ci: matrix ECS runner update with sudo install - Use matrix strategy (ecs-update-sg, ecs-update-64c) to update both physical ECS hosts in parallel (fail-fast: false). - Always use sudo npm install -g so the package lands in /usr/local (system-wide PATH) instead of the runner user's home directory. - Move concurrency to job level (matrix context not available at workflow level per actionlint). - Add repository_dispatch trigger for release-driven updates. - Register new runner labels in actionlint.yaml. * fix(ci): use dispatch version for runner update --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(web-shell): include managed id in artifact open requests (#7570) Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> * feat(serve): persist workspace channel configuration (#7514) * feat(serve): persist workspace channel configuration * fix(serve): harden channel settings snapshots * fix(serve): validate startup channel names * fix(serve): reserve all channel name --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(sdk-python): require canonical form in validate_session_id (#7532) uuid.UUID() accepts several non-canonical spellings — braced {...}, urn:uuid:..., and dash-less hex — so validate_session_id let them through after the RFC 4122 variant check. The value is then forwarded to the CLI verbatim as --session-id/--resume, producing a malformed session id downstream rather than a clear error at the SDK boundary. Reject anything whose canonical form differs from the input. Case is deliberately not part of the comparison: UUID() lowercases, and an all-uppercase spelling is still valid canonical input. Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(web-shell): sync background agent status (#7561) * fix(web-shell): sync background agent status * fix(web-shell): harden background agent reconciliation --------- Co-authored-by: ytahdn <ytahdn@gmail.com> * feat(core): propagate trusted daemon invocation context (#7279) * feat(core): propagate trusted daemon invocation context * test(cli): update ACP startup expectation * refactor(core): centralize ACP capability env key * test(cli): update worktree ACP core mock * test(integration): run daemon context smoke on PRs * test(ci): update no-AK smoke expectation * test(core): cover invocation context isolation * fix(cli): compare ACP capability safely * fix(docs): restore GitHub action input names * fix(core): sanitize private ACP capability from child env * fix(core): reuse private ACP capability env constant * test(cli): cover malformed trusted invocation context * test(acp-bridge): assert exact child environment --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: 易良 <1204183885@qq.com> * fix(feishu): await stream cancels in media download teardown (#7465) * fix(feishu): await stream cancels in media download teardown downloadMedia left two reject paths' stream teardown unawaited: - the oversize-stream path called reader.cancel() without awaiting, so a cancel error during teardown became an unhandled rejection (fatal under Node's default --unhandled-rejections=throw); - the Content-Length reject path returned without cancelling resp.body, leaving the connection pinned until GC. Both were already fixed for the sibling DingTalk downloader in #7361 (which was itself modelled on this Feishu code), so this brings Feishu to parity. Adds a regression test that pins the reader.cancel() await via a rejecting cancel, plus an assertion that the Content-Length path releases the body. * test(feishu): cover a rejecting body.cancel() on the Content-Length path Mirrors the existing reader.cancel() teardown test for the other reject path, per review feedback. Removing the await on resp.body?.cancel() flips execution onto the 'rejected: size ... exceeds' branch and the test fails. * fix(autofix): make the review-address report wrapper lines bilingual (#7569) The agent's address-summary.md / no-action.md already ends with a collapsed Chinese translation, but the workflow-appended wrapper lines around it — the "Addressed/Reviewed the latest feedback" lead-in, the "Base-conflict check" line, and the "Re-review when you have a moment" footer — were English-only and sat outside that block. So the posted comment was only half translated, unlike the takeover-ack comments (full collapsed Chinese block) and the "model/模型" sign-off in this same report (already inline-bilingual). Give each wrapper line an inline Chinese translation, matching the model/模型 idiom. The English halves are preserved verbatim — the streak-reset detector globs on "Addressed the latest review feedback" and "no changes needed", and a test extracts these lines — so behaviour is unchanged and old English-only comments still match. A new test pins each English-Chinese pair so a future reword that drops the Chinese fails. The terminal handoff/failure comment is left English-only for now (SKILL.md keeps it so by design); that is a separate change. Co-authored-by: wenshao <wenshao@example.com> * feat(cli): post the review body bilingually when the PR description is Chinese (#7564) When the PR author writes Chinese, the posted /review body was English-only. fetch-pr now records whether the PR description contains Han characters (prDescriptionHasHan, detected from the same gh pr view call and stamped into the plan report), and compose-review renders the body bilingually off that flag: the English body leads, the complete Chinese version rides collapsed in a <details><summary>中文说明</summary> block, and the model footer stays outside the fold. The signal is the CLI's own — the caller cannot toggle the register of a certified body — and a local plan has no field, so nothing changes for terminal-only reviews. Every deterministic body fragment carries an en/zh pair end to end: compose-review's clause templates and describeChunkGap phrases, the coverage disclosures (reasons, publicLabel role subjects via a new publicLabelZh, the path-free unread-brief reason) and the Step 4/5 gap texts including the combined same-shape sentence. Fragments with no deterministic translation — model-written findings, caller echoes, interpolated errors — ride verbatim in both halves. verificationGaps now returns structural {subject, reason, subjectZh, reasonZh} entries, which also removes compose-review's last recover-the-boundary-from-prose parse. SKILL.md instructs the same format for the model-authored inline comments: English finding first (marker and suggestion block stay in the English half — tooling filters on them), full Chinese translation collapsed beneath, footer last. Co-authored-by: verify <verify@local> * feat(autofix): auto-rerun a check that died on infrastructure, once (#7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> * fix(serve): detect stale SSE cursors across daemon restarts via epoch token; preserve turn attribution and surface compaction failures in replay (#7458) * fix(daemon): epoch-token restart detection, compaction attribution, and degraded-snapshot signaling (DAEMON-001/007/008) * fix(acp-bridge): field-level turn attribution merge and replayDegraded bridge test (#7458) * fix(serve): skip bus epoch lookup for virtual subagent SSE streams (#7458) The REST SSE route looked up the bus epoch for every session id, but virtual subagent sessions ride their own bus and their compound ids are not in the bridge's byId map, so the lookup threw and aborted the subscription — breaking subagent event streams. Skip the lookup for the virtual path and degrade a torn-down real session to a headerless stream (mirrors the /acp route). Also bumps the daemon browser SDK bundle budget (167KB -> 168KB) for the epoch fields and declares eventEpoch on DaemonSession so the create/attach path drops its inline type cast. * fix(serve): stamp eventEpoch on accepted continuations and surface replayDegraded in the SDK (#7458) Address three review suggestions: - POST /session/:id/continue now returns eventEpoch alongside lastEventId, mirroring the prompt 202 envelope so continuation-seeded SSE cursors detect daemon restarts (DAEMON-001) - DaemonSessionClient exposes replayDegraded from the load response so SDK consumers can prefer the full transcript over a degraded snapshot - add /acp dispatch-level regression test for the degraded-snapshot stderr breadcrumb (fires only when snapshot.degraded is set) * test(cli): fix load-reply race in the degraded-breadcrumb transport test Await each session/load reply frame before opening the session stream so the GET cannot race conn.ownSession() into a 403; addresses the review Critical on the deg-0 arm. * fix(serve): allow and expose X-Qwen-Event-Epoch in CORS headers Cross-origin SSE clients must send the epoch header through preflight and read it from the response, or stale-cursor detection (DAEMON-001) is silently disabled for every CORS client. --------- Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Autofix <qwen-autofix[bot]@users.noreply.github.com> * feat(core): Align GenAI telemetry with ARMS (#7536) * feat(core): align GenAI telemetry with ARMS Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): remove estimated token usage splits Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): address GenAI telemetry review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(serve): avoid TOCTOU race dropping live sessions from list response (#7556) * Initial plan * fix(serve): avoid TOCTOU race dropping live sessions from list response --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: 易良 <1204183885@qq.com> * fix(cli): prevent monitor turns after task_stop (#7573) --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: destire-mio <qppque@gmail.com> Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com> Co-authored-by: Dragon <52599892+DragonnZhang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: jinye <djy1989418@126.com> Co-authored-by: chinesepowered <nlai@rediffmail.com> Co-authored-by: ovochouovo <18212194+ovochouovo@users.noreply.github.com> Co-authored-by: Edenman <67549719+BZ-D@users.noreply.github.com> Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: ytahdn <1294726970@qq.com> Co-authored-by: ytahdn <ytahdn@gmail.com> Co-authored-by: Truraly <94105924+Truraly@users.noreply.github.com> Co-authored-by: zjgzx1988 <zjgzx1988@hotmail.com> Co-authored-by: hogeheer499-commits <hogeheer499@gmail.com> Co-authored-by: hogeheer <267467744+hogeheer499-commits@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Nothing Chan <chenliu.cl@alibaba-inc.com> Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> Co-authored-by: yuanyuanAli <135116774+yuanyuanAli@users.noreply.github.com> Co-authored-by: verify <verify@local> Co-authored-by: qqqys <qys177@gmail.com> Co-authored-by: callmeYe <512217680@qq.com> Co-authored-by: Qwen Autofix <qwen-autofix[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
|
@qwen-code /retry |
|
🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it. 中文说明🔄 已重新武装 AutoFix。下一次扫描会从头重新读取本 PR 的反馈,轮次计数也已重置。未删除任何内容 —— 本标记使其上方的评估标记失效。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review feedback addressedImplemented
Declined — already addressed in current code
Declined — out of scope / not worth diff growth
Verification
中文说明已处理的评审反馈已实现
已拒绝——当前代码中已解决
已拒绝——超出范围 / 不值得增加 diff
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No action requiredThe latest review round on PR #6506 contains no actionable feedback:
No code changes were made. 中文说明无需操作PR #6506 的最新审查轮次中没有需要处理的反馈:
未进行任何代码更改。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Thanks for the PR! Template: the body uses "Summary" / "Test plan" instead of the template's headings ("What this PR does", "Why it's needed", "Reviewer Test Plan", "Risk & Scope", "Linked Issues"). Content is there, just under different names — not blocking, but worth aligning next time. Problem: real and well-quantified. Pasting ~260K characters fires 260K per-character keypress events through readline, blocking the main thread for ~1.7s. The O(n²) Direction: aligned. Paste handling is core CLI input infrastructure, and this directly improves the interactive experience for a common user action. Size: not applicable — no core module paths touched. All changes are in Approach: the core fix — intercepting bracketed paste markers at the raw stdin One note: the PR description mentions clipboard size probing (pbpaste/xclip/powershell) and a percentage progress bar, but the actual code just shows received bytes ( The test coverage is thorough — chunk reassembly, partial marker straddling, idle timeout rescheduling, Ctrl+C semantics, the ESC-split tradeoff. Good regression tests. Moving on to code review. 🔍 中文说明感谢贡献! 模板:PR 正文使用了 "Summary" / "Test plan" 而非模板要求的标题("What this PR does"、"Why it's needed"、"Reviewer Test Plan"、"Risk & Scope"、"Linked Issues")。内容都有,只是标题不同——不阻塞,但下次建议对齐。 问题:真实且量化充分。粘贴约 260K 字符会通过 readline 触发 260K 个逐字符按键事件,阻塞主线程约 1.7 秒。回退路径上的 O(n²) 方向:对齐。粘贴处理是 CLI 输入基础设施的核心部分,这直接改善了常见用户操作的交互体验。 规模:不适用——未触及核心模块路径。所有更改都在 方案:核心修复——在原始 stdin 一点说明:PR 描述提到了剪贴板大小探测(pbpaste/xclip/powershell)和百分比进度条,但实际代码只显示接收字节数( 测试覆盖很全面——分块重组、部分标记跨界、空闲超时重调度、Ctrl+C 语义、ESC 分割权衡。回归测试写得很好。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (readline fires per-character keypress events for paste content), I would intercept bracketed paste markers at the raw stdin Comparison: the PR does exactly this. The No critical blockers found. Specific observations:
Real-Scenario TestingSmoke test (headless)Interactive: normal typingInteractive: small bracketed pasteSent Paste markers stripped, content inserted correctly. Interactive: large paste (50KB)Sent 50,000 characters via bracketed paste: Processed instantly — no freeze, no per-character event storm. The Unit testsTypecheck中文说明代码审查独立方案: 鉴于问题(readline 对粘贴内容逐字符触发按键事件),我会在原始 stdin 对比: PR 正是这样做的。 未发现关键阻塞问题。具体观察:
真实场景测试冒烟测试(无头模式)交互模式:正常输入交互模式:小括号粘贴通过 tmux paste-buffer 发送 粘贴标记被剥离,内容正确插入。 交互模式:大粘贴(50KB)通过括号粘贴发送 50,000 个字符: 即时处理——无冻结,无逐字符事件风暴。 单元测试108 个测试全部通过。 类型检查干净,无错误。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — solid fix for a real perf problem; only non-blocking nit is the inaccurate PR description. The approach is exactly right: intercept bracketed paste at the raw stdin level so readline never sees 260K per-character events. The implementation handles the hard parts well — partial markers straddling chunk boundaries, stuck-paste recovery via idle timeout, coordination between the raw and keypress-level paths to prevent duplicate events. The ESC-split tradeoff (minLen=2 vs minLen=1) is documented and pinned by a regression test, which is the kind of thing that saves the next maintainer an hour of confusion. The 50KB paste test confirmed the fix works: instant processing, The only reservation is the PR description claiming clipboard size probing and a percentage progress bar that don't exist in the code — the actual implementation is a simple byte counter. The code is better than the description suggests, but the mismatch is worth fixing so reviewers don't go looking for code that isn't there. 中文说明信心:4/5 — 对真实性能问题的扎实修复;唯一的非阻塞问题是 PR 描述不准确。 方案完全正确:在原始 stdin 层拦截括号粘贴,使 readline 不会看到 260K 个逐字符事件。实现很好地处理了困难部分——跨分块边界的部分标记、通过空闲超时恢复卡住的粘贴、raw 路径和按键级路径之间的协调以防止重复事件。ESC 分割权衡(minLen=2 vs minLen=1)有文档记录并由回归测试锁定,这是为下一个维护者节省一小时困惑的那种东西。 50KB 粘贴测试确认修复有效:即时处理, 唯一的保留是 PR 描述声称有剪贴板大小探测和百分比进度条,但代码中不存在——实际实现是一个简单的字节计数器。代码比描述的要好,但不匹配值得修复,以免审查者去寻找不存在的代码。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
🔬 Maintainer verification — round 4, re-run at head
|
| commit | what it is | why it matters |
|---|---|---|
8d184264 — Merge main into the branch |
brings the branch current; new merge-base e7097d0e |
main advanced a lot and its changes land in the same two files this PR touches (KeypressContext.tsx gained the modifyOtherKeys/Ghostty CSI-u decode; Footer.tsx gained the "Enter to steer" hint) — a real interaction surface I re-checked |
68c7cbc9 — test(cli): add multi-chunk raw paste and passthrough Ctrl+C escape tests |
test-only (+81 lines) | adds the handleStdinData-level coverage that was round-1 finding #3 (previously declined as out-of-scope) |
The paste runtime code (KeypressContext.tsx + Footer.tsx) is byte-identical to round 3 — I diffed it (git diff 2d8055e4:… 68c7cbc9:… on the two files shows only main's merged-in code, none of it in the paste path). So this round is about two things only: (1) does it still build + behave on the new main, and (2) is the newly-added coverage real. Both check out.
TL;DR — merge-ready on the code. Suite is green 108/108 on the merged tree (was 100 at round 3; the modifyOtherKeys code that merged into the same file does not disturb the paste path), the 7× paste win still holds when rebuilt on current main, and the last open thread (raw-path test coverage) is now closed. The only remaining item is the stale PR description, now 3 rounds old and describing removed behaviour — please refresh before merge.
Methodology unchanged from rounds 1–3: the real bundled CLI driven through a PTY (@lydell/node-pty + @xterm/headless, isolated HOME), genuine bracketed-paste bytes written to the tty, wall-clock from the paste write → rendered [Pasted Content N chars] placeholder. Both arms built from the same tree, differing only in the two runtime files (base = merge-base e7097d0e, pre-PR main).
Prior findings → status at 68c7cbc9
| finding (round) | status now |
|---|---|
| 🔴 raw path undoes #6708 for image pastes — red CI (r2 blocker) | ✅ still fixed — reports an unavailable native module for an empty paste green |
| 🟡 paste-end split while raw path committed — data loss (r2) | ✅ still fixed — tail-append + regression test green |
🟡 no handleStdinData-level test coverage (r1 #3, declined) |
✅ now added in 68c7cbc9 — 3-chunk reassembly + passthrough Ctrl+C escape |
| 🟡 stale PR description | 🟡 still stale — see bottom; now actively wrong on 3 counts |
interaction risk: main's modifyOtherKeys merged into the same file |
✅ new this round — 108/108 green, paste path untouched |
✅ 1 · Suite green on the merged tree — 108/108
vitest run src/ui/contexts/KeypressContext.test.tsx at head: 108 passed (108), 0 failed (round 3 was 100 — the merge pulled in main's kitty/modifyOtherKeys tests plus this PR's 2 new ones). The two new raw-path tests run and pass; the correctness-critical edges verified as load-bearing in round 3 (partial-marker split, idle-flush tail) are all still green.
Honest coverage note (non-blocking): the new reassembles … across three or more stdin chunks test also passes against pre-PR source — I swapped KeypressContext.tsx to e7097d0e and ran just that case (1 passed). The keypress-level fallback assembles the same single event, so the test locks the observable contract (multi-chunk → one paste) rather than specifically guarding the raw fast-path. That's fine — the raw path's tricky edges are covered by the round-3 tests that do flip red when reverted; this one documents the user-visible behaviour.
✅ 2 · Perf re-measured on current main — 7× at the top, no regression anywhere
Rebuilt both arms from source on the new base and re-ran the full size sweep (fresh CLI per run, interleaved, median of 3):
| paste size | base e7097d0e |
PR 68c7cbc9 |
result |
|---|---|---|---|
| 2,000 | 15 ms | 18 ms | ≈ parity (both instant) |
| 20,000 | 61 ms | 53 ms | ≈ parity |
| 100,000 | 570 ms | 194 ms | 2.9× faster |
| 260,000 | 3168 ms | 454 ms | 7.0× faster |
Base grows super-linearly (the O(n²) Buffer.concat + per-char readline events); PR stays near-flat (raw-level O(n) accumulation, one broadcast). There is no crossover — PR is equal-or-faster at every size I could measure. Pastes below the placeholder threshold (≈ a few KB) are inserted inline on both arms (I confirmed a 600-char paste renders as literal text, not a [Pasted Content] placeholder), so the perf-critical region is exactly the large pastes above.
✅ 3 · Progress indicator — re-confirmed live in the real binary
Footer.tsx is unchanged since round 2; re-ran the live capture anyway. A 400 KB paste in 16 KB chunks produced 25 monotonic Pasting… N KB readings (16 → 400 KB), clearing the instant the [Pasted Content 409600 chars] placeholder lands. Frame below is a real mid-paste capture (Pasting… 192 KB). The Pasting… key is present in en / zh / zh-TW.
CI / mergeability
mergeable: MERGEABLE; the substantive checks are green (13 success / 33 skipped, 0 failing); the latest automated review is LGTM, looks ready to ship. ✅. The BLOCKED state is the review-approval gate (REVIEW_REQUIRED), not CI.
🟡 Pre-merge cleanup (non-blocking): the description still advertises removed behaviour
Flagged since round 2, still unaddressed — the body describes code that no longer exists, and the merge commit will carry it:
- "asynchronously probe clipboard size … accurate percentage progress bar (
… 42% (128 KB))" → the clipboard probe was removed in round-2's6ca0c1c; it's a received-byte counter now (Pasting… N KB). - "reducing processing time from ~1.7s to ~8ms" → the user-perceived figure is ~454 ms end-to-end for 260K here, not 8 ms.
- "98 tests pass" → 108 now.
Also the triage bot has repeatedly asked for the repo's PR template (## What this PR does / ## Why it's needed / ## Reviewer Test Plan). Refreshing the body closes both at once.
Verdict
The paste runtime is unchanged from round 3 and has now been verified across four rounds; this round confirms it survives being brought current with main (the modifyOtherKeys change that landed in the same file does not perturb the paste path — 108/108) and that the last coverage gap is closed. The 7× large-paste win reproduces on the current base with no small-paste regression. Recommending merge on the code — please just refresh the stale description/test-plan first so the squash commit doesn't ship the removed clipboard-probe percentage bar.
🇨🇳 中文版本(点击展开)
🔬 维护者验证 —— 第 4 轮,在 head 68c7cbc9 重新验证(现已与 main 同步)
接续 第 1 轮(78a40ea)、第 2 轮(6ca0c1c)与 第 3 轮(2d8055e4)。自第 3 轮以来,分支恰好只做了一次 main 合并 + 一个纯测试提交 —— 生产逻辑没有任何改动:
| 提交 | 内容 | 为什么重要 |
|---|---|---|
8d184264 —— 将 main 合并进分支 |
使分支同步至最新;新合并基 e7097d0e |
main 期间推进很多,且其改动正好落在本 PR 改动的同两个文件上(KeypressContext.tsx 新增了 modifyOtherKeys/Ghostty CSI-u 解码;Footer.tsx 新增了 “Enter to steer” 提示)—— 这是一个真实的交互面,我重新做了检查 |
68c7cbc9 —— test(cli): add multi-chunk raw paste and passthrough Ctrl+C escape tests |
纯测试(+81 行) | 补上了 handleStdinData 级别的覆盖,即第 1 轮结论 #3(此前被以“超出范围”为由婉拒) |
粘贴运行时代码(KeypressContext.tsx + Footer.tsx)与第 3 轮逐字节一致 —— 我对这两个文件做了 diff,只有 main 合并进来的代码,均不在粘贴路径上。所以本轮只关注两件事:(1) 在新的 main 上是否仍能构建并正确运行;(2) 新增的覆盖是否真实。两者均通过。
要点 —— 就代码而言可以合并。 合并后的树上测试全绿 108/108(第 3 轮为 100;合并进同一文件的 modifyOtherKeys 代码不会扰动粘贴路径),在当前 main 上重建后 7× 的粘贴提速依然成立,最后一条遗留项(raw 路径测试覆盖)现已关闭。唯一剩下的是 PR 描述过时,至今已 3 轮、且描述的是已被移除的行为 —— 请在合并前刷新。
方法与第 1–3 轮一致:真实打包后的 CLI 通过 PTY(@lydell/node-pty + @xterm/headless,隔离 HOME)驱动,向 tty 写入真实的括号粘贴字节,从粘贴写入到渲染出 [Pasted Content N chars] 占位符计时。两个 arm 从同一棵树构建,仅在这两个运行时文件上不同(base = 合并基 e7097d0e,即 PR 之前的 main)。
既往结论 → 在 68c7cbc9 的现状
| 结论(轮次) | 现状 |
|---|---|
| 🔴 raw 路径对图片粘贴撤销了 #6708 —— CI 红(第 2 轮阻塞项) | ✅ 仍已修复 —— reports an unavailable native module for an empty paste 绿 |
| 🟡 raw 路径已提交时 paste-end 被拆分 —— 数据丢失(第 2 轮) | ✅ 仍已修复 —— 尾部追加 + 回归测试 绿 |
🟡 缺少 handleStdinData 级测试覆盖(第 1 轮 #3,曾被婉拒) |
✅ 现已补上(68c7cbc9)—— 3 块重组 + passthrough Ctrl+C 逃生 |
| 🟡 PR 描述过时 | 🟡 仍过时 —— 见文末;现已在 3 处与代码不符 |
交互风险:main 的 modifyOtherKeys 合并进同一文件 |
✅ 本轮新增 —— 108/108 全绿,粘贴路径未受影响 |
✅ 1 · 合并后的树上测试全绿 —— 108/108
在 head 上 vitest run … KeypressContext.test.tsx:108 passed (108),0 失败(第 3 轮为 100 —— 合并带入了 main 的 kitty/modifyOtherKeys 测试,加上本 PR 的 2 个新测试)。两个新的 raw 路径测试运行并通过;第 3 轮已确认“承重”的关键边界(partial-marker 拆分、idle-flush 尾部)依旧全绿。
(见上方 tests 截图)
诚实的覆盖说明(非阻塞):新增的 reassembles … across three or more stdin chunks 测试在 PR 之前的源码上也能通过 —— 我把 KeypressContext.tsx 换回 e7097d0e 并只跑该用例(1 passed)。keypress 级别的回退路径会组装出相同的单个事件,因此该测试锁定的是可观测契约(多块 → 单次粘贴),而非专门守护 raw 快路径。这没问题 —— raw 路径的棘手边界由第 3 轮那些一回退就变红的测试守护;这一条记录的是用户可见行为。
✅ 2 · 在当前 main 上重新测量性能 —— 顶端 7×,各处无回退
在新 base 上从源码重建两个 arm,重跑完整规模扫描(每次全新 CLI,交错进行,取 3 次中位数):
| 粘贴规模 | base e7097d0e |
PR 68c7cbc9 |
结果 |
|---|---|---|---|
| 2,000 | 15 ms | 18 ms | ≈ 持平(都瞬时) |
| 20,000 | 61 ms | 53 ms | ≈ 持平 |
| 100,000 | 570 ms | 194 ms | 快 2.9× |
| 260,000 | 3168 ms | 454 ms | 快 7.0× |
base 超线性增长(O(n²) 的 Buffer.concat + 逐字符 readline 事件);PR 近乎平坦(raw 级 O(n) 累积,一次广播)。不存在交叉点 —— 在我能测量的每个规模上,PR 都是持平或更快。低于占位符阈值(约几 KB)的粘贴在两个 arm 上都被内联插入(我确认 600 字符粘贴渲染为字面文本,而非 [Pasted Content] 占位符),因此性能关键区就是上面这些大粘贴。
(见上方 perf 截图)
✅ 3 · 进度指示器 —— 在真实二进制中复测
Footer.tsx 自第 2 轮未变,仍重新做了实时采集。400 KB 粘贴分 16 KB 块 → 25 个单调递增的 Pasting… N KB 读数(16 → 400 KB),在 [Pasted Content 409600 chars] 占位符出现的瞬间清除。上方为真实的粘贴中途截帧(Pasting… 192 KB)。Pasting… 键在 en / zh / zh-TW 中均存在。
(见上方 progress 截图)
CI / 可合并性
mergeable: MERGEABLE;实质性检查为绿(13 成功 / 33 跳过,0 失败);最新自动评审为 LGTM, looks ready to ship. ✅。BLOCKED 状态是评审审批门禁(REVIEW_REQUIRED),并非 CI。
🟡 合并前清理(非阻塞):描述仍在宣传已移除的行为
自第 2 轮已指出、至今未改 —— 描述的是已不存在的代码,且会被带进合并提交:
- “异步探测剪贴板大小 …… 精确百分比进度条(
… 42% (128 KB))” → 剪贴板探测已在第 2 轮6ca0c1c移除;现在是接收字节计数器(Pasting… N KB)。 - “将处理时间从 ~1.7s 降到 ~8ms” → 此处 26 万字符端到端实测约 454 ms,并非 8 ms。
- “98 个测试通过” → 现为 108。
此外 triage 机器人多次要求使用仓库的 PR 模板(## What this PR does / ## Why it's needed / ## Reviewer Test Plan)。刷新描述可一并解决。
结论
粘贴运行时自第 3 轮未变,至此已跨四轮验证;本轮确认它在被同步至最新 main 后依然成立(落入同一文件的 modifyOtherKeys 改动不会扰动粘贴路径 —— 108/108),并且最后一处覆盖缺口已关闭。7× 的大粘贴提速在当前 base 上复现,且小粘贴无回退。就代码而言建议合并 —— 只请先刷新过时的描述 / 测试计划,以免 squash 提交继续宣传已被移除的剪贴板探测百分比进度条。
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: No code changes requiredThe round-4 maintainer verification confirms the code is merge-ready: 108/108 tests pass, the 7× large-paste performance improvement reproduces on the current The only feedback item is a non-blocking request to refresh the stale PR description, which still describes removed behavior (clipboard probe with percentage progress bar, ~8 ms timing claim, 98-test count). Updated 中文说明无需代码变更第 4 轮维护者验证确认代码可以合并:108/108 测试通过,7 倍大粘贴性能提升在当前 唯一的反馈项是非阻塞性的 PR 描述刷新请求——描述仍在宣传已移除的行为(带百分比进度条的剪贴板探测、约 8 毫秒的计时声明、98 个测试)。已在工作目录中准备了更新后的 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |



Summary
Buffer.concatper-character accumulation (O(n²)) withstring[].push + join(O(n)) for the fallback paste pathPasting… ████████░░░░░░░░ 42% (128 KB)) while the terminal transmits data chunk by chunkTest plan
[Pasted Content XXX chars]appearsnpx vitest run packages/cli/src/ui/contexts/KeypressContext.test.tsx→ 98 tests passnpx tsc --noEmit --project packages/cli/tsconfig.json→ no errors