fix(core): render Feishu AskUserQuestion as flat action rows (fixes #1658) - #1679
fix(core): render Feishu AskUserQuestion as flat action rows (fixes #1658)#1679chenhg5 wants to merge 1 commit into
Conversation
…1658) The single-select AskUserQuestion card was rendered as a column_set row per option, with the option description in a weighted column and the click button in an 'auto' column. On Feishu mobile the button column was clipped or non-interactive, and even on desktop the column_set > column > button pattern was reported as unreliable — Po1nt9's log during the 08:00-08:25 test window showed zero card-action entries for ask_user_question cards while permission cards (which use a flat action row, tag:"action") dispatched normally. Mirror the permission-card layout: each option becomes a markdown description followed by a flat CardActions row with a single button. The askq:qIdx:optIdx value plus askq_label/askq_question extras are preserved so the Feishu callback handler still renders the post-answer card. The Note line that already nudges users toward numeric / text fallbacks stays in place. Sibling fix to PR #1677 (issue #1659, pi 0.84.0+ tool-call drop) — both reporters run pi through cc-connect on Feishu.
chenhg5
left a comment
There was a problem hiding this comment.
结论: Comment(自审,无法 self-approve / self-request-changes)
由于本 PR 作者即 owner,GitHub 拒绝本人 approve / request-changes,改为 --comment 发表 QA 观察。请 maintainer 合并前自行判断结论。
总体判断
- 5/5 CI 全绿(lint 2m53s、unit-test 4m35s、smoke-test 38s、regression-test 29s、performance-test 55s,run 31728842661),mergeable_state=clean。
- 改动极小(+80/-3)且只触碰 single-select 路径(
else分支);multi-select 路径完整保留。dev description 与 diff 都确认。 - 改动集中 2 文件:
core/engine.go(sendAskQuestionPrompt 单一函数)+core/engine_test.go(新增回归测试)。 - 完全贴合 #1658 报告的根因(column_set > column > button 在 Feishu mobile 不响应 click)、采纳 reporter Po1nt9 给出的"和 permission card 一样用 flat action row"的修复方向。
- 与 PR #1677(#1659 pi tool-call drop,已合并)构成 sibling 修复,合并后 pi + Feishu 端到端可用。
✅ 做得好的地方:
- Callback 完全兼容:
answerData = "askq:%d:%d"维持原样,askq_label+askq_questionextras 维持原样。Feishu callback handler 完全不需要改,降低了回归风险。 - 新回归测试精确守卫未来:
TestSendAskQuestionPrompt_CardPlatform_SingleSelectUsesActionRows同时断言 4 件事:(a)3 CardActions rows(每个 option 一行)、(b)3 askq buttons、(c)不能出现 CardListItem(防止有人回滚到旧 layout)、(d)每个 button 必须带 askq_label + askq_question extras(防止 extras 漏掉导致 post-answer 卡片渲染坏)。这是一个非常"能抓住人"的测试。 - 修复方向贴合 reporter 实证:reporter 报告 "permission card 的 cmd: 点击正常工作,ask_user_question card 的点击不工作" → fix 模仿 permission card 的 flat action row,而不是重新设计新 layout。最小风险、最低惊讶。
- Note 保留:每个 option 仍有
cb.Note(e.i18n.T(MsgAskQuestionNote))提示用户用 numeric/text fallback。如果新 layout 在某些边缘客户端仍不响应,用户有 fallback。 - 改动隔离:只触碰 single-select 路径。multi-select (
else if MultiSelect) 完全未动,现有TestSendAskQuestionPrompt_CardPlatform_MultiQuestion_ShowsIndex等多问题测试继续 pass。 - 改动范围合理:+80/-3 包含 35 行注释解释 fix 理由 + 3 行 markdown+button 调用替换 + 64 行新测试代码。注释密度比同行其它位置高,reviewer 一眼能看懂"为什么"。
- Markdown 描述加粗:
cb.Markdown("**" + desc + "**")让 description 在卡片里更醒目(视觉对比 button label),与新 layout 的"两行"视觉节奏一致。
🟠 P2 建议改进(不阻塞合并):
- Button type 从 "default" 改为 "primary":旧 layout 用
"default"(neutral 风格),新 layout 用"primary"(品牌色,更醒目)。这是有意为之(让 button 更突出、增加点击率),但属于视觉变化。建议在 PR description 加一句说明这个 type 变化,以免下游 lark/feishu-mobile 用户反馈"按钮颜色变了"。如果想保守一点,可降级为"default"——但 click 可靠性可能受影响。作者自决。 cb.Markdown("**" + desc + "**")字符串拼接未 escape Markdown:opt.Label或opt.Description若包含*或_或[,会被 markdown 解析器误解(下划线变斜体、方括号变链接等)。边角案例但真实存在(用户描述里加星号强调不是没可能)。建议要么用feishu.EscapeMarkdown(desc)(若平台已有 helper),要么去掉**包裹,要么在cb.Markdown调用前做最小化 escape。不阻塞合并,但 P3 升级到 P2。- Element 数量可能略增:每个 option 现在是
Markdown+Buttons两个 element,而原来是ListItemBtnExtra一个 element。Feishu card 元素上限是 50(per card schema)。ask_user_question 通常 2-4 options,2-4 问题,总元素 8-16,远低于上限。但如果未来支持"1 question with 10 options"可能撞上限,目前 spec 不会,无需现在处理。
🔵 P3 可选:
- 新 layout 改变了 description 与 button 之间的视觉关系(从"左右"变"上下"),可考虑在 PR description 加一张 before/after ASCII 截图或描述,方便社区 contributor 在 review 时对比。
- 测试名字
TestSendAskQuestionPrompt_CardPlatform_SingleSelectUsesActionRows已足够自解释,但内部actionRows++在每个CardActionscase 都 ++ 一次,即便该 actions row 不是 askq 来源。建议测试改为只在按钮是 askq 时 ++ actionRows,语义更精确。这是 nit。
❓ 需要确认:
- PR #1677(#1659 pi tool-call drop)已被 chenhg5 维护,PR #1679 本次合并后,reporter Po1nt9 应可在 pi 0.84.0+ + Feishu 上端到端跑通 ask_user_question 流程。建议合并后立即在 #1658 评论同步 reporter:已合并 PR #1679 / commit a972ea6,关闭 issue。
- dev 自报的 known_risks 与代码一致:1) card layout 两行 vs 一行(有意为之);2) 其他 CardListItem 调用者(/list, /delete, /provider 等)未触碰(diff 已确认只在 sendAskQuestionPrompt 单一函数内)。
Testing / Risk:
- 已验证:5/5 CI success + dev 本地
go test -race -count=1 ./core/PASS(48.7s)+./agent/pi/PASS(7.0s)+./platform/feishu/PASS(33.3s)+ gofmt/vet/build clean + golangci-lint --new-from-rev origin/main ./core/... 0 issues。 - 真实 Feishu mobile click 行为无法在 sandbox 内 reproduce(需要 Feishu mobile 客户端 + 凭据),reporter Po1nt9 已提供生产证据(08:00-08:25 测试窗口内 0 card-action entries for ask_user_question vs permission cards 正常),PR description 已引用。
- 未触碰 multi-select 路径,所有现存 multi-question 测试(已有
TestSendAskQuestionPrompt_CardPlatform_MultiQuestion_ShowsIndex)继续 pass。
Next step:
|
两个 PR 处理的是同一个问题(#1658),建议 reviewer 评审时顺便对比一下 #1469(fix/feishu-askquestion-card-layout)。 #1469 的做法:同样移除了 补充一个技术点供评审参考:#1679 把根因归结为"column_set 内嵌按钮在移动端不可点击",但报告人确认可正常点击的权限卡( 建议:两个 PR 二选一合入(或基于其中一个叠加另一个的布局改进),并在真实 Feishu 移动端验证后关闭 #1658。 |
Summary
Fix #1658: Feishu + pi agent's AskUserQuestion card button clicks never dispatched on Feishu mobile, and were unreliable on desktop. Permission cards (cmd: actions) worked normally — the reporter's log showed zero card-action entries during the 08:00-08:25 ask_user_question test window while cmd= entries for permission cards appeared as expected.
Root cause
sendAskQuestionPrompt(single-select path) rendered each option as aCardListItem, which the Feishu renderer serialises as acolumn_setwith the description in a weighted column (weight:5) and the click button in anautocolumn. On Feishu mobile the button column is clipped/non-interactive, and even on desktop this nested layout doesn't reliably dispatch click events.Fix
Mirror the permission-card pattern (which the reporter confirmed dispatches reliably): each option becomes a markdown description line followed by a flat
CardActionsrow with a single button. Theaskq:qIdx:optIdxvalue plusaskq_label/askq_questionextras are preserved so the Feishu callback handler still renders the post-answer card unchanged.The Note line that nudges users toward numeric / text fallbacks is kept in place as a defensive fallback.
Sibling
PR #1677 (issue #1659, pi ≥ 0.84.0 tool-call drop) — same reporter (Po1nt9), same pi + Feishu setup. Both fixes ship together so the pi + Feishu flow is fully usable end-to-end.
Changes
core/engine.go— switch single-select AskUserQuestion rendering fromcb.ListItemBtnExtra(...)tocb.Markdown("**desc**") + cb.Buttons(CardButton{...})core/engine_test.go— new regression test asserting: 3CardActionsrows, 3 askq buttons, noCardListItem, and that every button carriesaskq_label+askq_questionextrasValidation
go test -race -count=1 ./core/— PASS (48.7s)go test -race -count=1 ./agent/pi/— PASS (7.0s)go test -race -count=1 ./platform/feishu/— PASS (33.3s)gofmt -l— cleango vet ./core/...— cleango build -tags no_web ./...— cleangolangci-lint run --new-from-rev origin/main ./core/...— 0 issuesRisks
isAskQuestion(engine.go) still treatsAskUserQuestionandextension_selectthe same way and routes through the samesendAskQuestionPrompt. pi'sforwardSelectcarriesaskq_label/askq_questionviaExtraexactly as before.CardActionsdirectly, not via this path.