refactor: extract ExecWithOutput utility for command execution (gt-vurfr)
Create util.ExecWithOutput and util.ExecRun to consolidate repeated exec.Command patterns across witness/handlers.go and refinery/manager.go. Changes: - Add internal/util/exec.go with ExecWithOutput (returns stdout) and ExecRun (runs command without output) - Refactor witness/handlers.go to use utility functions (7 call sites) - Refactor refinery/manager.go, removing unused gitRun/gitOutput methods - Add comprehensive tests in exec_test.go 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,13 +1,11 @@
|
|||||||
package refinery
|
package refinery
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -489,56 +487,7 @@ func (m *Manager) runTests(testCmd string) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
cmd := exec.Command(parts[0], parts[1:]...) //nolint:gosec // G204: testCmd is from trusted rig config
|
return util.ExecRun(m.workDir, parts[0], parts[1:]...)
|
||||||
cmd.Dir = m.workDir
|
|
||||||
|
|
||||||
var stderr bytes.Buffer
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
return fmt.Errorf("%s: %s", err, strings.TrimSpace(stderr.String()))
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// gitRun executes a git command.
|
|
||||||
func (m *Manager) gitRun(args ...string) error {
|
|
||||||
cmd := exec.Command("git", args...)
|
|
||||||
cmd.Dir = m.workDir
|
|
||||||
|
|
||||||
var stderr bytes.Buffer
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// gitOutput executes a git command and returns stdout.
|
|
||||||
func (m *Manager) gitOutput(args ...string) (string, error) {
|
|
||||||
cmd := exec.Command("git", args...)
|
|
||||||
cmd.Dir = m.workDir
|
|
||||||
|
|
||||||
var stdout, stderr bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return "", fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return "", err
|
|
||||||
}
|
|
||||||
|
|
||||||
return strings.TrimSpace(stdout.String()), nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// getMergeConfig loads the merge configuration from disk.
|
// getMergeConfig loads the merge configuration from disk.
|
||||||
@@ -579,7 +528,7 @@ func (m *Manager) pushWithRetry(targetBranch string, config MergeConfig) error {
|
|||||||
delay *= 2 // Exponential backoff
|
delay *= 2 // Exponential backoff
|
||||||
}
|
}
|
||||||
|
|
||||||
err := m.gitRun("push", "origin", targetBranch)
|
err := util.ExecRun(m.workDir, "git", "push", "origin", targetBranch)
|
||||||
if err == nil {
|
if err == nil {
|
||||||
return nil // Success
|
return nil // Success
|
||||||
}
|
}
|
||||||
|
|||||||
49
internal/util/exec.go
Normal file
49
internal/util/exec.go
Normal file
@@ -0,0 +1,49 @@
|
|||||||
|
package util
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"fmt"
|
||||||
|
"os/exec"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ExecWithOutput runs a command in the specified directory and returns stdout.
|
||||||
|
// If the command fails, stderr content is included in the error message.
|
||||||
|
func ExecWithOutput(workDir, cmd string, args ...string) (string, error) {
|
||||||
|
c := exec.Command(cmd, args...) //nolint:gosec // G204: callers validate args
|
||||||
|
c.Dir = workDir
|
||||||
|
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
c.Stdout = &stdout
|
||||||
|
c.Stderr = &stderr
|
||||||
|
|
||||||
|
if err := c.Run(); err != nil {
|
||||||
|
errMsg := strings.TrimSpace(stderr.String())
|
||||||
|
if errMsg != "" {
|
||||||
|
return "", fmt.Errorf("%s", errMsg)
|
||||||
|
}
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
|
||||||
|
return strings.TrimSpace(stdout.String()), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// ExecRun runs a command in the specified directory.
|
||||||
|
// If the command fails, stderr content is included in the error message.
|
||||||
|
func ExecRun(workDir, cmd string, args ...string) error {
|
||||||
|
c := exec.Command(cmd, args...) //nolint:gosec // G204: callers validate args
|
||||||
|
c.Dir = workDir
|
||||||
|
|
||||||
|
var stderr bytes.Buffer
|
||||||
|
c.Stderr = &stderr
|
||||||
|
|
||||||
|
if err := c.Run(); err != nil {
|
||||||
|
errMsg := strings.TrimSpace(stderr.String())
|
||||||
|
if errMsg != "" {
|
||||||
|
return fmt.Errorf("%s", errMsg)
|
||||||
|
}
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
67
internal/util/exec_test.go
Normal file
67
internal/util/exec_test.go
Normal file
@@ -0,0 +1,67 @@
|
|||||||
|
package util
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestExecWithOutput(t *testing.T) {
|
||||||
|
// Test successful command
|
||||||
|
output, err := ExecWithOutput(".", "echo", "hello")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExecWithOutput failed: %v", err)
|
||||||
|
}
|
||||||
|
if output != "hello" {
|
||||||
|
t.Errorf("expected 'hello', got %q", output)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test command that fails
|
||||||
|
_, err = ExecWithOutput(".", "false")
|
||||||
|
if err == nil {
|
||||||
|
t.Error("expected error for failing command")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExecRun(t *testing.T) {
|
||||||
|
// Test successful command
|
||||||
|
err := ExecRun(".", "true")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExecRun failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test command that fails
|
||||||
|
err = ExecRun(".", "false")
|
||||||
|
if err == nil {
|
||||||
|
t.Error("expected error for failing command")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExecWithOutput_WorkDir(t *testing.T) {
|
||||||
|
// Create a temp directory
|
||||||
|
tmpDir, err := os.MkdirTemp("", "exec-test")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
defer os.RemoveAll(tmpDir)
|
||||||
|
|
||||||
|
// Test that workDir is respected
|
||||||
|
output, err := ExecWithOutput(tmpDir, "pwd")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ExecWithOutput failed: %v", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(output, tmpDir) && !strings.Contains(tmpDir, output) {
|
||||||
|
t.Errorf("expected output to contain %q, got %q", tmpDir, output)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExecWithOutput_StderrInError(t *testing.T) {
|
||||||
|
// Test that stderr is captured in error
|
||||||
|
_, err := ExecWithOutput(".", "sh", "-c", "echo 'error message' >&2; exit 1")
|
||||||
|
if err == nil {
|
||||||
|
t.Error("expected error")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "error message") {
|
||||||
|
t.Errorf("expected error to contain stderr, got %q", err.Error())
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,10 +1,8 @@
|
|||||||
package witness
|
package witness
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os/exec"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -14,6 +12,7 @@ import (
|
|||||||
"github.com/steveyegge/gastown/internal/mail"
|
"github.com/steveyegge/gastown/internal/mail"
|
||||||
"github.com/steveyegge/gastown/internal/rig"
|
"github.com/steveyegge/gastown/internal/rig"
|
||||||
"github.com/steveyegge/gastown/internal/tmux"
|
"github.com/steveyegge/gastown/internal/tmux"
|
||||||
|
"github.com/steveyegge/gastown/internal/util"
|
||||||
"github.com/steveyegge/gastown/internal/workspace"
|
"github.com/steveyegge/gastown/internal/workspace"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -337,28 +336,17 @@ func createCleanupWisp(workDir, polecatName, issueID, branch string) (string, er
|
|||||||
|
|
||||||
labels := strings.Join(CleanupWispLabels(polecatName, "pending"), ",")
|
labels := strings.Join(CleanupWispLabels(polecatName, "pending"), ",")
|
||||||
|
|
||||||
cmd := exec.Command("bd", "create", //nolint:gosec // G204: args are constructed internally
|
output, err := util.ExecWithOutput(workDir, "bd", "create",
|
||||||
"--wisp",
|
"--wisp",
|
||||||
"--title", title,
|
"--title", title,
|
||||||
"--description", description,
|
"--description", description,
|
||||||
"--labels", labels,
|
"--labels", labels,
|
||||||
)
|
)
|
||||||
cmd.Dir = workDir
|
if err != nil {
|
||||||
|
|
||||||
var stdout, stderr bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return "", fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract wisp ID from output (bd create outputs "Created: <id>")
|
// Extract wisp ID from output (bd create outputs "Created: <id>")
|
||||||
output := strings.TrimSpace(stdout.String())
|
|
||||||
if strings.HasPrefix(output, "Created:") {
|
if strings.HasPrefix(output, "Created:") {
|
||||||
return strings.TrimSpace(strings.TrimPrefix(output, "Created:")), nil
|
return strings.TrimSpace(strings.TrimPrefix(output, "Created:")), nil
|
||||||
}
|
}
|
||||||
@@ -382,27 +370,16 @@ func createSwarmWisp(workDir string, payload *SwarmStartPayload) (string, error)
|
|||||||
|
|
||||||
labels := strings.Join(SwarmWispLabels(payload.SwarmID, payload.Total, 0, payload.StartedAt), ",")
|
labels := strings.Join(SwarmWispLabels(payload.SwarmID, payload.Total, 0, payload.StartedAt), ",")
|
||||||
|
|
||||||
cmd := exec.Command("bd", "create", //nolint:gosec // G204: args are constructed internally
|
output, err := util.ExecWithOutput(workDir, "bd", "create",
|
||||||
"--wisp",
|
"--wisp",
|
||||||
"--title", title,
|
"--title", title,
|
||||||
"--description", description,
|
"--description", description,
|
||||||
"--labels", labels,
|
"--labels", labels,
|
||||||
)
|
)
|
||||||
cmd.Dir = workDir
|
if err != nil {
|
||||||
|
|
||||||
var stdout, stderr bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return "", fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
output := strings.TrimSpace(stdout.String())
|
|
||||||
if strings.HasPrefix(output, "Created:") {
|
if strings.HasPrefix(output, "Created:") {
|
||||||
return strings.TrimSpace(strings.TrimPrefix(output, "Created:")), nil
|
return strings.TrimSpace(strings.TrimPrefix(output, "Created:")), nil
|
||||||
}
|
}
|
||||||
@@ -412,32 +389,21 @@ func createSwarmWisp(workDir string, payload *SwarmStartPayload) (string, error)
|
|||||||
|
|
||||||
// findCleanupWisp finds an existing cleanup wisp for a polecat.
|
// findCleanupWisp finds an existing cleanup wisp for a polecat.
|
||||||
func findCleanupWisp(workDir, polecatName string) (string, error) {
|
func findCleanupWisp(workDir, polecatName string) (string, error) {
|
||||||
cmd := exec.Command("bd", "list", //nolint:gosec // G204: bd is a trusted internal tool
|
output, err := util.ExecWithOutput(workDir, "bd", "list",
|
||||||
"--wisp",
|
"--wisp",
|
||||||
"--labels", fmt.Sprintf("polecat:%s,state:merge-requested", polecatName),
|
"--labels", fmt.Sprintf("polecat:%s,state:merge-requested", polecatName),
|
||||||
"--status", "open",
|
"--status", "open",
|
||||||
"--json",
|
"--json",
|
||||||
)
|
)
|
||||||
cmd.Dir = workDir
|
if err != nil {
|
||||||
|
|
||||||
var stdout, stderr bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
// Empty result is fine
|
// Empty result is fine
|
||||||
if strings.Contains(stderr.String(), "no issues found") {
|
if strings.Contains(err.Error(), "no issues found") {
|
||||||
return "", nil
|
return "", nil
|
||||||
}
|
}
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return "", fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse JSON to get the wisp ID
|
// Parse JSON to get the wisp ID
|
||||||
output := strings.TrimSpace(stdout.String())
|
|
||||||
if output == "" || output == "[]" || output == "null" {
|
if output == "" || output == "[]" || output == "null" {
|
||||||
return "", nil
|
return "", nil
|
||||||
}
|
}
|
||||||
@@ -477,26 +443,19 @@ func getCleanupStatus(workDir, rigName, polecatName string) string {
|
|||||||
prefix := beads.GetPrefixForRig(townRoot, rigName)
|
prefix := beads.GetPrefixForRig(townRoot, rigName)
|
||||||
agentBeadID := beads.PolecatBeadIDWithPrefix(prefix, rigName, polecatName)
|
agentBeadID := beads.PolecatBeadIDWithPrefix(prefix, rigName, polecatName)
|
||||||
|
|
||||||
cmd := exec.Command("bd", "show", agentBeadID, "--json") //nolint:gosec // G204: agentBeadID is validated internally
|
output, err := util.ExecWithOutput(workDir, "bd", "show", agentBeadID, "--json")
|
||||||
cmd.Dir = workDir
|
if err != nil {
|
||||||
|
|
||||||
var stdout, stderr bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
// Agent bead doesn't exist or bd failed - return empty (unknown status)
|
// Agent bead doesn't exist or bd failed - return empty (unknown status)
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
output := stdout.Bytes()
|
if output == "" {
|
||||||
if len(output) == 0 {
|
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse the JSON response
|
// Parse the JSON response
|
||||||
var resp agentBeadResponse
|
var resp agentBeadResponse
|
||||||
if err := json.Unmarshal(output, &resp); err != nil {
|
if err := json.Unmarshal([]byte(output), &resp); err != nil {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -599,18 +558,12 @@ DO NOT nuke without --force after recovery.`,
|
|||||||
// UpdateCleanupWispState updates a cleanup wisp's state label.
|
// UpdateCleanupWispState updates a cleanup wisp's state label.
|
||||||
func UpdateCleanupWispState(workDir, wispID, newState string) error {
|
func UpdateCleanupWispState(workDir, wispID, newState string) error {
|
||||||
// Get current labels to preserve other labels
|
// Get current labels to preserve other labels
|
||||||
cmd := exec.Command("bd", "show", wispID, "--json")
|
output, err := util.ExecWithOutput(workDir, "bd", "show", wispID, "--json")
|
||||||
cmd.Dir = workDir
|
if err != nil {
|
||||||
|
|
||||||
var stdout bytes.Buffer
|
|
||||||
cmd.Stdout = &stdout
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
return fmt.Errorf("getting wisp: %w", err)
|
return fmt.Errorf("getting wisp: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract polecat name from existing labels for the update
|
// Extract polecat name from existing labels for the update
|
||||||
output := stdout.String()
|
|
||||||
var polecatName string
|
var polecatName string
|
||||||
if idx := strings.Index(output, `polecat:`); idx >= 0 {
|
if idx := strings.Index(output, `polecat:`); idx >= 0 {
|
||||||
rest := output[idx+8:]
|
rest := output[idx+8:]
|
||||||
@@ -626,21 +579,7 @@ func UpdateCleanupWispState(workDir, wispID, newState string) error {
|
|||||||
// Update with new state
|
// Update with new state
|
||||||
newLabels := strings.Join(CleanupWispLabels(polecatName, newState), ",")
|
newLabels := strings.Join(CleanupWispLabels(polecatName, newState), ",")
|
||||||
|
|
||||||
updateCmd := exec.Command("bd", "update", wispID, "--labels", newLabels) //nolint:gosec // G204: args are constructed internally
|
return util.ExecRun(workDir, "bd", "update", wispID, "--labels", newLabels)
|
||||||
updateCmd.Dir = workDir
|
|
||||||
|
|
||||||
var stderr bytes.Buffer
|
|
||||||
updateCmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := updateCmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return fmt.Errorf("%s", errMsg)
|
|
||||||
}
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// NukePolecat executes the actual nuke operation for a polecat.
|
// NukePolecat executes the actual nuke operation for a polecat.
|
||||||
@@ -671,17 +610,7 @@ func NukePolecat(workDir, rigName, polecatName string) error {
|
|||||||
// Now run gt polecat nuke to clean up worktree, branch, and beads
|
// Now run gt polecat nuke to clean up worktree, branch, and beads
|
||||||
address := fmt.Sprintf("%s/%s", rigName, polecatName)
|
address := fmt.Sprintf("%s/%s", rigName, polecatName)
|
||||||
|
|
||||||
cmd := exec.Command("gt", "polecat", "nuke", address) //nolint:gosec // G204: address is constructed from validated internal data
|
if err := util.ExecRun(workDir, "gt", "polecat", "nuke", address); err != nil {
|
||||||
cmd.Dir = workDir
|
|
||||||
|
|
||||||
var stderr bytes.Buffer
|
|
||||||
cmd.Stderr = &stderr
|
|
||||||
|
|
||||||
if err := cmd.Run(); err != nil {
|
|
||||||
errMsg := strings.TrimSpace(stderr.String())
|
|
||||||
if errMsg != "" {
|
|
||||||
return fmt.Errorf("nuke failed: %s", errMsg)
|
|
||||||
}
|
|
||||||
return fmt.Errorf("nuke failed: %w", err)
|
return fmt.Errorf("nuke failed: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user