From 630f21e3d02b9d13e635e0ad0d54d970cc0882a7 Mon Sep 17 00:00:00 2001 From: Claude Agento Date: Sat, 18 Apr 2026 12:47:31 -0700 Subject: [PATCH 1/3] feat(email): batch_move_messages MCP tool Accepts an array of message IDs and moves all to a destination folder in one call. Returns "moved N of M messages to Archive" with per-ID error lines for partial failures. 3 tests: batch move, partial failure, empty array. beadle-iue --- .beads/issues.jsonl | 1 + .ethos/missions.jsonl | 1 + CHANGELOG.md | 3 ++ README.md | 3 +- internal/mcp/format.go | 9 ++++++ internal/mcp/handler_test.go | 52 ++++++++++++++++++++++++++++++++ internal/mcp/smoke_test.go | 5 ++-- internal/mcp/tools.go | 57 ++++++++++++++++++++++++++++++++++++ 8 files changed, 128 insertions(+), 3 deletions(-) diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 63501d4..dc2d525 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -75,6 +75,7 @@ {"id":"beadle-g4g","title":"Implement top-level slash commands for beadle-email (/mail or /send TBD)","description":"Implement top-level slash commands for beadle-email once naming is resolved (beadle-bkw). Commands should use MCP tools in allowed-tools frontmatter (MCP-first, not Bash). Potential commands: check inbox, send email, send conversation summary. Each command needs proper allowed-tools declaration referencing mcp__plugin_beadle_email__ tools.","status":"closed","priority":2,"issue_type":"task","owner":"jmf@pobox.com","created_at":"2026-03-13T14:54:46.277581-07:00","created_by":"\"jmf-pobox\"","updated_at":"2026-03-13T17:13:24.034484-07:00","closed_at":"2026-03-13T17:13:24.034484-07:00","close_reason":"Slash commands /mail, /inbox, /send implemented and merged in PR #10. Both prod and dev tool prefixes in allowed-tools.","dependencies":[{"issue_id":"beadle-g4g","depends_on_id":"beadle-bkw","type":"blocks","created_at":"2026-03-13T14:55:14.297497-07:00","created_by":"\"jmf-pobox\""},{"issue_id":"beadle-g4g","depends_on_id":"beadle-fsj","type":"blocks","created_at":"2026-03-13T14:55:14.415297-07:00","created_by":"\"jmf-pobox\""}]} {"id":"beadle-glm","title":"SessionStart hook: auto-allow MCP permissions, deploy commands, first-run setup","description":"Implement SessionStart hook per punt-kit hooks.md standard. Must: (1) auto-allow beadle-email MCP tool permissions in ~/.claude/settings.json, (2) deploy top-level commands from commands/ to ~/.claude/commands/ using diff-and-copy pattern, (3) emit hookSpecificOutput with additionalContext describing setup state. Shell script is thin gate, delegates to beadle-email binary. Must handle both dev and prod plugin namespaces.","status":"closed","priority":1,"issue_type":"task","owner":"jmf@pobox.com","created_at":"2026-03-13T14:54:40.629858-07:00","created_by":"\"jmf-pobox\"","updated_at":"2026-03-13T17:31:27.549751-07:00","closed_at":"2026-03-13T17:31:27.549751-07:00","close_reason":"SessionStart hook refined and merged in PR #11. Dev mode detection, mode-specific permissions, first-run binary check, jq fallback.","dependencies":[{"issue_id":"beadle-glm","depends_on_id":"beadle-fsj","type":"blocks","created_at":"2026-03-13T14:55:14.050895-07:00","created_by":"\"jmf-pobox\""}]} {"id":"beadle-grl","title":"feat(cli): add identity subcommand to show/set per-repo identity","description":"beadle-email has no CLI or MCP tool to show or set the per-repo identity. Users must manually create .punt-labs/ethos/config.yaml. This causes permission errors when the repo's ethos active identity differs from the one with contacts/permissions configured. Add: beadle-email identity (show resolved identity + source), beadle-email identity set \u003chandle\u003e (write per-repo ethos config). May also need a corresponding MCP tool so Claude Code sessions can self-diagnose identity issues.","status":"closed","priority":2,"issue_type":"feature","owner":"jmf@pobox.com","created_at":"2026-03-21T12:06:38.531888-07:00","created_by":"\"jmf-pobox\"","updated_at":"2026-03-21T13:48:27.419423-07:00","closed_at":"2026-03-21T13:48:27.419423-07:00","close_reason":"PR #66 merged: identity subcommand + whoami MCP tool"} +{"id":"beadle-iue","title":"feat(email): batch archive MCP tool","description":"Add a batch_move or batch_archive MCP tool that accepts an array of message IDs and moves them all in one call. Current move_message handles one message at a time — archiving 588 messages requires 588 individual IMAP MOVE commands. A batch tool would reduce this to one call with one IMAP session. Critical for inbox hygiene when GitHub notifications accumulate.","status":"in_progress","priority":2,"issue_type":"task","owner":"claude@punt-labs.com","created_at":"2026-04-18T10:21:49.921967163-07:00","created_by":"J F","updated_at":"2026-04-18T12:41:55.711635303-07:00"} {"id":"beadle-iyr","title":"T4: CLIRunner — compound step execution","description":"Phase 3. Extend CLIRunner with compound steps support. Goroutine-per-step with io.Pipe chaining. All steps start concurrently under shared context.WithTimeout. First nonzero exit cancels context. Per-step stderr logging with step[N] labels. Final step stdout capped at 1MB via io.LimitReader on read side. Validate stdin rules at load time (step 0 = pipe, step N\u003e0 = stdout). Tests: 2-step chain, mid-chain failure, timeout, stderr routing. Depends on: beadle-427. Parent: beadle-mvd","status":"closed","priority":2,"issue_type":"task","owner":"claude@punt-labs.com","created_at":"2026-04-18T07:02:38.337684297-07:00","created_by":"J F","updated_at":"2026-04-18T09:34:11.886483785-07:00","closed_at":"2026-04-18T09:34:11.886483785-07:00","close_reason":"Closed"} {"id":"beadle-j1b","title":"Add install and uninstall subcommands","status":"closed","priority":1,"issue_type":"task","owner":"jmf@pobox.com","created_at":"2026-03-18T06:03:35.808438-07:00","created_by":"\"jmf-pobox\"","updated_at":"2026-03-18T09:03:41.550669-07:00","closed_at":"2026-03-18T09:03:41.550669-07:00","close_reason":"Merged in PR #38. CLI parity (list/read/send/move/folders), global flags (--json/--verbose/--quiet), install/uninstall subcommands."} {"id":"beadle-j25","title":"fix(chain): SendResult.Method should reflect actual SMTP server, not 'proton-bridge-smtp'","description":"TrySendChain hardcodes Method='proton-bridge-smtp' for all SMTP sends regardless of which server was used. Since #126 added smtp_host, the method could be Fastmail, Resend SMTP, or any other server. Should use the actual hostname or a generic 'smtp' label.","status":"closed","priority":3,"issue_type":"bug","owner":"claude@punt-labs.com","created_at":"2026-04-11T14:02:38.304423708-07:00","created_by":"J F","updated_at":"2026-04-12T06:17:30.478292035-07:00","closed_at":"2026-04-12T06:17:30.478292035-07:00","close_reason":"Closed"} diff --git a/.ethos/missions.jsonl b/.ethos/missions.jsonl index f03acae..5323ae3 100644 --- a/.ethos/missions.jsonl +++ b/.ethos/missions.jsonl @@ -31,3 +31,4 @@ {"id":"m-2026-04-18-075","created_at":"2026-04-18T14:56:51Z","closed_at":"2026-04-18T15:26:22Z","status":"closed","type":"implement","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":["internal/daemon/"],"success_criteria":["make check passes","all design decisions implemented"],"rounds_used":1,"rounds_budgeted":3,"verdict":"pass","files_changed":["internal/daemon/command.go","internal/daemon/command_test.go","internal/daemon/runner.go","internal/daemon/runner_test.go","internal/daemon/pipeline.go","internal/daemon/pipeline_test.go","internal/daemon/handler.go","internal/daemon/schema.go","internal/daemon/schema_test.go"],"pipeline":"standard-2026-04-18-7c0858"} {"id":"m-2026-04-18-076","created_at":"2026-04-18T14:56:51Z","closed_at":"2026-04-18T15:32:28Z","status":"closed","type":"test","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":["internal/daemon/"],"success_criteria":["coverage does not decrease","edge cases and error paths covered"],"rounds_used":1,"rounds_budgeted":2,"verdict":"pass","files_changed":["internal/daemon/pipeline_edge_test.go"],"pipeline":"standard-2026-04-18-7c0858"} {"id":"m-2026-04-18-077","created_at":"2026-04-18T14:56:51Z","closed_at":"2026-04-18T15:37:18Z","status":"failed","type":"report","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":[".tmp/review-pipeline-v2.md"],"success_criteria":["findings reported with severity and file:line references"],"rounds_used":1,"rounds_budgeted":1,"verdict":"fail","files_changed":[],"pipeline":"standard-2026-04-18-7c0858"} +{"id":"m-2026-04-18-106","created_at":"2026-04-18T19:42:09Z","closed_at":"2026-04-18T19:47:19Z","status":"closed","type":"implement","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":["internal/mcp/"],"success_criteria":["make check passes","new code has tests"],"rounds_used":1,"rounds_budgeted":3,"verdict":"pass","files_changed":["internal/mcp/tools.go","internal/mcp/format.go","internal/mcp/smoke_test.go","internal/mcp/handler_test.go"],"pipeline":"quick-2026-04-18-cc18e3"} diff --git a/CHANGELOG.md b/CHANGELOG.md index 28a7a5b..1b2710d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- `batch_move_messages` MCP tool: move multiple messages to another folder + in one call. Accepts an array of UIDs, returns a summary with per-message + errors for partial failures. - Pipeline v2: CLI runner executes binaries directly (milliseconds, not 45-second Claude sessions). Binary whitelist with `filepath.EvalSymlinks` at both load and execution time. Typed arg assembly (fixed, positional, diff --git a/README.md b/README.md index 40d13d0..85f7edf 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ Ensure `~/.local/bin` is on your `PATH`. Configure your MCP client to run `beadl ## Features -- **17 MCP tools** --- list, read, send, move/archive, download attachments, verify signatures, inspect MIME, classify trust, list folders, address book (list/find/add/remove contacts), whoami, `switch_identity`, inbox polling (set interval, get status) +- **18 MCP tools** --- list, read, send, move/archive, batch move, download attachments, verify signatures, inspect MIME, classify trust, list folders, address book (list/find/add/remove contacts), whoami, `switch_identity`, inbox polling (set interval, get status) - **Multi-identity via ethos** --- identity resolved per-request from ethos sidecar. Repo-local config pins identity. Mid-session switching via `switch_identity` tool. Fallback to `default-identity` file - **Two-dimensional trust** --- transport trust (trusted/verified/untrusted/unverified) + identity permissions (rwx per contact per identity). Both must pass before autonomous action - **Four-level transport trust** --- trusted (Proton-to-Proton E2E), verified (valid PGP), untrusted (bad PGP), unverified (no signature) @@ -125,6 +125,7 @@ Ensure `~/.local/bin` is on your `PATH`. Configure your MCP client to run `beadl | `read_message` | Read full message body, headers, attachments, and trust classification. | | `send_email` | Send via Proton Bridge SMTP (primary) or Resend API (fallback). Resolves contact names inline. | | `move_message` | Move a message to another folder. Defaults to Archive. | +| `batch_move_messages` | Move multiple messages to another folder in one call. Returns per-message success/failure summary. | | `list_folders` | List all IMAP mailbox folders. | | `show_mime` | Inspect multipart MIME structure, PGP parts, and attachments. | | `verify_signature` | Verify PGP signature on a message. Returns signer info and key ID. | diff --git a/internal/mcp/format.go b/internal/mcp/format.go index e06abdd..b16bc3c 100644 --- a/internal/mcp/format.go +++ b/internal/mcp/format.go @@ -273,6 +273,15 @@ func formatMoveResult(r *moveResult) string { return fmt.Sprintf("moved #%s → %s", r.MessageID, r.Destination) } +// formatBatchMoveResult formats a batch move summary. +func formatBatchMoveResult(moved, total int, destination string, errs []string) string { + s := fmt.Sprintf("moved %d of %d messages to %s", moved, total, destination) + for _, e := range errs { + s += "\n " + e + } + return s +} + // formatDownloadResult formats a download result. func formatDownloadResult(r *downloadResult) string { return fmt.Sprintf("%s: %s (%d bytes)\n%s", r.Status, r.Filename, r.Size, r.Path) diff --git a/internal/mcp/handler_test.go b/internal/mcp/handler_test.go index c4efd3d..89c6721 100644 --- a/internal/mcp/handler_test.go +++ b/internal/mcp/handler_test.go @@ -315,6 +315,58 @@ func TestHandler_MoveMessage(t *testing.T) { assert.Contains(t, r.text(), "moved") } +func TestHandler_BatchMoveMessages(t *testing.T) { + s, env, fix := setupHandler(t) + env.AddContact("Alice", "alice@test.com", "r--") + + uid1 := fix.AddMessage("INBOX", "alice@test.com", "Msg 1", "body 1") + uid2 := fix.AddMessage("INBOX", "alice@test.com", "Msg 2", "body 2") + uid3 := fix.AddMessage("INBOX", "alice@test.com", "Msg 3", "body 3") + fix.AddMessage("Archive", "system@test.com", "Placeholder", "x") + + r := callTool(t, s, "batch_move_messages", map[string]any{ + "message_ids": []any{ + fmt.Sprintf("%d", uid1), + fmt.Sprintf("%d", uid2), + fmt.Sprintf("%d", uid3), + }, + "destination": "Archive", + }) + assert.False(t, r.IsError, "batch move failed: %s", r.text()) + assert.Contains(t, r.text(), "moved 3 of 3") + assert.Contains(t, r.text(), "Archive") +} + +func TestHandler_BatchMoveMessages_PartialFailure(t *testing.T) { + s, env, fix := setupHandler(t) + env.AddContact("Alice", "alice@test.com", "r--") + + uid1 := fix.AddMessage("INBOX", "alice@test.com", "Msg 1", "body 1") + fix.AddMessage("Archive", "system@test.com", "Placeholder", "x") + + r := callTool(t, s, "batch_move_messages", map[string]any{ + "message_ids": []any{ + fmt.Sprintf("%d", uid1), + "not-a-number", // invalid UID triggers parse error + }, + "destination": "Archive", + }) + assert.False(t, r.IsError, "batch move failed: %s", r.text()) + assert.Contains(t, r.text(), "moved 1 of 2") + assert.Contains(t, r.text(), "#not-a-number") + assert.Contains(t, r.text(), "invalid id") +} + +func TestHandler_BatchMoveMessages_Empty(t *testing.T) { + s, _, _ := setupHandler(t) + + r := callTool(t, s, "batch_move_messages", map[string]any{ + "message_ids": []any{}, + }) + assert.False(t, r.IsError, "batch move failed: %s", r.text()) + assert.Contains(t, r.text(), "moved 0 of 0") +} + func TestHandler_Contacts_CRUD(t *testing.T) { s, _, _ := setupHandler(t) diff --git a/internal/mcp/smoke_test.go b/internal/mcp/smoke_test.go index 236cd52..72ed6af 100644 --- a/internal/mcp/smoke_test.go +++ b/internal/mcp/smoke_test.go @@ -101,8 +101,9 @@ func TestMCPSmoke_ToolRegistration(t *testing.T) { expectedTools := []string{ "list_messages", "read_message", "list_folders", "send_email", "verify_signature", "show_mime", "check_trust", "move_message", - "download_attachment", "list_contacts", "find_contact", - "add_contact", "remove_contact", "whoami", "switch_identity", + "batch_move_messages", "download_attachment", "list_contacts", + "find_contact", "add_contact", "remove_contact", "whoami", + "switch_identity", } for _, expected := range expectedTools { diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 271956d..e7ce8c0 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -62,6 +62,7 @@ func RegisterTools(s *server.MCPServer, resolver *identity.Resolver, logger *slo s.AddTool(showMIMETool(), h.showMIME) s.AddTool(checkTrustTool(), h.checkTrust) s.AddTool(moveMessageTool(), h.moveMessage) + s.AddTool(batchMoveMessagesTool(), h.batchMoveMessages) s.AddTool(downloadAttachmentTool(), h.downloadAttachment) s.AddTool(listContactsTool(), h.listContacts) @@ -292,6 +293,25 @@ func moveMessageTool() mcplib.Tool { ) } +func batchMoveMessagesTool() mcplib.Tool { + return mcplib.NewTool("batch_move_messages", + mcplib.WithDescription("Move multiple messages to another folder in one call. Returns a summary of successes and failures."), + mcplib.WithArray("message_ids", + mcplib.Required(), + mcplib.Description("Message UIDs to move (from list_messages)"), + mcplib.WithStringItems(), + ), + mcplib.WithString("folder", + mcplib.Description("Source IMAP folder name"), + mcplib.DefaultString("INBOX"), + ), + mcplib.WithString("destination", + mcplib.Description("Destination folder name"), + mcplib.DefaultString("Archive"), + ), + ) +} + // --- Contact Tool Definitions --- func listContactsTool() mcplib.Tool { @@ -795,6 +815,43 @@ func (h *handler) moveMessage(ctx context.Context, req mcplib.CallToolRequest) ( }) } +func (h *handler) batchMoveMessages(ctx context.Context, req mcplib.CallToolRequest) (*mcplib.CallToolResult, error) { + _, cfg, _, err := h.resolveIdentityAndConfig() + if err != nil { + return mcplib.NewToolResultError(err.Error()), nil + } + + ids, err := stringSliceParam(req, "message_ids") + if err != nil { + return mcplib.NewToolResultError(err.Error()), nil + } + + folder := stringParam(req, "folder", "INBOX") + destination := stringParam(req, "destination", "Archive") + + if len(ids) == 0 { + return textResult(formatBatchMoveResult(0, 0, destination, nil)) + } + + return h.withClient(cfg, func(c *email.Client) (*mcplib.CallToolResult, error) { + var moved int + var errs []string + for _, msgID := range ids { + uid, parseErr := strconv.ParseUint(msgID, 10, 32) + if parseErr != nil { + errs = append(errs, fmt.Sprintf("#%s: invalid id", msgID)) + continue + } + if moveErr := c.MoveMessage(folder, uint32(uid), destination); moveErr != nil { + errs = append(errs, fmt.Sprintf("#%s: %v", msgID, moveErr)) + continue + } + moved++ + } + return textResult(formatBatchMoveResult(moved, len(ids), destination, errs)) + }) +} + func (h *handler) downloadAttachment(ctx context.Context, req mcplib.CallToolRequest) (*mcplib.CallToolResult, error) { id, cfg, store, err := h.resolveContext() if err != nil { From dc0914753912ae9aea94e0835b33e024d58ba217 Mon Sep 17 00:00:00 2001 From: Claude Agento Date: Sat, 18 Apr 2026 12:52:38 -0700 Subject: [PATCH 2/3] fix(email): batch_move uses single SELECT + UID set MOVE Addresses 3 review findings: - MoveMessages method on email.Client: SELECT once, MOVE with full UID set in one IMAP round-trip (was SELECT per message) - nil message_ids returns error instead of misleading success - Invalid UIDs rejected before connecting (no partial IMAP state) --- .ethos/missions.jsonl | 1 + internal/email/imap.go | 22 +++++++++++++++++++++ internal/mcp/format.go | 8 ++------ internal/mcp/handler_test.go | 21 +++++++++++++------- internal/mcp/tools.go | 38 ++++++++++++++++++++++-------------- 5 files changed, 62 insertions(+), 28 deletions(-) diff --git a/.ethos/missions.jsonl b/.ethos/missions.jsonl index 5323ae3..19bba61 100644 --- a/.ethos/missions.jsonl +++ b/.ethos/missions.jsonl @@ -32,3 +32,4 @@ {"id":"m-2026-04-18-076","created_at":"2026-04-18T14:56:51Z","closed_at":"2026-04-18T15:32:28Z","status":"closed","type":"test","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":["internal/daemon/"],"success_criteria":["coverage does not decrease","edge cases and error paths covered"],"rounds_used":1,"rounds_budgeted":2,"verdict":"pass","files_changed":["internal/daemon/pipeline_edge_test.go"],"pipeline":"standard-2026-04-18-7c0858"} {"id":"m-2026-04-18-077","created_at":"2026-04-18T14:56:51Z","closed_at":"2026-04-18T15:37:18Z","status":"failed","type":"report","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":[".tmp/review-pipeline-v2.md"],"success_criteria":["findings reported with severity and file:line references"],"rounds_used":1,"rounds_budgeted":1,"verdict":"fail","files_changed":[],"pipeline":"standard-2026-04-18-7c0858"} {"id":"m-2026-04-18-106","created_at":"2026-04-18T19:42:09Z","closed_at":"2026-04-18T19:47:19Z","status":"closed","type":"implement","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":["internal/mcp/"],"success_criteria":["make check passes","new code has tests"],"rounds_used":1,"rounds_budgeted":3,"verdict":"pass","files_changed":["internal/mcp/tools.go","internal/mcp/format.go","internal/mcp/smoke_test.go","internal/mcp/handler_test.go"],"pipeline":"quick-2026-04-18-cc18e3"} +{"id":"m-2026-04-18-107","created_at":"2026-04-18T19:42:09Z","closed_at":"2026-04-18T19:52:22Z","status":"closed","type":"report","leader":"claude","worker":"bwk","evaluator":"mdm","write_set":[".tmp/review-batch-move.md"],"success_criteria":["findings reported with severity and file:line references"],"rounds_used":1,"rounds_budgeted":1,"verdict":"pass","files_changed":[],"pipeline":"quick-2026-04-18-cc18e3"} diff --git a/internal/email/imap.go b/internal/email/imap.go index a84f4e5..ee0e8b9 100644 --- a/internal/email/imap.go +++ b/internal/email/imap.go @@ -344,6 +344,28 @@ func (c *Client) MoveMessage(srcFolder string, uid uint32, dstFolder string) err return nil } +// MoveMessages moves multiple messages by UID from one folder to another +// in a single IMAP MOVE command. SELECTs the source folder once, builds +// a UID set, and issues one round-trip. UIDs that don't exist on the +// server are silently ignored by the IMAP protocol. +func (c *Client) MoveMessages(srcFolder string, uids []uint32, dstFolder string) error { + _, err := c.imap.Select(srcFolder, &imap.SelectOptions{ReadOnly: false}).Wait() + if err != nil { + return fmt.Errorf("select %q: %w", srcFolder, err) + } + + imapUIDs := make([]imap.UID, len(uids)) + for i, u := range uids { + imapUIDs[i] = imap.UID(u) + } + + _, err = c.imap.Move(imap.UIDSetNum(imapUIDs...), dstFolder).Wait() + if err != nil { + return fmt.Errorf("move %d messages to %q: %w", len(uids), dstFolder, err) + } + return nil +} + func formatAddress(addr imap.Address) string { if addr.Name != "" { return fmt.Sprintf("%s <%s@%s>", addr.Name, addr.Mailbox, addr.Host) diff --git a/internal/mcp/format.go b/internal/mcp/format.go index b16bc3c..3a73e0b 100644 --- a/internal/mcp/format.go +++ b/internal/mcp/format.go @@ -274,12 +274,8 @@ func formatMoveResult(r *moveResult) string { } // formatBatchMoveResult formats a batch move summary. -func formatBatchMoveResult(moved, total int, destination string, errs []string) string { - s := fmt.Sprintf("moved %d of %d messages to %s", moved, total, destination) - for _, e := range errs { - s += "\n " + e - } - return s +func formatBatchMoveResult(count int, destination string) string { + return fmt.Sprintf("moved %d messages to %s", count, destination) } // formatDownloadResult formats a download result. diff --git a/internal/mcp/handler_test.go b/internal/mcp/handler_test.go index 89c6721..b960226 100644 --- a/internal/mcp/handler_test.go +++ b/internal/mcp/handler_test.go @@ -333,11 +333,11 @@ func TestHandler_BatchMoveMessages(t *testing.T) { "destination": "Archive", }) assert.False(t, r.IsError, "batch move failed: %s", r.text()) - assert.Contains(t, r.text(), "moved 3 of 3") + assert.Contains(t, r.text(), "moved 3 messages") assert.Contains(t, r.text(), "Archive") } -func TestHandler_BatchMoveMessages_PartialFailure(t *testing.T) { +func TestHandler_BatchMoveMessages_InvalidUID(t *testing.T) { s, env, fix := setupHandler(t) env.AddContact("Alice", "alice@test.com", "r--") @@ -347,14 +347,13 @@ func TestHandler_BatchMoveMessages_PartialFailure(t *testing.T) { r := callTool(t, s, "batch_move_messages", map[string]any{ "message_ids": []any{ fmt.Sprintf("%d", uid1), - "not-a-number", // invalid UID triggers parse error + "not-a-number", }, "destination": "Archive", }) - assert.False(t, r.IsError, "batch move failed: %s", r.text()) - assert.Contains(t, r.text(), "moved 1 of 2") + assert.True(t, r.IsError, "invalid UID should produce error") assert.Contains(t, r.text(), "#not-a-number") - assert.Contains(t, r.text(), "invalid id") + assert.Contains(t, r.text(), "invalid") } func TestHandler_BatchMoveMessages_Empty(t *testing.T) { @@ -364,7 +363,15 @@ func TestHandler_BatchMoveMessages_Empty(t *testing.T) { "message_ids": []any{}, }) assert.False(t, r.IsError, "batch move failed: %s", r.text()) - assert.Contains(t, r.text(), "moved 0 of 0") + assert.Contains(t, r.text(), "moved 0 messages") +} + +func TestHandler_BatchMoveMessages_MissingParam(t *testing.T) { + s, _, _ := setupHandler(t) + + r := callTool(t, s, "batch_move_messages", map[string]any{}) + assert.True(t, r.IsError, "missing message_ids should produce error") + assert.Contains(t, r.text(), "message_ids is required") } func TestHandler_Contacts_CRUD(t *testing.T) { diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index e7ce8c0..eb4ab83 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -825,30 +825,38 @@ func (h *handler) batchMoveMessages(ctx context.Context, req mcplib.CallToolRequ if err != nil { return mcplib.NewToolResultError(err.Error()), nil } + if ids == nil { + return mcplib.NewToolResultError("message_ids is required"), nil + } folder := stringParam(req, "folder", "INBOX") destination := stringParam(req, "destination", "Archive") if len(ids) == 0 { - return textResult(formatBatchMoveResult(0, 0, destination, nil)) + return textResult(formatBatchMoveResult(0, destination)) + } + + // Parse all UIDs up front so we can report invalid IDs before + // opening a connection. + uids := make([]uint32, 0, len(ids)) + var parseErrs []string + for _, id := range ids { + uid, parseErr := strconv.ParseUint(id, 10, 32) + if parseErr != nil { + parseErrs = append(parseErrs, fmt.Sprintf("#%s: invalid id", id)) + continue + } + uids = append(uids, uint32(uid)) + } + if len(parseErrs) > 0 { + return mcplib.NewToolResultError(fmt.Sprintf("invalid message_ids: %s", strings.Join(parseErrs, ", "))), nil } return h.withClient(cfg, func(c *email.Client) (*mcplib.CallToolResult, error) { - var moved int - var errs []string - for _, msgID := range ids { - uid, parseErr := strconv.ParseUint(msgID, 10, 32) - if parseErr != nil { - errs = append(errs, fmt.Sprintf("#%s: invalid id", msgID)) - continue - } - if moveErr := c.MoveMessage(folder, uint32(uid), destination); moveErr != nil { - errs = append(errs, fmt.Sprintf("#%s: %v", msgID, moveErr)) - continue - } - moved++ + if err := c.MoveMessages(folder, uids, destination); err != nil { + return mcplib.NewToolResultError(fmt.Sprintf("batch move: %v", err)), nil } - return textResult(formatBatchMoveResult(moved, len(ids), destination, errs)) + return textResult(formatBatchMoveResult(len(uids), destination)) }) } From 65921ddc3aad10719566bc7764ea6e973324f790 Mon Sep 17 00:00:00 2001 From: Claude Agento Date: Sat, 18 Apr 2026 13:02:38 -0700 Subject: [PATCH 3/3] fix(email): address 6 Copilot/Bugbot findings on batch_move - Doc alignment: CHANGELOG, tool description, README match actual aggregate behavior (not per-message errors) - UID 0 rejected as invalid before IMAP connection - MoveMessages guards against empty uids slice - Doc comment: "single SELECT + single MOVE" not "one round-trip" --- CHANGELOG.md | 4 ++-- README.md | 2 +- internal/email/imap.go | 11 +++++++---- internal/mcp/tools.go | 6 +++++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2710d..0b56241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added - `batch_move_messages` MCP tool: move multiple messages to another folder - in one call. Accepts an array of UIDs, returns a summary with per-message - errors for partial failures. + in one call. Accepts an array of UIDs, returns an aggregate summary + ("moved N messages to Archive"). - Pipeline v2: CLI runner executes binaries directly (milliseconds, not 45-second Claude sessions). Binary whitelist with `filepath.EvalSymlinks` at both load and execution time. Typed arg assembly (fixed, positional, diff --git a/README.md b/README.md index 85f7edf..43d5a98 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Ensure `~/.local/bin` is on your `PATH`. Configure your MCP client to run `beadl | `read_message` | Read full message body, headers, attachments, and trust classification. | | `send_email` | Send via Proton Bridge SMTP (primary) or Resend API (fallback). Resolves contact names inline. | | `move_message` | Move a message to another folder. Defaults to Archive. | -| `batch_move_messages` | Move multiple messages to another folder in one call. Returns per-message success/failure summary. | +| `batch_move_messages` | Move multiple messages to another folder in one call. Returns the count of messages moved. | | `list_folders` | List all IMAP mailbox folders. | | `show_mime` | Inspect multipart MIME structure, PGP parts, and attachments. | | `verify_signature` | Verify PGP signature on a message. Returns signer info and key ID. | diff --git a/internal/email/imap.go b/internal/email/imap.go index ee0e8b9..51656da 100644 --- a/internal/email/imap.go +++ b/internal/email/imap.go @@ -344,11 +344,14 @@ func (c *Client) MoveMessage(srcFolder string, uid uint32, dstFolder string) err return nil } -// MoveMessages moves multiple messages by UID from one folder to another -// in a single IMAP MOVE command. SELECTs the source folder once, builds -// a UID set, and issues one round-trip. UIDs that don't exist on the -// server are silently ignored by the IMAP protocol. +// MoveMessages moves multiple messages by UID from one folder to another. +// Issues a single SELECT followed by a single MOVE command. UIDs that +// don't exist on the server are silently ignored by the IMAP protocol. func (c *Client) MoveMessages(srcFolder string, uids []uint32, dstFolder string) error { + if len(uids) == 0 { + return nil + } + _, err := c.imap.Select(srcFolder, &imap.SelectOptions{ReadOnly: false}).Wait() if err != nil { return fmt.Errorf("select %q: %w", srcFolder, err) diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index eb4ab83..3e0d183 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -295,7 +295,7 @@ func moveMessageTool() mcplib.Tool { func batchMoveMessagesTool() mcplib.Tool { return mcplib.NewTool("batch_move_messages", - mcplib.WithDescription("Move multiple messages to another folder in one call. Returns a summary of successes and failures."), + mcplib.WithDescription("Move multiple messages to another folder in one call. Returns the count of messages moved."), mcplib.WithArray("message_ids", mcplib.Required(), mcplib.Description("Message UIDs to move (from list_messages)"), @@ -846,6 +846,10 @@ func (h *handler) batchMoveMessages(ctx context.Context, req mcplib.CallToolRequ parseErrs = append(parseErrs, fmt.Sprintf("#%s: invalid id", id)) continue } + if uid == 0 { + parseErrs = append(parseErrs, fmt.Sprintf("#%s: invalid id", id)) + continue + } uids = append(uids, uint32(uid)) } if len(parseErrs) > 0 {