refactor(sub-blocks): make a registered selector the single source for a remote option list, and close the fork-sync reconfiguration gap - #6878
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Fork sync can reconfigure plain text dependents ( Docs and checks document selector vs Reviewed by Cursor Bugbot for commit cc8e7e2. Configure here. |
Greptile SummaryThe PR consolidates remote sub-block option loading behind registered selectors and expands fork-sync reconfiguration coverage.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-fetched-options.ts | Routes dynamic option loading and label hydration through registered selectors while retaining lifecycle guards. |
| apps/sim/lib/workflows/subblocks/context.ts | Builds explicit selector contexts, including canonical and type-based credential resolution. |
| apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts | Expands collection of fork-dependent fields that can be configured before synchronization. |
| apps/sim/ee/workspace-forking/lib/remap/remap-references.ts | Preserves target custom-block values when the existing target already has the mapped type. |
| scripts/check-fork-dependent-coverage.ts | Audits dependent sub-block definitions for fork-sync reconfigurability. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Config[Sub-block configuration] -->|selectorKey| Registry[Selector registry]
Registry --> Context[Explicit selector context]
Context --> Canvas[Canvas editor]
Context --> Fork[Fork-sync modal]
Fork --> Values[Dependent reconfiguration values]
Values --> Apply[Target workflow sync]
Reviews (2): Last reviewed commit: "feat(workspace-forking): make every fork..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
2405fc8 to
be7cb39
Compare
icecrasher321
changed the title
icecrasher321
changed the title
|
@greptileai @cursor review Both findings from the last review are fixed — replies inline on each thread. The PR has also grown well past its original scope since that review, so this is effectively a fresh diff:
New since then (6 commits): Three things I'd particularly like a second opinion on:
|
|
bugbot run |
|
Both Bugbot findings on
Added That third assertion immediately caught a fourth bug neither review flagged:
@cursor review — the last two were both mine and both real, so another pass on this commit is worth it. Same three areas I called out before still stand (trigger canonical grouping, the credential-by-type fallback, |
|
bugbot run |
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 ae690cb. Configure here.
|
Ran one more adversarial pass over the migration, targeting the failure class the last two findings shared: something a Found — lost label resolver. Found — the new test was too narrow. Its Checked and deliberately not changed:
Where I'd land on merging: the code is in good shape, but five real bugs in this migration passed a green
A five-minute pass opening a Gmail trigger, a HubSpot trigger, a Function block's sandbox picker and the Managed Agent pickers would cover most of the surface area. |
…, and derive their controls from the canvas Two problems with how a repointed custom block's inputs render in the sync modal. The field title printed twice. The row wrapper already draws the label and its required marker for every dependent field — `DependentFieldSelector` takes a `title` only to phrase its placeholder and renders a bare combobox. The custom-block branch used `ChipModalField`, which owns a label of its own, so every input showed its name twice. It now renders bare controls like its sibling does. The control was chosen by re-reading the raw field type instead of asking the function that already answers this. `subBlockTypeForField` decides what a Start field becomes on the canvas; the modal had a parallel switch that had already drifted, rendering a `file[]` input — an upload on the canvas — as a plain text box, which would write a bare string into a field expecting file references. `subBlockTypeForField` is now exported and the modal derives from it, so the two cannot disagree about what a field IS; the modal only decides how that kind draws. A file input is explicitly `unsupported` rather than falling through: it renders disabled, saying it is set in the workflow, instead of inviting a value that cannot work. A test walks every type a Start field can declare and asserts the modal's choice follows the canvas's, so a type added later surfaces here rather than silently becoming a text box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…arget environment, and stop a re-sync wiping its uploads
A repointed custom block's inputs are configured at sync time, but the modal
drew them as bare text fields against no environment at all:
- `{{SECRET}}` had no completion, and no way to know which secrets exist in the
workspace the value is written INTO.
- `<block.output>` had no completion. The canvas dropdown reads the workflow
open in the editor; on the fork settings page there is none, and the workflow
that matters is the target's.
- A `file[]` input has no control here (it is an upload on the canvas), so it
had no stored override — and the block was rebuilt from overrides alone, so
every sync silently dropped the target's uploaded files.
`WorkflowReferenceScope` lets a surface supply the workflow a reference resolves
against. Absent a provider, the hooks read the live editor stores exactly as
before, so the canvas is unchanged. The scope splits graph from values on
purpose: reachability cannot change with the text being typed, and the
validation hook runs in every reference-aware sub-block editor at once, so
subscribing it to live sub-block values would re-render all of them on every
keystroke. A test pins that split.
`replaceCustomBlockInputs` now seeds from the target block when it is ALREADY
the mapped type, layering the configured values on top. That keeps an input the
modal cannot offer a control for, and leaves a field the user simply did not
touch alone; a field they explicitly emptied stores `''`, which is an override
and still wins. Under a DIFFERENT current type nothing is carried over — those
values are keyed by another block's field ids, which is the orphaning this
function exists to prevent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both PR bots flagged this and they were right. A repointed custom block's `file[]` input renders as a disabled control — it is an upload on the canvas, and there is nothing to type here — but the Sync gate still demanded a non-empty value for every REQUIRED dependent. So a custom block with a required file input turned Sync off permanently, while the field's own hint told the user to go set it in a workflow they could only reach BY syncing. `isForkSyncConfigurableField` is the one predicate for "can the modal put a value in this field", used by the gate and by the per-kind status badge so the two cannot disagree. Skipping the gate is only safe because the sync no longer clears the field: the target keeps what it has, and a genuinely missing value is still caught by the block's own required-field validation at run/deploy time — the same fallback every other unconfigured required field already relies on. Also gives the disabled control an `aria-label` (the row's visible label is a sibling, not associated), closing the second review note. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r a remote option list `dropdown` and `combobox` could only load a remote list through a per-block `fetchOptions(blockId)`, which resolves its credential by reading the live workflow store. That works on the canvas and nowhere else — which is why the fork sync modal cannot offer those fields, and why every one of those fetchers turned out to be a hand-rolled duplicate of a selector that already exists (`triggers/gmail/poller.ts` calls the very contract `gmail.labels` wraps). Both controls now accept `selectorKey`, resolved through the registry inside `useFetchedOptions`. Deliberately NOT a second code path: the registry is presented through the same two function shapes the props already describe, so the existing lifecycle — request-id guards, dependency-scope reset, label hydration — is reused verbatim, and paginated selectors drain through the same `loadAllSelectorOptions` that search/replace and value resolution already use. `isDynamic` replaces the `fetchOptions &&` test the controls used to decide whether the fetched list or the static `options` array is authoritative; that question outlives the prop it was asking about. No block or trigger changes yet, so nothing moves off `fetchOptions` in this commit: subblock `type`, `multiSelect`, and the stored value shape are all untouched and no existing workflow is affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…egistered selector Each of these `fetchOptions` resolved its credential with `readSubBlockValue(blockId, 'triggerCredentials')` — a live-workflow-store read — and then called the very selector contract a registered selector already wraps. They were duplicates that only worked on the canvas. Migrated: webflow sites/collections (x4 triggers), clickup workspaces, gmail labels, outlook folders, and all six hubspot pickers. 425 lines of duplicated fetch logic deleted. The missing piece each one needed was `canonicalParamId: 'oauthCredential'` on its credential subblock: `buildSelectorContextFromBlock` keys the context on a subblock's CANONICAL id, so without it `context.oauthCredential` was never populated and the block had no way to reach its credential except the store — which is what forced the hand-rolled fetcher in the first place. Five new hubspot selectors. `hubspot.pipelineStages` reads the pipelines contract and narrows, because HubSpot returns stages inside the pipeline payload rather than behind an endpoint of their own; sharing the one response is also what keeps a stage list from ever describing a pipeline its sibling picker is not showing. `objectType`/`customObjectTypeId`/`pipelineId` join SelectorContext, and `resolveObjectType` keeps HubSpot's own `contact` default so an untouched dropdown still lists properties for what it visibly shows. Subblock `type`, `multiSelect`, and stored value shapes are unchanged, so existing workflows are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….columns `fetchTableColumns` resolved the workspace from the active-workflow store and the table id by reading two subblocks by name, then refetched the table list to find one table's schema. The registered `table.columns` selector takes both from the context — `tableSelector`/`manualTableId` already carry `canonicalParamId: 'tableId'`, so the canonical pair resolves on its own — and reads the table detail query directly. Deletes the helper and the four imports it was the only user of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four read one route distinguished only by `resource`, with the credential pulled from the store by name. They are now `managedAgent.agents` / `.vaults` / `.memoryStores` / `.environments`, and `lib/managed-agents/subblock-options.ts` is deleted entirely. The environment filter (cloud vs self_hosted expose different fields, so mixing them offers choices the rest of the form cannot honour) moves into the selector with `environmentType` on the context. Also decouples two things `canonicalParamId` was conflating. It is both a block's serialized PARAM NAME and the key `buildSelectorContextFromBlock` reads, so making this block's pickers resolvable appeared to require renaming its shipped `credential` param to `oauthCredential` — a rename that would change the serialized shape of every existing managed_agent block, and one that `blocks.test.ts` correctly refused. A picker should not be able to force a param rename, so the context now reads a credential off the subblock TYPE when no canonical id supplied one. It only fills a gap: a block that declares `canonicalParamId: 'oauthCredential'` has already resolved it, including the basic/advanced active-member logic the type check cannot express. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… a selector or derived, never both
Completes the migration. `fetchOptions`/`fetchOptionById` are off `SubBlockConfig`,
off both controls, and out of `useFetchedOptions`, leaving exactly two ways a
sub-block gets its options:
selectorKey — a registered selector. The ONLY way to load a remote list.
Parameterized by an explicit SelectorContext, so it works on the
canvas, in the fork sync modal, and anywhere else.
options — a static array, or a pure function of the block's own values.
No I/O.
Reading the remaining callsites showed most of the "derived" ones were nothing of
the kind — they were workspace-scoped remote fetches wearing a local-looking
signature. Those became seven `workspace.*` selectors (credential providers,
credential groups + their per-group providers, secret names, raw secret names,
sandboxes, trigger types) plus `providers.openrouterEmbeddingModels`. Only the
agent block's three capability dropdowns were genuinely derived; `options` now
takes the block's values so they can say so directly. The parameter is optional,
so every existing zero-argument options function is untouched.
`imap.mailboxes` is the one selector whose account is typed rather than stored.
Its password is deliberately absent from the query key: a query key identifies a
resource, a credential authorizes access to it. `oauthCredential` is safe there
because it is only an id — a typed password is a secret, and keys are cached and
surfaced by devtools. Host, port, TLS and username already identify the mailbox
list uniquely; the password rides the body exactly as before.
`selectorExcludeSelf` replaces the one thing a shared `sim.workflows` selector
could not express. It is a declared flag rather than a blanket rule because the
answer differs per field: the Sim trigger never receives events about its own
workflow, while the Logs block legitimately reads the logs of the workflow it
runs in.
Deletes `lib/workflows/subblocks/options.ts` and `triggers/editor-state.ts`
entirely — every caller was a `fetchOptions` resolver. The live-registry test for
the trigger vocabulary moves to the selector that now owns it, keeping its
lazy-import cycle guarantee under test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urable at sync time, and lint that it stays so `clearDependentsOnRemap` wipes every transitive dependent of a remapped parent, and a credential mapped between environments changes value on EVERY sync — so a dependent the sync modal could not offer was re-emptied on every push, with nowhere to set it that stuck. Setting it in the target did not survive. 36 fields were in that state. The selector migration closed most of it; this closes the rest. The collector now also emits plain text dependents (`short-input` / `long-input`), which need no selector — just somewhere to type — and the modal's no-selector branch renders them through the same control it already drew for custom-block inputs. It deliberately does NOT emit the manual half of a selector-backed canonical pair: that pair already represents the field once, and its manual member is verbatim by policy, so offering both would show one concept twice and invite writing into the inactive half. `forkDependentControl` replaces the direct `customBlockInputControl` call in the view, because `fieldType` now means two different things: a custom-block input declares a Start FIELD type (`string`, `file[]`), while every other no-selector dependent is a canvas SUB-BLOCK whose own type says it. They agreed by accident before; now they are classified separately. `check:fork-dependent-coverage` fails when a sub-block under a credential/knowledge-base/table anchor is none of: selector-backed, a canonical pair member, a preserved name-based type, or text. 656 dependents, zero uncovered, no baseline — verified to fail by seeding a regression. Picked up automatically by `check:audits` (all 30 green). Documented in `/add-block`, `/add-trigger`, and `.claude/rules/sim-integrations.md`, including the two rules the checks enforce: a secret never enters a selector's query key, and a fork-clearable dependent must be reconfigurable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ons, and pass derived values to ComboBox
Both found by Bugbot on the migration commit; both real, both mine.
A selector-backed field carries no static `options` — that is the point — but
`Dropdown` and `ComboBox` still read it on first paint, before any fetch resolves,
and `allOptions.map(...)` is unconditional. Every field moved to `selectorKey`
(Function sandboxes, Managed Agent pickers, OpenRouter embeddings, Logs
workflows, the migrated triggers) would throw on mount. The type said the prop was
required, so nothing caught it: the callsites pass `config.options`, which is
optional on `SubBlockConfig` and now genuinely absent.
Fixed on the controls rather than by restoring `options: []` to every migrated
sub-block: the absence is correct, so the component owns the default. `options`
is optional on both prop types and falls back to a shared empty array, which also
keeps a stable identity for the memo.
`ComboBox` never got the `options({ values })` wiring `Dropdown` received, so
agent's reasoning-effort, verbosity and thinking-level lists — all comboboxes —
silently stayed on their generic fallback instead of narrowing to the selected
model. Wired the same way, reading the block's own values from the store.
`selector-backed-subblocks.test.ts` pins the invariants against the real registry:
a named selector exists and can list, a selector-backed field never also declares
static options, and a field whose selector is gated on context declares the
`dependsOn` that rebuilds it. That last one immediately caught a third bug —
`clickup.triggerWorkspaceId` had no `dependsOn`, so its list would have loaded
once, empty, and never refetched once a credential was picked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… and probe getQueryKey for missing dependsOn Two findings from a final adversarial pass over the migration, both the same class as the three the review bots caught: something a `fetchOptions` sub-block declared that its replacement selector quietly does not. `credential-group.providerFilter` had a `fetchOptionById`; `workspace.credentialGroupProviders` had no `fetchById`, so the canvas card summarising several stored provider ids lost every label. The field is multi-select, which is exactly when a label has to resolve without the full list. The `dependsOn` assertion in `selector-backed-subblocks.test.ts` only probed `enabled` against three hand-listed context fields, which is why it caught `clickup.triggerWorkspaceId` and would have missed the rest. It now probes `getQueryKey` as well — a selector's key names every context field its RESULT depends on — and derives the sub-block-sourced set from `SELECTOR_CONTEXT_FIELDS` rather than a literal. Verified by deleting a real `dependsOn`: it fails naming the field and the fields it depends on. Also checked and NOT changed: `display.ts` and the copilot dropdown validator both guard `options` before use, so stripping `options: []` does not reach them. The validator's behaviour does shift from "reject every value" (an empty `validIds` array matched nothing) to "skip validation", which is a relaxation rather than a regression. `function.sandboxId` kept its `dependsOn: ['language']`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
54aad00 to
924cbab
Compare
…med its allowance CI's "Repo audits" step failed on `check:tool-registry-boundary`. Measured before touching anything, because the reported growth (+32 and +42 modules on two routes) looked like this branch had dragged the selector registry somewhere new. It had not. Recording a baseline on clean `origin/staging` and diffing against this branch attributes the growth precisely: this branch: +1 to +4 modules per route, +35 total across 25 routes staging: the rest Staging's six merged commits landed both failing routes at exactly their tolerance — knowledge/[id] at +31 of an allowed +31, layout at +41 of +41 — so `check:tool-registry-boundary` passed there with nothing left over. This branch's +1 tipped both past the line. The next PR to touch anything would have tripped it just the same, whatever it contained. The +1..+4 is the selector consolidation's real cost: `selectorRegistry` is one static object, so a page reaching any selector reaches every provider, and this branch adds four (hubspot, managed-agent, imap, workspace). That is the same cost the 27 existing providers already impose, and it is what buys one option-list mechanism that works off the canvas. Also tried deferring the workspace provider's data-layer imports to fetch time. Reverted: this checker follows dynamic imports, so the numbers did not move, leaving only a Promise.all-of-imports shape that reads worse than the 27 sibling providers it sits next to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
CI's "Repo audits" step was failing on It hadn't. Recording a baseline on clean
Staging landed both failing routes at exactly their tolerance — The +1..+4 is the consolidation's genuine cost: Also tried deferring the workspace provider's data-layer imports to fetch time — reverted, because this checker follows dynamic imports so the numbers didn't move, leaving only a
All 5 review threads are resolved, but note Greptile's 5/5 is stale — it last reviewed |
|
@cursor review |
…abels against the full selector context Both from Bugbot; both real, both mine. **Text dependents never persisted.** `applyDependentOverrides` allowlisted `dependsOn && selectorKey`, so the plain text fields the collector started emitting were offered in the modal, stored, and gated on by the Sync button — then dropped on apply. The field stayed wiped on every push and the typed value went nowhere, which is the exact treadmill the feature existed to end. The cause was the rule being written twice. `reconfigurableDependentIds` is now the single definition of "a dependent the modal can offer AND the sync can write back", used by the collector and by the apply side. A test asserts the two agree by round-tripping through `applyDependentOverrides`, and fails against the old allowlist. **Provider labels stayed raw ids.** `useDynamicSubBlockOptionDisplayName` called `fetchById` with a `workspaceId`-only context, which silently fails any selector scoped by a sibling — `workspace.credentialGroupProviders` needs the group before it can name a provider, so the `fetchById` restored last round returned null every time. It now builds the block's real context with `buildSelectorContextFromBlock`, the same one the canvas uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@cursor review |
…ontext Follow-on to 1f423ab, and a real gap in it. That commit taught `fetchById` to read sibling context but left the React Query key at `(workspaceId, blockId, subBlockId, optionId)`. A label resolved before its sibling was set — `workspace.credentialGroupProviders` with no group picked, which returns `null` — stayed cached under the same key and was reused once the group WAS picked, so the card kept showing the raw id. Changing between two groups collided the same way. This is the repo's own React Query rule ("every identifier the queryFn forwards into the fetch must appear in the queryKey"); `check:react-query` did not catch it because the context is built in the hook rather than passed as a named arg. The key now carries the selector's OWN `getQueryKey` for that context, rather than a second hand-maintained list of context fields. The cache is scoped by exactly what the selector reads, and stays correct if a selector's dependencies change later. The context also became reactive (subscribed rather than read via `getState()`), which is what lets the key move when the sibling does. 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 cc8e7e2. Configure here.
1 participant
Builds on #6877.
Started as three fixes to the fork sync modal's custom-block inputs. Tracing why one of them was hard surfaced a root cause with much wider reach, so this ends up deleting a whole parallel mechanism.
1. A sub-block's options now come from exactly two places
dropdownandcomboboxcould load a remote list through a per-blockfetchOptions(blockId), which resolved its credential by reading the live workflow store. That works on the canvas and nowhere else — which is why the fork sync modal could not offer those fields, and why every one of those fetchers turned out to be a hand-rolled duplicate of a selector that already existed.triggers/gmail/poller.tscalled the very contractgmail.labelswraps.fetchOptions/fetchOptionByIdare deleted. What remains:selectorKeySelectorContext, so one definition serves the canvas, the fork sync modal, and anything added later.optionsNot a fallback chain — the registry is presented through the same two function shapes the props already described, so the existing lifecycle (request-id guards, dependency-scope reset, label hydration) is reused verbatim and paginated selectors drain through the same
loadAllSelectorOptionssearch/replace already uses.The root cause
Each of these needed one missing line:
canonicalParamId: 'oauthCredential'on its credential sub-block.buildSelectorContextFromBlockkeys the context on a sub-block's canonical id, so without itcontext.oauthCredentialwas never populated and the block had no way to reach its credential except the store — which is what forced the hand-rolled fetcher in the first place.Migrated (17 new selectors, ~700 lines of duplicated fetch logic deleted)
options(values). The parameter is optional, so every existing zero-argument options function is untouched.Three modules deleted outright:
lib/workflows/subblocks/options.ts,triggers/editor-state.ts,lib/managed-agents/subblock-options.ts.Two rules worth calling out
A secret never enters a selector's query key. A query key identifies a resource; a credential authorizes access to it.
oauthCredentialis safe there because it is only an id — a typed password is not, and keys are cached and surfaced by devtools.imap.mailboxeskeys on host/port/TLS/username, which already identify the mailbox list uniquely; the password rides the body exactly as before.selectorExcludeSelfreplaces the one thing a sharedsim.workflowsselector could not express. A declared flag rather than a blanket rule, because the answer differs per field: the Sim trigger never receives events about its own workflow, while the Logs block legitimately reads the logs of the workflow it runs in.2. The fork sync gap, closed and linted
clearDependentsOnRemapwipes every transitive dependent of a remapped parent, and a credential mapped between environments changes value on every sync. So a dependent the modal could not offer was re-emptied on every push — and setting it in the target did not survive either. 36 fields were in that state.The selector migration closed most of it. The collector now also emits plain text dependents (
short-input/long-input), which need no selector — just somewhere to type. It deliberately does not emit the manual half of a selector-backed canonical pair: that pair already represents the field once, and its manual member is verbatim by policy.bun run check:fork-dependent-coveragefails when a sub-block under a credential / knowledge-base / table anchor is none of: selector-backed, a canonical pair member, a preserved name-based type, or text.656 covered, zero uncovered, no baseline — verified to actually fail by seeding a regression. Picked up automatically by
check:audits(all 30 green).3. The original fork sync fixes
{{SECRET}}and<block.output>now complete in the sync modal, scoped to the target side: secrets from the workspace being written into, block outputs from the workflow that will host the block.WorkflowReferenceScopelets a surface supply the graph a reference resolves against; absent a provider the hooks read the live editor stores exactly as before, so the canvas is unchanged. The scope splits graph from values deliberately — reachability cannot change with the text being typed, and the validation hook runs in every reference-aware editor at once, so widening it would re-render all of them on every keystroke. A test pins that split.replaceCustomBlockInputsseeds from the target block when it is already the mapped type. Under a different current type nothing is carried over — those values are keyed by another block's field ids, which is the orphaning the function exists to prevent.file[]deadlock both review bots caught (replies inline).Backwards compatibility
Sub-block
type,multiSelect, ids, and stored value shapes are unchanged throughout.When adding
canonicalParamId: 'oauthCredential'tomanaged_agentwould have renamed its shipped param,blocks.test.tscaught it. Rather than rename, the context now recognises a credential by itsoauth-inputtype when no canonical id supplied one — so a picker can never force a param rename, for this block or any future one.One thing worth a live smoke test before merge:
canonicalParamId: 'oauthCredential'added to the trigger credential sub-blocks changes their canonical grouping. Those had noinputscontract to collide with and the suite is green, but it's the change I'd most want eyes on with a real credential.Verification
tscclean ·biomeclean · 28052 tests passing · 30/30 audits greenTwo failures remain —
sso-trustandunreadable-document— both pre-existing, neither in this diff, both reproducing on staging without this branch.Known gap
The modal's inputs don't render the canvas's
{{}}/<>highlight overlay — completions work, the text is unstyled. It needs pixel-exact geometry and scroll sync per control, which I couldn't verify without the running UI, and a misaligned overlay is worse than none.Documented in
/add-block,/add-trigger, and.claude/rules/sim-integrations.md.🤖 Generated with Claude Code