From 98ce071b138ed2b9b36fd36ce4255ca8733238d0 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 21 Mar 2026 15:59:48 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20address=20Codex=20round=202=20findings?= =?UTF-8?q?=20=E2=80=94=203=20verified=20highs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit High: harvest no longer hardcodes 'main' — detects default branch via symbolic-ref/rev-parse fallback. Repos with master/other default branches are now harvested correctly. High: empty task no longer produces invalid 'agent/' branch name. Falls back to issue-N or work-timestamp. Branch creation errors are now surfaced instead of silently ignored. High: PHP verification no longer returns passed:true when no test runner exists. Untested PHP repos correctly fail verification. (brain/direct.go findings 5-6 verified as false positives — API returns top-level keys, not {data: ...} envelope) Co-Authored-By: Virgil --- pkg/agentic/prep.go | 11 ++++++++++- pkg/agentic/verify.go | 2 +- pkg/monitor/harvest.go | 40 +++++++++++++++++++++++++++++++++------- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/pkg/agentic/prep.go b/pkg/agentic/prep.go index e9634da..72cf1fb 100644 --- a/pkg/agentic/prep.go +++ b/pkg/agentic/prep.go @@ -186,11 +186,20 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques taskSlug = taskSlug[:40] } taskSlug = strings.Trim(taskSlug, "-") + if taskSlug == "" { + // Fallback for issue-only dispatches with no task text + taskSlug = fmt.Sprintf("issue-%d", input.Issue) + if input.Issue == 0 { + taskSlug = fmt.Sprintf("work-%d", time.Now().Unix()) + } + } branchName := fmt.Sprintf("agent/%s", taskSlug) branchCmd := exec.CommandContext(ctx, "git", "checkout", "-b", branchName) branchCmd.Dir = srcDir - branchCmd.Run() + if err := branchCmd.Run(); err != nil { + return nil, PrepOutput{}, coreerr.E("prep.branch", fmt.Sprintf("failed to create branch %q", branchName), err) + } out.Branch = branchName // Create context dirs inside src/ diff --git a/pkg/agentic/verify.go b/pkg/agentic/verify.go index 082bf02..d14505f 100644 --- a/pkg/agentic/verify.go +++ b/pkg/agentic/verify.go @@ -260,7 +260,7 @@ func (s *PrepSubsystem) runPHPTests(srcDir string) verifyResult { cmd2.Dir = srcDir out2, err2 := cmd2.CombinedOutput() if err2 != nil { - return verifyResult{passed: true, testCmd: "none", output: "No PHP test runner found"} + 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"} } diff --git a/pkg/monitor/harvest.go b/pkg/monitor/harvest.go index 603bcd7..7889b8d 100644 --- a/pkg/monitor/harvest.go +++ b/pkg/monitor/harvest.go @@ -147,14 +147,38 @@ func detectBranch(srcDir string) string { return strings.TrimSpace(string(out)) } -// countUnpushed returns the number of commits ahead of origin. +// defaultBranch detects the default branch of the repo (main, master, etc.). +func defaultBranch(srcDir string) string { + // Try origin/HEAD first + cmd := exec.Command("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short") + cmd.Dir = srcDir + if out, err := cmd.Output(); err == nil { + ref := strings.TrimSpace(string(out)) + // returns "origin/main" — strip prefix + if strings.HasPrefix(ref, "origin/") { + return strings.TrimPrefix(ref, "origin/") + } + return ref + } + // Fallback: check if main exists, else master + for _, branch := range []string{"main", "master"} { + cmd := exec.Command("git", "rev-parse", "--verify", branch) + cmd.Dir = srcDir + if cmd.Run() == nil { + return branch + } + } + return "main" +} + +// countUnpushed returns the number of commits ahead of origin's default branch. func countUnpushed(srcDir, branch string) int { - cmd := exec.Command("git", "rev-list", "--count", "origin/main.."+branch) + base := defaultBranch(srcDir) + cmd := exec.Command("git", "rev-list", "--count", "origin/"+base+".."+branch) cmd.Dir = srcDir out, err := cmd.Output() if err != nil { - // origin/main might not exist — try counting all commits on branch - cmd2 := exec.Command("git", "log", "--oneline", "main.."+branch) + cmd2 := exec.Command("git", "log", "--oneline", base+".."+branch) cmd2.Dir = srcDir out2, err2 := cmd2.Output() if err2 != nil { @@ -176,7 +200,8 @@ func countUnpushed(srcDir, branch string) int { // Fails closed: if git diff fails, rejects the workspace. func checkSafety(srcDir string) string { // Check all changed files — added, modified, renamed - cmd := exec.Command("git", "diff", "--name-only", "main...HEAD") + base := defaultBranch(srcDir) + cmd := exec.Command("git", "diff", "--name-only", base+"...HEAD") cmd.Dir = srcDir out, err := cmd.Output() if err != nil { @@ -213,9 +238,10 @@ func checkSafety(srcDir string) string { return "" } -// countChangedFiles returns the number of files changed vs main. +// countChangedFiles returns the number of files changed vs the default branch. func countChangedFiles(srcDir string) int { - cmd := exec.Command("git", "diff", "--name-only", "main...HEAD") + base := defaultBranch(srcDir) + cmd := exec.Command("git", "diff", "--name-only", base+"...HEAD") cmd.Dir = srcDir out, err := cmd.Output() if err != nil {