fix(tools): treat blank task_list filters as absent - #10159
Conversation
Blank (empty or whitespace-only) optional owner/blockedBy params on
task_list activate as filters because listTasks() activates every
filter whose value is !== undefined: blockedBy: '' filters out every
task ("No tasks found."), and owner: '' fails with "Cannot filter by
owner: owner must include at least one letter, number, or hyphen."
Both contradict the tool's own presentation — the schema describes
these params as optional and getDescription() only shows truthy
filter values.
Normalize blank owner/blockedBy to "no filter" in
TaskListInvocation.execute() before calling listTasks(). Non-blank
owner values that sanitize to nothing (e.g. "!!!") keep the existing
error, and the store-level listTasks() !== undefined contract stays
untouched.
Tests: red-to-green coverage for the four blank-filter shapes plus a
regression test for exact non-empty blockedBy filtering.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Re-run at the author's request (
Moving on to code review. 🔍 中文说明应作者要求(
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewHead moved Independent proposal for #9281: normalize blank Verified against the code at this head:
Non-blocking note: TestingUnattended CI run — per the review rules I never build or execute PR code; the evidence below is this PR's own CI on the reviewed head, fetched via the checks API. The unit tests genuinely pin the change: each blank-filter test asserts a populated result where the pre-fix code returned Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 CI on this head has settled fully green — every check completed with success (or a skipped platform variant), including 中文说明代码审查自上次完整审查以来,head 从 针对 #9281 的独立方案:在工具边界 在当前 head 的代码上逐项确认:
非阻塞备注: 测试无人值守 CI 运行——按审查规则我从不构建或执行 PR 代码;以下证据来自 API 拉取的、该 PR 自身在受审 head 上的 CI。单元测试确实钉住了改动:每条空白过滤测试都在旧代码返回 CI 表格见上方英文部分。本 head 的 CI 已全部落定为绿——所有检查以 success 完成(或为跳过的平台变体),包括 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean at every stage; this is exactly the fix I would have written. Reflection against my independent proposal: the PR matches it point for point — blank values normalize to absent at the tool boundary, the store's On the ledger: the Approval has landed: every check on this head completed green, and the deferred approval was posted pinned to the commit below (2026-08-30 20:42 UTC, via the finalize job). This re-run re-verified the head, the CI state, and the standing approval, and added nothing on top — approving twice for the same commit would be noise. 中文说明置信度:5/5 —— 各阶段均干净;这正是我会写的修复。 对照我的独立方案:PR 逐点对齐——空白值在工具边界归一化为"无过滤",存储层 关于历史记录:本 PR 上此前现存的 批准已落地:本 head 的全部检查以绿完成,暂缓的批准已钉住下方提交提交(2026-08-30 20:42 UTC,由 finalize 任务执行)。本次重跑重新核对了 head、CI 状态与已生效的批准,未叠加新批准——对同一提交重复批准只是噪音。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6a": running the new tests in packages/core/src/tools/task-list.test.ts — the worktree and main checkout both lack node_modules and a full npm ci (prepare → build) e….
Test Plan (not a blocker): src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory; 14 tests pass — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed; Tests 14 passed — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed; 10 passed — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed.
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 6a":running the new tests in packages/core/src/tools/task-list.test.ts — the worktree and main checkout both lack node_modules and a full npm ci (prepare → build) e…。
Test Plan(非阻断):src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory; 14 tests pass — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed; Tests 14 passed — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed; 10 passed — this review observed 21913, 1735, 24976, 1662, 605, 4290, 638 passed。
— qwen3.8-max via Qwen Code /review (v0.22.2)
tmux E2E test report (head
|
E2E verification report — head
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory.
中文说明
Test Plan(非阻断):src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Bring in the check:tui-dep-direction and typecheck scripts added on main so the CI gate steps stop failing with "Missing script". Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Blank (empty/whitespace-only) blockedBy values normalized to '' and
were forwarded to listTasks, where the filter activates on
`!== undefined` and `includes('')` never matches — so a populated
board reported "No tasks found." (the exact bug #9281 set out to
fix). Blanks now stay undefined, the store's absent marker.
A non-blank value that normalizes to nothing after the '#' strip
(a bare '#') now fails closed with an explicit error, mirroring the
owner path, instead of silently activating a never-matching filter
while getDescription() still advertises blockedBy=#.
The trim + '#' strip moves into a shared normalizeTaskId helper next
to assertValidTaskId in tasks.ts; its result is still routed through
assertValidTaskId before use. Aligning task-update's ID parameters
on the helper is left as a follow-up.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 41 passed · 0 failed · 41 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:41 通过 · 0 失败 · 41 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10159 — fix(tools): treat blank task_list filters as absentVerdict: 中文摘要
Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
12/12 expectations per arm (24 assertions). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is byte-identical across arms. Type-boundary probes on head dist: Corrections (description vs final code)The PR body describes the first commit; the final commit (
These are description staleness, not code defects — no code change requested. Findings (non-blocking)F1 (low, pre-existing, declared follow-up): F2 (note): on error paths Mutation matrix (vacuity + per-guard pinning)Runner:
Every guard the PR introduces is pinned by its own test; no single-hunk survivor, so no combination row was needed (the owner and blockedBy guards defend distinct hazards). R1 doubles as the red-to-green proof the PR claims. Targeted gates
Not covered
MethodologyEnvironment: CI verify container ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Closeout: current head has no unresolved review threads; the task_list comments are already fixed on the branch. The remaining Ubuntu test failure pointed at unrelated supervisor-process and shellAstParser timing tests, so I merged latest |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
CI attribution —
Triggering a failed-jobs rerun; leaving the infra attribution to maintainers. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/task-list.ts:111 — [probe] Uncapped error-message echo of model-supplied blockedBy in the new assertValidTaskId catch (getErrorMessage caps at 1000 chars; probe measured a 5055-char echo reaching the model context)
中文说明
Test Plan(非阻断):src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 66 passed · 0 failed · 66 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:66 通过 · 0 失败 · 66 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10159 — fix(tools): treat blank task_list filters as absent (follow-up round)Verdict: This is a follow-up round. The previous substantive report verified head 中文摘要
Previous-finding status (follow-up round)
No new findings this round. Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
Base 19/19, head 18/18 (12 cells × expectation + desc observations + 4 type-boundary probes per arm). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is identical across arms. Type-boundary probes (both arms): Store-contract claim (a): Mutation matrix (vacuity + per-guard pinning, re-run at new head)Runner:
R1's reds fail with the behavioral mismatches the tests exist to catch (from Targeted gates (re-run at new head)
Full-suite failure attribution (A/A across arms)The 16 failing files at head: 9 match the previous round's known pre-existing set (logger, ide-client, contentGenerator, config-session-env, skill-activation, skill-manager, memoryDiscovery, file-token-storage, telemetry/sdk). The 7 that differ from the previous round were A/A-tested in isolation on BOTH arms:
For the 3 persistent failers plus the 4 largest known flakes (logger, ide-client, contentGenerator, config-session-env), the failed test-name sets are byte-identical across arms when run isolated together: 49 common, 0 head-only, 0 base-only. Separation check: none of the 16 failing files imports Main-merge delta (this round's new commit)
Not covered
MethodologyEnvironment: CI verify container (node v22.23.2), merge-ref checkout at depth 2, Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
CI attribution —
Combined with the Test ubuntu attribution above (12:22Z), both red required checks at this head are infra/repo-wide, not caused by this PR. CR is stale (0 unresolved threads); leaving to maintainer/infra. |
|
CI attribution — Note: this covers the rerun of the failed jobs, which failed with a new, earlier signature than the original attempt (mid-test kill) attributed above.
Conclusion: runner-environment failure during the install/build phase of |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 108 passed · 0 failed · 108 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:108 通过 · 0 失败 · 108 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 108 passed · 0 failed · 108 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:108 通过 · 0 失败 · 108 总计 Verification reportPR #10159 — fix(tools): treat blank task_list filters as absent (follow-up round 2)Verdict: This is a follow-up round. The previous substantive report verified head 中文摘要
Previous-finding status (follow-up round)
No new findings this round. Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
Base 24/24, head 29/29 expectations met (12 cells × per-cell expectations + desc observations + store cells + 4 type-boundary assertions per arm). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is identical across arms. Store-contract cells (identical expectations on both arms, both pass): Type-boundary probes (both arms, identical): Store-contract claim (a), callers: 7 Mutation matrix (vacuity + per-guard pinning, at new head)Runner:
R1's reds fail with the behavioral mismatches the tests exist to catch (from Targeted gates (at new head)
Full-suite attributionFull Batched A/A: all 21 failing files run in one vitest run per arm (head main tree, base-tree), failing-test-name sets compared per file (
Separation check (scripted): none of the 21 failing files imports Main-merge delta (this round's new commit)
Not covered
MethodologyEnvironment: CI verify container (node v22.23.2), merge-ref checkout at depth 2, Evidence images— Qwen Code · sandboxed verification Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory; 14 tests pass — this review observed 1818 passed; Tests 14 passed — this review observed 1818 passed; 10 passed — this review observed 1818 passed.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/task-list.test.ts:227 — [probe] #-prefixed blockedBy test lacks a negative control (a mutation dropping #N filtering ships green)
中文说明
Test Plan(非阻断):src/tools/task-list.test.ts — no such file or directory; src/agents/team/tasks.test.ts — no such file or directory; 14 tests pass — this review observed 1818 passed; Tests 14 passed — this review observed 1818 passed; 10 passed — this review observed 1818 passed。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 184 passed · 0 failed · 184 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:184 通过 · 0 失败 · 184 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10159 — fix(tools): treat blank task_list filters as absent (follow-up round 3)Verdict: This is a follow-up round with zero delta: 中文摘要
Previous-finding status (follow-up round)
No new findings this round. Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
Base 36/36, head 36/36 assertions passed (12 cells with per-cell expectations incl. desc observations + 4 store cells + 4 type-boundary probes per arm). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is identical across arms. Store-contract cells (identical expectations on both arms, all pass): Type-boundary probes (both arms, identical outcomes): Store-contract claim (a), callers: 7 Mutation matrix (vacuity + per-guard pinning, re-run at this head)Runner:
R1's reds fail with the behavioral mismatches the tests exist to catch ( Targeted gates (re-run at this head)
Full-suite attribution (carried over on a proven-identical input closure)Round 2 ran the full Corrections
FindingsNone new this round. Carried F1/F2/Corr all stand (see status table). Not covered
MethodologyEnvironment: CI verify container (node v22.23.2), merge-ref checkout at depth 2, Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 90 passed · 0 failed · 90 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:90 通过 · 0 失败 · 90 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification report<!-- qwen-triage:verify --> PR #10159 — fix(tools): treat blank task_list filters as absent (follow-up round 4)Verdict: This is a follow-up round with zero delta: 中文摘要
Previous-finding status (follow-up round)
No new findings this round. Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
Base 38/38, head 39/39 assertions passed (12 tool cells with per-cell expectations incl. desc observations + 4 store cells + 4 type-boundary probes + 2 control checks per arm). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is identical across arms. Store-contract cells (identical expectations on both arms, all pass): Type-boundary probes (both arms, identical outcomes): Mutation matrix (vacuity + per-guard pinning, re-run at this head)Runner:
R1's reds fail with the behavioral mismatches the tests exist to catch ( Targeted gates (re-run at this head)
Full-suite attribution (carried over on a proven-identical input closure, separation re-checked fresh)Round 2 ran the full Rather than re-running the ~16-minute load-dependent suite, the separation property was re-established fresh at this head in contrapositive form: the complete set of test files importing either changed module is exactly five — Corrections
FindingsNone new this round. Carried F1/F2/Corr all stand (see status table). Not covered
MethodologyEnvironment: CI verify container (node v22), merge-ref checkout at depth 2, Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
@qwen-code /triage |
Superseded — this CHANGES_REQUESTED came from a malformed triage run on 4ddc36d (2026-08-29) and no longer reflects the review state: full re-reviews on c6e53b6 and 3e058ac found no code findings, CI on 3e058ac settled fully green, sandboxed /verify passed 184/184 assertions on it, and an approval pinned to 3e058ac stands. Dismissing my own stale review so it stops gating the PR.
该评审来自 2026-08-29 在 4ddc36d 上的一次异常运行,已被后续完整复审取代,不再反映当前评审状态(3e058acc 上 CI 全绿、沙箱验证 184/184 通过、已有钉住该提交的批准),现驳回我自己留下的过期评审。
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 189 passed · 0 failed · 189 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:189 通过 · 0 失败 · 189 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10159 — fix(tools): treat blank task_list filters as absent (follow-up round 5)Verdict: This is a follow-up round with zero delta: 中文摘要
Previous-finding status (follow-up round)
No new findings this round. Central claimBlank (empty/whitespace-only) values for the optional A/B load-bearing proofHarness:
Base 94/94, head 82/82 scripted checks passed (12 tool cells with per-check expectations including desc observations + 4 store cells + 4 type-boundary probes + 2 control checks per arm). The four issue shapes (C1–C4) flip broken→fixed; the silent-never-match family (C7, C8, C10, C12) flips silent→correct-or-explicit; every preserved-behavior control (C5, C6, C9, C11) is identical across arms. Store-contract cells (identical expectations on both arms, all pass): Type-boundary probes (both arms, identical outcomes): Mutation matrix (vacuity + per-guard pinning, re-run at this head)Runner:
R1's reds fail with the behavioral mismatches the tests exist to catch — Targeted gates (re-run at this head)
Full-suite attribution (carried over on a proven-identical input closure; separation re-checked fresh)Round 2 ran the full The separation property was re-established fresh at this head in contrapositive form: the complete set of test files importing either changed module is exactly five — Corrections
FindingsNone new this round. Carried F1/F2/Corr all stand (see status table). Not covered
MethodologyEnvironment: CI verify container (node v22.23.2), merge-ref checkout at depth 2, Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
Maintainer verification — real-CLI A/B at head
|
| call | before (3aa1b14) |
after (3e058ac) |
|---|---|---|
task_list({}) |
full board | full board (unchanged) |
task_list({ blockedBy: "" }) |
❌ No tasks found. |
✅ full board |
task_list({ blockedBy: "␣␣␣" }) |
❌ No tasks found. |
✅ full board |
task_list({ owner: "" }) |
❌ Cannot filter by owner: … |
✅ full board |
task_list({ owner: "␣␣␣" }) |
❌ Cannot filter by owner: … |
✅ full board |
task_list({ blockedBy: "1" }) |
#2 only |
#2 only (unchanged) |
task_list({ owner: "!!!" }) |
explicit error | explicit error (guard kept) |
The display layer moved with it: at base, getDescription() for {owner:"␣␣␣"} rendered List tasks (owner= ) — advertising a filter the same call then rejected. At head it renders List all tasks, matching what actually happens.
2. The new tests are load-bearing
Checking out only the PR's two test files onto the merge-base source: Tests 8 failed | 63 passed (71) — 6 in task-list.test.ts, 2 in tasks.test.ts. At PR head: 71 passed (71). Every new assertion is red without the source change.
3. No collateral
npx vitest run src/agents/team src/toolsinpackages/coreat head: 108 files / 3585 tests, all pass.eslint+prettier --checkclean on all four changed files.npm run typecheck:coreandcliclean. The one red workspace,integrations/external-context-mem0, fails with identical errors at the merge-base — pre-existing and unrelated to this PR.- The store-level
listTasks()activation contract is untouched, and theblockedByfilter has exactly one caller (task-list.ts), so no other consumer moves. - CI on this PR: 16 pass, 0 fail.
Findings
① The PR description is stale — please update before merge. (the only thing I'd block on, and it's prose, not code)
The body describes commit 67d9402 and predates 4ddc36d / c272661. Three concrete mismatches:
- It says "non-blank
blockedByvalues are passed through unchanged". They are not: the final code runsnormalizeTaskId()(trim + strip one leading#) and thenassertValidTaskId(), so#1now resolves to1, and#/task-1/01now return a tool error where they previously returnedNo tasks found. - Because of that, "Breaking changes / migration notes: none" understates the change:
task_listcan now surface an error on an input that previously produced an empty list. I think that behavior is the right call — failing closed and loudly beats a silent empty board, which is the same principle as the fix itself — but it should be stated. - The test counts are off:
task-list.test.tshas 15 tests (not 14) with 6 reds at base (not 4), andtasks.test.tshas 56 (not 54), because the PR itself addsnormalizeTaskIdand its two tests.
② Follow-up, non-blocking: #N is now accepted by task_list only. Reproduced on the real CLI at head (bottom half of the second screenshot): task_update({taskId:"#1"}), task_update({addBlockedBy:["#1"]}) and task_update({taskId:"␣1␣"}) all still fail with Invalid task ID. Since task_list's own output renders IDs as #1, a model that copies the rendered form now succeeds in one tool and fails in its neighbours. normalizeTaskId is exported from the store layer with a doc comment about "callers", but has exactly one. The errors are loud and self-correcting, so this is a follow-up, not a merge blocker — the commit message already declares it as such.
③ Out of scope, FYI: task_list({ owner: null }) — a very common way for a model to serialize an absent optional, the same defect class as #9281 — is still rejected upstream with params/owner must be string. No change inside the tool can reach that; it would need a schema change. Worth a separate issue rather than growing this PR.
Verdict
Approve. The fix is minimal, correct at the real tool boundary, tested with load-bearing regression tests, and clean on lint/typecheck/suite. Merge once the description is refreshed to match the shipped code; findings ② and ③ are follow-ups.
中文完整版
维护者验证 —— 在 head 3e058ac 上的真实 CLI A/B
我在本地搭建了验证环境,用真实 qwen CLI 跑通了这个 PR,而不只是跑单测。结论先行:修复正确、可以合入 —— 但请在合入前更新 PR 描述,因为它描述的是第一个 commit,而不是实际要发布的代码。
环境
- 在 merge-base
3aa1b14与 PR head3e058ac各建 worktree;从 PR worktree 完整构建core+cli;以node packages/cli -p … --auth-type openai --approval-mode yolo --output-format json运行。 - 模型侧是脚本化的 mock OpenAI provider,因此每一种
task_list参数形态都是确定性发出的,模型收到的工具返回直接从 provider 传输记录中逐字提取。 - 真实 team 运行时:
QWEN_CODE_ENABLE_AGENT_TEAM=1、每臂独立QWEN_HOME、通过真实工具建出真实的磁盘任务板 ——team_create→ 3 ×task_create→task_update(#2, addBlockedBy:["1"])。 - 两臂只替换 PR 触及的两个编译产物(
dist/src/agents/team/tasks.js、dist/src/tools/task-list.js)。工具链、node_modules、任务板完全相同。
这比本 PR 上已有的沙箱轮次高一层:那些直接驱动 TaskListTool,而这次走的是真实工具注册表、schema 校验以及 模型→provider→工具 的完整回路。结论一致,但是独立得到的。
1. 缺陷在真实工具边界上复现,并被修复
#9281 的四种复现形态在 base 臂全部失败、在 PR 臂全部正确,且对照组不动:
| 调用 | 修复前(3aa1b14) |
修复后(3e058ac) |
|---|---|---|
task_list({}) |
完整任务板 | 完整任务板(不变) |
task_list({ blockedBy: "" }) |
❌ No tasks found. |
✅ 完整任务板 |
task_list({ blockedBy: "␣␣␣" }) |
❌ No tasks found. |
✅ 完整任务板 |
task_list({ owner: "" }) |
❌ Cannot filter by owner: … |
✅ 完整任务板 |
task_list({ owner: "␣␣␣" }) |
❌ Cannot filter by owner: … |
✅ 完整任务板 |
task_list({ blockedBy: "1" }) |
仅 #2 |
仅 #2(不变) |
task_list({ owner: "!!!" }) |
显式报错 | 显式报错(守卫保留) |
展示层也跟着一起修好了:base 上 {owner:"␣␣␣"} 的 getDescription() 渲染为 List tasks (owner= ) —— 宣告了一个同一次调用随后又拒绝掉的过滤器。head 上渲染为 List all tasks,与实际行为一致。
2. 新增测试确实承重
只把 PR 的两个测试文件签出到 merge-base 源码上:Tests 8 failed | 63 passed (71) —— task-list.test.ts 6 条、tasks.test.ts 2 条。在 PR head 上:71 passed (71)。没有源码改动时,每一条新断言都是红的。
3. 无附带影响
- head 上
packages/core执行npx vitest run src/agents/team src/tools:108 个文件 / 3585 条测试全绿。 - 四个改动文件的
eslint与prettier --check均干净。 npm run typecheck:core与cli干净。唯一飘红的 workspaceintegrations/external-context-mem0在 merge-base 上报完全相同的错误 —— 属于既存问题,与本 PR 无关。- 存储层
listTasks()的激活契约未改动,且blockedBy过滤器只有一个调用方(task-list.ts),因此没有其他消费者受影响。 - 本 PR 的 CI:16 通过,0 失败。
结论清单
① PR 描述已过期 —— 请在合入前更新。(唯一需要卡的点,而且只是文字,不是代码)
描述对应的是 commit 67d9402,早于 4ddc36d / c272661。三处具体不符:
- 描述称 "非空
blockedBy值原样透传"。实际并非如此:最终代码会执行normalizeTaskId()(trim + 去掉一个前导#),再执行assertValidTaskId(),因此#1现在会解析为1,而#/task-1/01现在会返回工具错误,而此前返回的是No tasks found. - 因此,"破坏性变更 / 迁移说明:无" 低估了这次变化:
task_list现在会对此前只返回空列表的输入抛出错误。我认为这个行为是对的 —— 快速且显式地失败胜过静默的空任务板,这与修复本身的原则一致 —— 但应当写明。 - 测试数量对不上:
task-list.test.ts现在是 15 条(不是 14),base 上红 6 条(不是 4);tasks.test.ts是 56 条(不是 54),因为 PR 自己新增了normalizeTaskId及其两条测试。
② 后续项,不阻塞:#N 目前只有 task_list 接受。 已在 head 的真实 CLI 上复现(第二张截图下半部分):task_update({taskId:"#1"})、task_update({addBlockedBy:["#1"]}) 与 task_update({taskId:"␣1␣"}) 仍然以 Invalid task ID 失败。由于 task_list 自身的输出就把 ID 渲染成 #1,模型照抄这个展示形态时,会出现"在一个工具里成功、在相邻工具里失败"的割裂。normalizeTaskId 从存储层导出、注释里写着"callers",但实际只有一个调用方。报错足够响亮且模型可自纠,所以这是后续项而非合入阻塞 —— commit message 里也已声明为 follow-up。
③ 范围外,供参考: task_list({ owner: null }) —— 模型序列化"缺省可选参数"时非常常见的一种写法,与 #9281 属于同一缺陷类 —— 仍会在上游被 params/owner must be string 拒绝。这一点在工具内部无法修复,需要改 schema。建议另开 issue,而不是把本 PR 撑大。
判定
Approve。 修复足够小、在真实工具边界上正确、有承重的回归测试,lint / typecheck / 测试套件均干净。把描述更新到与实际代码一致后即可合入;② 和 ③ 作为后续项处理。



















What this PR does
Blank (empty or whitespace-only) values for the optional
ownerandblockedByparameters oftask_listare now treated as "no filter" instead of being forwarded as active filters. The normalization happens once inTaskListInvocation.execute()before callinglistTasks(); non-blank owner values that sanitize to nothing (e.g.!!!) keep the existing explicit error, and non-blankblockedByvalues are passed through unchanged.Why it's needed
The tool's schema describes both parameters as optional and
getDescription()only shows truthy filter values, so callers reasonably expect a blank value to mean "don't filter". Before this fix the behavior contradicted that contract:blockedBy: ''activated a filter that matches nothing and silently returnedNo tasks found.even when matching tasks existed, andowner: ''failed withCannot filter by owner: owner must include at least one letter, number, or hyphen.Fixes #9281.Reviewer Test Plan
How to verify
Run the tool-level tests in
packages/core:npx vitest run src/tools/task-list.test.ts src/agents/team/tasks.test.ts. The newblank filters are treated as absent (#9281)block covers the four reproduction shapes (owner: '',owner: ' ',blockedBy: '',blockedBy: ' ') plus a regression test that non-emptyblockedBystill filters exactly. Before the source change, the four blank-filter tests fail (emptyblockedBy→No tasks found.; blankowner→ the explicit owner error); after the change all 14 tests pass. Store-layer suitetasks.test.tspasses 54/54 (thelistTasks()!== undefinedcontract is untouched).npm run typecheckinpackages/coreexits 0;eslintandprettierare clean on both changed files.Evidence (Before & After)
N/A — no UI change. Test-level before/after: before the fix
vitest run src/tools/task-list.test.tsreportsTests 4 failed | 10 passed (14); after the fixTests 14 passed (14).Tested on
Environment (optional)
Unit tests only (
vitestunder Node v24), no runtime/TUI session.Risk & Scope
owner) or returning an empty list (blockedBy); this matches the documented optional-filter semantics, and blank values never had meaningful filter semantics to preserve.listTasks()callers (e.g. TeamManager) are unaffected because the store-level activation contract is unchanged; other tools with optional filters are out of scope.Linked Issues
Fixes #9281. The community PR #9286 targeted the same bug but was closed unmerged by its author; no other open fix exists.
中文说明
本 PR 做了什么
task_list的可选参数owner和blockedBy传入空白(空串或纯空白)值时,现在会被视为"不过滤",而不是被原样转发为激活的过滤条件。归一化在TaskListInvocation.execute()调用listTasks()之前单点完成;非空但 sanitize 后为空的 owner 垃圾值(如!!!)保留现有的显式报错,非空blockedBy值原样透传。为什么需要
工具的 schema 将这两个参数描述为可选,且
getDescription()只展示 truthy 的过滤值,因此调用方有理由认为空白值表示"不过滤"。修复前的行为与该契约矛盾:blockedBy: ''会激活一个永远匹配不到任何任务的过滤条件,即使存在匹配任务也静默返回No tasks found.;owner: ''则报错Cannot filter by owner: owner must include at least one letter, number, or hyphen.。修复 #9281。审阅者测试计划
如何验证
在
packages/core中运行工具层测试:npx vitest run src/tools/task-list.test.ts src/agents/team/tasks.test.ts。新增的blank filters are treated as absent (#9281)测试块覆盖四种复现形态(owner: ''、owner: ' '、blockedBy: ''、blockedBy: ' '),另有一条回归测试验证非空blockedBy仍精确过滤。源码修改前,四条空白过滤测试失败(空blockedBy→No tasks found.;空白owner→ 显式 owner 报错);修改后全部 14 条通过。存储层套件tasks.test.ts54/54 通过(listTasks()的!== undefined契约未改动)。packages/core中npm run typecheck退出码为 0;两个改动文件的eslint与prettier均干净。证据(修改前后)
N/A —— 无 UI 变化。测试层面的前后对比:修复前
vitest run src/tools/task-list.test.ts输出Tests 4 failed | 10 passed (14);修复后输出Tests 14 passed (14)。测试环境
运行环境(可选)
仅单元测试(Node v24 下的
vitest),未运行 TUI 会话。风险与范围
owner)或返回空列表(blockedBy);这与文档化的可选过滤语义一致,且空白值本来就不存在有意义的过滤语义需要保留。listTasks()调用方(如 TeamManager)不受影响,因为存储层的激活契约未变;其他带可选过滤器的工具不在本次范围内。关联 Issue
修复 #9281。社区 PR #9286 曾针对同一问题,但已被作者关闭未合并;目前没有其他打开的修复。