From fd689760ee6b29865e5acaaf06bc0a58de9b1a9a Mon Sep 17 00:00:00 2001 From: Mark Esler Date: Mon, 16 Mar 2026 09:53:47 -0700 Subject: [PATCH 1/2] github: add 30s timeout to all API calls Prevent indefinite hangs when GitHub is slow or unreachable by setting a 30s timeout on the OAuth2 HTTP client and all exec.CommandContext calls. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/github/client.go | 15 ++++-- internal/github/queries.go | 29 +++++++++++ internal/github/queries_test.go | 91 +++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 internal/github/queries_test.go diff --git a/internal/github/client.go b/internal/github/client.go index 18f9840..7e0096b 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -5,11 +5,14 @@ import ( "fmt" "os/exec" "strings" + "time" gh "github.com/google/go-github/v75/github" "golang.org/x/oauth2" ) +const apiTimeout = 30 * time.Second + // Client wraps go-github with auth from `gh auth token`. type Client struct { gh *gh.Client @@ -17,23 +20,29 @@ type Client struct { // NewClient creates a GitHub client using the token from `gh auth token`. func NewClient(ctx context.Context) (*Client, error) { - token, err := ghAuthToken() + token, err := ghAuthToken(ctx) if err != nil { return nil, fmt.Errorf("getting GitHub token: %w", err) } ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}) tc := oauth2.NewClient(ctx, ts) + tc.Timeout = apiTimeout client := gh.NewClient(tc) return &Client{gh: client}, nil } // ghAuthToken runs `gh auth token` and returns the token string. -func ghAuthToken() (string, error) { - cmd := exec.Command("gh", "auth", "token") +func ghAuthToken(ctx context.Context) (string, error) { + ctx, cancel := withTimeout(ctx) + defer cancel() + cmd := exec.CommandContext(ctx, "gh", "auth", "token") out, err := cmd.Output() if err != nil { + if ctx.Err() == context.DeadlineExceeded { + return "", fmt.Errorf("gh auth token timed out after %s", apiTimeout) + } return "", fmt.Errorf("gh auth token failed: %s (is gh CLI installed and authenticated?)", ghError(err)) } return strings.TrimSpace(string(out)), nil diff --git a/internal/github/queries.go b/internal/github/queries.go index e947236..09bd6fe 100644 --- a/internal/github/queries.go +++ b/internal/github/queries.go @@ -8,6 +8,15 @@ import ( "strings" ) +// withTimeout returns a context with apiTimeout applied, unless the caller +// already set a deadline. +func withTimeout(ctx context.Context) (context.Context, context.CancelFunc) { + if _, ok := ctx.Deadline(); ok { + return ctx, func() {} + } + return context.WithTimeout(ctx, apiTimeout) +} + // ghError extracts stderr from an exec.ExitError for better error messages. func ghError(err error) string { if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { @@ -50,9 +59,14 @@ type ApprovedPR struct { // GetCurrentUser returns the authenticated GitHub user's login. func GetCurrentUser(ctx context.Context) (string, error) { + ctx, cancel := withTimeout(ctx) + defer cancel() cmd := exec.CommandContext(ctx, "gh", "api", "user", "--jq", ".login") out, err := cmd.Output() if err != nil { + if ctx.Err() == context.DeadlineExceeded { + return "", fmt.Errorf("fetching current user timed out after %s", apiTimeout) + } return "", fmt.Errorf("fetching current user: %s", ghError(err)) } return strings.TrimSpace(string(out)), nil @@ -61,6 +75,8 @@ func GetCurrentUser(ctx context.Context) (string, error) { // GetReviewRequests fetches PRs where the user is a requested reviewer, // including re-reviews. Uses GraphQL via `gh api graphql`. func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, error) { + ctx, cancel := withTimeout(ctx) + defer cancel() query := `query($q1: String!, $q2: String!) { requested: search(query: $q1, type: ISSUE, first: 50) { nodes { @@ -103,6 +119,9 @@ func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, ) out, err := cmd.Output() if err != nil { + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("review requests query timed out after %s", apiTimeout) + } return nil, fmt.Errorf("GraphQL query failed: %s", ghError(err)) } @@ -139,6 +158,8 @@ func GetReviewRequests(ctx context.Context, repoFilter string) ([]ReviewRequest, // GetApprovedUnmerged fetches the user's own PRs that are approved but not yet merged. func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, error) { + ctx, cancel := withTimeout(ctx) + defer cancel() query := `query($q: String!) { search(query: $q, type: ISSUE, first: 50) { nodes { @@ -168,6 +189,9 @@ func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, ) out, err := cmd.Output() if err != nil { + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("approved PRs query timed out after %s", apiTimeout) + } return nil, fmt.Errorf("GraphQL query failed: %s", ghError(err)) } @@ -193,6 +217,8 @@ func GetApprovedUnmerged(ctx context.Context, repoFilter string) ([]ApprovedPR, // ListOpenPRs lists open PRs for a repository using `gh pr list`. func ListOpenPRs(ctx context.Context, fullRepo string, limit int) ([]ReviewRequest, error) { + ctx, cancel := withTimeout(ctx) + defer cancel() cmd := exec.CommandContext(ctx, "gh", "pr", "list", "-R", fullRepo, "--state", "open", @@ -201,6 +227,9 @@ func ListOpenPRs(ctx context.Context, fullRepo string, limit int) ([]ReviewReque ) out, err := cmd.Output() if err != nil { + if ctx.Err() == context.DeadlineExceeded { + return nil, fmt.Errorf("listing open PRs timed out after %s", apiTimeout) + } return nil, err } diff --git a/internal/github/queries_test.go b/internal/github/queries_test.go new file mode 100644 index 0000000..913f698 --- /dev/null +++ b/internal/github/queries_test.go @@ -0,0 +1,91 @@ +package github + +import ( + "context" + "strings" + "testing" + "time" +) + +func TestWithTimeout_addsDeadlineWhenNone(t *testing.T) { + ctx, cancel := withTimeout(context.Background()) + defer cancel() + + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("expected deadline to be set") + } + remaining := time.Until(deadline) + if remaining <= 0 || remaining > apiTimeout { + t.Fatalf("expected deadline within %s, got %s remaining", apiTimeout, remaining) + } +} + +func TestWithTimeout_preservesExistingDeadline(t *testing.T) { + existing := time.Now().Add(5 * time.Second) + parent, parentCancel := context.WithDeadline(context.Background(), existing) + defer parentCancel() + + ctx, cancel := withTimeout(parent) + defer cancel() + + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("expected deadline to be set") + } + if !deadline.Equal(existing) { + t.Fatalf("expected existing deadline %v, got %v", existing, deadline) + } +} + +func TestGetCurrentUser_timeoutError(t *testing.T) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + _, err := GetCurrentUser(ctx) + if err == nil { + t.Fatal("expected error from expired context") + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected timeout error message, got: %s", err) + } +} + +func TestGetReviewRequests_timeoutError(t *testing.T) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + _, err := GetReviewRequests(ctx, "") + if err == nil { + t.Fatal("expected error from expired context") + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected timeout error message, got: %s", err) + } +} + +func TestGetApprovedUnmerged_timeoutError(t *testing.T) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + _, err := GetApprovedUnmerged(ctx, "") + if err == nil { + t.Fatal("expected error from expired context") + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected timeout error message, got: %s", err) + } +} + +func TestListOpenPRs_timeoutError(t *testing.T) { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + _, err := ListOpenPRs(ctx, "owner/repo", 10) + if err == nil { + t.Fatal("expected error from expired context") + } + if !strings.Contains(err.Error(), "timed out") { + t.Fatalf("expected timeout error message, got: %s", err) + } +} From 8f16aaa4b93c25ca1c0466068b9688cd1cceb35d Mon Sep 17 00:00:00 2001 From: Mark Esler Date: Thu, 19 Mar 2026 15:44:59 -0700 Subject: [PATCH 2/2] Make feature branch prefix configurable Replace hardcoded "mgreau/" branch prefix with branch_prefix config field. Falls back to git config user.name, then no prefix. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/work.go | 8 +++++++- internal/config/config.go | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/cmd/work.go b/cmd/work.go index 8932833..4b8142d 100644 --- a/cmd/work.go +++ b/cmd/work.go @@ -145,7 +145,13 @@ func runWorkNew(cmd *cobra.Command, args []string) error { originPath := filepath.Join(basePath, repo) worktreeName := fmt.Sprintf("%s-%s", repo, branch) worktreePath := filepath.Join(basePath, worktreeName) - gitBranch := fmt.Sprintf("mgreau/%s", branch) + prefix := cfg.GetBranchPrefix() + var gitBranch string + if prefix != "" { + gitBranch = fmt.Sprintf("%s/%s", prefix, branch) + } else { + gitBranch = branch + } // Check if worktree already exists if _, err := os.Stat(worktreePath); err == nil { diff --git a/internal/config/config.go b/internal/config/config.go index 268f338..cce6875 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,6 +3,7 @@ package config import ( "fmt" "os" + "os/exec" "path/filepath" "strings" "time" @@ -18,6 +19,7 @@ type Config struct { PollInterval string `yaml:"poll_interval"` ClaudeBin string `yaml:"claude_bin"` Terminal string `yaml:"terminal"` // "iterm" or "ghostty" + BranchPrefix string `yaml:"branch_prefix"` Watch WatchConfig `yaml:"watch"` } @@ -127,6 +129,23 @@ func (c *Config) GetTerminal() string { return c.Terminal } +// GetBranchPrefix returns the prefix for feature branch names. +// Falls back to git config user name, then empty string. +func (c *Config) GetBranchPrefix() string { + if c.BranchPrefix != "" { + return c.BranchPrefix + } + // Try git config + out, err := exec.Command("git", "config", "user.name").Output() + if err == nil { + name := strings.TrimSpace(string(out)) + if name != "" { + return name + } + } + return "" +} + // expandPaths replaces ~ with $HOME in base paths. func (c *Config) expandPaths() { home := os.Getenv("HOME")