Revert "feat: integrate Claude Code SDK to replace process spawning (#327)" (#335)

* Revert "feat: integrate Claude Code SDK to replace process spawning (#327)"

This reverts commit 204266ca45.

* 1.0.59

base-action/test/run-claude.test.ts

@@ -1,260 +1,297 @@
 #!/usr/bin/env bun
 
-import {
-  describe,
-  test,
-  expect,
-  beforeAll,
-  afterAll,
-  afterEach,
-} from "bun:test";
-import {
-  runClaude,
-  type ClaudeOptions,
-  parseCustomEnvVars,
-  parseTools,
-  parseMcpConfig,
-} from "../src/run-claude";
-import { writeFile, unlink } from "fs/promises";
-import { join } from "path";
+import { describe, test, expect } from "bun:test";
+import { prepareRunConfig, type ClaudeOptions } from "../src/run-claude";
 
-// Since we can't easily mock the SDK, let's focus on testing input validation
-// and error cases that happen before the SDK is called
+describe("prepareRunConfig", () => {
+  test("should prepare config with basic arguments", () => {
+    const options: ClaudeOptions = {};
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
 
-describe("runClaude input validation", () => {
-  const testPromptPath = join(
-    process.env.RUNNER_TEMP || "/tmp",
-    "test-prompt-claude.txt",
-  );
-
-  // Create a test prompt file before tests
-  beforeAll(async () => {
-    await writeFile(testPromptPath, "Test prompt content");
+    expect(prepared.claudeArgs.slice(0, 4)).toEqual([
+      "-p",
+      "--verbose",
+      "--output-format",
+      "stream-json",
+    ]);
   });
 
-  // Clean up after tests
-  afterAll(async () => {
-    try {
-      await unlink(testPromptPath);
-    } catch (e) {
-      // Ignore if file doesn't exist
-    }
+  test("should include promptPath", () => {
+    const options: ClaudeOptions = {};
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.promptPath).toBe("/tmp/test-prompt.txt");
+  });
+
+  test("should include allowed tools in command arguments", () => {
+    const options: ClaudeOptions = {
+      allowedTools: "Bash,Read",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--allowedTools");
+    expect(prepared.claudeArgs).toContain("Bash,Read");
+  });
+
+  test("should include disallowed tools in command arguments", () => {
+    const options: ClaudeOptions = {
+      disallowedTools: "Bash,Read",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--disallowedTools");
+    expect(prepared.claudeArgs).toContain("Bash,Read");
+  });
+
+  test("should include max turns in command arguments", () => {
+    const options: ClaudeOptions = {
+      maxTurns: "5",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--max-turns");
+    expect(prepared.claudeArgs).toContain("5");
+  });
+
+  test("should include mcp config in command arguments", () => {
+    const options: ClaudeOptions = {
+      mcpConfig: "/path/to/mcp-config.json",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--mcp-config");
+    expect(prepared.claudeArgs).toContain("/path/to/mcp-config.json");
+  });
+
+  test("should include system prompt in command arguments", () => {
+    const options: ClaudeOptions = {
+      systemPrompt: "You are a senior backend engineer.",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--system-prompt");
+    expect(prepared.claudeArgs).toContain("You are a senior backend engineer.");
+  });
+
+  test("should include append system prompt in command arguments", () => {
+    const options: ClaudeOptions = {
+      appendSystemPrompt:
+        "After writing code, be sure to code review yourself.",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--append-system-prompt");
+    expect(prepared.claudeArgs).toContain(
+      "After writing code, be sure to code review yourself.",
+    );
+  });
+
+  test("should include fallback model in command arguments", () => {
+    const options: ClaudeOptions = {
+      fallbackModel: "claude-sonnet-4-20250514",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toContain("--fallback-model");
+    expect(prepared.claudeArgs).toContain("claude-sonnet-4-20250514");
+  });
+
+  test("should use provided prompt path", () => {
+    const options: ClaudeOptions = {};
+    const prepared = prepareRunConfig("/custom/prompt/path.txt", options);
+
+    expect(prepared.promptPath).toBe("/custom/prompt/path.txt");
+  });
+
+  test("should not include optional arguments when not set", () => {
+    const options: ClaudeOptions = {};
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).not.toContain("--allowedTools");
+    expect(prepared.claudeArgs).not.toContain("--disallowedTools");
+    expect(prepared.claudeArgs).not.toContain("--max-turns");
+    expect(prepared.claudeArgs).not.toContain("--mcp-config");
+    expect(prepared.claudeArgs).not.toContain("--system-prompt");
+    expect(prepared.claudeArgs).not.toContain("--append-system-prompt");
+    expect(prepared.claudeArgs).not.toContain("--fallback-model");
+  });
+
+  test("should preserve order of claude arguments", () => {
+    const options: ClaudeOptions = {
+      allowedTools: "Bash,Read",
+      maxTurns: "3",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toEqual([
+      "-p",
+      "--verbose",
+      "--output-format",
+      "stream-json",
+      "--allowedTools",
+      "Bash,Read",
+      "--max-turns",
+      "3",
+    ]);
+  });
+
+  test("should preserve order with all options including fallback model", () => {
+    const options: ClaudeOptions = {
+      allowedTools: "Bash,Read",
+      disallowedTools: "Write",
+      maxTurns: "3",
+      mcpConfig: "/path/to/config.json",
+      systemPrompt: "You are a helpful assistant",
+      appendSystemPrompt: "Be concise",
+      fallbackModel: "claude-sonnet-4-20250514",
+    };
+    const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+
+    expect(prepared.claudeArgs).toEqual([
+      "-p",
+      "--verbose",
+      "--output-format",
+      "stream-json",
+      "--allowedTools",
+      "Bash,Read",
+      "--disallowedTools",
+      "Write",
+      "--max-turns",
+      "3",
+      "--mcp-config",
+      "/path/to/config.json",
+      "--system-prompt",
+      "You are a helpful assistant",
+      "--append-system-prompt",
+      "Be concise",
+      "--fallback-model",
+      "claude-sonnet-4-20250514",
+    ]);
   });
 
   describe("maxTurns validation", () => {
-    test("should throw error for non-numeric maxTurns", async () => {
+    test("should accept valid maxTurns value", () => {
+      const options: ClaudeOptions = { maxTurns: "5" };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.claudeArgs).toContain("--max-turns");
+      expect(prepared.claudeArgs).toContain("5");
+    });
+
+    test("should throw error for non-numeric maxTurns", () => {
       const options: ClaudeOptions = { maxTurns: "abc" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "maxTurns must be a positive number, got: abc",
       );
     });
 
-    test("should throw error for negative maxTurns", async () => {
+    test("should throw error for negative maxTurns", () => {
       const options: ClaudeOptions = { maxTurns: "-1" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "maxTurns must be a positive number, got: -1",
       );
     });
 
-    test("should throw error for zero maxTurns", async () => {
+    test("should throw error for zero maxTurns", () => {
       const options: ClaudeOptions = { maxTurns: "0" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "maxTurns must be a positive number, got: 0",
       );
     });
   });
 
   describe("timeoutMinutes validation", () => {
-    test("should throw error for non-numeric timeoutMinutes", async () => {
+    test("should accept valid timeoutMinutes value", () => {
+      const options: ClaudeOptions = { timeoutMinutes: "15" };
+      expect(() =>
+        prepareRunConfig("/tmp/test-prompt.txt", options),
+      ).not.toThrow();
+    });
+
+    test("should throw error for non-numeric timeoutMinutes", () => {
       const options: ClaudeOptions = { timeoutMinutes: "abc" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "timeoutMinutes must be a positive number, got: abc",
       );
     });
 
-    test("should throw error for negative timeoutMinutes", async () => {
+    test("should throw error for negative timeoutMinutes", () => {
       const options: ClaudeOptions = { timeoutMinutes: "-5" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "timeoutMinutes must be a positive number, got: -5",
       );
     });
 
-    test("should throw error for zero timeoutMinutes", async () => {
+    test("should throw error for zero timeoutMinutes", () => {
       const options: ClaudeOptions = { timeoutMinutes: "0" };
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
+      expect(() => prepareRunConfig("/tmp/test-prompt.txt", options)).toThrow(
         "timeoutMinutes must be a positive number, got: 0",
       );
     });
   });
 
-  describe("environment variable validation from INPUT_TIMEOUT_MINUTES", () => {
-    const originalEnv = process.env.INPUT_TIMEOUT_MINUTES;
-
-    afterEach(() => {
-      // Restore original value
-      if (originalEnv !== undefined) {
-        process.env.INPUT_TIMEOUT_MINUTES = originalEnv;
-      } else {
-        delete process.env.INPUT_TIMEOUT_MINUTES;
-      }
+  describe("custom environment variables", () => {
+    test("should parse empty claudeEnv correctly", () => {
+      const options: ClaudeOptions = { claudeEnv: "" };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({});
     });
 
-    test("should throw error for invalid INPUT_TIMEOUT_MINUTES", async () => {
-      process.env.INPUT_TIMEOUT_MINUTES = "invalid";
+    test("should parse single environment variable", () => {
+      const options: ClaudeOptions = { claudeEnv: "API_KEY: secret123" };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({ API_KEY: "secret123" });
+    });
+
+    test("should parse multiple environment variables", () => {
+      const options: ClaudeOptions = {
+        claudeEnv: "API_KEY: secret123\nDEBUG: true\nUSER: testuser",
+      };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({
+        API_KEY: "secret123",
+        DEBUG: "true",
+        USER: "testuser",
+      });
+    });
+
+    test("should handle environment variables with spaces around values", () => {
+      const options: ClaudeOptions = {
+        claudeEnv: "API_KEY:  secret123  \n  DEBUG  :  true  ",
+      };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({
+        API_KEY: "secret123",
+        DEBUG: "true",
+      });
+    });
+
+    test("should skip empty lines and comments", () => {
+      const options: ClaudeOptions = {
+        claudeEnv:
+          "API_KEY: secret123\n\n# This is a comment\nDEBUG: true\n# Another comment",
+      };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({
+        API_KEY: "secret123",
+        DEBUG: "true",
+      });
+    });
+
+    test("should skip lines without colons", () => {
+      const options: ClaudeOptions = {
+        claudeEnv: "API_KEY: secret123\nINVALID_LINE\nDEBUG: true",
+      };
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({
+        API_KEY: "secret123",
+        DEBUG: "true",
+      });
+    });
+
+    test("should handle undefined claudeEnv", () => {
       const options: ClaudeOptions = {};
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
-        "INPUT_TIMEOUT_MINUTES must be a positive number, got: invalid",
-      );
+      const prepared = prepareRunConfig("/tmp/test-prompt.txt", options);
+      expect(prepared.env).toEqual({});
     });
-
-    test("should throw error for zero INPUT_TIMEOUT_MINUTES", async () => {
-      process.env.INPUT_TIMEOUT_MINUTES = "0";
-      const options: ClaudeOptions = {};
-      await expect(runClaude(testPromptPath, options)).rejects.toThrow(
-        "INPUT_TIMEOUT_MINUTES must be a positive number, got: 0",
-      );
-    });
-  });
-
-  // Note: We can't easily test the full execution flow without either:
-  // 1. Mocking the SDK (which seems difficult with Bun's current mocking capabilities)
-  // 2. Having a valid API key and actually calling the API (not suitable for unit tests)
-  // 3. Refactoring the code to be more testable (e.g., dependency injection)
-
-  // For now, we're testing what we can: input validation that happens before the SDK call
-});
-
-describe("parseCustomEnvVars", () => {
-  test("should parse empty string correctly", () => {
-    expect(parseCustomEnvVars("")).toEqual({});
-  });
-
-  test("should parse single environment variable", () => {
-    expect(parseCustomEnvVars("API_KEY: secret123")).toEqual({
-      API_KEY: "secret123",
-    });
-  });
-
-  test("should parse multiple environment variables", () => {
-    const input = "API_KEY: secret123\nDEBUG: true\nUSER: testuser";
-    expect(parseCustomEnvVars(input)).toEqual({
-      API_KEY: "secret123",
-      DEBUG: "true",
-      USER: "testuser",
-    });
-  });
-
-  test("should handle environment variables with spaces around values", () => {
-    const input = "API_KEY:  secret123  \n  DEBUG  :  true  ";
-    expect(parseCustomEnvVars(input)).toEqual({
-      API_KEY: "secret123",
-      DEBUG: "true",
-    });
-  });
-
-  test("should skip empty lines and comments", () => {
-    const input =
-      "API_KEY: secret123\n\n# This is a comment\nDEBUG: true\n# Another comment";
-    expect(parseCustomEnvVars(input)).toEqual({
-      API_KEY: "secret123",
-      DEBUG: "true",
-    });
-  });
-
-  test("should skip lines without colons", () => {
-    const input = "API_KEY: secret123\nINVALID_LINE\nDEBUG: true";
-    expect(parseCustomEnvVars(input)).toEqual({
-      API_KEY: "secret123",
-      DEBUG: "true",
-    });
-  });
-
-  test("should handle undefined input", () => {
-    expect(parseCustomEnvVars(undefined)).toEqual({});
-  });
-
-  test("should handle whitespace-only input", () => {
-    expect(parseCustomEnvVars("  \n  \t  ")).toEqual({});
-  });
-});
-
-describe("parseTools", () => {
-  test("should return undefined for empty string", () => {
-    expect(parseTools("")).toBeUndefined();
-  });
-
-  test("should return undefined for whitespace-only string", () => {
-    expect(parseTools("  \t  ")).toBeUndefined();
-  });
-
-  test("should return undefined for undefined input", () => {
-    expect(parseTools(undefined)).toBeUndefined();
-  });
-
-  test("should parse single tool", () => {
-    expect(parseTools("Bash")).toEqual(["Bash"]);
-  });
-
-  test("should parse multiple tools", () => {
-    expect(parseTools("Bash,Read,Write")).toEqual(["Bash", "Read", "Write"]);
-  });
-
-  test("should trim whitespace around tools", () => {
-    expect(parseTools(" Bash , Read , Write ")).toEqual([
-      "Bash",
-      "Read",
-      "Write",
-    ]);
-  });
-
-  test("should filter out empty tool names", () => {
-    expect(parseTools("Bash,,Read,,,Write")).toEqual(["Bash", "Read", "Write"]);
-  });
-});
-
-describe("parseMcpConfig", () => {
-  test("should return undefined for empty string", () => {
-    expect(parseMcpConfig("")).toBeUndefined();
-  });
-
-  test("should return undefined for whitespace-only string", () => {
-    expect(parseMcpConfig("  \t  ")).toBeUndefined();
-  });
-
-  test("should return undefined for undefined input", () => {
-    expect(parseMcpConfig(undefined)).toBeUndefined();
-  });
-
-  test("should parse valid JSON", () => {
-    const config = { "test-server": { command: "test", args: ["--test"] } };
-    expect(parseMcpConfig(JSON.stringify(config))).toEqual(config);
-  });
-
-  test("should return undefined for invalid JSON", () => {
-    // Check console warning is logged
-    const originalWarn = console.warn;
-    const warnings: string[] = [];
-    console.warn = (msg: string) => warnings.push(msg);
-
-    expect(parseMcpConfig("{ invalid json")).toBeUndefined();
-
-    console.warn = originalWarn;
-  });
-
-  test("should parse complex MCP config", () => {
-    const config = {
-      "github-mcp": {
-        command: "npx",
-        args: ["-y", "@modelcontextprotocol/server-github"],
-        env: {
-          GITHUB_TOKEN: "test-token",
-        },
-      },
-      "filesystem-mcp": {
-        command: "npx",
-        args: ["-y", "@modelcontextprotocol/server-filesystem", "/tmp"],
-      },
-    };
-    expect(parseMcpConfig(JSON.stringify(config))).toEqual(config);
   });
 });