Skip to content

Commit 1e0da06

Browse files
leggetterclaude
andcommitted
fix(connection): build rules in flag order
Rules built from --rule-<type>-* 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 <noreply@anthropic.com>
1 parent 98cc142 commit 1e0da06

4 files changed

Lines changed: 204 additions & 10 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,8 @@ $ hookdeck gateway connection create \
12191219
--rule-retry-count 3
12201220
```
12211221

1222+
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`.
1223+
12221224
#### Configure rate limiting
12231225

12241226
Control the rate of event delivery to your destination:

‎pkg/cmd/connection_common.go‎

Lines changed: 77 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/spf13/cobra"
11+
"github.com/spf13/pflag"
1112

1213
"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
1314
)
@@ -64,6 +65,58 @@ type connectionRuleFlags struct {
6465
RuleDeduplicateWindow int
6566
RuleDeduplicateIncludeFields string
6667
RuleDeduplicateExcludeFields string
68+
69+
// ruleOrder records rule types in the order their first --rule-<type>-* flag
70+
// appeared on the command line. Filter, transform and deduplicate run in
71+
// rules array order, so the flags must not impose a fixed order.
72+
ruleOrder []string
73+
}
74+
75+
// defaultRuleOrder is used for rule types with no recorded flag position, such as
76+
// when connectionRuleFlags is populated directly rather than from the command line.
77+
var defaultRuleOrder = []string{"deduplicate", "transform", "filter", "delay", "retry"}
78+
79+
// ruleFlagTypes maps each individual rule flag to the rule type it configures.
80+
var ruleFlagTypes = map[string]string{
81+
"rule-retry-strategy": "retry",
82+
"rule-retry-count": "retry",
83+
"rule-retry-interval": "retry",
84+
"rule-retry-response-status-codes": "retry",
85+
"rule-filter-body": "filter",
86+
"rule-filter-headers": "filter",
87+
"rule-filter-query": "filter",
88+
"rule-filter-path": "filter",
89+
"rule-transform-name": "transform",
90+
"rule-transform-code": "transform",
91+
"rule-transform-env": "transform",
92+
"rule-delay": "delay",
93+
"rule-deduplicate-window": "deduplicate",
94+
"rule-deduplicate-include-fields": "deduplicate",
95+
"rule-deduplicate-exclude-fields": "deduplicate",
96+
}
97+
98+
// recordRuleType appends ruleType to the recorded order unless it is already present.
99+
func (f *connectionRuleFlags) recordRuleType(ruleType string) {
100+
for _, t := range f.ruleOrder {
101+
if t == ruleType {
102+
return
103+
}
104+
}
105+
f.ruleOrder = append(f.ruleOrder, ruleType)
106+
}
107+
108+
// orderTrackingValue wraps a flag value so that setting it records the flag's rule type.
109+
type orderTrackingValue struct {
110+
pflag.Value
111+
onSet func()
112+
}
113+
114+
func (v *orderTrackingValue) Set(s string) error {
115+
if err := v.Value.Set(s); err != nil {
116+
return err
117+
}
118+
v.onSet()
119+
return nil
67120
}
68121

69122
// addConnectionRuleFlags binds rule flags to cmd. Pass a pointer to the flags struct
@@ -91,11 +144,18 @@ func addConnectionRuleFlags(cmd *cobra.Command, f *connectionRuleFlags) {
91144
cmd.Flags().IntVar(&f.RuleDeduplicateWindow, "rule-deduplicate-window", 0, "Time window in seconds for deduplication")
92145
cmd.Flags().StringVar(&f.RuleDeduplicateIncludeFields, "rule-deduplicate-include-fields", "", "Comma-separated list of fields to include for deduplication")
93146
cmd.Flags().StringVar(&f.RuleDeduplicateExcludeFields, "rule-deduplicate-exclude-fields", "", "Comma-separated list of fields to exclude for deduplication")
147+
148+
for name, ruleType := range ruleFlagTypes {
149+
flag := cmd.Flags().Lookup(name)
150+
ruleType := ruleType
151+
flag.Value = &orderTrackingValue{Value: flag.Value, onSet: func() { f.recordRuleType(ruleType) }}
152+
}
94153
}
95154

96155
// buildConnectionRules builds a slice of rules from connectionRuleFlags.
97156
// If rulesStr or rulesFile is non-empty, those are parsed as JSON and returned;
98-
// otherwise individual rule flags are assembled into rules.
157+
// otherwise individual rule flags are assembled into rules, ordered by the
158+
// position of the first flag for each rule type.
99159
// Shared by connection update and (for consistency) can be used by create/upsert.
100160
func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
101161
if f.Rules != "" {
@@ -118,8 +178,8 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
118178
return normalizeRulesForAPI(rules), nil
119179
}
120180

121-
// Build each rule type (order matches create: deduplicate -> transform -> filter -> delay -> retry)
122-
var rules []hookdeck.Rule
181+
// Build each rule type, then order them by flag position
182+
built := make(map[string]hookdeck.Rule)
123183

124184
if f.RuleDeduplicateWindow > 0 {
125185
rule := hookdeck.Rule{
@@ -132,7 +192,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
132192
if f.RuleDeduplicateExcludeFields != "" {
133193
rule["exclude_fields"] = strings.Split(f.RuleDeduplicateExcludeFields, ",")
134194
}
135-
rules = append(rules, rule)
195+
built["deduplicate"] = rule
136196
}
137197

138198
hasTransform := f.RuleTransformName != "" || f.RuleTransformCode != "" || f.RuleTransformEnv != ""
@@ -153,7 +213,7 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
153213
transformConfig["env"] = env
154214
}
155215
rule["transformation"] = transformConfig
156-
rules = append(rules, rule)
216+
built["transform"] = rule
157217
}
158218

159219
if f.RuleFilterBody != "" || f.RuleFilterHeaders != "" || f.RuleFilterQuery != "" || f.RuleFilterPath != "" {
@@ -170,14 +230,14 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
170230
if f.RuleFilterPath != "" {
171231
rule["path"] = parseJSONOrString(f.RuleFilterPath)
172232
}
173-
rules = append(rules, rule)
233+
built["filter"] = rule
174234
}
175235

176236
if f.RuleDelay > 0 {
177-
rules = append(rules, hookdeck.Rule{
237+
built["delay"] = hookdeck.Rule{
178238
"type": "delay",
179239
"delay": f.RuleDelay,
180-
})
240+
}
181241
}
182242

183243
if f.RuleRetryStrategy != "" {
@@ -211,7 +271,15 @@ func buildConnectionRules(f *connectionRuleFlags) ([]hookdeck.Rule, error) {
211271
}
212272
rule["response_status_codes"] = strCodes
213273
}
214-
rules = append(rules, rule)
274+
built["retry"] = rule
275+
}
276+
277+
var rules []hookdeck.Rule
278+
for _, ruleType := range append(append([]string{}, f.ruleOrder...), defaultRuleOrder...) {
279+
if rule, ok := built[ruleType]; ok {
280+
rules = append(rules, rule)
281+
delete(built, ruleType)
282+
}
215283
}
216284

217285
return rules, nil
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package cmd
2+
3+
import (
4+
"testing"
5+
6+
"github.com/spf13/cobra"
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/hookdeck/hookdeck-cli/pkg/hookdeck"
11+
)
12+
13+
// parseRuleFlags binds the rule flags to a throwaway command and parses args,
14+
// so flag values are set the same way cobra sets them at runtime.
15+
func parseRuleFlags(t *testing.T, args []string) *connectionRuleFlags {
16+
t.Helper()
17+
f := &connectionRuleFlags{}
18+
cmd := &cobra.Command{Use: "test"}
19+
addConnectionRuleFlags(cmd, f)
20+
require.NoError(t, cmd.ParseFlags(args))
21+
return f
22+
}
23+
24+
func ruleTypes(rules []hookdeck.Rule) []string {
25+
types := make([]string, 0, len(rules))
26+
for _, r := range rules {
27+
types = append(types, r["type"].(string))
28+
}
29+
return types
30+
}
31+
32+
// TestBuildConnectionRulesFollowsFlagOrder verifies that rules built from
33+
// --rule-<type>-* flags follow the position of the first flag for each type.
34+
// Filter, transform and deduplicate run in rules array order, so a fixed
35+
// order made filter-before-transform impossible to express with flags.
36+
func TestBuildConnectionRulesFollowsFlagOrder(t *testing.T) {
37+
tests := []struct {
38+
name string
39+
args []string
40+
want []string
41+
}{
42+
{
43+
name: "filter before transform",
44+
args: []string{"--rule-filter-body", `{"type":"order"}`, "--rule-transform-name", "tx1"},
45+
want: []string{"filter", "transform"},
46+
},
47+
{
48+
name: "transform before filter",
49+
args: []string{"--rule-transform-name", "tx1", "--rule-filter-body", `{"type":"order"}`},
50+
want: []string{"transform", "filter"},
51+
},
52+
{
53+
name: "first flag of a type sets its position",
54+
args: []string{
55+
"--rule-filter-body", `{"type":"order"}`,
56+
"--rule-transform-name", "tx1",
57+
"--rule-filter-headers", `{"x-topic":"orders/create"}`,
58+
},
59+
want: []string{"filter", "transform"},
60+
},
61+
{
62+
name: "all five types in flag order",
63+
args: []string{
64+
"--rule-retry-strategy", "exponential",
65+
"--rule-filter-body", `{"type":"order"}`,
66+
"--rule-delay", "1000",
67+
"--rule-transform-name", "tx1",
68+
"--rule-deduplicate-window", "60",
69+
},
70+
want: []string{"retry", "filter", "delay", "transform", "deduplicate"},
71+
},
72+
{
73+
name: "retry position set by a non-strategy retry flag",
74+
args: []string{"--rule-retry-count", "3", "--rule-filter-body", `{"type":"order"}`, "--rule-retry-strategy", "linear"},
75+
want: []string{"retry", "filter"},
76+
},
77+
}
78+
for _, tt := range tests {
79+
t.Run(tt.name, func(t *testing.T) {
80+
rules, err := buildConnectionRules(parseRuleFlags(t, tt.args))
81+
require.NoError(t, err)
82+
assert.Equal(t, tt.want, ruleTypes(rules))
83+
})
84+
}
85+
}
86+
87+
// TestBuildConnectionRulesDefaultOrderWithoutFlags verifies the fallback order
88+
// when connectionRuleFlags is populated directly rather than parsed from flags.
89+
func TestBuildConnectionRulesDefaultOrderWithoutFlags(t *testing.T) {
90+
flags := connectionRuleFlags{
91+
RuleRetryStrategy: "linear",
92+
RuleFilterBody: `{"type":"order"}`,
93+
RuleTransformName: "tx1",
94+
RuleDelay: 1000,
95+
RuleDeduplicateWindow: 60,
96+
}
97+
rules, err := buildConnectionRules(&flags)
98+
require.NoError(t, err)
99+
assert.Equal(t, []string{"deduplicate", "transform", "filter", "delay", "retry"}, ruleTypes(rules))
100+
}
101+
102+
// TestConnectionCommandsRuleFlagOrder verifies that create, update and upsert
103+
// all record rule flag order through their own flag sets.
104+
func TestConnectionCommandsRuleFlagOrder(t *testing.T) {
105+
args := []string{"--rule-filter-body", `{"type":"order"}`, "--rule-transform-name", "tx1"}
106+
107+
create := newConnectionCreateCmd()
108+
require.NoError(t, create.cmd.ParseFlags(args))
109+
rules, err := buildConnectionRules(&create.connectionRuleFlags)
110+
require.NoError(t, err)
111+
assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "create")
112+
113+
update := newConnectionUpdateCmd()
114+
require.NoError(t, update.cmd.ParseFlags(args))
115+
rules, err = buildConnectionRules(&update.connectionRuleFlags)
116+
require.NoError(t, err)
117+
assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "update")
118+
119+
upsert := newConnectionUpsertCmd()
120+
require.NoError(t, upsert.cmd.ParseFlags(args))
121+
rules, err = buildConnectionRules(&upsert.connectionCreateCmd.connectionRuleFlags)
122+
require.NoError(t, err)
123+
assert.Equal(t, []string{"filter", "transform"}, ruleTypes(rules), "upsert")
124+
}

‎pkg/gateway/mcp/tool_connections.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ var connectionsSpec = mcpcore.ToolSpec{
7171
{On: []string{"list"}, Text: "Filters on %s."},
7272
{On: []string{"create", "upsert", "update"}, Text: "Links the destination on %s."},
7373
}},
74-
"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"}},
74+
"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"}},
7575
"disabled": {Type: "boolean", Desc: "Filter disabled connections (list)", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}},
7676
"limit": {Type: "integer", Desc: "Max results (list)", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}},
7777
"next": {Type: "string", Desc: "Next page cursor", Only: []string{mcpcore.GroupRead}, Actions: []string{"list"}},

0 commit comments

Comments
 (0)