Skip to content

fix(acp): address review issues from PR #1700#1723

Merged
kaizhou-lab merged 1 commit intomainfrom
fix/acp-timeout-no-finish-signal-review-followup
Mar 26, 2026
Merged

fix(acp): address review issues from PR #1700#1723
kaizhou-lab merged 1 commit intomainfrom
fix/acp-timeout-no-finish-signal-review-followup

Conversation

@piorpua
Copy link
Copy Markdown
Contributor

@piorpua piorpua commented Mar 26, 2026

Summary

Follow-up to #1700 — addresses all issues identified in the code review.

Issues Fixed

# Severity File Issue Fix Applied
1 🟡 MEDIUM tests/unit/acpAgentTimeoutFinish.test.ts Tested hand-written stub instead of real AcpAgentManager, providing false coverage confidence Deleted the file; acpAgentManagerCronGuard.test.ts already covers the real class with 9 tests
2 🔵 LOW tests/unit/acpAgentManagerCronGuard.test.ts 15+ no-explicit-any / no-extraneous-class lint warnings Replaced any with unknown/precise types, used as unknown as T for private property access, replaced empty class {} mock with vi.fn()
3 🔵 LOW tests/unit/acpAgentTimeoutFinish.test.ts 3 no-explicit-any lint warnings Resolved by deleting the file (see #1)

Related

Follow-up to #1700

Test Plan

  • bun run test — 131 test files, 1432 tests, all pass
  • bunx tsc --noEmit — no type errors
  • bun run lint:fix — no acpAgent-related lint warnings remain
  • bun run format — formatting clean

- Remove acpAgentTimeoutFinish.test.ts: tested a hand-written stub instead
  of the real AcpAgentManager class, providing false coverage confidence
- Fix no-explicit-any lint warnings in acpAgentManagerCronGuard.test.ts:
  replace `any` with `unknown`/precise types, use `as unknown as T` for
  private property access, replace empty `class {}` mock with `vi.fn()`

Review follow-up for #1700
@piorpua
Copy link
Copy Markdown
Contributor Author

piorpua commented Mar 26, 2026

PR Fix 验证报告

原始 PR: #1700
Follow-up PR: #1723

# 严重级别 文件 问题 修复方式 状态
1 🟡 MEDIUM tests/unit/acpAgentTimeoutFinish.test.ts 测试手写 stub 而非真实 AcpAgentManager 删除文件,由 acpAgentManagerCronGuard.test.ts 提供完整覆盖 ✅ 已修复
2 🔵 LOW tests/unit/acpAgentManagerCronGuard.test.ts 15+ no-explicit-any / no-extraneous-class lint 警告 anyunknown/精确类型,私有属性访问用 as unknown as T,空 class {}vi.fn() ✅ 已修复
3 🔵 LOW tests/unit/acpAgentTimeoutFinish.test.ts 3 处 no-explicit-any lint 警告 随文件删除一并解决 ✅ 已修复

总结: ✅ 已修复 3 个 | ❌ 未能修复 0 个 | ⏭️ 跳过 0 个

@kaizhou-lab kaizhou-lab merged commit 6c13334 into main Mar 26, 2026
15 of 17 checks passed
@piorpua piorpua deleted the fix/acp-timeout-no-finish-signal-review-followup branch March 26, 2026 02:38
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.

2 participants