diff --git a/internal/cli/receive.go b/internal/cli/receive.go index 4e50c7d..5217dcd 100644 --- a/internal/cli/receive.go +++ b/internal/cli/receive.go @@ -12,165 +12,148 @@ import ( // ReceiveOptions configures the Receive command behavior. // Used for testing to mock tmux and file system operations. type ReceiveOptions struct { - SkipTmuxCheck bool // Skip tmux environment check - MockWindows []string // Mock list of tmux windows - MockReceiver string // Mock receiver window name - RepoRoot string // Repository root (defaults to current directory) - HookMode bool // Enable hook mode for Claude Code integration + SkipTmuxCheck bool // Skip tmux environment check + SkipTimestampUpdate bool // Skip updating last_read_at timestamp (for testing) + MockWindows []string // Mock list of tmux windows + MockReceiver string // Mock receiver window name + RepoRoot string // Repository root (defaults to current directory) + HookMode bool // Enable hook mode for Claude Code integration } -// Receive implements the agentmail receive command. -// T034: Implement Receive command structure -// T035: Add tmux validation (exit code 2 if not in tmux) -// T036: Add message retrieval and display formatting -// T037: Add "No unread messages" handling (exit code 0) -// -// Hook mode behavior (FR-001 through FR-005): -// - FR-001a/b/c: Write notification to STDERR, exit 2, mark as read when messages exist -// - FR-002: Exit 0 with no output when no messages -// - FR-003: Exit 0 with no output when not in tmux -// - FR-004a/b/c: Exit 0 with no output on any error -// - FR-005: All output to STDERR in hook mode -func Receive(stdout, stderr io.Writer, opts ReceiveOptions) int { - // T035: Validate running inside tmux - if !opts.SkipTmuxCheck { - if !tmux.InTmux() { - // FR-003: Hook mode exits silently when not in tmux - if opts.HookMode { - return 0 - } - fmt.Fprintln(stderr, "error: agentmail must run inside a tmux session") - return 2 - } - } +// receiveError represents an error during receive with context. +type receiveError struct { + msg string + exitCode int +} - // Get receiver identity - var receiver string +// getReceiver returns the receiver window name. +func (opts *ReceiveOptions) getReceiver() (string, error) { if opts.MockReceiver != "" { - receiver = opts.MockReceiver - } else { - var err error - receiver, err = tmux.GetCurrentWindow() - if err != nil { - // FR-004a: Hook mode exits silently on errors - if opts.HookMode { - return 0 - } - fmt.Fprintf(stderr, "error: failed to get current window: %v\n", err) - return 1 - } + return opts.MockReceiver, nil } + return tmux.GetCurrentWindow() +} - // Validate current window exists in tmux session - var receiverExists bool +// windowExists checks if a window exists in the tmux session. +func (opts *ReceiveOptions) windowExists(window string) (bool, error) { if opts.MockWindows != nil { for _, w := range opts.MockWindows { - if w == receiver { - receiverExists = true - break - } - } - } else { - var err error - receiverExists, err = tmux.WindowExists(receiver) - if err != nil { - // FR-004a: Hook mode exits silently on errors - if opts.HookMode { - return 0 + if w == window { + return true, nil } - fmt.Fprintf(stderr, "error: failed to check window: %v\n", err) - return 1 } + return false, nil } + return tmux.WindowExists(window) +} - if !receiverExists { - // FR-004a: Hook mode exits silently on errors - if opts.HookMode { - return 0 - } - fmt.Fprintf(stderr, "error: current window '%s' not found in tmux session\n", receiver) - return 1 - } - - // Determine repository root (find git root, not current directory) - repoRoot := opts.RepoRoot - if repoRoot == "" { - var err error - repoRoot, err = mail.FindGitRoot() - if err != nil { - // FR-004a: Hook mode exits silently on errors - if opts.HookMode { - return 0 - } - fmt.Fprintf(stderr, "error: not in a git repository: %v\n", err) - return 1 - } +// getRepoRoot returns the repository root path. +func (opts *ReceiveOptions) getRepoRoot() (string, error) { + if opts.RepoRoot != "" { + return opts.RepoRoot, nil + } + return mail.FindGitRoot() +} + +// receiveCore contains the core receive logic, returning message or error info. +func (opts *ReceiveOptions) receiveCore() (*mail.Message, *receiveError) { + // Get receiver identity + receiver, err := opts.getReceiver() + if err != nil { + return nil, &receiveError{"failed to get current window: " + err.Error(), 1} + } + + // Validate current window exists + exists, err := opts.windowExists(receiver) + if err != nil { + return nil, &receiveError{"failed to check window: " + err.Error(), 1} + } + if !exists { + return nil, &receiveError{fmt.Sprintf("current window '%s' not found in tmux session", receiver), 1} + } + + // Get repository root + repoRoot, err := opts.getRepoRoot() + if err != nil { + return nil, &receiveError{"not in a git repository: " + err.Error(), 1} } - // T036: Find unread messages for receiver + // Find unread messages unread, err := mail.FindUnread(repoRoot, receiver) if err != nil { - // FR-004a/b/c: Hook mode exits silently on file/lock/corruption errors - if opts.HookMode { - return 0 - } - fmt.Fprintf(stderr, "error: failed to read messages: %v\n", err) - return 1 + return nil, &receiveError{"failed to read messages: " + err.Error(), 1} } - // T037: Handle no unread messages + // No messages case if len(unread) == 0 { - // FR-002: Hook mode exits silently with no messages + return nil, nil + } + + // Get oldest message and mark as read + msg := unread[0] + if err := mail.MarkAsRead(repoRoot, receiver, msg.ID); err != nil { + return nil, &receiveError{"failed to mark message as read: " + err.Error(), 1} + } + + // Update last_read_at timestamp (best-effort, skipped in tests) + if !opts.SkipTimestampUpdate { + _ = mail.UpdateLastReadAt(repoRoot, receiver, time.Now().UnixMilli()) + } + + return &msg, nil +} + +// outputMessage writes the message to the given writer. +func outputMessage(w io.Writer, msg *mail.Message, prefix string) { + if prefix != "" { + fmt.Fprintln(w, prefix) + } + fmt.Fprintf(w, "From: %s\n", msg.From) + fmt.Fprintf(w, "ID: %s\n", msg.ID) + fmt.Fprintln(w) + fmt.Fprint(w, msg.Message) +} + +// Receive implements the agentmail receive command. +// Hook mode: silent on errors/no messages, output to stderr, exit 2 on message. +// Normal mode: verbose errors, output to stdout, exit 0 on success. +func Receive(stdout, stderr io.Writer, opts ReceiveOptions) int { + // Validate tmux environment + if !opts.SkipTmuxCheck && !tmux.InTmux() { if opts.HookMode { return 0 } - fmt.Fprintln(stdout, "No unread messages") - return 0 + fmt.Fprintln(stderr, "error: agentmail must run inside a tmux session") + return 2 } - // Get oldest unread message (FIFO - first in list) - msg := unread[0] + // Execute core logic + msg, recvErr := opts.receiveCore() - // FR-001c: Mark as read - if err := mail.MarkAsRead(repoRoot, receiver, msg.ID); err != nil { - // FR-004a: Hook mode exits silently on errors + // Handle errors + if recvErr != nil { if opts.HookMode { return 0 } - fmt.Fprintf(stderr, "error: failed to mark message as read: %v\n", err) - return 1 + fmt.Fprintf(stderr, "error: %s\n", recvErr.msg) + return recvErr.exitCode } - // FR-017, FR-018: Update last_read_at timestamp when inside tmux - // FR-021: Skip update when outside tmux (SkipTmuxCheck means we're in test mode or outside tmux) - if !opts.SkipTmuxCheck { - // We're inside tmux, update the last-read timestamp - timestamp := time.Now().UnixMilli() // FR-018: Unix timestamp in milliseconds - _ = mail.UpdateLastReadAt(repoRoot, receiver, timestamp) // G104: best-effort, errors don't affect receive + // Handle no messages + if msg == nil { + if opts.HookMode { + return 0 + } + fmt.Fprintln(stdout, "No unread messages") + return 0 } - // FR-005: Hook mode writes all output to STDERR - // FR-001a: Hook mode prefixes with "You got new mail\n" + // Output message if opts.HookMode { - fmt.Fprintln(stderr, "You got new mail") - fmt.Fprintf(stderr, "From: %s\n", msg.From) - fmt.Fprintf(stderr, "ID: %s\n", msg.ID) - fmt.Fprintln(stderr) - fmt.Fprint(stderr, msg.Message) - // FR-001b: Hook mode exits with code 2 when messages exist + outputMessage(stderr, msg, "You got new mail") return 2 } - // Normal mode: Display message to stdout - // Format: - // From: - // ID: - // - // - fmt.Fprintf(stdout, "From: %s\n", msg.From) - fmt.Fprintf(stdout, "ID: %s\n", msg.ID) - fmt.Fprintln(stdout) - fmt.Fprint(stdout, msg.Message) - + outputMessage(stdout, msg, "") return 0 } diff --git a/internal/cli/receive_test.go b/internal/cli/receive_test.go index 190c810..65ef3f4 100644 --- a/internal/cli/receive_test.go +++ b/internal/cli/receive_test.go @@ -26,10 +26,11 @@ func TestReceiveCommand_NoMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -65,10 +66,11 @@ func TestReceiveCommand_AllMessagesRead(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -106,10 +108,11 @@ func TestReceiveCommand_Success(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -165,10 +168,11 @@ func TestReceiveCommand_FIFO_Order(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2", "agent-3"}, - RepoRoot: tmpDir, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2", "agent-3"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -215,10 +219,11 @@ func TestReceiveCommand_CurrentWindowNotInSession(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "orphan-window", - MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list - RepoRoot: tmpDir, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "orphan-window", + MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list + RepoRoot: tmpDir, }) if exitCode != 1 { @@ -255,11 +260,12 @@ func TestReceiveCommand_HookMode_WithMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // FR-001b: Exit code should be 2 @@ -316,11 +322,12 @@ func TestReceiveCommand_HookMode_NoMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // FR-002: Exit code should be 0 @@ -378,11 +385,12 @@ func TestReceiveCommand_HookMode_WindowNotInSession(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "orphan-window", - MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list - RepoRoot: tmpDir, - HookMode: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "orphan-window", + MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list + RepoRoot: tmpDir, + HookMode: true, }) // FR-004a: Exit code should be 0 (silent error) @@ -422,11 +430,12 @@ func TestReceiveCommand_HookMode_AllMessagesRead(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // Exit code should be 0 (no unread messages) @@ -463,11 +472,12 @@ func TestReceiveCommand_NormalMode_Unchanged(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: false, // Explicitly false + SkipTmuxCheck: true, + SkipTimestampUpdate: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: false, // Explicitly false }) // Normal mode should exit 0 with message diff --git a/internal/cli/send.go b/internal/cli/send.go index aaf7463..39d01a9 100644 --- a/internal/cli/send.go +++ b/internal/cli/send.go @@ -22,40 +22,18 @@ type SendOptions struct { StdinIsPipe bool // Mock whether stdin is a pipe } -// Send implements the agentmail send command. -// T020: Implement Send command structure -// T021: Add tmux validation (exit code 2 if not in tmux) -// T022: Add recipient validation (check WindowExists) -// T023: Add message storage and ID output -// T045: Accept io.Reader for stdin -func Send(args []string, stdin io.Reader, stdout, stderr io.Writer, opts SendOptions) int { - // T021: Validate running inside tmux - if !opts.SkipTmuxCheck { - if !tmux.InTmux() { - fmt.Fprintln(stderr, "error: agentmail must run inside a tmux session") - return 2 - } - } - - // Validate recipient argument is provided - if len(args) == 0 { - fmt.Fprintln(stderr, "error: missing required arguments: recipient message") - return 1 - } - - recipient := args[0] - - // T046-T048: Get message from stdin or argument +// extractMessage reads message from stdin or args. +// Returns the message content and any error encountered. +func (opts *SendOptions) extractMessage(args []string, stdin io.Reader) (string, error) { var message string // Check if stdin is a pipe isStdinPipe := opts.StdinIsPipe - if !opts.SkipTmuxCheck { // If not mocking, use real detection + if !opts.SkipTmuxCheck { isStdinPipe = IsStdinPipe() } if isStdinPipe { - // T047: Read from stdin (use mock or real) var stdinContent []byte if opts.StdinContent != "" { stdinContent = []byte(opts.StdinContent) @@ -63,113 +41,141 @@ func Send(args []string, stdin io.Reader, stdout, stderr io.Writer, opts SendOpt var err error stdinContent, err = io.ReadAll(stdin) if err != nil { - fmt.Fprintf(stderr, "error: failed to read stdin: %v\n", err) - return 1 + return "", fmt.Errorf("failed to read stdin: %w", err) } } if len(stdinContent) > 0 { - // Trim trailing newline only (preserve internal newlines for multi-line messages) message = strings.TrimSuffix(string(stdinContent), "\n") } } - // T048: Fall back to argument if no stdin content + // Fall back to argument if no stdin content if message == "" && len(args) >= 2 { message = args[1] } - // Error if no message provided - if message == "" { - fmt.Fprintln(stderr, "error: no message provided") - fmt.Fprintln(stderr, "usage: agentmail send ") - return 1 - } + return message, nil +} - // Get sender identity - var sender string +// getSender returns the sender window name. +func (opts *SendOptions) getSender() (string, error) { if opts.MockSender != "" { - sender = opts.MockSender - } else { - var err error - sender, err = tmux.GetCurrentWindow() - if err != nil { - fmt.Fprintf(stderr, "error: failed to get current window: %v\n", err) - return 1 - } + return opts.MockSender, nil } + return tmux.GetCurrentWindow() +} - // T022: Validate recipient exists - var recipientExists bool +// windowExists checks if a window exists in the tmux session. +func (opts *SendOptions) windowExists(window string) (bool, error) { if opts.MockWindows != nil { for _, w := range opts.MockWindows { - if w == recipient { - recipientExists = true - break + if w == window { + return true, nil } } - } else { - var err error - recipientExists, err = tmux.WindowExists(recipient) - if err != nil { - fmt.Fprintf(stderr, "error: failed to check recipient: %v\n", err) - return 1 - } + return false, nil } + return tmux.WindowExists(window) +} +// loadIgnoreList loads the ignore list from file or returns mock. +func (opts *SendOptions) loadIgnoreList() map[string]bool { + if opts.MockIgnoreList != nil { + return opts.MockIgnoreList + } + + gitRoot := opts.MockGitRoot + if gitRoot == "" { + gitRoot, _ = mail.FindGitRoot() + } + if gitRoot == "" { + return nil + } + + ignoreList, _ := mail.LoadIgnoreList(gitRoot) + return ignoreList +} + +// getRepoRoot returns the repository root path. +func (opts *SendOptions) getRepoRoot() (string, error) { + if opts.RepoRoot != "" { + return opts.RepoRoot, nil + } + return mail.FindGitRoot() +} + +// Send implements the agentmail send command. +func Send(args []string, stdin io.Reader, stdout, stderr io.Writer, opts SendOptions) int { + // Validate running inside tmux + if !opts.SkipTmuxCheck && !tmux.InTmux() { + fmt.Fprintln(stderr, "error: agentmail must run inside a tmux session") + return 2 + } + + // Validate recipient argument is provided + if len(args) == 0 { + fmt.Fprintln(stderr, "error: missing required arguments: recipient message") + return 1 + } + + recipient := args[0] + + // Extract message from stdin or argument + message, err := opts.extractMessage(args, stdin) + if err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return 1 + } + if message == "" { + fmt.Fprintln(stderr, "error: no message provided") + fmt.Fprintln(stderr, "usage: agentmail send ") + return 1 + } + + // Get sender identity + sender, err := opts.getSender() + if err != nil { + fmt.Fprintf(stderr, "error: failed to get current window: %v\n", err) + return 1 + } + + // Validate recipient exists + recipientExists, err := opts.windowExists(recipient) + if err != nil { + fmt.Fprintf(stderr, "error: failed to check recipient: %v\n", err) + return 1 + } if !recipientExists { fmt.Fprintln(stderr, "error: recipient not found") return 1 } - // T029: Check if recipient is the sender (self-send not allowed) + // Check self-send if recipient == sender { - fmt.Fprintln(stderr, "error: recipient not found") + fmt.Fprintln(stderr, "error: cannot send message to self") return 1 } - // T029: Load and check ignore list - var ignoreList map[string]bool - if opts.MockIgnoreList != nil { - ignoreList = opts.MockIgnoreList - } else { - // Load from .agentmailignore file - var gitRoot string - if opts.MockGitRoot != "" { - gitRoot = opts.MockGitRoot - } else { - gitRoot, _ = mail.FindGitRoot() - // Errors from FindGitRoot mean not in a git repo - proceed without ignore list - } - if gitRoot != "" { - ignoreList, _ = mail.LoadIgnoreList(gitRoot) - // Errors from LoadIgnoreList are treated as no ignore file - } + // Check ignore list + if ignoreList := opts.loadIgnoreList(); ignoreList != nil && ignoreList[recipient] { + fmt.Fprintln(stderr, "error: recipient not found") + return 1 } - // T030: Check if recipient is in ignore list - if ignoreList != nil && ignoreList[recipient] { - fmt.Fprintln(stderr, "error: recipient not found") + // Get repository root + repoRoot, err := opts.getRepoRoot() + if err != nil { + fmt.Fprintf(stderr, "error: not in a git repository: %v\n", err) return 1 } - // Generate message ID + // Generate message ID and store id, err := mail.GenerateID() if err != nil { fmt.Fprintf(stderr, "error: failed to generate message ID: %v\n", err) return 1 } - // Determine repository root (find git root, not current directory) - repoRoot := opts.RepoRoot - if repoRoot == "" { - repoRoot, err = mail.FindGitRoot() - if err != nil { - fmt.Fprintf(stderr, "error: not in a git repository: %v\n", err) - return 1 - } - } - - // T023: Store message msg := mail.Message{ ID: id, From: sender, @@ -183,7 +189,6 @@ func Send(args []string, stdin io.Reader, stdout, stderr io.Writer, opts SendOpt return 1 } - // Output message confirmation fmt.Fprintf(stdout, "Message #%s sent\n", id) return 0 } diff --git a/internal/cli/send_test.go b/internal/cli/send_test.go index c1d1a46..aee2e36 100644 --- a/internal/cli/send_test.go +++ b/internal/cli/send_test.go @@ -252,8 +252,8 @@ func TestSendCommand_SendToSelf(t *testing.T) { } stderrStr := stderr.String() - if stderrStr != "error: recipient not found\n" { - t.Errorf("Expected 'error: recipient not found\\n', got: %q", stderrStr) + if stderrStr != "error: cannot send message to self\n" { + t.Errorf("Expected 'error: cannot send message to self\\n', got: %q", stderrStr) } if stdout.String() != "" { diff --git a/internal/daemon/loop.go b/internal/daemon/loop.go index be2ebd5..5ce2fa9 100644 --- a/internal/daemon/loop.go +++ b/internal/daemon/loop.go @@ -130,169 +130,175 @@ func CheckAndNotify(opts LoopOptions) error { return CheckAndNotifyWithNotifier(opts, NotifyAgent, tmux.WindowExists) } -// CheckAndNotifyWithNotifier performs a single notification cycle with a custom notifier. -// This allows for testing without actual tmux calls. -// When notify is non-nil, it will be called for each agent that should be notified. -// When windowChecker is non-nil, it will be used to verify window existence before notifying. -// The function handles two types of agents: -// - Phase 1: Stated agents (with recipient state in recipients.jsonl) -// - Phase 2: Stateless agents (mailbox but no recipient state) -func CheckAndNotifyWithNotifier(opts LoopOptions, notify NotifyFunc, windowChecker WindowCheckerFunc) error { - opts.log("Starting notification cycle") - - // ========================================================================= - // Phase 1: Stated agents (existing logic) - // ========================================================================= - - // Read all recipient states +// notifyStatedAgents processes agents with recipient state in recipients.jsonl. +// Returns the set of stated agent names for Phase 2 exclusion. +func notifyStatedAgents(opts LoopOptions, notify NotifyFunc) (map[string]struct{}, error) { recipients, err := mail.ReadAllRecipients(opts.RepoRoot) if err != nil { opts.log("Error reading recipients: %v", err) - return err + return nil, err } opts.log("Found %d stated agents", len(recipients)) - // T018: Build statedSet from recipients for Phase 2 lookup (FR-002) statedSet := make(map[string]struct{}, len(recipients)) for _, r := range recipients { statedSet[r.Recipient] = struct{}{} } - // Check each recipient for _, recipient := range recipients { - // Skip non-ready agents (work/offline have 1 hour protection) - if recipient.Status != mail.StatusReady { - if !recipient.ShouldNotify() { - opts.log("Skipping stated agent %q: status=%s, protected for 1h", recipient.Recipient, recipient.Status) - } else { - opts.log("Skipping stated agent %q: status=%s (not ready)", recipient.Recipient, recipient.Status) - } - continue - } - // Ready agents: check 60s debounce - if !recipient.ShouldNotify() { - opts.log("Skipping stated agent %q: notified within last 60s", recipient.Recipient) - continue - } + processStatedAgent(opts, notify, recipient) + } - // Check for unread messages - unread, err := mail.FindUnread(opts.RepoRoot, recipient.Recipient) - if err != nil { - opts.log("Error reading mailbox for stated agent %q: %v", recipient.Recipient, err) - continue - } + return statedSet, nil +} - if len(unread) == 0 { - opts.log("Skipping stated agent %q: no unread messages", recipient.Recipient) - continue +// processStatedAgent handles notification logic for a single stated agent. +func processStatedAgent(opts LoopOptions, notify NotifyFunc, recipient mail.RecipientState) { + // Skip non-ready agents + if recipient.Status != mail.StatusReady { + if recipient.IsProtected() { + opts.log("Skipping stated agent %q: status=%s, protected for 1h", recipient.Recipient, recipient.Status) + } else { + opts.log("Skipping stated agent %q: status=%s (not ready)", recipient.Recipient, recipient.Status) } + return + } - opts.log("Stated agent %q has %d unread message(s)", recipient.Recipient, len(unread)) + // Check 60s debounce for ready agents + if !recipient.ShouldNotify() { + opts.log("Skipping stated agent %q: notified within last 60s", recipient.Recipient) + return + } - // Send notification - if notify != nil { - opts.log("Notifying stated agent %q", recipient.Recipient) - if err := notify(recipient.Recipient); err != nil { - opts.log("Notification failed for stated agent %q: %v", recipient.Recipient, err) - continue - } - opts.log("Notification sent to stated agent %q", recipient.Recipient) - } + // Check for unread messages + unread, err := mail.FindUnread(opts.RepoRoot, recipient.Recipient) + if err != nil { + opts.log("Error reading mailbox for stated agent %q: %v", recipient.Recipient, err) + return + } + if len(unread) == 0 { + opts.log("Skipping stated agent %q: no unread messages", recipient.Recipient) + return + } - // Update notified flag - if err := mail.SetNotifiedFlag(opts.RepoRoot, recipient.Recipient, true); err != nil { - opts.log("Error setting notified flag for %q: %v", recipient.Recipient, err) - continue + opts.log("Stated agent %q has %d unread message(s)", recipient.Recipient, len(unread)) + + // Send notification + if notify != nil { + opts.log("Notifying stated agent %q", recipient.Recipient) + if err := notify(recipient.Recipient); err != nil { + opts.log("Notification failed for stated agent %q: %v", recipient.Recipient, err) + return } - opts.log("Marked stated agent %q as notified", recipient.Recipient) + opts.log("Notification sent to stated agent %q", recipient.Recipient) } - // ========================================================================= - // Phase 2: Stateless agents (T019-T024) - // ========================================================================= + // Update notified flag + if err := mail.SetNotifiedFlag(opts.RepoRoot, recipient.Recipient, true); err != nil { + opts.log("Error setting notified flag for %q: %v", recipient.Recipient, err) + return + } + opts.log("Marked stated agent %q as notified", recipient.Recipient) +} - // Skip Phase 2 if no tracker is configured +// notifyStatelessAgents processes agents with mailboxes but no recipient state. +func notifyStatelessAgents(opts LoopOptions, notify NotifyFunc, windowChecker WindowCheckerFunc, statedSet map[string]struct{}) { if opts.StatelessTracker == nil { opts.log("Stateless tracking disabled, skipping Phase 2") - return nil + return } - // T019: Get all mailbox recipients (FR-001) mailboxRecipients, err := mail.ListMailboxRecipients(opts.RepoRoot) if err != nil { opts.log("Error listing mailbox recipients: %v", err) - return nil + return } opts.log("Found %d mailbox recipients, checking for stateless agents", len(mailboxRecipients)) - // T020-T024: Process each stateless agent statelessCount := 0 - for _, mailboxRecipient := range mailboxRecipients { - // T020: Skip agents that have recipient state (FR-003) - if _, isStated := statedSet[mailboxRecipient]; isStated { + for _, agent := range mailboxRecipients { + if _, isStated := statedSet[agent]; isStated { continue } statelessCount++ + processStatelessAgent(opts, notify, windowChecker, agent) + } + + opts.log("Found %d stateless agents", statelessCount) - // T021: Check for unread messages (FR-006) - unread, err := mail.FindUnread(opts.RepoRoot, mailboxRecipient) + opts.StatelessTracker.Cleanup(mailboxRecipients) + opts.log("Cleaned up stale entries from stateless tracker") +} + +// processStatelessAgent handles notification logic for a single stateless agent. +func processStatelessAgent(opts LoopOptions, notify NotifyFunc, windowChecker WindowCheckerFunc, agent string) { + // Check for unread messages + unread, err := mail.FindUnread(opts.RepoRoot, agent) + if err != nil { + opts.log("Error reading mailbox for stateless agent %q: %v", agent, err) + return + } + if len(unread) == 0 { + opts.log("Skipping stateless agent %q: no unread messages", agent) + return + } + + opts.log("Stateless agent %q has %d unread message(s)", agent, len(unread)) + + // Check if notification is due + if !opts.StatelessTracker.ShouldNotify(agent) { + opts.log("Skipping stateless agent %q: interval not elapsed", agent) + return + } + + // Verify window exists + if windowChecker != nil { + exists, err := windowChecker(agent) if err != nil { - opts.log("Error reading mailbox for stateless agent %q: %v", mailboxRecipient, err) - continue + opts.log("Error checking window existence for %q: %v", agent, err) + return } - if len(unread) == 0 { - opts.log("Skipping stateless agent %q: no unread messages", mailboxRecipient) - continue + if !exists { + opts.log("Skipping stateless agent %q: window does not exist", agent) + opts.StatelessTracker.MarkNotified(agent) + return } + } - opts.log("Stateless agent %q has %d unread message(s)", mailboxRecipient, len(unread)) - - // T022: Check if notification is due (FR-004, FR-005) - if !opts.StatelessTracker.ShouldNotify(mailboxRecipient) { - opts.log("Skipping stateless agent %q: interval not elapsed", mailboxRecipient) - continue + // Send notification + if notify != nil { + opts.log("Notifying stateless agent %q", agent) + if err := notify(agent); err != nil { + opts.log("Notification failed for stateless agent %q: %v", agent, err) + opts.StatelessTracker.MarkNotified(agent) + opts.log("Marked stateless agent %q in tracker (after failure)", agent) + return } + opts.log("Notification sent to stateless agent %q", agent) + } - // Check if window exists before attempting notification - if windowChecker != nil { - exists, err := windowChecker(mailboxRecipient) - if err != nil { - opts.log("Error checking window existence for %q: %v", mailboxRecipient, err) - continue - } - if !exists { - opts.log("Skipping stateless agent %q: window does not exist", mailboxRecipient) - // Mark as notified to rate-limit window existence checks - opts.StatelessTracker.MarkNotified(mailboxRecipient) - continue - } - } + opts.StatelessTracker.MarkNotified(agent) + opts.log("Marked stateless agent %q in tracker", agent) +} - // Send notification - if notify != nil { - opts.log("Notifying stateless agent %q", mailboxRecipient) - if err := notify(mailboxRecipient); err != nil { - opts.log("Notification failed for stateless agent %q: %v", mailboxRecipient, err) - // Mark as notified even on failure to rate-limit retries - opts.StatelessTracker.MarkNotified(mailboxRecipient) - opts.log("Marked stateless agent %q in tracker (after failure)", mailboxRecipient) - continue - } - opts.log("Notification sent to stateless agent %q", mailboxRecipient) - } +// CheckAndNotifyWithNotifier performs a single notification cycle with a custom notifier. +// This allows for testing without actual tmux calls. +// The function handles two types of agents: +// - Phase 1: Stated agents (with recipient state in recipients.jsonl) +// - Phase 2: Stateless agents (mailbox but no recipient state) +func CheckAndNotifyWithNotifier(opts LoopOptions, notify NotifyFunc, windowChecker WindowCheckerFunc) error { + opts.log("Starting notification cycle") - // T023: Mark as notified - opts.StatelessTracker.MarkNotified(mailboxRecipient) - opts.log("Marked stateless agent %q in tracker", mailboxRecipient) + // Phase 1: Stated agents + statedSet, err := notifyStatedAgents(opts, notify) + if err != nil { + return err } - opts.log("Found %d stateless agents", statelessCount) - - // T024: Cleanup tracker with current mailbox list (FR-011) - opts.StatelessTracker.Cleanup(mailboxRecipients) - opts.log("Cleaned up stale entries from stateless tracker") + // Phase 2: Stateless agents + notifyStatelessAgents(opts, notify, windowChecker, statedSet) opts.log("Notification cycle complete") return nil diff --git a/internal/mail/recipients.go b/internal/mail/recipients.go index 2520b93..b78b937 100644 --- a/internal/mail/recipients.go +++ b/internal/mail/recipients.go @@ -35,15 +35,18 @@ type RecipientState struct { LastReadAt int64 `json:"last_read_at,omitempty"` // Unix timestamp in milliseconds when agent last called receive } -// ShouldNotify returns true if notification is allowed (debounce elapsed or never notified). -// For work/offline agents, applies WorkProtectionInterval (1 hour) instead of NotifyDebounceInterval. -func (r *RecipientState) ShouldNotify() bool { - // Work/offline agents have a longer protection interval (1 hour since status change) - if r.Status == StatusWork || r.Status == StatusOffline { - return time.Since(r.UpdatedAt) >= WorkProtectionInterval +// IsProtected returns true if the agent is in a protected state (work/offline within 1h). +// Protected agents should not receive notifications regardless of debounce status. +func (r *RecipientState) IsProtected() bool { + if r.Status != StatusWork && r.Status != StatusOffline { + return false } + return time.Since(r.UpdatedAt) < WorkProtectionInterval +} - // Ready agents use the standard 60s debounce based on last notification +// ShouldNotify returns true if the 60s debounce has elapsed since last notification. +// This only applies to ready agents; for work/offline agents, use IsProtected() first. +func (r *RecipientState) ShouldNotify() bool { if r.NotifiedAt.IsZero() { return true } diff --git a/internal/mcp/handlers.go b/internal/mcp/handlers.go index 93b07f4..df35b6a 100644 --- a/internal/mcp/handlers.go +++ b/internal/mcp/handlers.go @@ -28,6 +28,67 @@ type HandlerOptions struct { RepoRoot string } +// getSender returns the sender window name. +func (opts *HandlerOptions) getSender() (string, error) { + if opts.MockSender != "" { + return opts.MockSender, nil + } + return tmux.GetCurrentWindow() +} + +// getReceiver returns the receiver window name. +func (opts *HandlerOptions) getReceiver() (string, error) { + if opts.MockReceiver != "" { + return opts.MockReceiver, nil + } + return tmux.GetCurrentWindow() +} + +// windowExists checks if a window exists. +func (opts *HandlerOptions) windowExists(window string) (bool, error) { + if opts.MockWindows != nil { + for _, w := range opts.MockWindows { + if w == window { + return true, nil + } + } + return false, nil + } + return tmux.WindowExists(window) +} + +// listWindows returns all tmux windows. +func (opts *HandlerOptions) listWindows() ([]string, error) { + if opts.MockWindows != nil { + return opts.MockWindows, nil + } + return tmux.ListWindows() +} + +// loadIgnoreList loads the ignore list. +func (opts *HandlerOptions) loadIgnoreList() map[string]bool { + if opts.MockIgnoreList != nil { + return opts.MockIgnoreList + } + gitRoot := opts.RepoRoot + if gitRoot == "" { + gitRoot, _ = mail.FindGitRoot() + } + if gitRoot == "" { + return nil + } + ignoreList, _ := mail.LoadIgnoreList(gitRoot) + return ignoreList +} + +// getRepoRoot returns the repository root. +func (opts *HandlerOptions) getRepoRoot() (string, error) { + if opts.RepoRoot != "" { + return opts.RepoRoot, nil + } + return mail.FindGitRoot() +} + // handlerOptions holds the current handler options. // Set via SetHandlerOptions for testing, nil for production. // Protected by handlerOptionsMu for thread-safe access. @@ -92,93 +153,51 @@ func doSend(ctx context.Context, recipient, message string) (any, error) { opts = &HandlerOptions{} } - // Validate message is not empty + // Validate message if message == "" { return nil, fmt.Errorf("no message provided") } - - // FR-013: Validate message size (64KB limit) if len(message) > MaxMessageSize { return nil, fmt.Errorf("message exceeds maximum size of 64KB") } // Get sender identity - var sender string - if opts.MockSender != "" { - sender = opts.MockSender - } else { - var err error - sender, err = tmux.GetCurrentWindow() - if err != nil { - return nil, fmt.Errorf("failed to get current window: %w", err) - } + sender, err := opts.getSender() + if err != nil { + return nil, fmt.Errorf("failed to get current window: %w", err) } - // FR-009: Validate recipient exists - var recipientExists bool - if opts.MockWindows != nil { - for _, w := range opts.MockWindows { - if w == recipient { - recipientExists = true - break - } - } - } else { - var err error - recipientExists, err = tmux.WindowExists(recipient) - if err != nil { - return nil, fmt.Errorf("failed to check recipient: %w", err) - } + // Validate recipient exists + exists, err := opts.windowExists(recipient) + if err != nil { + return nil, fmt.Errorf("failed to check recipient: %w", err) } - - if !recipientExists { + if !exists { return nil, fmt.Errorf("recipient not found") } - // Check if sending to self (not allowed) + // Check self-send if recipient == sender { return nil, fmt.Errorf("cannot send message to self") } - // Load and check ignore list - var ignoreList map[string]bool - if opts.MockIgnoreList != nil { - ignoreList = opts.MockIgnoreList - } else { - // Determine git root for loading ignore list. - // Errors are intentionally ignored: if we can't find git root or load - // the ignore list, we proceed without filtering - this is acceptable - // as the ignore list is optional. - gitRoot := opts.RepoRoot - if gitRoot == "" { - gitRoot, _ = mail.FindGitRoot() // Error ignored: proceed without ignore list - } - if gitRoot != "" { - ignoreList, _ = mail.LoadIgnoreList(gitRoot) // Error ignored: proceed without ignore list - } + // Check ignore list + if ignoreList := opts.loadIgnoreList(); ignoreList != nil && ignoreList[recipient] { + return nil, fmt.Errorf("recipient not found") } - // Check if recipient is in ignore list - if ignoreList != nil && ignoreList[recipient] { - return nil, fmt.Errorf("recipient not found") + // Get repository root + repoRoot, err := opts.getRepoRoot() + if err != nil { + return nil, fmt.Errorf("not in a git repository: %w", err) } - // Generate message ID + // Generate ID and store message id, err := mail.GenerateID() if err != nil { return nil, fmt.Errorf("failed to generate message ID: %w", err) } - // Determine repository root - repoRoot := opts.RepoRoot - if repoRoot == "" { - repoRoot, err = mail.FindGitRoot() - if err != nil { - return nil, fmt.Errorf("not in a git repository: %w", err) - } - } - - // Store message msg := mail.Message{ ID: id, From: sender, @@ -191,10 +210,7 @@ func doSend(ctx context.Context, recipient, message string) (any, error) { return nil, fmt.Errorf("failed to write message: %w", err) } - // FR-004: Return response with message_id - return SendResponse{ - MessageID: id, - }, nil + return SendResponse{MessageID: id}, nil } // sendParams holds the unmarshaled parameters for the send tool. @@ -256,54 +272,33 @@ func doReceive(ctx context.Context) (any, error) { } // Get receiver identity - var receiver string - if opts.MockReceiver != "" { - receiver = opts.MockReceiver - } else { - var err error - receiver, err = tmux.GetCurrentWindow() - if err != nil { - return nil, fmt.Errorf("failed to get current window: %w", err) - } + receiver, err := opts.getReceiver() + if err != nil { + return nil, fmt.Errorf("failed to get current window: %w", err) } - // Determine repository root - repoRoot := opts.RepoRoot - if repoRoot == "" { - var err error - repoRoot, err = mail.FindGitRoot() - if err != nil { - return nil, fmt.Errorf("not in a git repository: %w", err) - } + // Get repository root + repoRoot, err := opts.getRepoRoot() + if err != nil { + return nil, fmt.Errorf("not in a git repository: %w", err) } - // Find unread messages for receiver (FR-003: FIFO order) + // Find unread messages (FIFO order) unread, err := mail.FindUnread(repoRoot, receiver) if err != nil { return nil, fmt.Errorf("failed to read messages: %w", err) } - - // FR-008: Handle no unread messages if len(unread) == 0 { - return ReceiveEmptyResponse{ - Status: "No unread messages", - }, nil + return ReceiveEmptyResponse{Status: "No unread messages"}, nil } - // Get oldest unread message (FIFO - first in list) per FR-003 + // Get oldest and mark as read msg := unread[0] - - // FR-012: Mark as read if err := mail.MarkAsRead(repoRoot, receiver, msg.ID); err != nil { return nil, fmt.Errorf("failed to mark message as read: %w", err) } - // Return response with from, id, message fields per data-model.md - return ReceiveResponse{ - From: msg.From, - ID: msg.ID, - Message: msg.Message, - }, nil + return ReceiveResponse{From: msg.From, ID: msg.ID, Message: msg.Message}, nil } // handleReceive is the MCP handler function for the receive tool. @@ -358,46 +353,30 @@ func doStatus(ctx context.Context, status string) (any, error) { opts = &HandlerOptions{} } - // T039: Validate status value (ready/work/offline only) + // Validate status if !validateStatus(status) { - // T040: Return error message matching FR-016 format return nil, fmt.Errorf("Invalid status: %s. Valid: ready, work, offline", status) } - // Get agent identity (current tmux window) - var agent string - if opts.MockReceiver != "" { - agent = opts.MockReceiver - } else { - var err error - agent, err = tmux.GetCurrentWindow() - if err != nil { - return nil, fmt.Errorf("failed to get current window: %w", err) - } + // Get agent identity + agent, err := opts.getReceiver() + if err != nil { + return nil, fmt.Errorf("failed to get current window: %w", err) } - // Determine repository root - repoRoot := opts.RepoRoot - if repoRoot == "" { - var err error - repoRoot, err = mail.FindGitRoot() - if err != nil { - return nil, fmt.Errorf("not in a git repository: %w", err) - } + // Get repository root + repoRoot, err := opts.getRepoRoot() + if err != nil { + return nil, fmt.Errorf("not in a git repository: %w", err) } - // Reset notified flag when status is work or offline - resetNotified := (status == mail.StatusWork || status == mail.StatusOffline) - - // Update recipient state using existing mail infrastructure + // Update state (reset notified for work/offline) + resetNotified := status == mail.StatusWork || status == mail.StatusOffline if err := mail.UpdateRecipientState(repoRoot, agent, status, resetNotified); err != nil { return nil, fmt.Errorf("failed to update status: %w", err) } - // T041: Return {"status": "ok"} on success - return StatusResponse{ - Status: "ok", - }, nil + return StatusResponse{Status: "ok"}, nil } // statusParams holds the unmarshaled parameters for the status tool. @@ -458,69 +437,30 @@ func doListRecipients(ctx context.Context) (any, error) { opts = &HandlerOptions{} } - // Get current window (agent identity) - var currentWindow string - if opts.MockReceiver != "" { - currentWindow = opts.MockReceiver - } else { - var err error - currentWindow, err = tmux.GetCurrentWindow() - if err != nil { - return nil, fmt.Errorf("failed to get current window: %w", err) - } - } - - // Get list of all windows - var windows []string - if opts.MockWindows != nil { - windows = opts.MockWindows - } else { - var err error - windows, err = tmux.ListWindows() - if err != nil { - return nil, fmt.Errorf("failed to list windows: %w", err) - } + // Get current window + currentWindow, err := opts.getReceiver() + if err != nil { + return nil, fmt.Errorf("failed to get current window: %w", err) } - // Load ignore list - var ignoreList map[string]bool - if opts.MockIgnoreList != nil { - ignoreList = opts.MockIgnoreList - } else { - // Determine git root for loading ignore list. - // Errors are intentionally ignored: if we can't find git root or load - // the ignore list, we proceed without filtering - this is acceptable - // as the ignore list is optional. - gitRoot := opts.RepoRoot - if gitRoot == "" { - gitRoot, _ = mail.FindGitRoot() // Error ignored: proceed without ignore list - } - if gitRoot != "" { - ignoreList, _ = mail.LoadIgnoreList(gitRoot) // Error ignored: proceed without ignore list - } + // Get all windows + windows, err := opts.listWindows() + if err != nil { + return nil, fmt.Errorf("failed to list windows: %w", err) } - // Build recipients list, filtering ignored windows but always including current - recipients := []RecipientInfo{} + // Build filtered recipients list + ignoreList := opts.loadIgnoreList() + recipients := make([]RecipientInfo, 0, len(windows)) for _, window := range windows { - // Current window is always shown (even if in ignore list) if window == currentWindow { - recipients = append(recipients, RecipientInfo{ - Name: window, - IsCurrent: true, - }) + recipients = append(recipients, RecipientInfo{Name: window, IsCurrent: true}) } else if ignoreList == nil || !ignoreList[window] { - // Only show non-current windows if they're not in the ignore list - recipients = append(recipients, RecipientInfo{ - Name: window, - IsCurrent: false, - }) + recipients = append(recipients, RecipientInfo{Name: window, IsCurrent: false}) } } - return ListRecipientsResponse{ - Recipients: recipients, - }, nil + return ListRecipientsResponse{Recipients: recipients}, nil } // handleListRecipients is the MCP handler function for the list-recipients tool.