fix(cli): ignore expired live agents in focus navigation#5070
Conversation
| ) { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
[Suggestion] After the defensive auto-defocus fires (all visible agents have expired), non-printable keys are silently consumed via return true without any fallback navigation. If the user pressed Down arrow expecting to reach the tab bar, the keystroke is lost — focus is released back to the composer but doesn't descend further.
Consider re-routing the key instead of swallowing it:
| return true; | |
| return descendFromComposer(); |
Or at minimum return false for arrow keys so they fall through to the normal handler. The printable-character path (return false) is already correct.
— qwen3.7-max via Qwen Code /review
| setBgSelectedIndex(agentIdx); | ||
| const entry = visibleBgAgents[agentIdx]; | ||
| const entryIdx = entry | ||
| ? bgEntries.findIndex( |
There was a problem hiding this comment.
[Suggestion] This new findIndex remapping — mapping visibleBgAgents[agentIdx] back to the bgEntries index via agentId match — has no test coverage with a mixed roster. A regression here would silently open the wrong agent's detail view or fail to open any.
Suggested test: bgEntries = [shellTask, expiredAgent, runningAgent] with livePanelSelectedIndex: 2 (pointing at the running agent in the visible list). Press Enter and assert setSelectedIndex was called with the correct bgEntries index (2, not 0).
— qwen3.7-max via Qwen Code /review
| // Gate panel focus on the panel's own render condition (`kind === 'agent'`), | ||
| // not `bgEntries.length` (which also counts shell/monitor/dream tasks). | ||
| const hasBgAgentRoster = bgEntries.some((e) => e.kind === 'agent'); | ||
| const hasBgAgentRoster = () => |
There was a problem hiding this comment.
[Suggestion] hasBgAgentRoster was changed from a boolean to an arrow function, but the has* prefix conventionally implies a boolean value. A future maintainer writing if (hasBgAgentRoster) (truthy check on the function reference) would always get true, silently breaking the Up-arrow guard.
Consider renaming to signal it's callable:
| const hasBgAgentRoster = () => | |
| const hasVisibleBgAgentRoster = () => | |
| bgEntries.some((e) => isLiveAgentPanelVisibleEntry(e, Date.now())); |
Then update the call site at line 91 to hasVisibleBgAgentRoster().
— qwen3.7-max via Qwen Code /review
✅ Local real-TUI verification (maintainer)Follow-up to the real-TUI verification done for #4911 — same approach, applied to this fix for #5067. Verified locally in a git worktree off the PR head ( Verdict: the fix works end-to-end and the new tests genuinely guard it. No regressions found. LGTM to merge. Environment
1. PR "To verify" commands — all green
(The 15 2. Mutation / guard test — the new tests really catch the bugReverted only the 3 source files to the pre-fix parent ( Exactly the 3 new 3. Real-TUI A/B in
|
| 命令 | 结果 |
|---|---|
npm test(3 个测试文件) |
3 文件通过 · 188 passed, 1 skipped |
npm run typecheck |
exit 0 |
npm run lint -- --max-warnings 0 |
exit 0 |
(完整 npm run build 时出现的 15 个 curly warning 全在 packages/vscode-ide-companion/*,是既有的、与本 PR 无关;@qwen-code/qwen-code 包在 --max-warnings 0 下零 warning。)
2. 变异 / 守护测试 —— 证明新测试真能抓到 bug
把仅 3 个源文件回退到修复前的父提交(d967ecf^),保留 PR 的测试后重跑:恰好 3 个 #5067 新用例失败,其余 185 个仍通过(说明对既有测试的改动向后兼容);恢复修复后 188 全过。这证明这些测试非空壳,且正是该修复让它们由挂变绿。
3. tmux 真实 TUI A/B —— 修复前复现症状 1,修复后消失
我同时构建了 fixed 二进制和 pre-fix 二进制(回退 3 个源文件后重建),在两者上跑完全相同的步骤。
前置条件(两个构建一致): /fork → agent 出现在 LiveAgentPanel,运行、完成,8 秒 TERMINAL_VISIBLE_MS 窗口过后该行老化消失 —— 面板渲染为空,但条目仍被保留(1 task done)。这正是 #5067 的场景:门控仍计数它,面板已不再渲染它。
该症状是静默的(焦点跑到隐藏面板,界面无变化),所以我借助 composer 的"连按两次 Esc 清空"提示来让它可观测:输入 ABC → ↓ → Esc,谁持有焦点谁就会响应这个 Esc。
- Fixed:
↓是 no-op,焦点留在 composer,第一个Esc就触发 "Press Esc again to clear." - Pre-fix:
↓静默聚焦隐藏面板,第一个Esc被吞(状态栏不变),需要第二个Esc才出现清空提示。
即"死键":修复前需按 2 次 Esc,修复后只需 1 次 —— 正是 issue 描述的症状 1("下一个 ↑/↓/Esc/Enter 被隐藏面板 handler 静默吞掉"),在真实二进制里复现并被修复消除。
备注(非阻断)
- 症状 2(幽灵选择槽 /
maxIdx)没有专门的回归测试。 三个新测试覆盖的是进入门控(composer 的↓、tab bar 的↑)和面板侧焦点自动释放。maxIdx = visibleBgAgents.length以及"可见索引→bgEntries"的重映射(按agentIdfindIndex)实现正确、被间接覆盖,但没有用例专门构造一个可见 + 一个过期的 agent 来断言"多出来的那一下↓"幽灵槽已消失。补一个小测试可锁住 issue 的后半部分。 - 设计与 issue 提议方向一致:一个共享纯函数(
liveAgentPanelVisibility.ts)被三处调用方共用(门控不会再和渲染漂移)、门控采用按键时刻快照、面板侧用useEffect兜底——当 roster 在用户脚下老化时释放焦点。
|
@qwen-code /triage |
|
Thanks for the update, @he-yufeng! Template still partial (no What/Why/Test Plan/Risk/中文说明 sections) — same note as before, not blocking since the code and maintainer verification speak for themselves. On direction: unchanged — clearly aligned. Fixes #5067, a real focus-navigation bug. No concerns. On approach: the updated PR addresses all three review suggestions from the first pass:
The scope is still proportional — +276/-45 across 7 files. Moving on to code review. 🔍 中文说明感谢更新！ 模板仍不完整（缺 What/Why/Test Plan/Risk/中文说明），与上次相同，不阻断。 方向：不变，明确对齐，修复 #5067 的焦点导航 bug。 方案：更新后的 PR 解决了首轮审查的三个建议：
范围仍然成比例。进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code Review (re-run — updated PR)Independent proposal (before reading the diff): Same as before — extract the panel's visibility predicate into a shared pure function, use it in all gate call sites, add a panel-side effect to release stale focus. The updated PR should additionally address: callable naming, non-printable key fallthrough, and a mixed-roster regression test. Assessment: The updated implementation matches the independent proposal and addresses all three review suggestions. Changes since the first pass:
No correctness bugs. No regressions. No AGENTS.md violations. TestingUnit tests: 3 files passed · 190 passed | 1 skipped (191) — up from 188 with 2 new tests Typecheck: Tmux dev build: Interactive focus-navigation test: this bug requires Arena mode + background sub-agent spawn + 8s wait for the visibility window to expire — a timing-dependent scenario that can't be meaningfully automated with 中文说明代码审查（重新运行 — 更新后的 PR）独立方案： 与之前相同。更新后的实现匹配独立方案并解决了三个审查建议：
无正确性 bug，无回归，无 AGENTS.md 违规。 测试
— Qwen Code · qwen3.7-max |
|
The updated PR is a strict improvement over the original. All three review suggestions were addressed cleanly:
190 tests pass (up from 188), typecheck and lint both clean. The maintainer's earlier A/B verification already proved the fix end-to-end, and the code hasn't changed in any way that would invalidate that evidence. Approving. ✅ 中文说明更新后的 PR 是原始版本的严格改进。三个审查建议都已干净解决：
190 个测试通过（比上轮多 2 个），类型检查和 lint 均干净。批准合并 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
he-yufeng
force-pushed
the
fix/live-agent-rendered-roster
branch
from
d967ecf to
650dd25
Compare
|
Addressed the three review suggestions in the latest force-push. Updates:
Validated on Windows:
Notes: one earlier attempt ran two coverage-enabled Vitest targets in parallel and hit the known coverage temp-file race after tests had passed; rerunning the target by itself passed. After rebasing onto the latest |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, updated code addresses all review feedback. ✅
Re-verification at the new head (merge reference)This supersedes my earlier real-TUI report, which tested head Headline: the LGTM holds at the new head, the three force-push changes are correct, and the symptom-2 gap I flagged last time (phantom What changed since
|
| Command | Result |
|---|---|
vitest run on the 3 test files |
3 files passed · 190 passed, 1 skipped |
npm run typecheck --workspace @qwen-code/qwen-code |
exit 0 |
npm run lint -- --max-warnings 0 |
exit 0 |
prettier --check (7 changed files) + git diff --check |
clean |
(190 vs the 188 from my d967ecf run = the two extra force-push tests.)
2. Mutation test — the new tests are (mostly) non-vacuous
Reverted the source files to the pre-fix parent 0db3273174 (helper deleted) while keeping the PR's tests, then re-ran:
× AgentTabBar › Up ignores terminal bg agents after the live panel visibility window (#5067)
× LiveAgentPanel › releases focus when the selected terminal row has aged out (#5067)
× InputPrompt › arrow Down skips terminal bg agents after the live panel visibility window (#5067)
× InputPrompt › Enter on the live panel maps visible agents back to their bg entry index ← closes my symptom-2 gap
✓ InputPrompt › Down from an expired live panel falls through to the tab bar ← passes on pre-fix too
Tests 4 failed | 186 passed | 1 skipped
4 of the 5 new tests fail on pre-fix → they are real regression guards. The important one for me: the mixed-roster Enter test (entries = [shell, running, expired, running], selectedIndex = 2, asserts setBgSelectedIndex(3) — the correct bgEntries index, not the visible index 1) fails on pre-fix and passes on the fix. That is exactly the symptom-2 / phantom-slot half of #5067 I noted was untested last time. Gap closed.
One honest caveat (non-blocking): the Down from an expired live panel falls through to the tab bar test passes on pre-fix too, so it documents intent but does not catch the regression. With its setup (livePanelSelectedIndex: 1, one expired agent), pre-fix computes maxIdx = bgAgentCount = 1, so 1 < 1 is false and the existing "bottom of panel → descend" branch already fires setLivePanelFocused(false) + setAgentTabBarFocused(true) — the same observable result as the new fall-through path. Using livePanelSelectedIndex: 0 (where pre-fix would instead navigate into the phantom slot) would make it discriminating. Minor test-strength nit, not a code issue.
3. Real-TUI A/B in tmux at the new head — symptom 1 reproduced on pre-fix, gone on the fix
Built both binaries from 650dd25c02 (fixed) and 0db3273174 (pre-fix — the 3 source files reverted + helper removed, re-bundled), and ran the identical steps on each: /fork a background agent → it completes → after the 8 s TERMINAL_VISIBLE_MS window the row ages out (panel renders nothing, status bar still shows · 1 task done) → then ABC, ↓, Enter. Whoever owns focus decides whether Enter submits.
Stable observable: does Enter submit the ABC draft? (composer focused → yes; phantom panel focused → swallowed).
Fixed — ↓ is a no-op, the composer keeps focus, the first Enter submits:
* ABC ← after typing
* ABC · 1 task done ← after ↓ (no-op, no phantom focus)
> ABC … ✦ done ← after Enter #1 ✅ submitted
Pre-fix — ↓ silently focuses the hidden empty panel, the first Enter is swallowed:
* ABC · 1 task done ← after ↓ (no visual change — phantom focus on aged-out panel)
* ABC · 1 task done ← after Enter #1 ❌ swallowed by the panel, NOT submitted
> ABC … ✦ done ← after Enter #2 (only now does the composer submit)
So a dead key: pre-fix needs 2× Enter, the fix needs 1× — precisely symptom 1 ("the next ↑/↓/Esc/Enter is silently swallowed by the hidden panel's handler"), reproduced in the real binary at the new head and eliminated by the fix.
Verdict
Re-confirmed at 650dd25c02: the fix works end-to-end, the focus logic is green under unit + mutation testing, the design keeps one source of truth, and the previously-untested symptom-2 path now has a genuine (non-vacuous) regression test. The only nit is that one of the two added tests (the expired-panel Down fall-through) is non-discriminating as written. LGTM to merge.
🇨🇳 中文版（点击展开）
在新 head 上的复验（合并参考）
本评论取代我此前的真实 TUI 报告（当时测的是 head d967ecf）。该提交已被强推覆盖；分支已 rebase 到 0db3273174，并在强推中处理了三条 triage 建议。我在当前 head 650dd25c02 上、在 Linux（Node 22.22.2）重新构建出真实 qwen 二进制，通过 tmux 驱动验证。
要点：新 head 上 LGTM 依然成立，三处强推改动正确，且我上次指出的"症状 2 缺口"（幽灵 maxIdx 槽无专门测试）现已被新增的 mixed-roster Enter 测试补上——我用变异测试确认了它非空壳。
相对 d967ecf 的变化（已确认存在于 head）
- 重命名
hasBgAgentRoster→hasVisibleBgAgentRoster()（现为按键时调用的函数）。packages/cli/src中无任何对旧名或已删除的bgAgentCount的悬挂引用。 - 行为变化：当 live 面板被聚焦但可见 roster 已清空时，非可打印导航键现在会释放焦点并经
descendFromComposer()下沉，而不再被吞掉（可打印单字符仍通过return false输入到 composer）。 - 新测试：一个 mixed-roster Enter 重映射测试（shell + 过期项与可见 agent 交错）和一个"从过期面板按 Down"测试。
共享谓词 isLiveAgentPanelVisibleEntry 被三处调用方共用（InputPrompt 的 ↓ 门控、AgentTabBar 的 ↑ 门控、LiveAgentPanel 渲染 + 焦点释放 effect）——单一真相源，门控不会再和渲染漂移。
1. PR "To verify" —— 新 head 上全绿
| 命令 | 结果 |
|---|---|
vitest run 3 个测试文件 |
3 文件通过 · 190 passed, 1 skipped |
npm run typecheck |
exit 0 |
npm run lint -- --max-warnings 0 |
exit 0 |
prettier --check（7 个改动文件）+ git diff --check |
干净 |
（190 vs 我在 d967ecf 上的 188 = 强推新增的两个测试。）
2. 变异测试 —— 新测试（大部分）非空壳
把源文件回退到修复前父提交 0db3273174（删除 helper），保留 PR 测试后重跑：
× AgentTabBar › Up ignores terminal bg agents … (#5067)
× LiveAgentPanel › releases focus when the selected terminal row has aged out (#5067)
× InputPrompt › arrow Down skips terminal bg agents … (#5067)
× InputPrompt › Enter on the live panel maps visible agents back to their bg entry index ← 补上我指出的症状 2 缺口
✓ InputPrompt › Down from an expired live panel falls through to the tab bar ← 在 pre-fix 上也通过
Tests 4 failed | 186 passed | 1 skipped
5 个新测试中 4 个在 pre-fix 上失败 → 它们是真正的回归守护。 对我最关键的是 mixed-roster Enter 测试（entries = [shell, running, expired, running]、selectedIndex = 2，断言 setBgSelectedIndex(3)——正确的 bgEntries 索引，而非可见索引 1）在 pre-fix 上失败、在修复后通过。这正是上次我说没被测到的 #5067 症状 2 / 幽灵槽那一半。缺口已补。
一个诚实的提醒（不阻断）：Down from an expired live panel falls through to the tab bar 这个测试在 pre-fix 上也通过，因此它只是记录了预期行为，并不能抓到回归。按其设置（livePanelSelectedIndex: 1、一个过期 agent），pre-fix 算出 maxIdx = bgAgentCount = 1，于是 1 < 1 为假，既有的"面板底部 → 下沉"分支已经触发 setLivePanelFocused(false) + setAgentTabBarFocused(true)——与新的 fall-through 路径同样的可观测结果。若改用 livePanelSelectedIndex: 0（此时 pre-fix 会改为进入幽灵槽）就能让它具备区分力。属于测试强度的小瑕疵，不是代码问题。
3. tmux 真实 TUI A/B（新 head）—— 修复前复现症状 1，修复后消失
用 650dd25c02（fixed）和 0db3273174（pre-fix——回退 3 个源文件 + 删除 helper 后重新 bundle）分别构建二进制，在两者上跑完全相同的步骤：/fork 一个后台 agent → 完成 → 8 秒 TERMINAL_VISIBLE_MS 窗口过后该行老化（面板不再渲染，状态栏仍显示 · 1 task done）→ 然后 ABC、↓、Enter。谁持有焦点，谁决定 Enter 是否提交。
稳定观测点：Enter 是否提交 ABC 草稿？（composer 持焦 → 提交；幽灵面板持焦 → 被吞）。
Fixed —— ↓ 是 no-op，composer 保持焦点，第一个 Enter 即提交：
* ABC ← 输入后
* ABC · 1 task done ← ↓ 之后（no-op，无幽灵焦点）
> ABC … ✦ done ← Enter #1 之后 ✅ 已提交
Pre-fix —— ↓ 静默聚焦隐藏的空面板，第一个 Enter 被吞：
* ABC · 1 task done ← ↓ 之后（界面无变化——焦点落在已老化的面板上）
* ABC · 1 task done ← Enter #1 之后 ❌ 被面板吞掉，未提交
> ABC … ✦ done ← Enter #2 之后（此时 composer 才提交）
即"死键"：pre-fix 需按 2 次 Enter，修复后只需 1 次——正是症状 1（"下一个 ↑/↓/Esc/Enter 被隐藏面板 handler 静默吞掉"），在新 head 的真实二进制里复现并被修复消除。
结论
在 650dd25c02 上再次确认：修复端到端有效，焦点逻辑在单元 + 变异测试下全绿，设计保持单一真相源，且此前未被测到的症状 2 路径现在有了真正（非空壳）的回归测试。唯一小瑕疵是新增两个测试中的一个（过期面板 Down fall-through）按当前写法不具区分力。建议合并。
3 participants
Fixes #5067.
Summary
To verify