diff --git a/README.md b/README.md index 12b1b328..7d394897 100644 --- a/README.md +++ b/README.md @@ -698,6 +698,8 @@ The usual flow is plural to find an ID, then singular with that ID. The split ke `gateway_events_read` and `gateway_requests_read` **list** actions support the same filters as `hookdeck gateway event list` and `hookdeck gateway request list` — including payload search (`body`, `headers`, `parsed_query`, `path`) and date windows via `*_after` / `*_before` (ISO 8601; maps to API `field[gte]` / `field[lte]`). See `gateway_help` with topic `gateway_events_read` or `gateway_requests_read` for the full parameter list. +Both **list** actions also take `include: "data"`, which returns each event's or request's payload (headers, body, path, parsed query) in the list itself, so an agent that needs several payloads makes one call instead of one per record. Payloads make responses much larger, so pair it with a small `limit` and page with `next`. When `include` is set without a `limit`, the tool sends `limit=10` rather than the API's default page size. `list_ignored` does not take it: its route does not declare `include`. On the command line, the same option is `--include data` on `gateway event list`, `gateway request list` and `gateway request events`; it requires `--output json`, and the limit defaults to 10 there too. + The only relationship traversal the API supports is request → events, and `gateway_events_read` owns both directions of it. Pass `request_id` with action `list` for the events a request produced, or action `list_ignored` for the ones a connection filter dropped. `GET /events` declares no `request_id` filter, so the tool queries the request's own events route instead — it takes the same filters, so every argument still applies (`list_ignored`'s route is the exception: it takes `id`, paging and ordering only). There is no `event_id` filter on requests. To go the other way, read `request_id` off an event and call `gateway_request_read` with action `get`. `gateway_help` reports which mode the session is in and lists only the actions it can perform. diff --git a/REFERENCE.md b/REFERENCE.md index ba436a18..7937483e 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -1593,6 +1593,9 @@ List events (processed webhook deliveries). Filter by connection ID, source, des Use `--search-term` to find a complete value across the body, headers, parsed query or path at once, when you know the value but not which field carries it. +Use `--include`=data with `--output`=json to get each event's payload in the results, instead of +fetching events one at a time. Without `--limit`, the limit is then 10 instead of 100. + **Usage:** ```bash @@ -1615,6 +1618,7 @@ hookdeck gateway event list [flags] | `--error-code` | `string` | Filter by error code | | `--headers` | `string` | Filter by headers (JSON string) | | `--id` | `string` | Filter by event ID(s) (comma-separated) | +| `--include` | `string` | Set to "data" to include each event's payload (headers, body, path, parsed query) in the results. Requires `--output` json. Responses grow with every payload: when `--limit` is not set it defaults to 10 instead of 100 | | `--issue-id` | `string` | Filter by issue ID | | `--last-attempt-at-after` | `string` | Filter by last_attempt_at after (ISO date-time) | | `--last-attempt-at-before` | `string` | Filter by last_attempt_at before (ISO date-time) | @@ -1642,6 +1646,7 @@ hookdeck gateway event list --connection-id web_abc123 hookdeck gateway event list --status FAILED --limit 20 hookdeck gateway event list --search-term cus_1234 hookdeck gateway event list --status QUEUED --next-attempt-at-before 2026-01-01T00:00:00Z +hookdeck gateway event list --status FAILED --include data --output json ``` ### hookdeck gateway event get @@ -1743,6 +1748,9 @@ Use `--search-term` to find a complete value across the body, headers, parsed qu at once. `--events-count` 0 finds requests that produced no events, which is the usual reason a webhook appears to have gone missing. +Use `--include`=data with `--output`=json to get each request's payload in the results, instead of +fetching requests one at a time. Without `--limit`, the limit is then 10 instead of 100. + **Usage:** ```bash @@ -1762,6 +1770,7 @@ hookdeck gateway request list [flags] | `--headers` | `string` | Filter by headers (JSON string) | | `--id` | `string` | Filter by request ID(s) (comma-separated) | | `--ignored-count` | `string` | Filter by number of ignored events. A whole number | +| `--include` | `string` | Set to "data" to include each request's payload (headers, body, path, parsed query) in the results. Requires `--output` json. Responses grow with every payload: when `--limit` is not set it defaults to 10 instead of 100 | | `--ingested-at-after` | `string` | Filter by ingested_at after (ISO date-time) | | `--ingested-at-before` | `string` | Filter by ingested_at before (ISO date-time) | | `--limit` | `int` | Limit number of results (default "100") | @@ -1784,6 +1793,7 @@ hookdeck gateway request list hookdeck gateway request list --source-id src_abc123 --limit 20 hookdeck gateway request list --search-term cus_1234 hookdeck gateway request list --events-count 0 +hookdeck gateway request list --source-id src_abc123 --include data --output json ``` ### hookdeck gateway request get @@ -1835,6 +1845,12 @@ List events (deliveries) created from a request. Filters match `hookdeck gateway event list`: this command queries the same event collection, narrowed to one request. +Use `--include`=data with `--output`=json to get each event's payload in the results. Without +`--limit`, the limit is then 10 instead of 100. + +When the request has more events than fit on one page, a note on stderr gives the cursor to +pass to `--next` for the following page. + **Usage:** ```bash @@ -1856,6 +1872,7 @@ hookdeck gateway request events [flags] | `--dir` | `string` | Sort direction (asc, desc) | | `--error-code` | `string` | Filter by error code | | `--headers` | `string` | Filter by headers (JSON string) | +| `--include` | `string` | Set to "data" to include each event's payload (headers, body, path, parsed query) in the results. Requires `--output` json. Responses grow with every payload: when `--limit` is not set it defaults to 10 instead of 100 | | `--issue-id` | `string` | Filter by issue ID | | `--last-attempt-at-after` | `string` | Filter by last_attempt_at after (ISO date-time) | | `--last-attempt-at-before` | `string` | Filter by last_attempt_at before (ISO date-time) | @@ -1881,6 +1898,7 @@ hookdeck gateway request events [flags] hookdeck gateway request events req_abc123 hookdeck gateway request events req_abc123 --status FAILED hookdeck gateway request events req_abc123 --destination-id des_abc123 +hookdeck gateway request events req_abc123 --include data --output json ``` ### hookdeck gateway request ignored-events diff --git a/pkg/cmd/event_list.go b/pkg/cmd/event_list.go index aaadd5e8..deecdb0c 100644 --- a/pkg/cmd/event_list.go +++ b/pkg/cmd/event_list.go @@ -39,6 +39,7 @@ type eventListCmd struct { path string parsedQuery string searchTerm string + include string orderBy string dir string limit int @@ -59,12 +60,16 @@ func newEventListCmd() *eventListCmd { Use --search-term to find a complete value across the body, headers, parsed query or path at once, when you know the value but not which field carries it. +Use --include=data with --output=json to get each event's payload in the results, instead of +fetching events one at a time. Without --limit, the limit is then 10 instead of 100. + Examples: hookdeck gateway event list hookdeck gateway event list --connection-id web_abc123 hookdeck gateway event list --status FAILED --limit 20 hookdeck gateway event list --search-term cus_1234 - hookdeck gateway event list --status QUEUED --next-attempt-at-before 2026-01-01T00:00:00Z`, + hookdeck gateway event list --status QUEUED --next-attempt-at-before 2026-01-01T00:00:00Z + hookdeck gateway event list --status FAILED --include data --output json`, RunE: ec.runEventListCmd, } @@ -92,6 +97,7 @@ Examples: ec.cmd.Flags().StringVar(&ec.path, "path", "", "Filter by path") ec.cmd.Flags().StringVar(&ec.parsedQuery, "parsed-query", "", "Filter by parsed query (JSON string)") ec.cmd.Flags().StringVar(&ec.searchTerm, "search-term", "", "Match a whole value in body, headers, parsed query or path. Not a substring (min 3 characters)") + ec.cmd.Flags().StringVar(&ec.include, "include", "", includeFlagUsage("event")) ec.cmd.Flags().StringVar(&ec.orderBy, "order-by", "", "Sort key (e.g. created_at)") ec.cmd.Flags().StringVar(&ec.dir, "dir", "", "Sort direction (asc, desc)") ec.cmd.Flags().IntVar(&ec.limit, "limit", 100, "Limit number of results") @@ -197,7 +203,11 @@ func (ec *eventListCmd) runEventListCmd(cmd *cobra.Command, args []string) error if ec.dir != "" { params["dir"] = ec.dir } - params["limit"] = strconv.Itoa(ec.limit) + limit, err := applyIncludeData(cmd, ec.include, ec.output, ec.limit, params) + if err != nil { + return err + } + params["limit"] = strconv.Itoa(limit) if ec.next != "" { params["next"] = ec.next } diff --git a/pkg/cmd/gateway_list_filters_test.go b/pkg/cmd/gateway_list_filters_test.go index f9a267b6..7f4a8478 100644 --- a/pkg/cmd/gateway_list_filters_test.go +++ b/pkg/cmd/gateway_list_filters_test.go @@ -26,6 +26,16 @@ import ( // filter, because the answer looks right. func captureListQuery(t *testing.T, args ...string) url.Values { t.Helper() + got, err := runListQuery(t, args...) + require.NoError(t, err) + require.NotNil(t, got, "the command made no list request") + return got +} + +// runListQuery is captureListQuery for a command that may fail: it returns the +// error and the query sent, which is nil when no list request was made. +func runListQuery(t *testing.T, args ...string) (url.Values, error) { + t.Helper() var got url.Values server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -60,9 +70,8 @@ func captureListQuery(t *testing.T, args ...string) url.Values { root.SetOut(io.Discard) root.SetErr(io.Discard) - require.NoError(t, root.Execute()) - require.NotNil(t, got, "the command made no list request") - return got + err := root.Execute() + return got, err } func TestEventListSendsNewFilters(t *testing.T) { @@ -128,13 +137,16 @@ func TestListFiltersAreOmittedWhenUnset(t *testing.T) { // forgot, which is how they would get added — this pins the omission as // intentional. The MCP side is pinned by // TestPluralToolsOmitParametersHiddenFromTheAPIDocs. +// +// include is the exception. It carries x-docs-hide too, but Hookdeck's API team +// has confirmed it can be exposed; see TestListIncludeData. func TestListCommandsOmitFlagsHiddenFromTheAPIDocs(t *testing.T) { root := &cobra.Command{Use: "hookdeck"} root.AddCommand(newGatewayCmd().cmd) cases := map[string][]string{ - "event": {"bulk-retry-id", "include", "progressive", "event-data-id", "cli-user-id"}, - "request": {"bulk-retry-id", "include", "progressive"}, + "event": {"bulk-retry-id", "progressive", "event-data-id", "cli-user-id"}, + "request": {"bulk-retry-id", "progressive"}, } for resource, hidden := range cases { @@ -151,3 +163,37 @@ func TestListCommandsOmitFlagsHiddenFromTheAPIDocs(t *testing.T) { }) } } + +// TestListIncludeData covers --include on both list commands, which share +// applyIncludeData but each call it separately, so each is tested. +func TestListIncludeData(t *testing.T) { + for _, resource := range []string{"event", "request"} { + t.Run(resource+" sends include and the smaller default limit", func(t *testing.T) { + query := captureListQuery(t, "gateway", resource, "list", "--include", "data", "--output", "json") + assert.Equal(t, "data", query.Get("include")) + assert.Equal(t, "10", query.Get("limit"), + "a page of payloads must not default to 100") + }) + + t.Run(resource+" honours an explicit limit", func(t *testing.T) { + query := captureListQuery(t, "gateway", resource, "list", "--include", "data", "--output", "json", "--limit", "50") + assert.Equal(t, "data", query.Get("include")) + assert.Equal(t, "50", query.Get("limit")) + }) + + t.Run(resource+" without include is unchanged", func(t *testing.T) { + query := captureListQuery(t, "gateway", resource, "list") + assert.NotContains(t, query, "include") + assert.Equal(t, "100", query.Get("limit")) + }) + + // The list output prints IDs and a summary line, so payloads fetched + // for it would be thrown away. Refused before any request is made. + t.Run(resource+" refuses include without --output json", func(t *testing.T) { + query, err := runListQuery(t, "gateway", resource, "list", "--include", "data") + require.Error(t, err) + assert.Contains(t, err.Error(), "--output json") + assert.Nil(t, query, "no list request should be made") + }) + } +} diff --git a/pkg/cmd/list_include.go b/pkg/cmd/list_include.go new file mode 100644 index 00000000..5725be6b --- /dev/null +++ b/pkg/cmd/list_include.go @@ -0,0 +1,44 @@ +package cmd + +import ( + "errors" + "strconv" + + "github.com/spf13/cobra" +) + +// includeDataDefaultLimit is the page size sent when --include is set without +// --limit, matching the MCP list tools. +// +// The usual default of 100 is fine for IDs and metadata. With payloads it is a +// hundred webhook bodies in one response, which is more than a terminal or a +// script piping to jq wants by accident. An explicit --limit is honoured. +const includeDataDefaultLimit = 10 + +// includeFlagUsage is the --include help for event list and request list. +func includeFlagUsage(resource string) string { + return `Set to "data" to include each ` + resource + `'s payload (headers, body, path, parsed query) in the results. ` + + `Requires --output json. Responses grow with every payload: when --limit is not set it defaults to ` + + strconv.Itoa(includeDataDefaultLimit) + ` instead of 100` +} + +// applyIncludeData adds include to the list query and returns the limit to +// send. +// +// The API declares include on GET /events and GET /requests with one value, +// "data"; any other value is left for the API to reject. --include is refused +// without --output json because the list output prints IDs and a summary +// line, so the payloads would be fetched and then not shown. +func applyIncludeData(cmd *cobra.Command, include, output string, limit int, params map[string]string) (int, error) { + if include == "" { + return limit, nil + } + if output != "json" { + return 0, errors.New("--include requires --output json: the list output does not show payloads") + } + params["include"] = include + if !cmd.Flags().Changed("limit") { + return includeDataDefaultLimit, nil + } + return limit, nil +} diff --git a/pkg/cmd/request_events.go b/pkg/cmd/request_events.go index 678e7b4d..d0b38bff 100644 --- a/pkg/cmd/request_events.go +++ b/pkg/cmd/request_events.go @@ -45,6 +45,7 @@ type requestEventsCmd struct { parsedQuery string orderBy string dir string + include string } func newRequestEventsCmd() *requestEventsCmd { @@ -59,10 +60,17 @@ func newRequestEventsCmd() *requestEventsCmd { Filters match ` + "`hookdeck gateway event list`" + `: this command queries the same event collection, narrowed to one request. +Use --include=data with --output=json to get each event's payload in the results. Without +--limit, the limit is then 10 instead of 100. + +When the request has more events than fit on one page, a note on stderr gives the cursor to +pass to --next for the following page. + Examples: hookdeck gateway request events req_abc123 hookdeck gateway request events req_abc123 --status FAILED - hookdeck gateway request events req_abc123 --destination-id des_abc123`, + hookdeck gateway request events req_abc123 --destination-id des_abc123 + hookdeck gateway request events req_abc123 --include data --output json`, RunE: rc.runRequestEventsCmd, } @@ -97,6 +105,7 @@ Examples: rc.cmd.Flags().StringVar(&rc.body, "body", "", "Filter by body (JSON string)") rc.cmd.Flags().StringVar(&rc.path, "path", "", "Filter by path") rc.cmd.Flags().StringVar(&rc.parsedQuery, "parsed-query", "", "Filter by parsed query (JSON string)") + rc.cmd.Flags().StringVar(&rc.include, "include", "", includeFlagUsage("event")) rc.cmd.Flags().StringVar(&rc.orderBy, "order-by", "", "Sort key (e.g. created_at)") rc.cmd.Flags().StringVar(&rc.dir, "dir", "", "Sort direction (asc, desc)") rc.cmd.Flags().IntVar(&rc.limit, "limit", 100, "Limit number of results") @@ -122,7 +131,12 @@ func (rc *requestEventsCmd) runRequestEventsCmd(cmd *cobra.Command, args []strin requestID := args[0] client := Config.GetAPIClient() ctx := context.Background() - params := map[string]string{"limit": strconv.Itoa(rc.limit)} + params := map[string]string{} + limit, err := applyIncludeData(cmd, rc.include, rc.output, rc.limit, params) + if err != nil { + return err + } + params["limit"] = strconv.Itoa(limit) if rc.next != "" { params["next"] = rc.next } @@ -211,6 +225,15 @@ func (rc *requestEventsCmd) runRequestEventsCmd(cmd *cobra.Command, args []strin return fmt.Errorf("failed to list request events: %w", err) } + // The output carries no pagination cursor (the JSON is a bare array), so a + // page the API cut short would look complete. When the API says there is + // more, say so on stderr, which leaves the JSON on stdout intact, and give + // the cursor --next takes. This applies with or without --include: the + // --include default of 10 makes it likelier, but 100 events is also a page. + if next := resp.Pagination.Next; next != nil && *next != "" { + fmt.Fprintf(os.Stderr, "More events exist for this request. Pass --next %s for the next page, or a larger --limit.\n", *next) + } + if rc.output == "json" { if len(resp.Models) == 0 { fmt.Println("[]") diff --git a/pkg/cmd/request_events_filters_test.go b/pkg/cmd/request_events_filters_test.go index e98895c9..7e46189f 100644 --- a/pkg/cmd/request_events_filters_test.go +++ b/pkg/cmd/request_events_filters_test.go @@ -5,6 +5,8 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" + "strconv" "testing" "github.com/spf13/pflag" @@ -140,3 +142,129 @@ func runRequestEventsAgainst(t *testing.T, server *httptest.Server, rc *requestE return rc.runRequestEventsCmd(rc.cmd, []string{requestID}) } + +// TestRequestEventsIncludeData: GET /requests/{id}/events declares include, so +// this command offers it the way event list does, with the same smaller +// default limit and the same refusal without --output json. It calls +// applyIncludeData separately from the two list commands, so it is tested +// separately. +func TestRequestEventsIncludeData(t *testing.T) { + cases := []struct { + name string + flags map[string]string + wantLimit string + }{ + {"default limit drops to 10", map[string]string{"include": "data", "output": "json"}, "10"}, + {"explicit limit is honoured", map[string]string{"include": "data", "output": "json", "limit": "50"}, "50"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var query url.Values + var path string + server := requestEventsStub(t, &path, &query) + + rc := newRequestEventsCmd() + for name, value := range tc.flags { + require.NoError(t, rc.cmd.Flags().Set(name, value)) + } + require.NoError(t, runRequestEventsAgainst(t, server, rc, "req_1")) + + assert.Equal(t, "data", query.Get("include")) + assert.Equal(t, tc.wantLimit, query.Get("limit")) + }) + } + + t.Run("refused without --output json", func(t *testing.T) { + var query url.Values + var path string + server := requestEventsStub(t, &path, &query) + + rc := newRequestEventsCmd() + require.NoError(t, rc.cmd.Flags().Set("include", "data")) + err := runRequestEventsAgainst(t, server, rc, "req_1") + require.Error(t, err) + assert.Contains(t, err.Error(), "--output json") + assert.Empty(t, path, "no request should be made") + }) + + // The negative control: without --include, nothing changes. Without it, a + // regression that sent include on every call would pass every case above. + t.Run("without include nothing changes", func(t *testing.T) { + var query url.Values + var path string + server := requestEventsStub(t, &path, &query) + + rc := newRequestEventsCmd() + require.NoError(t, runRequestEventsAgainst(t, server, rc, "req_1")) + + assert.NotContains(t, query, "include") + assert.Equal(t, "100", query.Get("limit")) + }) +} + +// TestRequestEventsPointsAtTheNextPage: the output of this command carries no +// pagination cursor, so a page the API cut short looks complete. The note is +// driven by the API's own next cursor, not by guessing from the page size: a +// full page with no next cursor is complete and gets no note. It goes to +// stderr so the JSON on stdout stays parseable, and it applies with or without +// --include. +func TestRequestEventsPointsAtTheNextPage(t *testing.T) { + cursor := "cur_next_page" + cases := []struct { + name string + returned int + next *string + flags map[string]string + wantNote bool + }{ + {"include, more pages", 10, &cursor, map[string]string{"include": "data", "output": "json"}, true}, + {"no include, more pages", 3, &cursor, map[string]string{"output": "json"}, true}, + {"full page, no more", 10, nil, map[string]string{"include": "data", "output": "json"}, false}, + {"short page, no more", 3, nil, map[string]string{"output": "json"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + models := make([]hookdeck.Event, tc.returned) + for i := range models { + models[i] = hookdeck.Event{ID: "evt_" + strconv.Itoa(i)} + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(hookdeck.EventListResponse{ + Models: models, + Pagination: hookdeck.PaginationResponse{Limit: tc.returned, Next: tc.next}, + }) + })) + t.Cleanup(server.Close) + + rc := newRequestEventsCmd() + for name, value := range tc.flags { + require.NoError(t, rc.cmd.Flags().Set(name, value)) + } + + oldStdout, oldStderr := os.Stdout, os.Stderr + // Restore even if a require below fails, so later tests do not + // write into closed pipes. + t.Cleanup(func() { os.Stdout, os.Stderr = oldStdout, oldStderr }) + rOut, wOut, err := os.Pipe() + require.NoError(t, err) + rErr, wErr, err := os.Pipe() + require.NoError(t, err) + os.Stdout, os.Stderr = wOut, wErr + runErr := runRequestEventsAgainst(t, server, rc, "req_1") + os.Stdout, os.Stderr = oldStdout, oldStderr + require.NoError(t, wOut.Close()) + require.NoError(t, wErr.Close()) + stdout, stderr := drainPipe(rOut), drainPipe(rErr) + + require.NoError(t, runErr) + var printed []hookdeck.Event + require.NoError(t, json.Unmarshal([]byte(stdout), &printed), "stdout must stay valid JSON: %s", stdout) + if tc.wantNote { + assert.Contains(t, stderr, "--next "+cursor, "the note must give the cursor") + } else { + assert.Empty(t, stderr) + } + }) + } +} diff --git a/pkg/cmd/request_list.go b/pkg/cmd/request_list.go index 0820b6f4..f999bb97 100644 --- a/pkg/cmd/request_list.go +++ b/pkg/cmd/request_list.go @@ -29,6 +29,7 @@ type requestListCmd struct { path string parsedQuery string searchTerm string + include string eventsCount string ignoredCount string cliEventsCount string @@ -53,11 +54,15 @@ Use --search-term to find a complete value across the body, headers, parsed quer at once. --events-count 0 finds requests that produced no events, which is the usual reason a webhook appears to have gone missing. +Use --include=data with --output=json to get each request's payload in the results, instead of +fetching requests one at a time. Without --limit, the limit is then 10 instead of 100. + Examples: hookdeck gateway request list hookdeck gateway request list --source-id src_abc123 --limit 20 hookdeck gateway request list --search-term cus_1234 - hookdeck gateway request list --events-count 0`, + hookdeck gateway request list --events-count 0 + hookdeck gateway request list --source-id src_abc123 --include data --output json`, RunE: rc.runRequestListCmd, } @@ -78,6 +83,7 @@ Examples: rc.cmd.Flags().StringVar(&rc.eventsCount, "events-count", "", "Filter by number of events produced. A whole number, e.g. 0 for requests that produced none") rc.cmd.Flags().StringVar(&rc.ignoredCount, "ignored-count", "", "Filter by number of ignored events. A whole number") rc.cmd.Flags().StringVar(&rc.cliEventsCount, "cli-events-count", "", "Filter by number of CLI events. A whole number") + rc.cmd.Flags().StringVar(&rc.include, "include", "", includeFlagUsage("request")) rc.cmd.Flags().StringVar(&rc.orderBy, "order-by", "", "Sort key (e.g. created_at)") rc.cmd.Flags().StringVar(&rc.dir, "dir", "", "Sort direction (asc, desc)") rc.cmd.Flags().IntVar(&rc.limit, "limit", 100, "Limit number of results") @@ -160,7 +166,11 @@ func (rc *requestListCmd) runRequestListCmd(cmd *cobra.Command, args []string) e if rc.dir != "" { params["dir"] = rc.dir } - params["limit"] = strconv.Itoa(rc.limit) + limit, err := applyIncludeData(cmd, rc.include, rc.output, rc.limit, params) + if err != nil { + return err + } + params["limit"] = strconv.Itoa(limit) if rc.next != "" { params["next"] = rc.next } diff --git a/pkg/gateway/mcp/server_test.go b/pkg/gateway/mcp/server_test.go index b9ab5739..75d9650c 100644 --- a/pkg/gateway/mcp/server_test.go +++ b/pkg/gateway/mcp/server_test.go @@ -344,15 +344,20 @@ func TestReadOnlyModeHidesWriteOnlyPropsAndProse(t *testing.T) { // surface them either. Reading the spec without that context makes them look // like filters we simply forgot, which is exactly how they would get added — // this pins the omission as intentional. +// +// include is the exception. It carries x-docs-hide too, but Hookdeck's API +// team has confirmed it can be exposed, and without it an agent that needs +// payloads fetches every event by ID, one call each. It is pinned the other +// way round by TestListToolsOfferIncludeData. func TestPluralToolsOmitParametersHiddenFromTheAPIDocs(t *testing.T) { session := connectInMemory(t, newTestClient("https://api.hookdeck.com", "test-key")) tools := listTools(t, session) hidden := map[string][]string{ "gateway_events_read": { - "bulk_retry_id", "include", "progressive", "event_data_id", "cli_user_id", + "bulk_retry_id", "progressive", "event_data_id", "cli_user_id", }, - "gateway_requests_read": {"bulk_retry_id", "include", "progressive"}, + "gateway_requests_read": {"bulk_retry_id", "progressive"}, } for tool, names := range hidden { diff --git a/pkg/gateway/mcp/tool_events.go b/pkg/gateway/mcp/tool_events.go index 1dd8a73a..eb2ff8e0 100644 --- a/pkg/gateway/mcp/tool_events.go +++ b/pkg/gateway/mcp/tool_events.go @@ -27,6 +27,8 @@ var eventsSpec = mcpcore.ToolSpec{ "List supports the same filters as `hookdeck gateway event list` (metadata, date range, payload search, sort). " + "To act on a specific event you already have an ID for, use " + eventToolName + " (singular). This tool only searches. " + "To see the events one request produced, pass request_id — this tool queries the request's own events route, which takes these same filters. " + + "Pass include: \"data\" with action list to get each event's payload (headers, body, path, parsed_query) in the list itself, rather than calling " + eventToolName + " once per event. " + + "Payloads make the response much larger, so keep limit small when you include them; without a limit, it defaults to 10. " + "Results are scoped to the active project — call the projects tool first if the user has specified a project.", Actions: eventsActions, Props: map[string]mcpcore.Prop{ @@ -55,6 +57,7 @@ var eventsSpec = mcpcore.ToolSpec{ "parsed_query": {Type: "string", JSONValue: true, Desc: "Filter by parsed query as JSON. " + descJSONFilter}, "path": {Type: "string", Desc: descPathFilter}, "search_term": {Type: "string", Desc: descSearchTerm}, + "include": {Type: "string", Enum: []string{"data"}, Desc: descIncludeData("event", eventToolName), Actions: []string{"list"}}, "limit": {Type: "integer", Desc: "Max results", Actions: []string{"list", "list_ignored"}}, "order_by": {Type: "string", Desc: "Sort field"}, "dir": {Type: "string", Desc: "Sort direction: asc or desc"}, @@ -90,6 +93,20 @@ Payload search: Example: {"action":"list","body":{"type":"charge.succeeded"}} Example: {"action":"list","search_term":"cus_1234"} +Including payloads: + By default list returns each event's metadata only. include: "data" adds a data object to every + event: the event's own headers, body, path and parsed_query. That is the payload after any + transformation the connection applied, so it can differ from the request's; for the inbound + payload, list requests with include instead. Use it when you need the payloads of several + events, instead of fetching each one with ` + eventToolName + ` get or raw_body: one list call + replaces one call per event, which makes the rate limit far less likely to bite. + The response grows with every payload, and webhook bodies can be large, so keep limit small and + page with next rather than asking for one big page. If you set include without a limit, limit + defaults to 10 instead of the API's default page size, so a page of 10 does not mean there are + no more: check pagination.next. For a bigger page, pass limit explicitly. + Example: {"action":"list","status":"FAILED","include":"data","limit":5} + Not available on list_ignored: GET /requests/{id}/ignored_events does not declare include. + Delivery group: delivery_group filters by the group an event was delivered in; comma-separate several. The API's own schema is nullable and documents null as "matches events without a delivery @@ -259,7 +276,8 @@ func eventsListing(ctx context.Context, client *hookdeck.Client, in mcpcore.Inpu mcpcore.SetIfNonEmpty(params, "last_attempt_at[lte]", in.String("last_attempt_before")) mcpcore.SetIfNonEmpty(params, "next_attempt_at[gte]", in.String("next_attempt_after")) mcpcore.SetIfNonEmpty(params, "next_attempt_at[lte]", in.String("next_attempt_before")) - mcpcore.SetInt(params, "limit", in.Int("limit", 0)) + mcpcore.SetIfNonEmpty(params, "include", in.String("include")) + mcpcore.SetInt(params, "limit", listLimit(in)) mcpcore.SetIfNonEmpty(params, "order_by", in.String("order_by")) mcpcore.SetIfNonEmpty(params, "dir", in.String("dir")) mcpcore.SetIfNonEmpty(params, "next", in.String("next")) diff --git a/pkg/gateway/mcp/tool_list_include_data_test.go b/pkg/gateway/mcp/tool_list_include_data_test.go new file mode 100644 index 00000000..a74875ec --- /dev/null +++ b/pkg/gateway/mcp/tool_list_include_data_test.go @@ -0,0 +1,205 @@ +package mcp + +import ( + "encoding/json" + "net/http" + "strconv" + "testing" + + mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +) + +// include=data puts each item's payload in the list response. Without it an +// agent that needs payloads calls the singular tool once per event, and a list +// of a hundred events becomes a hundred more calls. + +// schemaProperty returns one property of a tool's input schema as decoded JSON. +func schemaProperty(t *testing.T, tool *mcpsdk.Tool, name string) map[string]any { + t.Helper() + require.NotNil(t, tool) + raw, err := json.Marshal(tool.InputSchema) + require.NoError(t, err) + var schema struct { + Properties map[string]map[string]any `json:"properties"` + } + require.NoError(t, json.Unmarshal(raw, &schema)) + prop, ok := schema.Properties[name] + require.True(t, ok, "%s does not advertise %q", tool.Name, name) + return prop +} + +// TestListToolsOfferIncludeData pins the schema: include exists, takes only +// "data" (the one value the API declares), and both the property and the tool +// description warn about response size where the agent decides to use it. +func TestListToolsOfferIncludeData(t *testing.T) { + session := connectInMemory(t, newTestClient("https://api.hookdeck.com", "test-key")) + tools := listTools(t, session) + + for _, name := range []string{eventsToolName, requestsToolName} { + t.Run(name, func(t *testing.T) { + tool := tools[name] + prop := schemaProperty(t, tool, "include") + assert.Equal(t, []any{"data"}, prop["enum"]) + + desc, _ := prop["description"].(string) + assert.Contains(t, desc, "limit", "the property must tell the agent to keep limit small") + + assert.Contains(t, tool.Description, `include: "data"`) + assert.Contains(t, tool.Description, "much larger", + "the tool description must warn that payloads make responses larger") + assert.Contains(t, tool.Description, "keep limit small") + }) + } +} + +// TestRequestsListForwardsIncludeData: the requests collection route declares +// include, so it has to reach the wire. The events routes are covered by +// eventFilterForwarding. +func TestRequestsListForwardsIncludeData(t *testing.T) { + var saw string + session := mockAPIWithClient(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + "/requests": func(w http.ResponseWriter, r *http.Request) { + saw = r.URL.Query().Get("include") + _ = json.NewEncoder(w).Encode(listResponse()) + }, + }) + result := callTool(t, session, requestsToolName, map[string]any{"action": "list", "include": "data"}) + require.False(t, result.IsError, textContent(t, result)) + assert.Equal(t, "data", saw) +} + +// TestListOmitsIncludeWhenUnset: payloads are opt-in. Sending include by +// default would make every list call as heavy as the warning says. +func TestListOmitsIncludeWhenUnset(t *testing.T) { + for _, tt := range []struct { + tool, path string + }{ + {eventsToolName, "/events"}, + {requestsToolName, "/requests"}, + } { + t.Run(tt.tool, func(t *testing.T) { + present := false + called := false + session := mockAPIWithClient(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + tt.path: func(w http.ResponseWriter, r *http.Request) { + called = true + _, present = r.URL.Query()["include"] + _ = json.NewEncoder(w).Encode(listResponse()) + }, + }) + result := callTool(t, session, tt.tool, map[string]any{"action": "list"}) + require.False(t, result.IsError, textContent(t, result)) + require.True(t, called) + assert.False(t, present, "include must not be sent unless the caller asks for it") + }) + } +} + +// TestListReturnsIncludedData: forwarding the parameter is only half of it. +// The payload has to survive decoding into the response models and come back +// out in the tool result, or the agent asked for data and silently got none. +func TestListReturnsIncludedData(t *testing.T) { + data := map[string]any{ + "headers": map[string]any{"x-marker": "hdr-include-test"}, + "body": map[string]any{"marker": "body-include-test"}, + "path": "/include-test", + "parsed_query": map[string]any{"q": "query-include-test"}, + } + for _, tt := range []struct { + tool, path, id string + }{ + {eventsToolName, "/events", "evt_1"}, + {requestsToolName, "/requests", "req_1"}, + } { + t.Run(tt.tool, func(t *testing.T) { + session := mockAPIWithClient(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + tt.path: func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(listResponse(map[string]any{"id": tt.id, "data": data})) + }, + }) + result := callTool(t, session, tt.tool, map[string]any{"action": "list", "include": "data"}) + require.False(t, result.IsError, textContent(t, result)) + text := textContent(t, result) + for _, marker := range []string{"hdr-include-test", "body-include-test", "/include-test", "query-include-test"} { + assert.Contains(t, text, marker) + } + }) + } +} + +// TestIncludeDataDefaultsTheLimit pins the backstop behind the size warning. +// Asking for payloads without a limit gets a small page; an explicit limit is +// honoured; and a list without include is untouched, so the API's own default +// still applies there. Run against both call sites, eventsListing and +// requestsList, because each sets the limit separately. +func TestIncludeDataDefaultsTheLimit(t *testing.T) { + type sent struct { + limit string + present bool + } + cases := []struct { + name string + args map[string]any + want sent + }{ + {"include without limit sends the default", map[string]any{"action": "list", "include": "data"}, sent{"10", true}}, + {"include with an explicit limit honours it", map[string]any{"action": "list", "include": "data", "limit": float64(25)}, sent{"25", true}}, + {"no include leaves the limit unset", map[string]any{"action": "list"}, sent{"", false}}, + } + for _, route := range []struct{ tool, path string }{ + {eventsToolName, "/events"}, + {requestsToolName, "/requests"}, + } { + for _, tc := range cases { + t.Run(route.tool+"/"+tc.name, func(t *testing.T) { + var got sent + called := false + session := mockAPIWithClient(t, map[string]http.HandlerFunc{ + hookdeck.APIPathPrefix + route.path: func(w http.ResponseWriter, r *http.Request) { + called = true + _, got.present = r.URL.Query()["limit"] + got.limit = r.URL.Query().Get("limit") + _ = json.NewEncoder(w).Encode(listResponse()) + }, + }) + result := callTool(t, session, route.tool, tc.args) + require.False(t, result.IsError, textContent(t, result)) + require.True(t, called) + assert.Equal(t, tc.want, got) + }) + } + } +} + +// TestAgentIsToldAboutTheIncludeDataDefaultLimit: a default the agent does not +// know about looks like a short result. A list that stops at 10 reads as +// "there are only 10", so the default has to be stated everywhere an agent +// reads about include (the tool description it sees in tools/list, the +// property it fills in, and the help topic it asks for), along with how to get +// more. +func TestAgentIsToldAboutTheIncludeDataDefaultLimit(t *testing.T) { + session := connectInMemory(t, newTestClient("https://api.hookdeck.com", "test-key")) + tools := listTools(t, session) + want := "defaults to " + strconv.Itoa(includeDataDefaultLimit) + + for _, name := range []string{eventsToolName, requestsToolName} { + t.Run(name, func(t *testing.T) { + assert.Contains(t, tools[name].Description, want, "tool description") + + desc, _ := schemaProperty(t, tools[name], "include")["description"].(string) + assert.Contains(t, desc, want, "include property") + assert.Contains(t, desc, "pass limit", "include property must say how to get more") + + result := callTool(t, session, helpToolName, map[string]any{"topic": name}) + require.False(t, result.IsError, textContent(t, result)) + help := textContent(t, result) + assert.Contains(t, help, want, "help topic") + assert.Contains(t, help, "pass limit", "help topic must say how to get more") + t.Log(help) + }) + } +} diff --git a/pkg/gateway/mcp/tool_requests.go b/pkg/gateway/mcp/tool_requests.go index 5525bb66..ca7c5f98 100644 --- a/pkg/gateway/mcp/tool_requests.go +++ b/pkg/gateway/mcp/tool_requests.go @@ -23,6 +23,8 @@ var requestsSpec = mcpcore.ToolSpec{ Summary: "SEARCH MANY requests — plural, collection only. Find inbound requests (raw HTTP data received by Hookdeck before routing) matching filters and get back their IDs. " + "List supports the same filters as `hookdeck gateway request list` (metadata, date range, payload search, sort). " + "To act on a specific request you already have an ID for, use " + requestToolName + " (singular). This tool only searches. " + + "Pass include: \"data\" with action list to get each request's payload (headers, body, path, parsed_query) in the list itself, rather than calling " + requestToolName + " once per request. " + + "Payloads make the response much larger, so keep limit small when you include them; without a limit, it defaults to 10. " + "There is no event_id filter here: to go from an event to its request, read request_id off the event and call " + requestToolName + " with action get. " + "Results are scoped to the active project — call the projects tool first if the user has specified a project.", Actions: requestsActions, @@ -48,6 +50,7 @@ var requestsSpec = mcpcore.ToolSpec{ "cli_events_count": {Type: "string", Desc: "Filter by count of events delivered to a CLI listen session. " + descCountFilter}, "order_by": {Type: "string", Desc: "Sort field, e.g. created_at"}, "dir": {Type: "string", Desc: "Sort direction: asc or desc"}, + "include": {Type: "string", Enum: []string{"data"}, Desc: descIncludeData("request", requestToolName), Actions: []string{"list"}}, "limit": {Type: "integer", Desc: "Max results", Actions: []string{"list"}}, "next": {Type: "string", Desc: "Next page cursor", Actions: []string{"list"}}, "prev": {Type: "string", Desc: "Previous page cursor", Actions: []string{"list"}}, @@ -81,6 +84,16 @@ Count filters: {"action":"list","events_count":"0"} finds requests that matched no connection, and {"action":"list","ignored_count":"1"} finds ones a connection filter dropped an event from. +Including payloads: + By default list returns each request's metadata only. include: "data" adds a data object to every + request: its headers, body, path and parsed_query. Use it when you need the payloads of several + requests, instead of fetching each one with ` + requestToolName + ` get or raw_body. + The response grows with every payload, and webhook bodies can be large, so keep limit small and + page with next rather than asking for one big page. If you set include without a limit, limit + defaults to 10 instead of the API's default page size, so a page of 10 does not mean there are + no more: check pagination.next. For a bigger page, pass limit explicitly. + Example: {"action":"list","status":"rejected","include":"data","limit":5} + Requests and events: The API offers one traversal direction only. Requests cannot be filtered by event_id — there is no such filter, so do not look for one. From an event, read its request_id and call @@ -155,7 +168,8 @@ func requestsList(ctx context.Context, client *hookdeck.Client, in mcpcore.Input mcpcore.SetIfNonEmpty(params, "created_at[lte]", in.String("created_before")) mcpcore.SetIfNonEmpty(params, "ingested_at[gte]", in.String("ingested_after")) mcpcore.SetIfNonEmpty(params, "ingested_at[lte]", in.String("ingested_before")) - mcpcore.SetInt(params, "limit", in.Int("limit", 0)) + mcpcore.SetIfNonEmpty(params, "include", in.String("include")) + mcpcore.SetInt(params, "limit", listLimit(in)) mcpcore.SetIfNonEmpty(params, "order_by", in.String("order_by")) mcpcore.SetIfNonEmpty(params, "dir", in.String("dir")) mcpcore.SetIfNonEmpty(params, "next", in.String("next")) diff --git a/pkg/gateway/mcp/tool_requests_events_filters_test.go b/pkg/gateway/mcp/tool_requests_events_filters_test.go index e09da895..b6604728 100644 --- a/pkg/gateway/mcp/tool_requests_events_filters_test.go +++ b/pkg/gateway/mcp/tool_requests_events_filters_test.go @@ -46,6 +46,7 @@ var eventFilterForwarding = []struct { {"headers", `{"x-trace":"1"}`, "headers", `{"x-trace":"1"}`}, {"parsed_query", `{"q":"1"}`, "parsed_query", `{"q":"1"}`}, {"path", "/hook", "path", "/hook"}, + {"include", "data", "include", "data"}, {"order_by", "created_at", "order_by", "created_at"}, {"dir", "asc", "dir", "asc"}, {"limit", float64(5), "limit", "5"}, @@ -116,7 +117,7 @@ func TestEventsIgnoredForwardsOnlyWhatTheRouteDeclares(t *testing.T) { // is the house rule — a wrong answer that reads like a right one is the failure // this package keeps finding. func TestEventsIgnoredRefusesFiltersTheRouteDrops(t *testing.T) { - for _, arg := range []string{"status", "source_id", "destination_id", "attempts", "search_term", "created_after"} { + for _, arg := range []string{"status", "source_id", "destination_id", "attempts", "search_term", "created_after", "include"} { t.Run(arg, func(t *testing.T) { session := mockAPIWithClient(t, map[string]http.HandlerFunc{ hookdeck.APIPathPrefix + "/requests/req_1/ignored_events": func(w http.ResponseWriter, r *http.Request) { @@ -124,8 +125,14 @@ func TestEventsIgnoredRefusesFiltersTheRouteDrops(t *testing.T) { }, }) value := any("x") - if arg == "created_after" { + switch arg { + case "created_after": value = "2025-01-01T00:00:00Z" + case "include": + // The one valid value: anything else is refused by the enum + // check first, which would pass this test without exercising + // the action scoping it is here for. + value = "data" } result := callTool(t, session, eventsToolName, map[string]any{ "action": "list_ignored", "request_id": "req_1", arg: value, diff --git a/pkg/gateway/mcp/tools.go b/pkg/gateway/mcp/tools.go index c1ccd72d..a8bffd7d 100644 --- a/pkg/gateway/mcp/tools.go +++ b/pkg/gateway/mcp/tools.go @@ -1,6 +1,8 @@ package mcp import ( + "strconv" + mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/hookdeck/hookdeck-cli/pkg/config" @@ -166,6 +168,45 @@ const ( descCountFilter = "A whole number, e.g. 0 to find records that produced none. Comparisons such as \">0\" are not supported here." ) +// descIncludeData describes include on the events and requests list tools. +// +// The API declares include on GET /events, GET /requests and +// GET /requests/{id}/events. It carries x-docs-hide, which is why the MCP +// omitted it until now. Hookdeck's API team has since confirmed it can be +// documented and exposed, with the caveat that it can return a lot of data. +// Without it, an agent that needs payloads has to fetch each event by ID, one +// call per event, and a fast agent can hit the rate limit doing so. The size +// warning is that caveat, stated where the agent chooses the argument. +func descIncludeData(resource, singularTool string) string { + return "Set to \"data\" to return each " + resource + "'s payload (headers, body, path, parsed_query) as a data object on every result, " + + "instead of fetching each one with " + singularTool + ". " + + "Responses grow with every payload and webhook bodies can be large: keep limit small and page with next. " + + "When include is set and limit is not, limit defaults to " + strconv.Itoa(includeDataDefaultLimit) + " rather than the API's default page size, " + + "so a page of " + strconv.Itoa(includeDataDefaultLimit) + " does not mean there are no more: check pagination.next. " + + "For a bigger page, pass limit explicitly. " + + "Omit include when IDs and metadata are enough." +} + +// includeDataDefaultLimit is the page size sent when a list asks for payloads +// without saying how many. +// +// The warning in descIncludeData is advice; this is the backstop. Left to the +// API's default page size, {"action":"list","include":"data"} returns a full +// page of payloads, which can fill an agent's context in one call. An explicit +// limit is honoured as given: an agent that asks for more has chosen to. +const includeDataDefaultLimit = 10 + +// listLimit returns the limit to send for a list action, applying +// includeDataDefaultLimit when payloads are requested and no limit was given. +// A limit of 0 or less is treated as not given, as SetInt already does. +func listLimit(in mcpcore.Input) int { + limit := in.Int("limit", 0) + if limit <= 0 && in.String("include") != "" { + return includeDataDefaultLimit + } + return limit +} + // measures, dimensions and status decide whether a metrics call succeeds, and // each of the four actions has its own vocabulary. One flat schema cannot carry // four enums, so the per-action lists go in the description - built from the diff --git a/pkg/hookdeck/events.go b/pkg/hookdeck/events.go index 96769b86..a5e245ad 100644 --- a/pkg/hookdeck/events.go +++ b/pkg/hookdeck/events.go @@ -32,12 +32,26 @@ type Event struct { TeamID string `json:"team_id"` } -// EventData holds optional request snapshot on the event +// EventData is the payload on an event, returned by GET /events with +// include=data and on a single event. +// +// headers and parsed_query are typed `any` because the spec allows each to be +// an object or a string. A map field would fail to decode the string form, and +// on a list that one record would fail the whole page. body_format is left out +// because it carries x-docs-hide. type EventData struct { - Headers map[string]interface{} `json:"headers,omitempty"` - Body interface{} `json:"body,omitempty"` - Path string `json:"path,omitempty"` - ParsedQuery map[string]interface{} `json:"parsed_query,omitempty"` + URL string `json:"url,omitempty"` + Method string `json:"method,omitempty"` + Headers interface{} `json:"headers,omitempty"` + AppendedHeaders []string `json:"appended_headers,omitempty"` + Body interface{} `json:"body,omitempty"` + Path string `json:"path,omitempty"` + Query *string `json:"query,omitempty"` + ParsedQuery interface{} `json:"parsed_query,omitempty"` + // IsLargePayload is the API's flag for a payload it considers large and + // does not index for search. Kept so callers see it rather than having it + // dropped in decoding. + IsLargePayload *bool `json:"is_large_payload,omitempty"` } // EventListResponse is the response from listing events diff --git a/pkg/hookdeck/list_data_decode_test.go b/pkg/hookdeck/list_data_decode_test.go new file mode 100644 index 00000000..3217c347 --- /dev/null +++ b/pkg/hookdeck/list_data_decode_test.go @@ -0,0 +1,58 @@ +package hookdeck + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// With include=data a list carries one payload per record, decoded into +// EventData / RequestData. The spec allows headers and parsed_query to be a +// string as well as an object, so a record in the string form must decode +// rather than fail the whole page. is_large_payload, the API's flag for a +// payload it does not index for search, must survive decoding too. +const listWithMixedDataShapes = `{"models":[ + {"id":"rec_object","data":{"headers":{"x-a":"1"},"parsed_query":{"q":"1"},"body":{"k":"v"},"path":"/a"}}, + {"id":"rec_string","data":{"headers":"x-a: 1","parsed_query":"q=1","body":"raw","path":"/b","query":"q=1","is_large_payload":true}} +],"pagination":{"limit":10}}` + +func listDataServer(t *testing.T) *Client { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(listWithMixedDataShapes)) + })) + t.Cleanup(srv.Close) + base, err := url.Parse(srv.URL) + require.NoError(t, err) + return &Client{BaseURL: base} +} + +func TestListEventsDecodesEveryDataShape(t *testing.T) { + resp, err := listDataServer(t).ListEvents(context.Background(), map[string]string{"include": "data"}) + require.NoError(t, err, "a string-form headers or parsed_query must not fail the page") + require.Len(t, resp.Models, 2) + + assert.Equal(t, map[string]any{"x-a": "1"}, resp.Models[0].Data.Headers) + assert.Equal(t, "x-a: 1", resp.Models[1].Data.Headers) + assert.Equal(t, "q=1", resp.Models[1].Data.ParsedQuery) + require.NotNil(t, resp.Models[1].Data.IsLargePayload) + assert.True(t, *resp.Models[1].Data.IsLargePayload) +} + +func TestListRequestsDecodesEveryDataShape(t *testing.T) { + resp, err := listDataServer(t).ListRequests(context.Background(), map[string]string{"include": "data"}) + require.NoError(t, err, "a string-form headers or parsed_query must not fail the page") + require.Len(t, resp.Models, 2) + + assert.Equal(t, map[string]any{"x-a": "1"}, resp.Models[0].Data.Headers) + assert.Equal(t, "x-a: 1", resp.Models[1].Data.Headers) + assert.Equal(t, "q=1", resp.Models[1].Data.ParsedQuery) + require.NotNil(t, resp.Models[1].Data.IsLargePayload) + assert.True(t, *resp.Models[1].Data.IsLargePayload) +} diff --git a/pkg/hookdeck/requests.go b/pkg/hookdeck/requests.go index a91b9343..a0cb8655 100644 --- a/pkg/hookdeck/requests.go +++ b/pkg/hookdeck/requests.go @@ -26,12 +26,22 @@ type Request struct { TeamID string `json:"team_id"` } -// RequestData holds optional request snapshot +// RequestData is the payload on a request (the spec's ShortEventData), +// returned by GET /requests with include=data and on a single request. +// +// headers and parsed_query are typed `any` for the reason given on EventData: +// the spec allows a string or an object, and one record in the string form +// would otherwise fail a whole page. body_format is left out because it +// carries x-docs-hide. type RequestData struct { - Headers map[string]interface{} `json:"headers,omitempty"` - Body interface{} `json:"body,omitempty"` - Path string `json:"path,omitempty"` - ParsedQuery map[string]interface{} `json:"parsed_query,omitempty"` + Headers interface{} `json:"headers,omitempty"` + Body interface{} `json:"body,omitempty"` + Path string `json:"path,omitempty"` + Query *string `json:"query,omitempty"` + ParsedQuery interface{} `json:"parsed_query,omitempty"` + // IsLargePayload is the API's flag for a payload it considers large and + // does not index for search. + IsLargePayload *bool `json:"is_large_payload,omitempty"` } // RequestListResponse is the response from listing requests diff --git a/test/acceptance/event_include_data_test.go b/test/acceptance/event_include_data_test.go new file mode 100644 index 00000000..1d4e252d --- /dev/null +++ b/test/acceptance/event_include_data_test.go @@ -0,0 +1,71 @@ +//go:build event + +package acceptance + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestEventListIncludeData checks --include data on `gateway event list` +// against the live API. +// +// The negative control matters as much as the positive one: the API ignores +// query parameters it does not declare, so a dropped include would return a +// list with no payloads and exit zero. Asserting both directions is what shows +// the flag is doing the work. +func TestEventListIncludeData(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + cli := NewCLIRunner(t) + connID, _ := createConnectionAndTriggerEvent(t, cli) + t.Cleanup(func() { deleteConnection(t, cli, connID) }) + + type listed struct { + Models []struct { + ID string `json:"id"` + Data map[string]any `json:"data"` + } `json:"models"` + Pagination struct { + Limit int `json:"limit"` + } `json:"pagination"` + } + + t.Run("without include there is no payload", func(t *testing.T) { + var resp listed + require.NoError(t, cli.RunJSON(&resp, "gateway", "event", "list", "--connection-id", connID, "--limit", "1")) + require.NotEmpty(t, resp.Models) + assert.Nil(t, resp.Models[0].Data) + }) + + t.Run("with include data the payload comes back", func(t *testing.T) { + var resp listed + require.NoError(t, cli.RunJSON(&resp, "gateway", "event", "list", "--connection-id", connID, "--limit", "1", "--include", "data")) + require.NotEmpty(t, resp.Models) + require.NotNil(t, resp.Models[0].Data, "include data must put a data object on the event") + // triggerTestEvent posts {"test":true}. + assert.Equal(t, map[string]any{"test": true}, resp.Models[0].Data["body"]) + }) + + t.Run("with include data and no limit the page size is 10", func(t *testing.T) { + var resp listed + require.NoError(t, cli.RunJSON(&resp, "gateway", "event", "list", "--connection-id", connID, "--include", "data")) + assert.Equal(t, 10, resp.Pagination.Limit) + }) + + t.Run("include without output json is refused", func(t *testing.T) { + stdout, stderr, err := cli.Run("gateway", "event", "list", "--connection-id", connID, "--include", "data") + require.Error(t, err) + assert.Contains(t, stdout+stderr, "--output json") + }) + + // The API owns the value set. This pins that a wrong value fails loudly + // rather than returning a list without payloads. + t.Run("an unknown include value is rejected", func(t *testing.T) { + _, _, err := cli.Run("gateway", "event", "list", "--connection-id", connID, "--include", "bogus", "--output", "json") + require.Error(t, err) + }) +} diff --git a/test/acceptance/mcp_include_data_test.go b/test/acceptance/mcp_include_data_test.go new file mode 100644 index 00000000..f90fe295 --- /dev/null +++ b/test/acceptance/mcp_include_data_test.go @@ -0,0 +1,106 @@ +//go:build mcp + +package acceptance + +import ( + "encoding/json" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// firstModelData lists through an MCP tool and returns the first model's data +// object, or nil when the model has none. +func firstModelData(t *testing.T, cli *CLIRunner, tool string, args map[string]any) map[string]any { + t.Helper() + result := CallGatewayMCPTool(t, cli.projectRoot, cli.configPath, tool, args, 30*time.Second) + require.False(t, result.IsError, "tool error: %s", result.Text) + + var envelope struct { + Data struct { + Models []map[string]any `json:"models"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(result.Text), &envelope), result.Text) + require.NotEmpty(t, envelope.Data.Models, "expected at least one model from %s: %s", tool, result.Text) + + data, _ := envelope.Data.Models[0]["data"].(map[string]any) + return data +} + +// TestMCPListIncludeData is the check a mock cannot make: that the live API +// honours include=data on the routes the MCP list actions query, and that the +// payload it returns comes back out of the tool. +// +// The negative control matters as much as the positive one. The API ignores +// query parameters it does not declare, so a dropped include would return 200 +// and a list with no payloads, and a test that only asked for data would never +// notice whether it was the parameter or the default that put it there. +func TestMCPListIncludeData(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + cli := NewCLIRunner(t) + + connID, _ := createConnectionAndTriggerEvent(t, cli) + t.Cleanup(func() { deleteConnection(t, cli, connID) }) + + var conn Connection + require.NoError(t, cli.RunJSON(&conn, "gateway", "connection", "get", connID)) + sourceID := conn.Source.ID + require.NotEmpty(t, sourceID) + + // triggerTestEvent posts {"test":true}. + assertPayload := func(t *testing.T, data map[string]any) { + t.Helper() + require.NotNil(t, data, "include=data must put a data object on the model") + assert.Equal(t, map[string]any{"test": true}, data["body"], + "data.body should be the payload that was sent") + } + + cases := []struct { + name string + tool string + args map[string]any + }{ + {"events", "gateway_events_read", map[string]any{"action": "list", "connection_id": connID, "limit": 1}}, + {"requests", "gateway_requests_read", map[string]any{"action": "list", "source_id": sourceID, "limit": 1}}, + } + + for _, tc := range cases { + t.Run(tc.name+" without include returns no payload", func(t *testing.T) { + assert.Nil(t, firstModelData(t, cli, tc.tool, tc.args), + "payloads must be opt-in: without include the model should carry no data") + }) + + t.Run(tc.name+" with include=data returns the payload", func(t *testing.T) { + args := map[string]any{"include": "data"} + for k, v := range tc.args { + args[k] = v + } + assertPayload(t, firstModelData(t, cli, tc.tool, args)) + }) + } + + // The request-scoped events route declares include too. + t.Run("events scoped to a request with include=data returns the payload", func(t *testing.T) { + result := CallGatewayMCPTool(t, cli.projectRoot, cli.configPath, "gateway_requests_read", + map[string]any{"action": "list", "source_id": sourceID, "limit": 1}, 30*time.Second) + require.False(t, result.IsError, result.Text) + var envelope struct { + Data struct { + Models []struct { + ID string `json:"id"` + } `json:"models"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(result.Text), &envelope)) + require.NotEmpty(t, envelope.Data.Models) + + assertPayload(t, firstModelData(t, cli, "gateway_events_read", map[string]any{ + "action": "list", "request_id": envelope.Data.Models[0].ID, "include": "data", "limit": 1, + })) + }) +} diff --git a/test/acceptance/request_events_paging_test.go b/test/acceptance/request_events_paging_test.go new file mode 100644 index 00000000..3da91d88 --- /dev/null +++ b/test/acceptance/request_events_paging_test.go @@ -0,0 +1,120 @@ +//go:build request + +package acceptance + +import ( + "encoding/json" + "fmt" + "regexp" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRequestEventsPointsAtTheNextPage checks the stderr paging note on +// `gateway request events` against the live API. +// +// That command's JSON is a bare array with no cursor, so a page the API cut +// short looks complete. The note is driven by the API's pagination.next, and +// what a mock cannot tell us is whether the API sets next exactly when more +// events exist: not on a page that is merely full. Two connections on one +// source make one request fan out to two events, which gives both cases. +func TestRequestEventsPointsAtTheNextPage(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + cli := NewCLIRunner(t) + + firstConnID := createTestConnectionWithMockDestination(t, cli) + t.Cleanup(func() { deleteConnection(t, cli, firstConnID) }) + + var first Connection + require.NoError(t, cli.RunJSON(&first, "gateway", "connection", "get", firstConnID)) + sourceID := first.Source.ID + require.NotEmpty(t, sourceID) + + var second Connection + stamp := generateTimestamp() + require.NoError(t, cli.RunJSON(&second, + "gateway", "connection", "create", + "--name", "test-conn-paging-"+stamp, + "--source-id", sourceID, + "--destination-name", "test-dst-paging-"+stamp, + "--destination-type", "MOCK_API", + )) + require.NotEmpty(t, second.ID) + t.Cleanup(func() { deleteConnection(t, cli, second.ID) }) + + var src Source + require.NoError(t, cli.RunJSON(&src, "gateway", "source", "get", sourceID)) + triggerTestEvent(t, src.URL) + + // Wait for the request to show both of its events. + requestID := "" + for i := 0; i < propagationAttempts && requestID == ""; i++ { + time.Sleep(propagationInterval) + var resp struct { + Models []Request `json:"models"` + } + require.NoError(t, cli.RunJSON(&resp, "gateway", "request", "list", "--source-id", sourceID, "--limit", "5")) + for _, r := range resp.Models { + if r.EventsCount >= 2 { + requestID = r.ID + break + } + } + } + require.NotEmpty(t, requestID, "no request reached two events within %s", propagationAttempts*propagationInterval) + + // eventIDs runs request events with --output json and returns the event + // IDs from stdout and whatever was written to stderr. + eventIDs := func(t *testing.T, args ...string) ([]string, string) { + t.Helper() + full := append([]string{"gateway", "request", "events", requestID, "--output", "json"}, args...) + stdout, stderr, err := cli.Run(full...) + require.NoError(t, err, "stdout=%s stderr=%s", stdout, stderr) + var events []struct { + ID string `json:"id"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &events), "stdout must stay valid JSON: %s", stdout) + ids := make([]string, len(events)) + for i, e := range events { + ids[i] = e.ID + } + return ids, stderr + } + + // events_count reaches two before the second event is listable on the + // request's events route, so wait on the route itself. + listed := 0 + for i := 0; i < propagationAttempts && listed < 2; i++ { + ids, _ := eventIDs(t, "--limit", "5") + listed = len(ids) + if listed < 2 { + time.Sleep(propagationInterval) + } + } + require.Equal(t, 2, listed, "request events never listed both events within %s", propagationAttempts*propagationInterval) + + cursorRE := regexp.MustCompile(`--next (\S+)`) + + t.Run("a cut-short page gives the cursor and the cursor reaches the rest", func(t *testing.T) { + firstPage, stderr := eventIDs(t, "--limit", "1") + require.Len(t, firstPage, 1) + m := cursorRE.FindStringSubmatch(stderr) + require.NotNil(t, m, "expected a --next cursor on stderr, got: %q", stderr) + + secondPage, stderr := eventIDs(t, "--limit", "1", "--next", m[1]) + require.Len(t, secondPage, 1) + assert.NotEqual(t, firstPage[0], secondPage[0], "the cursor must lead to a different event") + assert.NotContains(t, stderr, "--next", "the last page must not point at another one") + }) + + t.Run("a page that is exactly full gets no note", func(t *testing.T) { + ids, stderr := eventIDs(t, "--limit", "2") + require.Len(t, ids, 2) + assert.NotContains(t, stderr, "--next", fmt.Sprintf("no more events exist, so no cursor should be offered: %q", stderr)) + }) +} diff --git a/test/acceptance/request_include_data_test.go b/test/acceptance/request_include_data_test.go new file mode 100644 index 00000000..fd65323e --- /dev/null +++ b/test/acceptance/request_include_data_test.go @@ -0,0 +1,90 @@ +//go:build request + +package acceptance + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRequestIncludeData checks --include data on `gateway request list` and +// `gateway request events` against the live API. Each command calls the +// shared helper separately, so each is checked, with a negative control: the +// API ignores query parameters it does not declare, so only the difference +// between with and without shows the flag is doing the work. +func TestRequestIncludeData(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + cli := NewCLIRunner(t) + connID, _ := createConnectionAndTriggerEvent(t, cli) + t.Cleanup(func() { deleteConnection(t, cli, connID) }) + + var conn Connection + require.NoError(t, cli.RunJSON(&conn, "gateway", "connection", "get", connID)) + requestID := pollForRequestsBySourceID(t, cli, conn.Source.ID)[0].ID + + type model struct { + ID string `json:"id"` + Data map[string]any `json:"data"` + } + // request list prints {models, pagination}; request events prints a bare + // array, so it has no page size to read back. The default limit on request + // events is pinned by TestRequestEventsIncludeData instead. + type listed struct { + Models []model `json:"models"` + Pagination struct { + Limit int `json:"limit"` + } `json:"pagination"` + } + listModels := func(t *testing.T, args []string, bareArray bool) []model { + t.Helper() + if bareArray { + var models []model + require.NoError(t, cli.RunJSON(&models, args...)) + return models + } + var resp listed + require.NoError(t, cli.RunJSON(&resp, args...)) + return resp.Models + } + + commands := []struct { + name string + args []string + bareArray bool + }{ + {"request list", []string{"gateway", "request", "list", "--source-id", conn.Source.ID}, false}, + {"request events", []string{"gateway", "request", "events", requestID}, true}, + } + + t.Run("request list with include data and no limit the page size is 10", func(t *testing.T) { + var resp listed + require.NoError(t, cli.RunJSON(&resp, "gateway", "request", "list", "--source-id", conn.Source.ID, "--include", "data")) + assert.Equal(t, 10, resp.Pagination.Limit) + }) + + for _, c := range commands { + t.Run(c.name+" without include there is no payload", func(t *testing.T) { + models := listModels(t, append(c.args, "--limit", "1"), c.bareArray) + require.NotEmpty(t, models) + assert.Nil(t, models[0].Data) + }) + + t.Run(c.name+" with include data the payload comes back", func(t *testing.T) { + models := listModels(t, append(c.args, "--limit", "1", "--include", "data"), c.bareArray) + require.NotEmpty(t, models) + require.NotNil(t, models[0].Data, "include data must put a data object on the model") + // triggerTestEvent posts {"test":true}. + assert.Equal(t, map[string]any{"test": true}, models[0].Data["body"]) + }) + + t.Run(c.name+" include without output json is refused", func(t *testing.T) { + stdout, stderr, err := cli.Run(append(c.args, "--include", "data")...) + require.Error(t, err) + assert.Contains(t, stdout+stderr, "--output json") + }) + } +}