diff --git a/internal/git/git.go b/internal/git/git.go index d8f06d0a..ead1fc1d 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -3,7 +3,6 @@ package git import ( "bytes" - "errors" "fmt" "os" "os/exec" @@ -34,47 +33,6 @@ func (e *GitError) Unwrap() error { return e.Err } -// HasConflict returns true if the error output indicates a merge conflict. -// Deprecated: This exists for backwards compatibility. Agents should observe -// Stderr directly and make their own decisions (ZFC principle). -func (e *GitError) HasConflict() bool { - return strings.Contains(e.Stderr, "CONFLICT") || - strings.Contains(e.Stderr, "Merge conflict") || - strings.Contains(e.Stdout, "CONFLICT") -} - -// HasAuthFailure returns true if the error output indicates authentication failure. -// Deprecated: This exists for backwards compatibility. Agents should observe -// Stderr directly and make their own decisions (ZFC principle). -func (e *GitError) HasAuthFailure() bool { - return strings.Contains(e.Stderr, "Authentication failed") || - strings.Contains(e.Stderr, "could not read Username") -} - -// IsNotARepo returns true if the error indicates the path is not a git repository. -// Deprecated: This exists for backwards compatibility. Agents should observe -// Stderr directly and make their own decisions (ZFC principle). -func (e *GitError) IsNotARepo() bool { - return strings.Contains(e.Stderr, "not a git repository") -} - -// HasRebaseConflict returns true if the error indicates a rebase conflict. -// Deprecated: This exists for backwards compatibility. Agents should observe -// Stderr directly and make their own decisions (ZFC principle). -func (e *GitError) HasRebaseConflict() bool { - return strings.Contains(e.Stderr, "needs merge") || - strings.Contains(e.Stderr, "rebase in progress") -} - -// Common errors - deprecated, kept for backwards compatibility. -// ZFC: These should not be used; observe GitError.Stderr instead. -var ( - ErrNotARepo = errors.New("not a git repository") - ErrMergeConflict = errors.New("merge conflict") - ErrAuthFailure = errors.New("authentication failed") - ErrRebaseConflict = errors.New("rebase conflict") -) - // Git wraps git operations for a working directory. type Git struct { workDir string @@ -470,21 +428,16 @@ func (g *Git) CheckConflicts(source, target string) ([]string, error) { _, mergeErr := g.runMergeCheck("merge", "--no-commit", "--no-ff", source) if mergeErr != nil { - // Check if there are unmerged files (indicates conflict) - conflicts, err := g.getConflictingFiles() + // ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr. + // GetConflictingFiles() uses `git diff --diff-filter=U` which is the proper way. + conflicts, err := g.GetConflictingFiles() if err == nil && len(conflicts) > 0 { // Abort the test merge (best-effort cleanup) _ = g.AbortMerge() return conflicts, nil } - // ZFC: Check if the error output indicates a conflict - if gitErr, ok := mergeErr.(*GitError); ok && gitErr.HasConflict() { - _ = g.AbortMerge() // best-effort cleanup - return conflicts, nil - } - - // Some other merge error (best-effort cleanup) + // No unmerged files detected - this is some other merge error _ = g.AbortMerge() return nil, mergeErr } @@ -514,8 +467,10 @@ func (g *Git) runMergeCheck(args ...string) (string, error) { return strings.TrimSpace(stdout.String()), nil } -// getConflictingFiles returns the list of files with merge conflicts. -func (g *Git) getConflictingFiles() ([]string, error) { +// GetConflictingFiles returns the list of files with merge conflicts. +// ZFC: Uses git's porcelain output (diff --diff-filter=U) instead of parsing stderr. +// This is the proper way to detect conflicts without violating ZFC. +func (g *Git) GetConflictingFiles() ([]string, error) { // git diff --name-only --diff-filter=U shows unmerged files out, err := g.run("diff", "--name-only", "--diff-filter=U") if err != nil { diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 25fd3f8c..d7d4bacd 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -214,10 +214,16 @@ func TestNotARepo(t *testing.T) { g := NewGit(dir) _, err := g.CurrentBranch() - // ZFC: Check for not-a-repo via GitError method instead of sentinel error + // ZFC: Check for GitError with raw stderr for agent observation. + // Agents decide what "not a git repository" means, not Go code. gitErr, ok := err.(*GitError) - if !ok || !gitErr.IsNotARepo() { - t.Errorf("expected GitError with IsNotARepo(), got %v", err) + if !ok { + t.Errorf("expected GitError, got %T: %v", err, err) + return + } + // Verify raw stderr is available for agent observation + if gitErr.Stderr == "" { + t.Errorf("expected GitError with Stderr, got empty stderr") } } diff --git a/internal/refinery/engineer.go b/internal/refinery/engineer.go index 15a96199..af065d45 100644 --- a/internal/refinery/engineer.go +++ b/internal/refinery/engineer.go @@ -311,8 +311,10 @@ func (e *Engineer) doMerge(ctx context.Context, branch, target, sourceIssue stri } _, _ = fmt.Fprintf(e.output, "[Engineer] Merging with message: %s\n", mergeMsg) if err := e.git.MergeNoFF(branch, mergeMsg); err != nil { - // ZFC: Check for conflict via GitError method instead of sentinel error - if gitErr, ok := err.(*git.GitError); ok && gitErr.HasConflict() { + // ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr. + // GetConflictingFiles() uses `git diff --diff-filter=U` which is proper. + conflicts, conflictErr := e.git.GetConflictingFiles() + if conflictErr == nil && len(conflicts) > 0 { _ = e.git.AbortMerge() return ProcessResult{ Success: false, diff --git a/internal/swarm/integration.go b/internal/swarm/integration.go index 8c27ad69..c18f6f59 100644 --- a/internal/swarm/integration.go +++ b/internal/swarm/integration.go @@ -31,14 +31,6 @@ func (e *SwarmGitError) Error() string { return fmt.Sprintf("%s: %v", e.Command, e.Err) } -// HasConflict returns true if the error output indicates a merge conflict. -// Deprecated: Agents should observe Stderr directly (ZFC principle). -func (e *SwarmGitError) HasConflict() bool { - return strings.Contains(e.Stderr, "CONFLICT") || - strings.Contains(e.Stderr, "Merge conflict") || - strings.Contains(e.Stdout, "CONFLICT") -} - // CreateIntegrationBranch creates the integration branch for a swarm. // The branch is created from the swarm's BaseCommit and pushed to origin. func (m *Manager) CreateIntegrationBranch(swarmID string) error { @@ -92,9 +84,11 @@ func (m *Manager) MergeToIntegration(swarmID, workerBranch string) error { fmt.Sprintf("Merge %s into %s", workerBranch, swarm.Integration), workerBranch) if err != nil { - // ZFC: Check for conflict via SwarmGitError method - if gitErr, ok := err.(*SwarmGitError); ok && gitErr.HasConflict() { - return gitErr // Return the error with raw output for observation + // ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr. + conflicts, conflictErr := m.getConflictingFiles() + if conflictErr == nil && len(conflicts) > 0 { + // Return the original error with raw output for observation + return err } return fmt.Errorf("merging: %w", err) } @@ -127,9 +121,11 @@ func (m *Manager) LandToMain(swarmID string) error { fmt.Sprintf("Land swarm %s", swarmID), swarm.Integration) if err != nil { - // ZFC: Check for conflict via SwarmGitError method - if gitErr, ok := err.(*SwarmGitError); ok && gitErr.HasConflict() { - return gitErr // Return the error with raw output for observation + // ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr. + conflicts, conflictErr := m.getConflictingFiles() + if conflictErr == nil && len(conflicts) > 0 { + // Return the original error with raw output for observation + return err } return fmt.Errorf("merging to %s: %w", swarm.TargetBranch, err) } @@ -207,6 +203,34 @@ func (m *Manager) getCurrentBranch() (string, error) { return strings.TrimSpace(stdout.String()), nil } +// getConflictingFiles returns the list of files with merge conflicts. +// ZFC: Uses git's porcelain output (diff --diff-filter=U) instead of parsing stderr. +func (m *Manager) getConflictingFiles() ([]string, error) { + cmd := exec.Command("git", "diff", "--name-only", "--diff-filter=U") + cmd.Dir = m.gitDir + + var stdout bytes.Buffer + cmd.Stdout = &stdout + + if err := cmd.Run(); err != nil { + return nil, err + } + + out := strings.TrimSpace(stdout.String()) + if out == "" { + return nil, nil + } + + files := strings.Split(out, "\n") + var result []string + for _, f := range files { + if f != "" { + result = append(result, f) + } + } + return result, nil +} + // gitRun executes a git command. // ZFC: Returns SwarmGitError with raw output for agent observation. func (m *Manager) gitRun(args ...string) error {