Skip to content

fix: AskUserQuestion 默认永不超时 + 移除误导性 primary 高亮 - #243

Merged
lishuceo merged 3 commits into
mainfrom
feat/claude-session-a20383
May 24, 2026
Merged

fix: AskUserQuestion 默认永不超时 + 移除误导性 primary 高亮#243
lishuceo merged 3 commits into
mainfrom
feat/claude-session-a20383

Conversation

@lishuceo

Copy link
Copy Markdown
Owner

变更概述

  • 选项按钮统一 default 样式,不再把第一项染成 primary,避免被误以为已选中
  • 超时改为通过 ASK_USER_TIMEOUT_MS 环境变量配置,默认 0 = 永不超时(原硬编码 5 分钟)
  • 同步更新 ask-user-question.test.ts 用例

测试计划

  • npx vitest run src/__tests__/ask-user-question.test.ts 通过(11/11)
  • npm run typecheck 通过
  • 实际触发 AskUserQuestion 卡片,确认所有按钮均为白底
  • 长时间不回答,确认不再 timeout

🤖 Generated with Claude Code

- 选项按钮统一 default 样式,避免被误以为已选中
- 超时改为可通过 ASK_USER_TIMEOUT_MS 环境变量配置,默认 0 = 永不超时
- 同步更新测试用例

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @lishuceo's task in 2m 6s —— View job


PR Review Progress

  • Check previous review comments (none from claude[bot])
  • Read CLAUDE.md and understand context
  • Review changed files (3 files, +16/-10)
  • Post inline comments for new issues (none — only finding is on a non-diff line)
  • Post summary comment

Review Result · ✅ Approved (with one nit)

完整评审已发到 summary 评论。要点:

  • 改动正确:按钮全部 default、超时 env 解析的边界处理(Number.isFinite && > 0undefined/空字符串降级)都稳;clearTimeout 处都有 if (pending.timeoutTimer) 守卫。
  • 测试同步ask-user-question.test.ts 用例已对齐「全部 default」。
  • 🟡 nit: src/feishu/event-handler.ts:162 注释里的「5 分钟超时」字样已过时;该行不在 diff 内所以没法 inline,已放到 summary。
  • ℹ️ 设计提示:默认 0 = 永不超时 后,如果用户从不回答,pendingQuestions Map 条目永不清理、Promise 永不 settle,会占用 per-chat FIFO queue 槽位。By-design,建议确认是否有其他取消路径(详见 summary)。

@github-actions

Copy link
Copy Markdown
Contributor

Code Review · ✅ Approved (with one nit)

Verdict: 改动小、聚焦、有测试覆盖,可以合入。

改动分析

  • message-builder.ts — 移除 oi === 0 ? 'primary' : 'default' 的特殊高亮,全部按钮统一 default。语义改进合理(第一项不应被误解为「已选中」)。
  • event-handler.ts — 超时改为读取 ASK_USER_TIMEOUT_MS env,缺省 / 空 / 非有限正数都安全降级到 0(不启动 timer)。Number.isFinite(n) && n > 0 的边界处理是稳的;后续 clearTimeout(pending.timeoutTimer) 处都有 if (pending.timeoutTimer) 守卫,timer 为 undefined 不会报错。
  • ask-user-question.test.ts — 用例同步到「全部 default」,覆盖正确。

🟡 待跟进(轻微)· confidence 80

src/feishu/event-handler.ts:162 的注释仍写着「等待 5 分钟超时」,但默认值已改成 0(永不超时),描述与现状脱节。建议下一次顺手改成:

// 如果发送失败,立即 reject 而非等待用户回答(避免任务永久挂起)

(不是 diff 范围内的行,所以没有放成 inline 评论。)

ℹ️ 设计提示(非 blocker)

默认 0 = 永不超时 后,如果用户彻底不回答:

  • pendingQuestions Map 条目永不清理(条目本身很小,但持有 resolve/reject 闭包 → 间接 hold 住对应 Claude task 的执行上下文);
  • 包裹的 Promise 永不 settle,对应的 Claude 查询会一直挂在 await askUser(...),会一直占用该 chat 的 task queue 槽位(session/queue.ts 是 per-chat FIFO,单条挂住会卡死整个会话队列)。

PR 标题明确说「默认永不超时」,所以这是 by-design。但建议确认一下:

  1. 是否已有其他路径(例如 chat 重置、/cancel、bot 重启)能 reject 掉残留的 PendingQuestion
  2. 如果没有,是否考虑加一个手动取消入口,或把默认值设成「足够长但有上限」(例如 24h)以兜底?

不阻断合并,留作后续观察。

View job run

@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @lishuceo's task in 1m 30s —— View job


PR Review Progress

  • Check previous review comments (none from claude[bot] as inline)
  • Read CLAUDE.md and understand context
  • Review changed files (3 files, +16/-10)
  • Verify no new issues introduced by latest sync (merge from main — no new logic changes)
  • Post summary

Review Result · ✅ Approved (re-confirmed after sync)

最新 push 是 f3c0def(合并 origin/main),未引入新的逻辑改动。原 PR commit 8911e4f 的三处改动我重新确认了一遍,结论与上一轮 review 一致:可合入。

改动确认

文件 改动 验证
src/feishu/message-builder.ts 1285 type: oi === 0 ? 'primary' : 'default'type: 'default' ✅ 移除第一项 primary 高亮,避免被误以为"已选中"
src/feishu/event-handler.ts 135-141 超时改为读取 ASK_USER_TIMEOUT_MS env,默认 0 Number.isFinite(n) && n > 0 边界处理稳;undefined/空字符串 / NaN / ≤0 都安全降级到 0
src/feishu/event-handler.ts 169-174 if (ASK_USER_TIMEOUT_MS > 0) 时才启动 timer ✅ 配合 pending.timeoutTimer?: ... 可选字段;后续 clearTimeout(pending.timeoutTimer) 处也都有 if 守卫
src/__tests__/ask-user-question.test.ts 57-58 用例断言全部 default ✅ 与新行为对齐,11/11 通过

🟡 遗留 nit · confidence 90(与上一轮 review 一致,仍未修复)

src/feishu/event-handler.ts:179 的注释仍然写着「如果发送失败,立即 reject 而非等待 5 分钟超时」。默认值已改成 0(永不超时),这行描述已与现状脱节。该行不在本次 diff 范围内所以没法贴 inline,建议下次顺手改成:

// 如果发送失败,立即 reject 而非等待用户回答(避免任务永久挂起)

Fix this →

ℹ️ 设计提示(非 blocker,与上一轮一致)

默认 0 = 永不超时 意味着:用户始终不回答时,pendingQuestions Map 条目不会清理、外层 Promise 不会 settle,对应的 Claude 查询会一直挂在 await askUser(...),永久占用 session/queue.ts 中该 chat 的 FIFO 槽位(整个会话队列会被卡死)。

PR title 写明「默认永不超时」,所以是 by-design。建议确认是否有其他取消路径(如 /cancel、chat 重置、bot 重启)能 reject 残留的 PendingQuestion;如果暂时没有,可以考虑:

  1. 把默认值设为足够长但有上限的兜底(例如 24h);或
  2. 单独加一个手动取消入口(slash command 或卡片按钮)。

留作后续观察,不阻断本次合并。


· branch feat/claude-session-a20383

默认已改为永不超时,原注释 '5 分钟超时' 已与现状脱节。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Claude finished @lishuceo's task in 56s —— View job


Code Review · ✅ Approved

Verdict: 改动小、聚焦、有测试覆盖,并且上一轮 review 的 nit 已修复。可以合入。

上一轮 nit 修复确认

src/feishu/event-handler.ts:179 的注释由「等待 5 分钟超时」改为「等待用户回答(避免任务永久挂起)」(见 commit e982b24)。✅ 已对齐新的默认 0 = 永不超时 行为。

改动复核

文件 改动 验证
src/feishu/message-builder.ts 1285 type: oi === 0 ? 'primary' : 'default'type: 'default' ✅ 所有按钮统一 default,避免第一项被误以为「已选中」
src/feishu/event-handler.ts 135-141 超时改为读取 ASK_USER_TIMEOUT_MS env Number.isFinite(n) && n > 0 边界处理稳;undefined / 空字符串 / NaN / ≤0 全部安全降级到 0
src/feishu/event-handler.ts 169-174 仅当 ASK_USER_TIMEOUT_MS > 0 时才启动 timer timeoutTimer?: ReturnType<typeof setTimeout> 可选;后续两处 clearTimeout 调用(:190:227)都有 if (pending.timeoutTimer) 守卫,timer 为 undefined 不会报错
src/__tests__/ask-user-question.test.ts 57-58 用例断言全部 default ✅ 与新行为对齐

Resource leak / 队列阻塞确认

我审了一下 pendingQuestions Map 的生命周期,再次确认 PR 的「默认永不超时」是 by-design:

  • 用户始终不回答时,pendingQuestions 条目永不清理,外层 Promise 永不 settle;
  • 包裹的 Claude task 会一直挂在 await askUser(...),永久占用 session/queue.ts per-chat FIFO 槽位。

这是 PR title 明确声明的语义,不阻断合并。后续可以考虑加一个手动取消入口(slash command 或卡片「取消」按钮)作为兜底,但不影响本次合入。

Tasks

  • Check previous review comments
  • Read changed files (3 files)
  • Verify previous nit was addressed (✅ fixed in e982b24)
  • Confirm no new issues

branch feat/claude-session-a20383

@lishuceo
lishuceo merged commit 2ab1a44 into main May 24, 2026
4 checks passed
@lishuceo
lishuceo deleted the feat/claude-session-a20383 branch May 24, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant