feat(emailbison): block, tools#4470
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Introduces a new Documentation is extended with a new Reviewed by Cursor Bugbot for commit b3f6a09. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b3f6a09. Configure here.
| }, | ||
| }, | ||
| request: { | ||
| url: (params) => emailBisonUrl(`/api/campaigns/${params.campaignId}/update`), |
There was a problem hiding this comment.
Campaign updates hit wrong endpoint
Medium Severity
The emailbison_update_campaign tool sends campaign update requests to /api/campaigns/{id}/update. The Email Bison API, however, expects these updates at /api/campaigns/{id}, causing all campaign setting updates to fail.
Reviewed by Cursor Bugbot for commit b3f6a09. Configure here.
| }, | ||
| request: { | ||
| url: (params) => emailBisonUrl(`/api/campaigns/${params.campaignId}/${params.action}`), | ||
| method: 'PATCH', |
There was a problem hiding this comment.
Status updates use wrong method
Medium Severity
emailbison_update_campaign_status sends status actions with PATCH, while Email Bison’s campaign action endpoint uses POST. The default pause action fails before users can manage campaign state.
Reviewed by Cursor Bugbot for commit b3f6a09. Configure here.
Greptile SummaryThis PR adds full Email Bison integration to Sim — a block definition, 13 tool implementations (leads, campaigns, replies, tags), icon, docs, and registry entries. The architecture follows established patterns well, with a clean shared utilities layer and proper TypeScript types throughout.
Confidence Score: 3/5The integration is structurally sound but has two runtime issues that affect all 13 tools and the campaign status action endpoint before merging. The HTTP error masking in
|
| Filename | Overview |
|---|---|
| apps/sim/tools/emailbison/utils.ts | Shared utilities for Email Bison tools; HTTP responses are parsed without checking response.ok, causing API errors (401, 404, 500) to surface as misleading generic messages instead of the actual API error. |
| apps/sim/tools/emailbison/update_campaign_status.ts | Pause/resume/archive campaign tool; params.action is interpolated into the URL path without encodeURIComponent, and the declared output shape (full campaign) may not match what action endpoints actually return. |
| apps/sim/blocks/blocks/emailbison.ts | Block definition wiring 13 operations; parameter transformation logic is correct but the name mapping ternary is convoluted. |
| apps/sim/tools/emailbison/types.ts | Type definitions for all Email Bison params and responses; well-structured with nullable fields and proper union types for response variants. |
| apps/sim/tools/emailbison/attach_leads_to_campaign.ts | Attach leads to campaign tool; correctly uses actionOutputs shape and actionOutput mapper, consistent with what the action endpoint likely returns. |
| apps/sim/tools/registry.ts | Registers all 13 Email Bison tools in the global tool registry with correct IDs matching the block's tools.access list. |
Sequence Diagram
sequenceDiagram
participant UI as Block UI
participant Block as EmailBisonBlock
participant Tool as Tool (e.g. list_leads)
participant Utils as utils.ts
participant API as Email Bison API
UI->>Block: User selects operation + fills inputs
Block->>Block: tools.config.params() transforms inputs
Block->>Tool: Dispatches to emailbison_{operation}
Tool->>Utils: emailBisonUrl() builds URL
Tool->>Utils: emailBisonHeaders() adds Bearer token
Tool->>API: fetch(url, { method, headers, body })
API-->>Tool: Response (2xx or 4xx/5xx)
Tool->>Utils: emailBisonData() or emailBisonArrayData()
Note over Utils: response.ok NOT checked
Utils->>Utils: response.json() expects {data: ...}
Utils-->>Tool: mapped data or throws generic error
Tool-->>Block: { success: true, output: ... }
Block-->>UI: Rendered output
Comments Outside Diff (1)
-
apps/sim/blocks/blocks/emailbison.ts, line 1130-1136 (link)namemapping ternary is convolutedThe ternary for
namefalls through toemptyToUndefined(params.campaignName)forupdate_campaign. This is correct, but the logic is hard to follow and error-prone if new operations are added. The nested ternary could be simplified into a plain switch or early-return helper to make the mapping intent explicit.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (1): Last reviewed commit: "typecheck issue" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export async function emailBisonData<T>(response: Response): Promise<T | null> { | ||
| const payload = (await response.json()) as EmailBisonEnvelope<T> | ||
| return payload.data ?? null | ||
| } |
There was a problem hiding this comment.
response.json() is called unconditionally without checking response.ok. When Email Bison returns a 4xx/5xx (e.g., {"error": "Unauthorized"}), payload.data is undefined, so emailBisonData returns null. Callers then throw the generic "Email Bison response did not include a valid X" message, completely hiding the actual API error. Users see no actionable reason for the failure (wrong API key, resource not found, etc.).
| export async function emailBisonData<T>(response: Response): Promise<T | null> { | |
| const payload = (await response.json()) as EmailBisonEnvelope<T> | |
| return payload.data ?? null | |
| } | |
| export async function emailBisonData<T>(response: Response): Promise<T | null> { | |
| const payload = (await response.json()) as EmailBisonEnvelope<T> & { error?: string; message?: string } | |
| if (!response.ok) { | |
| const reason = payload.error ?? payload.message ?? `HTTP ${response.status}` | |
| throw new Error(`Email Bison API error: ${reason}`) | |
| } | |
| return payload.data ?? null | |
| } |
| }, | ||
| }, | ||
| request: { | ||
| url: (params) => emailBisonUrl(`/api/campaigns/${params.campaignId}/${params.action}`), |
There was a problem hiding this comment.
params.action is a raw string interpolated directly into the URL path without encodeURIComponent. The emailBisonUrl helper passes the path string to new URL(path, base), which does NOT percent-encode path segments already in the string. If action is "pause/../../admin", the resulting URL normalizes to the base domain's /admin path. TypeScript's union type and the block dropdown prevent this in normal UI flow, but the tool can also be invoked directly via the tool API without those guards.
| transformResponse: async (response) => { | ||
| const data = await emailBisonRecordData(response, 'campaign') | ||
|
|
||
| return { | ||
| success: true, | ||
| output: mapCampaign(data), | ||
| } | ||
| }, | ||
| outputs: campaignOutputs, |
There was a problem hiding this comment.
The tool declares campaignOutputs as its output schema and calls mapCampaign(data), expecting a full campaign object. However, action endpoints like /pause, /resume, and /archive typically return a lightweight success/status response rather than the complete campaign record. If the API returns {"data": {"success": true, "message": "Campaign paused"}}, mapCampaign will silently produce a campaign object full of null values, discarding the success/message fields. Consider whether actionOutputs / actionOutput (as used in attach_leads_to_campaign.ts) is the correct shape here.
1 participant
Summary
Email Bison tools (https://emailbison.com/)
Type of Change
Testing
WIP
Checklist