From 22977d15ba5d07b564219939058b055975477dd6 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Fri, 19 Dec 2025 16:28:59 -0800 Subject: [PATCH] feat(daemon): verify requesting_cycle before kill (gt-y5o) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before killing an agent session, the daemon now verifies that the agent has set requesting_=true in its state.json file. This ensures agents have completed pre-shutdown tasks (git clean, handoff mail, etc) before being terminated. - Added verifyAgentRequestingState() for state file verification - Added identityToStateFile() to map identity to state.json path - Added comprehensive tests in lifecycle_test.go 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/daemon/lifecycle.go | 64 +++++++++++++ internal/daemon/lifecycle_test.go | 150 ++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 internal/daemon/lifecycle_test.go diff --git a/internal/daemon/lifecycle.go b/internal/daemon/lifecycle.go index e8c317b8..f535123c 100644 --- a/internal/daemon/lifecycle.go +++ b/internal/daemon/lifecycle.go @@ -3,7 +3,9 @@ package daemon import ( "encoding/json" "fmt" + "os" "os/exec" + "path/filepath" "strings" "time" ) @@ -117,6 +119,11 @@ func (d *Daemon) executeLifecycleAction(request *LifecycleRequest) error { d.logger.Printf("Executing %s for session %s", request.Action, sessionName) + // Verify agent state shows requesting_=true before killing + if err := d.verifyAgentRequestingState(request.From, request.Action); err != nil { + return fmt.Errorf("state verification failed: %w", err) + } + // Check if session exists running, err := d.tmux.HasSession(sessionName) if err != nil { @@ -217,3 +224,60 @@ func (d *Daemon) closeMessage(id string) error { cmd.Dir = d.config.TownRoot return cmd.Run() } + +// verifyAgentRequestingState verifies that the agent has set requesting_=true +// in its state.json before we kill its session. This ensures the agent is actually +// ready to be killed and has completed its pre-shutdown tasks (git clean, handoff mail, etc). +func (d *Daemon) verifyAgentRequestingState(identity string, action LifecycleAction) error { + stateFile := d.identityToStateFile(identity) + if stateFile == "" { + // If we can't determine state file, log warning but allow action + // This maintains backwards compatibility with agents that don't support state files yet + d.logger.Printf("Warning: cannot determine state file for %s, skipping verification", identity) + return nil + } + + data, err := os.ReadFile(stateFile) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("agent state file not found: %s (agent must set requesting_%s=true before lifecycle request)", stateFile, action) + } + return fmt.Errorf("reading agent state: %w", err) + } + + var state map[string]interface{} + if err := json.Unmarshal(data, &state); err != nil { + return fmt.Errorf("parsing agent state: %w", err) + } + + // Check for requesting_=true + key := "requesting_" + string(action) + val, ok := state[key] + if !ok { + return fmt.Errorf("agent state missing %s field (agent must set this before lifecycle request)", key) + } + + requesting, ok := val.(bool) + if !ok || !requesting { + return fmt.Errorf("agent state %s is not true (got: %v)", key, val) + } + + d.logger.Printf("Verified agent %s has %s=true", identity, key) + return nil +} + +// identityToStateFile maps an agent identity to its state.json file path. +func (d *Daemon) identityToStateFile(identity string) string { + switch identity { + case "mayor": + return filepath.Join(d.config.TownRoot, "mayor", "state.json") + default: + // Pattern: -witness → //witness/state.json + if strings.HasSuffix(identity, "-witness") { + rigName := strings.TrimSuffix(identity, "-witness") + return filepath.Join(d.config.TownRoot, rigName, "witness", "state.json") + } + // Unknown identity - can't determine state file + return "" + } +} diff --git a/internal/daemon/lifecycle_test.go b/internal/daemon/lifecycle_test.go new file mode 100644 index 00000000..247360c4 --- /dev/null +++ b/internal/daemon/lifecycle_test.go @@ -0,0 +1,150 @@ +package daemon + +import ( + "encoding/json" + "log" + "os" + "path/filepath" + "testing" +) + +func TestIdentityToStateFile(t *testing.T) { + d := &Daemon{ + config: &Config{ + TownRoot: "/test/town", + }, + } + + tests := []struct { + identity string + want string + }{ + {"mayor", "/test/town/mayor/state.json"}, + {"gastown-witness", "/test/town/gastown/witness/state.json"}, + {"anotherrig-witness", "/test/town/anotherrig/witness/state.json"}, + {"unknown", ""}, // Unknown identity returns empty + {"polecat", ""}, // Polecats not handled by daemon + {"gastown-refinery", ""}, // Refinery not handled by daemon + } + + for _, tt := range tests { + t.Run(tt.identity, func(t *testing.T) { + got := d.identityToStateFile(tt.identity) + if got != tt.want { + t.Errorf("identityToStateFile(%q) = %q, want %q", tt.identity, got, tt.want) + } + }) + } +} + +func TestVerifyAgentRequestingState(t *testing.T) { + // Create temp directory for test + tmpDir, err := os.MkdirTemp("", "daemon-test-*") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpDir) + + d := &Daemon{ + config: &Config{ + TownRoot: tmpDir, + }, + logger: log.New(os.Stderr, "[test] ", log.LstdFlags), + } + + // Create mayor directory + mayorDir := filepath.Join(tmpDir, "mayor") + if err := os.MkdirAll(mayorDir, 0755); err != nil { + t.Fatal(err) + } + + stateFile := filepath.Join(mayorDir, "state.json") + + t.Run("missing state file", func(t *testing.T) { + // Remove any existing state file + os.Remove(stateFile) + + err := d.verifyAgentRequestingState("mayor", ActionCycle) + if err == nil { + t.Error("expected error for missing state file") + } + }) + + t.Run("missing requesting_cycle field", func(t *testing.T) { + state := map[string]interface{}{ + "some_other_field": true, + } + writeStateFile(t, stateFile, state) + + err := d.verifyAgentRequestingState("mayor", ActionCycle) + if err == nil { + t.Error("expected error for missing requesting_cycle field") + } + }) + + t.Run("requesting_cycle is false", func(t *testing.T) { + state := map[string]interface{}{ + "requesting_cycle": false, + } + writeStateFile(t, stateFile, state) + + err := d.verifyAgentRequestingState("mayor", ActionCycle) + if err == nil { + t.Error("expected error when requesting_cycle is false") + } + }) + + t.Run("requesting_cycle is true", func(t *testing.T) { + state := map[string]interface{}{ + "requesting_cycle": true, + } + writeStateFile(t, stateFile, state) + + err := d.verifyAgentRequestingState("mayor", ActionCycle) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + + t.Run("requesting_shutdown is true", func(t *testing.T) { + state := map[string]interface{}{ + "requesting_shutdown": true, + } + writeStateFile(t, stateFile, state) + + err := d.verifyAgentRequestingState("mayor", ActionShutdown) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + + t.Run("requesting_restart is true", func(t *testing.T) { + state := map[string]interface{}{ + "requesting_restart": true, + } + writeStateFile(t, stateFile, state) + + err := d.verifyAgentRequestingState("mayor", ActionRestart) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + + t.Run("unknown identity skips verification", func(t *testing.T) { + // Unknown identities should not cause error (backwards compatibility) + err := d.verifyAgentRequestingState("unknown-agent", ActionCycle) + if err != nil { + t.Errorf("unexpected error for unknown identity: %v", err) + } + }) +} + +func writeStateFile(t *testing.T, path string, state map[string]interface{}) { + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0644); err != nil { + t.Fatal(err) + } +}