fix(zfc): remove strings.Contains conflict detection from Go code
hq-hcil1: Remove deprecated HasConflict/HasAuthFailure/IsNotARepo/HasRebaseConflict methods that violated ZFC by having Go code decide error types based on stderr parsing. Changes: - Remove deprecated helper methods from GitError and SwarmGitError - Export GetConflictingFiles() which uses git porcelain output (diff --diff-filter=U) - Update CheckConflicts(), engineer.go, and integration.go to use GetConflictingFiles() - Update tests to verify raw stderr is available for agent observation ZFC principle: Go code transports raw output to agents; agents observe and decide. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Steve Yegge
parent
131dac91c8
commit
f7d497ba07
@@ -3,7 +3,6 @@ package git
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
@@ -34,47 +33,6 @@ func (e *GitError) Unwrap() error {
|
|||||||
return e.Err
|
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.
|
// Git wraps git operations for a working directory.
|
||||||
type Git struct {
|
type Git struct {
|
||||||
workDir string
|
workDir string
|
||||||
@@ -470,21 +428,16 @@ func (g *Git) CheckConflicts(source, target string) ([]string, error) {
|
|||||||
_, mergeErr := g.runMergeCheck("merge", "--no-commit", "--no-ff", source)
|
_, mergeErr := g.runMergeCheck("merge", "--no-commit", "--no-ff", source)
|
||||||
|
|
||||||
if mergeErr != nil {
|
if mergeErr != nil {
|
||||||
// Check if there are unmerged files (indicates conflict)
|
// ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr.
|
||||||
conflicts, err := g.getConflictingFiles()
|
// GetConflictingFiles() uses `git diff --diff-filter=U` which is the proper way.
|
||||||
|
conflicts, err := g.GetConflictingFiles()
|
||||||
if err == nil && len(conflicts) > 0 {
|
if err == nil && len(conflicts) > 0 {
|
||||||
// Abort the test merge (best-effort cleanup)
|
// Abort the test merge (best-effort cleanup)
|
||||||
_ = g.AbortMerge()
|
_ = g.AbortMerge()
|
||||||
return conflicts, nil
|
return conflicts, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// ZFC: Check if the error output indicates a conflict
|
// No unmerged files detected - this is some other merge error
|
||||||
if gitErr, ok := mergeErr.(*GitError); ok && gitErr.HasConflict() {
|
|
||||||
_ = g.AbortMerge() // best-effort cleanup
|
|
||||||
return conflicts, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// Some other merge error (best-effort cleanup)
|
|
||||||
_ = g.AbortMerge()
|
_ = g.AbortMerge()
|
||||||
return nil, mergeErr
|
return nil, mergeErr
|
||||||
}
|
}
|
||||||
@@ -514,8 +467,10 @@ func (g *Git) runMergeCheck(args ...string) (string, error) {
|
|||||||
return strings.TrimSpace(stdout.String()), nil
|
return strings.TrimSpace(stdout.String()), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// getConflictingFiles returns the list of files with merge conflicts.
|
// GetConflictingFiles returns the list of files with merge conflicts.
|
||||||
func (g *Git) getConflictingFiles() ([]string, error) {
|
// 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
|
// git diff --name-only --diff-filter=U shows unmerged files
|
||||||
out, err := g.run("diff", "--name-only", "--diff-filter=U")
|
out, err := g.run("diff", "--name-only", "--diff-filter=U")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -214,10 +214,16 @@ func TestNotARepo(t *testing.T) {
|
|||||||
g := NewGit(dir)
|
g := NewGit(dir)
|
||||||
|
|
||||||
_, err := g.CurrentBranch()
|
_, 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)
|
gitErr, ok := err.(*GitError)
|
||||||
if !ok || !gitErr.IsNotARepo() {
|
if !ok {
|
||||||
t.Errorf("expected GitError with IsNotARepo(), got %v", err)
|
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")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
_, _ = fmt.Fprintf(e.output, "[Engineer] Merging with message: %s\n", mergeMsg)
|
||||||
if err := e.git.MergeNoFF(branch, mergeMsg); err != nil {
|
if err := e.git.MergeNoFF(branch, mergeMsg); err != nil {
|
||||||
// ZFC: Check for conflict via GitError method instead of sentinel error
|
// ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr.
|
||||||
if gitErr, ok := err.(*git.GitError); ok && gitErr.HasConflict() {
|
// GetConflictingFiles() uses `git diff --diff-filter=U` which is proper.
|
||||||
|
conflicts, conflictErr := e.git.GetConflictingFiles()
|
||||||
|
if conflictErr == nil && len(conflicts) > 0 {
|
||||||
_ = e.git.AbortMerge()
|
_ = e.git.AbortMerge()
|
||||||
return ProcessResult{
|
return ProcessResult{
|
||||||
Success: false,
|
Success: false,
|
||||||
|
|||||||
@@ -31,14 +31,6 @@ func (e *SwarmGitError) Error() string {
|
|||||||
return fmt.Sprintf("%s: %v", e.Command, e.Err)
|
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.
|
// CreateIntegrationBranch creates the integration branch for a swarm.
|
||||||
// The branch is created from the swarm's BaseCommit and pushed to origin.
|
// The branch is created from the swarm's BaseCommit and pushed to origin.
|
||||||
func (m *Manager) CreateIntegrationBranch(swarmID string) error {
|
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),
|
fmt.Sprintf("Merge %s into %s", workerBranch, swarm.Integration),
|
||||||
workerBranch)
|
workerBranch)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// ZFC: Check for conflict via SwarmGitError method
|
// ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr.
|
||||||
if gitErr, ok := err.(*SwarmGitError); ok && gitErr.HasConflict() {
|
conflicts, conflictErr := m.getConflictingFiles()
|
||||||
return gitErr // Return the error with raw output for observation
|
if conflictErr == nil && len(conflicts) > 0 {
|
||||||
|
// Return the original error with raw output for observation
|
||||||
|
return err
|
||||||
}
|
}
|
||||||
return fmt.Errorf("merging: %w", err)
|
return fmt.Errorf("merging: %w", err)
|
||||||
}
|
}
|
||||||
@@ -127,9 +121,11 @@ func (m *Manager) LandToMain(swarmID string) error {
|
|||||||
fmt.Sprintf("Land swarm %s", swarmID),
|
fmt.Sprintf("Land swarm %s", swarmID),
|
||||||
swarm.Integration)
|
swarm.Integration)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// ZFC: Check for conflict via SwarmGitError method
|
// ZFC: Use git's porcelain output to detect conflicts instead of parsing stderr.
|
||||||
if gitErr, ok := err.(*SwarmGitError); ok && gitErr.HasConflict() {
|
conflicts, conflictErr := m.getConflictingFiles()
|
||||||
return gitErr // Return the error with raw output for observation
|
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)
|
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
|
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.
|
// gitRun executes a git command.
|
||||||
// ZFC: Returns SwarmGitError with raw output for agent observation.
|
// ZFC: Returns SwarmGitError with raw output for agent observation.
|
||||||
func (m *Manager) gitRun(args ...string) error {
|
func (m *Manager) gitRun(args ...string) error {
|
||||||
|
|||||||
Reference in New Issue
Block a user