refactor(FN-1615): convert CLI execSync to async exec
- Replace execSync with promisified execAsync in init.ts detectProjectName() for non-blocking git remote lookups - Replace execSync with promisified execAsync in task-lifecycle.ts cleanupMergedTaskArtifacts() for non-blocking git worktree/branch cleanup - Add 30s timeout to git operations to prevent indefinite hangs - Update test mocks to support both callback-style and promise-style exec usage
This commit is contained in:
@@ -163,7 +163,14 @@ const {
|
||||
mockGetPrMergeStatus,
|
||||
mockMergePr,
|
||||
} = vi.hoisted(() => ({
|
||||
mockExec: vi.fn((_command: string, callback?: () => void) => callback?.()),
|
||||
mockExec: vi.fn((_command: string, _options?: any, callback?: (err: null, stdout: string, stderr: string) => void) => {
|
||||
// Handle both callback-style (original exec) and promise-style (promisified execAsync)
|
||||
if (typeof callback === "function") {
|
||||
callback(null, "", "");
|
||||
}
|
||||
// Return resolved promise for promisified usage
|
||||
return Promise.resolve({ stdout: "", stderr: "" });
|
||||
}),
|
||||
mockExecSync: vi.fn(() => ""),
|
||||
mockFindPrForBranch: vi.fn(),
|
||||
mockCreatePr: vi.fn(),
|
||||
@@ -547,8 +554,9 @@ describe("processPullRequestMergeTask", () => {
|
||||
expect(result).toBe("merged");
|
||||
expect(mockMergePr).toHaveBeenCalledWith({ number: 42, method: "squash" });
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-093", "done");
|
||||
expect(mockExecSync).toHaveBeenCalledWith('git worktree remove "/tmp/kb-093" --force', expect.any(Object));
|
||||
expect(mockExecSync).toHaveBeenCalledWith('git branch -d "fusion/fn-093"', expect.any(Object));
|
||||
// Check that exec was called with the expected commands (options object and callback may follow)
|
||||
expect(mockExec.mock.calls.some((call) => call[0] === 'git worktree remove "/tmp/kb-093" --force')).toBe(true);
|
||||
expect(mockExec.mock.calls.some((call) => call[0] === 'git branch -d "fusion/fn-093"')).toBe(true);
|
||||
});
|
||||
|
||||
it("does not merge when required checks or reviews are blocking", async () => {
|
||||
|
||||
@@ -11,7 +11,9 @@
|
||||
import { existsSync, mkdirSync, writeFileSync } from "node:fs";
|
||||
import { join, resolve, basename } from "node:path";
|
||||
import { homedir } from "node:os";
|
||||
import { execSync } from "node:child_process";
|
||||
import { exec } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
const execAsync = promisify(exec);
|
||||
import { CentralCore } from "@fusion/core";
|
||||
import { resolveGlobalDir } from "@fusion/core";
|
||||
|
||||
@@ -53,7 +55,7 @@ export async function runInit(options: InitOptions = {}): Promise<void> {
|
||||
}
|
||||
|
||||
// Has .fusion/ but not registered - offer to register
|
||||
const projectName = options.name ?? detectProjectName(cwd);
|
||||
const projectName = options.name ?? await detectProjectName(cwd);
|
||||
console.log(`⚠ Project directory exists but not registered.`);
|
||||
console.log(` Run: fn project add ${projectName} ${cwd}`);
|
||||
console.log(` Or: rm -rf ${fusionDir} && fn init`);
|
||||
@@ -62,7 +64,7 @@ export async function runInit(options: InitOptions = {}): Promise<void> {
|
||||
}
|
||||
|
||||
// Get or generate project name
|
||||
const projectName = options.name ?? detectProjectName(cwd);
|
||||
const projectName = options.name ?? await detectProjectName(cwd);
|
||||
|
||||
console.log(`Initializing fn project: "${projectName}"`);
|
||||
console.log(` Path: ${cwd}`);
|
||||
@@ -133,18 +135,19 @@ export async function runInit(options: InitOptions = {}): Promise<void> {
|
||||
/**
|
||||
* Detect a project name from git remote or directory name.
|
||||
*/
|
||||
function detectProjectName(dir: string): string {
|
||||
async function detectProjectName(dir: string): Promise<string> {
|
||||
// Try git remote first
|
||||
try {
|
||||
const remoteUrl = execSync("git remote get-url origin 2>/dev/null", {
|
||||
const { stdout: remoteUrl } = await execAsync("git remote get-url origin", {
|
||||
cwd: dir,
|
||||
encoding: "utf-8",
|
||||
}).trim();
|
||||
timeout: 10_000,
|
||||
});
|
||||
|
||||
if (remoteUrl) {
|
||||
const trimmed = remoteUrl.trim();
|
||||
if (trimmed) {
|
||||
// Extract repo name from URL
|
||||
// Handles: https://github.com/user/repo.git, git@github.com:user/repo.git
|
||||
const match = remoteUrl.match(/[:/]([^/]+)\/([^/.]+?)(?:\.git)?$/);
|
||||
const match = trimmed.match(/[:/]([^/]+)\/([^/.]+?)(?:\.git)?$/);
|
||||
if (match) {
|
||||
return match[2];
|
||||
}
|
||||
|
||||
@@ -13,7 +13,9 @@
|
||||
* - Full PR lifecycle orchestration (create → status check → merge)
|
||||
*/
|
||||
|
||||
import { execSync } from "node:child_process";
|
||||
import { exec } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
const execAsync = promisify(exec);
|
||||
import type { TaskStore } from "@fusion/core";
|
||||
import type { Settings, TaskDetail, PrInfo } from "@fusion/core";
|
||||
|
||||
@@ -75,14 +77,14 @@ function buildPullRequestBody(task: Pick<TaskDetail, "id" | "description">): str
|
||||
* Clean up worktree and branch artifacts after a successful merge.
|
||||
* Both operations are best-effort; errors are logged but don't propagate.
|
||||
*/
|
||||
export function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "id" | "worktree">): void {
|
||||
export async function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "id" | "worktree">): Promise<void> {
|
||||
const branch = getTaskBranchName(task.id);
|
||||
|
||||
if (task.worktree) {
|
||||
try {
|
||||
execSync(`git worktree remove "${task.worktree}" --force`, {
|
||||
await execAsync(`git worktree remove "${task.worktree}" --force`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
timeout: 30_000,
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup — worktree may already be gone.
|
||||
@@ -90,15 +92,15 @@ export function cleanupMergedTaskArtifacts(cwd: string, task: Pick<TaskDetail, "
|
||||
}
|
||||
|
||||
try {
|
||||
execSync(`git branch -d "${branch}"`, {
|
||||
await execAsync(`git branch -d "${branch}"`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
timeout: 30_000,
|
||||
});
|
||||
} catch {
|
||||
try {
|
||||
execSync(`git branch -D "${branch}"`, {
|
||||
await execAsync(`git branch -D "${branch}"`, {
|
||||
cwd,
|
||||
stdio: "pipe",
|
||||
timeout: 30_000,
|
||||
});
|
||||
} catch {
|
||||
// Best-effort cleanup — branch may already be gone.
|
||||
@@ -186,7 +188,7 @@ export async function processPullRequestMergeTask(
|
||||
await store.updatePrInfo(task.id, refreshedPrInfo);
|
||||
|
||||
if (mergeStatus.prInfo.status === "merged") {
|
||||
cleanupMergedTaskArtifacts(cwd, task);
|
||||
await cleanupMergedTaskArtifacts(cwd, task);
|
||||
await store.moveTask(task.id, "done");
|
||||
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
|
||||
await store.logEntry(task.id, "Pull request merged", `PR #${prInfo.number}: ${prInfo.url}`);
|
||||
@@ -205,7 +207,7 @@ export async function processPullRequestMergeTask(
|
||||
await store.updateTask(task.id, { status: "merging-pr" });
|
||||
const mergedPr = await github.mergePr({ number: prInfo.number, method: "squash" });
|
||||
await store.updatePrInfo(task.id, { ...mergedPr, lastCheckedAt: new Date().toISOString() });
|
||||
cleanupMergedTaskArtifacts(cwd, task);
|
||||
await cleanupMergedTaskArtifacts(cwd, task);
|
||||
await store.moveTask(task.id, "done");
|
||||
await store.updateTask(task.id, { status: null, mergeRetries: 0 });
|
||||
await store.logEntry(task.id, "Pull request merged", `PR #${mergedPr.number}: ${mergedPr.url}`);
|
||||
|
||||
Reference in New Issue
Block a user