refactor: migrate executor to Bun.spawn + test improvements (closes #30) (#31)

- Replace execSync with Bun.spawn() for async process spawning
- Implement timeout via setTimeout + proc.kill()
- Convert resolveApiKey to async using Bun.spawn(['sh', '-c', ...])
- Add lazy API key caching in createCodingExecutor closure
- Clean up temp prompt files in finally block
- Add runtime effect.type validation
- Consolidate 5 prompt scenario tests into test.each
- Use expect().rejects.toThrow for error assertion
- Restore strong expect(result.success).toBe(true) for exit code 0
- Remove node:child_process dependency entirely

Made-with: Cursor

Co-authored-by: 小橘 <xiaoju@shazhou.work>

packages/pulse/src/executors/coding-agent.test.ts

@@ -8,6 +8,7 @@
 import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
 import {
   chmodSync,
+  existsSync,
   mkdirSync,
   readdirSync,
   rmSync,
@@ -37,13 +38,6 @@ function makeStub(name: string, script: string): string {
   return path;
 }
 
-/** Read the most recently written prompt file (using Bun.file to avoid mock interference). */
-async function readLatestPrompt(): Promise<string> {
-  const files = readdirSync(PROMPT_DIR).sort();
-  if (files.length === 0) throw new Error('No prompt files found');
-  return Bun.file(join(PROMPT_DIR, files[files.length - 1])).text();
-}
-
 function makeEffect(overrides?: Partial<CodingEffect>): CodingEffect {
   return {
     type: 'coding-task',
@@ -79,6 +73,18 @@ describe('createCodingExecutor', () => {
       expect(effect.scenario).toBe(scenario);
     }
   });
+
+  test('rejects unexpected effect type', async () => {
+    const agentStub = makeStub('agent-type-check', 'exit 0');
+    const exec = createCodingExecutor({
+      tmpDir: TEST_TMP,
+      apiKey: 'test-key',
+      agentBin: agentStub,
+    });
+
+    const badEffect = { ...makeEffect(), type: 'other-task' } as CodingEffect;
+    expect(exec(badEffect)).rejects.toThrow('Unexpected effect type');
+  });
 });
 
 describe('CodingEffect structure', () => {
@@ -109,12 +115,36 @@ describe('CodingEffect structure', () => {
 });
 
 describe('prompt assembly', () => {
-  // Strategy: run executor with a stub that succeeds quickly,
-  // then read the prompt file directly from disk to verify content.
-  // This avoids stdout pollution from parallel test files.
-
-  test('bug-fix scenario preamble in prompt file', async () => {
-    const agentStub = makeStub('agent-ok-1', 'exit 0');
+  // The stub cats the prompt file (passed via -f) to stdout so we can
+  // verify content even after the executor cleans up the temp file.
+  test.each([
+    {
+      scenario: 'bug-fix' as const,
+      expectedParts: ['Bug Fix', '先复现', 'Fix the broken test'],
+    },
+    {
+      scenario: 'debug' as const,
+      expectedParts: ['不要改任何代码', '诊断报告'],
+    },
+    {
+      scenario: 'rfc-impl' as const,
+      expectedParts: ['RFC', '逐步实现'],
+    },
+    {
+      scenario: 'ci-fix' as const,
+      expectedParts: ['CI', '失败'],
+    },
+    {
+      scenario: 'refactor' as const,
+      expectedParts: ['不改行为'],
+    },
+  ])('$scenario scenario includes expected preamble', async ({
+    scenario,
+    expectedParts,
+  }) => {
+    // Stub reads the prompt file path (last positional arg) and cats it
+    const catLastArg = 'cat "$' + '{@: -1}"';
+    const agentStub = makeStub(`agent-${scenario}`, catLastArg);
     const exec = createCodingExecutor({
       tmpDir: TEST_TMP,
       apiKey: 'test-api-key',
@@ -122,76 +152,51 @@ describe('prompt assembly', () => {
       defaultTimeoutMs: 5000,
     });
 
-    await exec(makeEffect({ scenario: 'bug-fix' }));
-    const prompt = await readLatestPrompt();
-    expect(prompt).toContain('Bug Fix');
-    expect(prompt).toContain('先复现');
-    expect(prompt).toContain('Fix the broken test');
+    const result = await exec(makeEffect({ scenario }));
+    for (const part of expectedParts) {
+      expect(result.output).toContain(part);
+    }
+  });
+});
+
+describe('temp file cleanup', () => {
+  test('prompt file is deleted after execution', async () => {
+    const agentStub = makeStub('agent-cleanup', 'exit 0');
+    const exec = createCodingExecutor({
+      tmpDir: TEST_TMP,
+      apiKey: 'test-key',
+      agentBin: agentStub,
+      defaultTimeoutMs: 5000,
+    });
+
+    await exec(makeEffect());
+
+    const remaining = existsSync(PROMPT_DIR)
+      ? readdirSync(PROMPT_DIR).filter((f) => f.startsWith('task-'))
+      : [];
+    expect(remaining.length).toBe(0);
   });
 
-  test('debug scenario includes read-only instruction', async () => {
-    const agentStub = makeStub('agent-ok-2', 'exit 0');
+  test('prompt file is deleted even on failure', async () => {
+    const agentStub = makeStub('agent-cleanup-fail', 'exit 1');
     const exec = createCodingExecutor({
       tmpDir: TEST_TMP,
-      apiKey: 'test-api-key',
+      apiKey: 'test-key',
       agentBin: agentStub,
       defaultTimeoutMs: 5000,
     });
 
-    await exec(makeEffect({ scenario: 'debug' }));
-    const prompt = await readLatestPrompt();
-    expect(prompt).toContain('不要改任何代码');
-    expect(prompt).toContain('诊断报告');
-  });
+    await exec(makeEffect());
 
-  test('rfc-impl scenario includes RFC instruction', async () => {
-    const agentStub = makeStub('agent-ok-3', 'exit 0');
-    const exec = createCodingExecutor({
-      tmpDir: TEST_TMP,
-      apiKey: 'test-api-key',
-      agentBin: agentStub,
-      defaultTimeoutMs: 5000,
-    });
-
-    await exec(makeEffect({ scenario: 'rfc-impl' }));
-    const prompt = await readLatestPrompt();
-    expect(prompt).toContain('RFC');
-    expect(prompt).toContain('逐步实现');
-  });
-
-  test('ci-fix scenario includes CI instruction', async () => {
-    const agentStub = makeStub('agent-ok-4', 'exit 0');
-    const exec = createCodingExecutor({
-      tmpDir: TEST_TMP,
-      apiKey: 'test-api-key',
-      agentBin: agentStub,
-      defaultTimeoutMs: 5000,
-    });
-
-    await exec(makeEffect({ scenario: 'ci-fix' }));
-    const prompt = await readLatestPrompt();
-    expect(prompt).toContain('CI');
-    expect(prompt).toContain('失败');
-  });
-
-  test('refactor scenario includes refactor instruction', async () => {
-    const agentStub = makeStub('agent-ok-5', 'exit 0');
-    const exec = createCodingExecutor({
-      tmpDir: TEST_TMP,
-      apiKey: 'test-api-key',
-      agentBin: agentStub,
-      defaultTimeoutMs: 5000,
-    });
-
-    await exec(makeEffect({ scenario: 'refactor' }));
-    const prompt = await readLatestPrompt();
-    expect(prompt).toContain('不改行为');
+    const remaining = existsSync(PROMPT_DIR)
+      ? readdirSync(PROMPT_DIR).filter((f) => f.startsWith('task-'))
+      : [];
+    expect(remaining.length).toBe(0);
   });
 });
 
 describe('error handling', () => {
   test('missing apiKey and failed apiKeyCommand throws', async () => {
-    // Use a command that definitely doesn't exist
     const exec = createCodingExecutor({
       tmpDir: TEST_TMP,
       apiKeyCommand: '/nonexistent/path/to/binary',
@@ -199,15 +204,9 @@ describe('error handling', () => {
       defaultTimeoutMs: 5000,
     });
 
-    try {
-      await exec(makeEffect());
-      // If it didn't throw, that's also acceptable in CI — the important
-      // thing is that the direct apiKey path works reliably.
-    } catch (err: unknown) {
-      expect((err as Error).message).toContain(
-        'Failed to resolve CURSOR_API_KEY',
-      );
-    }
+    expect(exec(makeEffect())).rejects.toThrow(
+      'Failed to resolve CURSOR_API_KEY',
+    );
   });
 
   test('agent exit code 1 returns success=false', async () => {
@@ -224,8 +223,8 @@ describe('error handling', () => {
     expect(typeof result.durationMs).toBe('number');
   });
 
-  test('agent exit code 0 returns result with durationMs', async () => {
-    const okStub = makeStub('agent-ok', 'exit 0');
+  test('agent exit code 0 returns success=true', async () => {
+    const okStub = makeStub('agent-ok', 'echo "task done"; exit 0');
     const exec = createCodingExecutor({
       tmpDir: TEST_TMP,
       apiKey: 'test-key',
@@ -234,10 +233,41 @@ describe('error handling', () => {
     });
 
     const result = await exec(makeEffect());
-    // In CI, parallel test stdout pollution may cause execSync to throw
-    // even on exit 0. The important thing is we get a result with durationMs.
-    expect(typeof result.success).toBe('boolean');
+    expect(result.success).toBe(true);
+    expect(result.output).toContain('task done');
     expect(typeof result.durationMs).toBe('number');
     expect(result.durationMs).toBeGreaterThanOrEqual(0);
   });
 });
+
+describe('API key caching', () => {
+  test('apiKey is resolved once and cached', async () => {
+    const keyStub = makeStub('key-counter', `echo "cached-key-$(date +%s%N)"`);
+    const agentStub = makeStub('agent-cache', 'exit 0');
+    const exec = createCodingExecutor({
+      tmpDir: TEST_TMP,
+      apiKeyCommand: keyStub,
+      agentBin: agentStub,
+      defaultTimeoutMs: 5000,
+    });
+
+    const r1 = await exec(makeEffect());
+    const r2 = await exec(makeEffect());
+    expect(r1.success).toBe(true);
+    expect(r2.success).toBe(true);
+  });
+
+  test('direct apiKey skips command resolution', async () => {
+    const agentStub = makeStub('agent-direct', 'exit 0');
+    const exec = createCodingExecutor({
+      tmpDir: TEST_TMP,
+      apiKey: 'direct-key',
+      apiKeyCommand: '/nonexistent/should-not-be-called',
+      agentBin: agentStub,
+      defaultTimeoutMs: 5000,
+    });
+
+    const result = await exec(makeEffect());
+    expect(result.success).toBe(true);
+  });
+});

packages/pulse/src/executors/coding-agent.ts

@@ -9,8 +9,7 @@
  * @see https://github.com/oc-xiaoju/pulse/issues/25
  */
 
-import { type ExecSyncOptions, execSync } from 'node:child_process';
-import { mkdirSync, writeFileSync } from 'node:fs';
+import { mkdirSync, unlinkSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 
@@ -109,9 +108,15 @@ export function createCodingExecutor(opts: CodingExecutorOptions = {}) {
     tmpDir = tmpdir(),
   } = opts;
 
+  let cachedApiKey: string | undefined = directApiKey;
+
   return async function executeCodingTask(
     effect: CodingEffect,
   ): Promise<CodingResult> {
+    if (effect.type !== 'coding-task') {
+      throw new Error(`Unexpected effect type: ${effect.type}`);
+    }
+
     const startTime = Date.now();
     const timeoutMs = effect.timeoutMs ?? defaultTimeoutMs;
 
@@ -128,59 +133,73 @@ export function createCodingExecutor(opts: CodingExecutorOptions = {}) {
     );
     writeFileSync(promptFile, fullPrompt, 'utf-8');
 
-    // Resolve API key
-    const apiKey = directApiKey ?? resolveApiKey(apiKeyCommand);
-
-    const execOpts: ExecSyncOptions = {
-      cwd: effect.repoDir,
-      timeout: timeoutMs,
-      encoding: 'utf-8',
-      env: {
-        ...process.env,
-        CURSOR_API_KEY: apiKey,
-      },
-      stdio: ['pipe', 'pipe', 'pipe'],
-      maxBuffer: 10 * 1024 * 1024, // 10MB
-    };
-
-    const command = `${agentBin} --yolo -p --output-format text -f ${promptFile}`;
-
     try {
-      const output = execSync(command, execOpts) as string;
-      return {
-        success: true,
-        output: output.trim(),
-        durationMs: Date.now() - startTime,
-      };
-    } catch (err: unknown) {
-      const e = err as {
-        stdout?: string;
-        stderr?: string;
-        message?: string;
-        killed?: boolean;
-      };
-      const output = [e.stdout, e.stderr, e.message]
-        .filter(Boolean)
-        .join('\n')
-        .trim();
-      return {
-        success: false,
-        output: e.killed ? `[TIMEOUT after ${timeoutMs}ms]\n${output}` : output,
-        durationMs: Date.now() - startTime,
-      };
+      // Resolve API key (lazy: resolve once, then cache)
+      if (cachedApiKey === undefined) {
+        cachedApiKey = await resolveApiKey(apiKeyCommand);
+      }
+      const apiKey = cachedApiKey;
+
+      const proc = Bun.spawn(
+        [agentBin, '--yolo', '-p', '--output-format', 'text', '-f', promptFile],
+        {
+          cwd: effect.repoDir,
+          env: { ...process.env, CURSOR_API_KEY: apiKey },
+          stdout: 'pipe',
+          stderr: 'pipe',
+        },
+      );
+
+      let timedOut = false;
+      const timer = setTimeout(() => {
+        timedOut = true;
+        proc.kill();
+      }, timeoutMs);
+
+      const exitCode = await proc.exited;
+      clearTimeout(timer);
+
+      const stdout = await new Response(proc.stdout).text();
+      const stderr = await new Response(proc.stderr).text();
+      const durationMs = Date.now() - startTime;
+
+      if (timedOut) {
+        const output = [stdout, stderr].filter(Boolean).join('\n').trim();
+        return {
+          success: false,
+          output: `[TIMEOUT after ${timeoutMs}ms]\n${output}`,
+          durationMs,
+        };
+      }
+
+      if (exitCode !== 0) {
+        const output = [stdout, stderr].filter(Boolean).join('\n').trim();
+        return { success: false, output, durationMs };
+      }
+
+      return { success: true, output: stdout.trim(), durationMs };
+    } finally {
+      try {
+        unlinkSync(promptFile);
+      } catch {}
     }
   };
 }
 
 // ── Helpers ────────────────────────────────────────────────────
 
-function resolveApiKey(command: string): string {
+async function resolveApiKey(command: string): Promise<string> {
   try {
-    return execSync(command, {
-      encoding: 'utf-8',
-      timeout: 10_000,
-      stdio: ['pipe', 'pipe', 'pipe'],
-    }).trim();
+    const proc = Bun.spawn(['sh', '-c', command], {
+      stdout: 'pipe',
+      stderr: 'pipe',
+    });
+    const exitCode = await proc.exited;
+    if (exitCode !== 0) {
+      throw new Error(`exit code ${exitCode}`);
+    }
+    const output = await new Response(proc.stdout).text();
+    return output.trim();
   } catch {
     throw new Error(`Failed to resolve CURSOR_API_KEY via: ${command}`);
   }