diff --git a/REFERENCE.md b/REFERENCE.md index 086322f3..e5841841 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -1936,6 +1936,8 @@ Query Event Gateway metrics (events, requests, attempts, queue depth, pending ev **Common flags (all metrics subcommands):** `--start`, `--end` (required), `--granularity` (e.g. 1h, 5m, 1d), `--measures`, `--dimensions`, `--source-id`, `--destination-id`, `--connection-id`, `--status`, `--output` (json). +`--delivery-group` filters by delivery group on `metrics events` and `metrics attempts` only. The requests and transformations endpoints do not accept it, so the flag is not offered there, and it cannot be combined with `--measures pending` or per-issue metrics. + ## Utilities diff --git a/pkg/cmd/gateway.go b/pkg/cmd/gateway.go index 580a7fa8..b96ac471 100644 --- a/pkg/cmd/gateway.go +++ b/pkg/cmd/gateway.go @@ -46,13 +46,7 @@ func requireGatewayProject(cfg *config.Config) error { if cfg.Profile.ProjectId == "" { return fmt.Errorf("no project selected. Run 'hookdeck project use' to select a project") } - projectType := cfg.Profile.ProjectType - if projectType == "" && cfg.Profile.ProjectProduct != "" { - projectType = config.ProductToProjectType(cfg.Profile.ProjectProduct) - } - if projectType == "" && cfg.Profile.ProjectMode != "" { - projectType = config.ModeToProjectType(cfg.Profile.ProjectMode) - } + projectType := cfg.Profile.ResolveProjectType() if projectType == "" { // Resolve team/project/mode/type from API (authoritative for the key). Do not clear // guest_url here — gateway PreRun may run for users who still have a guest upgrade link. diff --git a/pkg/cmd/metrics.go b/pkg/cmd/metrics.go index 93ca5bab..c0ad5bfa 100644 --- a/pkg/cmd/metrics.go +++ b/pkg/cmd/metrics.go @@ -68,14 +68,25 @@ type metricsCommonFlags struct { output string } +// metricsFlagOpts omits flags the target endpoint would reject. A flag the API +// refuses is worse than a missing one: the filter is silently accepted by cobra +// and comes back as an opaque 422 from the server. +type metricsFlagOpts struct { + // skipIssueID omits --issue-id for subcommands that take the id as an + // argument instead (e.g. events-by-issue ). + skipIssueID bool + // skipDeliveryGroup omits --delivery-group. Only the events, attempts and + // queue-depth filter schemas accept delivery_group; requests and + // transformations do not, and their filters are additionalProperties:false. + skipDeliveryGroup bool +} + // addMetricsCommonFlags adds common metrics flags to cmd and binds them to f. -// For subcommands that take a required resource id as an argument (e.g. events-by-issue ), -// pass skipIssueID true so --issue-id is not added as a flag. func addMetricsCommonFlags(cmd *cobra.Command, f *metricsCommonFlags) { - addMetricsCommonFlagsEx(cmd, f, false) + addMetricsCommonFlagsEx(cmd, f, metricsFlagOpts{}) } -func addMetricsCommonFlagsEx(cmd *cobra.Command, f *metricsCommonFlags, skipIssueID bool) { +func addMetricsCommonFlagsEx(cmd *cobra.Command, f *metricsCommonFlags, opts metricsFlagOpts) { cmd.Flags().StringVar(&f.start, "start", "", "Start of time range (ISO 8601 date-time, required)") cmd.Flags().StringVar(&f.end, "end", "", "End of time range (ISO 8601 date-time, required)") cmd.Flags().StringVar(&f.granularity, "granularity", "", granularityHelp) @@ -83,10 +94,12 @@ func addMetricsCommonFlagsEx(cmd *cobra.Command, f *metricsCommonFlags, skipIssu cmd.Flags().StringVar(&f.dimensions, "dimensions", "", "Comma-separated dimensions to group by (e.g. connection_id, source_id, destination_id, delivery_group, status)") cmd.Flags().StringVar(&f.sourceID, "source-id", "", "Filter by source ID") cmd.Flags().StringVar(&f.destinationID, "destination-id", "", "Filter by destination ID") - cmd.Flags().StringVar(&f.deliveryGroup, "delivery-group", "", "Filter by delivery group") + if !opts.skipDeliveryGroup { + cmd.Flags().StringVar(&f.deliveryGroup, "delivery-group", "", "Filter by delivery group") + } cmd.Flags().StringVar(&f.connectionID, "connection-id", "", "Filter by connection ID") cmd.Flags().StringVar(&f.status, "status", "", "Filter by status (e.g. SUCCESSFUL, FAILED)") - if !skipIssueID { + if !opts.skipIssueID { cmd.Flags().StringVar(&f.issueID, "issue-id", "", "Filter by issue ID (required for per-issue metrics, e.g. when using --dimensions issue_id)") } cmd.Flags().StringVar(&f.output, "output", "", "Output format (json)") diff --git a/pkg/cmd/metrics_delivery_group_test.go b/pkg/cmd/metrics_delivery_group_test.go new file mode 100644 index 00000000..52e02741 --- /dev/null +++ b/pkg/cmd/metrics_delivery_group_test.go @@ -0,0 +1,74 @@ +package cmd + +import ( + "context" + "testing" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestDeliveryGroupFlagOnlyWhereTheAPIAcceptsIt covers the filter schemas the API +// actually declares: delivery_group exists on events, attempts and queue-depth, +// and not on requests or transformations. Those filters are additionalProperties: +// false, so offering the flag where it is not accepted turns a typo-level mistake +// into an opaque 422 from the server. +func TestDeliveryGroupFlagOnlyWhereTheAPIAcceptsIt(t *testing.T) { + tests := []struct { + name string + cmd *cobra.Command + expected bool + }{ + {"events", newMetricsEventsCmd().cmd, true}, + {"attempts", newMetricsAttemptsCmd().cmd, true}, + {"requests", newMetricsRequestsCmd().cmd, false}, + {"transformations", newMetricsTransformationsCmd().cmd, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + flag := tt.cmd.Flags().Lookup("delivery-group") + if tt.expected { + assert.NotNil(t, flag, "%s accepts delivery_group and should offer the flag", tt.name) + } else { + assert.Nil(t, flag, "%s rejects unknown filters; the flag must not be offered", tt.name) + } + }) + } +} + +// TestEventMetricsRejectDeliveryGroupOnUnsupportedRoutes covers the two routes +// `metrics events` can take where the target endpoint has no delivery_group in +// its filter schema. The client must say so rather than let the API answer 422. +func TestEventMetricsRejectDeliveryGroupOnUnsupportedRoutes(t *testing.T) { + t.Run("pending timeseries", func(t *testing.T) { + _, err := queryEventMetricsConsolidated(context.Background(), nil, hookdeck.MetricsQueryParams{ + Measures: []string{"pending"}, + Granularity: "1h", + DeliveryGroup: "dg_1", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "--delivery-group") + }) + + t.Run("per-issue", func(t *testing.T) { + _, err := queryEventMetricsConsolidated(context.Background(), nil, hookdeck.MetricsQueryParams{ + Dimensions: []string{"issue_id"}, + IssueID: "iss_1", + DeliveryGroup: "dg_1", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "--delivery-group") + }) + + t.Run("per-issue still reports the missing issue id first", func(t *testing.T) { + _, err := queryEventMetricsConsolidated(context.Background(), nil, hookdeck.MetricsQueryParams{ + Dimensions: []string{"issue_id"}, + DeliveryGroup: "dg_1", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "--issue-id") + }) +} diff --git a/pkg/cmd/metrics_events.go b/pkg/cmd/metrics_events.go index df0430a1..5c29de63 100644 --- a/pkg/cmd/metrics_events.go +++ b/pkg/cmd/metrics_events.go @@ -75,6 +75,9 @@ func queryEventMetricsConsolidated(ctx context.Context, client *hookdeck.Client, // 2. If measures include "pending" with granularity → QueryEventsPendingTimeseries // API expects measures[]=count; "pending" is only used for routing. if hasMeasure(params, map[string]bool{"pending": true}) && params.Granularity != "" { + if params.DeliveryGroup != "" { + return nil, errors.New("--delivery-group cannot be used with --measures pending; the pending timeseries endpoint filters on destination only") + } pendingParams := params pendingParams.Measures = []string{"count"} return client.QueryEventsPendingTimeseries(ctx, pendingParams) @@ -85,6 +88,9 @@ func queryEventMetricsConsolidated(ctx context.Context, client *hookdeck.Client, if params.IssueID == "" { return nil, errors.New("per-issue metrics require --issue-id (required when using --dimensions issue_id)") } + if params.DeliveryGroup != "" { + return nil, errors.New("--delivery-group cannot be used with per-issue metrics; the events-by-issue endpoint does not filter on delivery group") + } return client.QueryEventsByIssue(ctx, params) } // 4. Default → QueryEventMetrics diff --git a/pkg/cmd/metrics_requests.go b/pkg/cmd/metrics_requests.go index 084dbf11..5d515924 100644 --- a/pkg/cmd/metrics_requests.go +++ b/pkg/cmd/metrics_requests.go @@ -10,7 +10,7 @@ import ( const metricsRequestsMeasures = "count, accepted_count, rejected_count, discarded_count, avg_events_per_request, avg_ignored_per_request" type metricsRequestsCmd struct { - cmd *cobra.Command + cmd *cobra.Command flags metricsCommonFlags } @@ -23,7 +23,8 @@ func newMetricsRequestsCmd() *metricsRequestsCmd { Long: LongBeta(`Query metrics for requests (acceptance, rejection, etc.). Measures: ` + metricsRequestsMeasures + `.`), RunE: c.runE, } - addMetricsCommonFlags(c.cmd, &c.flags) + // The requests filter schema has no delivery_group, and rejects unknown filters. + addMetricsCommonFlagsEx(c.cmd, &c.flags, metricsFlagOpts{skipDeliveryGroup: true}) return c } diff --git a/pkg/cmd/metrics_transformations.go b/pkg/cmd/metrics_transformations.go index a47b6e8d..3a7a6b5b 100644 --- a/pkg/cmd/metrics_transformations.go +++ b/pkg/cmd/metrics_transformations.go @@ -10,7 +10,7 @@ import ( const metricsTransformationsMeasures = "count, successful_count, failed_count, error_rate, error_count, warn_count, info_count, debug_count" type metricsTransformationsCmd struct { - cmd *cobra.Command + cmd *cobra.Command flags metricsCommonFlags } @@ -23,7 +23,8 @@ func newMetricsTransformationsCmd() *metricsTransformationsCmd { Long: LongBeta(`Query metrics for transformations. Measures: ` + metricsTransformationsMeasures + `.`), RunE: c.runE, } - addMetricsCommonFlags(c.cmd, &c.flags) + // The transformations filter schema has no delivery_group, and rejects unknown filters. + addMetricsCommonFlagsEx(c.cmd, &c.flags, metricsFlagOpts{skipDeliveryGroup: true}) return c } diff --git a/pkg/cmd/whoami.go b/pkg/cmd/whoami.go index 2545f69c..f4a21912 100644 --- a/pkg/cmd/whoami.go +++ b/pkg/cmd/whoami.go @@ -68,13 +68,7 @@ func (lc *whoamiCmd) runWhoamiCmd(cmd *cobra.Command, args []string) error { fmt.Printf("%s\n", note) } - projectType := Config.Profile.ProjectType - if projectType == "" && Config.Profile.ProjectProduct != "" { - projectType = config.ProductToProjectType(Config.Profile.ProjectProduct) - } - if projectType == "" && Config.Profile.ProjectMode != "" { - projectType = config.ModeToProjectType(Config.Profile.ProjectMode) - } + projectType := Config.Profile.ResolveProjectType() if projectType == "" && projectProduct != "" { projectType = config.ProductToProjectType(projectProduct) } diff --git a/pkg/config/config.go b/pkg/config/config.go index 9c405b55..ad6a8e54 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -274,14 +274,7 @@ func (c *Config) setProfileFieldsInViper(v *viper.Viper) { v.Set(c.Profile.getConfigField("project_id"), c.Profile.ProjectId) v.Set(c.Profile.getConfigField("project_product"), c.Profile.ProjectProduct) v.Set(c.Profile.getConfigField("project_mode"), c.Profile.ProjectMode) - projectType := c.Profile.ProjectType - if projectType == "" && c.Profile.ProjectProduct != "" { - projectType = ProductToProjectType(c.Profile.ProjectProduct) - } - if projectType == "" && c.Profile.ProjectMode != "" { - projectType = ModeToProjectType(c.Profile.ProjectMode) - } - v.Set(c.Profile.getConfigField("project_type"), projectType) + v.Set(c.Profile.getConfigField("project_type"), c.Profile.ResolveProjectType()) if c.Profile.GuestURL != "" { v.Set(c.Profile.getConfigField("guest_url"), c.Profile.GuestURL) } @@ -401,12 +394,7 @@ func (c *Config) constructConfig() { // ProjectType: prefer project_type, then derive from the public product, then legacy mode. c.Profile.ProjectType = stringCoalesce(c.Profile.ProjectType, c.viper.GetString(c.Profile.getConfigField("project_type")), c.viper.GetString("project_type"), "") - if c.Profile.ProjectType == "" && c.Profile.ProjectProduct != "" { - c.Profile.ProjectType = ProductToProjectType(c.Profile.ProjectProduct) - } - if c.Profile.ProjectType == "" && c.Profile.ProjectMode != "" { - c.Profile.ProjectType = ModeToProjectType(c.Profile.ProjectMode) - } + c.Profile.ProjectType = c.Profile.ResolveProjectType() if c.Profile.ProjectProduct == "" && c.Profile.ProjectMode != "" { c.Profile.ProjectProduct = ModeToProduct(c.Profile.ProjectMode) } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 22851d2b..f23183ca 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -260,6 +260,11 @@ func TestInitConfig(t *testing.T) { assert.Equal(t, "inbound", c.Profile.ProjectMode) assert.Equal(t, "Gateway", c.Profile.ProjectType) + // The upgrade path: a config written before 2026-09-01 has no + // project_product, so it has to be derived from the legacy mode. + // Without this, an upgraded user has an empty product until they + // log in again, and IsGatewayProject("") fails every gateway command. + assert.Equal(t, "event_gateway", c.Profile.ProjectProduct) }) t.Run("project_type and project_mode - prefer project_type", func(t *testing.T) { @@ -313,6 +318,25 @@ func TestWriteConfig(t *testing.T) { contentBytes, _ := ioutil.ReadFile(c.viper.ConfigFileUsed()) assert.Contains(t, string(contentBytes), `project_id = 'new_team_id'`) assert.Contains(t, string(contentBytes), `project_type = 'Gateway'`) + // A legacy mode in, the public product written back out. + assert.Contains(t, string(contentBytes), `project_product = 'event_gateway'`) + assert.Contains(t, string(contentBytes), `project_mode = 'inbound'`) + }) + + t.Run("use project with a product", func(t *testing.T) { + t.Parallel() + + c := Config{LogLevel: "info"} + c.ConfigFileFlag = setupTempConfig(t, "./testdata/default-profile.toml") + c.InitConfig() + + err := c.UseProject("new_team_id", "outpost") + + assert.NoError(t, err) + contentBytes, _ := ioutil.ReadFile(c.viper.ConfigFileUsed()) + assert.Contains(t, string(contentBytes), `project_product = 'outpost'`) + assert.Contains(t, string(contentBytes), `project_type = 'Outpost'`) + assert.Contains(t, string(contentBytes), `project_mode = 'outpost'`) }) t.Run("use profile", func(t *testing.T) { diff --git a/pkg/config/profile.go b/pkg/config/profile.go index 517f5ddb..3d262588 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -5,13 +5,13 @@ import ( ) type Profile struct { - Name string // profile name - APIKey string - ProjectId string + Name string // profile name + APIKey string + ProjectId string ProjectProduct string - ProjectMode string - ProjectType string // display type: Gateway, Outpost, Console - GuestURL string // URL to create permanent account for guest users + ProjectMode string + ProjectType string // display type: Gateway, Outpost, Console + GuestURL string // URL to create permanent account for guest users Config *Config } @@ -21,19 +21,26 @@ func (p *Profile) getConfigField(field string) string { return p.Name + "." + field } +// ResolveProjectType returns the display type for this profile: the stored type +// if there is one, otherwise derived from the product, otherwise from the legacy +// mode. This precedence was open-coded in five places; keep it here so a caller +// cannot get the order subtly wrong. +func (p *Profile) ResolveProjectType() string { + if p.ProjectType != "" { + return p.ProjectType + } + if t := ProductToProjectType(p.ProjectProduct); t != "" { + return t + } + return ModeToProjectType(p.ProjectMode) +} + func (p *Profile) SaveProfile() error { p.Config.viper.Set(p.getConfigField("api_key"), p.APIKey) p.Config.viper.Set(p.getConfigField("project_id"), p.ProjectId) p.Config.viper.Set(p.getConfigField("project_product"), p.ProjectProduct) p.Config.viper.Set(p.getConfigField("project_mode"), p.ProjectMode) - projectType := p.ProjectType - if projectType == "" && p.ProjectProduct != "" { - projectType = ProductToProjectType(p.ProjectProduct) - } - if projectType == "" && p.ProjectMode != "" { - projectType = ModeToProjectType(p.ProjectMode) - } - p.Config.viper.Set(p.getConfigField("project_type"), projectType) + p.Config.viper.Set(p.getConfigField("project_type"), p.ResolveProjectType()) p.Config.viper.Set(p.getConfigField("guest_url"), p.GuestURL) if err := p.removeLegacyConfigKeys(); err != nil { diff --git a/pkg/config/profile_credentials.go b/pkg/config/profile_credentials.go index 8f8914fd..8790b1cd 100644 --- a/pkg/config/profile_credentials.go +++ b/pkg/config/profile_credentials.go @@ -2,6 +2,17 @@ package config import "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +// resolveProduct returns the product to store. It prefers the current +// team_product field and falls back to the pre-2026-09-01 team_mode, so a +// response missing the new field leaves the profile usable rather than blank: +// an empty product makes IsGatewayProject false and fails every gateway command. +func resolveProduct(product, legacyMode string) string { + if product != "" { + return product + } + return ModeToProduct(legacyMode) +} + // ApplyValidateAPIKeyResponse updates project fields from GET /cli-auth/validate. // When clearGuestURL is true, GuestURL is cleared (e.g. hookdeck login re-verify). // When false, GuestURL is left unchanged (e.g. gateway PreRun resolving type only). @@ -10,9 +21,10 @@ func (p *Profile) ApplyValidateAPIKeyResponse(resp *hookdeck.ValidateAPIKeyRespo return } p.ProjectId = resp.ProjectID - p.ProjectProduct = resp.ProjectProduct - p.ProjectMode = ProductToLegacyMode(resp.ProjectProduct) - p.ProjectType = ProductToProjectType(resp.ProjectProduct) + product := resolveProduct(resp.ProjectProduct, resp.ProjectMode) + p.ProjectProduct = product + p.ProjectMode = ProductToLegacyMode(product) + p.ProjectType = ProductToProjectType(product) if clearGuestURL { p.GuestURL = "" } @@ -26,9 +38,10 @@ func (p *Profile) ApplyPollAPIKeyResponse(resp *hookdeck.PollAPIKeyResponse, gue } p.APIKey = resp.APIKey p.ProjectId = resp.ProjectID - p.ProjectProduct = resp.ProjectProduct - p.ProjectMode = ProductToLegacyMode(resp.ProjectProduct) - p.ProjectType = ProductToProjectType(resp.ProjectProduct) + product := resolveProduct(resp.ProjectProduct, resp.ProjectMode) + p.ProjectProduct = product + p.ProjectMode = ProductToLegacyMode(product) + p.ProjectType = ProductToProjectType(product) p.GuestURL = guestURL } @@ -36,8 +49,9 @@ func (p *Profile) ApplyPollAPIKeyResponse(resp *hookdeck.PollAPIKeyResponse, gue func (p *Profile) ApplyCIClient(ci hookdeck.CIClient) { p.APIKey = ci.APIKey p.ProjectId = ci.ProjectID - p.ProjectProduct = ci.ProjectProduct - p.ProjectMode = ProductToLegacyMode(ci.ProjectProduct) - p.ProjectType = ProductToProjectType(ci.ProjectProduct) + product := resolveProduct(ci.ProjectProduct, ci.ProjectMode) + p.ProjectProduct = product + p.ProjectMode = ProductToLegacyMode(product) + p.ProjectType = ProductToProjectType(product) p.GuestURL = "" } diff --git a/pkg/config/profile_credentials_test.go b/pkg/config/profile_credentials_test.go index 6e6a9a48..85607f4b 100644 --- a/pkg/config/profile_credentials_test.go +++ b/pkg/config/profile_credentials_test.go @@ -21,7 +21,7 @@ func TestProfile_ApplyValidateAPIKeyResponse(t *testing.T) { t.Run("sets project fields and clears guest when requested", func(t *testing.T) { p := &Profile{GuestURL: "https://guest"} p.ApplyValidateAPIKeyResponse(&hookdeck.ValidateAPIKeyResponse{ - ProjectID: "team_1", + ProjectID: "team_1", ProjectProduct: "event_gateway", }, true) require.Equal(t, "team_1", p.ProjectId) @@ -34,7 +34,7 @@ func TestProfile_ApplyValidateAPIKeyResponse(t *testing.T) { t.Run("preserves guest URL when clearGuestURL is false", func(t *testing.T) { p := &Profile{GuestURL: "https://guest.example/x"} p.ApplyValidateAPIKeyResponse(&hookdeck.ValidateAPIKeyResponse{ - ProjectID: "team_2", + ProjectID: "team_2", ProjectProduct: "console", }, false) require.Equal(t, "team_2", p.ProjectId) @@ -43,6 +43,62 @@ func TestProfile_ApplyValidateAPIKeyResponse(t *testing.T) { }) } +// TestProfile_LegacyModeFallback covers a response that predates team_product, +// or one where the field is absent for any other reason. Without the fallback +// the profile is blanked: ProjectType becomes "", IsGatewayProject("") is false, +// and every `hookdeck gateway ...` command fails with an empty project type. +func TestProfile_LegacyModeFallback(t *testing.T) { + t.Run("validate response falls back to team_mode", func(t *testing.T) { + p := &Profile{} + p.ApplyValidateAPIKeyResponse(&hookdeck.ValidateAPIKeyResponse{ + ProjectID: "team_legacy", + ProjectMode: "outbound", + }, false) + require.Equal(t, "event_gateway", p.ProjectProduct) + require.Equal(t, ProjectTypeGateway, p.ProjectType) + }) + + t.Run("poll response falls back to team_mode", func(t *testing.T) { + p := &Profile{} + p.ApplyPollAPIKeyResponse(&hookdeck.PollAPIKeyResponse{ + APIKey: "key", + ProjectID: "team_legacy", + ProjectMode: "console", + }, "") + require.Equal(t, "console", p.ProjectProduct) + require.Equal(t, ProjectTypeConsole, p.ProjectType) + }) + + t.Run("ci client falls back to team_mode", func(t *testing.T) { + p := &Profile{} + p.ApplyCIClient(hookdeck.CIClient{ + APIKey: "key", + ProjectID: "team_legacy", + ProjectMode: "outpost", + }) + require.Equal(t, "outpost", p.ProjectProduct) + require.Equal(t, ProjectTypeOutpost, p.ProjectType) + }) + + t.Run("product wins when both are present", func(t *testing.T) { + p := &Profile{} + p.ApplyValidateAPIKeyResponse(&hookdeck.ValidateAPIKeyResponse{ + ProjectID: "team_both", + ProjectProduct: "outpost", + ProjectMode: "inbound", + }, false) + require.Equal(t, "outpost", p.ProjectProduct) + require.Equal(t, ProjectTypeOutpost, p.ProjectType) + }) + + t.Run("both absent leaves the type empty", func(t *testing.T) { + p := &Profile{} + p.ApplyValidateAPIKeyResponse(&hookdeck.ValidateAPIKeyResponse{ProjectID: "team_none"}, false) + require.Empty(t, p.ProjectProduct) + require.Empty(t, p.ProjectType) + }) +} + func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { t.Run("nil response is no-op", func(t *testing.T) { p := &Profile{APIKey: "k", ProjectId: "p"} @@ -54,8 +110,8 @@ func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { t.Run("sets credentials and guest URL", func(t *testing.T) { p := &Profile{} p.ApplyPollAPIKeyResponse(&hookdeck.PollAPIKeyResponse{ - APIKey: "key_from_poll", - ProjectID: "team_p", + APIKey: "key_from_poll", + ProjectID: "team_p", ProjectProduct: "event_gateway", }, "https://guest") require.Equal(t, "key_from_poll", p.APIKey) @@ -67,8 +123,8 @@ func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { t.Run("clears guest URL when empty string passed", func(t *testing.T) { p := &Profile{GuestURL: "old"} p.ApplyPollAPIKeyResponse(&hookdeck.PollAPIKeyResponse{ - APIKey: "k123456789012", - ProjectID: "t", + APIKey: "k123456789012", + ProjectID: "t", ProjectProduct: "event_gateway", }, "") require.Empty(t, p.GuestURL) @@ -78,8 +134,8 @@ func TestProfile_ApplyPollAPIKeyResponse(t *testing.T) { func TestProfile_ApplyCIClient(t *testing.T) { p := &Profile{} p.ApplyCIClient(hookdeck.CIClient{ - APIKey: "ci_key_123456", - ProjectID: "team_ci", + APIKey: "ci_key_123456", + ProjectID: "team_ci", ProjectProduct: "event_gateway", }) require.Equal(t, "ci_key_123456", p.APIKey) diff --git a/pkg/config/project_type.go b/pkg/config/project_type.go index 271f65c3..0dccbc6b 100644 --- a/pkg/config/project_type.go +++ b/pkg/config/project_type.go @@ -48,13 +48,14 @@ func ProductToProjectType(product string) string { } // ProjectTypeToProduct maps the CLI display type to the public API product. +// Case-insensitive, like the other mappers in this file. func ProjectTypeToProduct(projectType string) string { - switch projectType { - case ProjectTypeGateway: + switch strings.ToLower(projectType) { + case strings.ToLower(ProjectTypeGateway): return ProjectProductEventGateway - case ProjectTypeConsole: + case strings.ToLower(ProjectTypeConsole): return ProjectProductConsole - case ProjectTypeOutpost: + case strings.ToLower(ProjectTypeOutpost): return ProjectProductOutpost default: return "" @@ -106,9 +107,10 @@ func ProjectTypeToMode(projectType string) string { } // IsGatewayProject returns true if the given type, product, or legacy mode represents a Gateway project. +// Case-insensitive, like the other mappers in this file. func IsGatewayProject(typeProductOrMode string) bool { - switch typeProductOrMode { - case ProjectTypeGateway, ProjectTypeConsole, ProjectProductEventGateway, "inbound", "outbound", "console": + switch strings.ToLower(typeProductOrMode) { + case strings.ToLower(ProjectTypeGateway), strings.ToLower(ProjectTypeConsole), ProjectProductEventGateway, "inbound", "outbound", "console": return true default: return false diff --git a/pkg/config/project_type_test.go b/pkg/config/project_type_test.go index bdf1d9a2..a956a875 100644 --- a/pkg/config/project_type_test.go +++ b/pkg/config/project_type_test.go @@ -48,15 +48,101 @@ func TestProjectTypeToMode(t *testing.T) { } } -func TestProductMappings(t *testing.T) { - assert.Equal(t, ProjectTypeGateway, ProductToProjectType("event_gateway")) - assert.Equal(t, ProjectTypeConsole, ProductToProjectType("console")) - assert.Equal(t, ProjectTypeOutpost, ProductToProjectType("outpost")) - assert.Equal(t, "", ProductToProjectType("unknown")) +func TestProductToProjectType(t *testing.T) { + tests := []struct { + product string + expected string + }{ + {ProjectProductEventGateway, ProjectTypeGateway}, + {ProjectProductConsole, ProjectTypeConsole}, + {ProjectProductOutpost, ProjectTypeOutpost}, + {"EVENT_GATEWAY", ProjectTypeGateway}, + {"unknown", ""}, + // An empty product is the response-missing-the-field case. It must not + // resolve to a type, and callers have to treat "" as "unknown", not as + // a project that happens to be a Gateway. + {"", ""}, + } + for _, tt := range tests { + t.Run(tt.product, func(t *testing.T) { + assert.Equal(t, tt.expected, ProductToProjectType(tt.product)) + }) + } +} + +func TestProjectTypeToProduct(t *testing.T) { + tests := []struct { + projectType string + expected string + }{ + {ProjectTypeGateway, ProjectProductEventGateway}, + {ProjectTypeConsole, ProjectProductConsole}, + {ProjectTypeOutpost, ProjectProductOutpost}, + {"gateway", ProjectProductEventGateway}, + {"Unknown", ""}, + {"", ""}, + } + for _, tt := range tests { + t.Run(tt.projectType, func(t *testing.T) { + assert.Equal(t, tt.expected, ProjectTypeToProduct(tt.projectType)) + }) + } +} + +func TestProductToLegacyMode(t *testing.T) { + tests := []struct { + product string + expected string + }{ + // event_gateway covers both inbound and outbound; "inbound" is the + // representative value written back to config. + {ProjectProductEventGateway, "inbound"}, + {ProjectProductConsole, "console"}, + {ProjectProductOutpost, "outpost"}, + {"OUTPOST", "outpost"}, + {"unknown", ""}, + {"", ""}, + } + for _, tt := range tests { + t.Run(tt.product, func(t *testing.T) { + assert.Equal(t, tt.expected, ProductToLegacyMode(tt.product)) + }) + } +} + +func TestModeToProduct(t *testing.T) { + tests := []struct { + mode string + expected string + }{ + {"inbound", ProjectProductEventGateway}, + {OutboundMode, ProjectProductEventGateway}, + {"console", ProjectProductConsole}, + {"outpost", ProjectProductOutpost}, + {"Inbound", ProjectProductEventGateway}, + {"unknown", ""}, + {"", ""}, + } + for _, tt := range tests { + t.Run(tt.mode, func(t *testing.T) { + assert.Equal(t, tt.expected, ModeToProduct(tt.mode)) + }) + } +} + +// TestProductRoundTrip pins the deliberate lossiness of the mapping: the public +// API folds inbound and outbound into one product, so a round trip through +// product normalizes outbound to inbound. Type survives; mode does not. +func TestProductRoundTrip(t *testing.T) { + for _, projectType := range []string{ProjectTypeGateway, ProjectTypeConsole, ProjectTypeOutpost} { + t.Run(projectType, func(t *testing.T) { + assert.Equal(t, projectType, ProductToProjectType(ProjectTypeToProduct(projectType))) + }) + } - assert.Equal(t, "event_gateway", ProjectTypeToProduct(ProjectTypeGateway)) - assert.Equal(t, "inbound", ProductToLegacyMode("event_gateway")) - assert.Equal(t, "event_gateway", ModeToProduct("outbound")) + assert.Equal(t, "inbound", ProductToLegacyMode(ModeToProduct("outbound")), + "outbound is expected to normalize to inbound through the product mapping") + assert.Equal(t, "inbound", ProductToLegacyMode(ModeToProduct("inbound"))) } func TestIsGatewayProject(t *testing.T) { @@ -67,7 +153,8 @@ func TestIsGatewayProject(t *testing.T) { assert.True(t, IsGatewayProject(v)) }) } - falseCases := []string{ProjectTypeOutpost, ""} + trueCases = append(trueCases, "EVENT_GATEWAY", "Inbound") + falseCases := []string{ProjectTypeOutpost, ProjectProductOutpost, "", "unknown"} for _, v := range falseCases { t.Run("false_"+v, func(t *testing.T) { assert.False(t, IsGatewayProject(v)) diff --git a/pkg/hookdeck/auth.go b/pkg/hookdeck/auth.go index abefc803..16f90c14 100644 --- a/pkg/hookdeck/auth.go +++ b/pkg/hookdeck/auth.go @@ -27,7 +27,10 @@ type ValidateAPIKeyResponse struct { ProjectID string `json:"team_id"` ProjectName string `json:"team_name_no_org"` ProjectProduct string `json:"team_product"` - ClientID string `json:"client_id"` + // ProjectMode is the pre-2026-09-01 field. Kept so a response without + // team_product still resolves a project type instead of blanking it. + ProjectMode string `json:"team_mode"` + ClientID string `json:"client_id"` } // PollAPIKeyResponse returns the data of the polling client login @@ -41,8 +44,11 @@ type PollAPIKeyResponse struct { ProjectID string `json:"team_id"` ProjectName string `json:"team_name"` ProjectProduct string `json:"team_product"` - APIKey string `json:"key"` - ClientID string `json:"client_id"` + // ProjectMode is the pre-2026-09-01 field. Kept so a response without + // team_product still resolves a project type instead of blanking it. + ProjectMode string `json:"team_mode"` + APIKey string `json:"key"` + ClientID string `json:"client_id"` } // UpdateClientInput represents the input for updating a CLI client diff --git a/pkg/hookdeck/ci.go b/pkg/hookdeck/ci.go index d8805aae..9a27dfa8 100644 --- a/pkg/hookdeck/ci.go +++ b/pkg/hookdeck/ci.go @@ -16,8 +16,11 @@ type CIClient struct { ProjectID string `json:"team_id"` ProjectName string `json:"team_name"` ProjectProduct string `json:"team_product"` - APIKey string `json:"key"` - ClientID string `json:"client_id"` + // ProjectMode is the pre-2026-09-01 field. Kept so a response without + // team_product still resolves a project type instead of blanking it. + ProjectMode string `json:"team_mode"` + APIKey string `json:"key"` + ClientID string `json:"client_id"` } type CreateCIClientInput struct { diff --git a/pkg/hookdeck/events.go b/pkg/hookdeck/events.go index 08131da9..fe058ba4 100644 --- a/pkg/hookdeck/events.go +++ b/pkg/hookdeck/events.go @@ -15,7 +15,7 @@ type Event struct { WebhookID string `json:"webhook_id"` SourceID string `json:"source_id"` DestinationID string `json:"destination_id"` - DeliveryGroup *string `json:"delivery_group"` + DeliveryGroup *string `json:"delivery_group,omitempty"` RequestID string `json:"request_id"` Attempts int `json:"attempts"` ResponseStatus *int `json:"response_status,omitempty"` diff --git a/pkg/hookdeck/projects.go b/pkg/hookdeck/projects.go index ea4e9488..eb14c8c3 100644 --- a/pkg/hookdeck/projects.go +++ b/pkg/hookdeck/projects.go @@ -2,6 +2,7 @@ package hookdeck import ( "context" + "fmt" ) type Project struct { @@ -19,7 +20,11 @@ func (c *Client) ListProjects() ([]Project, error) { return []Project{}, err } projects := []Project{} - postprocessJsonResponse(res, &projects) + // A shape mismatch here used to return an empty list and a nil error, so a + // renamed field or a wrapped envelope read as "you have no projects". + if _, err := postprocessJsonResponse(res, &projects); err != nil { + return []Project{}, fmt.Errorf("failed to parse project list response: %w", err) + } return projects, nil } diff --git a/test/acceptance/login_auth_acceptance_test.go b/test/acceptance/login_auth_acceptance_test.go index 90432853..224f2eb1 100644 --- a/test/acceptance/login_auth_acceptance_test.go +++ b/test/acceptance/login_auth_acceptance_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -99,6 +100,16 @@ api_key = "hk_test_stale_accept01" require.NoError(t, err, "stdout=%q stderr=%q", stdout.String(), stderr.String()) require.Contains(t, stdout.String(), "no longer valid", "user should see stale-key message") require.Equal(t, 1, pollHits, "mock should see exactly one poll after cli-auth") + + // End-to-end check that the 2026-09-01 product survives the whole round trip: + // API response -> profile -> config file. Everything else about the rename is + // covered by unit tests against mocks that this repo also writes, so this is + // the only place the persisted field is verified against a real CLI run. + written, readErr := os.ReadFile(configPath) + require.NoError(t, readErr) + assert.Contains(t, string(written), "project_product = 'event_gateway'") + assert.Contains(t, string(written), "project_type = 'Gateway'") + assert.Contains(t, string(written), "project_id = 'tm_accept'") } // TestCIFailsFastWithInvalidAPIKeyAcceptance verifies hookdeck ci does not enter the