Skip to content

Extract interfaces for testability in CLI wrapper packages #244

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 9 commits into from
Jan 29, 2026
Merged

Extract interfaces for testability in CLI wrapper packages #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a3adaf4
Add Operations interface to internal/git
newhook Jan 29, 2026
6526eeb
Add Operations interface to internal/mise
newhook Jan 29, 2026
52f0879
Add ClientInterface to internal/github
newhook Jan 29, 2026
e3a10db
Add ClientInterface to internal/zellij
newhook Jan 29, 2026
324313b
Add CLI and Reader interfaces to internal/beads
newhook Jan 29, 2026
2a283cf
Refactor beads CLI interface to bind beadsDir at construction
newhook Jan 29, 2026
9aa030f
Refactor mise Operations interface to bind dir at construction
newhook Jan 29, 2026
cb4b6b6
Split zellij ClientInterface into SessionManager and Session
newhook Jan 29, 2026
1611331
Remove ClientInterface, update callers to use SessionManager/Session
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
103 changes: 103 additions & 0 deletions internal/beads/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package beads

import (
"context"
)

// CLI defines the interface for bd command operations.
// This abstraction enables testing without actual bd CLI calls.
// Each CLI instance is bound to a specific beads directory.
//
// Note: Init and InstallHooks are package-level functions, not part of this
// interface, since they are setup operations that run before a CLI is created.
type CLI interface {
// Create creates a new bead and returns its ID.
Create(ctx context.Context, opts CreateOptions) (string, error)
// Close closes a bead.
Close(ctx context.Context, beadID string) error
// Reopen reopens a closed bead.
Reopen(ctx context.Context, beadID string) error
// Update updates a bead's fields.
Update(ctx context.Context, beadID string, opts UpdateOptions) error
// AddComment adds a comment to a bead.
AddComment(ctx context.Context, beadID, comment string) error
// AddLabels adds labels to a bead.
AddLabels(ctx context.Context, beadID string, labels []string) error
// SetExternalRef sets the external reference for a bead.
SetExternalRef(ctx context.Context, beadID, externalRef string) error
// AddDependency adds a dependency between two beads.
AddDependency(ctx context.Context, beadID, dependsOnID string) error
}

// Reader defines the interface for reading beads from the database.
// This abstraction enables testing without actual database access.
type Reader interface {
// GetBead retrieves a single bead by ID with its dependencies/dependents.
GetBead(ctx context.Context, id string) (*BeadWithDeps, error)
// GetBeadsWithDeps retrieves beads and their dependencies/dependents.
GetBeadsWithDeps(ctx context.Context, beadIDs []string) (*BeadsWithDepsResult, error)
// ListBeads lists all beads with optional status filter.
ListBeads(ctx context.Context, status string) ([]Bead, error)
// GetReadyBeads returns all open beads where all dependencies are satisfied.
GetReadyBeads(ctx context.Context) ([]Bead, error)
// GetTransitiveDependencies collects all transitive dependencies for a bead.
GetTransitiveDependencies(ctx context.Context, id string) ([]Bead, error)
// GetBeadWithChildren retrieves a bead and all its child beads recursively.
GetBeadWithChildren(ctx context.Context, id string) ([]Bead, error)
}

// cliImpl implements CLI using the bd command-line tool.
type cliImpl struct {
beadsDir string
}

// Compile-time check that cliImpl implements CLI.
var _ CLI = (*cliImpl)(nil)

// NewCLI creates a new CLI instance bound to the specified beads directory.
func NewCLI(beadsDir string) CLI {
return &cliImpl{beadsDir: beadsDir}
}

// Create implements CLI.Create.
func (c *cliImpl) Create(ctx context.Context, opts CreateOptions) (string, error) {
return Create(ctx, c.beadsDir, opts)
}

// Close implements CLI.Close.
func (c *cliImpl) Close(ctx context.Context, beadID string) error {
return Close(ctx, beadID, c.beadsDir)
}

// Reopen implements CLI.Reopen.
func (c *cliImpl) Reopen(ctx context.Context, beadID string) error {
return Reopen(ctx, beadID, c.beadsDir)
}

// Update implements CLI.Update.
func (c *cliImpl) Update(ctx context.Context, beadID string, opts UpdateOptions) error {
return Update(ctx, beadID, c.beadsDir, opts)
}

// AddComment implements CLI.AddComment.
func (c *cliImpl) AddComment(ctx context.Context, beadID, comment string) error {
return AddComment(ctx, beadID, comment, c.beadsDir)
}

// AddLabels implements CLI.AddLabels.
func (c *cliImpl) AddLabels(ctx context.Context, beadID string, labels []string) error {
return AddLabels(ctx, beadID, c.beadsDir, labels)
}

// SetExternalRef implements CLI.SetExternalRef.
func (c *cliImpl) SetExternalRef(ctx context.Context, beadID, externalRef string) error {
return SetExternalRef(ctx, beadID, externalRef, c.beadsDir)
}

// AddDependency implements CLI.AddDependency.
func (c *cliImpl) AddDependency(ctx context.Context, beadID, dependsOnID string) error {
return AddDependency(ctx, beadID, dependsOnID, c.beadsDir)
}

// Compile-time check that Client implements Reader.
var _ Reader = (*Client)(nil)
2 changes: 1 addition & 1 deletion internal/feedback/integration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type BeadInfo struct {

// Integration handles the integration between GitHub PR feedback and beads.
type Integration struct {
client *github.Client
client github.ClientInterface
processor *FeedbackProcessor
}

Expand Down
6 changes: 3 additions & 3 deletions internal/feedback/processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,22 @@ const maxLogContentSize = 50 * 1024 // 50KB

// FeedbackProcessor processes PR feedback and generates actionable items.
type FeedbackProcessor struct {
client *github.Client
client github.ClientInterface
// Optional fields for Claude log analysis integration
proj *project.Project
workID string
}

// NewFeedbackProcessor creates a new feedback processor.
func NewFeedbackProcessor(client *github.Client) *FeedbackProcessor {
func NewFeedbackProcessor(client github.ClientInterface) *FeedbackProcessor {
return &FeedbackProcessor{
client: client,
}
}

// NewFeedbackProcessorWithProject creates a feedback processor with project context.
// This enables Claude-based log analysis when configured.
func NewFeedbackProcessorWithProject(client *github.Client, proj *project.Project, workID string) *FeedbackProcessor {
func NewFeedbackProcessorWithProject(client github.ClientInterface, proj *project.Project, workID string) *FeedbackProcessor {
return &FeedbackProcessor{
client: client,
proj: proj,
Expand Down
100 changes: 84 additions & 16 deletions internal/git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,36 @@ import (
"strings"
)

// PushSetUpstreamInDir pushes the specified branch and sets upstream tracking.
func PushSetUpstreamInDir(ctx context.Context, branch, dir string) error {
// Operations defines the interface for git operations.
// This abstraction enables testing without actual git commands.
type Operations interface {
// PushSetUpstream pushes the specified branch and sets upstream tracking.
PushSetUpstream(ctx context.Context, branch, dir string) error
// Pull pulls the latest changes in a specific directory.
Pull(ctx context.Context, dir string) error
// Clone clones a repository from source to dest.
Clone(ctx context.Context, source, dest string) error
// FetchBranch fetches a specific branch from origin.
FetchBranch(ctx context.Context, repoPath, branch string) error
// BranchExists checks if a branch exists locally or remotely.
BranchExists(ctx context.Context, repoPath, branchName string) bool
// ValidateExistingBranch checks if a branch exists locally, remotely, or both.
ValidateExistingBranch(ctx context.Context, repoPath, branchName string) (existsLocal, existsRemote bool, err error)
// ListBranches returns a deduplicated list of all branches (local and remote).
ListBranches(ctx context.Context, repoPath string) ([]string, error)
}

// cliOperations implements Operations using the git CLI.
type cliOperations struct{}

// Compile-time check that cliOperations implements Operations.
var _ Operations = (*cliOperations)(nil)

// Default is the default Operations implementation using the git CLI.
var Default Operations = &cliOperations{}

// PushSetUpstream implements Operations.PushSetUpstream.
func (c *cliOperations) PushSetUpstream(ctx context.Context, branch, dir string) error {
cmd := exec.CommandContext(ctx, "git", "push", "--set-upstream", "origin", branch)
if dir != "" {
cmd.Dir = dir
Expand All @@ -19,8 +47,14 @@ func PushSetUpstreamInDir(ctx context.Context, branch, dir string) error {
return nil
}

// PullInDir pulls the latest changes in a specific directory.
func PullInDir(ctx context.Context, dir string) error {
// PushSetUpstreamInDir pushes the specified branch and sets upstream tracking.
// Deprecated: Use Default.PushSetUpstream instead.
func PushSetUpstreamInDir(ctx context.Context, branch, dir string) error {
return Default.PushSetUpstream(ctx, branch, dir)
}

// Pull implements Operations.Pull.
func (c *cliOperations) Pull(ctx context.Context, dir string) error {
cmd := exec.CommandContext(ctx, "git", "pull")
if dir != "" {
cmd.Dir = dir
Expand All @@ -31,17 +65,29 @@ func PullInDir(ctx context.Context, dir string) error {
return nil
}

// Clone clones a repository from source to dest.
func Clone(ctx context.Context, source, dest string) error {
// PullInDir pulls the latest changes in a specific directory.
// Deprecated: Use Default.Pull instead.
func PullInDir(ctx context.Context, dir string) error {
return Default.Pull(ctx, dir)
}

// Clone implements Operations.Clone.
func (c *cliOperations) Clone(ctx context.Context, source, dest string) error {
cmd := exec.CommandContext(ctx, "git", "clone", source, dest)
if output, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("failed to clone repository: %w\n%s", err, output)
}
return nil
}

// FetchBranch fetches a specific branch from origin.
func FetchBranch(ctx context.Context, repoPath, branch string) error {
// Clone clones a repository from source to dest.
// Deprecated: Use Default.Clone instead.
func Clone(ctx context.Context, source, dest string) error {
return Default.Clone(ctx, source, dest)
}

// FetchBranch implements Operations.FetchBranch.
func (c *cliOperations) FetchBranch(ctx context.Context, repoPath, branch string) error {
cmd := exec.CommandContext(ctx, "git", "fetch", "origin", branch)
cmd.Dir = repoPath
if output, err := cmd.CombinedOutput(); err != nil {
Expand All @@ -50,8 +96,14 @@ func FetchBranch(ctx context.Context, repoPath, branch string) error {
return nil
}

// BranchExists checks if a branch exists locally or remotely.
func BranchExists(ctx context.Context, repoPath, branchName string) bool {
// FetchBranch fetches a specific branch from origin.
// Deprecated: Use Default.FetchBranch instead.
func FetchBranch(ctx context.Context, repoPath, branch string) error {
return Default.FetchBranch(ctx, repoPath, branch)
}

// BranchExists implements Operations.BranchExists.
func (c *cliOperations) BranchExists(ctx context.Context, repoPath, branchName string) bool {
// Check local branches
cmd := exec.CommandContext(ctx, "git", "show-ref", "--verify", "--quiet", "refs/heads/"+branchName)
cmd.Dir = repoPath
Expand All @@ -65,9 +117,14 @@ func BranchExists(ctx context.Context, repoPath, branchName string) bool {
return cmd.Run() == nil
}

// ValidateExistingBranch checks if a branch exists locally, remotely, or both.
// Returns (existsLocal, existsRemote, error).
func ValidateExistingBranch(ctx context.Context, repoPath, branchName string) (bool, bool, error) {
// BranchExists checks if a branch exists locally or remotely.
// Deprecated: Use Default.BranchExists instead.
func BranchExists(ctx context.Context, repoPath, branchName string) bool {
return Default.BranchExists(ctx, repoPath, branchName)
}

// ValidateExistingBranch implements Operations.ValidateExistingBranch.
func (c *cliOperations) ValidateExistingBranch(ctx context.Context, repoPath, branchName string) (bool, bool, error) {
// Check local branches
cmd := exec.CommandContext(ctx, "git", "show-ref", "--verify", "--quiet", "refs/heads/"+branchName)
cmd.Dir = repoPath
Expand All @@ -81,9 +138,14 @@ func ValidateExistingBranch(ctx context.Context, repoPath, branchName string) (b
return existsLocal, existsRemote, nil
}

// ListBranches returns a deduplicated list of all branches (local and remote).
// Excludes HEAD and the current branch. Remote branches have their origin/ prefix stripped.
func ListBranches(ctx context.Context, repoPath string) ([]string, error) {
// ValidateExistingBranch checks if a branch exists locally, remotely, or both.
// Deprecated: Use Default.ValidateExistingBranch instead.
func ValidateExistingBranch(ctx context.Context, repoPath, branchName string) (bool, bool, error) {
return Default.ValidateExistingBranch(ctx, repoPath, branchName)
}

// ListBranches implements Operations.ListBranches.
func (c *cliOperations) ListBranches(ctx context.Context, repoPath string) ([]string, error) {
// Get local branches
cmd := exec.CommandContext(ctx, "git", "branch", "--format=%(refname:short)")
cmd.Dir = repoPath
Expand Down Expand Up @@ -145,3 +207,9 @@ func ListBranches(ctx context.Context, repoPath string) ([]string, error) {

return branches, nil
}

// ListBranches returns a deduplicated list of all branches (local and remote).
// Deprecated: Use Default.ListBranches instead.
func ListBranches(ctx context.Context, repoPath string) ([]string, error) {
return Default.ListBranches(ctx, repoPath)
}
20 changes: 20 additions & 0 deletions internal/github/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,29 @@ import (
"github.com/newhook/co/internal/logging"
)

// ClientInterface defines the interface for GitHub API operations.
// This abstraction enables testing without actual GitHub API calls.
type ClientInterface interface {
// GetPRStatus fetches comprehensive PR status information.
GetPRStatus(ctx context.Context, prURL string) (*PRStatus, error)
// PostPRComment posts a comment on a PR issue.
PostPRComment(ctx context.Context, prURL string, body string) error
// PostReplyToComment posts a reply to a specific comment on a PR.
PostReplyToComment(ctx context.Context, prURL string, commentID int, body string) error
// PostReviewReply posts a reply to a review comment.
PostReviewReply(ctx context.Context, prURL string, reviewCommentID int, body string) error
// ResolveReviewThread resolves a review thread containing the specified comment.
ResolveReviewThread(ctx context.Context, prURL string, commentID int) error
// GetJobLogs fetches the logs for a specific job.
GetJobLogs(ctx context.Context, repo string, jobID int64) (string, error)
}

// Client wraps the gh CLI for GitHub API operations.
type Client struct{}

// Compile-time check that Client implements ClientInterface.
var _ ClientInterface = (*Client)(nil)

// NewClient creates a new GitHub client.
func NewClient() *Client {
return &Client{}
Expand Down
Loading