Skip to content

Commit 8a0c111

Browse files
Michael McNeesclaude
authored andcommitted
fix(connector): address PR #145 review feedback
- pagerduty/resolve: make from_email required (PagerDuty rejects PUT /incidents without a From header when using REST API keys); accepted from credential or as an explicit param - twilio/call: error when both url and twiml are supplied (mutually exclusive) - asana/create_task: use toStringSlice for projects so []string and []any are both accepted without a silent nil fallback Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 964e4d0 commit 8a0c111

6 files changed

Lines changed: 94 additions & 7 deletions

File tree

packages/engine/internal/connector/asana.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (c *AsanaCreateTaskConnector) Execute(ctx context.Context, params map[strin
3838
}
3939

4040
workspace, _ := params["workspace"].(string)
41-
projects, _ := params["projects"].([]any)
41+
projects := toStringSlice(params["projects"])
4242
if workspace == "" && len(projects) == 0 {
4343
return nil, fmt.Errorf("asana/create_task: workspace or projects is required")
4444
}

packages/engine/internal/connector/asana_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ func TestAsanaCreateTaskConnector_WithProjects(t *testing.T) {
9595
var body map[string]any
9696
json.NewDecoder(r.Body).Decode(&body)
9797
data := body["data"].(map[string]any)
98+
// JSON round-trip produces []any from []string in the serialized body.
9899
projects, _ := data["projects"].([]any)
99100
if len(projects) != 1 || projects[0] != "PROJ123" {
100101
t.Errorf("unexpected projects: %v", projects)
@@ -117,6 +118,33 @@ func TestAsanaCreateTaskConnector_WithProjects(t *testing.T) {
117118
}
118119
}
119120

121+
func TestAsanaCreateTaskConnector_ProjectsAsStringSlice(t *testing.T) {
122+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
123+
var body map[string]any
124+
json.NewDecoder(r.Body).Decode(&body)
125+
data := body["data"].(map[string]any)
126+
projects, _ := data["projects"].([]any)
127+
if len(projects) != 1 || projects[0] != "PROJ456" {
128+
t.Errorf("unexpected projects: %v", projects)
129+
}
130+
w.WriteHeader(http.StatusCreated)
131+
json.NewEncoder(w).Encode(map[string]any{
132+
"data": map[string]any{"gid": "1", "name": "t", "permalink_url": ""},
133+
})
134+
}))
135+
defer srv.Close()
136+
137+
c := &AsanaCreateTaskConnector{baseURL: srv.URL}
138+
_, err := c.Execute(t.Context(), map[string]any{
139+
"_credential": map[string]string{"token": "asana-token"},
140+
"name": "t",
141+
"projects": []string{"PROJ456"}, // native []string, not []any
142+
})
143+
if err != nil {
144+
t.Fatal(err)
145+
}
146+
}
147+
120148
func TestAsanaCreateTaskConnector_MissingName(t *testing.T) {
121149
c := &AsanaCreateTaskConnector{baseURL: "http://unused"}
122150
_, err := c.Execute(t.Context(), map[string]any{

packages/engine/internal/connector/pagerduty.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,15 @@ func (c *PagerDutyResolveConnector) Execute(ctx context.Context, params map[stri
139139
return nil, fmt.Errorf("pagerduty/resolve: incident_id is required")
140140
}
141141

142+
// from_email is required for REST API key auth: PagerDuty rejects incident
143+
// updates without a From header when using non-user-context credentials.
144+
if fromEmail == "" {
145+
fromEmail, _ = params["from_email"].(string)
146+
}
147+
if fromEmail == "" {
148+
return nil, fmt.Errorf("pagerduty/resolve: from_email is required (set in credential or as param)")
149+
}
150+
142151
body := map[string]any{
143152
"incident": map[string]any{
144153
"type": "incident",
@@ -158,11 +167,7 @@ func (c *PagerDutyResolveConnector) Execute(ctx context.Context, params map[stri
158167
req.Header.Set("Authorization", "Token token="+token)
159168
req.Header.Set("Content-Type", "application/json")
160169
req.Header.Set("Accept", "application/vnd.pagerduty+json;version=2")
161-
if fromEmail != "" {
162-
req.Header.Set("From", fromEmail)
163-
} else if fe, ok := params["from_email"].(string); ok && fe != "" {
164-
req.Header.Set("From", fe)
165-
}
170+
req.Header.Set("From", fromEmail)
166171

167172
resp, err := httpClient(c.Client).Do(req)
168173
if err != nil {

packages/engine/internal/connector/pagerduty_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ func TestPagerDutyResolveConnector_ResolvesIncident(t *testing.T) {
181181
if r.Method != "PUT" || r.URL.Path != "/incidents/Q2AVLPZB5RX" {
182182
t.Errorf("unexpected %s %s", r.Method, r.URL.Path)
183183
}
184+
if r.Header.Get("From") != "oncall@example.com" {
185+
t.Errorf("expected From header, got %s", r.Header.Get("From"))
186+
}
184187
var body map[string]any
185188
json.NewDecoder(r.Body).Decode(&body)
186189
inc := body["incident"].(map[string]any)
@@ -199,7 +202,7 @@ func TestPagerDutyResolveConnector_ResolvesIncident(t *testing.T) {
199202

200203
c := &PagerDutyResolveConnector{baseURL: srv.URL}
201204
out, err := c.Execute(t.Context(), map[string]any{
202-
"_credential": map[string]string{"token": "u+abc"},
205+
"_credential": map[string]string{"token": "u+abc", "from_email": "oncall@example.com"},
203206
"incident_id": "Q2AVLPZB5RX",
204207
})
205208
if err != nil {
@@ -210,6 +213,40 @@ func TestPagerDutyResolveConnector_ResolvesIncident(t *testing.T) {
210213
}
211214
}
212215

216+
func TestPagerDutyResolveConnector_FromEmailInParam(t *testing.T) {
217+
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
218+
if r.Header.Get("From") != "param@example.com" {
219+
t.Errorf("expected From=param@example.com, got %s", r.Header.Get("From"))
220+
}
221+
w.Header().Set("Content-Type", "application/json")
222+
json.NewEncoder(w).Encode(map[string]any{
223+
"incident": map[string]any{"id": "Q1", "status": "resolved"},
224+
})
225+
}))
226+
defer srv.Close()
227+
228+
c := &PagerDutyResolveConnector{baseURL: srv.URL}
229+
_, err := c.Execute(t.Context(), map[string]any{
230+
"_credential": map[string]string{"token": "u+abc"},
231+
"incident_id": "Q1",
232+
"from_email": "param@example.com",
233+
})
234+
if err != nil {
235+
t.Fatal(err)
236+
}
237+
}
238+
239+
func TestPagerDutyResolveConnector_MissingFromEmail(t *testing.T) {
240+
c := &PagerDutyResolveConnector{baseURL: "http://unused"}
241+
_, err := c.Execute(t.Context(), map[string]any{
242+
"_credential": map[string]string{"token": "u+abc"},
243+
"incident_id": "Q1",
244+
})
245+
if err == nil {
246+
t.Fatal("expected error when from_email is missing")
247+
}
248+
}
249+
213250
func TestPagerDutyResolveConnector_MissingIncidentID(t *testing.T) {
214251
c := &PagerDutyResolveConnector{baseURL: "http://unused"}
215252
_, err := c.Execute(t.Context(), map[string]any{

packages/engine/internal/connector/twilio.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ func (c *TwilioCallConnector) Execute(ctx context.Context, params map[string]any
126126
if callURL == "" && twiml == "" {
127127
return nil, fmt.Errorf("twilio/call: url or twiml is required")
128128
}
129+
if callURL != "" && twiml != "" {
130+
return nil, fmt.Errorf("twilio/call: url and twiml are mutually exclusive; provide only one")
131+
}
129132

130133
form := url.Values{}
131134
form.Set("To", to)

packages/engine/internal/connector/twilio_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,20 @@ func TestTwilioCallConnector_MissingURLAndTwiml(t *testing.T) {
207207
}
208208
}
209209

210+
func TestTwilioCallConnector_BothURLAndTwiml(t *testing.T) {
211+
c := &TwilioCallConnector{baseURL: "http://unused"}
212+
_, err := c.Execute(t.Context(), map[string]any{
213+
"_credential": map[string]string{"account_sid": "ACtest123", "auth_token": "secret"},
214+
"to": "+15551234567",
215+
"from": "+15559876543",
216+
"url": "https://example.com/twiml",
217+
"twiml": "<Response><Say>Hi</Say></Response>",
218+
})
219+
if err == nil {
220+
t.Fatal("expected error when both url and twiml are set")
221+
}
222+
}
223+
210224
func TestTwilioCallConnector_MissingTo(t *testing.T) {
211225
c := &TwilioCallConnector{baseURL: "http://unused"}
212226
_, err := c.Execute(t.Context(), map[string]any{

0 commit comments

Comments
 (0)