fix(github): scope each param alias to the operations that declare it

Author: waleedlatif1

apps/sim/blocks/blocks/github.param-wiring.test.ts

@@ -39,8 +39,8 @@ const RENAMES = [
 ] as const
 
 describe('every renamed subBlock reaches its tool param', () => {
-  it.each(RENAMES)('%s -> %s', (subBlockId, paramName) => {
-    expect(map({ [subBlockId]: 'x' })).toHaveProperty(paramName)
+  it.each(RENAMES)('%s -> %s', (subBlockId, paramName, toolId) => {
+    expect(map({ operation: toolId, [subBlockId]: 'x' })).toHaveProperty(paramName)
   })
 
   it.each(RENAMES)(
@@ -73,12 +73,16 @@ describe('guarded assignment protects the agent tool-calling path', () => {
   })
 
   it('leaves a model-supplied value untouched when the block field is absent', () => {
-    const modelArgs = { content: 'rocket', title: 'from the model' }
+    const modelArgs = {
+      operation: 'github_create_issue_reaction',
+      content: 'rocket',
+      title: 'from the model',
+    }
     expect({ ...modelArgs, ...map(modelArgs) }).toEqual(modelArgs)
   })
 
   it.each(['', null, undefined])('treats %o as not provided', (empty) => {
-    expect(map({ reaction_content: empty })).toEqual({})
+    expect(map({ operation: 'github_create_issue_reaction', reaction_content: empty })).toEqual({})
   })
 })
 
@@ -88,15 +92,17 @@ describe('gist_public coercion', () => {
     ['false', false],
     [true, true],
   ])('maps %o to %o', (input, expected) => {
-    expect(map({ gist_public: input }).public).toBe(expected)
+    expect(map({ operation: 'github_create_gist', gist_public: input }).public).toBe(expected)
   })
 
   it('omits public entirely when untouched, leaving the tool default', () => {
     expect(map({ operation: 'github_create_gist' })).not.toHaveProperty('public')
   })
 
   it.each([null, undefined, ''])('treats %o as unset rather than Secret', (unset) => {
-    expect(map({ gist_public: unset })).not.toHaveProperty('public')
+    expect(map({ operation: 'github_create_gist', gist_public: unset })).not.toHaveProperty(
+      'public'
+    )
   })
 
   /**
@@ -109,11 +115,12 @@ describe('gist_public coercion', () => {
     ['false', false],
     [false, false],
   ])('treats %o as an explicit Secret selection', (input, expected) => {
-    expect(map({ gist_public: input }).public).toBe(expected)
+    expect(map({ operation: 'github_create_gist', gist_public: input }).public).toBe(expected)
   })
 
   it.each(['false', false])('overrides a model-supplied public for %o', (secret) => {
-    expect({ public: true, ...map({ gist_public: secret }) }.public).toBe(false)
+    const inputs = { operation: 'github_create_gist', gist_public: secret }
+    expect({ public: true, ...map(inputs) }.public).toBe(false)
   })
 
   it('matches the dropdown option ids the block actually renders', () => {
@@ -161,3 +168,54 @@ describe('the v2 block inherits the same mapper', () => {
     expect(GitHubV2Block.tools.config?.params).toBe(GitHubBlock.tools.config?.params)
   })
 })
+
+/**
+ * `shouldSerializeSubBlock` (`serializer/index.ts:91-93`) serializes a
+ * non-empty `mode: 'advanced'` field WITHOUT evaluating its condition. Seven of
+ * the aliased sources are advanced, so a value left behind by an earlier
+ * operation is still present in `params` after the user switches operations.
+ * An unscoped alias would rewrite it onto the new operation's tool param.
+ */
+describe('a stale advanced field cannot leak onto another operation', () => {
+  it('does not turn a leftover milestone_title into github_update_pr title', () => {
+    const mapped = map({ operation: 'github_update_pr', milestone_title: 'Q3 milestone' })
+    expect(mapped).not.toHaveProperty('title')
+  })
+
+  it('does not clobber the PR title the user actually typed', () => {
+    const inputs = {
+      operation: 'github_update_pr',
+      title: 'Fix the parser',
+      milestone_title: 'Q3 milestone',
+    }
+    expect({ ...inputs, ...map(inputs) }.title).toBe('Fix the parser')
+  })
+
+  it.each([
+    ['github_create_pr', 'milestone_title', 'title'],
+    ['github_create_issue', 'milestone_description', 'description'],
+    ['github_list_issues', 'milestone_state', 'state'],
+    ['github_search_repos', 'milestone_sort', 'sort'],
+    ['github_search_repos', 'fork_sort', 'sort'],
+    ['github_create_gist', 'fork_name', 'name'],
+    ['github_update_project', 'gist_public', 'public'],
+  ])('%s ignores a stale %s', (operation, from, to) => {
+    expect(map({ operation, [from]: 'stale' })).not.toHaveProperty(to)
+  })
+
+  it('every advanced source is scoped to at least one operation', () => {
+    const advanced = GitHubBlock.subBlocks.filter((s) => s.mode === 'advanced').map((s) => s.id)
+    for (const src of [
+      'milestone_title',
+      'milestone_description',
+      'milestone_state',
+      'milestone_sort',
+      'fork_name',
+      'fork_sort',
+      'gist_public',
+    ]) {
+      expect(advanced, `${src} is expected to be an advanced field`).toContain(src)
+      expect(map({ operation: '', [src]: 'x' })).toEqual({})
+    }
+  })
+})

apps/sim/blocks/blocks/github.ts

@@ -9,6 +9,44 @@ import { getTrigger } from '@/triggers'
 /** Reviewers can be named individually or by team slug; either identifies the request. */
 const REVIEWER_FIELD = ['reviewers', 'team_reviewers'] as const
 
+/**
+ * Block subBlock ids that differ from the tool param they feed, each scoped to
+ * the operations whose tool declares that target. `sort` has two sources and
+ * `title`/`description`/`state` share their names with fields on other
+ * operations, so the scoping is what keeps them from colliding.
+ *
+ * `toBoolean` marks a dropdown feeding a boolean tool param: a dropdown stores
+ * its option id, so the value arrives as the string 'true'/'false' and the
+ * generic handler only JSON-parses `json`/`array` inputs.
+ */
+const GITHUB_PARAM_ALIASES: ReadonlyArray<{
+  from: string
+  to: string
+  operations: readonly string[]
+  toBoolean?: true
+}> = [
+  {
+    from: 'reaction_content',
+    to: 'content',
+    operations: ['github_create_issue_reaction', 'github_create_comment_reaction'],
+  },
+  {
+    from: 'milestone_title',
+    to: 'title',
+    operations: ['github_create_milestone', 'github_update_milestone'],
+  },
+  {
+    from: 'milestone_description',
+    to: 'description',
+    operations: ['github_create_milestone', 'github_update_milestone'],
+  },
+  { from: 'milestone_state', to: 'state', operations: ['github_list_milestones'] },
+  { from: 'milestone_sort', to: 'sort', operations: ['github_list_milestones'] },
+  { from: 'fork_name', to: 'name', operations: ['github_fork_repo'] },
+  { from: 'fork_sort', to: 'sort', operations: ['github_list_forks'] },
+  { from: 'gist_public', to: 'public', operations: ['github_create_gist'], toBoolean: true },
+]
+
 export const GitHubBlock: BlockConfig<GitHubResponse> = {
   type: 'github',
   name: 'GitHub (Legacy)',
@@ -2284,53 +2322,39 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
        *
        * A tool param is populated only when a subBlock's `id` equals it — the
        * serializer keys values by subBlock id, and nothing else renames them.
-       * Each field below renders, accepts input, and then arrives under a name
-       * its tool never reads.
+       * Each aliased field below renders, accepts input, and then arrives under
+       * a name its tool never reads.
+       *
+       * Every alias is scoped to the operations whose tool actually declares
+       * the target param, and that scoping is load-bearing. Seven of these
+       * sources are `mode: 'advanced'`, and `shouldSerializeSubBlock`
+       * (`serializer/index.ts:91-93`) serializes a non-empty advanced field
+       * WITHOUT evaluating its condition. So a `milestone_title` left over from
+       * an earlier operation is still in `params` after the user switches to,
+       * say, Update PR — and an unscoped alias would rewrite it to `title` and
+       * clobber the PR's own title with stale milestone data.
        *
-       * Every assignment is guarded, and that is load-bearing rather than
-       * defensive. `generic-handler.ts` merges `{ ...inputs, ...params(inputs) }`
-       * and `providers/utils.ts` installs this same function as the provider
-       * `paramsTransform`, spreading its result over the model's tool-call
-       * arguments. An unconditional write would therefore clobber a
-       * model-supplied `content`/`title`/`sort` with `undefined` on the agent
-       * tool-calling path — which is the one path these fields work on today.
+       * Presence is tested rather than truthiness so that a deliberate `false`
+       * or `'false'` is not mistaken for an unset field; only nullish and empty
+       * defer to the tool's own default.
        *
-       * `sort` has two sources and `title`/`description`/`state` share their
-       * names with fields on other operations. That is safe only because each
-       * source subBlock's `condition` binds it to a single operation, so at
-       * most one source of a given target is ever present in `params`.
+       * `generic-handler.ts` merges `{ ...inputs, ...params(inputs) }` and
+       * `providers/utils.ts` installs this as the provider `paramsTransform`,
+       * spreading over the model's tool-call arguments — so emitting a key the
+       * block did not supply would clobber a model-supplied value on the agent
+       * path, which is the one path these fields work on today.
        */
       params: (params) => {
         const result: Record<string, unknown> = {}
+        const operation = typeof params.operation === 'string' ? params.operation : ''
 
-        if (params.reaction_content) result.content = params.reaction_content
-        if (params.milestone_title) result.title = params.milestone_title
-        if (params.milestone_description) result.description = params.milestone_description
-        if (params.milestone_state) result.state = params.milestone_state
-        if (params.milestone_sort) result.sort = params.milestone_sort
-        if (params.fork_name) result.name = params.fork_name
-        if (params.fork_sort) result.sort = params.fork_sort
+        const isSet = (value: unknown) => value !== undefined && value !== null && value !== ''
 
-        /**
-         * A dropdown stores its option id, so this arrives as the string
-         * 'true'/'false' while the tool declares `public` as a boolean. The
-         * generic handler only JSON-parses `json`/`array` inputs, so nothing
-         * else coerces it.
-         *
-         * Presence is tested rather than truthiness, because boolean `false` is
-         * a real selection: the block declares this input as `boolean`, so a
-         * writer following that schema stores `false` rather than `'false'`.
-         * Under a truthy check the two disagree — `'false'` would force the
-         * gist secret while `false` was dropped, letting a model-supplied
-         * `public: true` through on the agent path. Only an unset field
-         * (nullish or empty) defers to the tool's own default.
-         */
-        if (
-          params.gist_public !== undefined &&
-          params.gist_public !== null &&
-          params.gist_public !== ''
-        ) {
-          result.public = params.gist_public === true || params.gist_public === 'true'
+        for (const alias of GITHUB_PARAM_ALIASES) {
+          if (!alias.operations.includes(operation)) continue
+          const value = params[alias.from]
+          if (!isSet(value)) continue
+          result[alias.to] = alias.toBoolean ? value === true || value === 'true' : value
         }
 
         return result