fix(workspace-forking): stop a remapped custom block losing every input, and let its target inputs be configured at sync time - #6871
Conversation
Repointing a placed custom block at another environment's block left its inputs behind. They are keyed by the SOURCE block's Start field ids, so against the new config they are fields that do not exist, and the serializer drops a stored value with no matching config as a deleted input. The block synced with its name intact and every field blank — and because both environments' blocks share a name, that read as "the sync did nothing and corrupted the block". The type rewrite itself was landing; a test now pins that rather than leaving it to the eye, since a successful rewrite is visually identical. On a type change the inputs are now replaced outright with the ones configured for the TARGET block, and reserved wiring is preserved. There is deliberately no attempt to migrate values across the swap: two custom blocks are independent workflows, so a field id that happened to collide would carry a value meaning something else. When the type does not change — no mapping, or an explicit identity mapping — nothing is touched and values carry as they always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…at sync time Repointing a custom block leaves it with no usable inputs — its sub-blocks are keyed by the SOURCE block's Start field ids, which describe nothing on the new block. Until now the user had to open the synced workflow and re-enter them by hand, with no indication anything was missing. A credential or table swap already makes its `dependsOn` fields reconfigurable in the sync modal. Repointing a custom block is the same idea at its limit: not a subset of fields is invalidated but ALL of them, so all of them are offered. They travel the existing dependent-value channel end to end — collected into the diff, stored per (target workflow, block, sub-block), pre-filled from the store, gating Sync when required and empty, and applied to the written state — so nothing about storage, pre-fill, or the Sync gate is new. `parentKind`/`parentSourceId` are the block itself, which is already the key a reconfig is joined to its mapping row on, so the fields render under their own row with no extra wiring. `selectorKey`/`parentContextKey` become optional: a custom block's inputs are typed values, not selectors, and the modal renders a plain field (a textarea for the JSON-valued types) instead of an option list. Deliberately no seeding from the source value: it belongs to a different block's field of the same position, so pre-filling it would carry a value meaning something else. A block whose type does not change is skipped entirely — its ids still describe it, so its values carry as they always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview On type change,
The fork contract extends Reviewed by Cursor Bugbot for commit ac84737. Configure here. |
Greptile SummaryThe PR fixes custom-block remapping so source-specific inputs are replaced with explicitly configured target inputs while preserving reserved wiring.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/ee/workspace-forking/lib/remap/remap-references.ts | Adds target-type-aware custom-block input keys and replacement logic, including explicit boolean restoration and reserved-field protection. |
| apps/sim/ee/workspace-forking/lib/mapping/custom-block-reconfigs.ts | Resolves mapped custom-block schemas and emits target input fields through the dependent-reconfiguration contract. |
| apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts | Replaces source inputs only when a custom block's type changes and applies values configured for the mapped target. |
| apps/sim/app/api/workspaces/[id]/fork/diff/route.ts | Adds custom-block target inputs to the fork diff and pre-fills them from target-namespaced stored values. |
| apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx | Renders custom-block inputs with type-appropriate controls while sharing effective-value semantics with validation and submission. |
| apps/sim/ee/workspace-forking/components/fork-sync/custom-block-input-control.ts | Centralizes custom-block control selection and boolean true, false, and unset options outside the rendered component. |
| apps/sim/lib/api/contracts/workspace-fork.ts | Extends dependent reconfiguration contracts to represent custom-block typed inputs without selector-specific fields. |
Sequence Diagram
sequenceDiagram
participant Diff as Fork diff API
participant Schema as Target custom-block schema
participant Modal as Sync modal
participant Store as Dependent-value store
participant Copy as Workflow copy
Diff->>Schema: Resolve mapped target inputs
Schema-->>Diff: Field IDs, types, and required flags
Diff->>Store: Load values keyed by workflow, block, and target-type field key
Diff-->>Modal: Return configurable target inputs
Modal->>Store: Persist configured values
Store->>Copy: Supply namespaced dependent overrides
Copy->>Copy: Rewrite custom-block type
Copy->>Copy: Replace old inputs with matching target inputs
Reviews (4): Last reviewed commit: "fix(workspace-forking): let an optional ..." | Re-trigger Greptile
…lock's configured inputs Four review findings, all real, three of them the same root cause: the dependent store holds a plain string keyed only by (target workflow, block, sub-block), and that key carried none of what applying the value correctly needs. The key now carries the TARGET TYPE and the field's declared TYPE. Target type, because remapping a block to A, configuring it, then remapping to B would otherwise pre-fill and submit A's value into any field id the two happened to share — a different workflow's field of the same name. Namespacing makes that structurally impossible instead of a rule to remember. Field type, because the canvas stores a boolean input as a real boolean (its sub-block is a `switch`), so a stored `'false'` written as text is truthy to the child workflow. The apply side reads the type off the key and restores it, and the modal offers a switch rather than a text field. `object`/`array` stay strings: they are authored as JSON and parsed by the executor. Separately, the carve-out that keeps a custom block's stored value alive through its always-true `parentChanged` was applied at the render site, so the modal showed the stored value while the Sync gate and the submitted payload still saw blank — required fields looked filled but kept Sync disabled, and optional ones submitted empty and wiped the stored mapping. It now lives in `effectiveDependentValue`, the one place all three read through. Field-type-to-control selection moves out of the component into its own module, where it sits beside the boolean round-trip constants it has to agree with. Reported by Greptile and Cursor Bugbot on #6871. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cursor review |
Boolean handling collapsed a tri-state. `''` is a flag the user never touched, and it is not `false`. On apply, any non-`'true'` string became `false` — so an untouched optional flag was written as one. `assembleCustomBlockInputMapping` skips `''` but keeps `false`, so that value reached the child's `inputMapping` and overrode whatever default the Start field declares. Only an explicit `'true'`/`'false'` is applied now; anything else leaves the field unset, and the child's own default stands. In the modal the switch mapped `''` to the False segment, so a required flag rendered as configured while the Sync gate still read it as empty — the same display-versus-gate split the previous commit moved into `effectiveDependentValue` to close, reintroduced one layer up. The value is passed through unmapped instead: `''` matches neither segment, so the switch renders with nothing selected, which is what it is. Reported by Greptile and Cursor Bugbot on #6871. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8973f61. Configure here.
…o its default A two-segment switch has no transition back to "nothing selected", so once a user picked True or False there was no way to stop overriding the target workflow's declared default — a single click pinned the flag for every later sync. An optional boolean now carries a third `Default` segment, trailing the two real values because choosing one is the common action and reverting is the escape hatch. A required boolean keeps two: the Sync gate demands a value, so unset is not a state it can end in and offering it would present an unsubmittable choice. Reported by Greptile on #6871. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ac84737. Configure here.
…urable after the mapping is saved (#6877) Mapping a custom block to a different block and syncing showed "no changes required" with no fields to fill, so the inputs #6871 added were unreachable on every sync after the one where the mapping was picked. `parentChanged` comes from `shouldReconfigureEntry`, which asks whether the target was edited IN THIS SESSION. Saving the mapping makes it false, and the reconfigure listing then keeps only fields that are both required and empty — so an optional input disappeared entirely and a filled required one never came back. That test is right for every other kind: an unchanged credential or table mapping leaves its stored dependent picks valid, and a Gmail label picked under the same credential still resolves. A custom block has no such continuity. Its sub-blocks are keyed by the SOURCE block's Start field ids, so under a different target they describe fields that do not exist and nothing carries over — the mapping standing IS the reason to configure, whenever it was made. A custom block mapped to a different block is now always actionable; mapped to itself ("keep the same block across environments") it is not, since its own field ids still describe it. An in-session re-pick still wins over the saved target. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Follow-up to #6857. Mapping a custom block across environments and syncing produced a block that looked corrupted — its name intact, every field blank — and appeared not to have been remapped at all.
The type rewrite was landing. Both environments' blocks share a name, so a successful rewrite is visually identical to nothing happening; a test now pins the type itself rather than leaving it to the eye. The real damage was the inputs: a custom block's sub-blocks are keyed by the source block's Start field ids, and
serializer/index.ts:547-561drops a stored value with no matching config as a deleted input. Repointing the type invalidated every id at once, so every value silently evaporated. (There was already a passing test for that rule — written for a deleted field.)Two commits:
1. Stop the silent loss. On a type change the inputs are replaced outright with the ones configured for the target; reserved wiring is preserved. There is deliberately no value migration across the swap — two custom blocks are independent workflows, so a field id that happened to collide would carry a value meaning something else. When the type does not change (no mapping, or an explicit identity mapping) nothing is touched and values carry as they always did.
2. Let the target's inputs be configured at sync time. A credential or table swap already makes its
dependsOnfields reconfigurable in the sync modal. Repointing a custom block is the same idea at its limit: not a subset of fields is invalidated but all of them, so all of them are offered. They travel the existing dependent-value channel end to end — collected into the diff, stored per(target workflow, block, sub-block), pre-filled from the store, gating Sync when required-and-empty, and applied to the written state — so nothing about storage, pre-fill, or the Sync gate is new.parentKind/parentSourceIdare the block itself, which is already the key a reconfig is joined to its mapping row on, so the fields render under their own row with no extra wiring.selectorKey/parentContextKeybecome optional — a custom block's inputs are typed values, not selectors — and the modal renders a plain field (a textarea for the JSON-valued types) instead of an option list.Type of Change
Testing
turbo run type-check(24/24),check:api-validation, biome — clean on this branch rebased onto latest staging. 3563 tests across the affected surfaces; 12 new.Full suite: the only two failures are the same pre-existing ones established in #6857 —
unreadable-document.test.ts(fails against staging's ownlib/knowledge/documents/) andsso-trust.test.ts(a 10s-timeout test that fails on a cold transform cache regardless of branch).Reviewers should focus on:
replaceCustomBlockInputs(the drop-and-replace decision — no migration is deliberate) andcollectForkCustomBlockReconfigs, particularly that it skips blocks whose type does not change and that an unresolvable target degrades to offering no fields rather than failing the whole diff.One thing worth knowing: routing these through
applyDependentOverridesdoes not work — it allowlists ondependsOn && selectorKey, which no custom-block input has, and callsgetBlock, which doesn't resolve a custom block server-side outside the overlay. Every override would be silently skipped. The planner allowlists instead (it can resolve the target config viaresolveCustomBlockToolBinding) and the copy writes what it is handed.Checklist
🤖 Generated with Claude Code