Skip to content

Commit 140ce11

Browse files
icecrasher321claude
andcommitted
feat(workspace-forking): make every fork-clearable sub-block reconfigurable 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>
1 parent b360887 commit 140ce11

9 files changed

Lines changed: 299 additions & 12 deletions

File tree

.agents/skills/add-block/SKILL.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,3 +1052,34 @@ After creating the block, you MUST validate it against every tool it references:
10521052
4. **Verify conditions** — each subBlock should only show for the operations that actually use it
10531053
5. **Verify `{Service}BlockMeta` is exported** with at least 7 templates, each having `icon`, `title`, `prompt`, `modules`, `category`, and `tags`
10541054
6. **If any tool outputs are still unknown**, explicitly tell the user instead of guessing block outputs
1055+
1056+
## Option Lists: `selectorKey` or `options`, never a per-block fetcher
1057+
1058+
A sub-block gets its choices from exactly one of two places. There is no third.
1059+
1060+
**`selectorKey` — every remote list.** Register the list in `hooks/selectors/providers/<service>/selectors.ts`, add its key to `SelectorKey`, and point the sub-block at it. A selector is parameterized by an explicit `SelectorContext`, so the same definition serves the canvas, the workspace-fork sync modal, and anything added later.
1061+
1062+
```ts
1063+
{ id: 'triggerCredentials', type: 'oauth-input', canonicalParamId: 'oauthCredential', mode: 'trigger' },
1064+
{ id: 'labelIds', type: 'dropdown', multiSelect: true,
1065+
selectorKey: 'gmail.labels', dependsOn: ['triggerCredentials'], mode: 'trigger' },
1066+
{ id: 'manualLabelIds', type: 'short-input', mode: 'trigger-advanced' },
1067+
```
1068+
1069+
`canonicalParamId: 'oauthCredential'` on the credential sub-block is the line people forget. `buildSelectorContextFromBlock` keys the context on a sub-block's CANONICAL id, so without it `context.oauthCredential` is never set and the picker looks unfixable without reading the store. (A credential field is also recognised by its `oauth-input` TYPE as a fallback, so a block whose shipped param is already named something else does not have to rename it.)
1070+
1071+
**`options` — everything else.** A static array, or a pure function of the block's own values for a list that narrows to a sibling's selection. No I/O.
1072+
1073+
```ts
1074+
options: (params) => {
1075+
const model = params?.values.model
1076+
return typeof model === 'string' ? effortsFor(model) : DEFAULT_EFFORTS
1077+
}
1078+
```
1079+
1080+
**Never fetch inside `options`, and never reach into the stores from a block definition.** A fetcher that resolves its credential with `readSubBlockValue(blockId, ...)` only works on the canvas — every surface that is not the editor gets an empty list. `fetchOptions`/`fetchOptionById` were removed for exactly this reason.
1081+
1082+
Two rules the checks enforce:
1083+
1084+
- **A secret never enters a selector's `getQueryKey`.** A query key identifies a resource; a credential authorizes access to it. A credential *id* is fine; a typed password is not (see `imap.mailboxes`).
1085+
- **A sub-block that `dependsOn` a credential / knowledge-base / table selector must be reconfigurable at fork-sync time** — a `selectorKey`, a canonical pair whose basic member is a selector, or a `short-input`/`long-input`. `bun run check:fork-dependent-coverage` fails otherwise, because a fork sync clears those fields on every push and an unofferable one can never be set anywhere that sticks.

.agents/skills/add-trigger/SKILL.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,37 @@ Add to `helm/sim/values.yaml` under the existing polling cron jobs:
472472
- Cursor-based (changes API): `apps/sim/lib/webhooks/polling/google-drive.ts`
473473
- Timestamp-based: `apps/sim/lib/webhooks/polling/google-calendar.ts`
474474

475+
## Option Lists: `selectorKey` or `options`, never a per-block fetcher
476+
477+
A sub-block gets its choices from exactly one of two places. There is no third.
478+
479+
**`selectorKey` — every remote list.** Register the list in `hooks/selectors/providers/<service>/selectors.ts`, add its key to `SelectorKey`, and point the sub-block at it. A selector is parameterized by an explicit `SelectorContext`, so the same definition serves the canvas, the workspace-fork sync modal, and anything added later.
480+
481+
```ts
482+
{ id: 'triggerCredentials', type: 'oauth-input', canonicalParamId: 'oauthCredential', mode: 'trigger' },
483+
{ id: 'labelIds', type: 'dropdown', multiSelect: true,
484+
selectorKey: 'gmail.labels', dependsOn: ['triggerCredentials'], mode: 'trigger' },
485+
{ id: 'manualLabelIds', type: 'short-input', mode: 'trigger-advanced' },
486+
```
487+
488+
`canonicalParamId: 'oauthCredential'` on the credential sub-block is the line people forget. `buildSelectorContextFromBlock` keys the context on a sub-block's CANONICAL id, so without it `context.oauthCredential` is never set and the picker looks unfixable without reading the store. (A credential field is also recognised by its `oauth-input` TYPE as a fallback, so a block whose shipped param is already named something else does not have to rename it.)
489+
490+
**`options` — everything else.** A static array, or a pure function of the block's own values for a list that narrows to a sibling's selection. No I/O.
491+
492+
```ts
493+
options: (params) => {
494+
const model = params?.values.model
495+
return typeof model === 'string' ? effortsFor(model) : DEFAULT_EFFORTS
496+
}
497+
```
498+
499+
**Never fetch inside `options`, and never reach into the stores from a block definition.** A fetcher that resolves its credential with `readSubBlockValue(blockId, ...)` only works on the canvas — every surface that is not the editor gets an empty list. `fetchOptions`/`fetchOptionById` were removed for exactly this reason.
500+
501+
Two rules the checks enforce:
502+
503+
- **A secret never enters a selector's `getQueryKey`.** A query key identifies a resource; a credential authorizes access to it. A credential *id* is fine; a typed password is not (see `imap.mailboxes`).
504+
- **A sub-block that `dependsOn` a credential / knowledge-base / table selector must be reconfigurable at fork-sync time** — a `selectorKey`, a canonical pair whose basic member is a selector, or a `short-input`/`long-input`. `bun run check:fork-dependent-coverage` fails otherwise, because a fork sync clears those fields on every push and an unofferable one can never be set anywhere that sticks.
505+
475506
## Checklist
476507

477508
### Trigger Definition

.claude/rules/sim-integrations.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,5 @@ The full authoring instructions — tool/block/icon/trigger scaffolding, SubBloc
1717
- Type coercions (`Number()`, etc.) belong in `tools.config.params` (runs at execution, after variable resolution) — never in `tools.config.tool` (runs at serialization; coercing there destroys dynamic `<Block.output>` references).
1818
- `canonicalParamId` must NOT match any subblock's `id`, must be unique **block-wide** (groups are keyed by canonical id across every subblock and hold exactly one `basicId`, so two operations that each need a pair need two different canonical ids), and all subblocks in a canonical group must share the same `required` status. The `inputs` section and the params function reference canonical IDs, not raw subblock IDs — the serializer deletes the subblock IDs and republishes the active member's value under the canonical ID.
1919
- A canonical pair carries ONE concept. For files that is upload (basic) + file reference (advanced), as in Gmail attachments (`blocks/blocks/gmail.ts`). Never overload the advanced side with alternate identifiers (URL, provider asset ID) — give those their own subblocks, mark mutually exclusive sources `required: false`, and enforce "exactly one" at execution.
20+
- A sub-block's option list is EITHER `selectorKey` (a registered selector — the only way to load a remote list, and the only one that works off the canvas) OR `options` (a static array, or a pure function of the block's own values). Never fetch from a block definition, and never read the workflow stores there. A credential sub-block needs `canonicalParamId: 'oauthCredential'` for its dependants' selectors to resolve. A secret must never appear in a selector's `getQueryKey`. `bun run check:fork-dependent-coverage` fails a `dependsOn` under a credential/KB/table anchor that the fork sync modal cannot offer.
2021
- Blocks must also set the catalog/UI metadata fields `integrationType`, `tags`, `authMode`, `docsLink`, and export a `{Service}BlockMeta` — see the `/add-block` skill's BlockMeta section for details.

apps/sim/ee/workspace-forking/components/fork-sync/custom-block-input-control.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,24 @@ export const CUSTOM_BLOCK_UNSUPPORTED_HINT = 'Uploaded in the workflow — kept
9090
* unconfigured required field relies on.
9191
*/
9292
export function isForkSyncConfigurableField(
93-
field: Pick<ForkDependentReconfig, 'parentKind' | 'fieldType'>
93+
field: Pick<ForkDependentReconfig, 'parentKind' | 'fieldType' | 'selectorKey'>
9494
): boolean {
95-
if (field.parentKind !== 'custom-block') return true
96-
return customBlockInputControl(field.fieldType) !== 'unsupported'
95+
return forkDependentControl(field) !== 'unsupported'
96+
}
97+
98+
/**
99+
* The control the sync modal renders for one dependent field.
100+
*
101+
* `fieldType` means two different things depending on where the field came from, so the two
102+
* are classified separately rather than by one lookup that happens to agree:
103+
* - a **custom-block input** declares a Start FIELD type (`string`, `boolean`, `file[]`),
104+
* which {@link customBlockInputControl} maps through the canvas's own mapping;
105+
* - every other no-selector dependent is a canvas SUB-BLOCK, whose own type says it.
106+
*/
107+
export function forkDependentControl(
108+
field: Pick<ForkDependentReconfig, 'parentKind' | 'fieldType' | 'selectorKey'>
109+
): CustomBlockInputControl | 'selector' {
110+
if (field.selectorKey) return 'selector'
111+
if (field.parentKind === 'custom-block') return customBlockInputControl(field.fieldType)
112+
return field.fieldType === 'long-input' ? 'textarea' : 'input'
97113
}

apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import { forkRefKey } from '@/ee/workspace-forking/components/fork-sync/copy-rec
3636
import {
3737
CUSTOM_BLOCK_UNSUPPORTED_HINT,
3838
customBlockBooleanOptions,
39-
customBlockInputControl,
39+
forkDependentControl,
4040
} from '@/ee/workspace-forking/components/fork-sync/custom-block-input-control'
4141
import { CustomBlockInputField } from '@/ee/workspace-forking/components/fork-sync/custom-block-input-field'
4242
import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-sync/dependent-field-selector'
@@ -221,15 +221,18 @@ function DependentSelector({
221221
: effectiveDependentValue(f, state, parentChanged)
222222
const baselineValueFor = (f: ForkDependentReconfig) => effectiveValueIn(f, {})
223223
const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig)
224-
if (isCustomBlockInput) {
225-
// Not a selector: there is no parent resource to browse and no options to fetch, just the
226-
// target block's own declared input. Renders a BARE control, like `DependentFieldSelector`
227-
// does — the row wrapper above already draws the field's label and required marker, so a
228-
// labelled `ChipModalField` printed the title twice.
224+
// A dependent with no selector has no parent resource to browse and no options to fetch —
225+
// just a value to type. That is every custom-block input, and also a plain text field under
226+
// a remapped credential (a Jira issue type, a Notion block id), which the sync clears on
227+
// every push and so must be re-settable here.
228+
if (!field.selectorKey) {
229+
// Renders a BARE control, like `DependentFieldSelector` does — the row wrapper above
230+
// already draws the field's label and required marker, so a labelled `ChipModalField`
231+
// printed the title twice.
229232
const setValue = (value: string) =>
230233
setReconfig((current) => ({ ...current, [dependentKey(field)]: value }))
231234
const value = effectiveValue(field)
232-
switch (customBlockInputControl(field.fieldType)) {
235+
switch (forkDependentControl(field)) {
233236
case 'switch':
234237
return (
235238
<ChipSwitch

apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,72 @@ describe('collectForkDependentReconfigs', () => {
359359
expect(sheet?.context.spreadsheetId).toBe('ss-src')
360360
})
361361

362+
it('offers a plain text dependent of a remapped credential', () => {
363+
// `clearDependentsOnRemap` wipes it on EVERY sync (a credential mapped across environments
364+
// changes value each time), so a text field the modal never offered was re-emptied on every
365+
// push with nowhere to set it that stuck.
366+
vi.mocked(getBlock).mockReturnValue(
367+
blockWith([
368+
{ id: 'credential', title: 'Credential', type: 'oauth-input' },
369+
{
370+
id: 'issueType',
371+
title: 'Issue Type',
372+
type: 'short-input',
373+
dependsOn: ['credential'],
374+
},
375+
])
376+
)
377+
const states = new Map<string, WorkflowState>([
378+
[
379+
'wf-src',
380+
sourceState('jira', { credential: { value: 'cred-src' }, issueType: { value: 'Bug' } }),
381+
],
382+
])
383+
const fields = collectForkDependentReconfigs([replaceItem], states, resolve)
384+
expect(fields).toHaveLength(1)
385+
expect(fields[0]).toMatchObject({ subBlockKey: 'issueType', fieldType: 'short-input' })
386+
expect(fields[0].selectorKey).toBeUndefined()
387+
})
388+
389+
it('does not offer the manual half of a selector-backed canonical pair', () => {
390+
// The pair's selector member already represents the field, and the manual member is
391+
// verbatim by policy — offering both shows one concept twice and invites writing into the
392+
// inactive half.
393+
vi.mocked(getBlock).mockReturnValue(
394+
blockWith([
395+
{ id: 'credential', title: 'Credential', type: 'oauth-input' },
396+
{
397+
id: 'projectId',
398+
title: 'Project',
399+
type: 'project-selector',
400+
canonicalParamId: 'projectId',
401+
mode: 'basic',
402+
selectorKey: 'jira.projects',
403+
dependsOn: ['credential'],
404+
},
405+
{
406+
id: 'manualProjectId',
407+
title: 'Project ID',
408+
type: 'short-input',
409+
canonicalParamId: 'projectId',
410+
mode: 'advanced',
411+
dependsOn: ['credential'],
412+
},
413+
])
414+
)
415+
const states = new Map<string, WorkflowState>([
416+
[
417+
'wf-src',
418+
sourceState('jira', { credential: { value: 'cred-src' }, projectId: { value: 'PROJ' } }),
419+
],
420+
])
421+
const keys = collectForkDependentReconfigs([replaceItem], states, resolve).map(
422+
(field) => field.subBlockKey
423+
)
424+
expect(keys).toContain('projectId')
425+
expect(keys).not.toContain('manualProjectId')
426+
})
427+
362428
it('uses the persisted canonical mode when building a dependent selector context', () => {
363429
vi.mocked(getBlock).mockReturnValue(
364430
blockWith([

apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ interface ReconfigItem {
4848
* intentionally excluded: their tool dependent has no `selectorKey` and a separate
4949
* (non-`useSelectorOptions`) stack, so it falls back to the needs-config surfacing.
5050
*/
51+
/**
52+
* Dependent sub-block types the modal renders as a free-text field rather than a picker.
53+
* They carry no options to fetch, so they need no selector — just somewhere to type.
54+
*/
55+
const TEXT_DEPENDENT_TYPES = new Set<string>(['short-input', 'long-input'])
56+
5157
const PARENT_ANCHORS: ReadonlyArray<{
5258
subBlockType: string
5359
parentKind: ForkDependentReconfig['parentKind']
@@ -130,6 +136,22 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
130136
const canonicalIndex = buildCanonicalIndex(config.subBlocks)
131137
const gates = createCanonicalModeGates(config.subBlocks, values, canonicalModes)
132138
const configById = new Map(config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
139+
// Text members of a canonical pair whose basic side IS a selector. The pair already
140+
// represents the field: its selector member is offered, and the manual member is verbatim by
141+
// policy (`clearDependentsOnRemap` never clears it), so offering it too would show the same
142+
// concept twice and invite writing into the inactive half.
143+
const canonicalWithSelector = new Set(
144+
config.subBlocks
145+
.filter((cfg) => cfg.canonicalParamId && cfg.selectorKey)
146+
.map((cfg) => cfg.canonicalParamId)
147+
)
148+
const canonicalPairMembers = new Set(
149+
config.subBlocks
150+
.filter(
151+
(cfg) => cfg.id && cfg.canonicalParamId && canonicalWithSelector.has(cfg.canonicalParamId)
152+
)
153+
.map((cfg) => cfg.id as string)
154+
)
133155
// A field could hang off two anchors (or be reachable via two paths); emit it once.
134156
const seen = new Set<string>()
135157

@@ -167,7 +189,19 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
167189

168190
for (const clear of getTransitiveSubBlockDependents(config.subBlocks, [anchorCfg.id])) {
169191
const dependent = configById.get(clear.subBlockId)
170-
if (!dependent?.id || !dependent.selectorKey) continue
192+
// A dependent is offered when the modal can actually render a control for it: a
193+
// registered selector, or a plain text field. Anything else is skipped and the
194+
// fork-dependent-coverage check keeps that set empty — see
195+
// `scripts/check-fork-dependent-coverage.ts`.
196+
//
197+
// Text fields matter as much as selectors here. `clearDependentsOnRemap` wipes every
198+
// transitive dependent of a remapped parent on EVERY sync (a credential mapped across
199+
// environments changes value each time), so a field the modal never offered was
200+
// re-emptied on every push and could not be fixed by setting it in the target either.
201+
if (!dependent?.id) continue
202+
const isTextDependent =
203+
TEXT_DEPENDENT_TYPES.has(dependent.type) && !canonicalPairMembers.has(dependent.id)
204+
if (!dependent.selectorKey && !isTextDependent) continue
171205
// Skip fields gated off by their `condition` - a selector under a now-inactive
172206
// operation (e.g. a move-only label while the block reads) isn't in play. We do
173207
// NOT require a source value: an active selector the source left empty is still
@@ -233,7 +267,9 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
233267
targetBlockId: resolveTargetBlockId(),
234268
blockName,
235269
subBlockKey: makeSubBlockKey(dependent.id),
236-
selectorKey: dependent.selectorKey,
270+
...(dependent.selectorKey
271+
? { selectorKey: dependent.selectorKey }
272+
: { fieldType: dependent.type }),
237273
title: makeTitle(dependent),
238274
...(toolName ? { toolName } : {}),
239275
...(dependencyScope ? { dependencyScope } : {}),

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"check": "turbo run format:check",
2626
"check:boundaries": "bun run scripts/check-monorepo-boundaries.ts",
2727
"check:api-validation": "bun run scripts/check-api-validation-contracts.ts --check",
28+
"check:fork-dependent-coverage": "bun run scripts/check-fork-dependent-coverage.ts",
2829
"generate:openapi": "bun run scripts/generate-openapi.ts",
2930
"check:openapi": "bun run scripts/check-openapi.ts",
3031
"generate:cli-api": "bun run scripts/generate-v2-cli-api.ts",

0 commit comments

Comments
 (0)