Skip to content

Commit fe3a41f

Browse files
authored
fix(workflow-diff): roll back to prior state when n8n PUT fails post-save (czlonkowski#769)
* fix(workflow-diff): roll back to prior state when n8n PUT fails post-save When n8n_update_partial_workflow's underlying PUT fails (e.g. n8n persisted the workflow body but the activation step within the same PUT then tripped on an unsupported typeVersion), the workflow was being left in a broken state. The handler already snapshots the prior workflow into workflowBefore for telemetry, so we piggyback on it: on PUT failure, attempt a rollback PUT to restore the prior state. Three return paths on failure: - Pre-mutation (no snapshot, e.g. validateOnly) — preserve current behaviour - Rollback succeeded — error suffixed with "(workflow restored to prior state)" + details.rollbackPerformed = true - Rollback failed — composite error pointing to n8n_workflow_versions for manual recovery + details.rollbackPerformed = false + rollbackError Success path is byte-identical to before. Tests: - New "should roll back to prior state when n8n PUT fails after persisting body" test asserts the rollback PUT fires with the prior snapshot - New "should report rollback failure when both PUTs fail" test asserts the composite error is surfaced - Existing "should not attempt rollback in validateOnly mode" remains intact Known limitations (documented in PR description): - Concurrent writers can race the rollback. Same race window as today's plain PUT. A future iteration could use versionId for optimistic concurrency. - Rollback fires even when n8n rejected pre-save (body never persisted). Rollback is a no-op in that case — one extra HTTP request on the failure path. Chose this over guessing n8n's error semantics. * fix(workflow-diff): skip rollback on pre-save rejection + actionable recovery Building on the rollback-on-error fix, distinguish two failure modes via a post-failure GET that compares versionId / versionCounter / updatedAt against the snapshot: - Persist-then-fail (n8n saved the body before activation died): roll back by re-PUTting the prior snapshot. Error gets the existing "(workflow restored to prior state)" suffix. - Pre-save rejection (body never persisted): skip the rollback PUT — it would be wasted and the suffix would mislead the caller. details.rollbackPerformed: false; no suffix. When rollback fails, surface workflowBefore.versionId as details.priorVersionId so callers can recover via n8n_workflow_versions. The version comparison is tri-state (same / changed / unknown). On "unknown" — older n8n versions that omit all three fields, or a failing post-failure GET — fall back to attempting rollback. The silent-corruption class from czlonkowski#770 is far worse than a redundant PUT. Tests cover persist-then-fail rollback, pre-save no-rollback, GET-failure best-effort rollback, versionCounter-only fallback, no-version-fields safety net, and double-PUT-failure with priorVersionId. 45/45 in handlers-workflow-diff.test.ts; 611/611 across tests/unit/mcp. Bumped to v2.50.3.
1 parent bcaba83 commit fe3a41f

7 files changed

Lines changed: 426 additions & 10 deletions

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [2.50.3] - 2026-05-04
11+
12+
### Fixed
13+
14+
- `n8n_update_partial_workflow` now rolls back the prior workflow snapshot when n8n persists a body before failing (e.g. unsupported `typeVersion` trips the activation step inside the same PUT), preventing silent corruption of active workflows. Reported and originally fixed by @pybe (#769, closes #770).
15+
- The rollback no longer fires (and no longer claims `(workflow restored to prior state)`) when n8n rejected the PUT before persisting. The handler now compares `versionId` / `versionCounter` / `updatedAt` from a fresh GET to detect whether persistence actually happened.
16+
- Rollback-failure responses include `details.priorVersionId` so callers can recover the right snapshot via `n8n_workflow_versions`.
17+
18+
Conceived by Romuald Członkowski - https://www.aiadvisors.pl/en
19+
1020
## [2.50.2] - 2026-05-04
1121

1222
### Security

dist/mcp/handlers-workflow-diff.d.ts.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/mcp/handlers-workflow-diff.js

Lines changed: 75 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/mcp/handlers-workflow-diff.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "n8n-mcp",
3-
"version": "2.50.2",
3+
"version": "2.50.3",
44
"description": "Integration between n8n workflow automation and Model Context Protocol (MCP)",
55
"main": "dist/index.js",
66
"types": "dist/index.d.ts",

src/mcp/handlers-workflow-diff.ts

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,28 @@ import { EnhancedConfigValidator } from '../services/enhanced-config-validator';
2121
// Cached validator instance to avoid recreating on every mutation
2222
let cachedValidator: WorkflowValidator | null = null;
2323

24+
// Detect whether a fetched workflow has moved past the snapshot we hold.
25+
// Tries versionId first (most reliable), then versionCounter (n8n 1.118.1+),
26+
// then updatedAt. Returns 'unknown' when no comparable field is present on
27+
// both sides; caller falls back to attempting rollback so the safety net
28+
// is preserved on older n8n versions.
29+
type VersionCompare = 'same' | 'changed' | 'unknown';
30+
function compareVersions(
31+
a: { versionId?: string; versionCounter?: number; updatedAt?: string },
32+
b: { versionId?: string; versionCounter?: number; updatedAt?: string },
33+
): VersionCompare {
34+
if (a.versionId !== undefined && b.versionId !== undefined) {
35+
return a.versionId === b.versionId ? 'same' : 'changed';
36+
}
37+
if (a.versionCounter !== undefined && b.versionCounter !== undefined) {
38+
return a.versionCounter === b.versionCounter ? 'same' : 'changed';
39+
}
40+
if (a.updatedAt !== undefined && b.updatedAt !== undefined) {
41+
return a.updatedAt === b.updatedAt ? 'same' : 'changed';
42+
}
43+
return 'unknown';
44+
}
45+
2446
/**
2547
* Get or create cached workflow validator instance
2648
* Reuses the same validator to avoid redundant NodeSimilarityService initialization
@@ -325,7 +347,100 @@ export async function handleUpdatePartialWorkflow(
325347

326348
// Update workflow via API
327349
try {
328-
const updatedWorkflow = await client.updateWorkflow(input.id, diffResult.workflow!);
350+
// Rollback-on-error: if the PUT fails, n8n may have persisted the body
351+
// before failing (e.g. an unsupported typeVersion trips the activation
352+
// step within the same PUT, but the body is already saved). Re-PUT the
353+
// workflowBefore snapshot in that case to restore prior state. The
354+
// snapshot is captured earlier in this handler for telemetry and is
355+
// safe to reuse here.
356+
//
357+
// To distinguish persist-then-fail from pre-save rejection, GET the
358+
// server state after the failed PUT and compare versionId (or
359+
// versionCounter / updatedAt — whichever the running n8n exposes). If
360+
// unchanged, the body never persisted and rolling back would be both
361+
// a wasted PUT and a misleading "(restored to prior state)" message.
362+
let updatedWorkflow;
363+
try {
364+
updatedWorkflow = await client.updateWorkflow(input.id, diffResult.workflow!);
365+
} catch (updateError) {
366+
if (workflowBefore && !input.validateOnly) {
367+
let serverState: any = null;
368+
try {
369+
serverState = await client.getWorkflow(input.id);
370+
} catch (getErr) {
371+
logger.debug('Post-failure GET failed; falling back to best-effort rollback', getErr);
372+
}
373+
// Only skip rollback when we KNOW the body never persisted.
374+
// If serverState is missing or we can't compare versions, attempt
375+
// rollback as a safety net — the bug class in #770 is silent
376+
// corruption, and a redundant PUT is far less harmful than a
377+
// missed rollback.
378+
const versionState = serverState
379+
? compareVersions(serverState, workflowBefore)
380+
: 'unknown';
381+
382+
if (versionState === 'same') {
383+
// Pre-save rejection: nothing to roll back.
384+
logger.debug('PUT failed before persisting; skipping rollback', {
385+
workflowId: input.id,
386+
});
387+
if (updateError instanceof N8nApiError) {
388+
throw new N8nApiError(
389+
updateError.message,
390+
updateError.statusCode,
391+
updateError.code,
392+
{
393+
...((updateError.details as Record<string, unknown>) ?? {}),
394+
rollbackPerformed: false,
395+
},
396+
);
397+
}
398+
throw updateError;
399+
}
400+
401+
// Either persist-then-fail OR couldn't determine — attempt rollback.
402+
let rollbackPerformed = false;
403+
let rollbackErrorMessage: string | undefined;
404+
try {
405+
await client.updateWorkflow(input.id, workflowBefore);
406+
rollbackPerformed = true;
407+
logger.warn('updateWorkflow failed; rolled back to prior state', {
408+
workflowId: input.id,
409+
originalError: updateError instanceof Error ? updateError.message : String(updateError),
410+
});
411+
} catch (rollbackErr) {
412+
rollbackErrorMessage = rollbackErr instanceof Error ? rollbackErr.message : String(rollbackErr);
413+
logger.error('updateWorkflow failed AND rollback failed', {
414+
workflowId: input.id,
415+
originalError: updateError instanceof Error ? updateError.message : String(updateError),
416+
rollbackError: rollbackErrorMessage,
417+
});
418+
}
419+
420+
// Re-throw with rollback context attached so the outer N8nApiError
421+
// catch (below) surfaces it with the user-friendly formatting.
422+
if (updateError instanceof N8nApiError) {
423+
const augmentedDetails: Record<string, unknown> = {
424+
...((updateError.details as Record<string, unknown>) ?? {}),
425+
rollbackPerformed,
426+
...(rollbackErrorMessage ? { rollbackError: rollbackErrorMessage } : {}),
427+
...(workflowBefore.versionId ? { priorVersionId: workflowBefore.versionId } : {}),
428+
};
429+
const suffix = rollbackPerformed
430+
? ' (workflow restored to prior state)'
431+
: (rollbackErrorMessage
432+
? ' (rollback also failed; workflow may be in a broken state — try n8n_workflow_versions for a backup)'
433+
: '');
434+
throw new N8nApiError(
435+
`${updateError.message}${suffix}`,
436+
updateError.statusCode,
437+
updateError.code,
438+
augmentedDetails,
439+
);
440+
}
441+
}
442+
throw updateError;
443+
}
329444

330445
// Handle tag operations via dedicated API (#599)
331446
let tagWarnings: string[] = [];

0 commit comments

Comments
 (0)