From fc8718e6806c6b68870d45167634c2d4da048650 Mon Sep 17 00:00:00 2001 From: gastown/crew/joe Date: Fri, 9 Jan 2026 22:10:41 -0800 Subject: [PATCH] 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 --- internal/beads/beads.go | 23 ++++++++--------- internal/beads/beads_test.go | 7 ++---- internal/cmd/done.go | 49 +++++++++++++++++------------------- 3 files changed, 35 insertions(+), 44 deletions(-) diff --git a/internal/beads/beads.go b/internal/beads/beads.go index 6252d55c..be329c6b 100644 --- a/internal/beads/beads.go +++ b/internal/beads/beads.go @@ -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. diff --git a/internal/beads/beads_test.go b/internal/beads/beads_test.go index e707c3f0..3d581cd1 100644 --- a/internal/beads/beads_test.go +++ b/internal/beads/beads_test.go @@ -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}, } diff --git a/internal/cmd/done.go b/internal/cmd/done.go index e06849a9..cd55d1e2 100644 --- a/internal/cmd/done.go +++ b/internal/cmd/done.go @@ -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,13 +430,14 @@ 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) - 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 + // 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 { + fmt.Fprintf(os.Stderr, "Warning: couldn't update agent %s cleanup status: %v\n", agentBeadID, err) + return + } } } } @@ -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 }