fix(cli): show mode indicator alongside steering hint during streaming - #7219
Merged
Conversation
PR #7090 added a `StreamingState.Responding` branch in the Footer's `leftBottomContent` priority chain that takes precedence over the `AutoAcceptIndicator`, causing the approval mode name text to disappear while the agent is responding. Users switching modes with Shift+Tab during streaming only saw the input prompt border color change. Include the `AutoAcceptIndicator` in the streaming branch so both the steering hint and the mode name are visible at the same time. Fixes #7217
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed regression with clear evidence — issue #7217 documents that PR #7090's steering hint branch in the Footer ternary chain short-circuits the Direction: aligned — this restores visibility of the mode indicator during streaming, which users rely on for confirmation after Shift+Tab cycling. No CHANGELOG reference needed for a regression fix. Size: not applicable (no core paths touched — Approach: the scope is exactly right. Six lines of production code that append the existing Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的回归，有明确证据——issue #7217 记录了 PR #7090 的 steering hint 分支在 Footer 三元链中优先于 方向：对齐——恢复 streaming 期间模式指示器的可见性，用户依赖它确认 Shift+Tab 切换结果。回归修复无需 CHANGELOG 参考。 规模：不适用（未触及核心路径——仅 方案：范围恰好。6 行生产代码将已有的 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Contributor
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. |
Collaborator
Code ReviewIndependent proposal: given the ternary chain in Findings: no issues. The change reuses the existing No correctness bugs, no security concerns, no convention violations. Real-Scenario TestingRan the CLI in tmux with For the footer specifically, the Ink framework renders it via cursor positioning that tmux Before (main branch After (PR Full suite: 25/25 tests pass. 中文说明代码审查独立方案： 鉴于 发现： 无问题。改动复用了已有的 无正确性 bug、无安全隐患、无规范违反。 真实场景测试在 tmux 中以 对于 footer 本身，Ink 框架通过光标定位渲染，tmux 修复前（main 分支 完整套件：25/25 测试通过。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean regression fix, minimal diff, verified before/after. This is exactly what a regression fix should look like. PR #7090 added the steering hint branch and inadvertently hid the mode indicator during streaming. This PR appends the indicator back in six lines of production code, reusing the existing component, matching the existing separator style, and adding a focused test that fails without the fix and passes with it. My independent proposal was identical to what the author did — render The before/after unit test is definitive: the new test fails on main's Ships it. ✅ 中文说明置信度：5/5 — 干净的回归修复，最小 diff，before/after 已验证。 这正是回归修复应有的样子。PR #7090 添加了 steering hint 分支并意外隐藏了 streaming 期间的模式指示器。本 PR 用 6 行生产代码将指示器追加回来，复用已有组件，匹配已有分隔符风格，并添加了一个聚焦的测试（无修复时失败，有修复时通过）。 我的独立方案与作者完全一致——在 streaming 分支内渲染 before/after 单元测试是确定性的：新测试在 main 的 可以合并 ✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 19, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
Author
E2E Verification Reporttmux-based E2E: blocked by pre-existing build issueAttempted to run the CLI in a tmux session for live verification. The current
These are pre-existing issues on Unit test verification: ✅ 25/25 passedThe new test Code-level analysisBefore fix (Footer.tsx lines 109-113): ) : uiState.streamingState === StreamingState.Responding ? (
<Text color={theme.text.secondary}>
{t('Enter to steer · Ctrl+Q to queue')}
</Text> // ← mode indicator unreachable
) : showAutoAcceptIndicator !== undefined ? (
<AutoAcceptIndicator ... />After fix (Footer.tsx lines 109-119): ) : uiState.streamingState === StreamingState.Responding ? (
<Text color={theme.text.secondary}>
{t('Enter to steer · Ctrl+Q to queue')}
{showAutoAcceptIndicator !== undefined && (
<>
{' · '}
<AutoAcceptIndicator approvalMode={showAutoAcceptIndicator} />
</>
)}
</Text>
) : showAutoAcceptIndicator !== undefined ? (
<AutoAcceptIndicator ... />The fix nests Related test suites also pass
|
Collaborator
Review — verified at
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
Comment on lines
+165
to
+167
| it('shows mode indicator alongside steering hint during streaming', () => { | ||
| const { lastFrame } = renderWithWidth( | ||
| 120, |
Collaborator
There was a problem hiding this comment.
[Suggestion] The new test covers the positive path (streaming + ApprovalMode.YOLO), but nothing exercises the negative branch of the new showAutoAcceptIndicator !== undefined guard — streaming with no approval mode set. — Concrete cost: if a future edit removes or inverts that guard, the footer renders a dangling · separator (followed by an empty AutoAcceptIndicator) on every streaming response where no mode is active, and no test fails. A companion test would lock the guard in place.
Suggested change
| it('shows mode indicator alongside steering hint during streaming', () => { | |
| const { lastFrame } = renderWithWidth( | |
| 120, | |
| it('shows mode indicator alongside steering hint during streaming', () => { | |
| const { lastFrame } = renderWithWidth( | |
| 120, | |
| createMockUIState({ | |
| streamingState: StreamingState.Responding, | |
| showAutoAcceptIndicator: ApprovalMode.YOLO, | |
| }), | |
| ); | |
| const frame = lastFrame()!; | |
| expect(frame).toContain('Enter to steer · Ctrl+Q to queue'); | |
| expect(frame).toContain('YOLO mode'); | |
| }); | |
| it('does not show mode indicator during streaming when no approval mode is set', () => { | |
| const { lastFrame } = renderWithWidth( | |
| 120, | |
| createMockUIState({ | |
| streamingState: StreamingState.Responding, | |
| }), | |
| ); | |
| const frame = lastFrame()!; | |
| expect(frame).toContain('Enter to steer · Ctrl+Q to queue'); | |
| expect(frame).not.toContain('YOLO mode'); | |
| expect(frame).not.toContain('Ask permissions'); | |
| expect(frame).not.toContain('auto-accept edits'); | |
| }); |
— qwen3.8-max-preview via Qwen Code /review
Collaborator
Author
|
🤖 Could not address the latest feedback automatically (round 1/5). A human should take over this PR. AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why). Run log: https://github.com/QwenLM/qwen-code/actions/runs/29682544334 |
This was referenced Jul 19, 2026
wenshao
pushed a commit
that referenced
this pull request
Jul 19, 2026
…tion The self-hosted CI runners are heavily oversubscribed (core runs maxThreads: 16), and a recurring class of tests blows vitest's 5s default timeout purely under that contention — not from any logic fault. Observed repeatedly across unrelated PRs (#7213, #7219, and noted in prior sessions): - packages/core/src/utils/shell-ast-parser-lazy.test.ts — fully mocked, but the dynamic import + async coordination exceeds 5s when 16 threads contend. - packages/cli/src/serve/workspace-registration-store.test.ts — tempdir round-trip. - packages/core/src/extension/github.test.ts > extractFile — its waitForFileData helper polled a FIXED 1_000 setImmediate turns, which elapse in <100ms while the tar extraction I/O is still catching up, throwing 'Timed out waiting for extracted data'. Fixes: - testTimeout: 15000 in the core and cli vitest configs — 3x the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a real hang still fails, just later, and the job timeout still bounds it). - waitForFileData now polls a real ~10s wall-clock budget (2_000 x 5ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15s ceiling. These are the deterministic root-cause fixes for the flake class the autofix loop and CI Failure Patrol were papering over with reruns.
wenshao
added a commit
to wenshao/qwen-code
that referenced
this pull request
Jul 19, 2026
…tion (QwenLM#7230) The self-hosted CI runners are heavily oversubscribed (core runs maxThreads: 16), and a recurring class of tests blows vitest's 5s default timeout purely under that contention — not from any logic fault. Observed repeatedly across unrelated PRs (QwenLM#7213, QwenLM#7219, and noted in prior sessions): - packages/core/src/utils/shell-ast-parser-lazy.test.ts — fully mocked, but the dynamic import + async coordination exceeds 5s when 16 threads contend. - packages/cli/src/serve/workspace-registration-store.test.ts — tempdir round-trip. - packages/core/src/extension/github.test.ts > extractFile — its waitForFileData helper polled a FIXED 1_000 setImmediate turns, which elapse in <100ms while the tar extraction I/O is still catching up, throwing 'Timed out waiting for extracted data'. Fixes: - testTimeout: 15000 in the core and cli vitest configs — 3x the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a real hang still fails, just later, and the job timeout still bounds it). - waitForFileData now polls a real ~10s wall-clock budget (2_000 x 5ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15s ceiling. These are the deterministic root-cause fixes for the flake class the autofix loop and CI Failure Patrol were papering over with reruns. Co-authored-by: wenshao <wenshao@example.com>
wenshao
approved these changes
Jul 19, 2026
wenshao
approved these changes
Jul 19, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max-preview via Qwen Code /review
wenshao
added a commit
to yiliang114/qwen-code
that referenced
this pull request
Jul 19, 2026
* feat(autofix): direct takeover of maintainer-fork PRs Maintainer-approved v2: many maintainers work from personal forks, and adoption-snapshotting breaks their local workflow. A fork PR is now directly manageable when three live conditions hold — the takeover label, 'Allow edits from maintainers' (org-owned forks cannot enable it; adoption remains their path), and a fork author who holds write+ RIGHT NOW (the same live-privilege rule as the comment command, so an ex-member's fork can never summon secret-bearing runs). Plumbing: - Scan: fork takeover candidates are admitted per candidate (allow- edits + no-skip filtered in jq; the author's live write+ gate is one permission call each — a rare set); every matrix target now carries its head repo. - Address: prepare fetches the fork branch (origin has no copy) and checks out FETCH_HEAD with hooks already severed; the eligibility gate re-verifies takeover + allow-edits + author write+ live; the report step pushes back to the fork via the allow-edits grant. - Triggers: fork pull_request label events carry NO secrets, so the route notes them and the next scheduled scan engages (≤10m); the comment command now toggles fork PRs too (write+ senders only — fork authors stay silently dropped) and refuses only when allow-edits is missing, with the actionable ask. The scan posts a first-pickup engage ack (identity-verified, deduped on any existing ack, ic.json re-fetched so the same scan counts under the fresh window key) — closing the fork/manual-label ack gap and anchoring the round window. Behavioral coverage: fork-candidate admission jq (allow-edits, skip, in-repo exclusion, tsv rows), eligibility across fork+takeover+allow-edits+write / no-allow-edits / read-author, the toggle's fork split (refusal vs managed), plus plumbing pins (fork fetch/push forms, head_repo threading, first-pickup ack dedup). 60/60 + 12/12. * fix(autofix): strip stray patch-artifact quotes after two fi keywords Two inserted blocks ended 'fi"' — the quotes balanced against each other inside the same script, so bash -n stayed green while runtime would have lexed 'fi' as a command word and swallowed the span between them (the fork head-repo resolution tail and the engage-ack block) into one string. Removed both, and pinned the artifact class in the suite: a lone fi/done/esac followed by a quote now fails the tests. 61/61 + 12/12. * fix(autofix): author-filtered, re-armable first-pickup engage ack Reverse-audit findings on the scan-side ack: - Dedup was a raw grep over ic.json — a forged human comment carrying the engaged marker would have suppressed the real ack (and with it the window anchor). Dedup now selects bot-authored engaged acks via jq, same author rule as the window key itself. - Fork PRs get NO ack job (label events carry no secrets), so the documented re-arm gesture — remove and re-add the label to reset the round window — silently kept the old window: any historical ack blocked a fresh one. When a bot ack already exists, the scan now compares it against the takeover label's latest application time (issue events, fetched only in that rare case); a newer application posts a fresh ack, resetting window and cap as documented. Coverage: verbatim jq replays for both selections (forged-marker and released-marker exclusion, label-name filter, sort|last) plus the lexicographic re-arm gate pin. 61/61 + 12/12. * fix(autofix): review round 1 — ack ordering, dry-run, ghost-engage gate Addresses the maintainer review on QwenLM#7213 (all findings confirmed): - Critical: the first-pickup ack read ic.json BEFORE the per-PR fetch — the first takeover candidate killed the whole scan step (missing file under -eo pipefail; every in-repo label-forced scan regressed), and later candidates dedup'd against the PREVIOUS PR's comments (bot PR ahead → fresh ack every 10min → window reset → cap never binds). The block now sits directly AFTER the fetch; its post-ack re-fetch keeps the downstream MARKERS/window-key reads fresh. A contract pin asserts the fetch precedes the first ack-timestamp read. - Medium: the ack now honors DRY_RUN (log only, window key untouched). - Medium: the command refused forks only for missing allow-edits — a below-write fork author was a silent ghost engagement (label sticks, no ack, nothing ever manages it). The command now mirrors the scan's author write+ gate with an actionable bilingual refusal. Found while fixing it: PR_INFO never fetched maintainerCanModify (or author), so EVERY fork toggle refused regardless of allow-edits — the test stub carried the field and masked the gap. Both engage-side fork gates are now also scoped to 'add': release is never blocked. - Low: the two new paginated jq reads are slurped (add-merged) so >100 comments/events cannot scramble the timestamp comparisons; replays now feed two concatenated page-documents. Fork fetch pins refs/heads/ (tag shadowing); HEAD_REPO_FULL guards each component (deleted fork = owner XOR name empty); fork-rotation caveat documented; forced-path refusal mentions the scheduled fork path. - Security caveat adopted: prepare proves fork push access with a --dry-run push right after checkout (allow-edits rides the classic-PAT grant only) and discards gracefully instead of 403ing after a full agent round. 61/61 + 12/12; YAML parses; every run block passes bash -n. * fix(autofix): fork targets keep base/branch invariants + 3 hardening follow-ups Blocking (yiliang114): the last fork elif ends the eligibility ladder for every eligible fork, so the LIVE_BASE/LIVE_BRANCH re-checks were unreachable for exactly the PR class the loop fetches and pushes — a labeled fork retargeted off main (or head-renamed) between scan and address would have had conflicts resolved against the wrong base. The base/branch invariants now sit ABOVE the fork chain (comment explains why the order is load-bearing), with replay cases pinning a retargeted and a renamed fork to the discard path. Follow-ups from the same review, all adopted: - PR_LIVE re-reads headRepositoryOwner/headRepository; a fork renamed or transferred since the scan discards at the live re-check (moved or unresolved, fail-closed) instead of fetching and token-pushing a stale path. The replay's fork fixture now carries its head repo and the harness provides the matrix HEAD_REPO to compare against. - The fork fetch failure (force-push/rename race) discards through the standard no-action path instead of a red run. - The first-pickup engage ack defers to the in-repo label event's DEDICATED ack job within a 3-minute grace after the label lands, so a concurrent ack job is never double-posted (which would shift the round-window anchor); a failed ack job is still healed by the next scan, and forks (no ack job) keep immediate pickup. Events are read once, before the branch split. Both hooks-order regex windows widened to span the new fork-arm guards (the assertions are about order; one hooksPath site genuinely covers both checkout arms). 61/61 + 12/12. * test: raise timeout ceiling for I/O-bound tests flaky under CI contention The self-hosted CI runners are heavily oversubscribed (core runs maxThreads: 16), and a recurring class of tests blows vitest's 5s default timeout purely under that contention — not from any logic fault. Observed repeatedly across unrelated PRs (QwenLM#7213, QwenLM#7219, and noted in prior sessions): - packages/core/src/utils/shell-ast-parser-lazy.test.ts — fully mocked, but the dynamic import + async coordination exceeds 5s when 16 threads contend. - packages/cli/src/serve/workspace-registration-store.test.ts — tempdir round-trip. - packages/core/src/extension/github.test.ts > extractFile — its waitForFileData helper polled a FIXED 1_000 setImmediate turns, which elapse in <100ms while the tar extraction I/O is still catching up, throwing 'Timed out waiting for extracted data'. Fixes: - testTimeout: 15000 in the core and cli vitest configs — 3x the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a real hang still fails, just later, and the job timeout still bounds it). - waitForFileData now polls a real ~10s wall-clock budget (2_000 x 5ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15s ceiling. These are the deterministic root-cause fixes for the flake class the autofix loop and CI Failure Patrol were papering over with reruns. --------- Co-authored-by: wenshao <wenshao@example.com>
pull Bot
pushed a commit
to bhardwajRahul/qwen-code
that referenced
this pull request
Jul 19, 2026
…QwenLM#7229) * fix(autofix): a no-output crash must not advance the review watermark When a review-address run crashes AFTER prepare (so NEWEST is set) but BEFORE the agent writes any verdict — no address-summary.md, no-action.md, or failure.md — the handoff stamped the marker with ts=NEWEST, advancing the feedback watermark as if the feedback had been evaluated. It hadn't. The next scan then saw 'nothing new since <NEWEST>' and never retried, stranding the PR on a purely transient crash. That is exactly what happened to QwenLM#7219 during the QwenLM#7165 SKILL-staging outage: the run crashed at promptFor (ENOENT) at 09:50, the handoff advanced the watermark to 09:50:56, and even after QwenLM#7225 fixed the crash the loop considered all prior feedback 'addressed' and would not re-engage. Fix: on a no-output crash (NEWEST set, DETAIL_FILE empty) stamp the sentinel ts instead — it is excluded from EVAL_WM, so the watermark does not move and the next scan retries the same feedback. The round still increments, so a PERSISTENT crash is bounded by MAX_ROUNDS and ends in a terminal handoff rather than looping forever. Agent-produced handoffs (verify failed after real output) keep advancing the watermark as before. Replay test extended to assert BOTH MARK_TS and MARK_ROUND across all three shapes: output+verify-fail → advance; no-output crash → sentinel (retry); pre-prepare crash → terminal. 60/60 + 12/12. * fix(autofix): correct the final-attempt crash headline per review Two review findings on the no-output-crash handoff: - The headline promised 'it will retry on the next scan' even on the final attempt, but at MARK_ROUND == MAX_ROUNDS the scan's round-cap gate skips the PR and the cap-reached notice is takeover-only — so a maintainer was told a retry was coming that never comes. The headline now branches: 'it will retry' only while MARK_ROUND < MAX_ROUNDS, otherwise 'this was the last automatic attempt; a human should take over'. - It embedded a Run log URL that the report block already appends to every handoff, duplicating it in the comment. Removed from the headline. Replay test extended: mid-attempt headline promises retry and carries no Run log; final-attempt headline says human-takeover and never 'retry'. 60/60 + 12/12. --------- Co-authored-by: wenshao <wenshao@example.com>
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
When the agent is responding (streaming state), the footer now shows the approval mode name (e.g. "YOLO mode", "Auto mode") alongside the "Enter to steer · Ctrl+Q to queue" steering hint, instead of hiding it entirely.
Why it's needed
PR #7090 introduced a regression where the
StreamingState.Respondingbranch in the Footer'sleftBottomContentternary chain takes priority over theAutoAcceptIndicatorcomponent. This means that during streaming — which is most of the time the agent is working — the mode name text disappears from the footer. Users switching modes with Shift+Tab only see the input prompt border color change, with no text confirmation of the active mode.Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: footer during streaming shows only
Enter to steer · Ctrl+Q to queue— mode name absent.After: footer during streaming shows
Enter to steer · Ctrl+Q to queue · YOLO mode (shift + tab to cycle)— mode name visible with correct color.Tested on
Environment (optional)
Local
npm run devon Linux, unit tests viavitest.Risk & Scope
wrap="truncate"on the parent<Text>will clip the mode indicator tail, which is acceptable.Linked Issues
Fixes #7217
中文说明
这个 PR 做了什么
当 agent 正在响应（streaming 状态）时，footer 现在会在 "Enter to steer · Ctrl+Q to queue" 提示后面同时显示当前审批模式名称（如 "YOLO mode"、"Auto mode"），而不是完全隐藏。
为什么需要
PR #7090 引入了一个回归：Footer 的
leftBottomContent三元链中StreamingState.Responding分支优先于AutoAcceptIndicator组件，导致 streaming 期间模式名称文字从 footer 消失。用户用 Shift+Tab 切换模式时只能看到输入框边框颜色变化，没有文字确认当前模式。风险与范围
wrap="truncate"会截断模式名尾部，可接受。