From 50752cfb978ecc3899cb343e2aceb497171dfeb1 Mon Sep 17 00:00:00 2001 From: Konstantin Tumalevich Date: Tue, 20 Jan 2026 11:15:29 +0500 Subject: [PATCH] feat: add mailman stop command for graceful daemon shutdown Implement file-based signaling mechanism to stop the mailman daemon: - Add `agentmail mailman stop` subcommand that creates .agentmail/.stop file - Daemon's file watcher detects .stop file and initiates graceful shutdown - Daemon removes both .stop and .pid files during shutdown - Uses atomic file creation (O_CREATE|O_EXCL) to detect "stop already pending" Exit codes: 0 (success), 1 (error/already pending) --- CLAUDE.md | 2 + cmd/agentmail/main.go | 45 +++- internal/cli/mailman_stop.go | 50 +++++ internal/cli/mailman_stop_test.go | 168 +++++++++++++++ internal/daemon/daemon.go | 25 ++- internal/daemon/watcher.go | 23 ++ internal/daemon/watcher_test.go | 69 ++++++ .../checklists/requirements.md | 66 ++++++ specs/012-mailman-stop/contracts/cli.md | 166 +++++++++++++++ specs/012-mailman-stop/data-model.md | 127 +++++++++++ specs/012-mailman-stop/plan.md | 87 ++++++++ specs/012-mailman-stop/quickstart.md | 200 ++++++++++++++++++ specs/012-mailman-stop/research.md | 129 +++++++++++ specs/012-mailman-stop/spec.md | 85 ++++++++ specs/012-mailman-stop/tasks.md | 191 +++++++++++++++++ 15 files changed, 1422 insertions(+), 11 deletions(-) create mode 100644 internal/cli/mailman_stop.go create mode 100644 internal/cli/mailman_stop_test.go create mode 100644 specs/012-mailman-stop/checklists/requirements.md create mode 100644 specs/012-mailman-stop/contracts/cli.md create mode 100644 specs/012-mailman-stop/data-model.md create mode 100644 specs/012-mailman-stop/plan.md create mode 100644 specs/012-mailman-stop/quickstart.md create mode 100644 specs/012-mailman-stop/research.md create mode 100644 specs/012-mailman-stop/spec.md create mode 100644 specs/012-mailman-stop/tasks.md diff --git a/CLAUDE.md b/CLAUDE.md index a4e4794..7e598a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -102,6 +102,8 @@ Templates are stored in `.specify/templates/` and project constitution in `.spec - JSONL files in `.agentmail/` directory (existing infrastructure), MCP server via STDIO transport (010-mcp-server) - Go 1.21+ (per constitution IC-001, project uses Go 1.25.3) + Standard library only (os/exec, encoding/json, syscall, time, os) (011-cleanup) - JSONL files in `.agentmail/` directory (recipients.jsonl, mailboxes/*.jsonl) (011-cleanup) +- Go 1.25.5 (minimum 1.21+ per IC-001) + Standard library only (os, os/exec, syscall, strconv, strings) (012-mailman-stop) +- `.agentmail/mailman.pid` (existing PID file from 006-mailman-daemon) (012-mailman-stop) ## Recent Changes - 001-agent-mail-structure: Added Go 1.21+ (per IC-001) + Standard library only (os/exec for tmux, encoding/json for JSONL) diff --git a/cmd/agentmail/main.go b/cmd/agentmail/main.go index 324b624..6d4f37c 100644 --- a/cmd/agentmail/main.go +++ b/cmd/agentmail/main.go @@ -178,26 +178,59 @@ Examples: var daemonMode bool mailmanFlagSet.BoolVar(&daemonMode, "daemon", false, "run in background (daemonize)") + // Mailman stop subcommand + stopFlagSet := flag.NewFlagSet("agentmail mailman stop", flag.ContinueOnError) + stopCmd := &ffcli.Command{ + Name: "stop", + ShortUsage: "agentmail mailman stop", + ShortHelp: "Stop the mailman daemon", + LongHelp: `Stop the running mailman daemon using file-based signaling. + +Creates a .stop file in .agentmail/ that the daemon detects and +initiates graceful shutdown. + +Exit codes: + 0 Success (stop signal sent) + 1 Error (stop already pending or filesystem error) + +Examples: + agentmail mailman stop`, + FlagSet: stopFlagSet, + Exec: func(ctx context.Context, args []string) error { + exitCode := cli.MailmanStop(os.Stdout, os.Stderr, cli.MailmanStopOptions{}) + if exitCode != 0 { + os.Exit(exitCode) + } + return nil + }, + } + mailmanCmd := &ffcli.Command{ Name: "mailman", - ShortUsage: "agentmail mailman [--daemon]", - ShortHelp: "Start the mailman daemon", - LongHelp: `Start the mailman daemon for message delivery notifications. + ShortUsage: "agentmail mailman [--daemon] | agentmail mailman stop", + ShortHelp: "Start or stop the mailman daemon", + LongHelp: `Start or stop the mailman daemon for message delivery notifications. The mailman daemon monitors mailboxes and can notify agents when new messages arrive. +Commands: + stop Stop the running mailman daemon + Flags: --daemon Run in background (daemonize) Exit codes: 0 Success - 2 Daemon already running + 1 Error + 2 Daemon already running (start only) Examples: agentmail mailman # Run in foreground - agentmail mailman --daemon # Run in background`, - FlagSet: mailmanFlagSet, + agentmail mailman --daemon # Run in background + agentmail mailman stop # Stop the daemon`, + FlagSet: mailmanFlagSet, + Subcommands: []*ffcli.Command{stopCmd}, Exec: func(ctx context.Context, args []string) error { exitCode := cli.Mailman(os.Stdout, os.Stderr, cli.MailmanOptions{ Daemonize: daemonMode, diff --git a/internal/cli/mailman_stop.go b/internal/cli/mailman_stop.go new file mode 100644 index 0000000..f6d9880 --- /dev/null +++ b/internal/cli/mailman_stop.go @@ -0,0 +1,50 @@ +package cli + +import ( + "fmt" + "io" + "os" + + "agentmail/internal/daemon" + "agentmail/internal/mail" +) + +// MailmanStopOptions configures the MailmanStop command behavior. +type MailmanStopOptions struct { + RepoRoot string // Repository root (defaults to finding git root) +} + +// MailmanStop implements the agentmail mailman stop command. +// Creates a .stop file to signal the daemon to shut down. +// +// Exit codes: +// - 0: Success (stop signal sent) +// - 1: Error (file exists or filesystem error) +func MailmanStop(stdout, stderr io.Writer, opts MailmanStopOptions) int { + // Find repository root + repoRoot := opts.RepoRoot + if repoRoot == "" { + var err error + repoRoot, err = mail.FindGitRoot() + if err != nil { + repoRoot, _ = os.Getwd() + } + } + + stopPath := daemon.StopFilePath(repoRoot) + + // Atomic create - fails if file exists (O_CREATE|O_EXCL) + f, err := os.OpenFile(stopPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) // #nosec G304 - stopPath is constructed from constants + if err != nil { + if os.IsExist(err) { + fmt.Fprintln(stderr, "Stop already pending") + return 1 + } + fmt.Fprintf(stderr, "Failed to send stop signal: %v\n", err) + return 1 + } + _ = f.Close() // G104: file was just created successfully, close error is non-critical + + fmt.Fprintln(stdout, "Stop signal sent") + return 0 +} diff --git a/internal/cli/mailman_stop_test.go b/internal/cli/mailman_stop_test.go new file mode 100644 index 0000000..6df4d6a --- /dev/null +++ b/internal/cli/mailman_stop_test.go @@ -0,0 +1,168 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "agentmail/internal/daemon" +) + +// ============================================================================= +// T003-T004: Tests for MailmanStop - successful stop file creation +// ============================================================================= + +func TestMailmanStop_CreatesStopFile(t *testing.T) { + // Create temp directory for test + tmpDir, err := os.MkdirTemp("", "agentmail-stop-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create .agentmail directory (simulating existing daemon setup) + agentmailDir := filepath.Join(tmpDir, ".agentmail") + if err := os.MkdirAll(agentmailDir, 0755); err != nil { + t.Fatalf("Failed to create .agentmail dir: %v", err) + } + + var stdout, stderr bytes.Buffer + + // Run stop command + exitCode := MailmanStop(&stdout, &stderr, MailmanStopOptions{ + RepoRoot: tmpDir, + }) + + // Verify exit code 0 + if exitCode != 0 { + t.Errorf("Expected exit code 0, got %d. stderr: %s", exitCode, stderr.String()) + } + + // Verify stop file was created + stopPath := daemon.StopFilePath(tmpDir) + if _, err := os.Stat(stopPath); os.IsNotExist(err) { + t.Error("Stop file was not created") + } +} + +func TestMailmanStop_OutputsSuccessMessage(t *testing.T) { + // Create temp directory for test + tmpDir, err := os.MkdirTemp("", "agentmail-stop-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create .agentmail directory + agentmailDir := filepath.Join(tmpDir, ".agentmail") + if err := os.MkdirAll(agentmailDir, 0755); err != nil { + t.Fatalf("Failed to create .agentmail dir: %v", err) + } + + var stdout, stderr bytes.Buffer + + // Run stop command + exitCode := MailmanStop(&stdout, &stderr, MailmanStopOptions{ + RepoRoot: tmpDir, + }) + + // Verify exit code 0 + if exitCode != 0 { + t.Errorf("Expected exit code 0, got %d", exitCode) + } + + // Verify success message + expectedMsg := "Stop signal sent\n" + if stdout.String() != expectedMsg { + t.Errorf("Expected stdout %q, got %q", expectedMsg, stdout.String()) + } + + // Verify no stderr output + if stderr.String() != "" { + t.Errorf("Expected empty stderr, got %q", stderr.String()) + } +} + +// ============================================================================= +// T015-T016: Tests for MailmanStop - stop already pending +// ============================================================================= + +func TestMailmanStop_StopAlreadyPending_ReturnsError(t *testing.T) { + // Create temp directory for test + tmpDir, err := os.MkdirTemp("", "agentmail-stop-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Create .agentmail directory + agentmailDir := filepath.Join(tmpDir, ".agentmail") + if err := os.MkdirAll(agentmailDir, 0755); err != nil { + t.Fatalf("Failed to create .agentmail dir: %v", err) + } + + // Pre-create the stop file to simulate pending stop + stopPath := daemon.StopFilePath(tmpDir) + if err := os.WriteFile(stopPath, []byte{}, 0600); err != nil { + t.Fatalf("Failed to create stop file: %v", err) + } + + var stdout, stderr bytes.Buffer + + // Run stop command + exitCode := MailmanStop(&stdout, &stderr, MailmanStopOptions{ + RepoRoot: tmpDir, + }) + + // Verify exit code 1 + if exitCode != 1 { + t.Errorf("Expected exit code 1, got %d", exitCode) + } + + // Verify error message + expectedMsg := "Stop already pending\n" + if stderr.String() != expectedMsg { + t.Errorf("Expected stderr %q, got %q", expectedMsg, stderr.String()) + } + + // Verify no stdout output + if stdout.String() != "" { + t.Errorf("Expected empty stdout, got %q", stdout.String()) + } +} + +func TestMailmanStop_FilesystemError_ReturnsError(t *testing.T) { + // Create temp directory for test + tmpDir, err := os.MkdirTemp("", "agentmail-stop-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + // Do NOT create .agentmail directory - this should cause a filesystem error + // when trying to create the stop file + + var stdout, stderr bytes.Buffer + + // Run stop command + exitCode := MailmanStop(&stdout, &stderr, MailmanStopOptions{ + RepoRoot: tmpDir, + }) + + // Verify exit code 1 + if exitCode != 1 { + t.Errorf("Expected exit code 1, got %d", exitCode) + } + + // Verify error message contains expected prefix + expectedPrefix := "Failed to send stop signal:" + if len(stderr.String()) < len(expectedPrefix) || stderr.String()[:len(expectedPrefix)] != expectedPrefix { + t.Errorf("Expected stderr to start with %q, got %q", expectedPrefix, stderr.String()) + } + + // Verify no stdout output + if stdout.String() != "" { + t.Errorf("Expected empty stdout, got %q", stdout.String()) + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index d8f7888..7fc0675 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -18,6 +18,9 @@ import ( // PIDFile is the filename for the mailman daemon PID file within .agentmail/ const PIDFile = "mailman.pid" +// StopFile is the filename for the mailman daemon stop signal within .agentmail/ +const StopFile = ".stop" + // DaemonStatus represents the status of an existing daemon process. type DaemonStatus int @@ -45,6 +48,11 @@ func PIDFilePath(repoRoot string) string { return filepath.Join(repoRoot, mail.RootDir, PIDFile) } +// StopFilePath returns the full path to the stop signal file for a given repository root. +func StopFilePath(repoRoot string) string { + return filepath.Join(repoRoot, mail.RootDir, StopFile) +} + // ReadPID reads the PID from the mailman.pid file. // Returns 0 if the file doesn't exist (not an error). // Returns an error if the file exists but contains invalid content. @@ -246,18 +254,25 @@ func runForeground(repoRoot string, stdout, stderr io.Writer) int { close(loopDone) }() - // Wait for shutdown signal or test stop + // Wait for shutdown signal, test stop, or file-based stop if stopChan != nil { - // Test mode: wait on either signal or stop channel + // Test mode: wait on signal, stop channel, or file-based stop select { case <-sigChan: case <-stopChan: + case <-fileWatcher.StopChan(): } } else { - // Production mode: wait only on signals - <-sigChan + // Production mode: wait on signals or file-based stop + select { + case <-sigChan: + case <-fileWatcher.StopChan(): + } } + // Remove stop file if it exists (FR-007 from 012-mailman-stop) + _ = os.Remove(StopFilePath(repoRoot)) // G104: best-effort cleanup + // Close file watcher to stop the notification loop if fileWatcher != nil { _ = fileWatcher.Close() // G104: best-effort cleanup @@ -265,7 +280,7 @@ func runForeground(repoRoot string, stdout, stderr io.Writer) int { <-loopDone // Wait for loop to finish - // Clean up PID file on shutdown + // Clean up PID file on shutdown (FR-008 from 012-mailman-stop) _ = DeletePID(repoRoot) // G104: best-effort cleanup, errors don't affect exit status return 0 diff --git a/internal/daemon/watcher.go b/internal/daemon/watcher.go index ff3de51..c6bfd83 100644 --- a/internal/daemon/watcher.go +++ b/internal/daemon/watcher.go @@ -100,6 +100,7 @@ type FileWatcher struct { mailboxDir string // Path to .agentmail/mailboxes/ agentmailDir string // Path to .agentmail/ stopChan chan struct{} // Signal to stop the watcher + fileStopChan chan struct{} // Signal when .stop file detected (for daemon shutdown) mode MonitoringMode // Current monitoring mode mu sync.Mutex // Protects mode logger io.Writer // Logger for foreground mode (nil = no logging) @@ -133,6 +134,7 @@ func NewFileWatcher(repoRoot string) (*FileWatcher, error) { mailboxDir: filepath.Join(repoRoot, mail.MailDir), agentmailDir: filepath.Join(repoRoot, mail.RootDir), stopChan: make(chan struct{}), + fileStopChan: make(chan struct{}), mode: ModeWatching, } @@ -144,6 +146,20 @@ func (fw *FileWatcher) SetLogger(logger io.Writer) { fw.logger = logger } +// StopChan returns a channel that is closed when the .stop file is detected. +// This can be used by the daemon to detect file-based stop signals. +func (fw *FileWatcher) StopChan() <-chan struct{} { + return fw.fileStopChan +} + +// isStopFileEvent checks if the event is a Create event for the .stop file. +func (fw *FileWatcher) isStopFileEvent(event fsnotify.Event) bool { + if !event.Has(fsnotify.Create) { + return false + } + return filepath.Base(event.Name) == StopFile +} + // AddWatches adds watches for .agentmail/ and .agentmail/mailboxes/ directories (FR-001, FR-004). func (fw *FileWatcher) AddWatches() error { // Watch .agentmail/ for recipients.jsonl changes (FR-005) @@ -226,6 +242,13 @@ func (fw *FileWatcher) Run(processFunc func()) error { return nil } + // Check for stop signal file (FR-005, FR-006 from 012-mailman-stop) + if fw.isStopFileEvent(event) { + fw.log("Stop signal file detected, initiating shutdown") + close(fw.fileStopChan) + return nil + } + // Check if this is a mailbox event (FR-009) if fw.isMailboxEvent(event) { fw.log("Mailbox change detected: %s (%s)", filepath.Base(event.Name), event.Op) diff --git a/internal/daemon/watcher_test.go b/internal/daemon/watcher_test.go index 2fd3e20..e16b02f 100644 --- a/internal/daemon/watcher_test.go +++ b/internal/daemon/watcher_test.go @@ -416,3 +416,72 @@ func TestFileWatcher_Run_StopsOnClose(t *testing.T) { t.Error("Watcher did not stop within timeout after Close") } } + +// ============================================================================= +// T005: Tests for FileWatcher stop file detection (FR-005, FR-006) +// ============================================================================= + +func TestFileWatcher_Run_DetectsStopFileCreation(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agentmail-watcher-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + fw, err := NewFileWatcher(tmpDir) + if err != nil { + t.Fatalf("NewFileWatcher failed: %v", err) + } + + err = fw.AddWatches() + if err != nil { + t.Fatalf("AddWatches failed: %v", err) + } + + processFunc := func() {} + + // Run watcher in goroutine + done := make(chan struct{}) + go func() { + _ = fw.Run(processFunc) + close(done) + }() + + // Give watcher time to start + time.Sleep(50 * time.Millisecond) + + // Create stop file - this should trigger shutdown + stopPath := StopFilePath(tmpDir) + if err := os.WriteFile(stopPath, []byte{}, 0600); err != nil { + t.Fatalf("Failed to create stop file: %v", err) + } + + // Run should return after detecting stop file + select { + case <-done: + // Success - watcher detected stop file and stopped + case <-time.After(1 * time.Second): + t.Error("Watcher did not stop within timeout after stop file creation") + _ = fw.Close() // Clean up + } +} + +func TestFileWatcher_StopChan_ReturnsChannelForShutdown(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "agentmail-watcher-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + fw, err := NewFileWatcher(tmpDir) + if err != nil { + t.Fatalf("NewFileWatcher failed: %v", err) + } + defer fw.Close() + + // StopChan should return a channel + ch := fw.StopChan() + if ch == nil { + t.Error("StopChan returned nil") + } +} diff --git a/specs/012-mailman-stop/checklists/requirements.md b/specs/012-mailman-stop/checklists/requirements.md new file mode 100644 index 0000000..8095fce --- /dev/null +++ b/specs/012-mailman-stop/checklists/requirements.md @@ -0,0 +1,66 @@ +# Specification Quality Checklist: Mailman Daemon Stop Command + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-01-20 +**Updated**: 2026-01-20 (File-based approach) +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## EARS Compliance + +- [x] All functional requirements follow EARS patterns (Ubiquitous/When/While/If-Then/Where) +- [x] Each requirement has explicit system name +- [x] All requirements use active voice +- [x] Each requirement contains only one "shall" +- [x] Numerical values include units (seconds, milliseconds, percent, etc.) +- [x] No vague terms (fast, efficient, user-friendly, robust) +- [x] No escape clauses (if possible, where appropriate) + +## EARS Pattern Mapping + +| Requirement | Pattern | Validation | +|-------------|---------|------------| +| FR-001 | Event-Driven (When) | ✅ | +| FR-002 | Event-Driven (When) | ✅ | +| FR-003 | Unwanted Behavior (If-Then) | ✅ | +| FR-004 | Unwanted Behavior (If-Then) | ✅ | +| FR-005 | State-Driven (While) | ✅ | +| FR-006 | Event-Driven (When) | ✅ | +| FR-007 | Event-Driven (When) | ✅ | +| FR-008 | Event-Driven (When) | ✅ | + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass validation +- Specification updated from SIGTERM to file-based approach +- File-based approach is simpler and cross-platform +- Reduced from 12 requirements to 8 (simpler design) +- EARS patterns used correctly: + - FR-001, FR-002: Event-Driven (When) - command invocation and success + - FR-003, FR-004: Unwanted Behavior (If-Then) - error conditions + - FR-005: State-Driven (While) - continuous monitoring + - FR-006, FR-007, FR-008: Event-Driven (When) - daemon events diff --git a/specs/012-mailman-stop/contracts/cli.md b/specs/012-mailman-stop/contracts/cli.md new file mode 100644 index 0000000..d76f281 --- /dev/null +++ b/specs/012-mailman-stop/contracts/cli.md @@ -0,0 +1,166 @@ +# CLI Contract: Mailman Stop Command + +**Feature**: 012-mailman-stop +**Date**: 2026-01-20 + +## Command Signature + +```text +agentmail mailman stop +``` + +**No flags or arguments required.** + +## Exit Codes + +| Code | Meaning | Condition | +|------|---------|-----------| +| 0 | Success | Stop signal file created | +| 1 | Error | File already exists or filesystem error | + +## Output Specification + +### Success (Exit 0) + +**Stream**: stdout +**Format**: `Stop signal sent\n` + +```text +Stop signal sent +``` + +### Error Conditions (Exit 1) + +All error messages are written to stderr. + +#### Stop Already Pending + +```text +Stop already pending +``` + +**Condition**: `.agentmail/.stop` file already exists. + +#### Filesystem Error + +```text +Failed to send stop signal: +``` + +**Condition**: Unable to create `.stop` file due to permissions, disk space, etc. + +## Behavior Specification + +### Fire-and-Forget Pattern + +The stop command creates a signal file and immediately exits with code 0. It does NOT: +- Wait for the daemon to terminate +- Verify the daemon is running +- Verify the daemon received the signal +- Delete any files + +The daemon is responsible for: +- Detecting the `.stop` file via file watcher +- Removing the `.stop` file +- Removing the `.pid` file +- Graceful shutdown + +### File-Based IPC + +The stop mechanism uses file creation as inter-process communication: + +1. Stop command checks if `.agentmail/.stop` exists +2. If exists → error "Stop already pending" +3. If not exists → create the file +4. Daemon's file watcher detects CREATE event +5. Daemon initiates shutdown and cleans up files + +This approach: +- Requires no process validation +- Works cross-platform +- Uses existing file watcher infrastructure +- Simplifies error handling + +## Integration with Existing Commands + +### Relationship to `agentmail mailman` + +```text +agentmail mailman # Start daemon (foreground) +agentmail mailman --daemon # Start daemon (background) +agentmail mailman stop # Stop daemon (NEW) +``` + +The `stop` subcommand is a peer to the existing start behavior, not a flag. + +### CLI Help Output (Expected) + +```text +$ agentmail mailman --help +Start or stop the mailman daemon + +Usage: + agentmail mailman [--daemon] + agentmail mailman stop + +Commands: + stop Stop the running mailman daemon + +Flags: + --daemon Run in background (daemonize) + +Exit codes: + 0 Success + 1 Error + 2 Daemon already running (start only) +``` + +## Test Scenarios + +### Happy Path + +```bash +# Start daemon +$ agentmail mailman --daemon +Mailman daemon started in background (PID: 12345) + +# Stop daemon +$ agentmail mailman stop +Stop signal sent +$ echo $? +0 + +# Verify daemon stopped (after brief delay) +$ cat .agentmail/mailman.pid +cat: .agentmail/mailman.pid: No such file or directory +$ cat .agentmail/.stop +cat: .agentmail/.stop: No such file or directory +``` + +### Stop Already Pending + +```bash +# Simulate pending stop +$ touch .agentmail/.stop + +$ agentmail mailman stop +Stop already pending +$ echo $? +1 +``` + +### No Daemon Running (Still Succeeds) + +```bash +# No daemon running, but command still creates file +$ agentmail mailman stop +Stop signal sent +$ echo $? +0 + +# File exists until something removes it +$ ls .agentmail/.stop +.agentmail/.stop +``` + +**Note**: The stop command doesn't verify if a daemon is running. It simply creates the signal file. If no daemon is running, the file will remain until manually deleted or a daemon starts and detects it. diff --git a/specs/012-mailman-stop/data-model.md b/specs/012-mailman-stop/data-model.md new file mode 100644 index 0000000..14c44d4 --- /dev/null +++ b/specs/012-mailman-stop/data-model.md @@ -0,0 +1,127 @@ +# Data Model: Mailman Stop Command + +**Feature**: 012-mailman-stop +**Date**: 2026-01-20 + +## Overview + +This feature introduces a file-based signaling mechanism for stopping the daemon. The stop command creates a signal file that the daemon detects via its file watcher. + +## New Entity + +### Stop Signal File + +**Location**: `.agentmail/.stop` +**Format**: Empty file (presence indicates stop request) +**Permissions**: `0600` (owner read/write only) +**Lifecycle**: Created by stop command, removed by daemon during shutdown + +**Purpose**: Acts as an inter-process communication (IPC) mechanism between the CLI stop command and the running daemon. + +**Operations**: +| Operation | Actor | Trigger | +|-----------|-------|---------| +| Create | CLI (stop command) | User runs `agentmail mailman stop` | +| Detect | Daemon (file watcher) | fsnotify CREATE event | +| Delete | Daemon | During graceful shutdown | + +## Existing Entity (Read-Only by Stop Command) + +### Mailman PID File + +**Location**: `.agentmail/mailman.pid` +**Format**: Plain text, single line containing numeric PID followed by newline +**Permissions**: `0600` (owner read/write only) + +**Note**: The stop command does NOT read or modify the PID file. The daemon removes this file during shutdown. + +## State Transitions + +```text +Stop Command Flow (Simplified): + +[Start: agentmail mailman stop] + │ + ▼ +┌─────────────────────┐ +│ Check .stop exists? │ +└──────────┬──────────┘ + │ + Yes ─┼─ No + │ │ + ▼ ▼ + [Exit 1] ┌─────────────────┐ + "Stop │ Create .stop │ + already └────────┬────────┘ + pending" │ + ┌────┴────┐ + │ Success?│ + └────┬────┘ + │ + No ──┼── Yes + │ │ + ▼ ▼ + [Exit 1] [Exit 0] + "Failed" "Stop signal + sent" +``` + +```text +Daemon Shutdown Flow: + +[Daemon Running] + │ + ▼ +┌─────────────────────────┐ +│ File watcher monitoring │ +│ .agentmail/ directory │ +└──────────┬──────────────┘ + │ + │ CREATE event for .stop + ▼ +┌─────────────────────────┐ +│ Initiate graceful │ +│ shutdown │ +└──────────┬──────────────┘ + │ + ▼ +┌─────────────────────────┐ +│ Remove .stop file │ +└──────────┬──────────────┘ + │ + ▼ +┌─────────────────────────┐ +│ Close file watcher │ +│ Wait for loop done │ +└──────────┬──────────────┘ + │ + ▼ +┌─────────────────────────┐ +│ Remove mailman.pid │ +└──────────┬──────────────┘ + │ + ▼ + [Exit 0] +``` + +## File System Layout + +```text +.agentmail/ +├── mailman.pid # Daemon PID (existing, created by daemon) +├── .stop # Stop signal (NEW, created by stop command) +├── recipients.jsonl # Agent states (existing) +└── mailboxes/ # Message storage (existing) + └── *.jsonl +``` + +## Comparison: Old vs New Approach + +| Aspect | Old (SIGTERM) | New (File-based) | +|--------|---------------|------------------| +| IPC Mechanism | Unix signals | File creation | +| Validation | Process name check | None (fire-and-forget) | +| Dependencies | syscall, os/exec | os (file ops only) | +| Cross-platform | Unix only | All platforms | +| Daemon detection | Instant (signal) | Instant (fsnotify) | +| Error scenarios | Permission denied, wrong process | File exists, filesystem error | diff --git a/specs/012-mailman-stop/plan.md b/specs/012-mailman-stop/plan.md new file mode 100644 index 0000000..9e9b510 --- /dev/null +++ b/specs/012-mailman-stop/plan.md @@ -0,0 +1,87 @@ +# Implementation Plan: Mailman Daemon Stop Command + +**Branch**: `012-mailman-stop` | **Date**: 2026-01-20 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/012-mailman-stop/spec.md` + +## Summary + +Add `agentmail mailman stop` subcommand to gracefully terminate the mailman daemon using file-based signaling. The command creates a `.stop` file in `.agentmail/` directory. The daemon detects this file via its existing fsnotify watcher and initiates graceful shutdown, removing both the stop file and PID file. This approach is simpler and more cross-platform than signal-based termination. + +## Technical Context + +**Language/Version**: Go 1.25.5 (minimum 1.21+ per IC-001) +**Primary Dependencies**: Standard library only (os for file operations) +**Storage**: `.agentmail/.stop` (new stop signal file), `.agentmail/mailman.pid` (existing) +**Testing**: `go test -v -race -cover` with >= 80% coverage +**Target Platform**: macOS and Linux (per constitution, with tmux installed) +**Project Type**: Single CLI tool +**Performance Goals**: Stop command returns within 1 second (fire-and-forget) +**Constraints**: No external dependencies for stop functionality +**Scale/Scope**: Single daemon process per repository + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| I. CLI-First Design | ✅ PASS | New subcommand with text I/O, exit codes 0/1 | +| II. Simplicity (YAGNI) | ✅ PASS | File-based IPC, reuses existing fsnotify watcher | +| III. Test Coverage | ✅ REQUIRED | Must achieve >= 80% coverage on new code | +| IV. Standard Library | ✅ PASS | Uses only stdlib (os package for file operations) | + +**Quality Gates Required**: +1. `gofmt -l .` - no output +2. `go mod verify` - pass +3. `go vet ./...` - pass +4. `go test -v -race -coverprofile=coverage.out ./...` - pass with >= 80% +5. `govulncheck ./...` - no vulnerabilities +6. `gosec ./...` - no issues + +## Project Structure + +### Documentation (this feature) + +```text +specs/012-mailman-stop/ +├── spec.md # Feature specification +├── plan.md # This file +├── research.md # Phase 0: File-based IPC research +├── data-model.md # Phase 1: Stop signal file entity +├── quickstart.md # Phase 1: Quick implementation guide +├── contracts/ # Phase 1: CLI contract +│ └── cli.md +└── tasks.md # Phase 2: Implementation tasks +``` + +### Source Code (repository root) + +```text +cmd/ +└── agentmail/ + └── main.go # Add 'stop' subcommand to mailman command + +internal/ +├── cli/ +│ ├── mailman.go # Existing mailman command handler +│ ├── mailman_stop.go # NEW: Stop command implementation +│ └── mailman_stop_test.go # NEW: Tests for stop command +└── daemon/ + ├── daemon.go # MODIFY: Add StopFile constant, StopFilePath, stop file removal + ├── watcher.go # MODIFY: Add stop file detection, StopChan() method + └── watcher_test.go # MODIFY: Add stop file detection tests +``` + +**Structure Decision**: Follows existing project layout. New `mailman_stop.go` in `internal/cli/` for the stop command handler. Modifications to existing `daemon.go` and `watcher.go` for stop file detection. + +## Complexity Tracking + +No violations - this feature aligns with all constitution principles. The file-based approach is actually simpler than the originally planned SIGTERM approach: + +| Aspect | SIGTERM Approach | File-Based Approach | +|--------|------------------|---------------------| +| New files | 4 (process.go, process_test.go, stop.go, stop_test.go) | 2 (stop.go, stop_test.go) | +| Dependencies | os, os/exec, syscall, strings | os only | +| Platform support | Unix only | macOS/Linux | +| Process validation | Required | Not needed | +| Error scenarios | Many (permissions, wrong process) | Few (file exists, filesystem error) | diff --git a/specs/012-mailman-stop/quickstart.md b/specs/012-mailman-stop/quickstart.md new file mode 100644 index 0000000..5de6109 --- /dev/null +++ b/specs/012-mailman-stop/quickstart.md @@ -0,0 +1,200 @@ +# Quickstart: Mailman Stop Command + +**Feature**: 012-mailman-stop +**Date**: 2026-01-20 + +## Overview + +Add `agentmail mailman stop` subcommand to stop the running mailman daemon using file-based signaling. + +## Implementation Steps + +### Step 1: Add Stop File Path Constant (internal/daemon/daemon.go) + +Add constant for the stop file path: + +```go +// StopFile is the filename for the mailman daemon stop signal within .agentmail/ +const StopFile = ".stop" + +// StopFilePath returns the full path to the stop signal file for a given repository root. +func StopFilePath(repoRoot string) string { + return filepath.Join(repoRoot, mail.RootDir, StopFile) +} +``` + +### Step 2: Add Stop Command Handler (internal/cli/mailman_stop.go) + +Create new file with the stop command implementation: + +```go +// MailmanStopOptions configures the MailmanStop command behavior. +type MailmanStopOptions struct { + RepoRoot string // Repository root (defaults to finding git root) +} + +// MailmanStop implements the agentmail mailman stop command. +// Creates a .stop file to signal the daemon to shut down. +// +// Exit codes: +// - 0: Success (stop signal sent) +// - 1: Error (file exists or filesystem error) +func MailmanStop(stdout, stderr io.Writer, opts MailmanStopOptions) int { + // Find repository root + repoRoot := opts.RepoRoot + if repoRoot == "" { + var err error + repoRoot, err = mail.FindGitRoot() + if err != nil { + repoRoot, _ = os.Getwd() + } + } + + stopPath := daemon.StopFilePath(repoRoot) + + // Atomic create - fails if file exists + f, err := os.OpenFile(stopPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) + if err != nil { + if os.IsExist(err) { + fmt.Fprintln(stderr, "Stop already pending") + return 1 + } + fmt.Fprintf(stderr, "Failed to send stop signal: %v\n", err) + return 1 + } + f.Close() + + fmt.Fprintln(stdout, "Stop signal sent") + return 0 +} +``` + +### Step 3: Update Daemon File Watcher (internal/daemon/watcher.go) + +Modify the file watcher to detect `.stop` file and signal shutdown: + +```go +// In Run() method, add check for .stop file: +case event, ok := <-fw.watcher.Events: + if !ok { + return nil + } + + // Check for stop signal file + if event.Op&fsnotify.Create != 0 && filepath.Base(event.Name) == StopFile { + // Signal shutdown via channel + close(fw.stopChan) + return nil + } + + // Existing event handling... +``` + +### Step 4: Update Daemon Shutdown (internal/daemon/daemon.go) + +Update `runForeground()` to handle file-based stop: + +```go +// Wait for shutdown signal, test stop, or file-based stop +select { +case <-sigChan: +case <-stopChan: +case <-fileWatcher.StopChan(): +} + +// Remove stop file if it exists +_ = os.Remove(StopFilePath(repoRoot)) + +// Rest of existing shutdown sequence... +``` + +### Step 5: Register Subcommand (cmd/agentmail/main.go) + +Add `stop` as subcommand to `mailmanCmd`: + +```go +stopCmd := &ffcli.Command{ + Name: "stop", + ShortUsage: "agentmail mailman stop", + ShortHelp: "Stop the mailman daemon", + Exec: func(ctx context.Context, args []string) error { + exitCode := cli.MailmanStop(os.Stdout, os.Stderr, cli.MailmanStopOptions{}) + if exitCode != 0 { + os.Exit(exitCode) + } + return nil + }, +} + +mailmanCmd := &ffcli.Command{ + // ... existing config ... + Subcommands: []*ffcli.Command{stopCmd}, +} +``` + +### Step 6: Write Tests + +#### internal/cli/mailman_stop_test.go +- Test success case (file created) +- Test "stop already pending" (file exists) +- Test filesystem error (permissions) + +#### internal/daemon/watcher_test.go (extend existing) +- Test stop file detection triggers shutdown + +## Key Files to Modify + +| File | Change | +|------|--------| +| `internal/daemon/daemon.go` | Add StopFile constant, StopFilePath function | +| `internal/daemon/watcher.go` | Add stop file detection, StopChan() method | +| `internal/cli/mailman_stop.go` | NEW: Stop command handler | +| `internal/cli/mailman_stop_test.go` | NEW: Tests | +| `cmd/agentmail/main.go` | Add stop subcommand | + +## Testing Commands + +```bash +# Run all tests with coverage +go test -v -race -coverprofile=coverage.out ./... + +# Run only stop-related tests +go test -v -race ./internal/cli/... -run MailmanStop +go test -v -race ./internal/daemon/... -run Stop + +# Check coverage percentage +go tool cover -func=coverage.out | grep total + +# Verify quality gates +gofmt -l . +go vet ./... +govulncheck ./... +gosec ./... +``` + +## Manual Testing + +```bash +# Build +go build -o agentmail ./cmd/agentmail + +# Start daemon +./agentmail mailman --daemon + +# Verify running +cat .agentmail/mailman.pid + +# Stop daemon +./agentmail mailman stop +# Output: Stop signal sent + +# Verify stopped (wait a moment) +cat .agentmail/mailman.pid # Should not exist +cat .agentmail/.stop # Should not exist (daemon removes it) + +# Test "stop already pending" +touch .agentmail/.stop +./agentmail mailman stop +# Output: Stop already pending +rm .agentmail/.stop +``` diff --git a/specs/012-mailman-stop/research.md b/specs/012-mailman-stop/research.md new file mode 100644 index 0000000..ecf2b1b --- /dev/null +++ b/specs/012-mailman-stop/research.md @@ -0,0 +1,129 @@ +# Research: Mailman Stop Command + +**Feature**: 012-mailman-stop +**Date**: 2026-01-20 + +## Research Questions + +### RQ-1: File-Based IPC Mechanism + +**Question**: How to reliably signal the daemon to stop using a file? + +**Decision**: Create an empty `.stop` file in `.agentmail/` directory. Daemon detects via existing fsnotify watcher. + +**Rationale**: +- File creation is a simple, cross-platform IPC mechanism +- The daemon already has fsnotify watching `.agentmail/` for mailbox changes +- No need for Unix signals, process validation, or syscall dependencies +- Works on all platforms (Windows, macOS, Linux) + +**Alternatives Considered**: + +| Approach | Pros | Cons | Decision | +|----------|------|------|----------| +| SIGTERM signal | Standard Unix pattern | Unix-only, requires process validation | ❌ Rejected | +| Named pipe/FIFO | Bidirectional communication | Complex, Unix-only | ❌ Rejected | +| Unix socket | Reliable IPC | Complex setup, Unix-only | ❌ Rejected | +| File creation | Simple, cross-platform, uses existing watcher | One-way signal only | ✅ Selected | + +### RQ-2: Atomic File Creation for Exclusivity + +**Question**: How to ensure only one stop signal can be pending at a time? + +**Decision**: Use `os.OpenFile` with `O_CREATE|O_EXCL` flags to atomically create the file only if it doesn't exist. + +**Rationale**: +- `O_EXCL` flag causes the open to fail if file exists +- This is atomic at the filesystem level +- No race conditions between check and create +- Go's `os.OpenFile` supports this directly + +**Implementation**: +```go +// Atomic create - fails if file exists +f, err := os.OpenFile(stopFilePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) +if err != nil { + if os.IsExist(err) { + return "Stop already pending" + } + return fmt.Sprintf("Failed to send stop signal: %v", err) +} +f.Close() +return "Stop signal sent" +``` + +### RQ-3: Daemon Stop File Detection + +**Question**: How should the daemon detect the `.stop` file? + +**Decision**: Extend the existing `FileWatcher` to detect CREATE events for `.stop` file in `.agentmail/` directory. + +**Rationale**: +- The daemon already watches `.agentmail/` and `mailboxes/` directories +- fsnotify provides CREATE events for new files +- No additional polling or infrastructure needed +- Detection is nearly instant (< 100ms typically) + +**Implementation Notes**: +- The watcher's `Run()` function receives all events +- Filter for `fsnotify.Create` events where filename is `.stop` +- When detected, trigger graceful shutdown sequence + +### RQ-4: Graceful Shutdown Sequence + +**Question**: What's the correct order for daemon shutdown? + +**Decision**: Follow this sequence: +1. Detect `.stop` file +2. Remove `.stop` file (acknowledge receipt) +3. Close file watcher (stops notification loop) +4. Wait for notification loop to finish +5. Remove PID file +6. Exit with code 0 + +**Rationale**: +- Removing `.stop` first prevents stale signal files +- Closing watcher before PID removal ensures clean state +- Matches existing signal-based shutdown sequence in `runForeground()` + +**Existing Code Reference** (`internal/daemon/daemon.go:249-271`): +```go +// Wait for shutdown signal or test stop +<-sigChan + +// Close file watcher to stop the notification loop +if fileWatcher != nil { + _ = fileWatcher.Close() +} + +<-loopDone // Wait for loop to finish + +// Clean up PID file on shutdown +_ = DeletePID(repoRoot) + +return 0 +``` + +The new approach will add `.stop` file detection alongside signal handling. + +## Dependencies Review + +**No new dependencies required.** This approach simplifies dependencies: + +| Before (SIGTERM) | After (File-based) | +|------------------|-------------------| +| os | os | +| os/exec | ~~removed~~ | +| syscall | ~~removed~~ | +| strings | ~~removed~~ | + +The file-based approach uses only the `os` package for file operations. + +## Conclusion + +All research questions resolved. The file-based approach is simpler, more cross-platform, and leverages existing infrastructure (fsnotify watcher). Implementation requires: + +1. **Stop command**: ~20 lines of code (check exists, create file, print message) +2. **Daemon changes**: ~10 lines (detect .stop file, add to shutdown sequence) + +No process validation, signal handling, or platform-specific code needed. diff --git a/specs/012-mailman-stop/spec.md b/specs/012-mailman-stop/spec.md new file mode 100644 index 0000000..eb9b4d2 --- /dev/null +++ b/specs/012-mailman-stop/spec.md @@ -0,0 +1,85 @@ +# Feature Specification: Mailman Daemon Stop Command + +**Feature Branch**: `012-mailman-stop` +**Created**: 2026-01-20 +**Status**: Implemented +**Input**: User description: "Add ability to stop mailman daemon via file-based signaling mechanism" + +## Clarifications + +### Session 2026-01-20 + +- Q: Should the stop command verify the daemon has terminated, or return immediately after sending SIGTERM? → A: Fire-and-forget (create stop file and exit immediately) +- Q: Should the stop command validate that a daemon is running before creating the .stop file? → A: No, just create the file unconditionally +- Q: How should the daemon detect the .stop file? → A: File watcher event (use existing fsnotify) +- Q: What should happen if .stop file already exists when stop command runs? → A: Report "stop already pending" and exit with code 1 + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Stop Running Daemon (Priority: P1) + +An agent operator wants to gracefully stop a running mailman daemon. They invoke `agentmail mailman stop` which creates a stop signal file. The daemon detects this file via its file watcher and shuts down gracefully. + +**Why this priority**: This is the core functionality - the primary use case for the stop command is to signal the daemon to terminate. + +**Independent Test**: Can be fully tested by starting a daemon with `agentmail mailman`, then running `agentmail mailman stop` and verifying the daemon terminates and cleans up its files. + +**Acceptance Scenarios**: + +1. **Given** no `.stop` file exists in `.agentmail/`, **When** `agentmail mailman stop` is invoked, **Then** the command creates `.agentmail/.stop` file, outputs "Stop signal sent" and exits with code 0 +2. **Given** a mailman daemon is running with file watcher, **When** `.agentmail/.stop` file is created, **Then** the daemon detects the file, removes it, and initiates graceful shutdown +3. **Given** a mailman daemon is shutting down, **When** shutdown completes, **Then** the daemon removes the PID file at `.agentmail/mailman.pid` + +--- + +### User Story 2 - Stop Already Pending (Priority: P2) + +An agent operator runs the stop command when a previous stop is already pending (stop file exists). The system informs them that a stop is already in progress. + +**Why this priority**: Important for user feedback to avoid confusion about multiple stop attempts. + +**Independent Test**: Can be fully tested by creating `.agentmail/.stop` file manually, then invoking `agentmail mailman stop` and verifying the error message. + +**Acceptance Scenarios**: + +1. **Given** `.agentmail/.stop` file already exists, **When** `agentmail mailman stop` is invoked, **Then** the command outputs "Stop already pending" to stderr and exits with code 1 + +## Requirements *(mandatory)* + +### Functional Requirements + +**Stop Command:** + +- **FR-001**: When `agentmail mailman stop` is invoked, the CLI shall attempt to create the file `.agentmail/.stop`. +- **FR-002**: When the `.stop` file is created successfully, the CLI shall output "Stop signal sent" to stdout and exit with code 0. +- **FR-003**: If the `.stop` file already exists, then the CLI shall output "Stop already pending" to stderr and exit with code 1. +- **FR-004**: If the `.stop` file cannot be created due to a filesystem error, then the CLI shall output "Failed to send stop signal: " to stderr and exit with code 1. + +**Daemon Stop File Detection:** + +- **FR-005**: While the daemon is running, the daemon shall monitor the `.agentmail/` directory for file creation events using the existing file watcher. +- **FR-006**: When the daemon detects creation of `.agentmail/.stop` file, the daemon shall initiate graceful shutdown. +- **FR-007**: When the daemon initiates shutdown, the daemon shall remove the `.agentmail/.stop` file. +- **FR-008**: When the daemon completes shutdown, the daemon shall remove the `.agentmail/mailman.pid` file. + +### Key Entities + +- **Stop Signal File**: File at `.agentmail/.stop` used to signal the daemon to shut down +- **Mailman PID File**: File at `.agentmail/mailman.pid` containing the daemon's process ID (existing) + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Stop command creates signal file and returns within 1 second (fire-and-forget) +- **SC-002**: Daemon detects stop file within 1 second of creation (via file watcher event) +- **SC-003**: Daemon removes both `.stop` and `.pid` files during shutdown in 100% of cases +- **SC-004**: Stop command correctly identifies pending stop in 100% of cases +- **SC-005**: All error conditions produce clear, actionable error messages to stderr + +## Assumptions + +- The daemon's existing file watcher (fsnotify) can detect file creation in `.agentmail/` directory +- The daemon already has graceful shutdown logic via signal handling that can be reused +- File creation is atomic enough to serve as a reliable IPC mechanism +- The `.agentmail/` directory exists (created by daemon on startup) diff --git a/specs/012-mailman-stop/tasks.md b/specs/012-mailman-stop/tasks.md new file mode 100644 index 0000000..aa31d67 --- /dev/null +++ b/specs/012-mailman-stop/tasks.md @@ -0,0 +1,191 @@ +# Tasks: Mailman Daemon Stop Command + +**Input**: Design documents from `/specs/012-mailman-stop/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/cli.md, quickstart.md + +**Tests**: Included (constitution requires >= 80% coverage) + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (e.g., US1, US2) +- Include exact file paths in descriptions + +## Path Conventions + +Based on plan.md structure (Go CLI project): +- `cmd/agentmail/main.go` - CLI entry point +- `internal/daemon/` - Daemon package (existing + modifications) +- `internal/cli/` - CLI handlers (existing + new) + +--- + +## Phase 1: Setup + +**Purpose**: Add stop file constant and path function + +- [x] T001 Add StopFile constant ".stop" in internal/daemon/daemon.go +- [x] T002 Add StopFilePath(repoRoot string) function in internal/daemon/daemon.go + +**Checkpoint**: Stop file path infrastructure ready + +--- + +## Phase 2: User Story 1 - Stop Running Daemon (Priority: P1) 🎯 MVP + +**Goal**: Create stop signal file and have daemon detect it and shut down + +**Independent Test**: Start daemon, run `agentmail mailman stop`, verify daemon terminates and cleans up files + +### Tests for User Story 1 + +- [x] T003 [P] [US1] Write test for successful stop file creation in internal/cli/mailman_stop_test.go +- [x] T004 [P] [US1] Write test for "Stop signal sent" message and exit code 0 in internal/cli/mailman_stop_test.go +- [x] T005 [P] [US1] Write test for daemon stop file detection in internal/daemon/watcher_test.go + +### Implementation for User Story 1 + +- [x] T006 [US1] Create MailmanStopOptions struct in internal/cli/mailman_stop.go +- [x] T007 [US1] Implement MailmanStop function with atomic file creation (O_CREATE|O_EXCL) in internal/cli/mailman_stop.go +- [x] T008 [US1] Add success message "Stop signal sent" output in internal/cli/mailman_stop.go +- [x] T009 [US1] Add stop file detection (fsnotify.Create event for .stop) in internal/daemon/watcher.go +- [x] T010 [US1] Add StopChan() method to FileWatcher to signal shutdown in internal/daemon/watcher.go +- [x] T011 [US1] Update runForeground() to select on fileWatcher.StopChan() in internal/daemon/daemon.go +- [x] T012 [US1] Add stop file removal during daemon shutdown in internal/daemon/daemon.go +- [x] T013 [US1] Register 'stop' subcommand under mailmanCmd in cmd/agentmail/main.go +- [x] T014 [US1] Run tests to verify US1: `go test -v ./internal/cli/... -run MailmanStop && go test -v ./internal/daemon/... -run Stop` + +**Checkpoint**: User Story 1 complete - can stop a running daemon via file signal + +--- + +## Phase 3: User Story 2 - Stop Already Pending (Priority: P2) + +**Goal**: Detect and report when a stop is already pending + +**Independent Test**: Create `.agentmail/.stop` file manually, run `agentmail mailman stop`, verify error message + +### Tests for User Story 2 + +- [x] T015 [P] [US2] Write test for "Stop already pending" when file exists in internal/cli/mailman_stop_test.go +- [x] T016 [P] [US2] Write test for exit code 1 when file exists in internal/cli/mailman_stop_test.go + +### Implementation for User Story 2 + +- [x] T017 [US2] Add os.IsExist error handling for "Stop already pending" message in internal/cli/mailman_stop.go +- [x] T018 [US2] Add generic filesystem error handling "Failed to send stop signal: " in internal/cli/mailman_stop.go +- [x] T019 [US2] Run tests to verify US2: `go test -v ./internal/cli/... -run MailmanStop` + +**Checkpoint**: Both user stories complete - full stop functionality with error handling + +--- + +## Phase 4: Polish & Quality Gates + +**Purpose**: Ensure code meets all quality requirements + +- [x] T020 [P] Update mailman command help text in cmd/agentmail/main.go to show stop subcommand +- [x] T021 [P] Run gofmt and fix any formatting issues: `gofmt -w .` +- [x] T022 [P] Run go vet and fix any issues: `go vet ./...` +- [x] T023 Run full test suite with coverage: `go test -v -race -coverprofile=coverage.out ./...` +- [x] T024 Verify coverage >= 80%: `go tool cover -func=coverage.out | grep total` +- [x] T025 Run govulncheck: `govulncheck ./...` +- [x] T026 Run gosec: `gosec ./...` +- [x] T027 Build and manual test per quickstart.md: `go build -o agentmail ./cmd/agentmail` + +**Checkpoint**: All quality gates pass - ready for merge + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +``` +Phase 1 (Setup) → No dependencies +Phase 2 (US1) → Depends on Phase 1 +Phase 3 (US2) → Depends on Phase 2 (shares MailmanStop function) +Phase 4 (Polish) → Depends on all user stories +``` + +### User Story Dependencies + +- **User Story 1 (P1)**: Requires Phase 1 setup (StopFilePath function) +- **User Story 2 (P2)**: Builds on US1 implementation (error handling in same function) + +### Within Each Phase + +- Tests MUST be written and FAIL before implementation +- Implementation tasks are sequential within a story +- [P] tasks can run in parallel + +--- + +## Parallel Opportunities + +### User Story 1 Tests (T003-T005) + +```bash +# Run in parallel - different test files: +T003: Test stop file creation (cli tests) +T004: Test success message (cli tests) +T005: Test daemon detection (watcher tests) +``` + +### User Story 2 Tests (T015-T016) + +```bash +# Run in parallel - different test cases: +T015: Test "already pending" message +T016: Test exit code 1 +``` + +### Polish Phase (T020-T022) + +```bash +# Run in parallel - independent operations: +T020: Update help text +T021: Run gofmt +T022: Run go vet +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1: Setup (T001-T002) +2. Complete Phase 2: User Story 1 (T003-T014) +3. **STOP and VALIDATE**: Test `agentmail mailman stop` with running daemon +4. Can deploy MVP with basic stop functionality + +### Full Implementation + +1. Complete Setup → StopFilePath ready +2. Add User Story 1 → Can stop running daemons +3. Add User Story 2 → Handles "already pending" case +4. Run Polish phase → Quality gates pass +5. Ready for merge + +### Estimated Effort + +| Phase | Tasks | Complexity | +|-------|-------|------------| +| Setup | 2 | Simple (constants) | +| US1 | 12 | Medium (file ops + watcher) | +| US2 | 5 | Simple (error handling) | +| Polish | 8 | Standard | +| **Total** | **27** | **~2 hours** | + +--- + +## Notes + +- File-based approach is simpler than SIGTERM (no process validation needed) +- Uses existing fsnotify watcher infrastructure +- O_CREATE|O_EXCL provides atomic file creation (prevents race conditions) +- Daemon removes both .stop and .pid files during shutdown +- Cross-platform compatible (works on Windows, macOS, Linux)