fix(zfc): remove Go-side computation and stderr parsing violations
hq-u0ach: done.go - Add --cleanup-status flag so agents can pass cleanup status directly. Removes computeCleanupStatus() which violated ZFC by having Go compute cleanup status from git state. hq-z0zqw: beads.go - Remove strings.Contains parsing for ErrNotARepo and ErrSyncConflict. Per ZFC, Go should transport errors to agents, not parse them to make decisions. IsBeadsRepo() now uses file existence check. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Steve Yegge
parent
2f50a59e74
commit
fc8718e680
@@ -15,10 +15,10 @@ import (
|
||||
)
|
||||
|
||||
// Common errors
|
||||
// ZFC: Only define errors that don't require stderr parsing for decisions.
|
||||
// ErrNotARepo and ErrSyncConflict were removed - agents should handle these directly.
|
||||
var (
|
||||
ErrNotInstalled = errors.New("bd not installed: run 'pip install beads-cli' or see https://github.com/anthropics/beads")
|
||||
ErrNotARepo = errors.New("not a beads repository (no .beads directory found)")
|
||||
ErrSyncConflict = errors.New("beads sync conflict")
|
||||
ErrNotFound = errors.New("issue not found")
|
||||
)
|
||||
|
||||
@@ -431,6 +431,9 @@ func (b *Beads) Run(args ...string) ([]byte, error) {
|
||||
}
|
||||
|
||||
// wrapError wraps bd errors with context.
|
||||
// ZFC: Avoid parsing stderr to make decisions. Transport errors to agents instead.
|
||||
// Exception: ErrNotInstalled (exec.ErrNotFound) and ErrNotFound (issue lookup) are
|
||||
// acceptable as they enable basic error handling without decision-making.
|
||||
func (b *Beads) wrapError(err error, stderr string, args []string) error {
|
||||
stderr = strings.TrimSpace(stderr)
|
||||
|
||||
@@ -439,15 +442,7 @@ func (b *Beads) wrapError(err error, stderr string, args []string) error {
|
||||
return ErrNotInstalled
|
||||
}
|
||||
|
||||
// Detect specific error types from stderr
|
||||
if strings.Contains(stderr, "not a beads repository") ||
|
||||
strings.Contains(stderr, "No .beads directory") ||
|
||||
strings.Contains(stderr, ".beads") && strings.Contains(stderr, "not found") {
|
||||
return ErrNotARepo
|
||||
}
|
||||
if strings.Contains(stderr, "sync conflict") || strings.Contains(stderr, "CONFLICT") {
|
||||
return ErrSyncConflict
|
||||
}
|
||||
// ErrNotFound is widely used for issue lookups - acceptable exception
|
||||
if strings.Contains(stderr, "not found") || strings.Contains(stderr, "Issue not found") {
|
||||
return ErrNotFound
|
||||
}
|
||||
@@ -1005,9 +1000,11 @@ func (b *Beads) Stats() (string, error) {
|
||||
}
|
||||
|
||||
// IsBeadsRepo checks if the working directory is a beads repository.
|
||||
// ZFC: Check file existence directly instead of parsing bd errors.
|
||||
func (b *Beads) IsBeadsRepo() bool {
|
||||
_, err := b.run("list", "--limit=1")
|
||||
return err == nil || !errors.Is(err, ErrNotARepo)
|
||||
beadsDir := ResolveBeadsDir(b.workDir)
|
||||
info, err := os.Stat(beadsDir)
|
||||
return err == nil && info.IsDir()
|
||||
}
|
||||
|
||||
// AgentFields holds structured fields for agent beads.
|
||||
|
||||
@@ -84,6 +84,8 @@ func TestIsBeadsRepo(t *testing.T) {
|
||||
}
|
||||
|
||||
// TestWrapError tests error wrapping.
|
||||
// ZFC: Only test ErrNotFound detection. ErrNotARepo and ErrSyncConflict
|
||||
// were removed as per ZFC - agents should handle those errors directly.
|
||||
func TestWrapError(t *testing.T) {
|
||||
b := New("/test")
|
||||
|
||||
@@ -92,11 +94,6 @@ func TestWrapError(t *testing.T) {
|
||||
wantErr error
|
||||
wantNil bool
|
||||
}{
|
||||
{"not a beads repository", ErrNotARepo, false},
|
||||
{"No .beads directory found", ErrNotARepo, false},
|
||||
{".beads directory not found", ErrNotARepo, false},
|
||||
{"sync conflict detected", ErrSyncConflict, false},
|
||||
{"CONFLICT in file.md", ErrSyncConflict, false},
|
||||
{"Issue not found: gt-xyz", ErrNotFound, false},
|
||||
{"gt-xyz not found", ErrNotFound, false},
|
||||
}
|
||||
|
||||
@@ -58,6 +58,7 @@ var (
|
||||
doneExit bool
|
||||
donePhaseComplete bool
|
||||
doneGate string
|
||||
doneCleanupStatus string
|
||||
)
|
||||
|
||||
// Valid exit types for gt done
|
||||
@@ -75,6 +76,7 @@ func init() {
|
||||
doneCmd.Flags().BoolVar(&doneExit, "exit", false, "Exit Claude session after MR submission (self-terminate)")
|
||||
doneCmd.Flags().BoolVar(&donePhaseComplete, "phase-complete", false, "Signal phase complete - await gate before continuing")
|
||||
doneCmd.Flags().StringVar(&doneGate, "gate", "", "Gate bead ID to wait on (with --phase-complete)")
|
||||
doneCmd.Flags().StringVar(&doneCleanupStatus, "cleanup-status", "", "Git cleanup status: clean, uncommitted, unpushed, stash, unknown (ZFC: agent-observed)")
|
||||
|
||||
rootCmd.AddCommand(doneCmd)
|
||||
}
|
||||
@@ -428,15 +430,16 @@ func updateAgentStateOnDone(cwd, townRoot, exitType, _ string) { // issueID unus
|
||||
}
|
||||
|
||||
// ZFC #10: Self-report cleanup status
|
||||
// Compute git state and report so Witness can decide removal safety
|
||||
cleanupStatus := computeCleanupStatus(cwd)
|
||||
// Agent observes git state and passes cleanup status via --cleanup-status flag
|
||||
if doneCleanupStatus != "" {
|
||||
cleanupStatus := parseCleanupStatus(doneCleanupStatus)
|
||||
if cleanupStatus != polecat.CleanupUnknown {
|
||||
if err := bd.UpdateAgentCleanupStatus(agentBeadID, string(cleanupStatus)); err != nil {
|
||||
// Log warning instead of silent ignore
|
||||
fmt.Fprintf(os.Stderr, "Warning: couldn't update agent %s cleanup status: %v\n", agentBeadID, err)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// getDispatcherFromBead retrieves the dispatcher agent ID from the bead's attachment fields.
|
||||
@@ -460,25 +463,19 @@ func getDispatcherFromBead(cwd, issueID string) string {
|
||||
return fields.DispatchedBy
|
||||
}
|
||||
|
||||
// computeCleanupStatus checks git state and returns the cleanup status.
|
||||
// Returns the most critical issue: has_unpushed > has_stash > has_uncommitted > clean
|
||||
func computeCleanupStatus(cwd string) polecat.CleanupStatus {
|
||||
g := git.NewGit(cwd)
|
||||
status, err := g.CheckUncommittedWork()
|
||||
if err != nil {
|
||||
// If we can't check, report unknown - Witness should be cautious
|
||||
// parseCleanupStatus converts a string flag value to a CleanupStatus.
|
||||
// ZFC: Agent observes git state and passes the appropriate status.
|
||||
func parseCleanupStatus(s string) polecat.CleanupStatus {
|
||||
switch strings.ToLower(s) {
|
||||
case "clean":
|
||||
return polecat.CleanupClean
|
||||
case "uncommitted", "has_uncommitted":
|
||||
return polecat.CleanupUncommitted
|
||||
case "stash", "has_stash":
|
||||
return polecat.CleanupStash
|
||||
case "unpushed", "has_unpushed":
|
||||
return polecat.CleanupUnpushed
|
||||
default:
|
||||
return polecat.CleanupUnknown
|
||||
}
|
||||
|
||||
// Check in priority order (most critical first)
|
||||
if status.UnpushedCommits > 0 {
|
||||
return polecat.CleanupUnpushed
|
||||
}
|
||||
if status.StashCount > 0 {
|
||||
return polecat.CleanupStash
|
||||
}
|
||||
if status.HasUncommittedChanges {
|
||||
return polecat.CleanupUncommitted
|
||||
}
|
||||
return polecat.CleanupClean
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user