Skip to content

Conversation

elsmr
Copy link
Member

@elsmr elsmr commented Oct 2, 2025

Summary

  • Enable n8n-nodes-base/node-param-default-wrong-for-string lint rule
  • Run lint:fix on nodes-base and nodes-langchain

Blocked by: ivov/eslint-plugin-n8n-nodes-base#201

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/NODE-3037/add-lint-rule-to-ensure-the-default-option-value-is-one-of-the

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

14 issues found across 191 files

Note: This PR contains a large number of files. cubic only reviews up to 150 files per PR, so some files may not have been reviewed.

Prompt for AI agents (all 14 issues)

Understand the root cause of the following 14 issues and fix them.


<file name="packages/nodes-base/nodes/Freshdesk/ContactDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Freshdesk/ContactDescription.ts:371">
Defaulting the Freshdesk contact state to &#39;blocked&#39; means anyone adding this optional field will create/update contacts as blocked unless they remember to change it. Please default it to the normal active state instead to avoid locking out contacts.</violation>
</file>

<file name="packages/nodes-base/nodes/Bitwarden/descriptions/MemberDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Bitwarden/descriptions/MemberDescription.ts:214">
Setting the update “Type” field default to 0 now auto-selects “Owner”, so adding this optional field silently escalates members to owner unless the user changes it. Please default to the safe, non-privileged value instead (e.g. match the create default of 2).</violation>
</file>

<file name="packages/nodes-base/nodes/Box/FolderDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Box/FolderDescription.ts:291">
Setting the direction field’s default to &#39;ASC&#39; contradicts both the description (“Default value is DESC”) and Box’s API default, so users who add this field without changing the value will get ascending ordering instead of the documented default. Please default to &#39;DESC&#39;.</violation>
</file>

<file name="packages/nodes-base/nodes/Phantombuster/AgentDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Phantombuster/AgentDescription.ts:212">
Changing the default prevStatus to `&#39;finished&#39;` forces every added field to filter for finished runs instead of leaving the status unset; please keep the default blank and satisfy the lint rule by introducing an explicit &quot;Not set&quot; option instead.</violation>
</file>

<file name="packages/nodes-base/nodes/Intercom/LeadDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Intercom/LeadDescription.ts:76">
Switching the default from an unset value to `&#39;id&#39;` changes existing workflows: previously missing `deleteBy` values followed the userId code path, but now they call `/contacts/{value}`. Workflows that stored a `user_id` will suddenly fail to delete the target lead. Please keep the default aligned with the previous behavior by defaulting to `&#39;userId&#39;`.</violation>
</file>

<file name="packages/nodes-base/nodes/Shopify/OrderDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Shopify/OrderDescription.ts:236">
Setting this dropdown’s default to &#39;fulfilled&#39; means that merely adding the Fulfillment Status field now submits orders as fulfilled unless the user manually changes it. Shopify orders should default to the unfulfilled (&#39;null&#39;) state, so please keep the default at that value.</violation>
</file>

<file name="packages/nodes-base/nodes/MessageBird/MessageBird.node.ts">

<violation number="1" location="packages/nodes-base/nodes/MessageBird/MessageBird.node.ts:257">
Changing the Type option default to &#39;binary&#39; means that simply adding this optional field now posts type=&#39;binary&#39;, forcing MessageBird to expect binary payloads instead of the normal SMS default. Please default to &#39;sms&#39; so behaviour matches the API default and the pre-PR behaviour.</violation>
</file>

<file name="packages/nodes-base/nodes/Hubspot/V2/EngagementDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Hubspot/V2/EngagementDescription.ts:157">
Setting the task status default to &quot;COMPLETED&quot; means any task created without manually changing this field is immediately marked done; default to &quot;NOT_STARTED&quot; (HubSpot&#39;s initial state) instead so tasks start open.</violation>
</file>

<file name="packages/nodes-base/nodes/Hubspot/V2/ContactDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Hubspot/V2/ContactDescription.ts:282">
These optional HubSpot contact properties used to default to an empty string so the field stayed unset until the user explicitly picked a value. Changing the default to a real enum (e.g. &#39;SUSPENSION_REMEDIATION&#39;) now preselects data, so simply adding the field without touching the dropdown will send incorrect information. Please keep the default at &#39;&#39; and add an explicit &quot;Not set&quot; option (value &#39;&#39;) to satisfy the lint rule instead of auto-selecting a real value. Apply the same fix to employmentRole, employmentSeniority, employmentSubRole, inferredLanguageCodes, latestTrafficSource, and timeZone.</violation>
</file>

<file name="packages/nodes-base/nodes/BambooHr/v1/actions/employee/create/shareDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/BambooHr/v1/actions/employee/create/shareDescription.ts:105">
Setting this default to &#39;exempt&#39; makes create() always send FLSA status even when the user leaves the field unset, because execute.ts unconditionally assigns the parameter. The empty-string default kept the field optional. The same regression hits the other option defaults changed in this file, so please keep them blank.</violation>
</file>

<file name="packages/nodes-base/nodes/Merge/v2/descriptions.ts">

<violation number="1" location="packages/nodes-base/nodes/Merge/v2/descriptions.ts:19">
Resetting this parameter now chooses the add-suffix behavior even though the node default remains preferInput2, so &quot;reset to default&quot; renames fields instead of preserving the intended preference. Please align the default with the existing node default.</violation>
</file>

<file name="packages/nodes-base/nodes/Mailchimp/Mailchimp.node.ts">

<violation number="1" location="packages/nodes-base/nodes/Mailchimp/Mailchimp.node.ts:344">
Setting the required member status default to &#39;cleaned&#39; now auto-submits new members as cleaned (bounced) accounts; keep the empty default so users must choose an appropriate status.</violation>
</file>

<file name="packages/nodes-base/nodes/HelpScout/CustomerDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/HelpScout/CustomerDescription.ts:118">
Setting the update Gender field’s default to `female` forces an unintended overwrite when the user merely adds the field. Use the neutral `unknown` option instead so adding Gender doesn’t silently flip customers to female.</violation>
</file>

<file name="packages/nodes-base/nodes/Onfleet/descriptions/ContainerDescription.ts">

<violation number="1" location="packages/nodes-base/nodes/Onfleet/descriptions/ContainerDescription.ts:56">
The new default container type is set to &#39;organizations&#39;, but addTask/updateTask still assume &#39;workers&#39; as the fallback. This mismatch causes the node to hit the wrong containers endpoint by default.</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team node/improvement New feature or request labels Oct 2, 2025
Copy link

codecov bot commented Oct 2, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
11556 1 11555 0
View the top 1 failed test(s) by shortest run time
ActiveExecutions shutdown Should cancel all executions when cancelAll is true
Stack Traces | 0.002s run time
Error: expect(received).toHaveLength(expected)

Expected length: 4
Received length: 3
Received array:  [{"id": "1322", "mode": "manual", "retryOf": undefined, "startedAt": 2025-10-02T12:13:19.919Z, "status": "waiting", "workflowId": "123"}, {"id": "1568", "mode": "manual", "retryOf": undefined, "startedAt": 2025-10-02T12:13:19.919Z, "status": "waiting", "workflowId": "123"}, {"id": "1649", "mode": "manual", "retryOf": undefined, "startedAt": 2025-10-02T12:13:19.919Z, "status": "new", "workflowId": "123"}]
    at Object.<anonymous> (.../src/__tests__/active-executions.test.ts:367:51)
    at Promise.then.completed (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/utils.js:300:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/utils.js:233:10)
    at _callCircusTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:315:40)
    at _runTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:251:3)
    at _runTestsForDescribeBlock (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:125:9)
    at _runTestsForDescribeBlock (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:120:9)
    at _runTestsForDescribeBlock (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:120:9)
    at run (.../n8n/node_modules/.pnpm/[email protected]..../jest-circus/build/run.js:70:3)
    at runAndTransformResultsToJestFormat (.../n8n/node_modules/.pnpm/[email protected]..../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../n8n/node_modules/.pnpm/[email protected]..../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/runTest.js:367:16)
    at runTest (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../n8n/node_modules/.pnpm/[email protected]..../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/improvement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant