fix: mark timeSavedMode as derived to prevent 400 on workflow writes (n8n >= 2.33) - #1017
fix: mark timeSavedMode as derived to prevent 400 on workflow writes (n8n >= 2.33)#1017LouisBurette wants to merge 1 commit into
Conversation
n8n echoes timeSavedMode back on GET but its public API schema (additionalProperties: false) does not accept it on PUT, causing every n8n_update_partial_workflow and n8n_update_full_workflow call to fail with "request/body/settings must NOT have additional properties" on instances running n8n >= 2.33.0. binaryMode and credentialResolverId introduced in the same n8n release already carry derived: true and are correctly stripped. timeSavedMode was missing the flag by oversight. Verified: removing timeSavedMode from the PUT body turns a 400 into a 200 on a live n8n 2.33+ instance. Adding derived: true routes it through stripDerivedSettings(), which runs unconditionally before every write. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for digging into this, root cause matches what I independently found this week on my own instance (n8n on Railway, n8n-mcp 2.73.0). One data point that complicates the fix, though: binaryMode is failing the same way timeSavedMode is, on a version where it's supposedly already flagged derived: true and stripped. Wanted to flag it here in case #1017 alone doesn't fully resolve this class of bug for everyone. What I tested: Real writes against 26 workflows on my instance, ranging 1–265 nodes: When binaryMode was stripped, 26/26 succeeded. Critically, this isn't just a raw-API finding, the identical error reproduced through n8n_update_partial_workflow itself, on a plain addNode operation that never touched settings. So whatever stripDerivedSettings() is supposed to be doing for binaryMode on this version, it isn't happening in practice. Hypothesis: the registry entries are version-gated (since: v(2, 33, 0)), and n8n's public API stopped exposing its own version starting 1.119.0. If n8n-mcp can't reliably detect the connected instance is ≥2.33.0, none of the version-gated derived entries, including the ones that are supposedly already correct, would ever activate. Could explain why binaryMode behaves like it was never flagged, despite the registry saying otherwise. Happy to share full request/response logs or a repro script if that'd help narrow it down. Might be worth this PR (or a follow-up) also confirming binaryMode/credentialResolverId are actually being stripped at runtime on a 2.33+ instance, not just checking the registry definition. |
|
Thanks @LouisBurette and @jusched, the investigation was right about the symptom but the fix needed a different shape. Checked live against n8n 2.36.7: PUT accepts and echoes timeSavedMode (it validates the value against fixed/dynamic), so marking it derived would drop a real setting on current instances; the 400 you saw comes from an n8n whose write schema lagged its entity. #1059 instead retries a rejected write without the settings keys the instance does not accept (unknown keys first, then known ones newest first) and reports what it left out in the response warnings, verified live with a create and a partial update carrying an unknown key. binaryMode is stripped unconditionally on main since v2.70.0; if it still reaches PUT for you on a current release, please open an issue with the request body and n8n version. Closing this one in favour of #1059; you are both credited there. |
…cts as unknown Settings are forwarded on purpose (the table trails n8n's releases), and n8n answers an unknown key with a 400 that names the path but not the key. Instead of failing the whole write, the client now retries without candidates in order: keys absent from the settings table together, then known keys newest first, and reports what it left out through onWarning. A rejected key is remembered for the client's lifetime. This replaces marking timeSavedMode as derived (#1017): n8n 2.36 accepts and echoes it on PUT, so stripping it would drop a real setting on current instances. Also gives the round-trip tests from #925 their own describe block and rewrites the one assertion that encoded the pre-2.70 settings allowlist. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d
…cts as unknown Settings are forwarded on purpose (the table trails n8n's releases), and n8n answers an unknown key with a 400 that names the path but not the key. Instead of failing the whole write, the client now retries without candidates in order: keys absent from the settings table together, then known keys newest first, and reports what it left out through onWarning. A rejected key is remembered for the client's lifetime. This replaces marking timeSavedMode as derived (#1017): n8n 2.36 accepts and echoes it on PUT, so stripping it would drop a real setting on current instances. Also gives the round-trip tests from #925 their own describe block and rewrites the one assertion that encoded the pre-2.70 settings allowlist. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d
…rollback verification) (v2.81.0) (#1059) * fix(partial-workflow): strip unknown node properties echoed by n8n GET n8n's GET /workflows/{id} returns node properties that its PUT/PATCH schema rejects (e.g. server-managed fields like `issues`, `runIndex`, and previously-missing fields like `onError` and `webhookId`). - Add `onError` and `webhookId` to `workflowNodeSchema` - Introduce `cleanNodeForApi()` to filter each node through an allow-list of API-safe properties - Apply filtering in both `cleanWorkflowForCreate` and `cleanWorkflowForUpdate` - Add tests covering the stripping behaviour and the new function Conceived by Romuald Członkowski - www.aiadvisors.pl/en (cherry picked from commit 3638beb) * Verify rollback against the server before reporting it as failed A rollback PUT can persist and then throw. n8n's public API commits workflow content before it checks publish permission, so a caller allowed to edit but not to publish receives an error on a write that landed. The catch block then recorded rollbackPerformed: false and warned that the workflow may be in a broken state, when the content had in fact been restored. That invites a riskier recovery action than doing nothing. After the rollback PUT throws, re-read the workflow and compare the fields an update actually sends, via cleanWorkflowForUpdate. Version identity cannot settle it: a successful rollback writes a new version, so compareVersions() reports 'changed' either way. (cherry picked from commit eb41626) * Use deep equality on the update allowlist and ignore generated webhook ids cleanWorkflowForUpdate() assigns a random webhookId to webhook nodes that lack one, and does so in place. Comparing its output directly mutated both reads and gave each a different id, so any workflow containing a webhook node could never compare equal to itself and the verification silently never succeeded. Clone before cleaning, drop the generated field, and compare with isDeepStrictEqual instead of a hand-written serialiser. Tests now model an actual reverted change rather than workflows that were identical throughout, and cover the unrestored and webhook cases. (cherry picked from commit 8e0b2a9) * Compare webhook ids the workflow already carried Only the ids the update allowlist generates for nodes that lack one are unstable between reads, so drop just those. A webhookId already present is real content, and a change to one means the prior state was not restored. (cherry picked from commit d9ba578) * test: cover GET→UPDATE workflow round-trips and n8n API quirks (#433) Add unit and live-integration coverage for the common spread-from-GET update pattern that let description/read-only fields slip through (#431). - Unit: full GET-shaped payload cleaning, minimal payload, empty/unknown settings - Integration: GET→UPDATE, spread rename, nested settings, description strip, missing settings defaults, read-only field echo, minimal updates, settings filter - Document n8n read/write asymmetry in tests/integration/n8n-api/README.md (cherry picked from commit 46cd1c8) * fix: stale hardcoded version in health check, deprecated API, typos - healthCheck() was returning hardcoded version '2.24.1' instead of the actual package version. imported PROJECT_VERSION and used that instead. - res.finished was deprecated in Node 13, changed to res.writableEnded - fixed "paralel" → "parallel" and "Concieved" → "Conceived" in CLAUDE.md - replaced dead issue link (issues/XXX) with inline description - removed unnecessary (this as any) cast in workflow-validator since currentWorkflow is already properly typed on the class (cherry picked from commit 63f3542) * fix: retry a workflow write without the settings an n8n instance rejects as unknown Settings are forwarded on purpose (the table trails n8n's releases), and n8n answers an unknown key with a 400 that names the path but not the key. Instead of failing the whole write, the client now retries without candidates in order: keys absent from the settings table together, then known keys newest first, and reports what it left out through onWarning. A rejected key is remembered for the client's lifetime. This replaces marking timeSavedMode as derived (#1017): n8n 2.36 accepts and echoes it on PUT, so stripping it would drop a real setting on current instances. Also gives the round-trip tests from #925 their own describe block and rewrites the one assertion that encoded the pre-2.70 settings allowlist. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d * refactor: derive the node allowlist from the schema and tidy the settings ladder Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d * fix: remember only single-key settings rejections; guard node allowlist drift Review round on the write-path branch: - a batch of unknown settings keys dropped together is no longer remembered, since only one of them may be the culprit; single-key steps still are - the rollback comparison ignores generated webhook ids on both reads, so it no longer depends on cleanWorkflowForUpdate mutating the snapshot in place; id-less nodes guarded - customTelemetryTags added to the node schema (n8n 2.36's node schema lists it, so the allowlist would have stripped it), and check:settings-drift now compares the node schema against WRITABLE_NODE_PROPERTIES as well - notes on rejectedSettings lifetime and the retry bound; small doc and test-name fixes Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d * fix: let a settings rejection on the group ladder's retry reach the settings ladder Codex found that the confirmation retry without nodeGroups swallowed a settings-level 400 and rethrew the original groups error, so the settings ladder never ran. Also a health-check version test and a README correction. Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d * test: pin that a batch of unknown settings keys is probed again; attribute readOnly to the right property Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d * chore: release 2.81.0 Conceived by Romuald Członkowski - www.aiadvisors.pl/en Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQmLW2QcubiwyuemrLs62d --------- Co-authored-by: blouf <blouf@blouf.org> Co-authored-by: Ed St John <edward.stjohn@capitalontap.com> Co-authored-by: Pitchfork-and-Torch <297513015+Pitchfork-and-Torch@users.noreply.github.com> Co-authored-by: vitalii.semianchuk <fix20152@gmail.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Problem
On n8n instances >= 2.33.0, every write operation (
n8n_update_partial_workflow,n8n_update_full_workflow) fails with:Root cause
n8n 2.33.0 added
timeSavedModeto theworkflowSettingsDB column and echoes it back onGET /workflows/{id}. However, the public API PUT schema (additionalProperties: false) does not listtimeSavedModeas an accepted property — so it rejects it on write.binaryModeandcredentialResolverId, introduced in the same release, already carryderived: trueinWORKFLOW_SETTINGS_PROPERTIESand are correctly stripped bystripDerivedSettings()before every PUT.timeSavedModewas missing the flag by oversight.Fix
One-word change in
src/constants/workflow-settings.ts:stripDerivedSettings()runs unconditionally before every write, so this is enough — no other code needs to change.Verification
Confirmed on a live n8n 2.33+ instance (Railway-hosted):
timeSavedMode400 request/body/settings must NOT have additional propertiestimeSavedMode200 OKThe live instance's
GET /api/v1/openapi.ymlalso confirmstimeSavedModeis absent fromcomponents.schemas.workflowSettings.Note
timeSavedModeis derived server-side (n8n sets it from internal defaults, not from the PUT body), so stripping it loses nothing — identical to the existing treatment ofbinaryMode.