Skip to content

support the workflow status for cli and mcp and node sdk users #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jessehanley merged 4 commits into from
Apr 2, 2026
Merged

support the workflow status for cli and mcp and node sdk users #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b19f41
Add ESM package type and keep CJS configs
ziptied Feb 6, 2026
cdcc318
feat: add status field to workflow attributes
ziptied Mar 17, 2026
c91f868
Merge pull request #10 from ziptied/BEN-007
ziptied Mar 17, 2026
d5484a0
Merge pull request #8 from ziptied/codex/review-risks-of-adding-type-…
ziptied Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions __tests__/workflows/workflows.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('BentoWorkflows', () => {
type: EntityType.WORKFLOWS,
attributes: {
name: 'Welcome Workflow',
status: 'live',
created_at: '2024-01-01T00:00:00Z',
email_templates: [
{ id: 1, subject: 'Welcome!', stats: { opened: 150, clicked: 75 } },
Expand All @@ -35,6 +36,7 @@ describe('BentoWorkflows', () => {
type: EntityType.WORKFLOWS,
attributes: {
name: 'Abandoned Cart Workflow',
status: 'draft',
created_at: '2024-02-01T00:00:00Z',
email_templates: [{ id: 3, subject: 'You left something behind', stats: null }],
},
Expand All @@ -48,8 +50,10 @@ describe('BentoWorkflows', () => {

expect(result).toHaveLength(2);
expect(result[0].attributes.name).toBe('Welcome Workflow');
expect(result[0].attributes.status).toBe('live');
expect(result[0].attributes.email_templates).toHaveLength(2);
expect(result[1].attributes.name).toBe('Abandoned Cart Workflow');
expect(result[1].attributes.status).toBe('draft');
});

test('uses correct endpoint for GET request', async () => {
Expand Down Expand Up @@ -104,6 +108,7 @@ describe('BentoWorkflows', () => {
type: EntityType.WORKFLOWS,
attributes: {
name: 'Empty Workflow',
status: 'live',
created_at: '2024-01-01T00:00:00Z',
email_templates: [],
},
Expand Down
0 eslint.config.js → eslint.config.cjs
View file
Open in desktop
File renamed without changes.
0 jest.config.js → jest.config.cjs
View file
Open in desktop
File renamed without changes.
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"description": "🍱 Bento Node.JS SDK and tracking library",
"author": "Backpack Internet",
"license": "MIT",
"type": "module",
"main": "dist/index.js",
"module": "dist/index.js",
Comment on lines +7 to 9

@greptile-apps greptile-apps Bot Mar 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 ESM-only package breaks CJS consumers

Adding "type": "module" without a CJS build output or an exports field with a require condition means any consumer using require('@bentonow/bento-node-sdk') will get a runtime error: ERR_REQUIRE_ESM. The PR description mentions improving vanilla JS compatibility, but this change has the opposite effect for CJS environments.

To support both, add an exports field with dual entry points:

"exports": {
  ".": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  }
}

And update the build script to emit both ESM and CJS outputs.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: package.json
Line: 7-9

Comment:
**ESM-only package breaks CJS consumers**

Adding `"type": "module"` without a CJS build output or an `exports` field with a `require` condition means any consumer using `require('@bentonow/bento-node-sdk')` will get a runtime error: `ERR_REQUIRE_ESM`. The PR description mentions improving vanilla JS compatibility, but this change has the opposite effect for CJS environments.

To support both, add an `exports` field with dual entry points:
```json
"exports": {
  ".": {
    "import": "./dist/index.js",
    "require": "./dist/index.cjs"
  }
}
```
And update the build script to emit both ESM and CJS outputs.

How can I resolve this? If you propose a fix, please make it concise.

@ziptied ziptied Mar 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDK is ESM only anyway, this allows others to use it as a module with proper imports, we don't have any CJS in here so its a non issue.

"types": "dist/index.d.ts",
Expand Down
1 change: 1 addition & 0 deletions src/sdk/workflows/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type WorkflowEmailTemplate = {
*/
export type WorkflowAttributes = {
name: string;
status: 'live' | 'draft';

@greptile-apps greptile-apps Bot Mar 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 status as required field is a breaking change

Making status required means any existing cached/persisted API response that doesn't include this field will fail TypeScript type checks. If there's any chance the API returns workflows without status (e.g., older records), this should be optional:

Suggested change
status: 'live' | 'draft';
status?: 'live' | 'draft';
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/sdk/workflows/types.ts
Line: 17

Comment:
**`status` as required field is a breaking change**

Making `status` required means any existing cached/persisted API response that doesn't include this field will fail TypeScript type checks. If there's any chance the API returns workflows without `status` (e.g., older records), this should be optional:

```suggestion
  status?: 'live' | 'draft';
```

How can I resolve this? If you propose a fix, please make it concise.

created_at: string;
email_templates: WorkflowEmailTemplate[];
};
Expand Down
Loading