fix(forks): make the post-sync collector honor tool param visibility too

Greptile caught a real asymmetry, and corrected a wrong assumption in the first
commit: `needsConfiguration` is NOT warning-only. `promote.ts:1035` skips the
target's redeploy for any workflow in that list, and `:792` also withholds its
chat-deployment carry-over.

So with only the pre-sync collector fixed, an Agent block whose Jira `issueKey`
was populated in the target and cleared by a credential remap would let the sync
through (the modal correctly treats a model-supplied param as non-blocking) and
then silently decline to redeploy that workflow - leaving the fork running its
previous deployed version with no gate and no error.

Both collectors now resolve `required` through one shared
`resolveToolParamRequired`, so the pre-sync gate and the promote path cannot
disagree about what a nested tool param means. The lookup (sub-block id, then
canonical param id, then fail closed to the block-level rule) lives in one place
instead of being duplicated.

The `@/tools/params` mock in remap-references.test.ts is now overridable so a
test can opt into an authoritative resolution; its defaults are unchanged and
the other 74 tests pass untouched. The new case is verified to fail without the
fix.

Author: mzxchandra

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

@@ -15,10 +15,7 @@ import {
   isNonEmptyValue,
   scopeCanonicalModesForTool,
 } from '@/lib/workflows/subblocks/visibility'
-import {
-  isSubBlockRequired,
-  isToolParamUserRequired,
-} from '@/lib/workflows/tool-input/param-visibility'
+import { resolveToolParamRequired } from '@/lib/workflows/tool-input/param-visibility'
 import { getBlock } from '@/blocks/registry'
 import type { SubBlockConfig } from '@/blocks/types'
 import { getDependsOnFields } from '@/blocks/utils'
@@ -93,7 +90,7 @@ interface EmitAnchoredParams {
    * Present ONLY for the nested `tool-input` pass: each param's resolved
    * {@link ParameterVisibility}, keyed by sub-block id and by canonical param id. Its presence
    * is what marks a dependent as a tool param rather than a block sub-block, so `required`
-   * can apply the tool-row rule (see {@link isToolParamUserRequired}).
+   * can apply the tool-row rule (see {@link resolveToolParamRequired}).
    *
    * Two cases fall back to the block-level `required`, failing closed: a param absent from
    * the map (custom-tool / MCP generic fallback, or an unresolvable tool id), and a param
@@ -230,16 +227,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
         // Testing `rawSourceValue` directly is sound: the dormant guard above has already
         // returned for any pair in advanced mode, so the pair is basic-active here and
         // `rawSourceValue` IS the group's active canonical value.
-        const paramVisibility = paramVisibilityById
-          ? (paramVisibilityById.get(dependent.id) ??
-            (dependent.canonicalParamId
-              ? paramVisibilityById.get(dependent.canonicalParamId)
-              : undefined))
-          : undefined
-        const configuredRequired =
-          paramVisibility !== undefined
-            ? isToolParamUserRequired({ required: dependent.required, paramVisibility }, values)
-            : isSubBlockRequired(dependent.required, values)
+        const configuredRequired = resolveToolParamRequired(dependent, values, paramVisibilityById)
         out.push({
           parentKind: anchor.parentKind,
           parentSourceId,

apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts

@@ -6,16 +6,25 @@ import type { BlockConfig, SubBlockConfig } from '@/blocks/types'
 
 // The indexer resolves a tool's params via the tool registry; stub it so the
 // injected blockConfigs subBlocks drive resolution deterministically in tests.
+// Exposed as vi.fn()s (with the historical defaults) so a test that needs an
+// AUTHORITATIVE resolution - i.e. one carrying `paramVisibility` - can opt in.
+const { mockGetToolIdForOperation, mockGetSubBlocksForToolInput } = vi.hoisted(() => ({
+  mockGetToolIdForOperation: vi.fn((): string | undefined => undefined),
+  mockGetSubBlocksForToolInput: vi.fn(
+    (
+      _toolId: string,
+      _type: string,
+      _values: unknown,
+      _modes: unknown,
+      provided?: { subBlocks?: SubBlockConfig[] }
+    ) => ({ subBlocks: provided?.subBlocks ?? [] })
+  ),
+}))
+
 vi.mock('@/tools/params', () => ({
-  getToolIdForOperation: () => undefined,
+  getToolIdForOperation: mockGetToolIdForOperation,
   getToolParametersConfig: () => null,
-  getSubBlocksForToolInput: (
-    _toolId: string,
-    _type: string,
-    _values: unknown,
-    _modes: unknown,
-    provided?: { subBlocks?: SubBlockConfig[] }
-  ) => ({ subBlocks: provided?.subBlocks ?? [] }),
+  getSubBlocksForToolInput: mockGetSubBlocksForToolInput,
   formatParameterLabel: (label: string) => label,
 }))
 
@@ -1005,6 +1014,52 @@ describe('collectClearedDependents', () => {
       },
     ])
   })
+
+  it('does not mark a cleared model-supplied tool param as required', () => {
+    // The pre-sync modal treats a `user-or-llm` param as non-blocking (the agent fills it at
+    // runtime). This collector must agree: a `required` entry here makes promote SKIP the
+    // target's redeploy, so disagreeing would let a sync through and then silently withhold
+    // the deployment.
+    mockGetToolIdForOperation.mockReturnValueOnce('gmail_read')
+    vi.mocked(getBlock).mockImplementation((type) => {
+      if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }])
+      if (type === 'gmail')
+        return blockWith([
+          { id: 'credential', title: 'Credential', type: 'oauth-input' },
+          {
+            id: 'folder',
+            title: 'Label',
+            type: 'folder-selector',
+            dependsOn: ['credential'],
+            required: true,
+            paramVisibility: 'user-or-llm',
+          },
+        ])
+      return undefined as unknown as BlockConfig
+    })
+    const targetDraft: SubBlockRecord = {
+      tools: entry('tools', 'tool-input', [
+        { type: 'gmail', title: 'Gmail', params: { credential: 'c-target', folder: 'INBOX' } },
+      ]),
+    }
+    const merged: SubBlockRecord = {
+      tools: entry('tools', 'tool-input', [
+        { type: 'gmail', title: 'Gmail', params: { credential: 'c-new', folder: '' } },
+      ]),
+    }
+    const result = collectClearedDependents('agent', 'b1', 'Agent', targetDraft, merged)
+    // Still surfaced (the value really was cleared), just not gating the redeploy.
+    expect(result).toEqual([
+      {
+        blockId: 'b1',
+        blockName: 'Agent',
+        subBlockKey: 'tools[0].folder',
+        title: 'Label',
+        toolName: 'Gmail',
+        required: false,
+      },
+    ])
+  })
 })
 
 describe('applyDependentOverrides', () => {

apps/sim/ee/workspace-forking/lib/remap/remap-references.ts

@@ -29,7 +29,10 @@ import {
   resolveCanonicalMode,
   scopeCanonicalModesForTool,
 } from '@/lib/workflows/subblocks/visibility'
-import { isSubBlockRequired } from '@/lib/workflows/tool-input/param-visibility'
+import {
+  isSubBlockRequired,
+  resolveToolParamRequired,
+} from '@/lib/workflows/tool-input/param-visibility'
 import type { ParsedStoredTool } from '@/lib/workflows/tool-input/types'
 import { getBlock } from '@/blocks/registry'
 import type { SubBlockConfig } from '@/blocks/types'
@@ -39,6 +42,7 @@ import {
   remapForkFileUploadValue,
 } from '@/ee/workspace-forking/lib/remap/remap-files'
 import { isEnvVarReference, isReference } from '@/executor/constants'
+import type { ParameterVisibility } from '@/tools/types'
 
 /**
  * Resource kinds the fork remapper rewrites across workspaces, derived from the
@@ -1232,6 +1236,27 @@ function collectClearedToolParamDependents(
       scopeCanonicalModesForTool(parentCanonicalModes, index, tool.type)
     )
     const toolLabel = typeof tool.title === 'string' && tool.title ? tool.title : toolConfig.name
+    // Resolved visibility per param, so `required` here means the same thing it means in the
+    // pre-sync modal. Without this the two paths disagree: the modal would let a sync through
+    // (a model-supplied param is not the user's to fill) and then this collector would mark it
+    // required, which SKIPS the target's redeploy in `promote.ts` - leaving the fork silently
+    // running its previous deployed version.
+    const paramVisibilityById = new Map<string, ParameterVisibility | undefined>()
+    for (const resolved of getToolInputParamConfigs({
+      tool: { ...tool, type: tool.type, params: mergedParams },
+      toolIndex: index,
+      parentCanonicalModes,
+    })) {
+      if (!resolved.authoritative) continue
+      const visibility = resolved.config.paramVisibility
+      paramVisibilityById.set(resolved.paramId, visibility)
+      if (
+        resolved.config.canonicalParamId &&
+        !paramVisibilityById.has(resolved.config.canonicalParamId)
+      ) {
+        paramVisibilityById.set(resolved.config.canonicalParamId, visibility)
+      }
+    }
     for (const cfg of toolConfig.subBlocks) {
       if (!cfg.dependsOn || !cfg.id) continue
       // Only flag a param the TARGET tool had configured (not one the source carried in).
@@ -1247,7 +1272,7 @@ function collectClearedToolParamDependents(
         subBlockKey: `${toolInputKey}[${index}].${cfg.id}`,
         title: cfg.title ?? cfg.id,
         toolName: toolLabel,
-        required: isSubBlockRequired(cfg.required, mergedValues),
+        required: resolveToolParamRequired(cfg, mergedValues, paramVisibilityById),
       })
     }
   }

apps/sim/lib/workflows/tool-input/param-visibility.ts

@@ -42,6 +42,33 @@ export function isUserSuppliedToolParam(config: Pick<SubBlockConfig, 'paramVisib
   return (config.paramVisibility ?? DEFAULT_TOOL_PARAM_VISIBILITY) === 'user-only'
 }
 
+/**
+ * Whether a `tool-input` param must be supplied by the user, resolving its visibility from a
+ * map keyed by sub-block id and canonical param id.
+ *
+ * Shared by fork sync's PRE-sync collector (which decides whether a row blocks the Sync
+ * button) and its POST-sync collector (whose `required` entries make promote SKIP the
+ * target's redeploy). Those two must agree: if the modal lets a sync through because a param
+ * is the model's to fill, the promote path must not then withhold the deployment for the
+ * same param.
+ *
+ * `paramVisibilityById` omitted means the caller is not in a tool-input context (a block's
+ * own sub-blocks), so the plain block-level rule applies. A param absent from the map, or
+ * present with an `undefined` value, falls back the same way — failing closed.
+ */
+export function resolveToolParamRequired(
+  config: Pick<SubBlockConfig, 'id' | 'required' | 'canonicalParamId'>,
+  values: Record<string, unknown>,
+  paramVisibilityById?: ReadonlyMap<string, ParameterVisibility | undefined>
+): boolean {
+  if (!paramVisibilityById) return isSubBlockRequired(config.required, values)
+  const visibility =
+    paramVisibilityById.get(config.id) ??
+    (config.canonicalParamId ? paramVisibilityById.get(config.canonicalParamId) : undefined)
+  if (visibility === undefined) return isSubBlockRequired(config.required, values)
+  return isToolParamUserRequired({ required: config.required, paramVisibility: visibility }, values)
+}
+
 /**
  * Resolve a sub-block's `required` declaration against the surrounding values. `true` is
  * unconditional; the object form is structurally a `SubBlockCondition` evaluated against the