fix: address Codex round 2 findings — 3 verified highs
Some checks failed
CI / test (push) Failing after 3s

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 <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-21 15:59:48 +00:00
parent 422777580b
commit 98ce071b13
3 changed files with 44 additions and 9 deletions

View file

@ -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/

View file

@ -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"}
}

View file

@ -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 {