Skip to content

fix(acp): address review issues from PR #1702#1725

Merged
kaizhou-lab merged 1 commit intomainfrom
fix/acp-prompt-keepalive-review-followup
Mar 26, 2026
Merged

fix(acp): address review issues from PR #1702#1725
kaizhou-lab merged 1 commit intomainfrom
fix/acp-prompt-keepalive-review-followup

Conversation

@piorpua
Copy link
Copy Markdown
Contributor

@piorpua piorpua commented Mar 26, 2026

Summary

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

Issues Fixed

# Severity File Issue Fix Applied
1 🔵 LOW tests/unit/acpConnectionKeepalive.test.ts:26-45 vi.mock 路径相对测试文件解析,未能拦截源文件模块 删除无效的 3 个 vi.mock 调用,与 acpTimeout.test.ts 保持一致
2 🔵 LOW tests/unit/acpConnectionKeepalive.test.ts:50-51 priv() 函数使用 Record<string, any> 触发 lint 警告 改为 Record<string, unknown>

Related

Follow-up to #1702

Test Plan

  • bun run test — all tests pass
  • bunx tsc --noEmit — no type errors
  • bun run lint:fix — lint clean
  • Manually verify each fixed issue in the changed files

- Remove ineffective vi.mock calls with wrong relative paths that did
  not intercept source module dependencies
- Replace Record<string, any> with Record<string, unknown> in priv()
  helper to fix no-explicit-any lint warnings

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

piorpua commented Mar 26, 2026

PR Fix 验证报告

原始 PR: #1702
Follow-up PR: #1725

# 严重级别 文件 问题 修复方式 状态
1 🔵 LOW tests/unit/acpConnectionKeepalive.test.ts:26-45 vi.mock 路径相对测试文件解析,未能拦截源文件模块 删除无效的 3 个 vi.mock 调用(./acpConnectors./utils./modelInfo),与 acpTimeout.test.ts 保持一致 ✅ 已修复
2 🔵 LOW tests/unit/acpConnectionKeepalive.test.ts:50-51 priv() 函数使用 Record<string, any> 触发 no-explicit-any lint 警告 改为 Record<string, unknown> ✅ 已修复

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

@kaizhou-lab kaizhou-lab merged commit 9ecff60 into main Mar 26, 2026
14 of 17 checks passed
@piorpua piorpua deleted the fix/acp-prompt-keepalive-review-followup branch March 26, 2026 03:00
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