Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
45 changes: 39 additions & 6 deletions cmd/agentmail/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
50 changes: 50 additions & 0 deletions internal/cli/mailman_stop.go
Original file line number Diff line number Diff line change
@@ -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()
}
Comment on lines +29 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider logging or documenting the silent Getwd fallback behavior.

If both FindGitRoot and Getwd fail (rare but possible), the empty repoRoot will cause a predictable OpenFile error. The current behavior is acceptable since the error will surface at file creation, but documenting this edge case in the function comment could help future maintainers.

🤖 Prompt for AI Agents
In `@internal/cli/mailman_stop.go` around lines 29 - 31, The code silently falls
back to os.Getwd when FindGitRoot fails and leaves repoRoot empty if Getwd also
fails; update mailman_stop.go to either log that fallback and any Getwd error
(in the block where err != nil and repoRoot is assigned) or add a clear function
comment above the relevant function noting that repoRoot may be empty if both
FindGitRoot and os.Getwd fail, referencing FindGitRoot, os.Getwd, and repoRoot
so future maintainers understand the edge case.

}

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
}
168 changes: 168 additions & 0 deletions internal/cli/mailman_stop_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
Comment on lines +158 to +162
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Use strings.HasPrefix for clearer intent.

The manual string slicing works but is less readable than the idiomatic helper.

♻️ Suggested improvement
+import "strings"
+
 // Verify error message contains expected prefix
 expectedPrefix := "Failed to send stop signal:"
-if len(stderr.String()) < len(expectedPrefix) || stderr.String()[:len(expectedPrefix)] != expectedPrefix {
+if !strings.HasPrefix(stderr.String(), expectedPrefix) {
     t.Errorf("Expected stderr to start with %q, got %q", expectedPrefix, stderr.String())
 }
🤖 Prompt for AI Agents
In `@internal/cli/mailman_stop_test.go` around lines 158 - 162, Replace the manual
length check and slicing that compares stderr.String() to expectedPrefix with
the idiomatic strings.HasPrefix call: import the strings package, then use if
!strings.HasPrefix(stderr.String(), expectedPrefix) { t.Errorf(... ) }
referencing the existing stderr variable and expectedPrefix constant in the test
(mailman_stop_test.go) to make the intent clearer and avoid manual bounds
checks.


// Verify no stdout output
if stdout.String() != "" {
t.Errorf("Expected empty stdout, got %q", stdout.String())
}
}
25 changes: 20 additions & 5 deletions internal/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -246,26 +254,33 @@ 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
}

<-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
Expand Down
Loading