Skip to content

v0.6.70: legacy workflow sanitization#4491

Merged
icecrasher321 merged 1 commit intomainfrom
staging
May 7, 2026
Merged

v0.6.70: legacy workflow sanitization#4491
icecrasher321 merged 1 commit intomainfrom
staging

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

fix(sanitize): repair malformed subblock states (#4490)

* fix(sanitize): repair malformed subblock states

* address comments

* address comment
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 7, 2026 7:59am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 7, 2026

PR Summary

Medium Risk
Touches workflow migration, import parsing, and persistence writeback logic; mistakes could drop or mis-type subBlock values during load/import, but changes are scoped and covered by new tests.

Overview
Improves legacy workflow compatibility by repairing malformed subBlocks (missing/invalid id/type, type: "unknown", non-object entries, and the literal "undefined" key) while preserving values when the subBlock can be mapped to a known field.

Extends migrateSubblockIds to sanitize subBlocks in addition to renaming legacy IDs (e.g., knowledgeBaseIdknowledgeBaseSelector), and updates import parsing to unwrap API data envelopes, extract workflow names from workflow.name, and run the same subBlock migration/sanitization on imported JSON.

Updates normalized-table migration persistence to track per-block updatedAt and use it in the UPDATE predicate to avoid overwriting newer DB changes when writing back migrated blocks.

Reviewed by Cursor Bugbot for commit 0942555. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR introduces a sanitizeMalformedSubBlocks utility that repairs legacy subBlock state — dropping literal-key "undefined" entries, subblocks with type: "unknown" that have no registry config, and entries missing required id/type fields, while preserving values wherever possible. It also adds API-envelope unwrapping to the import path so workflows exported in the { data: { state: ... } } format round-trip correctly.

  • New subblocks.ts sanitizer — walks every subBlock entry, looks up the block config for the correct type, repairs or drops malformed metadata, and returns a changed flag so the migration pipeline only writes back when something actually changed.
  • Pipeline reordering in utils.ts — subblock-ID migration now runs before credential migration, and a null-guard prevents a crash when a non-object subBlock is encountered in migrateCredentialIds.
  • Optimistic-lock writes in persistMigratedBlocks — the write now includes workflowId scope and an updatedAt equality check, preventing stale migrations from overwriting concurrent edits.

Confidence Score: 4/5

Safe to merge; the sanitization logic is well-tested and the migration pipeline is additive with no destructive data changes.

The core repair logic is covered by targeted tests and the changes are backwards-compatible. The main rough edges are a redundant second sanitization pass in the import path, a missing log for sanitize-only writes, and concurrent migration misses going unrecorded — none of which affect correctness under normal load.

packages/workflow-persistence/src/load.ts (silent concurrent-update miss) and apps/sim/lib/workflows/operations/import-export.ts (double sanitization pass).

Important Files Changed

Filename Overview
apps/sim/lib/workflows/sanitization/subblocks.ts New utility that repairs malformed subBlock entries: drops literal-key "undefined", unknown types without config, and missing id/type fields; preserves values wherever possible; correctly gates the early-return optimisation on the changed flag.
apps/sim/lib/workflows/migrations/subblock-migrations.ts Integrates sanitizeMalformedSubBlocks into the rename pipeline; sanitization-only changes correctly set anyMigrated but no log is emitted for that path, leaving a silent audit gap.
apps/sim/lib/workflows/operations/import-export.ts Adds API-envelope unwrapping and delegates subblock repair to migrateSubblockIds; the second sanitizeMalformedSubBlocks call in normalizeSubblockValues is redundant for structural checks since the first pass already covers them.
packages/workflow-persistence/src/load.ts Adds optimistic-lock (updatedAt) check and workflowId scope to persistMigratedBlocks; concurrent-update misses are silently ignored without a warning log. blockUpdatedAt is correctly threaded through the raw workflow result.
apps/sim/lib/workflows/persistence/utils.ts Migration pipeline step order swapped so subblock-ID migration runs before credential migration, and a null-guard is added for non-object subblocks in migrateCredentialIds. Both changes are safe defensive improvements.
apps/sim/lib/workflows/migrations/subblock-migrations.test.ts New test cases cover malformed subBlock repair (drop unknown types, missing id/type, non-record entries) and legacy rename preservation end-to-end; all assertions look correct against the implementation.
apps/sim/lib/workflows/operations/import-export.test.ts Expands test coverage to include API envelope unwrapping, workflow name extraction, and malformed subBlock repair during import; removes the @vitest-environment node directive (block registry access requires the default environment).
bun.lock Removes the configVersion field and drops the xlsx package hash; cosmetic lockfile changes with no functional impact.

Comments Outside Diff (2)

  1. apps/sim/lib/workflows/operations/import-export.ts, line 477-499 (link)

    P2 Redundant second sanitization pass

    migrateSubblockIds (line 478) already calls sanitizeMalformedSubBlocks internally on every block. The second call to sanitizeMalformedSubBlocks inside the for-each loop re-runs all structural checks that have already been resolved, making the loop redundant for anything other than the convertEmptyStringToNull conversion. If a subblock's empty-string value needs normalizing, passing { convertEmptyStringToNull: true } directly to migrateSubblockIds — or exposing that option at the caller level — would eliminate the extra pass.

  2. packages/workflow-persistence/src/load.ts, line 171-194 (link)

    P2 Silent no-op when the optimistic-lock check fails

    When originalBlockUpdatedAt[blockId] is defined and the row has been concurrently updated, the updatedAt equality check causes the WHERE clause to match 0 rows. Drizzle's .update() succeeds without error, the migration write is silently skipped, and no warning is logged. This means a concurrent modification will cause the migration to re-run on every subsequent workflow load without any observable signal. Adding a logger.warn when the affected row count is 0 would surface these races.

Reviews (1): Last reviewed commit: "fix(sanitize): repair malformed subblock..." | Re-trigger Greptile

Comment thread apps/sim/lib/workflows/migrations/subblock-migrations.ts
@icecrasher321 icecrasher321 merged commit 5ea80a8 into main May 7, 2026
19 of 21 checks passed
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.

1 participant