fix: bare clone/fetch 异步化,git 网络操作不再阻塞事件循环

[lishuceo]

变更概述

• 根因:cloneBareAtomic / fetchIfStale 用 execFileSync 同步执行远程 git,clone 卡到 timeout(默认 5 分钟)期间整个单进程 bridge 的事件循环被冻死 —— 飞书事件无法处理/ACK,被飞书定时重投后又超过 5 分钟 staleness 阈值被丢弃,导致期间所有会话"毫无响应"。一次连不上远程的 clone 即可拖垮全服务(线上 06-23 10:39–10:44 实测冻结 5 分钟)。
• 改为 promisify(execFile) + await:cache.ts 的 clone/fetch/ensureBareCache 全部异步化;manager.ts setupWorkspace 改 async 并将本地 clone/set-url/checkout 一并异步化;isolation.ts/tool.ts/registry.ts 调用处补 await。
• execFile 不支持 stdio,改用 maxBuffer: 64MB 防输出溢出;env(GIT_LFS_SKIP_SMUDGE)保留。
• 与已合入的 fix: 修复 bare cache 分支永久冻结(fetch 空转) #266(bare cache refspec 修复)正交,无冲突。

测试计划

• npm run typecheck 通过
• eslint(改动文件)无错
• 新增回归守卫:bare clone 在途时事件循环仍存活,并发宏任务能推进(锁死同步阻塞根因)
• 更新 cache/manager/event-handler 测试为 async;修复 identity.test.ts 的 child_process mock 缺 execFile 致模块加载崩溃
• 全量 vitest:1723 passed(唯一 1 个 failure 为 memory/quality.test.ts,依赖 DASHSCOPE_API_KEY 的既有环境失败,与本改动无关)

🤖 Generated with Claude Code

[github-actions]

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

---

Code Review — PR #267

Verdict: ✅ Approve with one minor note

The async refactor is correct and well-scoped. The root cause (synchronous execFileSync freezing the single-process event loop during the entire git network timeout window) is real and the fix is the right one. Every call site of the now-async functions was updated:

Function	Callers updated
ensureBareCache	manager.ts:126, registry.ts:423 ✅
setupWorkspace	isolation.ts:154, tool.ts:116 ✅
ensureIsolatedWorkspace	(no production callers; tests await it) ✅

Other things verified:

• execFile error rejections carry .message; try/catch + await correctly replaces the old sync throw paths. ✅
• maxBuffer: 64MB is a sound replacement for stdio: pipe — bare clone/fetch produce little output in non-TTY. ✅
• GIT_LFS_SKIP_SMUDGE env preserved on the workspace clone; bare clones don't smudge anyway. ✅
• The event-loop-liveness regression test (cache.test.ts:272) genuinely guards the root cause. ✅
• Concurrent stale fetchIfStale collisions degrade gracefully (failure → return true → use stale, non-fatal). ✅

🟡 One issue (confidence 80) — left as inline comment

Concurrent cold-clone race in ensureBareCache / cloneBareAtomic. The previous sync version implicitly serialized first-time clones on the event loop; awaiting now lets two concurrent callers for the same uncached repo (different chats aren't serialized by the per-chat queue) both pass the existsSync check and both clone — the loser's renameSync hits ENOTEMPTY and fails that request. Self-healing on retry, but a new regression. Suggested fix: an in-flight Map<cachePath, Promise> dedup, or tolerate ENOTEMPTY at the rename. (See inline comment for details + a one-click fix link.)

Testing note

The CLAUDE.md testing policy is satisfied — async migration is covered by updated cache/manager/event-handler tests plus the new event-loop regression guard. If the race above is addressed, add a concurrent-ensureBareCache regression test alongside it.

---

• Gather context (read changed source files)
• Review cache.ts async changes
• Review manager.ts / isolation.ts / callers
• Check for unhandled rejections, races, behavior changes
• Post review summary

[github-actions]

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

---

Code Review — PR #267 (re-review after fix)

Verdict: ✅ Approve — 1 previous issue fixed.

The concurrent cold-clone race flagged in the prior review (github-actions[bot], confidence 80) is correctly resolved in commit d79d706, and the thread is already resolved.

Fix verification — in-flight clone dedup (cache.ts:38,149-156)

let clonePromise = inFlightClones.get(cachePath);
if (!clonePromise) {
  clonePromise = cloneBareAtomic(repoUrl, cachePath).finally(() => {
    inFlightClones.delete(cachePath);
  });
  inFlightClones.set(cachePath, clonePromise);
}
await clonePromise;

Traced and confirmed correct:

• No await between get() and set() — the whole prefix of ensureBareCache up to await clonePromise is synchronous. Invoking cloneBareAtomic() runs synchronously up to its own first await execFileAsync and returns a pending promise, which is registered before the first suspension point. A second concurrent caller for the same uncached repo is therefore guaranteed to observe the registered promise and reuse it → only one clone + one renameSync → no ENOTEMPTY. ✅
• .finally(() => delete) clears the entry on both resolve and reject, so a failed clone doesn't poison the cache key — the next request retries cleanly. ✅
• Keyed by the resolved absolute cachePath (post-normalization), so URL variants of the same repo collapse to one entry. ✅

Regression test (cache.test.ts:303-332)

Genuinely guards the root cause: two concurrent ensureBareCache calls for the same uncached repo with a suspended clone, asserting cloneCalls === 1 and renameSync called once. Pairs well with the existing event-loop-liveness guard (cache.test.ts:272). ✅

Notes

• Couldn't run typecheck/vitest in this environment (command approval not granted), but the PR documents both passing, and the changes are straightforward to verify by inspection.
• The CLAUDE.md testing policy is satisfied: async migration + the new concurrency regression test.

No new issues found. Nice tight fix.