Skip to content

Commit 2901b97

Browse files
committed
fix(github): wire the block fields whose ids do not match their tool params
1 parent c53837f commit 2901b97

2 files changed

Lines changed: 180 additions & 0 deletions

File tree

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
6+
// vitest.setup.ts stubs `@/tools/registry` to an empty map so unrelated suites
7+
// do not pay to load every tool. This suite asserts against the REAL tool
8+
// params, so it opts back in.
9+
vi.unmock('@/tools/registry')
10+
11+
import { GitHubBlock, GitHubV2Block } from '@/blocks/blocks/github'
12+
import { getTool } from '@/tools/utils'
13+
14+
function map(params: Record<string, unknown>): Record<string, unknown> {
15+
const fn = GitHubBlock.tools.config?.params
16+
if (!fn) throw new Error('GitHub block declares no params mapper')
17+
return fn(params) as Record<string, unknown>
18+
}
19+
20+
/**
21+
* Each pair is (block subBlock id, tool param name, tool id). The serializer
22+
* keys values by subBlock id, so without the mapper the tool param stays
23+
* undefined and the field is inert.
24+
*/
25+
const RENAMES = [
26+
['reaction_content', 'content', 'github_create_issue_reaction'],
27+
['reaction_content', 'content', 'github_create_comment_reaction'],
28+
['milestone_title', 'title', 'github_create_milestone'],
29+
['milestone_title', 'title', 'github_update_milestone'],
30+
['milestone_description', 'description', 'github_create_milestone'],
31+
['milestone_description', 'description', 'github_update_milestone'],
32+
['milestone_state', 'state', 'github_list_milestones'],
33+
['milestone_sort', 'sort', 'github_list_milestones'],
34+
['fork_name', 'name', 'github_fork_repo'],
35+
['fork_sort', 'sort', 'github_list_forks'],
36+
['gist_public', 'public', 'github_create_gist'],
37+
] as const
38+
39+
describe('every renamed subBlock reaches its tool param', () => {
40+
it.each(RENAMES)('%s -> %s', (subBlockId, paramName) => {
41+
expect(map({ [subBlockId]: 'x' })).toHaveProperty(paramName)
42+
})
43+
44+
it.each(RENAMES)(
45+
'%s targets a param the tool really declares (%s on %s)',
46+
(_s, paramName, toolId) => {
47+
const tool = getTool(toolId)
48+
expect(tool, `${toolId} is not registered`).toBeDefined()
49+
expect(Object.keys(tool!.params ?? {})).toContain(paramName)
50+
}
51+
)
52+
53+
it.each(RENAMES)('%s is not itself a param of %s', (subBlockId, _p, toolId) => {
54+
expect(Object.keys(getTool(toolId)!.params ?? {})).not.toContain(subBlockId)
55+
})
56+
})
57+
58+
/**
59+
* The mapper runs as the provider `paramsTransform` too, spreading over the
60+
* model's tool-call arguments. An unguarded assignment would overwrite a
61+
* model-supplied value with undefined — the agent path is the only path these
62+
* fields work on today, so it must not regress.
63+
*/
64+
describe('guarded assignment protects the agent tool-calling path', () => {
65+
it('emits nothing when no source field is present', () => {
66+
expect(map({ operation: 'github_create_milestone' })).toEqual({})
67+
})
68+
69+
it.each(RENAMES)('never writes %s target as undefined', (_s, paramName) => {
70+
expect(map({ operation: 'x' })).not.toHaveProperty(paramName)
71+
})
72+
73+
it('leaves a model-supplied value untouched when the block field is absent', () => {
74+
const modelArgs = { content: 'rocket', title: 'from the model' }
75+
expect({ ...modelArgs, ...map(modelArgs) }).toEqual(modelArgs)
76+
})
77+
78+
it.each(['', null, undefined])('treats %o as not provided', (empty) => {
79+
expect(map({ reaction_content: empty })).toEqual({})
80+
})
81+
})
82+
83+
describe('gist_public coercion', () => {
84+
it.each([
85+
['true', true],
86+
['false', false],
87+
[true, true],
88+
])('maps %o to %o', (input, expected) => {
89+
expect(map({ gist_public: input }).public).toBe(expected)
90+
})
91+
92+
it('omits public entirely when untouched, leaving the tool default', () => {
93+
expect(map({ operation: 'github_create_gist' })).not.toHaveProperty('public')
94+
})
95+
96+
it('matches the dropdown option ids the block actually renders', () => {
97+
const sub = GitHubBlock.subBlocks.find((s) => s.id === 'gist_public')
98+
expect(sub?.options).toBeDefined()
99+
const ids = (sub!.options as Array<{ id: string }>).map((o) => o.id)
100+
expect(ids).toEqual(['false', 'true'])
101+
})
102+
})
103+
104+
/** Sources that share a target are safe only if their conditions are disjoint. */
105+
describe('sources sharing a target param are condition-disjoint', () => {
106+
function opsFor(id: string): string[][] {
107+
return GitHubBlock.subBlocks
108+
.filter((s) => s.id === id)
109+
.map((s) => {
110+
const v = (s.condition as { value?: unknown })?.value
111+
return Array.isArray(v) ? (v as string[]) : [v as string]
112+
})
113+
}
114+
it.each([
115+
['sort', ['fork_sort', 'milestone_sort']],
116+
['title', ['milestone_title']],
117+
['description', ['milestone_description']],
118+
['state', ['milestone_state']],
119+
['content', ['reaction_content']],
120+
['name', ['fork_name']],
121+
['public', ['gist_public']],
122+
] as const)('%s sources never render together', (_target, sources) => {
123+
const seen = new Set<string>()
124+
for (const src of sources) {
125+
for (const group of opsFor(src)) {
126+
for (const op of group) {
127+
expect(seen.has(op), `${op} renders two sources of the same target`).toBe(false)
128+
seen.add(op)
129+
}
130+
}
131+
}
132+
expect(seen.size).toBeGreaterThan(0)
133+
})
134+
})
135+
136+
describe('the v2 block inherits the same mapper', () => {
137+
it('forwards params from the v1 block', () => {
138+
expect(GitHubV2Block.tools.config?.params).toBe(GitHubBlock.tools.config?.params)
139+
})
140+
})

‎apps/sim/blocks/blocks/github.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2279,6 +2279,46 @@ Return ONLY the timestamp string - no explanations, no quotes, no extra text.`,
22792279
return 'github_repo_info'
22802280
}
22812281
},
2282+
/**
2283+
* Bridges the subBlock ids that do not match their tool's param name.
2284+
*
2285+
* A tool param is populated only when a subBlock's `id` equals it — the
2286+
* serializer keys values by subBlock id, and nothing else renames them.
2287+
* Each field below renders, accepts input, and then arrives under a name
2288+
* its tool never reads.
2289+
*
2290+
* Every assignment is guarded, and that is load-bearing rather than
2291+
* defensive. `generic-handler.ts` merges `{ ...inputs, ...params(inputs) }`
2292+
* and `providers/utils.ts` installs this same function as the provider
2293+
* `paramsTransform`, spreading its result over the model's tool-call
2294+
* arguments. An unconditional write would therefore clobber a
2295+
* model-supplied `content`/`title`/`sort` with `undefined` on the agent
2296+
* tool-calling path — which is the one path these fields work on today.
2297+
*
2298+
* `sort` has two sources and `title`/`description`/`state` share their
2299+
* names with fields on other operations. That is safe only because each
2300+
* source subBlock's `condition` binds it to a single operation, so at
2301+
* most one source of a given target is ever present in `params`.
2302+
*/
2303+
params: (params) => {
2304+
const result: Record<string, unknown> = {}
2305+
2306+
if (params.reaction_content) result.content = params.reaction_content
2307+
if (params.milestone_title) result.title = params.milestone_title
2308+
if (params.milestone_description) result.description = params.milestone_description
2309+
if (params.milestone_state) result.state = params.milestone_state
2310+
if (params.milestone_sort) result.sort = params.milestone_sort
2311+
if (params.fork_name) result.name = params.fork_name
2312+
if (params.fork_sort) result.sort = params.fork_sort
2313+
2314+
// A dropdown stores its option id, so this arrives as the string
2315+
// 'true'/'false' while the tool declares `public` as a boolean.
2316+
if (params.gist_public) {
2317+
result.public = params.gist_public === true || params.gist_public === 'true'
2318+
}
2319+
2320+
return result
2321+
},
22822322
},
22832323
},
22842324
inputs: {

0 commit comments

Comments
 (0)