From 1e0da06f07419aaa214588ca92023f7bb74ed408 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 1 Oct 2026 11:55:08 +0100 Subject: [PATCH 1/4] fix(connection): build rules in flag order Rules built from --rule--* flags were always ordered deduplicate -> transform -> filter -> delay -> retry. Filter, transform and deduplicate run in rules array order, so filter-before-transform could not be expressed with flags, and every create, update or upsert with both flags produced transform -> filter. Each rule flag now records its rule type when set, and rules are emitted in the order of each type's first flag, as AGENTS.md describes. Types with no recorded position fall back to the previous order. Also state in the MCP connections tool that rule order matters. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 + pkg/cmd/connection_common.go | 86 ++++++++++++++++-- pkg/cmd/connection_rule_order_test.go | 124 ++++++++++++++++++++++++++ pkg/gateway/mcp/tool_connections.go | 2 +- 4 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 pkg/cmd/connection_rule_order_test.go diff --git a/README.md b/README.md index 8df85d8b..3e2532de 100644 --- a/README.md +++ b/README.md @@ -1219,6 +1219,8 @@ $ hookdeck gateway connection create \ --rule-retry-count 3 ``` +Rules built from `--rule-*` flags follow the order in which each rule type's first flag appears. Filter, transform and deduplicate rules run in that order, so put `--rule-filter-*` before `--rule-transform-*` to filter on the original payload. To set the whole array explicitly, use `--rules` or `--rules-file`. + #### Configure rate limiting Control the rate of event delivery to your destination: diff --git a/pkg/cmd/connection_common.go b/pkg/cmd/connection_common.go index 0029d16b..1ad78943 100644 --- a/pkg/cmd/connection_common.go +++ b/pkg/cmd/connection_common.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" ) @@ -64,6 +65,58 @@ type connectionRuleFlags struct { RuleDeduplicateWindow int RuleDeduplicateIncludeFields string RuleDeduplicateExcludeFields string + + // ruleOrder records rule types in the order their first --rule--* flag + // appeared on the command line. Filter, transform and deduplicate run in + // rules array order, so the flags must not impose a fixed order. + ruleOrder []string +} + +// defaultRuleOrder is used for rule types with no recorded flag position, such as +// when connectionRuleFlags is populated directly rather than from the command line. +var defaultRuleOrder = []string{"deduplicate", "transform", "filter", "delay", "retry"} + +// ruleFlagTypes maps each individual rule flag to the rule type it configures. +var ruleFlagTypes = map[string]string{ + "rule-retry-strategy": "retry", + "rule-retry-count": "retry", + "rule-retry-interval": "retry", + "rule-retry-response-status-codes": "retry", + "rule-filter-body": "filter", + "rule-filter-headers": "filter", + "rule-filter-query": "filter", + "rule-filter-path": "filter", + "rule-transform-name": "transform", + "rule-transform-code": "transform", + "rule-transform-env": "transform", + "rule-delay": "delay", + "rule-deduplicate-window": "deduplicate", + "rule-deduplicate-include-fields": "deduplicate", + "rule-deduplicate-exclude-fields": "deduplicate", +} + +// recordRuleType appends ruleType to the recorded order unless it is already present. +func (f *connectionRuleFlags) recordRuleType(ruleType string) { + for _, t := range f.ruleOrder { + if t == ruleType { + return + } + } + f.ruleOrder = append(f.ruleOrder, ruleType) +} + +// orderTrackingValue wraps a flag value so that setting it records the flag's rule type. +type orderTrackingValue struct { + pflag.Value + onSet func() +} + +func (v *orderTrackingValue) Set(s string) error { + if err := v.Value.Set(s); err != nil { + return err + } + v.onSet() + return nil } // addConnectionRuleFlags binds rule flags to cmd. Pass a pointer to the flags struct @@ -91,11 +144,18 @@ func addConnectionRuleFlags(cmd *cobra.Command, f *connectionRuleFlags) { cmd.Flags().IntVar(&f.RuleDeduplicateWindow, "rule-deduplicate-window", 0, "Time window in seconds for deduplication") cmd.Flags().StringVar(&f.RuleDeduplicateIncludeFields, "rule-deduplicate-include-fields", "", "Comma-separated list of fields to include for deduplication") cmd.Flags().StringVar(&f.RuleDeduplicateExcludeFields, "rule-deduplicate-exclude-fields", "", "Comma-separated list of fields to exclude for deduplication") + + for name, ruleType := range ruleFlagTypes { + flag := cmd.Flags().Lookup(name) + ruleType := ruleType + flag.Value = &orderTrackingValue{Value: flag.Value, onSet: func() { f.recordRuleType(ruleType) }} + } } // buildConnectionRules builds a slice of rules from connectionRuleFlags. // If rulesStr or rulesFile is non-empty, those are parsed as JSON and returned; -// otherwise individual rule flags are assembled into rules. +// otherwise individual rule flags are assembled into rules, ordered by the +// position of the first flag for each rule type. // Shared by connection update and (for consistency) can be used by create/upsert. func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { if f.Rules != "" { @@ -118,8 +178,8 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { return normalizeRulesForAPI(rules), nil } - // Build each rule type (order matches create: deduplicate -> transform -> filter -> delay -> retry) - var rules []hookdeck.Rule + // Build each rule type, then order them by flag position + built := make(map[string]hookdeck.Rule) if f.RuleDeduplicateWindow > 0 { rule := hookdeck.Rule{ @@ -132,7 +192,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { if f.RuleDeduplicateExcludeFields != "" { rule["exclude_fields"] = strings.Split(f.RuleDeduplicateExcludeFields, ",") } - rules = append(rules, rule) + built["deduplicate"] = rule } hasTransform := f.RuleTransformName != "" || f.RuleTransformCode != "" || f.RuleTransformEnv != "" @@ -153,7 +213,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { transformConfig["env"] = env } rule["transformation"] = transformConfig - rules = append(rules, rule) + built["transform"] = rule } if f.RuleFilterBody != "" || f.RuleFilterHeaders != "" || f.RuleFilterQuery != "" || f.RuleFilterPath != "" { @@ -170,14 +230,14 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { if f.RuleFilterPath != "" { rule["path"] = parseJSONOrString(f.RuleFilterPath) } - rules = append(rules, rule) + built["filter"] = rule } if f.RuleDelay > 0 { - rules = append(rules, hookdeck.Rule{ + built["delay"] = hookdeck.Rule{ "type": "delay", "delay": f.RuleDelay, - }) + } } if f.RuleRetryStrategy != "" { @@ -211,7 +271,15 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) { } rule["response_status_codes"] = strCodes } - rules = append(rules, rule) + built["retry"] = rule + } + + var rules []hookdeck.Rule + for _, ruleType := range append(append([]string{}, f.ruleOrder...), defaultRuleOrder...) { + if rule, ok := built[ruleType]; ok { + rules = append(rules, rule) + delete(built, ruleType) + } } return rules, nil diff --git a/pkg/cmd/connection_rule_order_test.go b/pkg/cmd/connection_rule_order_test.go new file mode 100644 index 00000000..ac0d7b30 --- /dev/null +++ b/pkg/cmd/connection_rule_order_test.go @@ -0,0 +1,124 @@ +package cmd + +import ( + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/hookdeck/hookdeck-cli/pkg/hookdeck" +) + +// parseRuleFlags binds the rule flags to a throwaway command and parses args, +// so flag values are set the same way cobra sets them at runtime. +func parseRuleFlags(t *testing.T, args []string) *connectionRuleFlags { + t.Helper() + f := &connectionRuleFlags{} + cmd := &cobra.Command{Use: "test"} + addConnectionRuleFlags(cmd, f) + require.NoError(t, cmd.ParseFlags(args)) + return f +} + +func ruleTypes(rules []hookdeck.Rule) []string { + types := make([]string, 0, len(rules)) + for _, r := range rules { + types = append(types, r["type"].(string)) + } + return types +} + +// TestBuildConnectionRulesFollowsFlagOrder verifies that rules built from +// --rule--* flags follow the position of the first flag for each type. +// Filter, transform and deduplicate run in rules array order, so a fixed +// order made filter-before-transform impossible to express with flags. +func TestBuildConnectionRulesFollowsFlagOrder(t *testing.T) { + tests := []struct { + name string + args []string + want []string + }{ + { + name: "filter before transform", + args: []string{"--rule-filter-body", `{"type":"order"}`, "--rule-transform-name", "tx1"}, + want: []string{"filter", "transform"}, + }, + { + name: "transform before filter", + args: []string{"--rule-transform-name", "tx1", "--rule-filter-body", `{"type":"order"}`}, + want: []string{"transform", "filter"}, + }, + { + name: "first flag of a type sets its position", + args: []string{ + "--rule-filter-body", `{"type":"order"}`, + "--rule-transform-name", "tx1", + "--rule-filter-headers", `{"x-topic":"orders/create"}`, + }, + want: []string{"filter", "transform"}, + }, + { + name: "all five types in flag order", + args: []string{ + "--rule-retry-strategy", "exponential", + "--rule-filter-body", `{"type":"order"}`, + "--rule-delay", "1000", + "--rule-transform-name", "tx1", + "--rule-deduplicate-window", "60", + }, + want: []string{"retry", "filter", "delay", "transform", "deduplicate"}, + }, + { + name: "retry position set by a non-strategy retry flag", + args: []string{"--rule-retry-count", "3", "--rule-filter-body", `{"type":"order"}`, "--rule-retry-strategy", "linear"}, + want: []string{"retry", "filter"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rules, err := buildConnectionRules(parseRuleFlags(t, tt.args)) + require.NoError(t, err) + assert.Equal(t, tt.want, ruleTypes(rules)) + }) + } +} + +// TestBuildConnectionRulesDefaultOrderWithoutFlags verifies the fallback order +// when connectionRuleFlags is populated directly rather than parsed from flags. +func TestBuildConnectionRulesDefaultOrderWithoutFlags(t *testing.T) { + flags := connectionRuleFlags{ + RuleRetryStrategy: "linear", + RuleFilterBody: `{"type":"order"}`, + RuleTransformName: "tx1", + RuleDelay: 1000, + RuleDeduplicateWindow: 60, + } + rules, err := buildConnectionRules(&flags) + require.NoError(t, err) + assert.Equal(t, []string{"deduplicate", "transform", "filter", "delay", "retry"}, ruleTypes(rules)) +} + +// TestConnectionCommandsRuleFlagOrder verifies that create, update and upsert +// all record rule flag order through their own flag sets. +func TestConnectionCommandsRuleFlagOrder(t *testing.T) { + args := []string{"--rule-filter-body", `{"type":"order"}`, "--rule-transform-name", "tx1"} + + create := newConnectionCreateCmd() + require.NoError(t, create.cmd.ParseFlags(args)) + rules, err := buildConnectionRules(&create.connectionRuleFlags) + require.NoError(t, err) + assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "create") + + update := newConnectionUpdateCmd() + require.NoError(t, update.cmd.ParseFlags(args)) + rules, err = buildConnectionRules(&update.connectionRuleFlags) + require.NoError(t, err) + assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "update") + + upsert := newConnectionUpsertCmd() + require.NoError(t, upsert.cmd.ParseFlags(args)) + rules, err = buildConnectionRules(&upsert.connectionCreateCmd.connectionRuleFlags) + require.NoError(t, err) + assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "upsert") +} diff --git a/pkg/gateway/mcp/tool_connections.go b/pkg/gateway/mcp/tool_connections.go index ce18d1b8..c72966d0 100644 --- a/pkg/gateway/mcp/tool_connections.go +++ b/pkg/gateway/mcp/tool_connections.go @@ -71,7 +71,7 @@ var connectionsSpec = mcpcore.ToolSpec{ {On: []string{"list"}, Text: "Filters on %s."}, {On: []string{"create", "upsert", "update"}, Text: "Links the destination on %s."}, }}, - "rules": {Type: "array", Desc: "Ruleset applied to the connection (create/upsert/update). Array of rule objects; replaces the stored ruleset.", Items: &mcpcore.Prop{Type: "object"}, Write: true, Actions: []string{"create", "upsert", "update"}}, + "rules": {Type: "array", Desc: "Ruleset applied to the connection (create/upsert/update). Array of rule objects; replaces the stored ruleset. Order matters: filter, transform and deduplicate rules run in array order, so send the full array in the intended order.", Items: &mcpcore.Prop{Type: "object"}, Write: true, Actions: []string{"create", "upsert", "update"}}, "disabled": {Type: "boolean", Desc: "Filter disabled connections (list)", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}}, "limit": {Type: "integer", Desc: "Max results (list)", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}}, "next": {Type: "string", Desc: "Next page cursor", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}}, From 742abb7150694c6a218c0fef5ddf0eafcd6fde5a Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 1 Oct 2026 15:00:56 +0100 Subject: [PATCH 2/4] test(acceptance): expect multi-rule connections in flag order TestConnectionWithMultipleRules passes filter, retry and delay flags and asserted the old fixed order (filter, delay, retry). Rules now follow flag order, so assert filter, retry, delay. Co-Authored-By: Claude Opus 5.5 --- test/acceptance/connection_test.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/acceptance/connection_test.go b/test/acceptance/connection_test.go index 8f6a4ea4..2e745508 100644 --- a/test/acceptance/connection_test.go +++ b/test/acceptance/connection_test.go @@ -1495,7 +1495,7 @@ func TestConnectionWithDeduplicateRule(t *testing.T) { t.Logf("Successfully created and verified connection with deduplicate rule: %s", conn.ID) } -// TestConnectionWithMultipleRules tests creating a connection with multiple rules and verifies logical ordering +// TestConnectionWithMultipleRules tests creating a connection with multiple rules and verifies they follow flag order func TestConnectionWithMultipleRules(t *testing.T) { if testing.Short() { t.Skip("Skipping acceptance test in short mode") @@ -1508,8 +1508,7 @@ func TestConnectionWithMultipleRules(t *testing.T) { sourceName := "test-src-multi-" + timestamp destName := "test-dst-multi-" + timestamp - // Note: Rules are created in logical order (deduplicate -> transform -> filter -> delay -> retry) - // This order matches the API's default ordering for proper data flow through the pipeline. + // Rules built from --rule-* flags follow the position of each rule type's first flag var conn Connection err := cli.RunJSON(&conn, "gateway", "connection", "create", @@ -1541,23 +1540,23 @@ func TestConnectionWithMultipleRules(t *testing.T) { require.NotEmpty(t, getConn.Rules, "Connection should have rules") require.Len(t, getConn.Rules, 3, "Connection should have exactly three rules") - // Verify logical order: filter -> delay -> retry (deduplicate/transform not present in this test) - assert.Equal(t, "filter", getConn.Rules[0]["type"], "First rule should be filter (logical order)") - assert.Equal(t, "delay", getConn.Rules[1]["type"], "Second rule should be delay (logical order)") - assert.Equal(t, "retry", getConn.Rules[2]["type"], "Third rule should be retry (logical order)") + // Verify flag order: filter -> retry -> delay + assert.Equal(t, "filter", getConn.Rules[0]["type"], "First rule should be filter (flag order)") + assert.Equal(t, "retry", getConn.Rules[1]["type"], "Second rule should be retry (flag order)") + assert.Equal(t, "delay", getConn.Rules[2]["type"], "Third rule should be delay (flag order)") // Verify filter rule details assertFilterRuleFieldMatches(t, getConn.Rules[0]["body"], `{"type":"payment"}`, "body") // Verify delay rule details - assert.Equal(t, float64(1000), getConn.Rules[1]["delay"], "Delay should be 1000 milliseconds") + assert.Equal(t, float64(1000), getConn.Rules[2]["delay"], "Delay should be 1000 milliseconds") // Verify retry rule details - assert.Equal(t, "exponential", getConn.Rules[2]["strategy"], "Retry strategy should be exponential") - assert.Equal(t, float64(5), getConn.Rules[2]["count"], "Retry count should be 5") - assert.Equal(t, float64(60000), getConn.Rules[2]["interval"], "Retry interval should be 60000") + assert.Equal(t, "exponential", getConn.Rules[1]["strategy"], "Retry strategy should be exponential") + assert.Equal(t, float64(5), getConn.Rules[1]["count"], "Retry count should be 5") + assert.Equal(t, float64(60000), getConn.Rules[1]["interval"], "Retry interval should be 60000") - t.Logf("Successfully created and verified connection with multiple rules in logical order: %s", conn.ID) + t.Logf("Successfully created and verified connection with multiple rules in flag order: %s", conn.ID) } // TestConnectionWithRateLimiting tests creating a connection with rate limiting From c1c80642226619136442596d42b6ceeca9984123 Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Thu, 1 Oct 2026 15:08:57 +0100 Subject: [PATCH 3/4] test(connection): cover zero-value rule flags in flag ordering Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/connection_rule_order_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/cmd/connection_rule_order_test.go b/pkg/cmd/connection_rule_order_test.go index ac0d7b30..13a439c4 100644 --- a/pkg/cmd/connection_rule_order_test.go +++ b/pkg/cmd/connection_rule_order_test.go @@ -74,6 +74,16 @@ func TestBuildConnectionRulesFollowsFlagOrder(t *testing.T) { args: []string{"--rule-retry-count", "3", "--rule-filter-body", `{"type":"order"}`, "--rule-retry-strategy", "linear"}, want: []string{"retry", "filter"}, }, + { + name: "zero-value flag records a position but builds no rule", + args: []string{"--rule-delay", "0", "--rule-filter-body", `{"type":"order"}`}, + want: []string{"filter"}, + }, + { + name: "zero-value flag position kept when a later flag sets the rule", + args: []string{"--rule-delay", "0", "--rule-filter-body", `{"type":"order"}`, "--rule-delay", "500"}, + want: []string{"delay", "filter"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From e18b530f173b01e68ec1171804ef3710cdcc47ce Mon Sep 17 00:00:00 2001 From: Phil Leggetter Date: Fri, 2 Oct 2026 09:32:32 +0100 Subject: [PATCH 4/4] test(acceptance): cover filter/transform rule order on create, update and upsert The only acceptance check for flag order used retry and delay, which don't affect execution. Add end-to-end checks for the filter/transform case: - create: filter-then-transform and transform-then-filter are stored as given - update: reorder transform-then-filter to filter-then-transform, then save again, and the order holds - upsert: create with filter-then-transform, upsert again, order holds Transformations get unique names and are deleted after the connection. Co-Authored-By: Claude Opus 5.5 --- test/acceptance/connection_test.go | 71 +++++++++++++++++++++++ test/acceptance/connection_update_test.go | 44 ++++++++++++++ test/acceptance/connection_upsert_test.go | 48 +++++++++++++++ test/acceptance/helpers.go | 17 ++++++ 4 files changed, 180 insertions(+) diff --git a/test/acceptance/connection_test.go b/test/acceptance/connection_test.go index 2e745508..0ee84ef7 100644 --- a/test/acceptance/connection_test.go +++ b/test/acceptance/connection_test.go @@ -1391,6 +1391,77 @@ func TestConnectionWithTransformRule(t *testing.T) { t.Logf("Successfully created and verified connection with transform rule: %s", conn.ID) } +// TestConnectionCreateRuleOrderFollowsFlags verifies that filter and transform rules +// are stored in the order their flags were given. Filter, transform and deduplicate +// rules run in array order, so filter-then-transform must survive the round trip. +func TestConnectionCreateRuleOrderFollowsFlags(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + tests := []struct { + name string + flags func(trnName string) []string + want []string + }{ + { + name: "filter before transform", + flags: func(trnName string) []string { + return []string{ + "--rule-filter-body", `{"type":"payment"}`, + "--rule-transform-name", trnName, + "--rule-transform-code", ruleOrderTransformCode, + } + }, + want: []string{"filter", "transform"}, + }, + { + name: "transform before filter", + flags: func(trnName string) []string { + return []string{ + "--rule-transform-name", trnName, + "--rule-transform-code", ruleOrderTransformCode, + "--rule-filter-body", `{"type":"payment"}`, + } + }, + want: []string{"transform", "filter"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + trnName := "test-trn-order-" + timestamp + + // Registered first so it runs after the connection is deleted + t.Cleanup(func() { + deleteTransformation(t, cli, trnName) + }) + + args := []string{ + "gateway", "connection", "create", + "--name", "test-rule-order-" + timestamp, + "--source-name", "test-src-order-" + timestamp, + "--source-type", "WEBHOOK", + "--destination-name", "test-dst-order-" + timestamp, + "--destination-type", "CLI", + "--destination-cli-path", "/webhooks", + } + var conn Connection + err := cli.RunJSON(&conn, append(args, tt.flags(trnName)...)...) + require.NoError(t, err, "Should create connection with filter and transform rules") + require.NotEmpty(t, conn.ID, "Connection should have an ID") + + t.Cleanup(func() { + deleteConnection(t, cli, conn.ID) + }) + + assert.Equal(t, tt.want, getConnectionRuleTypes(t, cli, conn.ID), "Rules should follow flag order") + }) + } +} + // TestConnectionWithDelayRule tests creating a connection with a delay rule func TestConnectionWithDelayRule(t *testing.T) { if testing.Short() { diff --git a/test/acceptance/connection_update_test.go b/test/acceptance/connection_update_test.go index 671b7b41..8f3d1109 100644 --- a/test/acceptance/connection_update_test.go +++ b/test/acceptance/connection_update_test.go @@ -150,6 +150,50 @@ func TestConnectionUpdateRules(t *testing.T) { t.Logf("Successfully updated connection rules: %s", connID) } +// TestConnectionUpdateRuleOrderFollowsFlags verifies that update stores filter and +// transform rules in flag order, and that saving again does not change that order. +func TestConnectionUpdateRuleOrderFollowsFlags(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + trnName := "test-trn-update-order-" + generateTimestamp() + + // Registered first so it runs after the connection is deleted + t.Cleanup(func() { + deleteTransformation(t, cli, trnName) + }) + + connID := createTestConnection(t, cli) + require.NotEmpty(t, connID, "Connection ID should not be empty") + + t.Cleanup(func() { + deleteConnection(t, cli, connID) + }) + + update := func(flags ...string) { + t.Helper() + var updated Connection + args := append([]string{"gateway", "connection", "update", connID}, flags...) + require.NoError(t, cli.RunJSON(&updated, args...), "Should update connection rules") + } + transformFlags := []string{"--rule-transform-name", trnName, "--rule-transform-code", ruleOrderTransformCode} + filterFlags := []string{"--rule-filter-body", `{"type":"payment"}`} + + // Start with transform before filter + update(append(append([]string{}, transformFlags...), filterFlags...)...) + assert.Equal(t, []string{"transform", "filter"}, getConnectionRuleTypes(t, cli, connID), + "Rules should follow flag order") + + // Reorder to filter before transform, then save the same rules again + for i := 0; i < 2; i++ { + update(append(append([]string{}, filterFlags...), transformFlags...)...) + assert.Equal(t, []string{"filter", "transform"}, getConnectionRuleTypes(t, cli, connID), + "Rules should follow flag order after save %d", i+1) + } +} + // TestConnectionUpdateNotFound tests error handling when updating a non-existent connection func TestConnectionUpdateNotFound(t *testing.T) { if testing.Short() { diff --git a/test/acceptance/connection_upsert_test.go b/test/acceptance/connection_upsert_test.go index 29908e69..c8ed0012 100644 --- a/test/acceptance/connection_upsert_test.go +++ b/test/acceptance/connection_upsert_test.go @@ -743,6 +743,54 @@ func TestConnectionUpsertPartialUpdates(t *testing.T) { }) } +// TestConnectionUpsertRuleOrderFollowsFlags verifies that upsert stores filter and +// transform rules in flag order on create, and keeps that order when upserted again. +func TestConnectionUpsertRuleOrderFollowsFlags(t *testing.T) { + if testing.Short() { + t.Skip("Skipping acceptance test in short mode") + } + + cli := NewCLIRunner(t) + timestamp := generateTimestamp() + connName := "test-upsert-order-" + timestamp + trnName := "test-trn-upsert-order-" + timestamp + + // Registered first so it runs after the connection is deleted + t.Cleanup(func() { + deleteTransformation(t, cli, trnName) + }) + + args := []string{ + "gateway", "connection", "upsert", connName, + "--source-name", "test-upsert-order-src-" + timestamp, + "--source-type", "WEBHOOK", + "--destination-name", "test-upsert-order-dst-" + timestamp, + "--destination-type", "CLI", + "--destination-cli-path", "/webhooks", + "--rule-filter-body", `{"type":"payment"}`, + "--rule-transform-name", trnName, + "--rule-transform-code", ruleOrderTransformCode, + } + + var conn Connection + require.NoError(t, cli.RunJSON(&conn, args...), "Should upsert (create) connection") + require.NotEmpty(t, conn.ID, "Connection should have an ID") + + t.Cleanup(func() { + deleteConnection(t, cli, conn.ID) + }) + + assert.Equal(t, []string{"filter", "transform"}, getConnectionRuleTypes(t, cli, conn.ID), + "Rules should follow flag order on create") + + // Upsert again with the same flags: the order must not change on a second save + var again Connection + require.NoError(t, cli.RunJSON(&again, args...), "Should upsert (update) connection") + assert.Equal(t, conn.ID, again.ID, "Second upsert should update the same connection") + assert.Equal(t, []string{"filter", "transform"}, getConnectionRuleTypes(t, cli, conn.ID), + "Rules should follow flag order after a second upsert") +} + // TestConnectionUpsertRejectsEmptySourceWebhookSecret is the connection-side // regression test for #335. This is the exact command shape that failed in the // hookdeck/evals benchmark: the provider secret lived in a workspace .env that diff --git a/test/acceptance/helpers.go b/test/acceptance/helpers.go index b6ab8047..231507c6 100644 --- a/test/acceptance/helpers.go +++ b/test/acceptance/helpers.go @@ -1773,3 +1773,20 @@ func RequireCLIAuthenticationOnce(t *testing.T) string { return cachedWhoamiOutput } + +// ruleOrderTransformCode is a pass-through transformation for rule order tests. +const ruleOrderTransformCode = `addHandler("transform", (request, context) => request);` + +// getConnectionRuleTypes fetches a connection and returns its rule types in stored order. +func getConnectionRuleTypes(t *testing.T, cli *CLIRunner, id string) []string { + t.Helper() + + var conn Connection + require.NoError(t, cli.RunJSON(&conn, "gateway", "connection", "get", id), "Should get connection %s", id) + types := make([]string, 0, len(conn.Rules)) + for _, rule := range conn.Rules { + ruleType, _ := rule["type"].(string) + types = append(types, ruleType) + } + return types +}