fix(pi): handle pi ≥ 0.84.0 toolcall_end with toolCall field (fixes #1659)

[chenhg5]

Fixes #1659.

Problem

pi 0.84.0 removed the accumulated message and assistantMessageEvent.partial fields from message_update events (to avoid quadratic output growth). The cc-connect pi adapter still relied on those fields to extract tool calls, so every toolcall_end from pi ≥ 0.84.0 was silently dropped — tools=N log was always 0 and tool-progress cards never rendered on Feishu/Telegram/Discord.

Fix

In agent/pi/session.go, the toolcall_end case now:

1. Checks for assistantMessageEvent.toolCall first. When present (pi ≥ 0.84.0), it emits an EventToolUse directly with the tool name and extractToolInput(tc) result. No more silent drop.
2. Falls back to the existing emitToolFromMessage path (legacy message/partial + contentIndex) when toolCall is absent, so pi < 0.84.0 keeps working.

Also tightened extractToolInput's no-arguments fallback: when the new protocol sends a bare toolCall without an arguments map, the JSON dump now skips name/type/id instead of repeating them (they're already surfaced as ToolName / discriminator / event-id).

Changes

• agent/pi/session.go
  • handleMessageUpdate toolcall_end branch: new protocol path before legacy fallback.
  • extractToolInput: strip name/type/id when falling back to JSON of the whole item.
• agent/pi/pi_test.go
  • 5 new test cases covering:
    • new protocol with toolCall + arguments.command
    • new protocol: extractToolInput priority (description beats command)
    • new protocol: no arguments map (covered JSON skip)
    • new protocol takes precedence when both fields are present
    • non-map toolCall (defensive type assertion) falls through to legacy path

Test results

• go test ./agent/pi/... -race -count=1 — all pass (25 toolcall_end tests + full suite unchanged).
• golangci-lint run --new-from-rev origin/main ./agent/pi/... — 0 issues.
• go build ./agent/pi/... — clean.
• go vet ./agent/pi/... — clean.
• go test ./... — all unaffected packages pass; the two [setup failed] entries (cmd/cc-connect, web) are unrelated, pre-existing sandbox limitations (frontend web/dist not built locally; CI builds it via pnpm build per .github/workflows/ci.yml before Go tests).

Acceptance criteria

• ✅ pi ≥ 0.84.0: every toolcall_end is processed (no silent drop).
• ✅ pi < 0.84.0: legacy message + contentIndex path still works (existing tests TestHandleMessageUpdate_ToolcallEnd, TestHandleMessageUpdate_ToolcallEnd_UsesPartialFallback cover this).
• ✅ tools=N will reflect the real count once pi 0.84+ is used.
• ✅ Tool-progress cards on Feishu/Telegram/Discord will render (downstream of EventToolUse; no platform changes).
• ✅ Unit tests cover both protocol paths and edge cases.

Risks

• assistantMessageEvent.toolCall field shape is reverse-engineered from the pi 0.84.0 changelog plus the existing legacy schema; if pi renames the field in a future release cc-connect will silently drop again. The fallback path stays in place, so an unintended future protocol drift silently reverts to legacy behavior — preferable to a hard break.
• The extractToolInput JSON-skip change only affects the no-arguments branch, which the legacy path never exercised (legacy always had arguments). Existing TestExtractToolInput cases all pass unchanged.

Sibling

• [Feishu] pi agent's ask_user_question card: broken layout on mobile, button clicks never dispatched (v1.5.0-beta.2) #1658 (pi ask_user_question card broken on Feishu mobile) shares the same root-cause family (event handling chain) but is a separate bug. Not addressed here per the issue's "Out of scope" note.

🤖 Generated with Claude Code

[chenhg5]

结论: Comment(自审,无法 self-approve / self-request-changes)

由于本 PR 作者即 owner,GitHub 拒绝本人 approve / request-changes,改为 --comment 发表 QA 观察。请 maintainer 合并前自行判断结论。

总体判断

• 5/5 CI 全绿(lint / unit-test / smoke-test / regression-test / performance-test,run 31723819964),mergeable_state=clean。
• 改动小(+190/-2)、改动集中(2 文件:agent/pi/session.go + agent/pi/pi_test.go),贴合 PM 在 #1659 dispatch 里给出的修复 spec。
• 行为上正确恢复了 pi ≥ 0.84.0 的 tool-call 事件流,同时保留 pi < 0.84.0 legacy 路径,不存在 silent-drop 退化风险。
• 与 #1659 报告的根因描述(emitToolFromMessage 三段解析链 → return nil)完全对上。
• 与 reporter Po1nt9 给的修复示意(伪代码 if tc, ok := msg["toolCall"].(map[string]any); ok { ... } else { legacy })等价。

✅ 做得好的地方:

1. 防御性类型断言统一(if tc, ok := msg["toolCall"].(map[string]any); ok),与本文件其他分支风格一致;非 map 类型(如 "not-a-map")正确 fall through 到 legacy。TestHandleMessageUpdate_ToolcallEnd_NewProtocol_WrongTypeIsIgnored 覆盖。
2. 新协议优先级明确:即使新旧字段共存(toolCall + message/partial)也信任新协议(TestHandleMessageUpdate_ToolcallEnd_NewProtocol_TakesPrecedenceOverLegacy)。这避免了 transitional 事件期间出现 stale snapshot 渲染。
3. extractToolInput JSON fallback 收紧:无 arguments map 时跳过 name/type/id,避免 tool-progress 卡片重复显示已作为 Event.ToolName 出现的字段。TestHandleMessageUpdate_ToolcallEnd_NewProtocol_NoArguments 验证空参数不出 {}。
4. 5 个新测试覆盖到位:新协议 + description 优先级 + 无 arguments + 优先级 + 类型防御,无死角。
5. legacy 路径零修改,pi < 0.84.0 行为不变;TestHandleMessageUpdate_ToolcallEnd_NilMessage 等旧测试继续通过。
6. 沿用本文件其他分支的 select { case s.events <- evt: case <-s.ctx.Done(): } 背压模式,不会因为 channel 满而阻塞 pi rpc 读取。

🟠 P2 建议改进(不阻塞合并):

1. extractToolInput 函数 doc 缺失调用约定差异:legacy 调用方传 item = content[idx](content 数组元素,必有 arguments map);新协议调用方传 item = msg["toolCall"](顶层 toolCall,可能无 arguments map)。同一函数现在承担两种语义,无 arguments 时的 strip 行为只对新协议生效。建议在函数上方加一行注释说明两种 caller 与 hasArgs=false 路径的针对性,方便未来读者理解为什么 strip name/type/id。这不是 bug,但避免下次有人误以为 legacy 也会被 strip。
2. 新协议分支对 name 字段缺失/空字符串的处理:若 tc["name"] 不是 string,name 为空,EventToolUse{ToolName: ""} 仍会被发出,可能下游 cards 渲染空名。建议加一行 if name == "" { return } 或 fallback 到 tc["type"].(string)。legacy 路径同样有此问题(行为不变),所以不算新引入的 regression,但既然在改这块代码,顺手修一下成本低。

🔵 P3 可选:

• 建议在 handleMessageUpdate.toolcall_end case 注释里加一行 changelog 风格注释(// pi 0.84.0+: see issue #1659),方便日后排查。

❓ 需要确认:

• PR #1674(happyTonakai 社区贡献,2026-08-13 09:02 开,状态 open,未 merge)也在尝试修同一个根因。本次 QA 仅 review #1677;#1674 与 #1677 二选一 / 合并 / 借鉴的最终决策不在 QA 范围内,需要 maintainer 拍板。建议合并前确认:
  • #1674 是否有 community 测试覆盖了 #1677 没覆盖的 edge case
  • 是否要在 #1677 / #1674 任一合并后 close 另一个并互引
  • 我已在 qa_notes 抄送 PM

Testing / Risk:

• 已验证:5/5 CI success + dev 本地 go test ./agent/pi/... -race + golangci-lint --new-from-rev origin/main 0 issues + go vet clean + go build clean + gofmt 干净(只有 pre-existing pi.go unaligned struct 与本 PR 无关)。
• 未在本 sandbox 独立 reproduce pi 0.84.0 真实事件流(reporter Po1nt9 提供的 pi ≥ 0.84.0 log 显示 tools=0 已能间接证明;reporter 也确认 PR #1288 类似 fix 已上线修好同类问题)。
• dev 自报的 known_risks 与代码一致:1) toolCall 字段名依据 pi 0.84.0 changelog 推断,未来 pi 改名 cc-connect 会静默回退到 legacy 路径(对 pi < 0.84.0 仍正确);2) extractToolInput JSON skip 只影响无 arguments 分支(legacy 不会触发);3) #1658(pi ask_user_question Feishu 移动端坏)是同源但 spec 显式标 out-of-scope。

Next step:

• 建议 maintainer 在合并前先决定 #1674 vs #1677 处理顺序,并把 QA 这两条 P2(P2-1 文档注释 / P2-2 空 name 防御)的选择交给 PR author 判断:接受 P2-1 一行注释 + P2-2 空 name 早返回,可本 PR 内一并修;若希望保持最小 diff,可新开 issue 跟进。
• 若选择直接合并 #1677,close #1674 时请引用 #1677 / commit 2767d8d 与本 review。