Skip to content

fix: strip settings.engineType so workflow updates survive n8n >= 2.36.0 (v2.76.1) - #1044

Merged
czlonkowski merged 6 commits into
mainfrom
fix/1043-engine-type-settings
Aug 31, 2026
Merged

fix: strip settings.engineType so workflow updates survive n8n >= 2.36.0 (v2.76.1)#1044
czlonkowski merged 6 commits into
mainfrom
fix/1043-engine-type-settings

Conversation

@czlonkowski

Copy link
Copy Markdown
Owner

Fixes #1043.

Problem

n8n_update_partial_workflow and n8n_update_full_workflow fail against n8n ≥ 2.36.0 with settings must NOT have additional properties. n8n 2.36.0 (n8n-io/n8n#36428) added engineType to the workflow entity's persisted settings without adding it to the Public API write schema (additionalProperties: false). Our writes are read-modify-write, so GET hands us the property and we echo it into a PUT that rejects it. The version-aware filter never fires because modern n8n hides its version from API clients, and the drift guard was blind to the entity-vs-schema axis — engineType was absent from both sides of the schema-only diff. 16 occurrences in 24h on the hosted server, growing as instances update.

Fix

  • src/constants/workflow-settings.ts: engineType is marked derived: true, routing it through stripDerivedSettings — removed from every create and update payload regardless of the (dead) version probe, the same treatment binaryMode and credentialResolverId get. Stripping is lossless: n8n's WorkflowService.update spreads stored settings under the request body, so an omitted key is preserved, not cleared (verified on the n8n@2.36.0 tag and live, below).
  • scripts/check-settings-drift.ts now checks the axis that was blind to this class: it parses IWorkflowSettings from the installed n8n-workflow package and fails when n8n persists a settings property the write schema rejects that our table does not strip — and fails again in the reverse direction, when n8n later publishes a property we strip (so an upstream schema fix cannot leave us stripping it forever). Verified red on the pre-fix tree, green post-fix. The entity parser fails loudly on extends/declaration merging instead of returning a silently incomplete set, and the classification gate is extracted into a pure diffSettingsProperties with direct tests.
  • Tests: engineType strip coverage on the create, update, and version-filter paths — including the null-version path production actually takes — plus an offline CI assertion that every entity settings property is known to the table (the full drift check only runs inside npm run update:n8n; this makes the same class fail in CI on any n8n-workflow bump).

Verification

  • Typecheck, full unit suite, and npm run check:settings-drift green (both default and explicit-version modes).
  • Live E2E against n8n 2.36.7: created a workflow, injected settings.engineType = "v2" server-side (the Public API cannot set it — that is the bug), confirmed GET echoes it, ran n8n_update_partial_workflow through the fixed server — success, where 2.76.0 fails with the 400 — and confirmed engineType: "v2" still persisted after the write.
  • Review gauntlet: code-simplifier (no changes needed), independent code review (no blocking findings; parser fail-loud gaps and CI-visibility hardening adopted), Codex review (no write-leak or data-loss findings; version-skew in explicit-version mode fixed by scoping the entity axis to the pinned set, classification gate now unit-tested).

Follow-up (out of scope)

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

🤖 Generated with Claude Code

…6.0 (v2.76.1)

n8n 2.36.0 added engineType to the workflow entity's persisted settings
without adding it to the Public API write schema, which is
additionalProperties: false. Our updates read-modify-write, so GET handed
us the property and the echoed PUT failed with "settings must NOT have
additional properties" (#1043). engineType is now marked derived and
stripped from every create and update, like binaryMode before it;
n8n keeps stored settings for omitted keys, so the value is preserved.

check-settings-drift gains the axis that was blind to this class: it
diffs IWorkflowSettings from the installed n8n-workflow package against
the OpenAPI schema, fails on an unhandled entity-only property, and
fails again when n8n later publishes a stripped property to the schema.
The entity parser fails loudly on extends/declaration merging, and the
classification gate is a pure function with direct tests plus an offline
CI assertion that every entity property is known to the table.

Verified end-to-end against n8n 2.36.7: a workflow with
settings.engineType = "v2" injected updates successfully through the
fixed server, and the setting survives the write unchanged.

Fixes #1043

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
Copilot AI lite review requested due to automatic review settings August 31, 2026 14:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

diffSettingsProperties() currently treats derived keys as “entity-only expected” when the entity axis is skipped (explicit-version mode) without respecting the since version, which can misclassify future settings for older targets.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes workflow update failures against n8n ≥ 2.36.0 by ensuring settings.engineType (persisted by n8n but rejected by the Public API write schema) is stripped from all workflow create/update payloads, and strengthens the settings drift guard to catch future entity-vs-schema mismatches.

Changes:

  • Mark engineType as a derived/entity-only workflow setting so it’s always stripped on writes.
  • Extend check-settings-drift to also diff the workflow entity settings type (IWorkflowSettings from n8n-workflow) against the Public API schema, with direct unit tests.
  • Bump package version to 2.76.1 and document the fix in CHANGELOG.md.
File summaries
File Description
tests/unit/services/n8n-version.test.ts Adds coverage that engineType is dropped even in pass-through and null-version paths.
tests/unit/services/n8n-validation.test.ts Ensures create/update workflow cleaning strips engineType alongside other derived settings.
tests/unit/scripts/check-settings-drift.test.ts Adds tests for parsing entity settings and classifying drift buckets (including entity-only).
src/constants/workflow-settings.ts Adds engineType as derived + entityOnly, and updates docs for derived/entity-only semantics.
scripts/check-settings-drift.ts Adds entity-axis parsing and drift classification to catch entity-vs-schema mismatches like engineType.
package.json Version bump to 2.76.1.
package-lock.json Lockfile version bump to 2.76.1.
CHANGELOG.md Documents the fix and drift-check enhancement in the 2.76.1 release notes.
Review details
  • Files reviewed: 7/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +257 to +263
const meta = WORKFLOW_SETTINGS_PROPERTIES[name];
if (meta.derived && (entityProperties === null || entityProperties.has(name))) {
entityOnly.push(name);
continue;
}
(compareVersions(meta.since, target) <= 0 ? removed : ahead).push(name);
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Adopted in 4a6c49e: without an entity set, a derived property whose since is later than the target is now classified ahead ("known from a later n8n than the pin") instead of entityOnly. Running the check explicitly against 2.23.0 now lists engineType/binaryMode/credentialResolverId under ahead, and the entity-only line disappears for old targets. Covered by the new test "classifies a derived property from a later n8n as ahead, not entity-only, without an entity set".

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Test Results Summary

📊 Artifacts


Generated at Mon, 31 Aug 2026 15:24:44 GMT
Commit: 0818ce1
Run: #1545

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Codex and Copilot findings on the new drift axis, all adopted:

- Verify the fetched schema's n8n release actually ships the installed
  n8n-workflow (exact pin in its package.json) and warn on a mismatch -
  the nodes-base pin names a sibling release, not the meta-package.
- An entity-only property is handled only when marked BOTH derived and
  entityOnly, and the remediation message says so - derived alone would
  leave the published-upstream detector disarmed.
- The entity parser strips block comments (a declaration-shaped line or
  stray brace inside one no longer derails it) and throws when content
  shares the opening-brace line instead of skipping it silently.
- Without an entity set, a derived property from a later n8n than the
  target is classified ahead-of-the-pin, not "entity-only, expected".
- An unhandled entity-only property is no longer double-reported as
  expected by the info line.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
Copilot AI review requested due to automatic review settings August 31, 2026 14:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Release metadata is inconsistent (the changelog needs the 2.76.0 section restored under its own header, and package.runtime.json still reports 2.76.0 while package.json is 2.76.1).

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

CHANGELOG.md:20

  • The 2.76.1 section currently includes the major “Added/Changed/Fixed/Security” release notes that appear to belong to 2.76.0 (the previous release), because the 2.76.0 header is missing. This makes the changelog history ambiguous/incorrect for both versions.
### Added
  • Files reviewed: 7/8 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…AST (review round 3)

- The schema is now fetched from the n8n release whose published pins
  match the installed packages, found by scanning the newest releases'
  package.json pins (nodes-base@2.36.4 ships in n8n@2.36.7, while
  n8n@2.36.4 pins nodes-base@2.36.3 - the same-number mapping read a
  neighbouring release's schema). Falls back to the old mapping with a
  pin warning when the registry is unreachable or nothing matches.
- The entity parser is the real TypeScript parser instead of a
  hand-rolled lexer: line comments, string literal types, inline braces
  and split declarations no longer silently truncate the property set,
  and any member it cannot enumerate (index signature, method, computed
  name) fails closed.
- Restore the 2.76.0 changelog header that the 2.76.1 entry accidentally
  swallowed, and bump package.runtime.json to 2.76.1 (both Copilot).

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
Copilot AI review requested due to automatic review settings August 31, 2026 14:44
@czlonkowski

Copy link
Copy Markdown
Owner Author

Round 3 pushed (c113cde), addressing Copilot's suppressed changelog/version findings and the remaining Codex review findings:

  • CHANGELOG: the 2.76.1 entry had accidentally swallowed the ## [2.76.0] header — restored, and package.runtime.json bumped to 2.76.1.
  • Release resolution: the drift check now fetches the schema from the n8n release whose published pins match the installed packages (verified live: resolves n8n 2.36.7 for the installed nodes-base@2.36.4/workflow@2.36.3, where the old same-number mapping fetched n8n@2.36.4, a neighbouring release).
  • Entity parser: replaced the hand-rolled lexer with the TypeScript AST — comments, string literal types, inline braces and split declarations can no longer silently truncate the property set, and non-enumerable members fail closed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix is narrowly scoped, well-covered by unit tests (including the production null-version path), and the new drift guard reduces recurrence risk.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/unit/scripts/check-settings-drift.test.ts:258

  • The comment says “everything except derived-only keys”, but the filter is actually excluding meta.entityOnly entries. This is minor, but it can mislead future maintenance of the test setup.
  // The published schema of n8n 2.36 as the table models it: everything except derived-only keys
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

… round 4)

- Reject any syntactic diagnostic before walking the AST: createSourceFile
  recovers from errors, so a truncated declarations file would otherwise
  yield a partial property set that reads as "no entity-only properties".
  The internal parseDiagnostics field disappearing also throws.
- Collect only top-level IWorkflowSettings declarations - a same-named
  interface inside a namespace does not merge with the export.
- The residual-skew warning now covers both pins (the fallback release
  can match n8n-workflow while shipping a different n8n-nodes-base) and
  the pins-unfetchable case.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
Copilot AI review requested due to automatic review settings August 31, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix cleanly routes engineType through the existing derived-settings stripping path and adds targeted drift-guard + tests to prevent recurrence.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

tests/unit/scripts/check-settings-drift.test.ts:278

  • The comment says the derived keys are excluded, but the code actually excludes only entityOnly keys (.filter(([, meta]) => !meta.entityOnly)). This is misleading when reading the test setup.
  const v236 = { major: 2, minor: 36, patch: 4 };
  // The published schema of n8n 2.36 as the table models it: everything except derived-only keys
  const schemaOf236 = new Set(
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…es (review round 5)

The skew warning and release resolution read the installed
n8n-nodes-base version from node_modules instead of the package.json
pin, so a stale install cannot pair this run's entity types with a
schema neither belongs to; an unreadable install is itself reported.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
Copilot AI review requested due to automatic review settings August 31, 2026 15:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

installedEntityPackageVersion() likely resolves the wrong package.json path (returning null in common layouts), weakening the schema-release matching logic in check-settings-drift.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/check-settings-drift.ts:280

  • installedEntityPackageVersion() builds a path by going two levels up from require.resolve('n8n-workflow'), which will point at the wrong package.json when the module entry is dist/index.js (common layout). That causes this function to frequently return null and weakens schema-release resolution. Prefer reading the version directly from n8n-workflow/package.json, consistent with installedNodesBaseVersion().
function installedEntityPackageVersion(): string | null {
  try {
    const pkgPath = join(dirname(require.resolve('n8n-workflow')), '..', '..', 'package.json');
    return (require(pkgPath) as { version?: string }).version ?? null;
  } catch {
    return null;
  }
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…path (Copilot round 3)

Consistent with installedNodesBaseVersion; the resolved-entry path walk
stays as a fallback for a future exports map that blocks the subpath.

Conceived by Romuald Członkowski - www.aiadvisors.pl/en

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JXDW1LGvRXaQydK21X89sj
@czlonkowski

Copy link
Copy Markdown
Owner Author

Copilot's round-3 suppressed comment adopted in the latest push: installedEntityPackageVersion() now reads n8n-workflow/package.json directly, consistent with installedNodesBaseVersion() (the resolved-entry path walk remains as a fallback for a future exports map that blocks the subpath; both forms verified to return 2.36.3 on the current install).

Copilot AI review requested due to automatic review settings August 31, 2026 15:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped, well-covered by targeted unit tests (including the production null-version path), and the new drift guard reduces the chance of regressions from future n8n schema/entity divergence.

Review details
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@czlonkowski
czlonkowski merged commit 2c1ec73 into main Aug 31, 2026
16 checks passed
@czlonkowski
czlonkowski deleted the fix/1043-engine-type-settings branch August 31, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: workflow updates fail with "settings must NOT have additional properties" on n8n >= 2.36.0 (settings.engineType)

2 participants