diff --git a/pkg/agentic/auto_pr.go b/pkg/agentic/auto_pr.go index c2471bd..b70ad71 100644 --- a/pkg/agentic/auto_pr.go +++ b/pkg/agentic/auto_pr.go @@ -4,7 +4,6 @@ package agentic import ( "context" - "os/exec" "time" core "dappco.re/go/core" @@ -18,21 +17,19 @@ func (s *PrepSubsystem) autoCreatePR(wsDir string) { return } + ctx := context.Background() repoDir := core.JoinPath(wsDir, "repo") // PRs target dev — agents never merge directly to main base := "dev" - diffCmd := exec.Command("git", "log", "--oneline", "origin/"+base+"..HEAD") - diffCmd.Dir = repoDir - out, err := diffCmd.Output() - if err != nil || len(core.Trim(string(out))) == 0 { + out := gitOutput(ctx, repoDir, "log", "--oneline", "origin/"+base+"..HEAD") + if out == "" { return } - commitCount := len(core.Split(core.Trim(string(out)), "\n")) + commitCount := len(core.Split(out, "\n")) - // Get the repo's forge remote URL to extract org/repo org := st.Org if org == "" { org = "core" @@ -40,12 +37,9 @@ func (s *PrepSubsystem) autoCreatePR(wsDir string) { // Push the branch to forge forgeRemote := core.Sprintf("ssh://git@forge.lthn.ai:2223/%s/%s.git", org, st.Repo) - pushCmd := exec.Command("git", "push", forgeRemote, st.Branch) - pushCmd.Dir = repoDir - if pushErr := pushCmd.Run(); pushErr != nil { - // Push failed — update status with error but don't block + if !gitCmdOK(ctx, repoDir, "push", forgeRemote, st.Branch) { if st2, err := ReadStatus(wsDir); err == nil { - st2.Question = core.Sprintf("PR push failed: %v", pushErr) + st2.Question = "PR push failed" writeStatus(wsDir, st2) } return diff --git a/pkg/agentic/dispatch.go b/pkg/agentic/dispatch.go index b744d3e..3239bdc 100644 --- a/pkg/agentic/dispatch.go +++ b/pkg/agentic/dispatch.go @@ -4,7 +4,6 @@ package agentic import ( "context" - "os/exec" "syscall" "time" @@ -394,20 +393,17 @@ func (s *PrepSubsystem) spawnAgent(agent, prompt, wsDir string) (int, string, er // runQA runs build + test checks on the repo after agent completion. // Returns true if QA passes, false if build or tests fail. func (s *PrepSubsystem) runQA(wsDir string) bool { + ctx := context.Background() repoDir := core.JoinPath(wsDir, "repo") - // Detect language and run appropriate checks if fs.IsFile(core.JoinPath(repoDir, "go.mod")) { - // Go: build + vet + test for _, args := range [][]string{ {"go", "build", "./..."}, {"go", "vet", "./..."}, {"go", "test", "./...", "-count=1", "-timeout", "120s"}, } { - cmd := exec.Command(args[0], args[1:]...) - cmd.Dir = repoDir - if err := cmd.Run(); err != nil { - core.Warn("QA failed", "cmd", core.Join(" ", args...), "err", err) + if !runCmdOK(ctx, repoDir, args[0], args[1:]...) { + core.Warn("QA failed", "cmd", core.Join(" ", args...)) return false } } @@ -415,28 +411,19 @@ func (s *PrepSubsystem) runQA(wsDir string) bool { } if fs.IsFile(core.JoinPath(repoDir, "composer.json")) { - install := exec.Command("composer", "install", "--no-interaction") - install.Dir = repoDir - if err := install.Run(); err != nil { + if !runCmdOK(ctx, repoDir, "composer", "install", "--no-interaction") { return false } - test := exec.Command("composer", "test") - test.Dir = repoDir - return test.Run() == nil + return runCmdOK(ctx, repoDir, "composer", "test") } if fs.IsFile(core.JoinPath(repoDir, "package.json")) { - install := exec.Command("npm", "install") - install.Dir = repoDir - if err := install.Run(); err != nil { + if !runCmdOK(ctx, repoDir, "npm", "install") { return false } - test := exec.Command("npm", "test") - test.Dir = repoDir - return test.Run() == nil + return runCmdOK(ctx, repoDir, "npm", "test") } - // Unknown language — pass QA (no checks to run) return true } diff --git a/pkg/agentic/mirror.go b/pkg/agentic/mirror.go index 2da4c7b..6d02644 100644 --- a/pkg/agentic/mirror.go +++ b/pkg/agentic/mirror.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "os" - "os/exec" core "dappco.re/go/core" "github.com/modelcontextprotocol/go-sdk/mcp" @@ -86,9 +85,7 @@ func (s *PrepSubsystem) mirror(ctx context.Context, _ *mcp.CallToolRequest, inpu } // Fetch github to get current state - fetchCmd := exec.CommandContext(ctx, "git", "fetch", "github") - fetchCmd.Dir = repoDir - fetchCmd.Run() + gitCmdOK(ctx, repoDir, "fetch", "github") // Check how far ahead local default branch is vs github localBase := DefaultBranch(repoDir) @@ -124,9 +121,7 @@ func (s *PrepSubsystem) mirror(ctx context.Context, _ *mcp.CallToolRequest, inpu // Push local main to github dev (explicit main, not HEAD) base := DefaultBranch(repoDir) - pushCmd := exec.CommandContext(ctx, "git", "push", "github", base+":refs/heads/dev", "--force") - pushCmd.Dir = repoDir - if err := pushCmd.Run(); err != nil { + if _, err := gitCmd(ctx, repoDir, "push", "github", base+":refs/heads/dev", "--force"); err != nil { sync.Skipped = core.Sprintf("push failed: %v", err) synced = append(synced, sync) continue @@ -154,93 +149,62 @@ func (s *PrepSubsystem) mirror(ctx context.Context, _ *mcp.CallToolRequest, inpu // createGitHubPR creates a PR from dev → main using the gh CLI. func (s *PrepSubsystem) createGitHubPR(ctx context.Context, repoDir, repo string, commits, files int) (string, error) { - // Check if there's already an open PR from dev ghRepo := core.Sprintf("%s/%s", GitHubOrg(), repo) - checkCmd := exec.CommandContext(ctx, "gh", "pr", "list", "--repo", ghRepo, "--head", "dev", "--state", "open", "--json", "url", "--limit", "1") - checkCmd.Dir = repoDir - out, err := checkCmd.Output() - if err == nil && core.Contains(string(out), "url") { - // PR already exists — extract URL - // Format: [{"url":"https://..."}] - url := extractJSONField(string(out), "url") - if url != "" { + + // Check if there's already an open PR from dev + out, err := runCmd(ctx, repoDir, "gh", "pr", "list", "--repo", ghRepo, "--head", "dev", "--state", "open", "--json", "url", "--limit", "1") + if err == nil && core.Contains(out, "url") { + if url := extractJSONField(out, "url"); url != "" { return url, nil } } - // Build PR body body := core.Sprintf("## Forge → GitHub Sync\n\n"+ - "**Commits:** %d\n"+ - "**Files changed:** %d\n\n"+ + "**Commits:** %d\n**Files changed:** %d\n\n"+ "Automated sync from Forge (forge.lthn.ai) to GitHub mirror.\n"+ - "Review with CodeRabbit before merging.\n\n"+ - "---\n"+ + "Review with CodeRabbit before merging.\n\n---\n"+ "Co-Authored-By: Virgil ", commits, files) title := core.Sprintf("[sync] %s: %d commits, %d files", repo, commits, files) - prCmd := exec.CommandContext(ctx, "gh", "pr", "create", - "--repo", ghRepo, - "--head", "dev", - "--base", "main", - "--title", title, - "--body", body, - ) - prCmd.Dir = repoDir - prOut, err := prCmd.CombinedOutput() + prOut, err := runCmd(ctx, repoDir, "gh", "pr", "create", + "--repo", ghRepo, "--head", "dev", "--base", "main", + "--title", title, "--body", body) if err != nil { - return "", core.E("createGitHubPR", string(prOut), err) + return "", core.E("createGitHubPR", prOut, err) } - // gh pr create outputs the PR URL on the last line - lines := core.Split(core.Trim(string(prOut)), "\n") + lines := core.Split(core.Trim(prOut), "\n") if len(lines) > 0 { return lines[len(lines)-1], nil } - return "", nil } // ensureDevBranch creates the dev branch on GitHub if it doesn't exist. func ensureDevBranch(repoDir string) { - // Try to push current main as dev — if dev exists this is a no-op (we force-push later) - cmd := exec.Command("git", "push", "github", "HEAD:refs/heads/dev") - cmd.Dir = repoDir - cmd.Run() // Ignore error — branch may already exist + gitCmdOK(context.Background(), repoDir, "push", "github", "HEAD:refs/heads/dev") } // hasRemote checks if a git remote exists. func hasRemote(repoDir, name string) bool { - cmd := exec.Command("git", "remote", "get-url", name) - cmd.Dir = repoDir - return cmd.Run() == nil + return gitCmdOK(context.Background(), repoDir, "remote", "get-url", name) } // commitsAhead returns how many commits HEAD is ahead of the ref. func commitsAhead(repoDir, base, head string) int { - cmd := exec.Command("git", "rev-list", base+".."+head, "--count") - cmd.Dir = repoDir - out, err := cmd.Output() - if err != nil { - return 0 - } - return parseInt(string(out)) + out := gitOutput(context.Background(), repoDir, "rev-list", base+".."+head, "--count") + return parseInt(out) } // filesChanged returns the number of files changed between two refs. func filesChanged(repoDir, base, head string) int { - cmd := exec.Command("git", "diff", "--name-only", base+".."+head) - cmd.Dir = repoDir - out, err := cmd.Output() - if err != nil { + out := gitOutput(context.Background(), repoDir, "diff", "--name-only", base+".."+head) + if out == "" { return 0 } - lines := core.Split(core.Trim(string(out)), "\n") - if len(lines) == 1 && lines[0] == "" { - return 0 - } - return len(lines) + return len(core.Split(out, "\n")) } // listLocalRepos returns repo names that exist as directories in basePath. diff --git a/pkg/agentic/paths.go b/pkg/agentic/paths.go index 4277a79..94b67fe 100644 --- a/pkg/agentic/paths.go +++ b/pkg/agentic/paths.go @@ -3,7 +3,7 @@ package agentic import ( - "os/exec" + "context" "strconv" "unsafe" @@ -76,19 +76,15 @@ func AgentName() string { // // base := agentic.DefaultBranch("./src") func DefaultBranch(repoDir string) string { - cmd := exec.Command("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short") - cmd.Dir = repoDir - if out, err := cmd.Output(); err == nil { - ref := core.Trim(string(out)) + ctx := context.Background() + if ref := gitOutput(ctx, repoDir, "symbolic-ref", "refs/remotes/origin/HEAD", "--short"); ref != "" { if core.HasPrefix(ref, "origin/") { return core.TrimPrefix(ref, "origin/") } return ref } for _, branch := range []string{"main", "master"} { - cmd := exec.Command("git", "rev-parse", "--verify", branch) - cmd.Dir = repoDir - if cmd.Run() == nil { + if gitCmdOK(ctx, repoDir, "rev-parse", "--verify", branch) { return branch } } diff --git a/pkg/agentic/pr.go b/pkg/agentic/pr.go index 2cc298a..285bf39 100644 --- a/pkg/agentic/pr.go +++ b/pkg/agentic/pr.go @@ -4,7 +4,6 @@ package agentic import ( "context" - "os/exec" core "dappco.re/go/core" "dappco.re/go/core/forge" @@ -67,14 +66,11 @@ func (s *PrepSubsystem) createPR(ctx context.Context, _ *mcp.CallToolRequest, in } if st.Branch == "" { - // Detect branch from git - branchCmd := exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "HEAD") - branchCmd.Dir = repoDir - out, err := branchCmd.Output() - if err != nil { - return nil, CreatePROutput{}, core.E("createPR", "failed to detect branch", err) + branch := gitOutput(ctx, repoDir, "rev-parse", "--abbrev-ref", "HEAD") + if branch == "" { + return nil, CreatePROutput{}, core.E("createPR", "failed to detect branch", nil) } - st.Branch = core.Trim(string(out)) + st.Branch = branch } org := st.Org @@ -112,11 +108,9 @@ func (s *PrepSubsystem) createPR(ctx context.Context, _ *mcp.CallToolRequest, in // Push branch to Forge (origin is the local clone, not Forge) forgeRemote := core.Sprintf("ssh://git@forge.lthn.ai:2223/%s/%s.git", org, st.Repo) - pushCmd := exec.CommandContext(ctx, "git", "push", forgeRemote, st.Branch) - pushCmd.Dir = repoDir - pushOut, err := pushCmd.CombinedOutput() - if err != nil { - return nil, CreatePROutput{}, core.E("createPR", "git push failed: "+string(pushOut), err) + pushOut, pushErr := gitCmd(ctx, repoDir, "push", forgeRemote, st.Branch) + if pushErr != nil { + return nil, CreatePROutput{}, core.E("createPR", "git push failed: "+pushOut, pushErr) } // Create PR via Forge API diff --git a/pkg/agentic/prep.go b/pkg/agentic/prep.go index 2f5af64..8c9c5fa 100644 --- a/pkg/agentic/prep.go +++ b/pkg/agentic/prep.go @@ -10,7 +10,6 @@ import ( "encoding/json" goio "io" "net/http" - "os/exec" "sync" "time" @@ -248,8 +247,7 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques if !resumed { // Clone repo into repo/ - cloneCmd := exec.CommandContext(ctx, "git", "clone", repoPath, repoDir) - if cloneErr := cloneCmd.Run(); cloneErr != nil { + if _, cloneErr := gitCmd(ctx, ".", "clone", repoPath, repoDir); cloneErr != nil { return nil, PrepOutput{}, core.E("prep", "git clone failed for "+input.Repo, cloneErr) } @@ -266,19 +264,13 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques } branchName := core.Sprintf("agent/%s", taskSlug) - branchCmd := exec.CommandContext(ctx, "git", "checkout", "-b", branchName) - branchCmd.Dir = repoDir - if branchErr := branchCmd.Run(); branchErr != nil { + if _, branchErr := gitCmd(ctx, repoDir, "checkout", "-b", branchName); branchErr != nil { return nil, PrepOutput{}, core.E("prep.branch", core.Sprintf("failed to create branch %q", branchName), branchErr) } out.Branch = branchName } else { // Resume: read branch from existing checkout - branchCmd := exec.CommandContext(ctx, "git", "rev-parse", "--abbrev-ref", "HEAD") - branchCmd.Dir = repoDir - if branchOut, branchErr := branchCmd.Output(); branchErr == nil { - out.Branch = core.Trim(string(branchOut)) - } + out.Branch = gitOutput(ctx, repoDir, "rev-parse", "--abbrev-ref", "HEAD") } // Build the rich prompt with all context @@ -492,13 +484,7 @@ func (s *PrepSubsystem) findConsumersList(repo string) (string, int) { } func (s *PrepSubsystem) getGitLog(repoPath string) string { - cmd := exec.Command("git", "log", "--oneline", "-20") - cmd.Dir = repoPath - output, err := cmd.Output() - if err != nil { - return "" - } - return core.Trim(string(output)) + return gitOutput(context.Background(), repoPath, "log", "--oneline", "-20") } func (s *PrepSubsystem) pullWikiContent(ctx context.Context, org, repo string) string { diff --git a/pkg/agentic/proc.go b/pkg/agentic/proc.go new file mode 100644 index 0000000..7cd835c --- /dev/null +++ b/pkg/agentic/proc.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: EUPL-1.2 + +// Process execution helpers — wraps go-process for testable command execution. +// All external command execution in the agentic package goes through these helpers. + +package agentic + +import ( + "context" + "sync" + + core "dappco.re/go/core" + "dappco.re/go/core/process" +) + +var procOnce sync.Once + +// ensureProcess lazily initialises the default process service. +func ensureProcess() { + procOnce.Do(func() { + if process.Default() == nil { + c := core.New() + svc, err := process.NewService(process.Options{})(c) + if err == nil { + if s, ok := svc.(*process.Service); ok { + process.SetDefault(s) + } + } + } + }) +} + +// runCmd executes a command in a directory and returns (output, error). +// Uses go-process RunWithOptions for testability. +// +// out, err := runCmd(ctx, repoDir, "git", "log", "--oneline", "-20") +func runCmd(ctx context.Context, dir string, command string, args ...string) (string, error) { + ensureProcess() + return process.RunWithOptions(ctx, process.RunOptions{ + Command: command, + Args: args, + Dir: dir, + }) +} + +// runCmdEnv executes a command with additional environment variables. +// +// out, err := runCmdEnv(ctx, repoDir, []string{"GOWORK=off"}, "go", "test", "./...") +func runCmdEnv(ctx context.Context, dir string, env []string, command string, args ...string) (string, error) { + ensureProcess() + return process.RunWithOptions(ctx, process.RunOptions{ + Command: command, + Args: args, + Dir: dir, + Env: env, + }) +} + +// runCmdOK executes a command and returns true if it exits 0. +// +// if runCmdOK(ctx, repoDir, "go", "build", "./...") { ... } +func runCmdOK(ctx context.Context, dir string, command string, args ...string) bool { + _, err := runCmd(ctx, dir, command, args...) + return err == nil +} + +// gitCmd runs a git command in the given directory. +// +// out, err := gitCmd(ctx, repoDir, "log", "--oneline", "-20") +func gitCmd(ctx context.Context, dir string, args ...string) (string, error) { + return runCmd(ctx, dir, "git", args...) +} + +// gitCmdOK runs a git command and returns true if it exits 0. +// +// if gitCmdOK(ctx, repoDir, "fetch", "origin", "main") { ... } +func gitCmdOK(ctx context.Context, dir string, args ...string) bool { + return runCmdOK(ctx, dir, "git", args...) +} + +// gitOutput runs a git command and returns trimmed stdout. +// +// branch := gitOutput(ctx, repoDir, "rev-parse", "--abbrev-ref", "HEAD") +func gitOutput(ctx context.Context, dir string, args ...string) string { + out, err := gitCmd(ctx, dir, args...) + if err != nil { + return "" + } + return core.Trim(out) +} diff --git a/pkg/agentic/review_queue.go b/pkg/agentic/review_queue.go index 2f8d4c8..5193189 100644 --- a/pkg/agentic/review_queue.go +++ b/pkg/agentic/review_queue.go @@ -7,7 +7,6 @@ import ( "encoding/json" "io" "os" - "os/exec" "regexp" "time" @@ -173,9 +172,8 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer if reviewer == "" { reviewer = "coderabbit" } - cmd := s.buildReviewCommand(ctx, repoDir, reviewer) - out, err := cmd.CombinedOutput() - output := string(out) + command, args := s.buildReviewCommand(repoDir, reviewer) + output, err := runCmd(ctx, repoDir, command, args...) // Parse rate limit (both reviewers use similar patterns) if core.Contains(output, "Rate limit exceeded") || core.Contains(output, "rate limit") { @@ -249,23 +247,15 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer // pushAndMerge pushes to GitHub dev and merges the PR. func (s *PrepSubsystem) pushAndMerge(ctx context.Context, repoDir, repo string) error { - // Push to dev - pushCmd := exec.CommandContext(ctx, "git", "push", "github", "HEAD:refs/heads/dev", "--force") - pushCmd.Dir = repoDir - if out, err := pushCmd.CombinedOutput(); err != nil { - return core.E("pushAndMerge", "push failed: "+string(out), err) + if out, err := gitCmd(ctx, repoDir, "push", "github", "HEAD:refs/heads/dev", "--force"); err != nil { + return core.E("pushAndMerge", "push failed: "+out, err) } // Mark PR ready if draft - readyCmd := exec.CommandContext(ctx, "gh", "pr", "ready", "--repo", GitHubOrg()+"/"+repo) - readyCmd.Dir = repoDir - readyCmd.Run() // Ignore error — might already be ready + runCmdOK(ctx, repoDir, "gh", "pr", "ready", "--repo", GitHubOrg()+"/"+repo) - // Try to merge - mergeCmd := exec.CommandContext(ctx, "gh", "pr", "merge", "--merge", "--delete-branch") - mergeCmd.Dir = repoDir - if out, err := mergeCmd.CombinedOutput(); err != nil { - return core.E("pushAndMerge", "merge failed: "+string(out), err) + if out, err := runCmd(ctx, repoDir, "gh", "pr", "merge", "--merge", "--delete-branch"); err != nil { + return core.E("pushAndMerge", "merge failed: "+out, err) } return nil @@ -324,16 +314,15 @@ func parseRetryAfter(message string) time.Duration { return 5 * time.Minute } -// buildReviewCommand creates the CLI command for the chosen reviewer. -func (s *PrepSubsystem) buildReviewCommand(ctx context.Context, repoDir, reviewer string) *exec.Cmd { +// buildReviewCommand returns the command and args for the chosen reviewer. +// +// cmd, args := s.buildReviewCommand(repoDir, "coderabbit") +func (s *PrepSubsystem) buildReviewCommand(repoDir, reviewer string) (string, []string) { switch reviewer { case "codex": - cmd := exec.CommandContext(ctx, "codex", "review", "--base", "github/main") - cmd.Dir = repoDir - return cmd + return "codex", []string{"review", "--base", "github/main"} default: // coderabbit - return exec.CommandContext(ctx, "coderabbit", "review", "--plain", - "--base", "github/main", "--config", "CLAUDE.md", "--cwd", repoDir) + return "coderabbit", []string{"review", "--plain", "--base", "github/main", "--config", "CLAUDE.md", "--cwd", repoDir} } } diff --git a/pkg/agentic/review_queue_extra_test.go b/pkg/agentic/review_queue_extra_test.go index 1b8f5de..85dc4af 100644 --- a/pkg/agentic/review_queue_extra_test.go +++ b/pkg/agentic/review_queue_extra_test.go @@ -19,38 +19,27 @@ import ( // --- buildReviewCommand --- func TestReviewQueue_BuildReviewCommand_Good_CodeRabbit(t *testing.T) { - s := &PrepSubsystem{ - backoff: make(map[string]time.Time), - failCount: make(map[string]int), - } - cmd := s.buildReviewCommand(context.Background(), "/tmp/repo", "coderabbit") - assert.Equal(t, "coderabbit", cmd.Path[len(cmd.Path)-len("coderabbit"):]) - assert.Contains(t, cmd.Args, "review") - assert.Contains(t, cmd.Args, "--plain") - assert.Contains(t, cmd.Args, "--base") - assert.Contains(t, cmd.Args, "github/main") + s := &PrepSubsystem{backoff: make(map[string]time.Time), failCount: make(map[string]int)} + cmd, args := s.buildReviewCommand("/tmp/repo", "coderabbit") + assert.Equal(t, "coderabbit", cmd) + assert.Contains(t, args, "review") + assert.Contains(t, args, "--plain") + assert.Contains(t, args, "github/main") } func TestReviewQueue_BuildReviewCommand_Good_Codex(t *testing.T) { - s := &PrepSubsystem{ - backoff: make(map[string]time.Time), - failCount: make(map[string]int), - } - cmd := s.buildReviewCommand(context.Background(), "/tmp/repo", "codex") - assert.Contains(t, cmd.Args, "review") - assert.Contains(t, cmd.Args, "--base") - assert.Contains(t, cmd.Args, "github/main") - assert.Equal(t, "/tmp/repo", cmd.Dir) + s := &PrepSubsystem{backoff: make(map[string]time.Time), failCount: make(map[string]int)} + cmd, args := s.buildReviewCommand("/tmp/repo", "codex") + assert.Equal(t, "codex", cmd) + assert.Contains(t, args, "review") + assert.Contains(t, args, "github/main") } func TestReviewQueue_BuildReviewCommand_Good_DefaultReviewer(t *testing.T) { - s := &PrepSubsystem{ - backoff: make(map[string]time.Time), - failCount: make(map[string]int), - } - // Empty string → defaults to coderabbit - cmd := s.buildReviewCommand(context.Background(), "/tmp/repo", "") - assert.Contains(t, cmd.Args, "--plain") + s := &PrepSubsystem{backoff: make(map[string]time.Time), failCount: make(map[string]int)} + cmd, args := s.buildReviewCommand("/tmp/repo", "") + assert.Equal(t, "coderabbit", cmd) + assert.Contains(t, args, "--plain") } // --- saveRateLimitState / loadRateLimitState --- @@ -253,19 +242,16 @@ func TestReviewQueue_BuildReviewCommand_Bad(t *testing.T) { backoff: make(map[string]time.Time), failCount: make(map[string]int), } - cmd := s.buildReviewCommand(context.Background(), "/tmp/repo", "") - assert.Contains(t, cmd.Args, "--plain") - assert.Contains(t, cmd.Args, "review") + cmd, args := s.buildReviewCommand("/tmp/repo", "") + assert.Equal(t, "coderabbit", cmd) + assert.Contains(t, args, "--plain") } func TestReviewQueue_BuildReviewCommand_Ugly(t *testing.T) { - // Unknown reviewer type — defaults to coderabbit - s := &PrepSubsystem{ - backoff: make(map[string]time.Time), - failCount: make(map[string]int), - } - cmd := s.buildReviewCommand(context.Background(), "/tmp/repo", "unknown-reviewer") - assert.Contains(t, cmd.Args, "--plain", "unknown reviewer should fall through to coderabbit default") + s := &PrepSubsystem{backoff: make(map[string]time.Time), failCount: make(map[string]int)} + cmd, args := s.buildReviewCommand("/tmp/repo", "unknown-reviewer") + assert.Equal(t, "coderabbit", cmd) + assert.Contains(t, args, "--plain") } // --- countFindings Bad/Ugly --- diff --git a/pkg/agentic/verify.go b/pkg/agentic/verify.go index 487bee9..8141b06 100644 --- a/pkg/agentic/verify.go +++ b/pkg/agentic/verify.go @@ -7,8 +7,6 @@ import ( "context" "encoding/json" "net/http" - "os" - "os/exec" "time" core "dappco.re/go/core" @@ -108,27 +106,18 @@ func (s *PrepSubsystem) attemptVerifyAndMerge(repoDir, org, repo, branch string, // rebaseBranch rebases the current branch onto the default branch and force-pushes. func (s *PrepSubsystem) rebaseBranch(repoDir, branch string) bool { + ctx := context.Background() base := DefaultBranch(repoDir) - // Fetch latest default branch - fetch := exec.Command("git", "fetch", "origin", base) - fetch.Dir = repoDir - if err := fetch.Run(); err != nil { + if !gitCmdOK(ctx, repoDir, "fetch", "origin", base) { return false } - // Rebase onto default branch - rebase := exec.Command("git", "rebase", "origin/"+base) - rebase.Dir = repoDir - if err := rebase.Run(); err != nil { - // Rebase failed — abort and give up - abort := exec.Command("git", "rebase", "--abort") - abort.Dir = repoDir - abort.Run() + if !gitCmdOK(ctx, repoDir, "rebase", "origin/"+base) { + gitCmdOK(ctx, repoDir, "rebase", "--abort") return false } - // Force-push the rebased branch to Forge (origin is local clone) st, _ := ReadStatus(core.PathDir(repoDir)) org := "core" repo := "" @@ -139,9 +128,7 @@ func (s *PrepSubsystem) rebaseBranch(repoDir, branch string) bool { repo = st.Repo } forgeRemote := core.Sprintf("ssh://git@forge.lthn.ai:2223/%s/%s.git", org, repo) - push := exec.Command("git", "push", "--force-with-lease", forgeRemote, branch) - push.Dir = repoDir - return push.Run() == nil + return gitCmdOK(ctx, repoDir, "push", "--force-with-lease", forgeRemote, branch) } // flagForReview adds the "needs-review" label to the PR via Forge API. @@ -237,44 +224,28 @@ func (s *PrepSubsystem) runVerification(repoDir string) verifyResult { } func (s *PrepSubsystem) runGoTests(repoDir string) verifyResult { - cmd := exec.Command("go", "test", "./...", "-count=1", "-timeout", "120s") - cmd.Dir = repoDir - cmd.Env = append(os.Environ(), "GOWORK=off") - out, err := cmd.CombinedOutput() - + ctx := context.Background() + out, err := runCmdEnv(ctx, repoDir, []string{"GOWORK=off"}, "go", "test", "./...", "-count=1", "-timeout", "120s") + passed := err == nil exitCode := 0 if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - exitCode = exitErr.ExitCode() - } else { - exitCode = 1 - } + exitCode = 1 } - - return verifyResult{passed: exitCode == 0, output: string(out), exitCode: exitCode, testCmd: "go test ./..."} + return verifyResult{passed: passed, output: out, exitCode: exitCode, testCmd: "go test ./..."} } func (s *PrepSubsystem) runPHPTests(repoDir string) verifyResult { - cmd := exec.Command("composer", "test", "--no-interaction") - cmd.Dir = repoDir - out, err := cmd.CombinedOutput() - - exitCode := 0 + ctx := context.Background() + out, err := runCmd(ctx, repoDir, "composer", "test", "--no-interaction") if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - exitCode = exitErr.ExitCode() - } else { - cmd2 := exec.Command("./vendor/bin/pest", "--no-interaction") - cmd2.Dir = repoDir - out2, err2 := cmd2.CombinedOutput() - if err2 != nil { - return verifyResult{passed: false, testCmd: "none", output: "No PHP test runner found (composer test and vendor/bin/pest both unavailable)", exitCode: 1} - } - return verifyResult{passed: true, output: string(out2), exitCode: 0, testCmd: "vendor/bin/pest"} + // Try pest as fallback + out2, err2 := runCmd(ctx, repoDir, "./vendor/bin/pest", "--no-interaction") + if err2 != nil { + return verifyResult{passed: false, testCmd: "none", output: "No PHP test runner found (composer test and vendor/bin/pest both unavailable)", exitCode: 1} } + return verifyResult{passed: true, output: out2, exitCode: 0, testCmd: "vendor/bin/pest"} } - - return verifyResult{passed: exitCode == 0, output: string(out), exitCode: exitCode, testCmd: "composer test"} + return verifyResult{passed: true, output: out, exitCode: 0, testCmd: "composer test"} } func (s *PrepSubsystem) runNodeTests(repoDir string) verifyResult { @@ -290,20 +261,14 @@ func (s *PrepSubsystem) runNodeTests(repoDir string) verifyResult { return verifyResult{passed: true, testCmd: "none", output: "No test script in package.json"} } - cmd := exec.Command("npm", "test") - cmd.Dir = repoDir - out, err := cmd.CombinedOutput() - + ctx := context.Background() + out, err := runCmd(ctx, repoDir, "npm", "test") + passed := err == nil exitCode := 0 if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - exitCode = exitErr.ExitCode() - } else { - exitCode = 1 - } + exitCode = 1 } - - return verifyResult{passed: exitCode == 0, output: string(out), exitCode: exitCode, testCmd: "npm test"} + return verifyResult{passed: passed, output: out, exitCode: exitCode, testCmd: "npm test"} } // forgeMergePR merges a PR via the Forge API.