From 7e3370c95ca22f0e41a20f23044160721fe88444 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 18:54:06 +0000 Subject: [PATCH 1/7] docs: add gocyclo cyclomatic complexity analysis Run gocyclo on the codebase and document findings with recommendations: - 5 functions exceed complexity 20 (critical threshold) - Main issues: mock infrastructure, hook mode branching, two-phase processing - Recommendations include extracting helpers, interface-based testing, and splitting large functions Top offenders: - cli.Send (30): Multiple validation + mock support - cli.Receive (25): Hook mode conditional branches - daemon.CheckAndNotifyWithNotifier (24): Two-phase agent processing - mcp.doSend (21): Duplicates cli.Send validation --- GOCYCLO_ANALYSIS.md | 239 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 239 insertions(+) create mode 100644 GOCYCLO_ANALYSIS.md diff --git a/GOCYCLO_ANALYSIS.md b/GOCYCLO_ANALYSIS.md new file mode 100644 index 0000000..1d9fdf8 --- /dev/null +++ b/GOCYCLO_ANALYSIS.md @@ -0,0 +1,239 @@ +# Gocyclo Cyclomatic Complexity Analysis + +**Generated:** 2026-01-15 +**Average complexity:** 6.0 +**Tool:** gocyclo v0.6.0 + +## Summary + +The codebase has an average cyclomatic complexity of ~6, which is acceptable. However, several functions exceed recommended thresholds and should be considered for refactoring. + +### Complexity Thresholds +- **1-5:** Simple, low risk +- **6-10:** Moderate complexity +- **11-15:** High complexity - consider refactoring +- **16-20:** Very high complexity - should refactor +- **21+:** Critical - needs immediate attention + +## High-Priority Functions (Complexity > 20) + +| Complexity | Function | File | Line | +|------------|----------|------|------| +| 30 | `cli.Send` | internal/cli/send.go | 31 | +| 25 | `cli.Receive` | internal/cli/receive.go | 34 | +| 24 | `daemon.CheckAndNotifyWithNotifier` | internal/daemon/loop.go | 140 | +| 22 | `mcp.TestStatusTool_SchemaValidation` | internal/mcp/tools_test.go | 251 | +| 21 | `mcp.doSend` | internal/mcp/handlers.go | 89 | + +## Medium-Priority Functions (Complexity 15-20) + +| Complexity | Function | File | Line | +|------------|----------|------|------| +| 19 | `mcp.TestSendTool_SchemaValidation` | internal/mcp/tools_test.go | 127 | +| 18 | `main.main` | cmd/agentmail/main.go | 16 | +| 16 | `mcp.TestServer_100ConsecutiveInvocations` | internal/mcp/handlers_test.go | 2736 | +| 16 | `mail.TestUpdateRecipientState_UpdateExisting` | internal/mail/recipients_test.go | 322 | +| 15 | `mail.TestUpdateLastReadAt_PreservesOtherRecipients` | internal/mail/recipients_test.go | 1061 | + +## Root Cause Analysis + +### 1. Mock/Test Infrastructure in Production Code + +Both `cli.Send` and `cli.Receive` contain significant complexity from mock support: + +```go +// Pattern repeating throughout: +if opts.MockWindows != nil { + for _, w := range opts.MockWindows { + if w == recipient { + recipientExists = true + break + } + } +} else { + var err error + recipientExists, err = tmux.WindowExists(recipient) + // ... +} +``` + +**Impact:** Each mock option adds 2-3 branches. + +### 2. Hook Mode Conditional Logic + +`cli.Receive` has complexity from hook mode which adds early-exit branches throughout: + +```go +if opts.HookMode { + return 0 // Silent exit +} +// Normal error handling path +``` + +**Impact:** Hook mode adds ~8 additional branches. + +### 3. Two-Phase Processing + +`daemon.CheckAndNotifyWithNotifier` processes two types of agents (stated and stateless), essentially doubling the code complexity: +- Phase 1: Loop through stated agents with multiple conditions +- Phase 2: Loop through stateless agents with similar conditions + +### 4. Duplicated Validation Logic + +`cli.Send` and `mcp.doSend` share nearly identical validation logic but are implemented separately, both with mock support. + +## Recommendations + +### Immediate (Complexity > 20) + +#### 1. Extract Validation Logic into Helper Functions + +Create a shared validation package or helper functions: + +```go +// internal/validation/recipient.go +type RecipientValidator struct { + Windows []string // nil means use real tmux + IgnoreList map[string]bool +} + +func (v *RecipientValidator) Exists(recipient string) (bool, error) +func (v *RecipientValidator) IsIgnored(recipient string) bool +func (v *RecipientValidator) IsSelf(recipient, sender string) bool +``` + +**Expected reduction:** cli.Send 30 → ~18, mcp.doSend 21 → ~12 + +#### 2. Split cli.Send Into Smaller Functions + +```go +func Send(...) int { + if err := validateTmux(opts); err != nil { ... } + + msg, err := parseMessage(args, stdin, opts) + if err != nil { ... } + + sender, err := getSender(opts) + if err != nil { ... } + + if err := validateRecipient(recipient, sender, opts); err != nil { ... } + + return storeAndRespond(msg, stdout, stderr, opts) +} +``` + +**Expected reduction:** 30 → ~10 + +#### 3. Separate Hook Mode Into Different Function + +```go +func Receive(stdout, stderr io.Writer, opts ReceiveOptions) int { + if opts.HookMode { + return receiveHookMode(stderr, opts) + } + return receiveNormalMode(stdout, stderr, opts) +} +``` + +**Expected reduction:** 25 → ~12 each + +#### 4. Refactor CheckAndNotifyWithNotifier + +Split into two functions: + +```go +func CheckAndNotifyWithNotifier(opts LoopOptions, ...) error { + if err := notifyStatedAgents(opts, notify, windowChecker); err != nil { + return err + } + return notifyStatelessAgents(opts, notify, windowChecker) +} +``` + +**Expected reduction:** 24 → ~12 each + +### Medium-Term + +#### 5. Interface-Based Testing + +Replace mock structs with interfaces to reduce conditional branches: + +```go +type WindowLister interface { + ListWindows() ([]string, error) + WindowExists(name string) (bool, error) + GetCurrentWindow() (string, error) +} + +// Production implementation uses real tmux +// Test implementation returns mock data +``` + +**Benefit:** Eliminates `if opts.Mock != nil` patterns throughout. + +#### 6. Use Table-Driven Tests + +Several test functions with high complexity (19-22) can be converted to table-driven tests: + +```go +func TestSendTool_SchemaValidation(t *testing.T) { + tests := []struct{ + name string + input map[string]any + wantErr bool + }{ + // test cases + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // single test implementation + }) + } +} +``` + +### Low Priority + +#### 7. Simplify main.go + +The main function (complexity 18) is acceptable for CLI setup but could be improved by: +- Moving command definitions to separate files +- Using a command registry pattern + +## Files Not Requiring Changes + +Functions with complexity 10 or below are acceptable and follow good practices. Test file complexity is less critical than production code. + +## Metrics After Recommended Changes + +| Function | Before | After (Est.) | +|----------|--------|--------------| +| cli.Send | 30 | 10-12 | +| cli.Receive | 25 | 10-12 | +| daemon.CheckAndNotifyWithNotifier | 24 | 10-12 | +| mcp.doSend | 21 | 10-12 | +| main.main | 18 | 10-14 | + +## Running Gocyclo + +```bash +# Install +go install github.com/fzipp/gocyclo/cmd/gocyclo@latest + +# Run with average +gocyclo -avg . + +# Show only functions with complexity > 15 +gocyclo -over 15 . + +# Top 10 most complex functions +gocyclo -top 10 . +``` + +## CI Integration + +Add to quality gates in CLAUDE.md: +```bash +# Fail if any function exceeds complexity of 20 +gocyclo -over 20 . && exit 1 || exit 0 +``` From de220d01906509770dd882383eb4f10e7784a032 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:02:04 +0000 Subject: [PATCH 2/7] refactor: reduce cyclomatic complexity across codebase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract helper methods and split large functions to reduce complexity: cli.Send: 30 → 15 (50% reduction) - Extract extractMessage, getSender, windowExists, loadIgnoreList, getRepoRoot cli.Receive: 25 → 9 (64% reduction) - Extract receiveCore with error struct - Add outputMessage helper - Consolidate hook mode handling daemon.CheckAndNotifyWithNotifier: 24 → 2 (92% reduction) - Split into notifyStatedAgents and notifyStatelessAgents - Extract processStatedAgent and processStatelessAgent mcp handlers: doSend 21 → 13, doListRecipients 13 → 8 - Add helper methods to HandlerOptions - Reuse patterns from CLI refactoring All tests pass. Only main.main (18) remains above 15, which is acceptable for CLI setup code. --- GOCYCLO_ANALYSIS.md | 26 +++- internal/cli/receive.go | 220 ++++++++++++++--------------- internal/cli/send.go | 189 +++++++++++++------------ internal/daemon/loop.go | 234 ++++++++++++++++--------------- internal/mcp/handlers.go | 294 ++++++++++++++++----------------------- 5 files changed, 455 insertions(+), 508 deletions(-) diff --git a/GOCYCLO_ANALYSIS.md b/GOCYCLO_ANALYSIS.md index 1d9fdf8..9bf5c4e 100644 --- a/GOCYCLO_ANALYSIS.md +++ b/GOCYCLO_ANALYSIS.md @@ -1,12 +1,32 @@ # Gocyclo Cyclomatic Complexity Analysis -**Generated:** 2026-01-15 -**Average complexity:** 6.0 +**Generated:** 2026-01-15 (Updated after refactoring) **Tool:** gocyclo v0.6.0 ## Summary -The codebase has an average cyclomatic complexity of ~6, which is acceptable. However, several functions exceed recommended thresholds and should be considered for refactoring. +After refactoring, all critical complexity issues have been resolved. Only CLI setup code (`main.main`) remains above 15, which is acceptable for command-line applications. + +### Refactoring Results + +| Function | Before | After | Reduction | +|----------|--------|-------|-----------| +| `cli.Send` | 30 | 15 | 50% | +| `cli.Receive` | 25 | 9 | 64% | +| `daemon.CheckAndNotifyWithNotifier` | 24 | 2 | 92% | +| `mcp.doSend` | 21 | 13 | 38% | +| `mcp.doListRecipients` | 13 | 8 | 38% | + +### Remaining High Complexity (Production Code) + +| Complexity | Function | File | Notes | +|------------|----------|------|-------| +| 18 | `main.main` | cmd/agentmail/main.go | Acceptable for CLI setup | +| 15 | `cli.Send` | internal/cli/send.go | At threshold | + +--- + +## Original Analysis ### Complexity Thresholds - **1-5:** Simple, low risk diff --git a/internal/cli/receive.go b/internal/cli/receive.go index 4e50c7d..f7e2fb5 100644 --- a/internal/cli/receive.go +++ b/internal/cli/receive.go @@ -19,158 +19,140 @@ type ReceiveOptions struct { 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) + if !opts.SkipTmuxCheck { + _ = 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/send.go b/internal/cli/send.go index aaf7463..1cabb63 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,135 @@ 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) +} - if !recipientExists { - fmt.Fprintln(stderr, "error: recipient not found") +// 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 } - // T029: Check if recipient is the sender (self-send not allowed) - if recipient == sender { - fmt.Fprintln(stderr, "error: recipient not found") + 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 } - // 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 - } + // Get sender identity + sender, err := opts.getSender() + if err != nil { + fmt.Fprintf(stderr, "error: failed to get current window: %v\n", err) + return 1 } - // T030: Check if recipient is in ignore list - if ignoreList != nil && ignoreList[recipient] { + // 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 || recipient == sender { fmt.Fprintln(stderr, "error: recipient not found") return 1 } - // Generate message ID - id, err := mail.GenerateID() + // Check ignore list + if ignoreList := opts.loadIgnoreList(); ignoreList != nil && ignoreList[recipient] { + fmt.Fprintln(stderr, "error: recipient not found") + return 1 + } + + // Get repository root + repoRoot, err := opts.getRepoRoot() if err != nil { - fmt.Fprintf(stderr, "error: failed to generate message ID: %v\n", err) + fmt.Fprintf(stderr, "error: not in a git repository: %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 - } + // 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 } - // T023: Store message msg := mail.Message{ ID: id, From: sender, @@ -183,7 +183,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/daemon/loop.go b/internal/daemon/loop.go index be2ebd5..6fe8e26 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.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) } + return + } - opts.log("Stated agent %q has %d unread message(s)", recipient.Recipient, len(unread)) + // Check 60s debounce + 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) + + 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 + } - // T021: Check for unread messages (FR-006) - unread, err := mail.FindUnread(opts.RepoRoot, mailboxRecipient) + 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/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. From fe6c0e9ef3f2076c063cbd4d8bb92a99becc49f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:03:21 +0000 Subject: [PATCH 3/7] chore: remove gocyclo analysis report --- GOCYCLO_ANALYSIS.md | 259 -------------------------------------------- 1 file changed, 259 deletions(-) delete mode 100644 GOCYCLO_ANALYSIS.md diff --git a/GOCYCLO_ANALYSIS.md b/GOCYCLO_ANALYSIS.md deleted file mode 100644 index 9bf5c4e..0000000 --- a/GOCYCLO_ANALYSIS.md +++ /dev/null @@ -1,259 +0,0 @@ -# Gocyclo Cyclomatic Complexity Analysis - -**Generated:** 2026-01-15 (Updated after refactoring) -**Tool:** gocyclo v0.6.0 - -## Summary - -After refactoring, all critical complexity issues have been resolved. Only CLI setup code (`main.main`) remains above 15, which is acceptable for command-line applications. - -### Refactoring Results - -| Function | Before | After | Reduction | -|----------|--------|-------|-----------| -| `cli.Send` | 30 | 15 | 50% | -| `cli.Receive` | 25 | 9 | 64% | -| `daemon.CheckAndNotifyWithNotifier` | 24 | 2 | 92% | -| `mcp.doSend` | 21 | 13 | 38% | -| `mcp.doListRecipients` | 13 | 8 | 38% | - -### Remaining High Complexity (Production Code) - -| Complexity | Function | File | Notes | -|------------|----------|------|-------| -| 18 | `main.main` | cmd/agentmail/main.go | Acceptable for CLI setup | -| 15 | `cli.Send` | internal/cli/send.go | At threshold | - ---- - -## Original Analysis - -### Complexity Thresholds -- **1-5:** Simple, low risk -- **6-10:** Moderate complexity -- **11-15:** High complexity - consider refactoring -- **16-20:** Very high complexity - should refactor -- **21+:** Critical - needs immediate attention - -## High-Priority Functions (Complexity > 20) - -| Complexity | Function | File | Line | -|------------|----------|------|------| -| 30 | `cli.Send` | internal/cli/send.go | 31 | -| 25 | `cli.Receive` | internal/cli/receive.go | 34 | -| 24 | `daemon.CheckAndNotifyWithNotifier` | internal/daemon/loop.go | 140 | -| 22 | `mcp.TestStatusTool_SchemaValidation` | internal/mcp/tools_test.go | 251 | -| 21 | `mcp.doSend` | internal/mcp/handlers.go | 89 | - -## Medium-Priority Functions (Complexity 15-20) - -| Complexity | Function | File | Line | -|------------|----------|------|------| -| 19 | `mcp.TestSendTool_SchemaValidation` | internal/mcp/tools_test.go | 127 | -| 18 | `main.main` | cmd/agentmail/main.go | 16 | -| 16 | `mcp.TestServer_100ConsecutiveInvocations` | internal/mcp/handlers_test.go | 2736 | -| 16 | `mail.TestUpdateRecipientState_UpdateExisting` | internal/mail/recipients_test.go | 322 | -| 15 | `mail.TestUpdateLastReadAt_PreservesOtherRecipients` | internal/mail/recipients_test.go | 1061 | - -## Root Cause Analysis - -### 1. Mock/Test Infrastructure in Production Code - -Both `cli.Send` and `cli.Receive` contain significant complexity from mock support: - -```go -// Pattern repeating throughout: -if opts.MockWindows != nil { - for _, w := range opts.MockWindows { - if w == recipient { - recipientExists = true - break - } - } -} else { - var err error - recipientExists, err = tmux.WindowExists(recipient) - // ... -} -``` - -**Impact:** Each mock option adds 2-3 branches. - -### 2. Hook Mode Conditional Logic - -`cli.Receive` has complexity from hook mode which adds early-exit branches throughout: - -```go -if opts.HookMode { - return 0 // Silent exit -} -// Normal error handling path -``` - -**Impact:** Hook mode adds ~8 additional branches. - -### 3. Two-Phase Processing - -`daemon.CheckAndNotifyWithNotifier` processes two types of agents (stated and stateless), essentially doubling the code complexity: -- Phase 1: Loop through stated agents with multiple conditions -- Phase 2: Loop through stateless agents with similar conditions - -### 4. Duplicated Validation Logic - -`cli.Send` and `mcp.doSend` share nearly identical validation logic but are implemented separately, both with mock support. - -## Recommendations - -### Immediate (Complexity > 20) - -#### 1. Extract Validation Logic into Helper Functions - -Create a shared validation package or helper functions: - -```go -// internal/validation/recipient.go -type RecipientValidator struct { - Windows []string // nil means use real tmux - IgnoreList map[string]bool -} - -func (v *RecipientValidator) Exists(recipient string) (bool, error) -func (v *RecipientValidator) IsIgnored(recipient string) bool -func (v *RecipientValidator) IsSelf(recipient, sender string) bool -``` - -**Expected reduction:** cli.Send 30 → ~18, mcp.doSend 21 → ~12 - -#### 2. Split cli.Send Into Smaller Functions - -```go -func Send(...) int { - if err := validateTmux(opts); err != nil { ... } - - msg, err := parseMessage(args, stdin, opts) - if err != nil { ... } - - sender, err := getSender(opts) - if err != nil { ... } - - if err := validateRecipient(recipient, sender, opts); err != nil { ... } - - return storeAndRespond(msg, stdout, stderr, opts) -} -``` - -**Expected reduction:** 30 → ~10 - -#### 3. Separate Hook Mode Into Different Function - -```go -func Receive(stdout, stderr io.Writer, opts ReceiveOptions) int { - if opts.HookMode { - return receiveHookMode(stderr, opts) - } - return receiveNormalMode(stdout, stderr, opts) -} -``` - -**Expected reduction:** 25 → ~12 each - -#### 4. Refactor CheckAndNotifyWithNotifier - -Split into two functions: - -```go -func CheckAndNotifyWithNotifier(opts LoopOptions, ...) error { - if err := notifyStatedAgents(opts, notify, windowChecker); err != nil { - return err - } - return notifyStatelessAgents(opts, notify, windowChecker) -} -``` - -**Expected reduction:** 24 → ~12 each - -### Medium-Term - -#### 5. Interface-Based Testing - -Replace mock structs with interfaces to reduce conditional branches: - -```go -type WindowLister interface { - ListWindows() ([]string, error) - WindowExists(name string) (bool, error) - GetCurrentWindow() (string, error) -} - -// Production implementation uses real tmux -// Test implementation returns mock data -``` - -**Benefit:** Eliminates `if opts.Mock != nil` patterns throughout. - -#### 6. Use Table-Driven Tests - -Several test functions with high complexity (19-22) can be converted to table-driven tests: - -```go -func TestSendTool_SchemaValidation(t *testing.T) { - tests := []struct{ - name string - input map[string]any - wantErr bool - }{ - // test cases - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // single test implementation - }) - } -} -``` - -### Low Priority - -#### 7. Simplify main.go - -The main function (complexity 18) is acceptable for CLI setup but could be improved by: -- Moving command definitions to separate files -- Using a command registry pattern - -## Files Not Requiring Changes - -Functions with complexity 10 or below are acceptable and follow good practices. Test file complexity is less critical than production code. - -## Metrics After Recommended Changes - -| Function | Before | After (Est.) | -|----------|--------|--------------| -| cli.Send | 30 | 10-12 | -| cli.Receive | 25 | 10-12 | -| daemon.CheckAndNotifyWithNotifier | 24 | 10-12 | -| mcp.doSend | 21 | 10-12 | -| main.main | 18 | 10-14 | - -## Running Gocyclo - -```bash -# Install -go install github.com/fzipp/gocyclo/cmd/gocyclo@latest - -# Run with average -gocyclo -avg . - -# Show only functions with complexity > 15 -gocyclo -over 15 . - -# Top 10 most complex functions -gocyclo -top 10 . -``` - -## CI Integration - -Add to quality gates in CLAUDE.md: -```bash -# Fail if any function exceeds complexity of 20 -gocyclo -over 20 . && exit 1 || exit 0 -``` From 501252f22e57b53f5b98508ebc7cbd3755129601 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:10:38 +0000 Subject: [PATCH 4/7] refactor: separate IsProtected from ShouldNotify predicates Add IsProtected() method to RecipientState for the 1h protection period (work/offline status), keeping ShouldNotify() focused on the 60s debounce interval. This clarifies the distinction in processStatedAgent: - IsProtected(): checks if work/offline agent is within 1h window - ShouldNotify(): checks if 60s debounce has elapsed (ready agents) --- internal/daemon/loop.go | 4 ++-- internal/mail/recipients.go | 17 ++++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/daemon/loop.go b/internal/daemon/loop.go index 6fe8e26..5ce2fa9 100644 --- a/internal/daemon/loop.go +++ b/internal/daemon/loop.go @@ -157,7 +157,7 @@ func notifyStatedAgents(opts LoopOptions, notify NotifyFunc) (map[string]struct{ func processStatedAgent(opts LoopOptions, notify NotifyFunc, recipient mail.RecipientState) { // Skip non-ready agents if recipient.Status != mail.StatusReady { - if !recipient.ShouldNotify() { + 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) @@ -165,7 +165,7 @@ func processStatedAgent(opts LoopOptions, notify NotifyFunc, recipient mail.Reci return } - // Check 60s debounce + // Check 60s debounce for ready agents if !recipient.ShouldNotify() { opts.log("Skipping stated agent %q: notified within last 60s", recipient.Recipient) return 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 } From 321b85a295b5cb87f2585b7035a6e95833435cc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:14:12 +0000 Subject: [PATCH 5/7] fix: use distinct error message for self-send in CLI Split the combined recipient check into two explicit checks: - "recipient not found" for non-existent recipients - "cannot send message to self" for self-send attempts This mirrors the MCP handler's error messages for consistency. --- internal/cli/send.go | 8 +++++++- internal/cli/send_test.go | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/cli/send.go b/internal/cli/send.go index 1cabb63..39d01a9 100644 --- a/internal/cli/send.go +++ b/internal/cli/send.go @@ -145,11 +145,17 @@ func Send(args []string, stdin io.Reader, stdout, stderr io.Writer, opts SendOpt fmt.Fprintf(stderr, "error: failed to check recipient: %v\n", err) return 1 } - if !recipientExists || recipient == sender { + if !recipientExists { fmt.Fprintln(stderr, "error: recipient not found") return 1 } + // Check self-send + if recipient == sender { + fmt.Fprintln(stderr, "error: cannot send message to self") + return 1 + } + // Check ignore list if ignoreList := opts.loadIgnoreList(); ignoreList != nil && ignoreList[recipient] { fmt.Fprintln(stderr, "error: recipient not found") 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() != "" { From 04486160689453b043407de322dd15326f46c553 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:17:43 +0000 Subject: [PATCH 6/7] refactor: add SkipTimestampUpdate option to ReceiveOptions Separates timestamp update control from tmux check to avoid overloading SkipTmuxCheck with unrelated behavior. The new SkipTimestampUpdate flag explicitly controls whether UpdateLastReadAt is called, providing clearer test isolation without coupling to tmux-related mocking. --- internal/cli/receive.go | 15 ++++++++------- internal/cli/receive_test.go | 30 ++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/internal/cli/receive.go b/internal/cli/receive.go index f7e2fb5..5217dcd 100644 --- a/internal/cli/receive.go +++ b/internal/cli/receive.go @@ -12,11 +12,12 @@ 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 } // receiveError represents an error during receive with context. @@ -94,8 +95,8 @@ func (opts *ReceiveOptions) receiveCore() (*mail.Message, *receiveError) { return nil, &receiveError{"failed to mark message as read: " + err.Error(), 1} } - // Update last_read_at timestamp (best-effort) - if !opts.SkipTmuxCheck { + // Update last_read_at timestamp (best-effort, skipped in tests) + if !opts.SkipTimestampUpdate { _ = mail.UpdateLastReadAt(repoRoot, receiver, time.Now().UnixMilli()) } diff --git a/internal/cli/receive_test.go b/internal/cli/receive_test.go index 190c810..209aa13 100644 --- a/internal/cli/receive_test.go +++ b/internal/cli/receive_test.go @@ -26,7 +26,8 @@ func TestReceiveCommand_NoMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -65,7 +66,8 @@ func TestReceiveCommand_AllMessagesRead(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -106,7 +108,8 @@ func TestReceiveCommand_Success(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -165,7 +168,8 @@ func TestReceiveCommand_FIFO_Order(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2", "agent-3"}, RepoRoot: tmpDir, @@ -215,7 +219,8 @@ func TestReceiveCommand_CurrentWindowNotInSession(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "orphan-window", MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list RepoRoot: tmpDir, @@ -255,7 +260,8 @@ func TestReceiveCommand_HookMode_WithMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -316,7 +322,8 @@ func TestReceiveCommand_HookMode_NoMessages(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -378,7 +385,8 @@ func TestReceiveCommand_HookMode_WindowNotInSession(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "orphan-window", MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list RepoRoot: tmpDir, @@ -422,7 +430,8 @@ func TestReceiveCommand_HookMode_AllMessagesRead(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, @@ -463,7 +472,8 @@ func TestReceiveCommand_NormalMode_Unchanged(t *testing.T) { var stdout, stderr bytes.Buffer exitCode := Receive(&stdout, &stderr, ReceiveOptions{ - SkipTmuxCheck: true, + SkipTmuxCheck: true, + SkipTimestampUpdate: true, MockReceiver: "agent-2", MockWindows: []string{"agent-1", "agent-2"}, RepoRoot: tmpDir, From 24196d8cc702f16ef67b09a0c217b20bf69a4b8a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Jan 2026 19:25:07 +0000 Subject: [PATCH 7/7] style: fix struct field alignment in receive_test.go Apply go fmt to fix struct field alignment after adding SkipTimestampUpdate field. --- internal/cli/receive_test.go | 70 ++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/internal/cli/receive_test.go b/internal/cli/receive_test.go index 209aa13..65ef3f4 100644 --- a/internal/cli/receive_test.go +++ b/internal/cli/receive_test.go @@ -28,9 +28,9 @@ func TestReceiveCommand_NoMessages(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -68,9 +68,9 @@ func TestReceiveCommand_AllMessagesRead(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -110,9 +110,9 @@ func TestReceiveCommand_Success(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -170,9 +170,9 @@ func TestReceiveCommand_FIFO_Order(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2", "agent-3"}, - RepoRoot: tmpDir, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2", "agent-3"}, + RepoRoot: tmpDir, }) if exitCode != 0 { @@ -221,9 +221,9 @@ func TestReceiveCommand_CurrentWindowNotInSession(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "orphan-window", - MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list - RepoRoot: tmpDir, + MockReceiver: "orphan-window", + MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list + RepoRoot: tmpDir, }) if exitCode != 1 { @@ -262,10 +262,10 @@ func TestReceiveCommand_HookMode_WithMessages(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // FR-001b: Exit code should be 2 @@ -324,10 +324,10 @@ func TestReceiveCommand_HookMode_NoMessages(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // FR-002: Exit code should be 0 @@ -387,10 +387,10 @@ func TestReceiveCommand_HookMode_WindowNotInSession(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "orphan-window", - MockWindows: []string{"agent-1", "agent-2"}, // orphan-window not in list - RepoRoot: tmpDir, - HookMode: 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) @@ -432,10 +432,10 @@ func TestReceiveCommand_HookMode_AllMessagesRead(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: true, + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: true, }) // Exit code should be 0 (no unread messages) @@ -474,10 +474,10 @@ func TestReceiveCommand_NormalMode_Unchanged(t *testing.T) { exitCode := Receive(&stdout, &stderr, ReceiveOptions{ SkipTmuxCheck: true, SkipTimestampUpdate: true, - MockReceiver: "agent-2", - MockWindows: []string{"agent-1", "agent-2"}, - RepoRoot: tmpDir, - HookMode: false, // Explicitly false + MockReceiver: "agent-2", + MockWindows: []string{"agent-1", "agent-2"}, + RepoRoot: tmpDir, + HookMode: false, // Explicitly false }) // Normal mode should exit 0 with message