Skip to content

fix(team): normalize blank task list filters - #9286

Closed
netbrah wants to merge 1 commit into
QwenLM:mainfrom
netbrah:pr-team-task-list-filters
Closed

fix(team): normalize blank task list filters#9286
netbrah wants to merge 1 commit into
QwenLM:mainfrom
netbrah:pr-team-task-list-filters

Conversation

@netbrah

@netbrah netbrah commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Treats literal empty owner and blockedBy values passed to team task listing as omitted optional filters.

Why it's needed

A model can serialize an optional task-list filter as an empty string. Previously that value reached exact storage filtering while the tool presentation treated it as absent, causing a populated task board to appear empty.

Reviewer Test Plan

How to verify

Create a team with a pending task. Request pending tasks with an empty owner value and separately with an empty blockedBy value. Both requests should list the pending task. A nonempty owner filter should remain selective.

Evidence (Before & After)

Before: either blank optional filter returned No tasks found. for an otherwise matching pending task. After: literal blank filters are omitted and the matching task is listed.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS package-local Core typecheck and Vitest.

Risk & Scope

  • Main risk or tradeoff: only literal empty strings are normalized; whitespace-only values and storage-layer exact-match semantics remain unchanged.
  • Not validated / out of scope: interactive model serialization and nonempty filter behavior beyond existing unit coverage.
  • Breaking changes / migration notes: none.

Linked Issues

Fixes #9281

中文说明

此 PR 的改动

将团队任务列表中传入的字面量空 ownerblockedBy 值视为省略的可选过滤条件。

为什么需要它

模型可能把可选任务列表过滤条件序列化为空字符串。此前该值会进入精确的存储过滤逻辑,但工具展示又将其视为未提供,从而使有任务的看板显示为空。

审阅者测试计划

如何验证

创建一个包含 pending 任务的团队。分别使用空 owner 和空 blockedBy 请求 pending 任务。两种请求都应列出该 pending 任务。非空 owner 过滤条件仍应保持选择性。

证据(前后对比)

修复前:任一空可选过滤条件都会让原本匹配的 pending 任务返回 No tasks found.。修复后:字面量空过滤条件会被省略,并列出匹配任务。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS 上的包级 Core typecheck 和 Vitest。

风险与范围

  • 主要风险或权衡:仅归一化字面量空字符串;纯空白字符串和存储层的精确匹配语义保持不变。
  • 未验证 / 超出范围:交互式模型序列化,以及现有单元覆盖范围之外的非空过滤行为。
  • 破坏性变更 / 迁移说明:无。

关联问题

Fixes #9281

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 16, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 9cbb5c2 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 9cbb5c2 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug, not theoretical. #9281 carries a deterministic reproduction (evidence branch with failing regression assertions), and the reported root cause matches the code as it stands: getDescription() uses truthy checks, so a blank filter is displayed as absent, while listTasks() activates any value that is merely !== undefined — so owner: '' becomes a literal comparison and blockedBy: '' an includes('') check that never matches. A populated board genuinely comes back as No tasks found.

Direction: aligned. This is the team task-list tool misreporting its own state — the fix makes runtime behavior match the contract the tool already presents. No auth/sandbox/public-contract surface involved.

Size: touches packages/core/src/tools/ (core path): 6 production lines (task-list.ts) vs. 52 test lines (task-list.test.ts). Far below any size threshold.

Approach: the scope feels right. Normalizing at createInvocation puts the fix at the single choke point every invocation passes through, and it deliberately leaves listTasks() storage semantics untouched for other callers. Literal empty strings only — whitespace-only values stay active — is a defensible cut, since the display/behavior mismatch only exists for the falsy '' case (a whitespace string is truthy, so it displays and filters consistently). The alternatives discussed in the issue (schema rejection, storage-layer normalization) would both be larger behavioral changes.

Risk: no elevated risk signals — none of the changed files match the repo's revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,而非理论性问题。#9281 提供了确定性复现(证据分支上有失败的回归断言),且报告的根因与当前代码一致:getDescription() 使用真值判断,空白过滤条件在展示上被视为"未提供",而 listTasks() 只要值 !== undefined 就会启用过滤 —— 于是 owner: '' 变成字面量比较,blockedBy: '' 变成永远不匹配的 includes('') 检查。有任务的面板确实会返回 No tasks found.

方向:对齐。这是 team task-list 工具在错误地报告自身状态 —— 本修复让运行时行为与工具自身已展示的契约一致。不涉及 auth/sandbox/公共契约层面。

规模:触及 packages/core/src/tools/(核心路径):6 行生产代码(task-list.ts)对 52 行测试(task-list.test.ts)。远低于任何规模阈值。

方案:范围合理。在 createInvocation 处归一化,正好落在每个调用必经的唯一入口,且有意不改动 listTasks() 的存储层语义(不影响其他调用方)。只处理字面量空字符串、保留纯空白字符串是合理的切分 —— 展示/行为错配只存在于 falsy 的 ''(空白字符串是 truthy,展示与过滤行为本就一致)。issue 中讨论过的替代方案(schema 拒绝、存储层归一化)都是更大的行为变更。

风险:无升级风险信号 —— 改动文件均未命中本仓库 revert 相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 9cbb5c2231d0b7b3ef52c5cbe518dd71d5943db4 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Code review

No blockers found. The fix does exactly what the independent read of the problem suggests: normalize the two optional filters at the tool boundary, before anything else sees them. createInvocation is the right spot — it is the single construction path for every invocation (BaseDeclarativeTool.buildcreateInvocation; no other constructor call sites exist), so getDescription() and execute() now agree, while listTasks() storage semantics stay untouched for internal callers. The spread preserves status, and undefined is valid for the optional owner/blockedBy fields.

The tests are the strong part. One control (omitted filters list tasks; a nonempty owner stays selective) plus two regression tests that genuinely pin the old failure: a freshly created task has owner: undefined and blockedBy: [], so under the old code t.owner !== '' excluded it and [].includes('') never matched — both blank-filter tests fail without this diff and pass with it. They run against a real temp-dir task store, matching the file's existing conventions.

Nothing else in the diff — no drive-by changes, no scope creep.

Test evidence

This is an unattended CI run — the PR's code is never executed here; the evidence below is the PR's own CI on the reviewed commit, read through the API. No red checks at review time. The main unit suite (Test (ubuntu-latest, Node 22.x)) is still running; the macOS and Windows test legs and the CLI integration suite are skipped on this commit. The table below is updated automatically once CI settles.

Final CI results for 9cbb5c2 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Not verified here: runtime behavior — unit-suite results land with CI above, and the author's local evidence (macOS package-local typecheck + Vitest) is their report, not independently re-run in this pass.

Sandboxed verification would settle the remaining gap: @qwen-code /verify — that the two new blank-filter tests actually fail on the base build and pass on this commit is proven statically above but not executed anywhere yet. The author lacks write access, so this would be a sponsored run: a maintainer's @qwen-code /verify comment approves the head it is written against, and the run carries a pre-execution risk screen plus a full workspace wipe. Read the resulting report with the same skepticism as the fork's own CI logs — the code under verification is adversarial input, and a crafted PR can shape what the report says even though the sandbox bounds what it can do.

中文说明

代码审查

未发现阻塞问题。修复做法与对问题的独立分析完全一致:在工具边界归一化两个可选过滤条件。createInvocation 是正确的位置 —— 它是每个调用的唯一构造路径(BaseDeclarativeTool.buildcreateInvocation,没有其他构造调用点),因此 getDescription()execute() 行为从此一致,同时 listTasks() 的存储层语义对内部调用方保持不变。spread 保留了 statusundefined 对可选的 owner/blockedBy 字段也是合法值。

测试是亮点:一个对照测试(省略过滤条件可列出任务;非空 owner 仍保持选择性),加两个真正锁定旧缺陷的回归测试 —— 新建任务 owner: undefinedblockedBy: [],旧代码下 t.owner !== '' 会将其排除、[].includes('') 永不匹配,因此这两个空过滤测试在没有本 diff 时必然失败、有 diff 后通过。测试基于真实临时目录的任务存储,与文件既有风格一致。

diff 中没有其他内容 —— 无顺手改动,无范围蔓延。

测试证据

本次为无人值守 CI 运行 —— 不在这里执行 PR 代码;以上证据是通过 API 读取的该 PR 自身 CI 在受审 commit 上的结果。审查时无红色检查。主单元测试套件(Test (ubuntu-latest, Node 22.x))仍在运行;macOS/Windows 测试腿与 CLI 集成套件在该 commit 上被跳过。上方表格会在 CI 结束后自动更新。

此处未验证:运行时行为 —— 单元测试结果以上述 CI 为准;作者本地证据(macOS 包级 typecheck + Vitest)为其自述,本次未独立复跑。

沙箱验证可以补上剩余缺口:@qwen-code /verify —— 两个新的空过滤测试在 base 构建上确实失败、在本 commit 上通过,这一点在上面已静态论证,但尚未在任何地方实际执行。作者无写权限,因此这将是一次资助运行:由 maintainer 的 @qwen-code /verify 评论批准对应 head,运行前有风险筛查与工作区清理。请像对待 fork 自身 CI 日志一样审慎阅读其报告 —— 受验证代码属于对抗性输入,恶意 PR 可以影响报告"说什么",尽管沙箱限制了它"能做什么"。

Qwen Code · qwen3.8-max

Reviewed at 9cbb5c2231d0b7b3ef52c5cbe518dd71d5943db4 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — textbook small bugfix: observed bug with a deterministic reproduction, root cause confirmed in code, minimal fix at the right boundary, and regression tests that provably fail without the diff.

Stepping back: this is exactly the shape of change the gate wants to let through. The problem is real and demonstrated, not hypothesized — #9281 shows a populated task board reporting No tasks found., which actively misleads a team coordinator. The fix is four production lines at the one choke point every invocation passes through, it makes runtime behavior match what the tool's own description already promises, and it deliberately leaves storage-layer semantics alone. I looked for the simpler path and there isn't one — schema rejection or storage-layer normalization would both be bigger behavioral changes for the same outcome. The whitespace-only exclusion is a reasoned scope cut, not a gap: whitespace strings display and filter consistently, so the display/behavior mismatch this fixes doesn't exist for them.

If I had to maintain this in six months, the answer is clear — the tests tell you why the normalization exists, and they fail loudly if anyone removes it. Approving because it is genuinely good, not because I ran out of objections.

Approval deferred until CI lands green on 9cbb5c2231d0b7b3ef52c5cbe518dd71d5943db4 — the unit suite is still running at review time; the approval posts automatically once every check on that commit completes green (and withholds if anything lands red or the head moves).

中文说明

置信度:5/5 —— 教科书式的小修复:已观测到的 bug、确定性复现、根因在代码中确认、在正确的边界做最小修复,且回归测试在没有本 diff 时必然失败。

退一步看:这正是 gate 应当放行的改动形态。问题真实且被证明,而非假设 —— #9281 展示了有任务的面板返回 No tasks found.,会主动误导 team coordinator。修复只有 4 行生产代码,落在每个调用必经的唯一入口,让运行时行为与工具自身描述已承诺的契约一致,并有意不触碰存储层语义。我找过更简单的路径,没有 —— schema 拒绝或存储层归一化对同一结果都是更大的行为变更。排除纯空白字符串是有理由的范围切分,而非遗漏:空白字符串在展示与过滤上本就一致,本次修复的展示/行为错配并不存在于该情形。

六个月后维护这段代码毫无负担 —— 测试说明了归一化存在的原因,任何人移除它都会立刻失败。批准是因为它确实好,而不是因为找不到反对理由。

审批延迟至 CI 在 9cbb5c2231d0b7b3ef52c5cbe518dd71d5943db4 上全绿 —— 审查时单元测试套件仍在运行;该 commit 上所有检查全绿后会自动提交审批(若有红色结果或 head 移动则不会提交)。

Qwen Code · qwen3.8-max

Reviewed at 9cbb5c2231d0b7b3ef52c5cbe518dd71d5943db4 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI still running。 已审查。

— qwen3.8-max via Qwen Code /review (v0.21.12)

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@wenshao

wenshao commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code attempted to resolve merge conflicts but the run did not complete successfully.

Check the workflow run for full logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

task_list treats blank optional filters as active filters

4 participants