refactor: Add CleanupStatus type to replace raw strings (gt-77gq7)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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"))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user