refactor: extract validateCount, replace CLI spawn with direct import #63

Merged
xiaomo merged 1 commits from chore/61-spawn-to-direct-import into main 2026-06-04 12:41:43 +00:00
2 changed files with 32 additions and 32 deletions
@@ -2,10 +2,15 @@ import { execFileSync } from "node:child_process";
import { dirname, join } from "node:path"; import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url"; import { fileURLToPath } from "node:url";
import { describe, expect, test } from "vitest"; import { describe, expect, test } from "vitest";
import { validateCount } from "../commands/thread.js";
const CLI_PATH = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "dist", "cli.js"); const CLI_PATH = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "dist", "cli.js");
function runCli(args: string[]): { stdout: string; stderr: string; exitCode: number } { function runCli(args: string[]): {
stdout: string;
stderr: string;
exitCode: number;
} {
try { try {
const stdout = execFileSync("node", [CLI_PATH, ...args], { const stdout = execFileSync("node", [CLI_PATH, ...args], {
encoding: "utf8", encoding: "utf8",
@@ -14,7 +19,11 @@ function runCli(args: string[]): { stdout: string; stderr: string; exitCode: num
}); });
return { stdout, stderr: "", exitCode: 0 }; return { stdout, stderr: "", exitCode: 0 };
} catch (e: unknown) { } catch (e: unknown) {
const err = e as NodeJS.ErrnoException & { stdout?: string; stderr?: string; status?: number }; const err = e as NodeJS.ErrnoException & {
stdout?: string;
stderr?: string;
status?: number;
};
return { return {
stdout: err.stdout ?? "", stdout: err.stdout ?? "",
stderr: err.stderr ?? "", stderr: err.stderr ?? "",
@@ -23,52 +32,39 @@ function runCli(args: string[]): { stdout: string; stderr: string; exitCode: num
} }
} }
describe("thread exec --count CLI parsing", () => { describe("thread exec --count CLI parsing", { timeout: 30_000 }, () => {
test("--help shows -c/--count option", { timeout: 30_000 }, () => { test("--help shows -c/--count option", () => {
const result = runCli(["thread", "exec", "--help"]); const result = runCli(["thread", "exec", "--help"]);
const combined = result.stdout + result.stderr; const combined = result.stdout + result.stderr;
expect(combined).toContain("--count"); expect(combined).toContain("--count");
expect(combined).toContain("-c"); expect(combined).toContain("-c");
}); });
test("description says 'one or more steps'", { timeout: 30_000 }, () => { test("description says 'one or more steps'", () => {
const result = runCli(["thread", "exec", "--help"]); const result = runCli(["thread", "exec", "--help"]);
const combined = result.stdout + result.stderr; const combined = result.stdout + result.stderr;
expect(combined).toContain("one or more steps"); expect(combined).toContain("one or more steps");
}); });
}); });
describe("cmdThreadExec count logic", { timeout: 30_000 }, () => { describe("validateCount", () => {
test("count=0 fails with validation error", () => { test("count=0 throws validation error", () => {
const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "0"]); expect(() => validateCount(0)).toThrow("positive integer");
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
}); });
test("negative count fails with validation error", () => { test("negative count throws validation error", () => {
const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "-1"]); expect(() => validateCount(-1)).toThrow("positive integer");
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
}); });
test("non-integer count fails with validation error", () => { test("non-integer count throws validation error", () => {
const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "1.5"]); expect(() => validateCount(1.5)).toThrow("positive integer");
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
}); });
test("count=1 is the default (no -c flag)", () => { test("count=1 passes validation", () => {
// Without -c, it should attempt to run 1 step (failing on missing thread, not on count validation) expect(() => validateCount(1)).not.toThrow();
const result = runCli(["thread", "exec", "FAKE_THREAD_ID"]);
expect(result.exitCode).not.toBe(0);
// Should NOT contain "positive integer" error — should fail on thread lookup instead
expect(result.stderr).not.toContain("positive integer");
}); });
test("count=3 passes validation (fails on thread lookup)", () => { test("count=3 passes validation", () => {
const result = runCli(["thread", "exec", "FAKE_THREAD_ID", "-c", "3"]); expect(() => validateCount(3)).not.toThrow();
expect(result.exitCode).not.toBe(0);
// Should NOT contain "positive integer" error — should fail on thread/storage lookup
expect(result.stderr).not.toContain("positive integer");
}); });
}); });
+7 -3
View File
@@ -1128,6 +1128,12 @@ export async function cmdThreadResume(
}); });
} }
export function validateCount(count: number): void {
if (count < 1 || !Number.isInteger(count)) {
throw new Error(`--count must be a positive integer, got: ${count}`);
}
}
export async function cmdThreadExec( export async function cmdThreadExec(
storageRoot: string, storageRoot: string,
threadId: ThreadId, threadId: ThreadId,
@@ -1136,9 +1142,7 @@ export async function cmdThreadExec(
background: boolean, background: boolean,
backgroundWorker: boolean, backgroundWorker: boolean,
): Promise<StepOutput[]> { ): Promise<StepOutput[]> {
if (count < 1 || !Number.isInteger(count)) { validateCount(count);
fail(`--count must be a positive integer, got: ${count}`);
}
// Check if thread is already running in background (unless we ARE the background worker) // Check if thread is already running in background (unless we ARE the background worker)
if (!backgroundWorker) { if (!backgroundWorker) {