From 904a773adeaca6ea2cebcd2e9e1c31855cceefc1 Mon Sep 17 00:00:00 2001 From: blackfinger Date: Mon, 5 Jan 2026 00:19:00 -0800 Subject: [PATCH] refactor: Add CleanupStatus type to replace raw strings (gt-77gq7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a typed CleanupStatus with constants: - CleanupClean, CleanupUncommitted, CleanupStash, CleanupUnpushed, CleanupUnknown Adds helper methods: - IsSafe(): true for clean status - RequiresRecovery(): true for uncommitted/stash/unpushed - CanForceRemove(): true if force flag can bypass Updated files to use the new type: - internal/polecat/types.go: Type definition and methods - internal/polecat/manager.go: Validation logic - internal/witness/handlers.go: Nuke safety checks - internal/cmd/done.go: Status reporting - internal/cmd/polecat.go: Recovery status checks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/cmd/done.go | 17 ++++++----- internal/cmd/polecat.go | 57 ++++++++++++++++++------------------ internal/polecat/manager.go | 37 ++++++++++++++--------- internal/polecat/types.go | 45 ++++++++++++++++++++++++++++ internal/witness/handlers.go | 33 +++++++++++---------- 5 files changed, 122 insertions(+), 67 deletions(-) diff --git a/internal/cmd/done.go b/internal/cmd/done.go index 8832555a..f7f4185a 100644 --- a/internal/cmd/done.go +++ b/internal/cmd/done.go @@ -11,6 +11,7 @@ import ( "github.com/steveyegge/gastown/internal/events" "github.com/steveyegge/gastown/internal/git" "github.com/steveyegge/gastown/internal/mail" + "github.com/steveyegge/gastown/internal/polecat" "github.com/steveyegge/gastown/internal/rig" "github.com/steveyegge/gastown/internal/style" "github.com/steveyegge/gastown/internal/workspace" @@ -429,8 +430,8 @@ 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 != "" { - if err := bd.UpdateAgentCleanupStatus(agentBeadID, cleanupStatus); err != nil { + 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 @@ -461,23 +462,23 @@ func getDispatcherFromBead(cwd, issueID string) string { // 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) string { +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 - return "unknown" + return polecat.CleanupUnknown } // Check in priority order (most critical first) if status.UnpushedCommits > 0 { - return "has_unpushed" + return polecat.CleanupUnpushed } if status.StashCount > 0 { - return "has_stash" + return polecat.CleanupStash } if status.HasUncommittedChanges { - return "has_uncommitted" + return polecat.CleanupUncommitted } - return "clean" + return polecat.CleanupClean } diff --git a/internal/cmd/polecat.go b/internal/cmd/polecat.go index c5a844df..224923bf 100644 --- a/internal/cmd/polecat.go +++ b/internal/cmd/polecat.go @@ -933,12 +933,12 @@ func getGitState(worktreePath string) (*GitState, error) { // RecoveryStatus represents whether a polecat needs recovery or is safe to nuke. type RecoveryStatus struct { - Rig string `json:"rig"` - Polecat string `json:"polecat"` - CleanupStatus string `json:"cleanup_status"` - NeedsRecovery bool `json:"needs_recovery"` - Verdict string `json:"verdict"` // SAFE_TO_NUKE or NEEDS_RECOVERY - Branch string `json:"branch,omitempty"` + Rig string `json:"rig"` + Polecat string `json:"polecat"` + CleanupStatus polecat.CleanupStatus `json:"cleanup_status"` + NeedsRecovery bool `json:"needs_recovery"` + Verdict string `json:"verdict"` // SAFE_TO_NUKE or NEEDS_RECOVERY + Branch string `json:"branch,omitempty"` Issue string `json:"issue,omitempty"` } @@ -978,38 +978,35 @@ func runPolecatCheckRecovery(cmd *cobra.Command, args []string) error { // This handles polecats that haven't self-reported yet gitState, gitErr := getGitState(p.ClonePath) if gitErr != nil { - status.CleanupStatus = "unknown" + status.CleanupStatus = polecat.CleanupUnknown status.NeedsRecovery = true status.Verdict = "NEEDS_RECOVERY" } else if gitState.Clean { - status.CleanupStatus = "clean" + status.CleanupStatus = polecat.CleanupClean status.NeedsRecovery = false status.Verdict = "SAFE_TO_NUKE" } else if gitState.UnpushedCommits > 0 { - status.CleanupStatus = "has_unpushed" + status.CleanupStatus = polecat.CleanupUnpushed status.NeedsRecovery = true status.Verdict = "NEEDS_RECOVERY" } else if gitState.StashCount > 0 { - status.CleanupStatus = "has_stash" + status.CleanupStatus = polecat.CleanupStash status.NeedsRecovery = true status.Verdict = "NEEDS_RECOVERY" } else { - status.CleanupStatus = "has_uncommitted" + status.CleanupStatus = polecat.CleanupUncommitted status.NeedsRecovery = true status.Verdict = "NEEDS_RECOVERY" } } else { // Use cleanup_status from agent bead - status.CleanupStatus = fields.CleanupStatus - switch fields.CleanupStatus { - case "clean": + status.CleanupStatus = polecat.CleanupStatus(fields.CleanupStatus) + if status.CleanupStatus.IsSafe() { status.NeedsRecovery = false status.Verdict = "SAFE_TO_NUKE" - case "has_uncommitted", "has_unpushed", "has_stash": - status.NeedsRecovery = true - status.Verdict = "NEEDS_RECOVERY" - default: - // Unknown or empty - be conservative + } else { + // RequiresRecovery covers uncommitted, stash, unpushed + // Unknown/empty also treated conservatively status.NeedsRecovery = true status.Verdict = "NEEDS_RECOVERY" } @@ -1238,19 +1235,20 @@ func runPolecatNuke(cmd *cobra.Command, args []string) error { } } else { // Check cleanup_status from agent bead - switch fields.CleanupStatus { - case "clean": + cleanupStatus := polecat.CleanupStatus(fields.CleanupStatus) + switch cleanupStatus { + case polecat.CleanupClean: // OK - case "has_unpushed": + case polecat.CleanupUnpushed: reasons = append(reasons, "has unpushed commits") - case "has_uncommitted": + case polecat.CleanupUncommitted: reasons = append(reasons, "has uncommitted changes") - case "has_stash": + case polecat.CleanupStash: reasons = append(reasons, "has stashed changes") - case "unknown", "": + case polecat.CleanupUnknown, "": reasons = append(reasons, "cleanup status unknown") default: - reasons = append(reasons, fmt.Sprintf("cleanup status: %s", fields.CleanupStatus)) + reasons = append(reasons, fmt.Sprintf("cleanup status: %s", cleanupStatus)) } // Check 3: Work on hook (check both Issue.HookBead from slot and fields.HookBead) @@ -1337,10 +1335,11 @@ func runPolecatNuke(cmd *cobra.Command, args []string) error { } fmt.Printf(" - Hook: %s\n", style.Dim.Render("unknown (no agent bead)")) } else { - if fields.CleanupStatus == "clean" { + cleanupStatus := polecat.CleanupStatus(fields.CleanupStatus) + if cleanupStatus.IsSafe() { fmt.Printf(" - Git state: %s\n", style.Success.Render("clean")) - } else if fields.CleanupStatus != "" { - fmt.Printf(" - Git state: %s (%s)\n", style.Error.Render("dirty"), fields.CleanupStatus) + } else if cleanupStatus.RequiresRecovery() { + fmt.Printf(" - Git state: %s (%s)\n", style.Error.Render("dirty"), cleanupStatus) } else { fmt.Printf(" - Git state: %s\n", style.Warning.Render("unknown")) } diff --git a/internal/polecat/manager.go b/internal/polecat/manager.go index b8e63e15..80bb8008 100644 --- a/internal/polecat/manager.go +++ b/internal/polecat/manager.go @@ -103,39 +103,48 @@ func (m *Manager) agentBeadID(name string) string { } // getCleanupStatusFromBead reads the cleanup_status from the polecat's agent bead. -// Returns empty string if the bead doesn't exist or has no cleanup_status. +// Returns CleanupUnknown if the bead doesn't exist or has no cleanup_status. // ZFC #10: This is the ZFC-compliant way to check if removal is safe. -func (m *Manager) getCleanupStatusFromBead(name string) string { +func (m *Manager) getCleanupStatusFromBead(name string) CleanupStatus { agentID := m.agentBeadID(name) _, fields, err := m.beads.GetAgentBead(agentID) if err != nil || fields == nil { - return "" + return CleanupUnknown } - return fields.CleanupStatus + if fields.CleanupStatus == "" { + return CleanupUnknown + } + return CleanupStatus(fields.CleanupStatus) } // checkCleanupStatus validates the cleanup status against removal safety rules. // Returns an error if removal should be blocked based on the status. // force=true: allow has_uncommitted, block has_stash and has_unpushed // force=false: block all non-clean statuses -func (m *Manager) checkCleanupStatus(name, cleanupStatus string, force bool) error { - switch cleanupStatus { - case "clean": +func (m *Manager) checkCleanupStatus(name string, status CleanupStatus, force bool) error { + // Clean status is always safe + if status.IsSafe() { return nil - case "has_uncommitted": - if force { - return nil // force bypasses uncommitted changes - } + } + + // With force, uncommitted changes can be bypassed + if force && status.CanForceRemove() { + return nil + } + + // Map status to appropriate error + switch status { + case CleanupUncommitted: return &UncommittedWorkError{ PolecatName: name, Status: &git.UncommittedWorkStatus{HasUncommittedChanges: true}, } - case "has_stash": + case CleanupStash: return &UncommittedWorkError{ PolecatName: name, Status: &git.UncommittedWorkStatus{StashCount: 1}, } - case "has_unpushed": + case CleanupUnpushed: return &UncommittedWorkError{ PolecatName: name, Status: &git.UncommittedWorkStatus{UnpushedCommits: 1}, @@ -301,7 +310,7 @@ func (m *Manager) RemoveWithOptions(name string, force, nuclear bool) error { // This is the ZFC-compliant path - trust what the polecat reported cleanupStatus := m.getCleanupStatusFromBead(name) - if cleanupStatus != "" && cleanupStatus != "unknown" { + if cleanupStatus != CleanupUnknown { // ZFC path: Use polecat's self-reported status if err := m.checkCleanupStatus(name, cleanupStatus, force); err != nil { return err diff --git a/internal/polecat/types.go b/internal/polecat/types.go index cf071f38..48bfc709 100644 --- a/internal/polecat/types.go +++ b/internal/polecat/types.go @@ -78,3 +78,48 @@ func (p *Polecat) Summary() Summary { Issue: p.Issue, } } + +// CleanupStatus represents the git state of a polecat for cleanup decisions. +// The Witness uses this to determine whether it's safe to nuke a polecat worktree. +type CleanupStatus string + +const ( + // CleanupClean means the worktree has no uncommitted work and is safe to remove. + CleanupClean CleanupStatus = "clean" + + // CleanupUncommitted means there are uncommitted changes in the worktree. + CleanupUncommitted CleanupStatus = "has_uncommitted" + + // CleanupStash means there are stashed changes that would be lost. + CleanupStash CleanupStatus = "has_stash" + + // CleanupUnpushed means there are commits not pushed to the remote. + CleanupUnpushed CleanupStatus = "has_unpushed" + + // CleanupUnknown means the status could not be determined. + CleanupUnknown CleanupStatus = "unknown" +) + +// IsSafe returns true if the status indicates it's safe to remove the worktree +// without losing any work. +func (s CleanupStatus) IsSafe() bool { + return s == CleanupClean +} + +// RequiresRecovery returns true if the status indicates there is work that +// needs to be recovered before removal. This includes uncommitted changes, +// stashes, and unpushed commits. +func (s CleanupStatus) RequiresRecovery() bool { + switch s { + case CleanupUncommitted, CleanupStash, CleanupUnpushed: + return true + default: + return false + } +} + +// CanForceRemove returns true if the status allows forced removal. +// Uncommitted changes can be force-removed, but stashes and unpushed commits cannot. +func (s CleanupStatus) CanForceRemove() bool { + return s == CleanupClean || s == CleanupUncommitted +} diff --git a/internal/witness/handlers.go b/internal/witness/handlers.go index b5dcf41a..9f6d7101 100644 --- a/internal/witness/handlers.go +++ b/internal/witness/handlers.go @@ -12,6 +12,7 @@ import ( "github.com/steveyegge/gastown/internal/beads" "github.com/steveyegge/gastown/internal/git" "github.com/steveyegge/gastown/internal/mail" + "github.com/steveyegge/gastown/internal/polecat" "github.com/steveyegge/gastown/internal/rig" "github.com/steveyegge/gastown/internal/tmux" "github.com/steveyegge/gastown/internal/workspace" @@ -242,7 +243,7 @@ func HandleMerged(workDir, rigName string, msg *mail.Message) *HandlerResult { cleanupStatus := getCleanupStatus(workDir, rigName, payload.PolecatName) switch cleanupStatus { - case "clean": + case polecat.CleanupClean: // Safe to nuke - polecat has confirmed clean state // Execute the nuke immediately if err := NukePolecat(workDir, rigName, payload.PolecatName); err != nil { @@ -256,21 +257,21 @@ func HandleMerged(workDir, rigName string, msg *mail.Message) *HandlerResult { result.Action = fmt.Sprintf("auto-nuked %s (cleanup_status=clean, wisp=%s)", payload.PolecatName, wispID) } - case "has_uncommitted": + case polecat.CleanupUncommitted: // Has uncommitted changes - might be WIP, escalate to Mayor result.Handled = true result.WispCreated = wispID result.Error = fmt.Errorf("polecat %s has uncommitted changes - escalate to Mayor before nuke", payload.PolecatName) result.Action = fmt.Sprintf("BLOCKED: %s has uncommitted work, needs escalation", payload.PolecatName) - case "has_stash": + case polecat.CleanupStash: // Has stashed work - definitely needs review result.Handled = true result.WispCreated = wispID result.Error = fmt.Errorf("polecat %s has stashed work - escalate to Mayor before nuke", payload.PolecatName) result.Action = fmt.Sprintf("BLOCKED: %s has stashed work, needs escalation", payload.PolecatName) - case "has_unpushed": + case polecat.CleanupUnpushed: // Critical: has unpushed commits that could be lost result.Handled = true result.WispCreated = wispID @@ -461,12 +462,12 @@ type agentBeadResponse struct { } // getCleanupStatus retrieves the cleanup_status from a polecat's agent bead. -// Returns the status string: "clean", "has_uncommitted", "has_stash", "has_unpushed" -// Returns empty string if agent bead doesn't exist or has no cleanup_status. +// Returns the typed CleanupStatus (clean, has_uncommitted, has_stash, has_unpushed). +// Returns CleanupUnknown if agent bead doesn't exist or has no cleanup_status. // // ZFC #10: This enables the Witness to verify it's safe to nuke before proceeding. // The polecat self-reports its git state when running `gt done`, and we trust that report. -func getCleanupStatus(workDir, rigName, polecatName string) string { +func getCleanupStatus(workDir, rigName, polecatName string) polecat.CleanupStatus { // Construct agent bead ID using the rig's configured prefix // This supports non-gt prefixes like "bd-" for the beads rig townRoot, err := workspace.Find(workDir) @@ -485,19 +486,19 @@ func getCleanupStatus(workDir, rigName, polecatName string) string { cmd.Stderr = &stderr if err := cmd.Run(); err != nil { - // Agent bead doesn't exist or bd failed - return empty (unknown status) - return "" + // Agent bead doesn't exist or bd failed - return unknown status + return polecat.CleanupUnknown } output := stdout.Bytes() if len(output) == 0 { - return "" + return polecat.CleanupUnknown } // Parse the JSON response var resp agentBeadResponse if err := json.Unmarshal(output, &resp); err != nil { - return "" + return polecat.CleanupUnknown } // Parse cleanup_status from description @@ -508,12 +509,12 @@ func getCleanupStatus(workDir, rigName, polecatName string) string { value := strings.TrimSpace(strings.TrimPrefix(line, "cleanup_status:")) value = strings.TrimSpace(strings.TrimPrefix(value, "Cleanup_status:")) if value != "" && value != "null" { - return value + return polecat.CleanupStatus(value) } } } - return "" + return polecat.CleanupUnknown } // escalateToMayor sends an escalation mail to the Mayor. @@ -706,7 +707,7 @@ func AutoNukeIfClean(workDir, rigName, polecatName string) *NukePolecatResult { cleanupStatus := getCleanupStatus(workDir, rigName, polecatName) switch cleanupStatus { - case "clean": + case polecat.CleanupClean: // Safe to nuke if err := NukePolecat(workDir, rigName, polecatName); err != nil { result.Error = err @@ -716,10 +717,10 @@ func AutoNukeIfClean(workDir, rigName, polecatName string) *NukePolecatResult { result.Reason = "auto-nuked (cleanup_status=clean, no MR)" } - case "has_uncommitted", "has_stash", "has_unpushed": + case polecat.CleanupUncommitted, polecat.CleanupStash, polecat.CleanupUnpushed: // Not safe - has work that could be lost result.Skipped = true - result.Reason = fmt.Sprintf("skipped: has %s", strings.TrimPrefix(cleanupStatus, "has_")) + result.Reason = fmt.Sprintf("skipped: has %s", strings.TrimPrefix(string(cleanupStatus), "has_")) default: // Unknown status - check git state directly as fallback