From beee4a17def944848578ba4ee61b2923479fe13c Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 5 Feb 2026 14:45:24 +0900 Subject: [PATCH 1/4] refactor(lib): use runGitCommand helper instead of Bun.spawn Replace direct Bun.spawn calls with the centralized runGitCommand helper in git-workflow.ts and repo-manager.ts for consistency with the codebase. - getAllLinkedBranches(): use runGitCommand for git and gh api calls - startDevelopWorkflow(): use runGitCommand for gh issue develop - fetchBranch(): use runGitCommand for git fetch and branch operations - isInGitRepo(): use runGitCommand for git rev-parse - getGitDir(): use runGitCommand for git rev-parse - cloneBareRepo(): use runGitCommand for gh repo clone - getCurrentRepoInfo(): use runGitCommand for gh repo view Issue: #267 --- src/lib/git-workflow.ts | 82 +++++++++++------------------------------ src/lib/repo-manager.ts | 52 +++++++------------------- 2 files changed, 35 insertions(+), 99 deletions(-) diff --git a/src/lib/git-workflow.ts b/src/lib/git-workflow.ts index b4ac770..a318f0b 100644 --- a/src/lib/git-workflow.ts +++ b/src/lib/git-workflow.ts @@ -1,5 +1,6 @@ import type { DevelopOptions } from '../types' import { getGhCommand } from './gh-command' +import { runGitCommand } from './git-exec' // Re-export worktree functions from dedicated module export { @@ -18,14 +19,9 @@ export async function getAllLinkedBranches( ): Promise { if (!repo) { // Try to get from current git repo - const proc = Bun.spawn(['git', 'rev-parse', '--show-toplevel'], { - stdout: 'pipe', - stderr: 'pipe', - }) - await new Response(proc.stdout).text() - const exitCode = await proc.exited - - if (exitCode !== 0) { + const result = await runGitCommand(['git', 'rev-parse', '--show-toplevel']) + + if (result.exitCode !== 0) { // Cannot determine repo, return empty array return [] } @@ -72,25 +68,18 @@ export async function getAllLinkedBranches( `issueNumber=${issueNumber}`, ] - const proc = Bun.spawn(args, { - stdout: 'pipe', - stderr: 'pipe', - }) - - const output = await new Response(proc.stdout).text() - const stderr = await new Response(proc.stderr).text() - const exitCode = await proc.exited + const result = await runGitCommand(args) - if (exitCode !== 0) { + if (result.exitCode !== 0) { // GraphQL query failed - if (stderr.trim()) { - console.warn(`⚠️ Failed to get linked branches: ${stderr.trim()}`) + if (result.stderr.trim()) { + console.warn(`⚠️ Failed to get linked branches: ${result.stderr.trim()}`) } return [] } try { - const data = JSON.parse(output) + const data = JSON.parse(result.stdout) const branches = data?.data?.repository?.issue?.linkedBranches?.nodes || [] return branches .map((node: unknown) => { @@ -142,28 +131,21 @@ export async function startDevelopWorkflow( args.push('-n', options.name) } - const proc = Bun.spawn(args, { - stdout: 'pipe', - stderr: 'pipe', - }) - - const output = await new Response(proc.stdout).text() - const exitCode = await proc.exited + const result = await runGitCommand(args) - if (exitCode !== 0) { - const error = await new Response(proc.stderr).text() - throw new Error(`Failed to develop issue: ${error.trim()}`) + if (result.exitCode !== 0) { + throw new Error(`Failed to develop issue: ${result.stderr.trim()}`) } // Parse branch name from gh issue develop output // With --checkout: "Switched to a new branch 'feat-123-title'" - const branchMatch = output.match(/'([^']+)'/) + const branchMatch = result.stdout.match(/'([^']+)'/) if (branchMatch) { return branchMatch[1]! } // Without --checkout: "github.com/owner/repo/tree/branch-name" - const urlMatch = output.match(/\/tree\/(\S+)/) + const urlMatch = result.stdout.match(/\/tree\/(\S+)/) if (urlMatch) { return urlMatch[1]!.trim() } @@ -188,48 +170,28 @@ export async function fetchBranch( branch: string, ): Promise { // Step 1: Fetch to remote tracking branch (refs/remotes/origin/{branch}) - const fetchProc = Bun.spawn( + const fetchResult = await runGitCommand( ['git', '-C', bareRepoPath, 'fetch', 'origin', `${branch}:refs/remotes/origin/${branch}`], - { - stdout: 'pipe', - stderr: 'pipe', - }, ) - const fetchExitCode = await fetchProc.exited - - if (fetchExitCode !== 0) { - const error = await new Response(fetchProc.stderr).text() - throw new Error(`Failed to fetch branch: ${error.trim()}`) + if (fetchResult.exitCode !== 0) { + throw new Error(`Failed to fetch branch: ${fetchResult.stderr.trim()}`) } // Step 2: Create or update local branch from remote tracking branch // Try to create the branch first - const createProc = Bun.spawn( + const createResult = await runGitCommand( ['git', '-C', bareRepoPath, 'branch', branch, `refs/remotes/origin/${branch}`], - { - stdout: 'pipe', - stderr: 'pipe', - }, ) - const createExitCode = await createProc.exited - - if (createExitCode !== 0) { + if (createResult.exitCode !== 0) { // Branch might already exist, try to update it - const updateProc = Bun.spawn( + const updateResult = await runGitCommand( ['git', '-C', bareRepoPath, 'branch', '-f', branch, `refs/remotes/origin/${branch}`], - { - stdout: 'pipe', - stderr: 'pipe', - }, ) - const updateExitCode = await updateProc.exited - - if (updateExitCode !== 0) { - const error = await new Response(updateProc.stderr).text() - throw new Error(`Failed to update local branch: ${error.trim()}`) + if (updateResult.exitCode !== 0) { + throw new Error(`Failed to update local branch: ${updateResult.stderr.trim()}`) } } } diff --git a/src/lib/repo-manager.ts b/src/lib/repo-manager.ts index 8e6f74c..d102f69 100644 --- a/src/lib/repo-manager.ts +++ b/src/lib/repo-manager.ts @@ -3,6 +3,7 @@ import * as fs from 'node:fs' import * as os from 'node:os' import * as path from 'node:path' import { getGhCommand } from './gh-command' +import { runGitCommand } from './git-exec' /** * Parse repository string in format "owner/repo" or GitHub URL @@ -51,13 +52,8 @@ export async function findBareRepo(owner: string, repo: string): Promise { - const proc = Bun.spawn(['git', 'rev-parse', '--git-dir'], { - stdout: 'pipe', - stderr: 'pipe', - }) - - const exitCode = await proc.exited - return exitCode === 0 + const result = await runGitCommand(['git', 'rev-parse', '--git-dir']) + return result.exitCode === 0 } /** @@ -65,21 +61,13 @@ export async function isInGitRepo(): Promise { * Returns absolute path if in a git repo, null otherwise */ export async function getGitDir(): Promise { - const proc = Bun.spawn(['git', 'rev-parse', '--absolute-git-dir'], { - stdout: 'pipe', - stderr: 'pipe', - }) - - const output = await new Response(proc.stdout).text() - const exitCode = await proc.exited + const result = await runGitCommand(['git', 'rev-parse', '--absolute-git-dir']) - if (exitCode !== 0) { - // Consume stderr to prevent potential pipe issues - await new Response(proc.stderr).text() + if (result.exitCode !== 0) { return null } - return output.trim() + return result.stdout.trim() } /** @@ -98,19 +86,12 @@ export async function cloneBareRepo(owner: string, repo: string): Promise { - const proc = Bun.spawn([getGhCommand(), 'repo', 'view', '--json', 'owner,name'], { - stdout: 'pipe', - stderr: 'pipe', - }) - - const output = await new Response(proc.stdout).text() - const exitCode = await proc.exited + const result = await runGitCommand([getGhCommand(), 'repo', 'view', '--json', 'owner,name']) - if (exitCode !== 0) { - const error = await new Response(proc.stderr).text() - throw new Error(`Failed to get repository info: ${error.trim() || 'Not in a git repository'}`) + if (result.exitCode !== 0) { + throw new Error(`Failed to get repository info: ${result.stderr.trim() || 'Not in a git repository'}`) } - const data = JSON.parse(output) + const data = JSON.parse(result.stdout) return { owner: data.owner?.login || data.owner, repo: data.name, From f4530e65afde3d1011063e10e1b203eb9faf4997 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 5 Feb 2026 14:55:20 +0900 Subject: [PATCH 2/4] chore: remove dead code in getAllLinkedBranches Remove redundant git command and check that was ineffective since the code unconditionally returns empty array regardless of outcome. Co-authored-by: gemini-code-assist[bot] <162626009+gemini-code-assist[bot]@users.noreply.github.com> --- src/lib/git-workflow.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/lib/git-workflow.ts b/src/lib/git-workflow.ts index a318f0b..99477fe 100644 --- a/src/lib/git-workflow.ts +++ b/src/lib/git-workflow.ts @@ -18,16 +18,8 @@ export async function getAllLinkedBranches( repo?: string, ): Promise { if (!repo) { - // Try to get from current git repo - const result = await runGitCommand(['git', 'rev-parse', '--show-toplevel']) - - if (result.exitCode !== 0) { - // Cannot determine repo, return empty array - return [] - } - - // For now, if no repo specified and can't auto-detect owner/repo, return empty - // This is a limitation of the current implementation + // For now, if no repo specified, return empty array + // This is a limitation of the current implementation - auto-detection not yet implemented return [] } From ae2b718caf6e74a7a7b192c6dbab4fe4156d06fd Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Sun, 5 Jul 2026 19:22:27 +0900 Subject: [PATCH 3/4] fix(lib): make isInGitRepo/getGitDir resilient to spawn errors Restore the .please/**/*.md eslint ignore that exists on main but was missing on this branch, fixing the lint check. Wrap runGitCommand calls in isInGitRepo/getGitDir with try/catch so a subprocess spawn failure returns false/null instead of throwing, matching their documented boolean/nullable contract and fixing the Unit Tests check failure surfaced in CI at repo-manager.ts:55. --- eslint.config.js | 2 ++ src/lib/repo-manager.ts | 22 ++++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index b58d330..6ef1c5b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -11,6 +11,8 @@ export default antfu({ 'CLAUDE.md', 'docs-dev/**/*.md', 'docs/**/*.md', + // Ignore planning and memory documents with illustrative code blocks + '.please/**/*.md', ], }, { rules: { diff --git a/src/lib/repo-manager.ts b/src/lib/repo-manager.ts index d102f69..65fb9b3 100644 --- a/src/lib/repo-manager.ts +++ b/src/lib/repo-manager.ts @@ -52,8 +52,13 @@ export async function findBareRepo(owner: string, repo: string): Promise { - const result = await runGitCommand(['git', 'rev-parse', '--git-dir']) - return result.exitCode === 0 + try { + const result = await runGitCommand(['git', 'rev-parse', '--git-dir']) + return result.exitCode === 0 + } + catch { + return false + } } /** @@ -61,13 +66,18 @@ export async function isInGitRepo(): Promise { * Returns absolute path if in a git repo, null otherwise */ export async function getGitDir(): Promise { - const result = await runGitCommand(['git', 'rev-parse', '--absolute-git-dir']) + try { + const result = await runGitCommand(['git', 'rev-parse', '--absolute-git-dir']) - if (result.exitCode !== 0) { + if (result.exitCode !== 0) { + return null + } + + return result.stdout.trim() + } + catch { return null } - - return result.stdout.trim() } /** From acbec2ec254ef82d3ac89e501dcb38b472994df0 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Sun, 5 Jul 2026 19:46:56 +0900 Subject: [PATCH 4/4] refactor(lib): rename runGitCommand to runCliCommand; harden gh error handling runGitCommand is used to spawn both git and gh CLI commands, so the name was misleading. Renamed to runCliCommand across git-exec, git-workflow, git-workflow-worktree, and repo-manager. Also wrap cloneBareRepo/getCurrentRepoInfo's runCliCommand calls in try/catch, matching the pattern already used in isInGitRepo/getGitDir, so a spawn failure (e.g. gh missing from PATH) surfaces a clear error instead of an unhandled rejection. --- src/lib/git-exec.ts | 4 ++-- src/lib/git-workflow-worktree.ts | 20 ++++++++--------- src/lib/git-workflow.ts | 12 +++++------ src/lib/repo-manager.ts | 26 +++++++++++++++++------ test/lib/git-workflow-worktree.preload.ts | 2 +- test/lib/git-workflow-worktree.test.ts | 2 +- 6 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/lib/git-exec.ts b/src/lib/git-exec.ts index 018fa23..289d920 100644 --- a/src/lib/git-exec.ts +++ b/src/lib/git-exec.ts @@ -8,9 +8,9 @@ export interface GitResult { } /** - * Execute a git command and return the result + * Execute a CLI command (git or gh) and return the result */ -export async function runGitCommand(args: string[]): Promise { +export async function runCliCommand(args: string[]): Promise { const proc = Bun.spawn(args, { stdout: 'pipe', stderr: 'pipe', diff --git a/src/lib/git-workflow-worktree.ts b/src/lib/git-workflow-worktree.ts index f5496f4..b39333f 100644 --- a/src/lib/git-workflow-worktree.ts +++ b/src/lib/git-workflow-worktree.ts @@ -1,7 +1,7 @@ import type { WorktreeInfo } from '../types' import * as fs from 'node:fs' import * as path from 'node:path' -import { runGitCommand, warnWithFollowup } from './git-exec' +import { runCliCommand, warnWithFollowup } from './git-exec' /** * Ensure parent directory exists for a target path @@ -34,7 +34,7 @@ export async function createWorktree( ): Promise { const expandedPath = await ensureParentDirectory(targetPath) - const result = await runGitCommand(['git', '-C', bareRepoPath, 'worktree', 'add', expandedPath, branch]) + const result = await runCliCommand(['git', '-C', bareRepoPath, 'worktree', 'add', expandedPath, branch]) if (result.exitCode !== 0) { throw new Error(`Failed to create worktree: ${result.stderr.trim()}`) @@ -55,7 +55,7 @@ export async function createWorktreeFromRepo( // Step 1: Fetch the branch with explicit refspec to update remote-tracking ref // Using refspec ensures refs/remotes/origin/{branch} is updated - const fetchResult = await runGitCommand([ + const fetchResult = await runCliCommand([ 'git', '--git-dir', gitDir, @@ -75,7 +75,7 @@ export async function createWorktreeFromRepo( // Step 2: Check if remote-tracking ref exists before updating local branch // Using rev-parse is language-agnostic (avoids relying on localized error messages) - const remoteRefCheck = await runGitCommand([ + const remoteRefCheck = await runCliCommand([ 'git', '--git-dir', gitDir, @@ -89,7 +89,7 @@ export async function createWorktreeFromRepo( // Step 3: Update local branch to match remote (only if remote ref exists) // This ensures the worktree uses the latest code, not stale local branch if (remoteRefExists) { - const branchResult = await runGitCommand([ + const branchResult = await runCliCommand([ 'git', '--git-dir', gitDir, @@ -109,7 +109,7 @@ export async function createWorktreeFromRepo( } // Check if branch exists locally (for worktree add command selection) - const checkResult = await runGitCommand(['git', '--git-dir', gitDir, 'rev-parse', '--verify', `refs/heads/${branch}`]) + const checkResult = await runCliCommand(['git', '--git-dir', gitDir, 'rev-parse', '--verify', `refs/heads/${branch}`]) const branchExists = checkResult.exitCode === 0 // Create worktree with the branch @@ -117,13 +117,13 @@ export async function createWorktreeFromRepo( ? ['git', '--git-dir', gitDir, 'worktree', 'add', expandedPath, branch] : ['git', '--git-dir', gitDir, 'worktree', 'add', '-b', branch, expandedPath, `origin/${branch}`] - const worktreeResult = await runGitCommand(worktreeArgs) + const worktreeResult = await runCliCommand(worktreeArgs) if (worktreeResult.exitCode !== 0) { throw new Error(`Failed to create worktree at '${expandedPath}' for branch '${branch}': ${worktreeResult.stderr.trim()}`) } // Set upstream tracking for the branch in the worktree - const trackingResult = await runGitCommand(['git', '-C', expandedPath, 'branch', '--set-upstream-to', `origin/${branch}`, branch]) + const trackingResult = await runCliCommand(['git', '-C', expandedPath, 'branch', '--set-upstream-to', `origin/${branch}`, branch]) if (trackingResult.exitCode !== 0) { warnWithFollowup( `Could not set upstream tracking: ${trackingResult.stderr.trim() || 'unknown error'}`, @@ -136,7 +136,7 @@ export async function createWorktreeFromRepo( * List all worktrees for a repository */ export async function listWorktrees(bareRepoPath: string): Promise { - const result = await runGitCommand(['git', '-C', bareRepoPath, 'worktree', 'list', '--porcelain']) + const result = await runCliCommand(['git', '-C', bareRepoPath, 'worktree', 'list', '--porcelain']) if (result.exitCode !== 0) { // Log warning for debugging but return empty to allow caller to continue @@ -196,7 +196,7 @@ function parseWorktreeOutput(output: string): WorktreeInfo[] { * Remove worktree at path */ export async function removeWorktree(worktreePath: string): Promise { - const result = await runGitCommand(['git', 'worktree', 'remove', worktreePath]) + const result = await runCliCommand(['git', 'worktree', 'remove', worktreePath]) if (result.exitCode !== 0) { throw new Error(`Failed to remove worktree: ${result.stderr.trim()}`) diff --git a/src/lib/git-workflow.ts b/src/lib/git-workflow.ts index 99477fe..7f83761 100644 --- a/src/lib/git-workflow.ts +++ b/src/lib/git-workflow.ts @@ -1,6 +1,6 @@ import type { DevelopOptions } from '../types' import { getGhCommand } from './gh-command' -import { runGitCommand } from './git-exec' +import { runCliCommand } from './git-exec' // Re-export worktree functions from dedicated module export { @@ -60,7 +60,7 @@ export async function getAllLinkedBranches( `issueNumber=${issueNumber}`, ] - const result = await runGitCommand(args) + const result = await runCliCommand(args) if (result.exitCode !== 0) { // GraphQL query failed @@ -123,7 +123,7 @@ export async function startDevelopWorkflow( args.push('-n', options.name) } - const result = await runGitCommand(args) + const result = await runCliCommand(args) if (result.exitCode !== 0) { throw new Error(`Failed to develop issue: ${result.stderr.trim()}`) @@ -162,7 +162,7 @@ export async function fetchBranch( branch: string, ): Promise { // Step 1: Fetch to remote tracking branch (refs/remotes/origin/{branch}) - const fetchResult = await runGitCommand( + const fetchResult = await runCliCommand( ['git', '-C', bareRepoPath, 'fetch', 'origin', `${branch}:refs/remotes/origin/${branch}`], ) @@ -172,13 +172,13 @@ export async function fetchBranch( // Step 2: Create or update local branch from remote tracking branch // Try to create the branch first - const createResult = await runGitCommand( + const createResult = await runCliCommand( ['git', '-C', bareRepoPath, 'branch', branch, `refs/remotes/origin/${branch}`], ) if (createResult.exitCode !== 0) { // Branch might already exist, try to update it - const updateResult = await runGitCommand( + const updateResult = await runCliCommand( ['git', '-C', bareRepoPath, 'branch', '-f', branch, `refs/remotes/origin/${branch}`], ) diff --git a/src/lib/repo-manager.ts b/src/lib/repo-manager.ts index 65fb9b3..72603f6 100644 --- a/src/lib/repo-manager.ts +++ b/src/lib/repo-manager.ts @@ -3,7 +3,7 @@ import * as fs from 'node:fs' import * as os from 'node:os' import * as path from 'node:path' import { getGhCommand } from './gh-command' -import { runGitCommand } from './git-exec' +import { runCliCommand } from './git-exec' /** * Parse repository string in format "owner/repo" or GitHub URL @@ -53,7 +53,7 @@ export async function findBareRepo(owner: string, repo: string): Promise { try { - const result = await runGitCommand(['git', 'rev-parse', '--git-dir']) + const result = await runCliCommand(['git', 'rev-parse', '--git-dir']) return result.exitCode === 0 } catch { @@ -67,7 +67,7 @@ export async function isInGitRepo(): Promise { */ export async function getGitDir(): Promise { try { - const result = await runGitCommand(['git', 'rev-parse', '--absolute-git-dir']) + const result = await runCliCommand(['git', 'rev-parse', '--absolute-git-dir']) if (result.exitCode !== 0) { return null @@ -96,9 +96,15 @@ export async function cloneBareRepo(owner: string, repo: string): Promise { - const result = await runGitCommand([getGhCommand(), 'repo', 'view', '--json', 'owner,name']) + let result + try { + result = await runCliCommand([getGhCommand(), 'repo', 'view', '--json', 'owner,name']) + } + catch (error) { + throw new Error(`Failed to get repository info: ${error instanceof Error ? error.message : String(error)}`) + } if (result.exitCode !== 0) { throw new Error(`Failed to get repository info: ${result.stderr.trim() || 'Not in a git repository'}`) diff --git a/test/lib/git-workflow-worktree.preload.ts b/test/lib/git-workflow-worktree.preload.ts index 7d4dbc7..35839fe 100644 --- a/test/lib/git-workflow-worktree.preload.ts +++ b/test/lib/git-workflow-worktree.preload.ts @@ -15,7 +15,7 @@ export const mockMkdir = mock(() => Promise.resolve()) // Mock git-exec module BEFORE it's imported mock.module('../../src/lib/git-exec', () => ({ - runGitCommand: mockRunGitCommand, + runCliCommand: mockRunGitCommand, warnWithFollowup: mockWarnWithFollowup, })) diff --git a/test/lib/git-workflow-worktree.test.ts b/test/lib/git-workflow-worktree.test.ts index 7b95c73..a28617b 100644 --- a/test/lib/git-workflow-worktree.test.ts +++ b/test/lib/git-workflow-worktree.test.ts @@ -2,7 +2,7 @@ * Comprehensive tests for git-workflow-worktree.ts * * These tests verify git command arguments and error handling by mocking - * the runGitCommand function. Run with: + * the runCliCommand function. Run with: * bun test test/lib/git-workflow-worktree.test.ts --preload test/lib/git-workflow-worktree.preload.ts */ import { afterEach, describe, expect, test } from 'bun:test'