Skip to content

Extract interfaces for testability in CLI wrapper packages #245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
newhook merged 5 commits into from
Jan 29, 2026
Merged

Extract interfaces for testability in CLI wrapper packages #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c53bcb9
Extract interfaces for testability in CLI wrapper packages
newhook Jan 29, 2026
ab23861
Remove package-level Run function from claude package
newhook Jan 29, 2026
7799055
Add Runner parameter for dependency injection in task execution
newhook Jan 29, 2026
fe58a01
Remove package-level convenience functions from git and worktree pack…
newhook Jan 29, 2026
aec92fc
Fix ac-ynyu.1: Handle unchecked error returns from RemoveForce calls
newhook Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions cmd/orchestrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func runOrchestrate(cmd *cobra.Command, args []string) error {
// git push retries, PR feedback polling, etc. This allows scheduled tasks
// to be processed even when no orchestrator is running for a theWork.

// Create runner once for all tasks
runner := claude.NewRunner()

// Main orchestration loop: poll for ready tasks and execute them
for {

Expand Down Expand Up @@ -239,14 +242,14 @@ func runOrchestrate(cmd *cobra.Command, args []string) error {
fmt.Printf("Warning: failed to update task activity at start: %v\n", err)
}

if err := executeTask(proj, task, theWork); err != nil {
if err := executeTask(proj, task, theWork, runner); err != nil {
return fmt.Errorf("task %s failed: %w", task.ID, err)
}
}
}

// executeTask executes a single task inline based on its type.
func executeTask(proj *project.Project, t *db.Task, work *db.Work) error {
func executeTask(proj *project.Project, t *db.Task, work *db.Work, runner claude.Runner) error {
ctx := GetContext()

// Create a context with timeout from configuration
Expand All @@ -263,7 +266,7 @@ func executeTask(proj *project.Project, t *db.Task, work *db.Work) error {
}

// Execute Claude inline with timeout context
if err = claude.Run(taskCtx, proj.DB, t.ID, prompt, work.WorktreePath, proj.Config); err != nil {
if err = runner.Run(taskCtx, proj.DB, t.ID, prompt, work.WorktreePath, proj.Config); err != nil {
// Check if it was a timeout error
if errors.Is(err, context.DeadlineExceeded) {
// Mark the task as failed due to timeout
Expand Down
8 changes: 5 additions & 3 deletions cmd/proj.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ func runProjDestroy(cmd *cobra.Command, args []string) error {
defer proj.Close()

// List worktrees
worktrees, err := worktree.List(ctx, proj.MainRepoPath())
wtOps := worktree.NewOperations()
worktrees, err := wtOps.List(ctx, proj.MainRepoPath())
if err != nil {
return fmt.Errorf("failed to list worktrees: %w", err)
}
Expand Down Expand Up @@ -131,7 +132,7 @@ func runProjDestroy(cmd *cobra.Command, args []string) error {
// Remove all worktrees
for _, wt := range taskWorktrees {
fmt.Printf("Removing worktree %s...\n", wt.Path)
if err := worktree.RemoveForce(ctx, proj.MainRepoPath(), wt.Path); err != nil {
if err := wtOps.RemoveForce(ctx, proj.MainRepoPath(), wt.Path); err != nil {
fmt.Printf("Warning: failed to remove worktree %s: %v\n", wt.Path, err)
}
}
Expand Down Expand Up @@ -165,7 +166,8 @@ func runProjStatus(cmd *cobra.Command, args []string) error {
fmt.Printf(" Path: %s\n", proj.MainRepoPath())

// List worktrees
worktrees, err := worktree.List(ctx, proj.MainRepoPath())
wtOps := worktree.NewOperations()
worktrees, err := wtOps.List(ctx, proj.MainRepoPath())
if err != nil {
return fmt.Errorf("failed to list worktrees: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func runTasks(cmd *cobra.Command, args []string) error {
return fmt.Errorf("work %s has no worktree path configured", workRecord.ID)
}

if !worktree.ExistsPath(workRecord.WorktreePath) {
if !worktree.NewOperations().ExistsPath(workRecord.WorktreePath) {
return fmt.Errorf("work %s worktree does not exist at %s", workRecord.ID, workRecord.WorktreePath)
}

Expand Down
6 changes: 4 additions & 2 deletions cmd/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ func runSync(cmd *cobra.Command, args []string) error {
fmt.Printf("Syncing project: %s\n", proj.Config.Project.Name)

// Get all worktrees
worktrees, err := worktree.List(ctx, proj.MainRepoPath())
wtOps := worktree.NewOperations()
gitOps := git.NewOperations()
worktrees, err := wtOps.List(ctx, proj.MainRepoPath())
if err != nil {
return fmt.Errorf("failed to list worktrees: %w", err)
}
Expand All @@ -58,7 +60,7 @@ func runSync(cmd *cobra.Command, args []string) error {

fmt.Printf(" Pulling %s [%s]... ", wt.Path, branchInfo)

if err := git.PullInDir(ctx, wt.Path); err != nil {
if err := gitOps.Pull(ctx, wt.Path); err != nil {
fmt.Printf("FAILED: %v\n", err)
failCount++
} else {
Expand Down
6 changes: 3 additions & 3 deletions cmd/task_processing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func getBeadsForTask(ctx context.Context, proj *project.Project, taskID string)

// processTask processes a single task by ID using inline execution.
// This blocks until the task is complete.
func processTask(proj *project.Project, taskID string) error {
func processTask(proj *project.Project, taskID string, runner claude.Runner) error {
ctx := GetContext()

// Get the task
Expand Down Expand Up @@ -192,7 +192,7 @@ func processTask(proj *project.Project, taskID string) error {
return fmt.Errorf("work %s has no worktree path configured", work.ID)
}

if !worktree.ExistsPath(work.WorktreePath) {
if !worktree.NewOperations().ExistsPath(work.WorktreePath) {
return fmt.Errorf("work %s worktree does not exist at %s", work.ID, work.WorktreePath)
}

Expand All @@ -203,7 +203,7 @@ func processTask(proj *project.Project, taskID string) error {
}

// Execute Claude inline (blocking)
if err := claude.Run(ctx, proj.DB, taskID, prompt, work.WorktreePath, proj.Config); err != nil {
if err := runner.Run(ctx, proj.DB, taskID, prompt, work.WorktreePath, proj.Config); err != nil {
return fmt.Errorf("task %s failed: %w", taskID, err)
}

Expand Down
32 changes: 18 additions & 14 deletions cmd/work.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ func runWorkCreate(cmd *cobra.Command, args []string) error {
baseBranch := proj.Config.Repo.GetBaseBranch()

mainRepoPath := proj.MainRepoPath()
gitOps := git.NewOperations()
wtOps := worktree.NewOperations()
beadID := args[0]

// Expand the bead (handles epics and transitive deps)
Expand Down Expand Up @@ -251,7 +253,7 @@ func runWorkCreate(cmd *cobra.Command, args []string) error {
useExistingBranch = true

// Validate the branch exists
existsLocal, existsRemote, err := git.ValidateExistingBranch(ctx, mainRepoPath, branchName)
existsLocal, existsRemote, err := gitOps.ValidateExistingBranch(ctx, mainRepoPath, branchName)
if err != nil {
return fmt.Errorf("failed to validate branch: %w", err)
}
Expand Down Expand Up @@ -301,41 +303,41 @@ func runWorkCreate(cmd *cobra.Command, args []string) error {

if useExistingBranch {
// For existing branches, fetch the branch first
if err := git.FetchBranch(ctx, mainRepoPath, branchName); err != nil {
if err := gitOps.FetchBranch(ctx, mainRepoPath, branchName); err != nil {
// Ignore fetch errors - branch might only exist locally
fmt.Printf("Note: Could not fetch branch %s from origin (may only exist locally)\n", branchName)
}

// Create worktree from existing branch
if err := worktree.CreateFromExisting(ctx, mainRepoPath, worktreePath, branchName); err != nil {
if err := wtOps.CreateFromExisting(ctx, mainRepoPath, worktreePath, branchName); err != nil {
os.RemoveAll(workDir)
return err
}

// Only push if branch doesn't exist on remote yet
if !branchExistsOnRemote {
if err := git.PushSetUpstreamInDir(ctx, branchName, worktreePath); err != nil {
worktree.RemoveForce(ctx, mainRepoPath, worktreePath)
if err := gitOps.PushSetUpstream(ctx, branchName, worktreePath); err != nil {
_ = wtOps.RemoveForce(ctx, mainRepoPath, worktreePath)
os.RemoveAll(workDir)
return err
}
}
} else {
// Fetch latest from origin for the base branch
if err := git.FetchBranch(ctx, mainRepoPath, baseBranch); err != nil {
if err := gitOps.FetchBranch(ctx, mainRepoPath, baseBranch); err != nil {
os.RemoveAll(workDir)
return fmt.Errorf("failed to fetch base branch: %w", err)
}

// Create worktree with new branch based on origin/<baseBranch>
if err := worktree.Create(ctx, mainRepoPath, worktreePath, branchName, "origin/"+baseBranch); err != nil {
if err := wtOps.Create(ctx, mainRepoPath, worktreePath, branchName, "origin/"+baseBranch); err != nil {
os.RemoveAll(workDir)
return err
}

// Push branch and set upstream
if err := git.PushSetUpstreamInDir(ctx, branchName, worktreePath); err != nil {
worktree.RemoveForce(ctx, mainRepoPath, worktreePath)
if err := gitOps.PushSetUpstream(ctx, branchName, worktreePath); err != nil {
_ = wtOps.RemoveForce(ctx, mainRepoPath, worktreePath)
os.RemoveAll(workDir)
return err
}
Expand All @@ -354,14 +356,14 @@ func runWorkCreate(cmd *cobra.Command, args []string) error {

// Create work record in database with the root issue ID (the original bead that was expanded)
if err := proj.DB.CreateWork(ctx, workID, workerName, worktreePath, branchName, baseBranch, beadID, flagAutoRun); err != nil {
worktree.RemoveForce(ctx, mainRepoPath, worktreePath)
_ = wtOps.RemoveForce(ctx, mainRepoPath, worktreePath)
os.RemoveAll(workDir)
return fmt.Errorf("failed to create work record: %w", err)
}

// Add beads to work_beads
if err := work.AddBeadsToWorkInternal(ctx, proj, workID, expandedIssueIDs); err != nil {
worktree.RemoveForce(ctx, mainRepoPath, worktreePath)
_ = wtOps.RemoveForce(ctx, mainRepoPath, worktreePath)
os.RemoveAll(workDir)
return fmt.Errorf("failed to add beads to work: %w", err)
}
Expand Down Expand Up @@ -892,7 +894,8 @@ func runWorkPR(cmd *cobra.Command, args []string) error {

// Auto-run the PR task
fmt.Printf("Running PR task...\n")
if err := processTask(proj, result.TaskID); err != nil {
runner := claude.NewRunner()
if err := processTask(proj, result.TaskID, runner); err != nil {
return err
}

Expand Down Expand Up @@ -973,6 +976,7 @@ func runWorkReview(cmd *cobra.Command, args []string) error {
}

// Run review-fix loop if --auto is set
runner := claude.NewRunner()
maxIterations := proj.Config.Workflow.GetMaxReviewIterations()
for iteration := 0; ; iteration++ {
// Check max iterations
Expand All @@ -999,7 +1003,7 @@ func runWorkReview(cmd *cobra.Command, args []string) error {

// Run the review task
fmt.Printf("Running review task...\n")
if err := processTask(proj, reviewTaskID); err != nil {
if err := processTask(proj, reviewTaskID, runner); err != nil {
return fmt.Errorf("review task failed: %w", err)
}

Expand Down Expand Up @@ -1057,7 +1061,7 @@ func runWorkReview(cmd *cobra.Command, args []string) error {
fmt.Printf("Created fix task %s for bead %s: %s\n", taskID, b.ID, b.Title)

// Run the fix task
if err := processTask(proj, taskID); err != nil {
if err := processTask(proj, taskID, runner); err != nil {
return fmt.Errorf("fix task %s failed: %w", taskID, err)
}
}
Expand Down
24 changes: 20 additions & 4 deletions internal/claude/inline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,26 @@ import (
trackingwatcher "github.com/newhook/co/internal/tracking/watcher"
)

// Run executes Claude directly in the current terminal (fork/exec).
// This blocks until Claude exits or the task is marked complete in the database.
// The config parameter controls Claude settings like --dangerously-skip-permissions.
func Run(ctx context.Context, database *db.DB, taskID string, prompt string, workDir string, cfg *project.Config) error {
// Runner defines the interface for running Claude.
// This abstraction enables testing without spawning the actual claude CLI.
type Runner interface {
// Run executes Claude directly in the current terminal (fork/exec).
Run(ctx context.Context, database *db.DB, taskID string, prompt string, workDir string, cfg *project.Config) error
}

// CLIRunner implements Runner using the claude CLI.
type CLIRunner struct{}

// Compile-time check that CLIRunner implements Runner.
var _ Runner = (*CLIRunner)(nil)

// NewRunner creates a new Runner that uses the claude CLI.
func NewRunner() Runner {
return &CLIRunner{}
}

// Run implements Runner.Run.
func (r *CLIRunner) Run(ctx context.Context, database *db.DB, taskID string, prompt string, workDir string, cfg *project.Config) error {
// Get task to verify it exists
task, err := database.GetTask(ctx, taskID)
if err != nil {
Expand Down
14 changes: 8 additions & 6 deletions internal/control/handler_create_worktree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ func HandleCreateWorktreeTask(ctx context.Context, proj *project.Project, task *
}

mainRepoPath := proj.MainRepoPath()
gitOps := git.NewOperations()
wtOps := worktree.NewOperations()
var branchExistsOnRemote bool

// For existing branches, check if branch exists on remote
if useExisting {
_, branchExistsOnRemote, _ = git.ValidateExistingBranch(ctx, mainRepoPath, branchName)
_, branchExistsOnRemote, _ = gitOps.ValidateExistingBranch(ctx, mainRepoPath, branchName)
}

// If worktree path is already set and exists, skip creation
Expand All @@ -70,25 +72,25 @@ func HandleCreateWorktreeTask(ctx context.Context, proj *project.Project, task *

if useExisting {
// For existing branches, fetch the branch first
if err := git.FetchBranch(ctx, mainRepoPath, branchName); err != nil {
if err := gitOps.FetchBranch(ctx, mainRepoPath, branchName); err != nil {
// Ignore fetch errors - branch might only exist locally
logging.Debug("Could not fetch branch from origin (may only exist locally)", "branch", branchName)
}

// Create worktree from existing branch
if err := worktree.CreateFromExisting(ctx, mainRepoPath, worktreePath, branchName); err != nil {
if err := wtOps.CreateFromExisting(ctx, mainRepoPath, worktreePath, branchName); err != nil {
_ = os.RemoveAll(workDir)
return fmt.Errorf("failed to create worktree from existing branch: %w", err)
}
} else {
// Fetch latest from origin for the base branch
if err := git.FetchBranch(ctx, mainRepoPath, baseBranch); err != nil {
if err := gitOps.FetchBranch(ctx, mainRepoPath, baseBranch); err != nil {
_ = os.RemoveAll(workDir)
return fmt.Errorf("failed to fetch base branch: %w", err)
}

// Create git worktree with new branch based on origin/<baseBranch>
if err := worktree.Create(ctx, mainRepoPath, worktreePath, branchName, "origin/"+baseBranch); err != nil {
if err := wtOps.Create(ctx, mainRepoPath, worktreePath, branchName, "origin/"+baseBranch); err != nil {
_ = os.RemoveAll(workDir)
return fmt.Errorf("failed to create worktree: %w", err)
}
Expand All @@ -112,7 +114,7 @@ func HandleCreateWorktreeTask(ctx context.Context, proj *project.Project, task *
if useExisting && branchExistsOnRemote {
logging.Info("Skipping git push - branch already exists on remote", "branch", branchName)
} else {
if err := git.PushSetUpstreamInDir(ctx, branchName, work.WorktreePath); err != nil {
if err := gitOps.PushSetUpstream(ctx, branchName, work.WorktreePath); err != nil {
return fmt.Errorf("git push failed: %w", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/control/handler_destroy_worktree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func HandleDestroyWorktreeTask(ctx context.Context, proj *project.Project, task
// but the directory might still exist. The os.RemoveAll below will clean up the directory.
if work.WorktreePath != "" {
logging.Info("Removing git worktree", "work_id", workID, "path", work.WorktreePath)
if err := worktree.RemoveForce(ctx, proj.MainRepoPath(), work.WorktreePath); err != nil {
if err := worktree.NewOperations().RemoveForce(ctx, proj.MainRepoPath(), work.WorktreePath); err != nil {
logging.Warn("failed to remove git worktree (continuing with directory removal)", "error", err, "work_id", workID)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/control/handler_git_push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func HandleGitPushTask(ctx context.Context, proj *project.Project, task *db.Sche

logging.Info("Executing git push", "branch", branch, "dir", dir, "attempt", task.AttemptCount+1)

if err := git.PushSetUpstreamInDir(ctx, branch, dir); err != nil {
if err := git.NewOperations().PushSetUpstream(ctx, branch, dir); err != nil {
return err
}

Expand Down
Loading