improvement(canonical): backfill for canonical modes on config changes

[icecrasher321]

Summary

When config changes to mark existing fields as advanced need to backfill automatically to render toggle correctly.

Type of Change

• Bug fix

Testing

Tested manually + added tests for this case

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[cursor]

PR Summary

Medium Risk
Touches workflow load/migration persistence and now writes updated data back to workflowBlocks, so any bug could incorrectly persist canonical mode overrides for existing workflows.

Overview
Adds a new migration backfillCanonicalModes that inspects each block’s registered canonical pairs and backfills missing data.canonicalModes entries by resolving whether the basic or advanced variant is currently populated.

Wires this migration into the workflow block migration pipeline and updates persistence so migrated blocks write both subBlocks and data back to workflowBlocks. Expands Vitest coverage with dedicated backfillCanonicalModes cases and un-mocks the block registry for migration tests.

^{Written by Cursor Bugbot for commit c0d41c3. Configure here.}

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Mar 7, 2026 0:03am

[greptile-apps]

Greptile Summary

This PR adds a backfillCanonicalModes migration that automatically resolves and populates missing data.canonicalModes entries for existing blocks when a block config gains a new canonical pair (basic/advanced toggle). It wires the migration into the existing applyBlockMigrations pipeline and extends the fire-and-forget write-back to persist data changes in addition to subBlocks changes.

Key changes:

• New backfillCanonicalModes function in subblock-migrations.ts — inspects each block against its config, resolves the correct mode using the existing resolveCanonicalMode helper, and backfills only missing entries (never overwrites existing ones)
• utils.ts migration pipeline now includes backfillCanonicalModes as the last step
• Write-back guard broadened from block.subBlocks !== blocksMap[blockId]?.subBlocks to block !== blocksMap[blockId] to catch data-only changes; data is now also included in the UPDATE payload
• 7 new focused tests covering basic/advanced resolution, idempotency, immutability, and defaulting

Issue found:

• The fire-and-forget closure captures finalBlocks by reference, but finalBlocks is mutated after the closure is scheduled (subflow processing at lines ~455–509 replaces loop/parallel block objects with new references). Because microtasks run after the synchronous code, the closure sees the post-mutation state, and all subflow container blocks now pass the block !== blocksMap[blockId] guard — triggering unnecessary DB writes for those blocks whenever any migration fires. With the old subBlocks-based guard this was safe since subflow processing never changes subBlocks.

Confidence Score: 3/5

• Safe to merge with a small risk of write amplification for workflows containing loops or parallels on first load after migration.
• The core migration logic in backfillCanonicalModes is correct and well-tested. The concern is in the fire-and-forget write-back path: the reference-equality guard combined with a stale finalBlocks closure means loop/parallel container blocks receive unnecessary DB updates whenever any migration fires. The data written is not incorrect, but the side effect is broader than intended and diverges from the previous behavior.
• apps/sim/lib/workflows/persistence/utils.ts — specifically the fire-and-forget write-back block at lines 418–437

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[loadWorkflowFromNormalizedTables] --> B[Build blocksMap from DB rows]
    B --> C[applyBlockMigrations pipeline]
    C --> C1[sanitizeAgentToolsInBlocks]
    C1 --> C2[migrateAgentBlocksToMessagesFormat]
    C2 --> C3[migrateCredentialIds]
    C3 --> C4[migrateSubblockIds]
    C4 --> C5[backfillCanonicalModes ✨ NEW]
    C5 --> D{migrated?}
    D -- yes --> E[Schedule fire-and-forget write-back\nclosure captures finalBlocks ref]
    D -- no --> F[Continue]
    E --> F
    F --> G[Process subflows\nmutates finalBlocks for loop/parallel blocks]
    G --> H[Return finalBlocks to caller]
    E -.->|microtask runs after sync code| I[Iterate finalBlocks\nblock !== blocksMap check]
    I --> J{block reference\nchanged?}
    J -- yes --> K[UPDATE workflowBlocks\nsubBlocks + data ✨ NEW]
    J -- no --> L[Skip]
    G -.->|⚠️ loop/parallel blocks now have\nnew references in finalBlocks| I

Loading

Comments Outside Diff (1)

1. apps/sim/lib/workflows/persistence/utils.ts, line 418-437 (link)

   The fire-and-forget closure at line 418 captures finalBlocks by reference. After the microtask is scheduled, the synchronous subflow processing code (lines 455–509) mutates finalBlocks entries for loop/parallel blocks:

   finalBlocks[subflow.id] = {
     ...block,
     data: { ...block.data, collection: ..., whileCondition: ..., doWhileCondition: ... },
   }

   Because microtasks run after the current event loop turn, the closure sees the fully-mutated finalBlocks. Every loop/parallel block now has a new object reference, so block !== blocksMap[blockId] evaluates to true for all of them.

   With the old guard (block.subBlocks !== blocksMap[blockId]?.subBlocks), this was safe — subflow processing never touches subBlocks, so those blocks were never written back. With the new guard, any workflow that contains loops or parallels will trigger an extra DB UPDATE for every subflow container block whenever a migration runs.

   The data written is not incorrect, but the write amplification diverges from the previous behavior. This can be fixed by snapshotting finalBlocks before subflow processing:

   // snapshot BEFORE subflow mutations
   const blocksAfterMigration = { ...finalBlocks }
   
   if (migrated) {
     Promise.resolve().then(async () => {
       for (const [blockId, block] of Object.entries(blocksAfterMigration)) {
         if (block !== blocksMap[blockId]) { /* persist */ }
       }
     })
   }
   
   // subflow mutations continue on finalBlocks...
   subflows.forEach(...)

_{Last reviewed commit: c0d41c3}

[icecrasher321]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

[icecrasher321]

@greptile