Merge pull request #452 from julianknutsen/fix/close-delete-hard-bug-workaround
fix(beads): use close instead of delete for agent bead lifecycle
This commit is contained in:
Steve Yegge
GitHub
@@ -178,9 +178,14 @@ func (b *Beads) CreateAgentBead(id, title string, fields *AgentFields) (*Issue,
|
||||
|
||||
// CreateOrReopenAgentBead creates an agent bead or reopens an existing one.
|
||||
// This handles the case where a polecat is nuked and re-spawned with the same name:
|
||||
// the old agent bead exists as a tombstone, so we reopen and update it instead of
|
||||
// the old agent bead exists as a closed bead, so we reopen and update it instead of
|
||||
// failing with a UNIQUE constraint error.
|
||||
//
|
||||
// NOTE: This does NOT handle tombstones. If the old bead was hard-deleted (creating
|
||||
// a tombstone), this function will fail. Use CloseAndClearAgentBead instead of DeleteAgentBead
|
||||
// when cleaning up agent beads to ensure they can be reopened later.
|
||||
//
|
||||
//
|
||||
// The function:
|
||||
// 1. Tries to create the agent bead
|
||||
// 2. If UNIQUE constraint fails, reopens the existing bead and updates its fields
|
||||
@@ -196,7 +201,7 @@ func (b *Beads) CreateOrReopenAgentBead(id, title string, fields *AgentFields) (
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// The bead already exists (likely a tombstone from a previous nuked polecat)
|
||||
// The bead already exists (should be closed from previous polecat lifecycle)
|
||||
// Reopen it and update its fields
|
||||
if _, reopenErr := b.run("reopen", id, "--reason=re-spawning agent"); reopenErr != nil {
|
||||
// If reopen fails, the bead might already be open - continue with update
|
||||
@@ -400,11 +405,70 @@ func (b *Beads) GetAgentNotificationLevel(id string) (string, error) {
|
||||
|
||||
// DeleteAgentBead permanently deletes an agent bead.
|
||||
// Uses --hard --force for immediate permanent deletion (no tombstone).
|
||||
//
|
||||
// WARNING: Due to a bd bug, --hard --force still creates tombstones instead of
|
||||
// truly deleting. This breaks CreateOrReopenAgentBead because tombstones are
|
||||
// invisible to bd show/reopen but still block bd create via UNIQUE constraint.
|
||||
//
|
||||
//
|
||||
// WORKAROUND: Use CloseAndClearAgentBead instead, which allows CreateOrReopenAgentBead
|
||||
// to reopen the bead on re-spawn.
|
||||
func (b *Beads) DeleteAgentBead(id string) error {
|
||||
_, err := b.run("delete", id, "--hard", "--force")
|
||||
return err
|
||||
}
|
||||
|
||||
// CloseAndClearAgentBead closes an agent bead (soft delete).
|
||||
// This is the recommended way to clean up agent beads because CreateOrReopenAgentBead
|
||||
// can reopen closed beads when re-spawning polecats with the same name.
|
||||
//
|
||||
// This is a workaround for the bd tombstone bug where DeleteAgentBead creates
|
||||
// tombstones that cannot be reopened.
|
||||
//
|
||||
// To emulate the clean slate of delete --force --hard, this clears all mutable
|
||||
// fields (hook_bead, active_mr, cleanup_status, agent_state) before closing.
|
||||
func (b *Beads) CloseAndClearAgentBead(id, reason string) error {
|
||||
// Clear mutable fields to emulate delete --force --hard behavior.
|
||||
// This ensures reopened agent beads don't have stale state.
|
||||
|
||||
// First get current issue to preserve immutable fields
|
||||
issue, err := b.Show(id)
|
||||
if err != nil {
|
||||
// If we can't read the issue, still attempt to close
|
||||
args := []string{"close", id}
|
||||
if reason != "" {
|
||||
args = append(args, "--reason="+reason)
|
||||
}
|
||||
_, closeErr := b.run(args...)
|
||||
return closeErr
|
||||
}
|
||||
|
||||
// Parse existing fields and clear mutable ones
|
||||
fields := ParseAgentFields(issue.Description)
|
||||
fields.HookBead = "" // Clear hook_bead
|
||||
fields.ActiveMR = "" // Clear active_mr
|
||||
fields.CleanupStatus = "" // Clear cleanup_status
|
||||
fields.AgentState = "closed"
|
||||
|
||||
// Update description with cleared fields
|
||||
description := FormatAgentDescription(issue.Title, fields)
|
||||
if err := b.Update(id, UpdateOptions{Description: &description}); err != nil {
|
||||
// Non-fatal: continue with close even if update fails
|
||||
}
|
||||
|
||||
// Also clear the hook slot in the database
|
||||
if err := b.ClearHookBead(id); err != nil {
|
||||
// Non-fatal
|
||||
}
|
||||
|
||||
args := []string{"close", id}
|
||||
if reason != "" {
|
||||
args = append(args, "--reason="+reason)
|
||||
}
|
||||
_, err = b.run(args...)
|
||||
return err
|
||||
}
|
||||
|
||||
// GetAgentBead retrieves an agent bead by ID.
|
||||
// Returns nil if not found.
|
||||
func (b *Beads) GetAgentBead(id string) (*Issue, *AgentFields, error) {
|
||||
|
||||
@@ -2,6 +2,7 @@ package beads
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -1799,3 +1800,603 @@ func TestSetupRedirect(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestAgentBeadTombstoneBug demonstrates the bd bug where `bd delete --hard --force`
|
||||
// creates tombstones instead of truly deleting records.
|
||||
//
|
||||
//
|
||||
// This test documents the bug behavior:
|
||||
// 1. Create agent bead
|
||||
// 2. Delete with --hard --force (supposed to permanently delete)
|
||||
// 3. BUG: Tombstone is created instead
|
||||
// 4. BUG: bd create fails with UNIQUE constraint
|
||||
// 5. BUG: bd reopen fails with "issue not found" (tombstones are invisible)
|
||||
func TestAgentBeadTombstoneBug(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Initialize beads database
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
agentID := "test-testrig-polecat-tombstone"
|
||||
|
||||
// Step 1: Create agent bead
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 2: Delete with --hard --force (supposed to permanently delete)
|
||||
err = bd.DeleteAgentBead(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("DeleteAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 3: BUG - Tombstone exists (check via bd list --status=tombstone)
|
||||
out, err := bd.run("list", "--status=tombstone", "--json")
|
||||
if err != nil {
|
||||
t.Fatalf("list tombstones: %v", err)
|
||||
}
|
||||
|
||||
// Parse to check if our agent is in the tombstone list
|
||||
var tombstones []Issue
|
||||
if err := json.Unmarshal(out, &tombstones); err != nil {
|
||||
t.Fatalf("parse tombstones: %v", err)
|
||||
}
|
||||
|
||||
foundTombstone := false
|
||||
for _, ts := range tombstones {
|
||||
if ts.ID == agentID {
|
||||
foundTombstone = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if !foundTombstone {
|
||||
// If bd ever fixes the --hard flag, this test will fail here
|
||||
// That's a good thing - it means the bug is fixed!
|
||||
t.Skip("bd --hard appears to be fixed (no tombstone created) - update this test")
|
||||
}
|
||||
|
||||
// Step 4: BUG - bd create fails with UNIQUE constraint
|
||||
_, err = bd.CreateAgentBead(agentID, "Test agent 2", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected UNIQUE constraint error, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "UNIQUE constraint") {
|
||||
t.Errorf("expected UNIQUE constraint error, got: %v", err)
|
||||
}
|
||||
|
||||
// Step 5: BUG - bd reopen fails (tombstones are invisible)
|
||||
_, err = bd.run("reopen", agentID, "--reason=test")
|
||||
if err == nil {
|
||||
t.Fatal("expected reopen to fail on tombstone, got nil")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "no issue found") && !strings.Contains(err.Error(), "issue not found") {
|
||||
t.Errorf("expected 'issue not found' error, got: %v", err)
|
||||
}
|
||||
|
||||
t.Log("BUG CONFIRMED: bd delete --hard creates tombstones that block recreation")
|
||||
}
|
||||
|
||||
// TestAgentBeadCloseReopenWorkaround demonstrates the workaround for the tombstone bug:
|
||||
// use Close instead of Delete, then Reopen works.
|
||||
func TestAgentBeadCloseReopenWorkaround(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Initialize beads database
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
agentID := "test-testrig-polecat-closereopen"
|
||||
|
||||
// Step 1: Create agent bead
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-task-1",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 2: Close (not delete) - this is the workaround
|
||||
err = bd.CloseAndClearAgentBead(agentID, "polecat removed")
|
||||
if err != nil {
|
||||
t.Fatalf("CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 3: Verify bead is closed (not tombstone)
|
||||
issue, err := bd.Show(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("Show after close: %v", err)
|
||||
}
|
||||
if issue.Status != "closed" {
|
||||
t.Errorf("status = %q, want 'closed'", issue.Status)
|
||||
}
|
||||
|
||||
// Step 4: Reopen works on closed beads
|
||||
_, err = bd.run("reopen", agentID, "--reason=re-spawning")
|
||||
if err != nil {
|
||||
t.Fatalf("reopen failed: %v", err)
|
||||
}
|
||||
|
||||
// Step 5: Verify bead is open again
|
||||
issue, err = bd.Show(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("Show after reopen: %v", err)
|
||||
}
|
||||
if issue.Status != "open" {
|
||||
t.Errorf("status = %q, want 'open'", issue.Status)
|
||||
}
|
||||
|
||||
t.Log("WORKAROUND CONFIRMED: Close + Reopen works for agent bead lifecycle")
|
||||
}
|
||||
|
||||
// TestCreateOrReopenAgentBead_ClosedBead tests that CreateOrReopenAgentBead
|
||||
// successfully reopens a closed agent bead and updates its fields.
|
||||
func TestCreateOrReopenAgentBead_ClosedBead(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Initialize beads database
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
agentID := "test-testrig-polecat-lifecycle"
|
||||
|
||||
// Simulate polecat lifecycle: spawn → nuke → respawn
|
||||
|
||||
// Spawn 1: Create agent bead with first task
|
||||
issue1, err := bd.CreateOrReopenAgentBead(agentID, agentID, &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-task-1",
|
||||
RoleBead: "test-polecat-role",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Spawn 1 - CreateOrReopenAgentBead: %v", err)
|
||||
}
|
||||
if issue1.Status != "open" {
|
||||
t.Errorf("Spawn 1: status = %q, want 'open'", issue1.Status)
|
||||
}
|
||||
|
||||
// Nuke 1: Close agent bead (workaround for tombstone bug)
|
||||
err = bd.CloseAndClearAgentBead(agentID, "polecat nuked")
|
||||
if err != nil {
|
||||
t.Fatalf("Nuke 1 - CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Spawn 2: CreateOrReopenAgentBead should reopen and update
|
||||
issue2, err := bd.CreateOrReopenAgentBead(agentID, agentID, &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-task-2", // Different task
|
||||
RoleBead: "test-polecat-role",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Spawn 2 - CreateOrReopenAgentBead: %v", err)
|
||||
}
|
||||
if issue2.Status != "open" {
|
||||
t.Errorf("Spawn 2: status = %q, want 'open'", issue2.Status)
|
||||
}
|
||||
|
||||
// Verify the hook was updated to the new task
|
||||
fields := ParseAgentFields(issue2.Description)
|
||||
if fields.HookBead != "test-task-2" {
|
||||
t.Errorf("Spawn 2: hook_bead = %q, want 'test-task-2'", fields.HookBead)
|
||||
}
|
||||
|
||||
// Nuke 2: Close again
|
||||
err = bd.CloseAndClearAgentBead(agentID, "polecat nuked again")
|
||||
if err != nil {
|
||||
t.Fatalf("Nuke 2 - CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Spawn 3: Should still work
|
||||
issue3, err := bd.CreateOrReopenAgentBead(agentID, agentID, &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-task-3",
|
||||
RoleBead: "test-polecat-role",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Spawn 3 - CreateOrReopenAgentBead: %v", err)
|
||||
}
|
||||
|
||||
fields = ParseAgentFields(issue3.Description)
|
||||
if fields.HookBead != "test-task-3" {
|
||||
t.Errorf("Spawn 3: hook_bead = %q, want 'test-task-3'", fields.HookBead)
|
||||
}
|
||||
|
||||
t.Log("LIFECYCLE TEST PASSED: spawn → nuke → respawn works with close/reopen")
|
||||
}
|
||||
|
||||
// TestCloseAndClearAgentBead_FieldClearing tests that CloseAndClearAgentBead clears all mutable
|
||||
// fields to emulate delete --force --hard behavior. This ensures reopened agent
|
||||
// beads don't have stale state from previous lifecycle.
|
||||
func TestCloseAndClearAgentBead_FieldClearing(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
// Initialize beads database
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
// Test cases for field clearing permutations
|
||||
tests := []struct {
|
||||
name string
|
||||
fields *AgentFields
|
||||
reason string
|
||||
}{
|
||||
{
|
||||
name: "all_fields_populated",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "running",
|
||||
HookBead: "test-issue-123",
|
||||
RoleBead: "test-polecat-role",
|
||||
CleanupStatus: "clean",
|
||||
ActiveMR: "test-mr-456",
|
||||
NotificationLevel: "normal",
|
||||
},
|
||||
reason: "polecat completed work",
|
||||
},
|
||||
{
|
||||
name: "only_hook_bead",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-issue-789",
|
||||
},
|
||||
reason: "polecat nuked",
|
||||
},
|
||||
{
|
||||
name: "only_active_mr",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "running",
|
||||
ActiveMR: "test-mr-abc",
|
||||
},
|
||||
reason: "",
|
||||
},
|
||||
{
|
||||
name: "only_cleanup_status",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "idle",
|
||||
CleanupStatus: "has_uncommitted",
|
||||
},
|
||||
reason: "cleanup required",
|
||||
},
|
||||
{
|
||||
name: "no_mutable_fields",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
},
|
||||
reason: "fresh spawn closed",
|
||||
},
|
||||
{
|
||||
name: "polecat_with_all_field_types",
|
||||
fields: &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "processing",
|
||||
HookBead: "test-task-xyz",
|
||||
ActiveMR: "test-mr-processing",
|
||||
CleanupStatus: "has_uncommitted",
|
||||
NotificationLevel: "verbose",
|
||||
},
|
||||
reason: "comprehensive cleanup",
|
||||
},
|
||||
}
|
||||
|
||||
for i, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Create unique agent ID for each test case
|
||||
agentID := fmt.Sprintf("test-testrig-%s-%d", tc.fields.RoleType, i)
|
||||
|
||||
// Step 1: Create agent bead with specified fields
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", tc.fields)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Verify fields were set
|
||||
issue, err := bd.Show(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("Show before close: %v", err)
|
||||
}
|
||||
beforeFields := ParseAgentFields(issue.Description)
|
||||
if tc.fields.HookBead != "" && beforeFields.HookBead != tc.fields.HookBead {
|
||||
t.Errorf("before close: hook_bead = %q, want %q", beforeFields.HookBead, tc.fields.HookBead)
|
||||
}
|
||||
|
||||
// Step 2: Close the agent bead
|
||||
err = bd.CloseAndClearAgentBead(agentID, tc.reason)
|
||||
if err != nil {
|
||||
t.Fatalf("CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 3: Verify bead is closed
|
||||
issue, err = bd.Show(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("Show after close: %v", err)
|
||||
}
|
||||
if issue.Status != "closed" {
|
||||
t.Errorf("status = %q, want 'closed'", issue.Status)
|
||||
}
|
||||
|
||||
// Step 4: Verify mutable fields were cleared
|
||||
afterFields := ParseAgentFields(issue.Description)
|
||||
|
||||
// hook_bead should be cleared (empty or "null")
|
||||
if afterFields.HookBead != "" {
|
||||
t.Errorf("after close: hook_bead = %q, want empty (was %q)", afterFields.HookBead, tc.fields.HookBead)
|
||||
}
|
||||
|
||||
// active_mr should be cleared
|
||||
if afterFields.ActiveMR != "" {
|
||||
t.Errorf("after close: active_mr = %q, want empty (was %q)", afterFields.ActiveMR, tc.fields.ActiveMR)
|
||||
}
|
||||
|
||||
// cleanup_status should be cleared
|
||||
if afterFields.CleanupStatus != "" {
|
||||
t.Errorf("after close: cleanup_status = %q, want empty (was %q)", afterFields.CleanupStatus, tc.fields.CleanupStatus)
|
||||
}
|
||||
|
||||
// agent_state should be "closed"
|
||||
if afterFields.AgentState != "closed" {
|
||||
t.Errorf("after close: agent_state = %q, want 'closed' (was %q)", afterFields.AgentState, tc.fields.AgentState)
|
||||
}
|
||||
|
||||
// Immutable fields should be preserved
|
||||
if afterFields.RoleType != tc.fields.RoleType {
|
||||
t.Errorf("after close: role_type = %q, want %q (should be preserved)", afterFields.RoleType, tc.fields.RoleType)
|
||||
}
|
||||
if afterFields.Rig != tc.fields.Rig {
|
||||
t.Errorf("after close: rig = %q, want %q (should be preserved)", afterFields.Rig, tc.fields.Rig)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestCloseAndClearAgentBead_NonExistent tests behavior when closing a non-existent agent bead.
|
||||
func TestCloseAndClearAgentBead_NonExistent(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
// Attempt to close non-existent bead
|
||||
err := bd.CloseAndClearAgentBead("test-nonexistent-polecat-xyz", "should fail")
|
||||
|
||||
// Should return an error (bd close on non-existent issue fails)
|
||||
if err == nil {
|
||||
t.Error("CloseAndClearAgentBead on non-existent bead should return error")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCloseAndClearAgentBead_AlreadyClosed tests behavior when closing an already-closed agent bead.
|
||||
func TestCloseAndClearAgentBead_AlreadyClosed(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
agentID := "test-testrig-polecat-doubleclosed"
|
||||
|
||||
// Create agent bead
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "running",
|
||||
HookBead: "test-issue-1",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// First close - should succeed
|
||||
err = bd.CloseAndClearAgentBead(agentID, "first close")
|
||||
if err != nil {
|
||||
t.Fatalf("First CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Second close - behavior depends on bd close semantics
|
||||
// Document actual behavior: bd close on already-closed bead may error or be idempotent
|
||||
err = bd.CloseAndClearAgentBead(agentID, "second close")
|
||||
|
||||
// Verify bead is still closed regardless of error
|
||||
issue, showErr := bd.Show(agentID)
|
||||
if showErr != nil {
|
||||
t.Fatalf("Show after double close: %v", showErr)
|
||||
}
|
||||
if issue.Status != "closed" {
|
||||
t.Errorf("status after double close = %q, want 'closed'", issue.Status)
|
||||
}
|
||||
|
||||
// Log actual behavior for documentation
|
||||
if err != nil {
|
||||
t.Logf("BEHAVIOR: CloseAndClearAgentBead on already-closed bead returns error: %v", err)
|
||||
} else {
|
||||
t.Log("BEHAVIOR: CloseAndClearAgentBead on already-closed bead is idempotent (no error)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCloseAndClearAgentBead_ReopenHasCleanState tests that reopening a closed agent bead
|
||||
// starts with clean state (no stale hook_bead, active_mr, etc.).
|
||||
func TestCloseAndClearAgentBead_ReopenHasCleanState(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
agentID := "test-testrig-polecat-cleanreopen"
|
||||
|
||||
// Step 1: Create agent with all fields populated
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "running",
|
||||
HookBead: "test-old-issue",
|
||||
RoleBead: "test-polecat-role",
|
||||
CleanupStatus: "clean",
|
||||
ActiveMR: "test-old-mr",
|
||||
NotificationLevel: "normal",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 2: Close - should clear mutable fields
|
||||
err = bd.CloseAndClearAgentBead(agentID, "completing old work")
|
||||
if err != nil {
|
||||
t.Fatalf("CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 3: Reopen with new fields
|
||||
newIssue, err := bd.CreateOrReopenAgentBead(agentID, agentID, &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "spawning",
|
||||
HookBead: "test-new-issue",
|
||||
RoleBead: "test-polecat-role",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateOrReopenAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Step 4: Verify new state - should have new hook, no stale data
|
||||
fields := ParseAgentFields(newIssue.Description)
|
||||
|
||||
if fields.HookBead != "test-new-issue" {
|
||||
t.Errorf("hook_bead = %q, want 'test-new-issue'", fields.HookBead)
|
||||
}
|
||||
|
||||
// The old active_mr should NOT be present (was cleared on close)
|
||||
if fields.ActiveMR == "test-old-mr" {
|
||||
t.Error("active_mr still has stale value 'test-old-mr' - CloseAndClearAgentBead didn't clear it")
|
||||
}
|
||||
|
||||
// agent_state should be the new state
|
||||
if fields.AgentState != "spawning" {
|
||||
t.Errorf("agent_state = %q, want 'spawning'", fields.AgentState)
|
||||
}
|
||||
|
||||
t.Log("CLEAN STATE CONFIRMED: Reopened agent bead has no stale mutable fields")
|
||||
}
|
||||
|
||||
// TestCloseAndClearAgentBead_ReasonVariations tests close with different reason values.
|
||||
func TestCloseAndClearAgentBead_ReasonVariations(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
cmd := exec.Command("bd", "--no-daemon", "init", "--prefix", "test", "--quiet")
|
||||
cmd.Dir = tmpDir
|
||||
if output, err := cmd.CombinedOutput(); err != nil {
|
||||
t.Fatalf("bd init: %v\n%s", err, output)
|
||||
}
|
||||
|
||||
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||
bd := New(beadsDir)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
reason string
|
||||
}{
|
||||
{"empty_reason", ""},
|
||||
{"simple_reason", "polecat nuked"},
|
||||
{"reason_with_spaces", "polecat completed work successfully"},
|
||||
{"reason_with_special_chars", "closed: issue #123 (resolved)"},
|
||||
{"long_reason", "This is a very long reason that explains in detail why the agent bead was closed including multiple sentences and detailed context about the situation."},
|
||||
}
|
||||
|
||||
for i, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
agentID := fmt.Sprintf("test-testrig-polecat-reason%d", i)
|
||||
|
||||
// Create agent bead
|
||||
_, err := bd.CreateAgentBead(agentID, "Test agent", &AgentFields{
|
||||
RoleType: "polecat",
|
||||
Rig: "testrig",
|
||||
AgentState: "running",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Close with specified reason
|
||||
err = bd.CloseAndClearAgentBead(agentID, tc.reason)
|
||||
if err != nil {
|
||||
t.Fatalf("CloseAndClearAgentBead: %v", err)
|
||||
}
|
||||
|
||||
// Verify closed
|
||||
issue, err := bd.Show(agentID)
|
||||
if err != nil {
|
||||
t.Fatalf("Show: %v", err)
|
||||
}
|
||||
if issue.Status != "closed" {
|
||||
t.Errorf("status = %q, want 'closed'", issue.Status)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -446,12 +446,14 @@ func (m *Manager) RemoveWithOptions(name string, force, nuclear bool) error {
|
||||
m.namePool.Release(name)
|
||||
_ = m.namePool.Save()
|
||||
|
||||
// Delete agent bead (non-fatal: may not exist or beads may not be available)
|
||||
// Close agent bead (non-fatal: may not exist or beads may not be available)
|
||||
// NOTE: We use CloseAndClearAgentBead instead of DeleteAgentBead because bd delete --hard
|
||||
// creates tombstones that cannot be reopened.
|
||||
agentID := m.agentBeadID(name)
|
||||
if err := m.beads.DeleteAgentBead(agentID); err != nil {
|
||||
if err := m.beads.CloseAndClearAgentBead(agentID, "polecat removed"); err != nil {
|
||||
// Only log if not "not found" - it's ok if it doesn't exist
|
||||
if !errors.Is(err, beads.ErrNotFound) {
|
||||
fmt.Printf("Warning: could not delete agent bead %s: %v\n", agentID, err)
|
||||
fmt.Printf("Warning: could not close agent bead %s: %v\n", agentID, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -529,11 +531,13 @@ func (m *Manager) RepairWorktreeWithOptions(name string, force bool, opts AddOpt
|
||||
}
|
||||
}
|
||||
|
||||
// Delete old agent bead before recreation (non-fatal)
|
||||
// Close old agent bead before recreation (non-fatal)
|
||||
// NOTE: We use CloseAndClearAgentBead instead of DeleteAgentBead because bd delete --hard
|
||||
// creates tombstones that cannot be reopened.
|
||||
agentID := m.agentBeadID(name)
|
||||
if err := m.beads.DeleteAgentBead(agentID); err != nil {
|
||||
if err := m.beads.CloseAndClearAgentBead(agentID, "polecat repair"); err != nil {
|
||||
if !errors.Is(err, beads.ErrNotFound) {
|
||||
fmt.Printf("Warning: could not delete old agent bead %s: %v\n", agentID, err)
|
||||
fmt.Printf("Warning: could not close old agent bead %s: %v\n", agentID, err)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user