From 2891cafd673f31d65520c3cad4a8f6e2ce3e06f1 Mon Sep 17 00:00:00 2001 From: Sertac Ozercan Date: Fri, 8 May 2026 00:31:11 -0700 Subject: [PATCH] Fix dry-run MCP argument validation --- internal/commands/admin/admin.go | 22 +- internal/commands/admin365/admin365.go | 51 ++-- internal/commands/calendar/calendar.go | 108 ++++---- internal/commands/calendar/calendar_test.go | 32 +++ internal/commands/excel/excel.go | 37 +-- internal/commands/exec/exec.go | 251 ++++++++++++++++++ internal/commands/exec/exec_test.go | 124 +++++++++ internal/commands/knowledge/knowledge.go | 39 +-- internal/commands/mail/mail.go | 246 +++++++---------- internal/commands/mail/mail_test.go | 32 +++ .../commands/onedriveremote/onedriveremote.go | 83 +++--- internal/commands/planner/planner.go | 67 +++-- internal/commands/root_test.go | 53 ++++ internal/commands/sharepoint/sharepoint.go | 177 ++++++------ .../commands/sharepoint/sharepoint_test.go | 39 +++ internal/commands/splists/splists.go | 118 ++++---- internal/commands/splists/splists_test.go | 39 +++ internal/commands/teams/channels.go | 113 ++++---- internal/commands/teams/chats.go | 104 ++++---- internal/commands/triggers/triggers.go | 56 ++-- internal/commands/word/word.go | 35 ++- internal/testutil/testutil.go | 103 ++++++- 22 files changed, 1335 insertions(+), 594 deletions(-) create mode 100644 internal/commands/exec/exec.go create mode 100644 internal/commands/exec/exec_test.go diff --git a/internal/commands/admin/admin.go b/internal/commands/admin/admin.go index af310be..1e22976 100644 --- a/internal/commands/admin/admin.go +++ b/internal/commands/admin/admin.go @@ -74,6 +74,16 @@ type AdminSetLicenseCmd struct { } func (c *AdminSetLicenseCmd) Run(ctx *commands.Context) error { + addList := make([]map[string]any, 0, len(c.AddLicenses)) + for _, sku := range c.AddLicenses { + addList = append(addList, map[string]any{"skuId": sku}) + } + args := map[string]any{ + "userId": c.UserID, + "addLicenses": addList, + "removeLicenses": c.RemoveLicenses, + } + if ctx.DryRun { return ctx.ValidateDryRun(adminEndpoint(), "mcp_Admin365_LicenseMgmtTools", fmt.Sprintf("update licenses for user %s", c.UserID), @@ -83,6 +93,7 @@ func (c *AdminSetLicenseCmd) Run(ctx *commands.Context) error { "addLicenses": c.AddLicenses, "removeLicenses": c.RemoveLicenses, }, + args, ) } @@ -91,16 +102,7 @@ func (c *AdminSetLicenseCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - addList := make([]map[string]any, 0, len(c.AddLicenses)) - for _, sku := range c.AddLicenses { - addList = append(addList, map[string]any{"skuId": sku}) - } - - resp, err := client.CallTool(ctx.Ctx, "mcp_Admin365_LicenseMgmtTools", map[string]any{ - "userId": c.UserID, - "addLicenses": addList, - "removeLicenses": c.RemoveLicenses, - }) + resp, err := client.CallTool(ctx.Ctx, "mcp_Admin365_LicenseMgmtTools", args) if err != nil { return fmt.Errorf("set license: %w", err) } diff --git a/internal/commands/admin365/admin365.go b/internal/commands/admin365/admin365.go index 8ce63d5..acb1094 100644 --- a/internal/commands/admin365/admin365.go +++ b/internal/commands/admin365/admin365.go @@ -15,7 +15,7 @@ func admin365Endpoint() string { // Admin365Cmd groups all Admin365 subcommands. type Admin365Cmd struct { - BulkAdd Admin365BulkAddCmd `cmd:"" name:"bulk-add" help:"Bulk add users to tenant"` + BulkAdd Admin365BulkAddCmd `cmd:"" name:"bulk-add" help:"Bulk add users to tenant"` AgentAccess Admin365AgentAccessCmd `cmd:"" name:"agent-access" help:"Get agent access settings"` AgentSharing Admin365AgentSharingCmd `cmd:"" name:"agent-sharing" help:"Get agent sharing settings"` MsApps Admin365MsAppsCmd `cmd:"" name:"ms-apps" help:"Get Microsoft apps install settings"` @@ -39,6 +39,8 @@ type Admin365BulkAddCmd struct { } func (c *Admin365BulkAddCmd) Run(ctx *commands.Context) error { + args := map[string]any{"fileContent": c.FileContent} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "BulkAddUsers", "bulk add users to tenant", @@ -46,6 +48,7 @@ func (c *Admin365BulkAddCmd) Run(ctx *commands.Context) error { "action": "admin365.bulk-add", "fileContent": c.FileContent, }, + args, ) } @@ -54,9 +57,7 @@ func (c *Admin365BulkAddCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "BulkAddUsers", map[string]any{ - "fileContent": c.FileContent, - }) + resp, err := client.CallTool(ctx.Ctx, "BulkAddUsers", args) if err != nil { return fmt.Errorf("bulk add users: %w", err) } @@ -191,6 +192,8 @@ type Admin365SetAccessCmd struct { } func (c *Admin365SetAccessCmd) Run(ctx *commands.Context) error { + args := map[string]any{"accessLevel": c.AccessLevel} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateWhoCanAccessAgentsSettings", fmt.Sprintf("update agent access to %q", c.AccessLevel), @@ -198,6 +201,7 @@ func (c *Admin365SetAccessCmd) Run(ctx *commands.Context) error { "action": "admin365.set-access", "accessLevel": c.AccessLevel, }, + args, ) } @@ -206,9 +210,7 @@ func (c *Admin365SetAccessCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateWhoCanAccessAgentsSettings", map[string]any{ - "accessLevel": c.AccessLevel, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateWhoCanAccessAgentsSettings", args) if err != nil { return fmt.Errorf("update agent access: %w", err) } @@ -228,6 +230,8 @@ type Admin365SetSharingCmd struct { } func (c *Admin365SetSharingCmd) Run(ctx *commands.Context) error { + args := map[string]any{"accessLevel": c.AccessLevel} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateWhoCanShareAgentsOrgWideSettings", fmt.Sprintf("update agent sharing to %q", c.AccessLevel), @@ -235,6 +239,7 @@ func (c *Admin365SetSharingCmd) Run(ctx *commands.Context) error { "action": "admin365.set-sharing", "accessLevel": c.AccessLevel, }, + args, ) } @@ -243,9 +248,7 @@ func (c *Admin365SetSharingCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateWhoCanShareAgentsOrgWideSettings", map[string]any{ - "accessLevel": c.AccessLevel, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateWhoCanShareAgentsOrgWideSettings", args) if err != nil { return fmt.Errorf("update agent sharing: %w", err) } @@ -265,6 +268,8 @@ type Admin365SetMsAppsCmd struct { } func (c *Admin365SetMsAppsCmd) Run(ctx *commands.Context) error { + args := map[string]any{"allowed": c.Allowed} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateCanInstallMicrosoftAppsAndAgentsSettings", fmt.Sprintf("update Microsoft apps install to %s", c.Allowed), @@ -272,6 +277,7 @@ func (c *Admin365SetMsAppsCmd) Run(ctx *commands.Context) error { "action": "admin365.set-ms-apps", "allowed": c.Allowed, }, + args, ) } @@ -280,9 +286,7 @@ func (c *Admin365SetMsAppsCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallMicrosoftAppsAndAgentsSettings", map[string]any{ - "allowed": c.Allowed, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallMicrosoftAppsAndAgentsSettings", args) if err != nil { return fmt.Errorf("update Microsoft apps settings: %w", err) } @@ -302,6 +306,8 @@ type Admin365SetThirdPartyCmd struct { } func (c *Admin365SetThirdPartyCmd) Run(ctx *commands.Context) error { + args := map[string]any{"allowed": c.Allowed} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateCanInstallThirdPartyAppsAndAgentsSettings", fmt.Sprintf("update third-party apps install to %s", c.Allowed), @@ -309,6 +315,7 @@ func (c *Admin365SetThirdPartyCmd) Run(ctx *commands.Context) error { "action": "admin365.set-third-party", "allowed": c.Allowed, }, + args, ) } @@ -317,9 +324,7 @@ func (c *Admin365SetThirdPartyCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallThirdPartyAppsAndAgentsSettings", map[string]any{ - "allowed": c.Allowed, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallThirdPartyAppsAndAgentsSettings", args) if err != nil { return fmt.Errorf("update third-party apps settings: %w", err) } @@ -339,6 +344,8 @@ type Admin365SetLobAppsCmd struct { } func (c *Admin365SetLobAppsCmd) Run(ctx *commands.Context) error { + args := map[string]any{"allowed": c.Allowed} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateCanInstallLOBAppsAndAgentsSettings", fmt.Sprintf("update LOB apps install to %s", c.Allowed), @@ -346,6 +353,7 @@ func (c *Admin365SetLobAppsCmd) Run(ctx *commands.Context) error { "action": "admin365.set-lob-apps", "allowed": c.Allowed, }, + args, ) } @@ -354,9 +362,7 @@ func (c *Admin365SetLobAppsCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallLOBAppsAndAgentsSettings", map[string]any{ - "allowed": c.Allowed, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateCanInstallLOBAppsAndAgentsSettings", args) if err != nil { return fmt.Errorf("update LOB apps settings: %w", err) } @@ -422,6 +428,8 @@ type Admin365SetCopilotCmd struct { } func (c *Admin365SetCopilotCmd) Run(ctx *commands.Context) error { + args := map[string]any{"isEnabled": c.IsEnabled} + if ctx.DryRun { return ctx.ValidateDryRun(admin365Endpoint(), "UpdateCopilotAdminSettings", fmt.Sprintf("update Copilot admin setting to isEnabled=%s", c.IsEnabled), @@ -429,6 +437,7 @@ func (c *Admin365SetCopilotCmd) Run(ctx *commands.Context) error { "action": "admin365.set-copilot", "isEnabled": c.IsEnabled, }, + args, ) } @@ -437,9 +446,7 @@ func (c *Admin365SetCopilotCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateCopilotAdminSettings", map[string]any{ - "isEnabled": c.IsEnabled, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateCopilotAdminSettings", args) if err != nil { return fmt.Errorf("update Copilot admin settings: %w", err) } diff --git a/internal/commands/calendar/calendar.go b/internal/commands/calendar/calendar.go index 551735f..2113c0b 100644 --- a/internal/commands/calendar/calendar.go +++ b/internal/commands/calendar/calendar.go @@ -4,25 +4,26 @@ import ( "fmt" "github.com/sozercan/a365cli/internal/commands" + cmdexec "github.com/sozercan/a365cli/internal/commands/exec" "github.com/sozercan/a365cli/internal/config" "github.com/sozercan/a365cli/internal/output" ) // CalendarCmd groups all Calendar subcommands. type CalendarCmd struct { - List CalListCmd `cmd:"" help:"List upcoming events"` - View CalViewCmd `cmd:"" help:"List events in a date range"` - Create CalCreateCmd `cmd:"" help:"Create a calendar event"` - Update CalUpdateCmd `cmd:"" help:"Update a calendar event"` - Delete CalDeleteCmd `cmd:"" help:"Delete a calendar event"` - Accept CalAcceptCmd `cmd:"" help:"Accept a meeting invite"` - Tentative CalTentativeCmd `cmd:"" help:"Tentatively accept a meeting invite"` - Decline CalDeclineCmd `cmd:"" help:"Decline a meeting invite"` - Cancel CalCancelCmd `cmd:"" help:"Cancel a meeting you organized"` - Forward CalForwardCmd `cmd:"" help:"Forward a meeting invite"` - FreeBusy CalFreeBusyCmd `cmd:"" name:"free-busy" help:"Find available meeting times"` - TimeZone CalTimeZoneCmd `cmd:"" name:"timezone" help:"Get user date/time zone settings"` - Rooms CalRoomsCmd `cmd:"" help:"List available rooms"` + List CalListCmd `cmd:"" help:"List upcoming events"` + View CalViewCmd `cmd:"" help:"List events in a date range"` + Create CalCreateCmd `cmd:"" help:"Create a calendar event"` + Update CalUpdateCmd `cmd:"" help:"Update a calendar event"` + Delete CalDeleteCmd `cmd:"" help:"Delete a calendar event"` + Accept CalAcceptCmd `cmd:"" help:"Accept a meeting invite"` + Tentative CalTentativeCmd `cmd:"" help:"Tentatively accept a meeting invite"` + Decline CalDeclineCmd `cmd:"" help:"Decline a meeting invite"` + Cancel CalCancelCmd `cmd:"" help:"Cancel a meeting you organized"` + Forward CalForwardCmd `cmd:"" help:"Forward a meeting invite"` + FreeBusy CalFreeBusyCmd `cmd:"" name:"free-busy" help:"Find available meeting times"` + TimeZone CalTimeZoneCmd `cmd:"" name:"timezone" help:"Get user date/time zone settings"` + Rooms CalRoomsCmd `cmd:"" help:"List available rooms"` } func calEndpoint() string { @@ -161,26 +162,14 @@ func (c *CalCreateCmd) Run(ctx *commands.Context) error { // CalUpdateCmd updates a calendar event. type CalUpdateCmd struct { - ID string `arg:"" help:"Event ID"` - Subject string `help:"New subject" optional:""` - Start string `help:"New start time" optional:""` - End string `help:"New end time" optional:""` - Body string `help:"New body" optional:""` + ID string `arg:"" help:"Event ID"` + Subject string `help:"New subject" optional:""` + Start string `help:"New start time" optional:""` + End string `help:"New end time" optional:""` + Body string `help:"New body" optional:""` } func (c *CalUpdateCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(calEndpoint(), "UpdateEvent", - fmt.Sprintf("update event %s", c.ID), - map[string]any{"action": "calendar.update", "eventId": c.ID}, - ) - } - - client := ctx.NewMCPClient(calEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - args := map[string]any{"eventId": c.ID} if c.Subject != "" { args["subject"] = c.Subject @@ -195,16 +184,15 @@ func (c *CalUpdateCmd) Run(ctx *commands.Context) error { args["body"] = c.Body } - resp, err := client.CallTool(ctx.Ctx, "UpdateEvent", args) - if err != nil { - return fmt.Errorf("update event: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintMutation("Event updated", data) + return cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "calendar", + Tool: "UpdateEvent", + Args: args, + Action: fmt.Sprintf("update event %s", c.ID), + Display: map[string]any{"action": "calendar.update", "eventId": c.ID}, + ErrPrefix: "update event", + Output: cmdexec.Mutation("Event updated"), + }) } // CalDeleteCmd deletes a calendar event. @@ -213,9 +201,13 @@ type CalDeleteCmd struct { } func (c *CalDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{"eventId": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(calEndpoint(), "DeleteEventById", fmt.Sprintf("delete event %s", c.ID), - map[string]any{"action": "calendar.delete", "eventId": c.ID}) + map[string]any{"action": "calendar.delete", "eventId": c.ID}, + args, + ) } if err := ctx.Confirm(fmt.Sprintf("delete event %s", c.ID)); err != nil { return err @@ -226,7 +218,7 @@ func (c *CalDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeleteEventById", map[string]any{"eventId": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "DeleteEventById", args) if err != nil { return fmt.Errorf("delete event: %w", err) } @@ -244,16 +236,20 @@ type CalAcceptCmd struct { } func (c *CalAcceptCmd) Run(ctx *commands.Context) error { + args := map[string]any{"eventId": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(calEndpoint(), "AcceptEvent", fmt.Sprintf("accept event %s", c.ID), - map[string]any{"action": "calendar.accept", "eventId": c.ID}) + map[string]any{"action": "calendar.accept", "eventId": c.ID}, + args, + ) } client := ctx.NewMCPClient(calEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "AcceptEvent", map[string]any{"eventId": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "AcceptEvent", args) if err != nil { return fmt.Errorf("accept: %w", err) } @@ -270,16 +266,20 @@ type CalTentativeCmd struct { } func (c *CalTentativeCmd) Run(ctx *commands.Context) error { + args := map[string]any{"eventId": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(calEndpoint(), "TentativelyAcceptEvent", fmt.Sprintf("tentatively accept event %s", c.ID), - map[string]any{"action": "calendar.tentative", "eventId": c.ID}) + map[string]any{"action": "calendar.tentative", "eventId": c.ID}, + args, + ) } client := ctx.NewMCPClient(calEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "TentativelyAcceptEvent", map[string]any{"eventId": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "TentativelyAcceptEvent", args) if err != nil { return fmt.Errorf("tentative accept: %w", err) } @@ -296,16 +296,20 @@ type CalDeclineCmd struct { } func (c *CalDeclineCmd) Run(ctx *commands.Context) error { + args := map[string]any{"eventId": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(calEndpoint(), "DeclineEvent", fmt.Sprintf("decline event %s", c.ID), - map[string]any{"action": "calendar.decline", "eventId": c.ID}) + map[string]any{"action": "calendar.decline", "eventId": c.ID}, + args, + ) } client := ctx.NewMCPClient(calEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeclineEvent", map[string]any{"eventId": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "DeclineEvent", args) if err != nil { return fmt.Errorf("decline: %w", err) } @@ -322,9 +326,13 @@ type CalCancelCmd struct { } func (c *CalCancelCmd) Run(ctx *commands.Context) error { + args := map[string]any{"eventId": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(calEndpoint(), "CancelEvent", fmt.Sprintf("cancel event %s", c.ID), - map[string]any{"action": "calendar.cancel", "eventId": c.ID}) + map[string]any{"action": "calendar.cancel", "eventId": c.ID}, + args, + ) } if err := ctx.Confirm(fmt.Sprintf("cancel event %s", c.ID)); err != nil { return err @@ -334,7 +342,7 @@ func (c *CalCancelCmd) Run(ctx *commands.Context) error { if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CancelEvent", map[string]any{"eventId": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "CancelEvent", args) if err != nil { return fmt.Errorf("cancel: %w", err) } diff --git a/internal/commands/calendar/calendar_test.go b/internal/commands/calendar/calendar_test.go index 1f2c38d..3f19481 100644 --- a/internal/commands/calendar/calendar_test.go +++ b/internal/commands/calendar/calendar_test.go @@ -142,3 +142,35 @@ func TestCalAcceptCmd_DryRun(t *testing.T) { t.Errorf("expected valid=true, got %v; errors: %v", val["valid"], val["errors"]) } } + +func TestCalUpdateCmd_DryRunValidatesActualArgs(t *testing.T) { + schemas := []mcp.ToolInfo{ + { + Name: "UpdateEvent", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "eventId": map[string]any{"type": "string"}, + "subject": map[string]any{"type": "string"}, + }, + "required": []any{"eventId", "subject"}, + }, + }, + } + ctx, buf := testutil.SetupTestServerWithSchemas(t, nil, schemas) + ctx.DryRun = true + + cmd := &CalUpdateCmd{ID: "evt-001", Subject: "Project Sync"} + if err := cmd.Run(ctx); err != nil { + t.Fatalf("Run() error: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + val, ok := result["validation"].(map[string]any) + if !ok || val["valid"] != true { + t.Fatalf("expected valid dry-run output, got %v", result["validation"]) + } +} diff --git a/internal/commands/excel/excel.go b/internal/commands/excel/excel.go index cc76cf9..b11fa07 100644 --- a/internal/commands/excel/excel.go +++ b/internal/commands/excel/excel.go @@ -26,10 +26,13 @@ type ExcelCreateCmd struct { } func (c *ExcelCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"desiredFileName": c.FileName} + if ctx.DryRun { return ctx.ValidateDryRun(excelEndpoint(), "CreateWorkbook", fmt.Sprintf("create Excel workbook %q", c.FileName), map[string]any{"action": "excel.create", "desiredFileName": c.FileName}, + args, ) } @@ -38,9 +41,7 @@ func (c *ExcelCreateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreateWorkbook", map[string]any{ - "desiredFileName": c.FileName, - }) + resp, err := client.CallTool(ctx.Ctx, "CreateWorkbook", args) if err != nil { return fmt.Errorf("create workbook: %w", err) } @@ -88,10 +89,18 @@ type ExcelCommentCmd struct { } func (c *ExcelCommentCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "documentId": c.DocumentID, + "cellAddress": c.CellAddress, + "text": c.Text, + } + if ctx.DryRun { return ctx.ValidateDryRun(excelEndpoint(), "CreateComment", fmt.Sprintf("add comment to workbook %s at cell %s", c.DocumentID, c.CellAddress), map[string]any{"action": "excel.comment", "driveId": c.DriveID, "documentId": c.DocumentID, "cellAddress": c.CellAddress}, + args, ) } @@ -100,12 +109,7 @@ func (c *ExcelCommentCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreateComment", map[string]any{ - "driveId": c.DriveID, - "documentId": c.DocumentID, - "cellAddress": c.CellAddress, - "text": c.Text, - }) + resp, err := client.CallTool(ctx.Ctx, "CreateComment", args) if err != nil { return fmt.Errorf("add comment: %w", err) } @@ -126,10 +130,18 @@ type ExcelReplyCmd struct { } func (c *ExcelReplyCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "commentId": c.CommentID, + "driveId": c.DriveID, + "documentId": c.DocumentID, + "text": c.Text, + } + if ctx.DryRun { return ctx.ValidateDryRun(excelEndpoint(), "ReplyToComment", fmt.Sprintf("reply to comment %s on workbook %s", c.CommentID, c.DocumentID), map[string]any{"action": "excel.reply", "commentId": c.CommentID, "driveId": c.DriveID, "documentId": c.DocumentID}, + args, ) } @@ -138,12 +150,7 @@ func (c *ExcelReplyCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ReplyToComment", map[string]any{ - "commentId": c.CommentID, - "driveId": c.DriveID, - "documentId": c.DocumentID, - "text": c.Text, - }) + resp, err := client.CallTool(ctx.Ctx, "ReplyToComment", args) if err != nil { return fmt.Errorf("reply to comment: %w", err) } diff --git a/internal/commands/exec/exec.go b/internal/commands/exec/exec.go new file mode 100644 index 0000000..01b6d49 --- /dev/null +++ b/internal/commands/exec/exec.go @@ -0,0 +1,251 @@ +// Package exec centralizes MCP command execution for CLI command handlers. +package exec + +import ( + "fmt" + + "github.com/sozercan/a365cli/internal/commands" + "github.com/sozercan/a365cli/internal/config" + "github.com/sozercan/a365cli/internal/output" +) + +// Executor runs MCP tool calls on behalf of command handlers. +type Executor struct { + ctx *commands.Context +} + +// New returns an Executor bound to the command context. +func New(ctx *commands.Context) *Executor { + return &Executor{ctx: ctx} +} + +// ToolCall describes a read-only MCP tool call. +type ToolCall struct { + // Service is resolved through config.Endpoint when Endpoint is empty. + Service string + // Endpoint overrides Service endpoint resolution when set. + Endpoint string + Tool string + Args map[string]any + // ErrPrefix is used when wrapping CallTool errors. + ErrPrefix string + Output OutputSpec +} + +// OperationPlan describes a write MCP operation, including safety metadata. +type OperationPlan struct { + // Service is resolved through config.Endpoint when Endpoint is empty. + Service string + // Endpoint overrides Service endpoint resolution when set. + Endpoint string + Tool string + Args map[string]any + + // Display is safe metadata shown in dry-run output. Args are used for + // schema validation and execution, so dry-run and execution cannot drift. + Display map[string]any + Action string + + Destructive bool + ConfirmText string + + // ErrPrefix is used when wrapping CallTool errors. + ErrPrefix string + Output OutputSpec +} + +// OutputKind selects how domain data should be rendered. +type OutputKind int + +const ( + // OutputItem renders the full response object. + OutputItem OutputKind = iota + // OutputList renders an extracted collection as a table/array. + OutputList + // OutputMutation renders a mutation status message and response payload. + OutputMutation +) + +// OutputSpec describes how extracted MCP content should be rendered. +type OutputSpec struct { + Kind OutputKind + + Entity string + Columns []output.Column + CollectionKeys []string + Max int + FallbackToItem bool + + Message string +} + +// Item renders extracted content as an item. +func Item() OutputSpec { + return OutputSpec{Kind: OutputItem} +} + +// List renders extracted content as a list. The first matching collection key +// is used; when no key is supplied, entity is used as the collection key. +func List(entity string, columns []output.Column, keys ...string) OutputSpec { + if len(keys) == 0 { + keys = []string{entity} + } + return OutputSpec{ + Kind: OutputList, + Entity: entity, + Columns: columns, + CollectionKeys: append([]string(nil), keys...), + FallbackToItem: true, + } +} + +// Mutation renders extracted content as a mutation result. +func Mutation(message string) OutputSpec { + return OutputSpec{Kind: OutputMutation, Message: message} +} + +// WithMax returns a copy of the output spec with a row limit. +func (s OutputSpec) WithMax(max int) OutputSpec { + s.Max = max + return s +} + +// WithFallbackToItem returns a copy of the output spec with list fallback +// behavior configured. +func (s OutputSpec) WithFallbackToItem(enabled bool) OutputSpec { + s.FallbackToItem = enabled + return s +} + +// Query executes a read-only MCP tool call and renders its response. +func (e *Executor) Query(call ToolCall) error { + if err := validateTool(call.Tool); err != nil { + return err + } + endpoint, err := resolveEndpoint(call.Service, call.Endpoint) + if err != nil { + return err + } + data, err := e.callTool(endpoint, call.Tool, call.Args, call.ErrPrefix) + if err != nil { + return err + } + return e.Render(data, call.Output) +} + +// Mutate executes a write MCP operation. Dry-run validation, destructive +// confirmation, tool execution, and output rendering all use the same Args map. +func (e *Executor) Mutate(plan OperationPlan) error { + if err := validateTool(plan.Tool); err != nil { + return err + } + endpoint, err := resolveEndpoint(plan.Service, plan.Endpoint) + if err != nil { + return err + } + + action := plan.Action + if action == "" { + action = plan.Tool + } + display := plan.Display + if display == nil { + display = map[string]any{"action": action} + } + args := plan.Args + if args == nil { + args = map[string]any{} + } + + if e.ctx.DryRun { + return e.ctx.ValidateDryRun(endpoint, plan.Tool, action, display, args) + } + + if plan.Destructive { + confirmText := plan.ConfirmText + if confirmText == "" { + confirmText = action + } + if err := e.ctx.Confirm(confirmText); err != nil { + return err + } + } + + data, err := e.callTool(endpoint, plan.Tool, args, plan.ErrPrefix) + if err != nil { + return err + } + return e.Render(data, plan.Output) +} + +// Render renders already-extracted MCP content according to an output spec. +func (e *Executor) Render(data map[string]any, spec OutputSpec) error { + switch spec.Kind { + case OutputList: + for _, key := range spec.CollectionKeys { + rows := output.ToRows(data, key) + if rows == nil { + continue + } + if spec.Max > 0 && len(rows) > spec.Max { + rows = rows[:spec.Max] + } + return e.ctx.Output.PrintList(spec.Entity, spec.Columns, rows) + } + if spec.FallbackToItem { + return e.ctx.Output.PrintItem(data) + } + return fmt.Errorf("response did not contain any of the expected collection keys: %v", spec.CollectionKeys) + case OutputMutation: + return e.ctx.Output.PrintMutation(spec.Message, data) + case OutputItem: + fallthrough + default: + return e.ctx.Output.PrintItem(data) + } +} + +func (e *Executor) callTool(endpoint, tool string, args map[string]any, errPrefix string) (map[string]any, error) { + client := e.ctx.NewMCPClient(endpoint) + if err := client.Initialize(e.ctx.Ctx); err != nil { + return nil, fmt.Errorf("initialize: %w", err) + } + + if args == nil { + args = map[string]any{} + } + resp, err := client.CallTool(e.ctx.Ctx, tool, args) + if err != nil { + if errPrefix == "" { + errPrefix = fmt.Sprintf("call %s", tool) + } + return nil, fmt.Errorf("%s: %w", errPrefix, err) + } + + data, err := output.ExtractContent(resp) + if err != nil { + return nil, err + } + return data, nil +} + +func resolveEndpoint(service, endpoint string) (string, error) { + if endpoint != "" { + return endpoint, nil + } + if service == "" { + return "", fmt.Errorf("service or endpoint is required") + } + endpoint = config.Endpoint(service) + if endpoint == "" { + return "", fmt.Errorf("unknown service %q", service) + } + return endpoint, nil +} + +func validateTool(tool string) error { + if tool == "" { + return fmt.Errorf("tool is required") + } + return nil +} diff --git a/internal/commands/exec/exec_test.go b/internal/commands/exec/exec_test.go new file mode 100644 index 0000000..f6d21e6 --- /dev/null +++ b/internal/commands/exec/exec_test.go @@ -0,0 +1,124 @@ +package exec_test + +import ( + "encoding/json" + "strings" + "testing" + + cmdexec "github.com/sozercan/a365cli/internal/commands/exec" + "github.com/sozercan/a365cli/internal/mcp" + "github.com/sozercan/a365cli/internal/testutil" +) + +func TestQueryRendersFirstMatchingListKeyAndAppliesMax(t *testing.T) { + ctx, buf, recorder := testutil.SetupTestServerWithRecorder(t, map[string]string{ + "ListThings": `{"value":[{"id":"one"},{"id":"two"}]}`, + }) + + err := cmdexec.New(ctx).Query(cmdexec.ToolCall{ + Service: "mail", + Tool: "ListThings", + Args: map[string]any{"filter": "recent"}, + ErrPrefix: "list things", + Output: cmdexec.List("things", nil, "things", "value").WithMax(1), + }) + if err != nil { + t.Fatalf("Query() error: %v", err) + } + + calls := recorder.Calls() + if len(calls) != 1 { + t.Fatalf("expected one tools/call, got %d", len(calls)) + } + if calls[0].Name != "ListThings" { + t.Fatalf("expected ListThings call, got %q", calls[0].Name) + } + if calls[0].Arguments["filter"] != "recent" { + t.Fatalf("expected recorded filter arg, got %v", calls[0].Arguments) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + things, ok := result["things"].([]any) + if !ok { + t.Fatalf("expected things array, got %T in %v", result["things"], result) + } + if len(things) != 1 { + t.Fatalf("expected max-limited list length 1, got %d", len(things)) + } +} + +func TestMutateDryRunValidatesActualArgsAndDoesNotCallTool(t *testing.T) { + schemas := []mcp.ToolInfo{{ + Name: "UpdateThing", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "id": map[string]any{"type": "string"}, + "state": map[string]any{"type": "string"}, + }, + "required": []any{"id", "state"}, + }, + }} + ctx, buf, recorder := testutil.SetupTestServerWithSchemasAndRecorder(t, nil, schemas) + ctx.DryRun = true + + err := cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "mail", + Tool: "UpdateThing", + Args: map[string]any{ + "id": "thing-001", + "state": "closed", + }, + Action: "update thing thing-001", + Display: map[string]any{"action": "things.update", "id": "thing-001"}, + Output: cmdexec.Mutation("Thing updated"), + }) + if err != nil { + t.Fatalf("Mutate() error: %v", err) + } + if len(recorder.Calls()) != 0 { + t.Fatalf("dry-run should not call tools/call, got %#v", recorder.Calls()) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + validation, ok := result["validation"].(map[string]any) + if !ok { + t.Fatalf("expected validation object, got %v", result["validation"]) + } + if validation["valid"] != true { + t.Fatalf("expected valid dry-run validation, got %v", validation) + } +} + +func TestMutateDestructiveNoInputDoesNotCallTool(t *testing.T) { + ctx, _, recorder := testutil.SetupTestServerWithRecorder(t, map[string]string{ + "DeleteThing": `{"deleted":true}`, + }) + ctx.NoInput = true + + err := cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "mail", + Tool: "DeleteThing", + Args: map[string]any{"id": "thing-001"}, + Action: "delete thing thing-001", + Display: map[string]any{"action": "things.delete", "id": "thing-001"}, + Destructive: true, + ConfirmText: "delete thing thing-001", + Output: cmdexec.Mutation("Thing deleted"), + }) + if err == nil { + t.Fatal("expected destructive non-interactive confirmation error") + } + if !strings.Contains(err.Error(), "without --force") { + t.Fatalf("expected --force guidance, got %v", err) + } + if len(recorder.Calls()) != 0 { + t.Fatalf("confirmation failure should not call tools/call, got %#v", recorder.Calls()) + } +} diff --git a/internal/commands/knowledge/knowledge.go b/internal/commands/knowledge/knowledge.go index e149014..930c5b0 100644 --- a/internal/commands/knowledge/knowledge.go +++ b/internal/commands/knowledge/knowledge.go @@ -82,6 +82,14 @@ type KnowledgeConfigureCmd struct { } func (c *KnowledgeConfigureCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "consumerId": c.ConsumerID, + "knowledgeConfig": map[string]any{}, + "sourceType": c.SourceType, + "displayName": c.DisplayName, + "description": c.Description, + } + if ctx.DryRun { return ctx.ValidateDryRun(knowledgeEndpoint(), "configure_federated_knowledge", fmt.Sprintf("configure knowledge source %q", c.DisplayName), @@ -91,6 +99,7 @@ func (c *KnowledgeConfigureCmd) Run(ctx *commands.Context) error { "sourceType": c.SourceType, "displayName": c.DisplayName, }, + args, ) } @@ -99,13 +108,7 @@ func (c *KnowledgeConfigureCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "configure_federated_knowledge", map[string]any{ - "consumerId": c.ConsumerID, - "knowledgeConfig": map[string]any{}, - "sourceType": c.SourceType, - "displayName": c.DisplayName, - "description": c.Description, - }) + resp, err := client.CallTool(ctx.Ctx, "configure_federated_knowledge", args) if err != nil { return fmt.Errorf("configure: %w", err) } @@ -124,6 +127,11 @@ type KnowledgeIngestCmd struct { } func (c *KnowledgeIngestCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "consumerId": c.ConsumerID, + "searchConfigurationId": c.ConfigID, + } + if ctx.DryRun { return ctx.ValidateDryRun(knowledgeEndpoint(), "ingest_federated_knowledge", fmt.Sprintf("ingest knowledge config %s", c.ConfigID), @@ -132,6 +140,7 @@ func (c *KnowledgeIngestCmd) Run(ctx *commands.Context) error { "consumerId": c.ConsumerID, "searchConfigurationId": c.ConfigID, }, + args, ) } @@ -140,10 +149,7 @@ func (c *KnowledgeIngestCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ingest_federated_knowledge", map[string]any{ - "consumerId": c.ConsumerID, - "searchConfigurationId": c.ConfigID, - }) + resp, err := client.CallTool(ctx.Ctx, "ingest_federated_knowledge", args) if err != nil { return fmt.Errorf("ingest: %w", err) } @@ -162,6 +168,11 @@ type KnowledgeDeleteCmd struct { } func (c *KnowledgeDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "searchConfigurationId": c.ConfigID, + "consumerId": c.ConsumerID, + } + if ctx.DryRun { return ctx.ValidateDryRun(knowledgeEndpoint(), "delete_federated_knowledge", fmt.Sprintf("delete knowledge config %s", c.ConfigID), @@ -170,6 +181,7 @@ func (c *KnowledgeDeleteCmd) Run(ctx *commands.Context) error { "consumerId": c.ConsumerID, "searchConfigurationId": c.ConfigID, }, + args, ) } @@ -182,10 +194,7 @@ func (c *KnowledgeDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "delete_federated_knowledge", map[string]any{ - "searchConfigurationId": c.ConfigID, - "consumerId": c.ConsumerID, - }) + resp, err := client.CallTool(ctx.Ctx, "delete_federated_knowledge", args) if err != nil { return fmt.Errorf("delete: %w", err) } diff --git a/internal/commands/mail/mail.go b/internal/commands/mail/mail.go index 2e0d56c..62541c8 100644 --- a/internal/commands/mail/mail.go +++ b/internal/commands/mail/mail.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/sozercan/a365cli/internal/commands" + cmdexec "github.com/sozercan/a365cli/internal/commands/exec" "github.com/sozercan/a365cli/internal/config" "github.com/sozercan/a365cli/internal/output" ) @@ -44,11 +45,6 @@ type MailSearchCmd struct { } func (c *MailSearchCmd) Run(ctx *commands.Context) error { - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - // Auto-wrap bare search terms into OData $search query parameters. // If the user provides a raw OData string (starting with ? or $), pass it through. query := c.Query @@ -56,28 +52,13 @@ func (c *MailSearchCmd) Run(ctx *commands.Context) error { query = `?$search="` + query + `"` } - resp, err := client.CallTool(ctx.Ctx, "SearchMessagesQueryParameters", map[string]any{ - "queryParameters": query, + return cmdexec.New(ctx).Query(cmdexec.ToolCall{ + Service: "mail", + Tool: "SearchMessagesQueryParameters", + Args: map[string]any{"queryParameters": query}, + ErrPrefix: "search", + Output: cmdexec.List("messages", output.MailColumns, "messages", "value").WithMax(c.Max), }) - if err != nil { - return fmt.Errorf("search: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - rows := output.ToRows(data, "messages") - if rows == nil { - rows = output.ToRows(data, "value") - } - if rows == nil { - return ctx.Output.PrintItem(data) - } - if c.Max > 0 && len(rows) > c.Max { - rows = rows[:c.Max] - } - return ctx.Output.PrintList("messages", output.MailColumns, rows) } // MailSearchNLCmd searches emails with natural language. @@ -111,23 +92,13 @@ type MailGetCmd struct { } func (c *MailGetCmd) Run(ctx *commands.Context) error { - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - - resp, err := client.CallTool(ctx.Ctx, "GetMessage", map[string]any{ - "id": c.ID, + return cmdexec.New(ctx).Query(cmdexec.ToolCall{ + Service: "mail", + Tool: "GetMessage", + Args: map[string]any{"id": c.ID}, + ErrPrefix: "get message", + Output: cmdexec.Item(), }) - if err != nil { - return fmt.Errorf("get message: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintItem(data) } // MailSendCmd sends an email. @@ -140,30 +111,6 @@ type MailSendCmd struct { } func (c *MailSendCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - mcpArgs := map[string]any{ - "to": c.To, - "subject": c.Subject, - "body": c.Body, - } - if len(c.CC) > 0 { - mcpArgs["cc"] = c.CC - } - if len(c.BCC) > 0 { - mcpArgs["bcc"] = c.BCC - } - return ctx.ValidateDryRun(mailEndpoint(), "SendEmailWithAttachments", - fmt.Sprintf("send email to %v with subject %q", c.To, c.Subject), - map[string]any{"action": "mail.send", "to": c.To, "subject": c.Subject, "body_len": len(c.Body)}, - mcpArgs, - ) - } - - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - args := map[string]any{ "to": c.To, "subject": c.Subject, @@ -176,16 +123,20 @@ func (c *MailSendCmd) Run(ctx *commands.Context) error { args["bcc"] = c.BCC } - resp, err := client.CallTool(ctx.Ctx, "SendEmailWithAttachments", args) - if err != nil { - return fmt.Errorf("send email: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintMutation("Email sent", data) + return cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "mail", + Tool: "SendEmailWithAttachments", + Args: args, + Action: fmt.Sprintf("send email to %v with subject %q", c.To, c.Subject), + Display: map[string]any{ + "action": "mail.send", + "to": c.To, + "subject": c.Subject, + "body_len": len(c.Body), + }, + ErrPrefix: "send email", + Output: cmdexec.Mutation("Email sent"), + }) } // MailReplyCmd replies to an email. @@ -324,10 +275,13 @@ type MailDeleteCmd struct { } func (c *MailDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{"id": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "DeleteMessage", fmt.Sprintf("delete message %s", c.ID), map[string]any{"action": "mail.delete", "id": c.ID}, + args, ) } if err := ctx.Confirm(fmt.Sprintf("delete email %s", c.ID)); err != nil { @@ -339,7 +293,7 @@ func (c *MailDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeleteMessage", map[string]any{"id": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "DeleteMessage", args) if err != nil { return fmt.Errorf("delete: %w", err) } @@ -358,10 +312,16 @@ type MailFlagCmd struct { } func (c *MailFlagCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "messageId": c.ID, + "flagStatus": c.Status, + } + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "FlagEmail", fmt.Sprintf("flag email %s as %s", c.ID, c.Status), map[string]any{"action": "mail.flag", "messageId": c.ID, "flagStatus": c.Status}, + args, ) } @@ -370,10 +330,7 @@ func (c *MailFlagCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "FlagEmail", map[string]any{ - "messageId": c.ID, - "flagStatus": c.Status, - }) + resp, err := client.CallTool(ctx.Ctx, "FlagEmail", args) if err != nil { return fmt.Errorf("flag: %w", err) } @@ -394,17 +351,6 @@ type MailDraftCmd struct { } func (c *MailDraftCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(mailEndpoint(), "CreateDraftMessage", "create draft email", - map[string]any{"action": "mail.draft", "subject": c.Subject, "to": c.To}, - ) - } - - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - args := map[string]any{} if c.Subject != "" { args["subject"] = c.Subject @@ -419,6 +365,18 @@ func (c *MailDraftCmd) Run(ctx *commands.Context) error { args["cc"] = c.CC } + if ctx.DryRun { + return ctx.ValidateDryRun(mailEndpoint(), "CreateDraftMessage", "create draft email", + map[string]any{"action": "mail.draft", "subject": c.Subject, "to": c.To}, + args, + ) + } + + client := ctx.NewMCPClient(mailEndpoint()) + if err := client.Initialize(ctx.Ctx); err != nil { + return fmt.Errorf("initialize: %w", err) + } + resp, err := client.CallTool(ctx.Ctx, "CreateDraftMessage", args) if err != nil { return fmt.Errorf("create draft: %w", err) @@ -437,10 +395,13 @@ type MailSendDraftCmd struct { } func (c *MailSendDraftCmd) Run(ctx *commands.Context) error { + args := map[string]any{"id": c.ID} + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "SendDraftMessage", fmt.Sprintf("send draft %s", c.ID), map[string]any{"action": "mail.send-draft", "id": c.ID}, + args, ) } @@ -449,7 +410,7 @@ func (c *MailSendDraftCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "SendDraftMessage", map[string]any{"id": c.ID}) + resp, err := client.CallTool(ctx.Ctx, "SendDraftMessage", args) if err != nil { return fmt.Errorf("send draft: %w", err) } @@ -496,18 +457,6 @@ type MailUpdateCmd struct { } func (c *MailUpdateCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(mailEndpoint(), "UpdateMessage", - fmt.Sprintf("update message %s", c.ID), - map[string]any{"action": "mail.update", "id": c.ID}, - ) - } - - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - args := map[string]any{ "id": c.ID, } @@ -524,16 +473,15 @@ func (c *MailUpdateCmd) Run(ctx *commands.Context) error { args["categories"] = c.Categories } - resp, err := client.CallTool(ctx.Ctx, "UpdateMessage", args) - if err != nil { - return fmt.Errorf("update message: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintMutation("Email updated", data) + return cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "mail", + Tool: "UpdateMessage", + Args: args, + Action: fmt.Sprintf("update message %s", c.ID), + Display: map[string]any{"action": "mail.update", "id": c.ID}, + ErrPrefix: "update message", + Output: cmdexec.Mutation("Email updated"), + }) } // MailDownloadCmd downloads an attachment from a message. @@ -621,10 +569,16 @@ type MailDeleteAttachCmd struct { } func (c *MailDeleteAttachCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "messageId": c.MessageID, + "attachmentId": c.AttachmentID, + } + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "DeleteAttachment", fmt.Sprintf("delete attachment %s from message %s", c.AttachmentID, c.MessageID), map[string]any{"action": "mail.delete-attachment", "messageId": c.MessageID, "attachmentId": c.AttachmentID}, + args, ) } if err := ctx.Confirm(fmt.Sprintf("delete attachment %s from message %s", c.AttachmentID, c.MessageID)); err != nil { @@ -636,10 +590,7 @@ func (c *MailDeleteAttachCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeleteAttachment", map[string]any{ - "messageId": c.MessageID, - "attachmentId": c.AttachmentID, - }) + resp, err := client.CallTool(ctx.Ctx, "DeleteAttachment", args) if err != nil { return fmt.Errorf("delete attachment: %w", err) } @@ -662,18 +613,6 @@ type MailUpdateDraftCmd struct { } func (c *MailUpdateDraftCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(mailEndpoint(), "UpdateDraft", - fmt.Sprintf("update draft %s", c.MessageID), - map[string]any{"action": "mail.update-draft", "messageId": c.MessageID}, - ) - } - - client := ctx.NewMCPClient(mailEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - args := map[string]any{ "messageId": c.MessageID, } @@ -693,6 +632,19 @@ func (c *MailUpdateDraftCmd) Run(ctx *commands.Context) error { args["body"] = c.Body } + if ctx.DryRun { + return ctx.ValidateDryRun(mailEndpoint(), "UpdateDraft", + fmt.Sprintf("update draft %s", c.MessageID), + map[string]any{"action": "mail.update-draft", "messageId": c.MessageID}, + args, + ) + } + + client := ctx.NewMCPClient(mailEndpoint()) + if err := client.Initialize(ctx.Ctx); err != nil { + return fmt.Errorf("initialize: %w", err) + } + resp, err := client.CallTool(ctx.Ctx, "UpdateDraft", args) if err != nil { return fmt.Errorf("update draft: %w", err) @@ -712,10 +664,16 @@ type MailDraftAttachCmd struct { } func (c *MailDraftAttachCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "messageId": c.MessageID, + "attachmentUris": c.AttachmentUris, + } + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "AddDraftAttachments", fmt.Sprintf("add attachments to draft %s", c.MessageID), map[string]any{"action": "mail.draft-attachments", "messageId": c.MessageID, "attachmentUris": c.AttachmentUris}, + args, ) } @@ -724,10 +682,7 @@ func (c *MailDraftAttachCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "AddDraftAttachments", map[string]any{ - "messageId": c.MessageID, - "attachmentUris": c.AttachmentUris, - }) + resp, err := client.CallTool(ctx.Ctx, "AddDraftAttachments", args) if err != nil { return fmt.Errorf("add draft attachments: %w", err) } @@ -745,10 +700,13 @@ type MailReplyThreadCmd struct { } func (c *MailReplyThreadCmd) Run(ctx *commands.Context) error { + args := map[string]any{"messageId": c.MessageID} + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "ReplyWithFullThread", fmt.Sprintf("reply with full thread to message %s", c.MessageID), map[string]any{"action": "mail.reply-thread", "messageId": c.MessageID}, + args, ) } @@ -757,9 +715,7 @@ func (c *MailReplyThreadCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ReplyWithFullThread", map[string]any{ - "messageId": c.MessageID, - }) + resp, err := client.CallTool(ctx.Ctx, "ReplyWithFullThread", args) if err != nil { return fmt.Errorf("reply with thread: %w", err) } @@ -777,10 +733,13 @@ type MailReplyAllThreadCmd struct { } func (c *MailReplyAllThreadCmd) Run(ctx *commands.Context) error { + args := map[string]any{"messageId": c.MessageID} + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "ReplyAllWithFullThread", fmt.Sprintf("reply-all with full thread to message %s", c.MessageID), map[string]any{"action": "mail.reply-all-thread", "messageId": c.MessageID}, + args, ) } @@ -789,9 +748,7 @@ func (c *MailReplyAllThreadCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ReplyAllWithFullThread", map[string]any{ - "messageId": c.MessageID, - }) + resp, err := client.CallTool(ctx.Ctx, "ReplyAllWithFullThread", args) if err != nil { return fmt.Errorf("reply-all with thread: %w", err) } @@ -809,10 +766,13 @@ type MailForwardThreadCmd struct { } func (c *MailForwardThreadCmd) Run(ctx *commands.Context) error { + args := map[string]any{"messageId": c.MessageID} + if ctx.DryRun { return ctx.ValidateDryRun(mailEndpoint(), "ForwardMessageWithFullThread", fmt.Sprintf("forward with full thread message %s", c.MessageID), map[string]any{"action": "mail.forward-thread", "messageId": c.MessageID}, + args, ) } @@ -821,9 +781,7 @@ func (c *MailForwardThreadCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ForwardMessageWithFullThread", map[string]any{ - "messageId": c.MessageID, - }) + resp, err := client.CallTool(ctx.Ctx, "ForwardMessageWithFullThread", args) if err != nil { return fmt.Errorf("forward with thread: %w", err) } diff --git a/internal/commands/mail/mail_test.go b/internal/commands/mail/mail_test.go index ec7c812..9e02b69 100644 --- a/internal/commands/mail/mail_test.go +++ b/internal/commands/mail/mail_test.go @@ -188,3 +188,35 @@ func TestMailUploadCmd_LargeDryRunUsesLargeToolSchema(t *testing.T) { t.Fatalf("expected valid dry-run output, got %v", result["validation"]) } } + +func TestMailUpdateCmd_DryRunValidatesActualArgs(t *testing.T) { + schemas := []mcp.ToolInfo{ + { + Name: "UpdateMessage", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "id": map[string]any{"type": "string"}, + "importance": map[string]any{"type": "string"}, + }, + "required": []any{"id", "importance"}, + }, + }, + } + ctx, buf := testutil.SetupTestServerWithSchemas(t, nil, schemas) + ctx.DryRun = true + + cmd := &MailUpdateCmd{ID: "msg-001", Importance: "High"} + if err := cmd.Run(ctx); err != nil { + t.Fatalf("Run() error: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + val, ok := result["validation"].(map[string]any) + if !ok || val["valid"] != true { + t.Fatalf("expected valid dry-run output, got %v", result["validation"]) + } +} diff --git a/internal/commands/onedriveremote/onedriveremote.go b/internal/commands/onedriveremote/onedriveremote.go index 42c0b8c..d0279a5 100644 --- a/internal/commands/onedriveremote/onedriveremote.go +++ b/internal/commands/onedriveremote/onedriveremote.go @@ -176,6 +176,8 @@ type ODRMkdirCmd struct { } func (c *ODRMkdirCmd) Run(ctx *commands.Context) error { + args := map[string]any{"folderName": c.FolderName} + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "createFolderInMyOnedrive", fmt.Sprintf("create folder %q", c.FolderName), @@ -183,6 +185,7 @@ func (c *ODRMkdirCmd) Run(ctx *commands.Context) error { "action": "onedrive-remote.mkdir", "folderName": c.FolderName, }, + args, ) } @@ -191,9 +194,7 @@ func (c *ODRMkdirCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createFolderInMyOnedrive", map[string]any{ - "folderName": c.FolderName, - }) + resp, err := client.CallTool(ctx.Ctx, "createFolderInMyOnedrive", args) if err != nil { return fmt.Errorf("create folder: %w", err) } @@ -214,6 +215,11 @@ type ODRWriteCmd struct { } func (c *ODRWriteCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "filename": c.Filename, + "contentText": c.ContentText, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "createSmallTextFileInMyOnedrive", fmt.Sprintf("create file %q", c.Filename), @@ -222,6 +228,7 @@ func (c *ODRWriteCmd) Run(ctx *commands.Context) error { "filename": c.Filename, "contentText": c.ContentText, }, + args, ) } @@ -230,10 +237,7 @@ func (c *ODRWriteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createSmallTextFileInMyOnedrive", map[string]any{ - "filename": c.Filename, - "contentText": c.ContentText, - }) + resp, err := client.CallTool(ctx.Ctx, "createSmallTextFileInMyOnedrive", args) if err != nil { return fmt.Errorf("create file: %w", err) } @@ -255,6 +259,12 @@ type ODRRenameCmd struct { } func (c *ODRRenameCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "fileOrFolderId": c.FileOrFolderID, + "newFileOrFolderName": c.NewFileOrFolderName, + "etag": c.Etag, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "renameFileOrFolderInMyOnedrive", fmt.Sprintf("rename %s to %q", c.FileOrFolderID, c.NewFileOrFolderName), @@ -262,8 +272,9 @@ func (c *ODRRenameCmd) Run(ctx *commands.Context) error { "action": "onedrive-remote.rename", "fileOrFolderId": c.FileOrFolderID, "newFileOrFolderName": c.NewFileOrFolderName, - "etag": c.Etag, + "etag": c.Etag, }, + args, ) } @@ -272,11 +283,7 @@ func (c *ODRRenameCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "renameFileOrFolderInMyOnedrive", map[string]any{ - "fileOrFolderId": c.FileOrFolderID, - "newFileOrFolderName": c.NewFileOrFolderName, - "etag": c.Etag, - }) + resp, err := client.CallTool(ctx.Ctx, "renameFileOrFolderInMyOnedrive", args) if err != nil { return fmt.Errorf("rename: %w", err) } @@ -298,6 +305,12 @@ type ODRMvCmd struct { } func (c *ODRMvCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "fileId": c.FileID, + "newParentFolderId": c.NewParentFolderID, + "etag": c.Etag, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "moveSmallFileInMyOnedrive", fmt.Sprintf("move file %s to folder %s", c.FileID, c.NewParentFolderID), @@ -305,8 +318,9 @@ func (c *ODRMvCmd) Run(ctx *commands.Context) error { "action": "onedrive-remote.mv", "fileId": c.FileID, "newParentFolderId": c.NewParentFolderID, - "etag": c.Etag, + "etag": c.Etag, }, + args, ) } @@ -315,11 +329,7 @@ func (c *ODRMvCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "moveSmallFileInMyOnedrive", map[string]any{ - "fileId": c.FileID, - "newParentFolderId": c.NewParentFolderID, - "etag": c.Etag, - }) + resp, err := client.CallTool(ctx.Ctx, "moveSmallFileInMyOnedrive", args) if err != nil { return fmt.Errorf("move file: %w", err) } @@ -340,6 +350,11 @@ type ODRRmCmd struct { } func (c *ODRRmCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "fileOrFolderId": c.FileOrFolderID, + "etag": c.Etag, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "deleteFileOrFolderInMyOnedrive", fmt.Sprintf("delete %s", c.FileOrFolderID), @@ -348,6 +363,7 @@ func (c *ODRRmCmd) Run(ctx *commands.Context) error { "fileOrFolderId": c.FileOrFolderID, "etag": c.Etag, }, + args, ) } @@ -360,10 +376,7 @@ func (c *ODRRmCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "deleteFileOrFolderInMyOnedrive", map[string]any{ - "fileOrFolderId": c.FileOrFolderID, - "etag": c.Etag, - }) + resp, err := client.CallTool(ctx.Ctx, "deleteFileOrFolderInMyOnedrive", args) if err != nil { return fmt.Errorf("delete: %w", err) } @@ -385,6 +398,12 @@ type ODRShareCmd struct { } func (c *ODRShareCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "fileOrFolderId": c.FileOrFolderID, + "recipientEmails": c.RecipientEmails, + "roles": c.Roles, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "shareFileOrFolderInMyOnedrive", fmt.Sprintf("share %s with %v", c.FileOrFolderID, c.RecipientEmails), @@ -394,6 +413,7 @@ func (c *ODRShareCmd) Run(ctx *commands.Context) error { "recipientEmails": c.RecipientEmails, "roles": c.Roles, }, + args, ) } @@ -402,11 +422,7 @@ func (c *ODRShareCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "shareFileOrFolderInMyOnedrive", map[string]any{ - "fileOrFolderId": c.FileOrFolderID, - "recipientEmails": c.RecipientEmails, - "roles": c.Roles, - }) + resp, err := client.CallTool(ctx.Ctx, "shareFileOrFolderInMyOnedrive", args) if err != nil { return fmt.Errorf("share: %w", err) } @@ -427,6 +443,11 @@ type ODRLabelCmd struct { } func (c *ODRLabelCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "fileId": c.FileID, + "sensitivityLabelId": c.SensitivityLabelID, + } + if ctx.DryRun { return ctx.ValidateDryRun(odrEndpoint(), "setSensitivityLabelOnFileInMyOnedrive", fmt.Sprintf("set sensitivity label %s on file %s", c.SensitivityLabelID, c.FileID), @@ -435,6 +456,7 @@ func (c *ODRLabelCmd) Run(ctx *commands.Context) error { "fileId": c.FileID, "sensitivityLabelId": c.SensitivityLabelID, }, + args, ) } @@ -443,10 +465,7 @@ func (c *ODRLabelCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "setSensitivityLabelOnFileInMyOnedrive", map[string]any{ - "fileId": c.FileID, - "sensitivityLabelId": c.SensitivityLabelID, - }) + resp, err := client.CallTool(ctx.Ctx, "setSensitivityLabelOnFileInMyOnedrive", args) if err != nil { return fmt.Errorf("set label: %w", err) } diff --git a/internal/commands/planner/planner.go b/internal/commands/planner/planner.go index 3e4338c..b3d5cb5 100644 --- a/internal/commands/planner/planner.go +++ b/internal/commands/planner/planner.go @@ -10,9 +10,9 @@ import ( // PlannerCmd groups all Planner subcommands. type PlannerCmd struct { - Plans PlansCmd `cmd:"" help:"Planner plans"` - Tasks TasksCmd `cmd:"" help:"Planner tasks"` - Goals GoalsCmd `cmd:"" help:"Planner goals"` + Plans PlansCmd `cmd:"" help:"Planner plans"` + Tasks TasksCmd `cmd:"" help:"Planner tasks"` + Goals GoalsCmd `cmd:"" help:"Planner goals"` } func plannerEndpoint() string { @@ -84,15 +84,18 @@ type PlansCreateCmd struct { } func (c *PlansCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"title": c.Title} + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "CreatePlan", fmt.Sprintf("create plan %q", c.Title), - map[string]any{"action": "planner.create-plan", "title": c.Title}) + map[string]any{"action": "planner.create-plan", "title": c.Title}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreatePlan", map[string]any{"title": c.Title}) + resp, err := client.CallTool(ctx.Ctx, "CreatePlan", args) if err != nil { return fmt.Errorf("create plan: %w", err) } @@ -109,18 +112,20 @@ type PlansUpdateCmd struct { } func (c *PlansUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"planId": c.ID} + if c.Title != "" { + args["title"] = c.Title + } + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "UpdatePlan", fmt.Sprintf("update plan %s", c.ID), - map[string]any{"action": "planner.update-plan", "planId": c.ID}) + map[string]any{"action": "planner.update-plan", "planId": c.ID}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{"planId": c.ID} - if c.Title != "" { - args["title"] = c.Title - } resp, err := client.CallTool(ctx.Ctx, "UpdatePlan", args) if err != nil { return fmt.Errorf("update plan: %w", err) @@ -198,17 +203,18 @@ type TasksCreateCmd struct { } func (c *TasksCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"planId": c.PlanID, "title": c.Title} + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "CreateTask", fmt.Sprintf("create task %q in plan %s", c.Title, c.PlanID), - map[string]any{"action": "planner.create-task", "planId": c.PlanID, "title": c.Title}) + map[string]any{"action": "planner.create-task", "planId": c.PlanID, "title": c.Title}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreateTask", map[string]any{ - "planId": c.PlanID, "title": c.Title, - }) + resp, err := client.CallTool(ctx.Ctx, "CreateTask", args) if err != nil { return fmt.Errorf("create task: %w", err) } @@ -225,18 +231,20 @@ type TasksUpdateCmd struct { } func (c *TasksUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"taskId": c.ID} + if c.Title != "" { + args["title"] = c.Title + } + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "UpdateTask", fmt.Sprintf("update task %s", c.ID), - map[string]any{"action": "planner.update-task", "taskId": c.ID}) + map[string]any{"action": "planner.update-task", "taskId": c.ID}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{"taskId": c.ID} - if c.Title != "" { - args["title"] = c.Title - } resp, err := client.CallTool(ctx.Ctx, "UpdateTask", args) if err != nil { return fmt.Errorf("update task: %w", err) @@ -303,17 +311,18 @@ type GoalsCreateCmd struct { } func (c *GoalsCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"planId": c.PlanID, "title": c.Title} + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "CreateGoal", fmt.Sprintf("create goal %q in plan %s", c.Title, c.PlanID), - map[string]any{"action": "planner.create-goal", "planId": c.PlanID, "title": c.Title}) + map[string]any{"action": "planner.create-goal", "planId": c.PlanID, "title": c.Title}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreateGoal", map[string]any{ - "planId": c.PlanID, "title": c.Title, - }) + resp, err := client.CallTool(ctx.Ctx, "CreateGoal", args) if err != nil { return fmt.Errorf("create goal: %w", err) } @@ -330,18 +339,20 @@ type GoalsUpdateCmd struct { } func (c *GoalsUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"goalId": c.ID} + if c.Title != "" { + args["title"] = c.Title + } + if ctx.DryRun { return ctx.ValidateDryRun(plannerEndpoint(), "UpdateGoal", fmt.Sprintf("update goal %s", c.ID), - map[string]any{"action": "planner.update-goal", "goalId": c.ID}) + map[string]any{"action": "planner.update-goal", "goalId": c.ID}, + args) } client := ctx.NewMCPClient(plannerEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{"goalId": c.ID} - if c.Title != "" { - args["title"] = c.Title - } resp, err := client.CallTool(ctx.Ctx, "UpdateGoal", args) if err != nil { return fmt.Errorf("update goal: %w", err) diff --git a/internal/commands/root_test.go b/internal/commands/root_test.go index 8b07546..7731ec5 100644 --- a/internal/commands/root_test.go +++ b/internal/commands/root_test.go @@ -174,6 +174,59 @@ func TestValidateDryRun_ValidArgs(t *testing.T) { } } +func TestValidateDryRun_UsesExplicitMCPArgsForValidation(t *testing.T) { + schemas := []mcp.ToolInfo{ + { + Name: "SendMessage", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "chatId": map[string]any{"type": "string"}, + "content": map[string]any{"type": "string"}, + }, + "required": []any{"chatId", "content"}, + }, + }, + } + server := setupMockMCPServer(t, schemas) + + var buf bytes.Buffer + ctx := &Context{ + Ctx: context.Background(), + TokenProvider: func(ctx context.Context) (string, error) { return "test-token", nil }, + Output: &output.Formatter{Format: output.FormatJSON, Writer: &buf}, + DryRun: true, + } + + err := ctx.ValidateDryRun(server.URL+"/", "SendMessage", "send message", + map[string]any{"action": "send-message", "message": "hello"}, + map[string]any{"chatId": "abc", "content": "hello"}) + if err != nil { + t.Fatalf("ValidateDryRun with explicit valid MCP args should not error: %v", err) + } + + var parsed map[string]any + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("output is not valid JSON: %v", err) + } + if parsed["dry_run"] != true { + t.Error("expected dry_run=true") + } + if parsed["action"] != "send-message" { + t.Errorf("expected display action to be preserved, got %v", parsed["action"]) + } + if _, ok := parsed["chatId"]; ok { + t.Error("expected raw MCP-only chatId to stay out of dry-run display data") + } + val, ok := parsed["validation"].(map[string]any) + if !ok { + t.Fatal("expected validation object") + } + if val["valid"] != true { + t.Error("expected valid=true") + } +} + func TestValidateDryRun_InvalidArgs(t *testing.T) { schemas := []mcp.ToolInfo{ { diff --git a/internal/commands/sharepoint/sharepoint.go b/internal/commands/sharepoint/sharepoint.go index 2387d5f..c8f7dba 100644 --- a/internal/commands/sharepoint/sharepoint.go +++ b/internal/commands/sharepoint/sharepoint.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/sozercan/a365cli/internal/commands" + cmdexec "github.com/sozercan/a365cli/internal/commands/exec" "github.com/sozercan/a365cli/internal/config" "github.com/sozercan/a365cli/internal/output" ) @@ -285,6 +286,12 @@ type SPMkdirCmd struct { } func (c *SPMkdirCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "parentPath": c.ParentPath, + "folderName": c.FolderName, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "createFolder", fmt.Sprintf("create folder %q in %s", c.FolderName, c.ParentPath), @@ -294,6 +301,7 @@ func (c *SPMkdirCmd) Run(ctx *commands.Context) error { "parentPath": c.ParentPath, "folderName": c.FolderName, }, + args, ) } @@ -302,11 +310,7 @@ func (c *SPMkdirCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createFolder", map[string]any{ - "driveId": c.DriveID, - "parentPath": c.ParentPath, - "folderName": c.FolderName, - }) + resp, err := client.CallTool(ctx.Ctx, "createFolder", args) if err != nil { return fmt.Errorf("create folder: %w", err) } @@ -330,24 +334,7 @@ type SPWriteCmd struct { } func (c *SPWriteCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(spEndpoint(), "createSmallTextFile", - fmt.Sprintf("create file %q in %s", c.FileName, c.FolderPath), - map[string]any{ - "action": "sharepoint.write", - "driveId": c.DriveID, - "folderPath": c.FolderPath, - "fileName": c.FileName, - }, - ) - } - - client := ctx.NewMCPClient(spEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } - - var toolName string + toolName := "createSmallTextFile" args := map[string]any{ "driveId": c.DriveID, "folderPath": c.FolderPath, @@ -358,20 +345,23 @@ func (c *SPWriteCmd) Run(ctx *commands.Context) error { toolName = "createSmallBinaryFile" args["contentBase64"] = c.ContentBase64 } else { - toolName = "createSmallTextFile" args["content"] = c.Content } - resp, err := client.CallTool(ctx.Ctx, toolName, args) - if err != nil { - return fmt.Errorf("create file: %w", err) - } - - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintMutation("File created", data) + return cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "sharepoint", + Tool: toolName, + Args: args, + Action: fmt.Sprintf("create file %q in %s", c.FileName, c.FolderPath), + Display: map[string]any{ + "action": "sharepoint.write", + "driveId": c.DriveID, + "folderPath": c.FolderPath, + "fileName": c.FileName, + }, + ErrPrefix: "create file", + Output: cmdexec.Mutation("File created"), + }) } // --- uploadFileFromUrl --- @@ -385,6 +375,13 @@ type SPUploadCmd struct { } func (c *SPUploadCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "sourceUrl": c.SourceURL, + "destinationDriveId": c.DestinationDriveID, + "destinationFolderPath": c.DestinationFolderPath, + "fileName": c.FileName, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "uploadFileFromUrl", fmt.Sprintf("upload %q from %s to %s", c.FileName, c.SourceURL, c.DestinationFolderPath), @@ -395,6 +392,7 @@ func (c *SPUploadCmd) Run(ctx *commands.Context) error { "destinationFolderPath": c.DestinationFolderPath, "fileName": c.FileName, }, + args, ) } @@ -403,12 +401,7 @@ func (c *SPUploadCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "uploadFileFromUrl", map[string]any{ - "sourceUrl": c.SourceURL, - "destinationDriveId": c.DestinationDriveID, - "destinationFolderPath": c.DestinationFolderPath, - "fileName": c.FileName, - }) + resp, err := client.CallTool(ctx.Ctx, "uploadFileFromUrl", args) if err != nil { return fmt.Errorf("upload file: %w", err) } @@ -430,11 +423,21 @@ type SPDeleteCmd struct { } func (c *SPDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + } + if c.ItemPath != "" { + args["itemPath"] = c.ItemPath + } + if c.ItemID != "" { + args["itemId"] = c.ItemID + } + target := c.ItemPath + if target == "" { + target = c.ItemID + } + if ctx.DryRun { - target := c.ItemPath - if target == "" { - target = c.ItemID - } return ctx.ValidateDryRun(spEndpoint(), "deleteFileOrFolder", fmt.Sprintf("delete %s from drive %s", target, c.DriveID), map[string]any{ @@ -443,13 +446,10 @@ func (c *SPDeleteCmd) Run(ctx *commands.Context) error { "itemPath": c.ItemPath, "itemId": c.ItemID, }, + args, ) } - target := c.ItemPath - if target == "" { - target = c.ItemID - } if err := ctx.Confirm(fmt.Sprintf("delete %s from drive %s", target, c.DriveID)); err != nil { return err } @@ -459,16 +459,6 @@ func (c *SPDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "driveId": c.DriveID, - } - if c.ItemPath != "" { - args["itemPath"] = c.ItemPath - } - if c.ItemID != "" { - args["itemId"] = c.ItemID - } - resp, err := client.CallTool(ctx.Ctx, "deleteFileOrFolder", args) if err != nil { return fmt.Errorf("delete: %w", err) @@ -492,6 +482,13 @@ type SPMoveCmd struct { } func (c *SPMoveCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "sourceDriveId": c.SourceDriveID, + "sourceItemPath": c.SourceItemPath, + "destinationDriveId": c.DestinationDriveID, + "destinationFolderPath": c.DestinationFolderPath, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "moveFileOrFolder", fmt.Sprintf("move %s to %s", c.SourceItemPath, c.DestinationFolderPath), @@ -502,6 +499,7 @@ func (c *SPMoveCmd) Run(ctx *commands.Context) error { "destinationDriveId": c.DestinationDriveID, "destinationFolderPath": c.DestinationFolderPath, }, + args, ) } @@ -510,12 +508,7 @@ func (c *SPMoveCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "moveFileOrFolder", map[string]any{ - "sourceDriveId": c.SourceDriveID, - "sourceItemPath": c.SourceItemPath, - "destinationDriveId": c.DestinationDriveID, - "destinationFolderPath": c.DestinationFolderPath, - }) + resp, err := client.CallTool(ctx.Ctx, "moveFileOrFolder", args) if err != nil { return fmt.Errorf("move: %w", err) } @@ -538,6 +531,13 @@ type SPCopyCmd struct { } func (c *SPCopyCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "sourceDriveId": c.SourceDriveID, + "sourceItemPath": c.SourceItemPath, + "destinationDriveId": c.DestinationDriveID, + "destinationFolderPath": c.DestinationFolderPath, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "copyFileOrFolder", fmt.Sprintf("copy %s to %s", c.SourceItemPath, c.DestinationFolderPath), @@ -548,6 +548,7 @@ func (c *SPCopyCmd) Run(ctx *commands.Context) error { "destinationDriveId": c.DestinationDriveID, "destinationFolderPath": c.DestinationFolderPath, }, + args, ) } @@ -556,12 +557,7 @@ func (c *SPCopyCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "copyFileOrFolder", map[string]any{ - "sourceDriveId": c.SourceDriveID, - "sourceItemPath": c.SourceItemPath, - "destinationDriveId": c.DestinationDriveID, - "destinationFolderPath": c.DestinationFolderPath, - }) + resp, err := client.CallTool(ctx.Ctx, "copyFileOrFolder", args) if err != nil { return fmt.Errorf("copy: %w", err) } @@ -583,6 +579,12 @@ type SPRenameCmd struct { } func (c *SPRenameCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "itemPath": c.ItemPath, + "newName": c.NewName, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "renameFileOrFolder", fmt.Sprintf("rename %s to %q", c.ItemPath, c.NewName), @@ -592,6 +594,7 @@ func (c *SPRenameCmd) Run(ctx *commands.Context) error { "itemPath": c.ItemPath, "newName": c.NewName, }, + args, ) } @@ -600,11 +603,7 @@ func (c *SPRenameCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "renameFileOrFolder", map[string]any{ - "driveId": c.DriveID, - "itemPath": c.ItemPath, - "newName": c.NewName, - }) + resp, err := client.CallTool(ctx.Ctx, "renameFileOrFolder", args) if err != nil { return fmt.Errorf("rename: %w", err) } @@ -627,6 +626,13 @@ type SPShareCmd struct { } func (c *SPShareCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "itemPath": c.ItemPath, + "type": c.Type, + "scope": c.Scope, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "shareFileOrFolder", fmt.Sprintf("create %s/%s sharing link for %s", c.Type, c.Scope, c.ItemPath), @@ -637,6 +643,7 @@ func (c *SPShareCmd) Run(ctx *commands.Context) error { "type": c.Type, "scope": c.Scope, }, + args, ) } @@ -645,12 +652,7 @@ func (c *SPShareCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "shareFileOrFolder", map[string]any{ - "driveId": c.DriveID, - "itemPath": c.ItemPath, - "type": c.Type, - "scope": c.Scope, - }) + resp, err := client.CallTool(ctx.Ctx, "shareFileOrFolder", args) if err != nil { return fmt.Errorf("share: %w", err) } @@ -672,6 +674,12 @@ type SPLabelCmd struct { } func (c *SPLabelCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "itemId": c.ItemID, + "labelId": c.LabelID, + } + if ctx.DryRun { return ctx.ValidateDryRun(spEndpoint(), "setSensitivityLabelOnFile", fmt.Sprintf("set sensitivity label %s on item %s", c.LabelID, c.ItemID), @@ -681,6 +689,7 @@ func (c *SPLabelCmd) Run(ctx *commands.Context) error { "itemId": c.ItemID, "labelId": c.LabelID, }, + args, ) } @@ -689,11 +698,7 @@ func (c *SPLabelCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "setSensitivityLabelOnFile", map[string]any{ - "driveId": c.DriveID, - "itemId": c.ItemID, - "labelId": c.LabelID, - }) + resp, err := client.CallTool(ctx.Ctx, "setSensitivityLabelOnFile", args) if err != nil { return fmt.Errorf("set label: %w", err) } diff --git a/internal/commands/sharepoint/sharepoint_test.go b/internal/commands/sharepoint/sharepoint_test.go index 5c9c183..37616bf 100644 --- a/internal/commands/sharepoint/sharepoint_test.go +++ b/internal/commands/sharepoint/sharepoint_test.go @@ -90,3 +90,42 @@ func TestSPRmCmd_NoInput(t *testing.T) { t.Errorf("expected error about --force, got: %v", err) } } + +func TestSPWriteCmd_DryRunValidatesSelectedBinaryToolAndActualArgs(t *testing.T) { + schemas := []mcp.ToolInfo{ + { + Name: "createSmallBinaryFile", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "driveId": map[string]any{"type": "string"}, + "folderPath": map[string]any{"type": "string"}, + "fileName": map[string]any{"type": "string"}, + "contentBase64": map[string]any{"type": "string"}, + }, + "required": []any{"driveId", "folderPath", "fileName", "contentBase64"}, + }, + }, + } + ctx, buf := testutil.SetupTestServerWithSchemas(t, nil, schemas) + ctx.DryRun = true + + cmd := &SPWriteCmd{ + DriveID: "drive-001", + FolderPath: "/Documents", + FileName: "sample.bin", + ContentBase64: "AAE=", + } + if err := cmd.Run(ctx); err != nil { + t.Fatalf("Run() error: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + val, ok := result["validation"].(map[string]any) + if !ok || val["valid"] != true { + t.Fatalf("expected valid dry-run output, got %v", result["validation"]) + } +} diff --git a/internal/commands/splists/splists.go b/internal/commands/splists/splists.go index 0ebf6bc..75ae129 100644 --- a/internal/commands/splists/splists.go +++ b/internal/commands/splists/splists.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/sozercan/a365cli/internal/commands" + cmdexec "github.com/sozercan/a365cli/internal/commands/exec" "github.com/sozercan/a365cli/internal/config" "github.com/sozercan/a365cli/internal/output" ) @@ -197,20 +198,23 @@ type SPLCreateCmd struct { } func (c *SPLCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "siteId": c.SiteID, + "displayName": c.DisplayName, + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "createList", fmt.Sprintf("create list %q in site %s", c.DisplayName, c.SiteID), map[string]any{"action": "sp-lists.create-list", "siteId": c.SiteID, "displayName": c.DisplayName}, + args, ) } client := ctx.NewMCPClient(spListsEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createList", map[string]any{ - "siteId": c.SiteID, - "displayName": c.DisplayName, - }) + resp, err := client.CallTool(ctx.Ctx, "createList", args) if err != nil { return fmt.Errorf("create list: %w", err) } @@ -232,22 +236,25 @@ type SPLAddColumnCmd struct { } func (c *SPLAddColumnCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "siteId": c.SiteID, + "listId": c.ListID, + "name": c.Name, + "columnType": c.ColumnType, + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "createListColumn", fmt.Sprintf("add column %q (type %s) to list %s in site %s", c.Name, c.ColumnType, c.ListID, c.SiteID), map[string]any{"action": "sp-lists.add-column", "siteId": c.SiteID, "listId": c.ListID, "name": c.Name, "columnType": c.ColumnType}, + args, ) } client := ctx.NewMCPClient(spListsEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createListColumn", map[string]any{ - "siteId": c.SiteID, - "listId": c.ListID, - "name": c.Name, - "columnType": c.ColumnType, - }) + resp, err := client.CallTool(ctx.Ctx, "createListColumn", args) if err != nil { return fmt.Errorf("add column: %w", err) } @@ -273,21 +280,24 @@ func (c *SPLAddItemCmd) Run(ctx *commands.Context) error { return fmt.Errorf("invalid fields JSON: %w", err) } + args := map[string]any{ + "siteId": c.SiteID, + "listId": c.ListID, + "fields": fields, + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "createListItem", fmt.Sprintf("add item to list %s in site %s", c.ListID, c.SiteID), map[string]any{"action": "sp-lists.add-item", "siteId": c.SiteID, "listId": c.ListID, "fields": fields}, + args, ) } client := ctx.NewMCPClient(spListsEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "createListItem", map[string]any{ - "siteId": c.SiteID, - "listId": c.ListID, - "fields": fields, - }) + resp, err := client.CallTool(ctx.Ctx, "createListItem", args) if err != nil { return fmt.Errorf("add item: %w", err) } @@ -309,16 +319,6 @@ type SPLUpdateItemCmd struct { } func (c *SPLUpdateItemCmd) Run(ctx *commands.Context) error { - if ctx.DryRun { - return ctx.ValidateDryRun(spListsEndpoint(), "updateListItem", - fmt.Sprintf("update item %s in list %s (site %s)", c.ItemID, c.ListID, c.SiteID), - map[string]any{"action": "sp-lists.update-item", "siteId": c.SiteID, "listId": c.ListID, "itemId": c.ItemID}, - ) - } - client := ctx.NewMCPClient(spListsEndpoint()) - if err := client.Initialize(ctx.Ctx); err != nil { - return fmt.Errorf("initialize: %w", err) - } args := map[string]any{ "siteId": c.SiteID, "listId": c.ListID, @@ -331,15 +331,21 @@ func (c *SPLUpdateItemCmd) Run(ctx *commands.Context) error { } args["fields"] = fields } - resp, err := client.CallTool(ctx.Ctx, "updateListItem", args) - if err != nil { - return fmt.Errorf("update item: %w", err) - } - data, err := output.ExtractContent(resp) - if err != nil { - return err - } - return ctx.Output.PrintMutation("Item updated", data) + + return cmdexec.New(ctx).Mutate(cmdexec.OperationPlan{ + Service: "sp-lists", + Tool: "updateListItem", + Args: args, + Action: fmt.Sprintf("update item %s in list %s (site %s)", c.ItemID, c.ListID, c.SiteID), + Display: map[string]any{ + "action": "sp-lists.update-item", + "siteId": c.SiteID, + "listId": c.ListID, + "itemId": c.ItemID, + }, + ErrPrefix: "update item", + Output: cmdexec.Mutation("Item updated"), + }) } // --- EditCol --- @@ -353,24 +359,26 @@ type SPLEditColCmd struct { } func (c *SPLEditColCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "siteId": c.SiteID, + "listId": c.ListID, + "columnId": c.ColumnID, + } + if c.Name != "" { + args["name"] = c.Name + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "editListColumn", fmt.Sprintf("edit column %s in list %s (site %s)", c.ColumnID, c.ListID, c.SiteID), map[string]any{"action": "sp-lists.edit-column", "siteId": c.SiteID, "listId": c.ListID, "columnId": c.ColumnID}, + args, ) } client := ctx.NewMCPClient(spListsEndpoint()) if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "siteId": c.SiteID, - "listId": c.ListID, - "columnId": c.ColumnID, - } - if c.Name != "" { - args["name"] = c.Name - } resp, err := client.CallTool(ctx.Ctx, "editListColumn", args) if err != nil { return fmt.Errorf("edit column: %w", err) @@ -392,10 +400,17 @@ type SPLDeleteItemCmd struct { } func (c *SPLDeleteItemCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "siteId": c.SiteID, + "listId": c.ListID, + "itemId": c.ItemID, + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "deleteListItem", fmt.Sprintf("delete item %s from list %s (site %s)", c.ItemID, c.ListID, c.SiteID), map[string]any{"action": "sp-lists.delete-item", "siteId": c.SiteID, "listId": c.ListID, "itemId": c.ItemID}, + args, ) } if err := ctx.Confirm(fmt.Sprintf("delete item %s from list %s", c.ItemID, c.ListID)); err != nil { @@ -405,11 +420,7 @@ func (c *SPLDeleteItemCmd) Run(ctx *commands.Context) error { if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "deleteListItem", map[string]any{ - "siteId": c.SiteID, - "listId": c.ListID, - "itemId": c.ItemID, - }) + resp, err := client.CallTool(ctx.Ctx, "deleteListItem", args) if err != nil { return fmt.Errorf("delete item: %w", err) } @@ -430,10 +441,17 @@ type SPLDeleteColCmd struct { } func (c *SPLDeleteColCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "siteId": c.SiteID, + "listId": c.ListID, + "columnId": c.ColumnID, + } + if ctx.DryRun { return ctx.ValidateDryRun(spListsEndpoint(), "deleteListColumn", fmt.Sprintf("delete column %s from list %s (site %s)", c.ColumnID, c.ListID, c.SiteID), map[string]any{"action": "sp-lists.delete-column", "siteId": c.SiteID, "listId": c.ListID, "columnId": c.ColumnID}, + args, ) } if err := ctx.Confirm(fmt.Sprintf("delete column %s from list %s", c.ColumnID, c.ListID)); err != nil { @@ -443,11 +461,7 @@ func (c *SPLDeleteColCmd) Run(ctx *commands.Context) error { if err := client.Initialize(ctx.Ctx); err != nil { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "deleteListColumn", map[string]any{ - "siteId": c.SiteID, - "listId": c.ListID, - "columnId": c.ColumnID, - }) + resp, err := client.CallTool(ctx.Ctx, "deleteListColumn", args) if err != nil { return fmt.Errorf("delete column: %w", err) } diff --git a/internal/commands/splists/splists_test.go b/internal/commands/splists/splists_test.go index 8510cf0..d4212a9 100644 --- a/internal/commands/splists/splists_test.go +++ b/internal/commands/splists/splists_test.go @@ -95,3 +95,42 @@ func TestSPLAddColumnCmd_DryRun(t *testing.T) { t.Errorf("expected valid=true, got %v; errors: %v", val["valid"], val["errors"]) } } + +func TestSPLUpdateItemCmd_DryRunValidatesActualArgs(t *testing.T) { + schemas := []mcp.ToolInfo{ + { + Name: "updateListItem", + InputSchema: map[string]any{ + "type": "object", + "properties": map[string]any{ + "siteId": map[string]any{"type": "string"}, + "listId": map[string]any{"type": "string"}, + "itemId": map[string]any{"type": "string"}, + "fields": map[string]any{"type": "object"}, + }, + "required": []any{"siteId", "listId", "itemId", "fields"}, + }, + }, + } + ctx, buf := testutil.SetupTestServerWithSchemas(t, nil, schemas) + ctx.DryRun = true + + cmd := &SPLUpdateItemCmd{ + SiteID: "site-001", + ListID: "list-001", + ItemID: "item-001", + Fields: `{"Priority":"High"}`, + } + if err := cmd.Run(ctx); err != nil { + t.Fatalf("Run() error: %v", err) + } + + var result map[string]any + if err := json.Unmarshal(buf.Bytes(), &result); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + val, ok := result["validation"].(map[string]any) + if !ok || val["valid"] != true { + t.Fatalf("expected valid dry-run output, got %v", result["validation"]) + } +} diff --git a/internal/commands/teams/channels.go b/internal/commands/teams/channels.go index fc39ad5..83efca1 100644 --- a/internal/commands/teams/channels.go +++ b/internal/commands/teams/channels.go @@ -124,6 +124,12 @@ type ChannelsPostCmd struct { } func (c *ChannelsPostCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "channelId": c.ChannelID, + "content": c.Message, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "PostChannelMessage", fmt.Sprintf("post message to channel %s in team %s", c.ChannelID, c.TeamID), @@ -133,6 +139,7 @@ func (c *ChannelsPostCmd) Run(ctx *commands.Context) error { "channelId": c.ChannelID, "content": c.Message, }, + args, ) } @@ -141,11 +148,7 @@ func (c *ChannelsPostCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "PostChannelMessage", map[string]any{ - "teamId": c.TeamID, - "channelId": c.ChannelID, - "content": c.Message, - }) + resp, err := client.CallTool(ctx.Ctx, "PostChannelMessage", args) if err != nil { return fmt.Errorf("post channel message: %w", err) } @@ -166,6 +169,13 @@ type ChannelsReplyCmd struct { } func (c *ChannelsReplyCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "channelId": c.ChannelID, + "messageId": c.MessageID, + "content": c.Message, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "ReplyToChannelMessage", fmt.Sprintf("reply to message %s in channel %s", c.MessageID, c.ChannelID), @@ -176,6 +186,7 @@ func (c *ChannelsReplyCmd) Run(ctx *commands.Context) error { "messageId": c.MessageID, "content": c.Message, }, + args, ) } @@ -184,12 +195,7 @@ func (c *ChannelsReplyCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ReplyToChannelMessage", map[string]any{ - "teamId": c.TeamID, - "channelId": c.ChannelID, - "messageId": c.MessageID, - "content": c.Message, - }) + resp, err := client.CallTool(ctx.Ctx, "ReplyToChannelMessage", args) if err != nil { return fmt.Errorf("reply to channel message: %w", err) } @@ -211,6 +217,14 @@ type ChannelsCreateCmd struct { } func (c *ChannelsCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "displayName": c.DisplayName, + } + if c.Description != "" { + args["description"] = c.Description + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "CreateChannel", fmt.Sprintf("create channel %q in team %s", c.DisplayName, c.TeamID), @@ -220,6 +234,7 @@ func (c *ChannelsCreateCmd) Run(ctx *commands.Context) error { "displayName": c.DisplayName, "description": c.Description, }, + args, ) } @@ -228,14 +243,6 @@ func (c *ChannelsCreateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "teamId": c.TeamID, - "displayName": c.DisplayName, - } - if c.Description != "" { - args["description"] = c.Description - } - resp, err := client.CallTool(ctx.Ctx, "CreateChannel", args) if err != nil { return fmt.Errorf("create channel: %w", err) @@ -256,6 +263,14 @@ type ChannelsCreatePrivateCmd struct { } func (c *ChannelsCreatePrivateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "displayName": c.DisplayName, + } + if c.Description != "" { + args["description"] = c.Description + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "CreatePrivateChannel", fmt.Sprintf("create private channel %q in team %s", c.DisplayName, c.TeamID), @@ -265,6 +280,7 @@ func (c *ChannelsCreatePrivateCmd) Run(ctx *commands.Context) error { "displayName": c.DisplayName, "description": c.Description, }, + args, ) } @@ -273,14 +289,6 @@ func (c *ChannelsCreatePrivateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "teamId": c.TeamID, - "displayName": c.DisplayName, - } - if c.Description != "" { - args["description"] = c.Description - } - resp, err := client.CallTool(ctx.Ctx, "CreatePrivateChannel", args) if err != nil { return fmt.Errorf("create private channel: %w", err) @@ -302,6 +310,17 @@ type ChannelsUpdateCmd struct { } func (c *ChannelsUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "channelId": c.ChannelID, + } + if c.DisplayName != "" { + args["displayName"] = c.DisplayName + } + if c.Description != "" { + args["description"] = c.Description + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "UpdateChannel", fmt.Sprintf("update channel %s in team %s", c.ChannelID, c.TeamID), @@ -312,6 +331,7 @@ func (c *ChannelsUpdateCmd) Run(ctx *commands.Context) error { "displayName": c.DisplayName, "description": c.Description, }, + args, ) } @@ -320,17 +340,6 @@ func (c *ChannelsUpdateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "teamId": c.TeamID, - "channelId": c.ChannelID, - } - if c.DisplayName != "" { - args["displayName"] = c.DisplayName - } - if c.Description != "" { - args["description"] = c.Description - } - resp, err := client.CallTool(ctx.Ctx, "UpdateChannel", args) if err != nil { return fmt.Errorf("update channel: %w", err) @@ -393,6 +402,12 @@ type ChannelsAddMemberCmd struct { } func (c *ChannelsAddMemberCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "channelId": c.ChannelID, + "userId": c.UserID, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "AddChannelMember", fmt.Sprintf("add member %s to channel %s", c.UserID, c.ChannelID), @@ -402,6 +417,7 @@ func (c *ChannelsAddMemberCmd) Run(ctx *commands.Context) error { "channelId": c.ChannelID, "userId": c.UserID, }, + args, ) } @@ -410,11 +426,7 @@ func (c *ChannelsAddMemberCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "AddChannelMember", map[string]any{ - "teamId": c.TeamID, - "channelId": c.ChannelID, - "userId": c.UserID, - }) + resp, err := client.CallTool(ctx.Ctx, "AddChannelMember", args) if err != nil { return fmt.Errorf("add channel member: %w", err) } @@ -435,6 +447,13 @@ type ChannelsUpdateMemberCmd struct { } func (c *ChannelsUpdateMemberCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "teamId": c.TeamID, + "channelId": c.ChannelID, + "membershipId": c.MembershipID, + "role": c.Role, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "UpdateChannelMember", fmt.Sprintf("update member %s role to %s in channel %s", c.MembershipID, c.Role, c.ChannelID), @@ -445,6 +464,7 @@ func (c *ChannelsUpdateMemberCmd) Run(ctx *commands.Context) error { "membershipId": c.MembershipID, "role": c.Role, }, + args, ) } @@ -453,12 +473,7 @@ func (c *ChannelsUpdateMemberCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateChannelMember", map[string]any{ - "teamId": c.TeamID, - "channelId": c.ChannelID, - "membershipId": c.MembershipID, - "role": c.Role, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateChannelMember", args) if err != nil { return fmt.Errorf("update channel member: %w", err) } diff --git a/internal/commands/teams/chats.go b/internal/commands/teams/chats.go index cb12bdc..14e4d91 100644 --- a/internal/commands/teams/chats.go +++ b/internal/commands/teams/chats.go @@ -130,6 +130,11 @@ type ChatsSendCmd struct { } func (c *ChatsSendCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + "content": c.Message, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "PostMessage", fmt.Sprintf("send message to chat %s", c.ChatID), @@ -138,6 +143,7 @@ func (c *ChatsSendCmd) Run(ctx *commands.Context) error { "chatId": c.ChatID, "content": c.Message, }, + args, ) } @@ -146,10 +152,7 @@ func (c *ChatsSendCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "PostMessage", map[string]any{ - "chatId": c.ChatID, - "content": c.Message, - }) + resp, err := client.CallTool(ctx.Ctx, "PostMessage", args) if err != nil { return fmt.Errorf("send message: %w", err) } @@ -167,11 +170,15 @@ type ChatsSendSelfCmd struct { } func (c *ChatsSendSelfCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "content": c.Message, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "SendMessageToSelf", "send message to self", map[string]any{ "action": "chats.send-self", "content": c.Message, - }) + }, args) } client := ctx.NewMCPClient(teamsEndpoint()) @@ -179,9 +186,7 @@ func (c *ChatsSendSelfCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "SendMessageToSelf", map[string]any{ - "content": c.Message, - }) + resp, err := client.CallTool(ctx.Ctx, "SendMessageToSelf", args) if err != nil { return fmt.Errorf("send to self: %w", err) } @@ -230,6 +235,14 @@ type ChatsCreateCmd struct { } func (c *ChatsCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatType": c.Type, + "members_upns": c.Members, + } + if c.Topic != "" { + args["topic"] = c.Topic + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "CreateChat", fmt.Sprintf("create %s chat with %v", c.Type, c.Members), @@ -239,6 +252,7 @@ func (c *ChatsCreateCmd) Run(ctx *commands.Context) error { "members_upns": c.Members, "topic": c.Topic, }, + args, ) } @@ -247,14 +261,6 @@ func (c *ChatsCreateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - args := map[string]any{ - "chatType": c.Type, - "members_upns": c.Members, - } - if c.Topic != "" { - args["topic"] = c.Topic - } - resp, err := client.CallTool(ctx.Ctx, "CreateChat", args) if err != nil { return fmt.Errorf("create chat: %w", err) @@ -273,10 +279,15 @@ type ChatsDeleteCmd struct { } func (c *ChatsDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "DeleteChat", fmt.Sprintf("delete chat %s", c.ChatID), map[string]any{"action": "chats.delete", "chatId": c.ChatID}, + args, ) } @@ -289,9 +300,7 @@ func (c *ChatsDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeleteChat", map[string]any{ - "chatId": c.ChatID, - }) + resp, err := client.CallTool(ctx.Ctx, "DeleteChat", args) if err != nil { return fmt.Errorf("delete chat: %w", err) } @@ -310,10 +319,16 @@ type ChatsUpdateCmd struct { } func (c *ChatsUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + "topic": c.Topic, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "UpdateChat", fmt.Sprintf("update chat %s topic to %q", c.ChatID, c.Topic), map[string]any{"action": "chats.update", "chatId": c.ChatID, "topic": c.Topic}, + args, ) } @@ -322,10 +337,7 @@ func (c *ChatsUpdateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateChat", map[string]any{ - "chatId": c.ChatID, - "topic": c.Topic, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateChat", args) if err != nil { return fmt.Errorf("update chat: %w", err) } @@ -347,6 +359,12 @@ type ChatsUpdateMessageCmd struct { } func (c *ChatsUpdateMessageCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + "messageId": c.MessageID, + "content": c.Content, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "UpdateChatMessage", fmt.Sprintf("update message %s in chat %s", c.MessageID, c.ChatID), @@ -356,6 +374,7 @@ func (c *ChatsUpdateMessageCmd) Run(ctx *commands.Context) error { "messageId": c.MessageID, "content": c.Content, }, + args, ) } @@ -364,11 +383,7 @@ func (c *ChatsUpdateMessageCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "UpdateChatMessage", map[string]any{ - "chatId": c.ChatID, - "messageId": c.MessageID, - "content": c.Content, - }) + resp, err := client.CallTool(ctx.Ctx, "UpdateChatMessage", args) if err != nil { return fmt.Errorf("update message: %w", err) } @@ -387,10 +402,16 @@ type ChatsDeleteMessageCmd struct { } func (c *ChatsDeleteMessageCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + "messageId": c.MessageID, + } + if ctx.DryRun { return ctx.ValidateDryRun(teamsEndpoint(), "DeleteChatMessage", fmt.Sprintf("delete message %s in chat %s", c.MessageID, c.ChatID), map[string]any{"action": "chats.delete-message", "chatId": c.ChatID, "messageId": c.MessageID}, + args, ) } @@ -403,10 +424,7 @@ func (c *ChatsDeleteMessageCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "DeleteChatMessage", map[string]any{ - "chatId": c.ChatID, - "messageId": c.MessageID, - }) + resp, err := client.CallTool(ctx.Ctx, "DeleteChatMessage", args) if err != nil { return fmt.Errorf("delete message: %w", err) } @@ -457,17 +475,18 @@ type ChatsAddMemberCmd struct { } func (c *ChatsAddMemberCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "chatId": c.ChatID, + "roles": c.Roles, + "userodata_bind": fmt.Sprintf("https://graph.microsoft.com/v1.0/users('%s')", c.UPN), + "odata_type": "#microsoft.graph.aadUserConversationMember", + } + if ctx.DryRun { - mcpArgs := map[string]any{ - "chatId": c.ChatID, - "roles": c.Roles, - "userodata_bind": fmt.Sprintf("https://graph.microsoft.com/v1.0/users('%s')", c.UPN), - "odata_type": "#microsoft.graph.aadUserConversationMember", - } return ctx.ValidateDryRun(teamsEndpoint(), "AddChatMember", fmt.Sprintf("add member %s to chat %s", c.UPN, c.ChatID), map[string]any{"action": "chats.add-member", "chatId": c.ChatID, "upn": c.UPN, "roles": c.Roles}, - mcpArgs, + args, ) } @@ -476,12 +495,7 @@ func (c *ChatsAddMemberCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "AddChatMember", map[string]any{ - "chatId": c.ChatID, - "roles": c.Roles, - "userodata_bind": fmt.Sprintf("https://graph.microsoft.com/v1.0/users('%s')", c.UPN), - "odata_type": "#microsoft.graph.aadUserConversationMember", - }) + resp, err := client.CallTool(ctx.Ctx, "AddChatMember", args) if err != nil { return fmt.Errorf("add chat member: %w", err) } diff --git a/internal/commands/triggers/triggers.go b/internal/commands/triggers/triggers.go index 33e0bd0..d16dd5a 100644 --- a/internal/commands/triggers/triggers.go +++ b/internal/commands/triggers/triggers.go @@ -115,22 +115,23 @@ type TriggersCreateCmd struct { } func (c *TriggersCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "validationToken": c.ValidationToken, + "name": c.Name, + "eventType": c.EventType, + "logic": c.Logic, + "conditions": c.Conditions, + "instructions": c.Instructions, + } + if ctx.DryRun { - mcpArgs := map[string]any{ - "validationToken": c.ValidationToken, - "name": c.Name, - "eventType": c.EventType, - "logic": c.Logic, - "conditions": c.Conditions, - "instructions": c.Instructions, - } return ctx.ValidateDryRun(triggersEndpoint(), "create_trigger_definition", fmt.Sprintf("create trigger %q", c.Name), map[string]any{ "action": "triggers.create", "name": c.Name, "eventType": c.EventType, }, - mcpArgs, + args, ) } @@ -139,14 +140,7 @@ func (c *TriggersCreateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "create_trigger_definition", map[string]any{ - "validationToken": c.ValidationToken, - "name": c.Name, - "eventType": c.EventType, - "logic": c.Logic, - "conditions": c.Conditions, - "instructions": c.Instructions, - }) + resp, err := client.CallTool(ctx.Ctx, "create_trigger_definition", args) if err != nil { return fmt.Errorf("create trigger: %w", err) } @@ -217,17 +211,18 @@ type TriggersUpdateCmd struct { } func (c *TriggersUpdateCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "validationToken": c.ValidationToken, + "id": c.ID, + } + if ctx.DryRun { - mcpArgs := map[string]any{ - "validationToken": c.ValidationToken, - "id": c.ID, - } return ctx.ValidateDryRun(triggersEndpoint(), "update_trigger_definition", fmt.Sprintf("update trigger %s", c.ID), map[string]any{ "action": "triggers.update", "id": c.ID, }, - mcpArgs, + args, ) } @@ -236,10 +231,7 @@ func (c *TriggersUpdateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "update_trigger_definition", map[string]any{ - "validationToken": c.ValidationToken, - "id": c.ID, - }) + resp, err := client.CallTool(ctx.Ctx, "update_trigger_definition", args) if err != nil { return fmt.Errorf("update trigger: %w", err) } @@ -259,12 +251,18 @@ type TriggersDeleteCmd struct { } func (c *TriggersDeleteCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "id": c.ID, + } + if ctx.DryRun { return ctx.ValidateDryRun(triggersEndpoint(), "delete_trigger_definition", fmt.Sprintf("delete trigger %s", c.ID), map[string]any{ "action": "triggers.delete", "id": c.ID, - }) + }, + args, + ) } if err := ctx.Confirm(fmt.Sprintf("delete trigger %s", c.ID)); err != nil { @@ -276,9 +274,7 @@ func (c *TriggersDeleteCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "delete_trigger_definition", map[string]any{ - "id": c.ID, - }) + resp, err := client.CallTool(ctx.Ctx, "delete_trigger_definition", args) if err != nil { return fmt.Errorf("delete trigger: %w", err) } diff --git a/internal/commands/word/word.go b/internal/commands/word/word.go index eb6080a..8c1291f 100644 --- a/internal/commands/word/word.go +++ b/internal/commands/word/word.go @@ -26,10 +26,13 @@ type WordCreateCmd struct { } func (c *WordCreateCmd) Run(ctx *commands.Context) error { + args := map[string]any{"desiredFileName": c.FileName} + if ctx.DryRun { return ctx.ValidateDryRun(wordEndpoint(), "CreateDocument", fmt.Sprintf("create Word document %q", c.FileName), map[string]any{"action": "word.create", "desiredFileName": c.FileName}, + args, ) } @@ -38,9 +41,7 @@ func (c *WordCreateCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "CreateDocument", map[string]any{ - "desiredFileName": c.FileName, - }) + resp, err := client.CallTool(ctx.Ctx, "CreateDocument", args) if err != nil { return fmt.Errorf("create document: %w", err) } @@ -87,10 +88,17 @@ type WordCommentCmd struct { } func (c *WordCommentCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "driveId": c.DriveID, + "documentId": c.DocumentID, + "text": c.Text, + } + if ctx.DryRun { return ctx.ValidateDryRun(wordEndpoint(), "AddComment", fmt.Sprintf("add comment to document %s", c.DocumentID), map[string]any{"action": "word.comment", "driveId": c.DriveID, "documentId": c.DocumentID}, + args, ) } @@ -99,11 +107,7 @@ func (c *WordCommentCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "AddComment", map[string]any{ - "driveId": c.DriveID, - "documentId": c.DocumentID, - "text": c.Text, - }) + resp, err := client.CallTool(ctx.Ctx, "AddComment", args) if err != nil { return fmt.Errorf("add comment: %w", err) } @@ -124,10 +128,18 @@ type WordReplyCmd struct { } func (c *WordReplyCmd) Run(ctx *commands.Context) error { + args := map[string]any{ + "commentId": c.CommentID, + "driveId": c.DriveID, + "documentId": c.DocumentID, + "text": c.Text, + } + if ctx.DryRun { return ctx.ValidateDryRun(wordEndpoint(), "ReplyToComment", fmt.Sprintf("reply to comment %s on document %s", c.CommentID, c.DocumentID), map[string]any{"action": "word.reply", "commentId": c.CommentID, "driveId": c.DriveID, "documentId": c.DocumentID}, + args, ) } @@ -136,12 +148,7 @@ func (c *WordReplyCmd) Run(ctx *commands.Context) error { return fmt.Errorf("initialize: %w", err) } - resp, err := client.CallTool(ctx.Ctx, "ReplyToComment", map[string]any{ - "commentId": c.CommentID, - "driveId": c.DriveID, - "documentId": c.DocumentID, - "text": c.Text, - }) + resp, err := client.CallTool(ctx.Ctx, "ReplyToComment", args) if err != nil { return fmt.Errorf("reply to comment: %w", err) } diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 3cdc4e9..e5d80ab 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "sync" "testing" "github.com/sozercan/a365cli/internal/commands" @@ -28,6 +29,81 @@ func SetupTestServer(t *testing.T, toolResponses map[string]string) (*commands.C // tools/list returns an empty list. func SetupTestServerWithSchemas(t *testing.T, toolResponses map[string]string, toolSchemas []mcp.ToolInfo) (*commands.Context, *bytes.Buffer) { t.Helper() + ctx, buf, _ := setupTestServer(t, toolResponses, toolSchemas, nil) + return ctx, buf +} + +// RecordedCall captures a tools/call request sent to the mock MCP server. +type RecordedCall struct { + Name string + Arguments map[string]any +} + +// MCPRecorder records tools/call requests for command tests. +type MCPRecorder struct { + mu sync.Mutex + calls []RecordedCall +} + +// SetupTestServerWithRecorder creates a mock MCP server and records tools/call requests. +func SetupTestServerWithRecorder(t *testing.T, toolResponses map[string]string) (*commands.Context, *bytes.Buffer, *MCPRecorder) { + t.Helper() + return SetupTestServerWithSchemasAndRecorder(t, toolResponses, nil) +} + +// SetupTestServerWithSchemasAndRecorder creates a mock MCP server with schemas +// and records tools/call requests. +func SetupTestServerWithSchemasAndRecorder(t *testing.T, toolResponses map[string]string, toolSchemas []mcp.ToolInfo) (*commands.Context, *bytes.Buffer, *MCPRecorder) { + t.Helper() + recorder := &MCPRecorder{} + return setupTestServer(t, toolResponses, toolSchemas, recorder) +} + +// Calls returns a snapshot of all recorded tools/call requests. +func (r *MCPRecorder) Calls() []RecordedCall { + if r == nil { + return nil + } + r.mu.Lock() + defer r.mu.Unlock() + calls := make([]RecordedCall, len(r.calls)) + for i, call := range r.calls { + calls[i] = RecordedCall{Name: call.Name, Arguments: cloneMap(call.Arguments)} + } + return calls +} + +// LastCall returns the most recent recorded tools/call request. +func (r *MCPRecorder) LastCall() (RecordedCall, bool) { + calls := r.Calls() + if len(calls) == 0 { + return RecordedCall{}, false + } + return calls[len(calls)-1], true +} + +// Count returns how many tools/call requests matched a tool name. +func (r *MCPRecorder) Count(name string) int { + count := 0 + for _, call := range r.Calls() { + if call.Name == name { + count++ + } + } + return count +} + +func (r *MCPRecorder) record(name string, arguments map[string]any) { + if r == nil { + return + } + r.mu.Lock() + defer r.mu.Unlock() + r.calls = append(r.calls, RecordedCall{Name: name, Arguments: cloneMap(arguments)}) +} + +func setupTestServer(t *testing.T, toolResponses map[string]string, toolSchemas []mcp.ToolInfo, recorder *MCPRecorder) (*commands.Context, *bytes.Buffer, *MCPRecorder) { + t.Helper() server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) @@ -68,6 +144,7 @@ func SetupTestServerWithSchemas(t *testing.T, toolResponses map[string]string, t Arguments map[string]any `json:"arguments"` } json.Unmarshal(req.Params, ¶ms) //nolint:errcheck + recorder.record(params.Name, params.Arguments) respText, ok := toolResponses[params.Name] if !ok { @@ -116,10 +193,32 @@ func SetupTestServerWithSchemas(t *testing.T, toolResponses map[string]string, t return "test-token", nil }, Output: &output.Formatter{Format: output.FormatJSON, Writer: &buf}, - UserUPN: "test@example.com", + UserUPN: "user@contoso.com", } - return ctx, &buf + return ctx, &buf, recorder +} + +func cloneMap(in map[string]any) map[string]any { + if in == nil { + return nil + } + b, err := json.Marshal(in) + if err != nil { + out := make(map[string]any, len(in)) + for k, v := range in { + out[k] = v + } + return out + } + var out map[string]any + if err := json.Unmarshal(b, &out); err != nil { + out = make(map[string]any, len(in)) + for k, v := range in { + out[k] = v + } + } + return out } // MustJSON marshals v to a JSON string, panicking on error.