Skip to content

Commit eb1abb2

Browse files
committed
fix(langsmith,jira,linkedin): correct claims and coercion, align block outputs
The feedback description told the model the official SDK omits session_id. It does not -- it warns, and raises on SmithDB-only deployments. LangSmith documents the field as required while only 'key' sits in the OpenAPI required array; both halves are now stated. parseLangsmithFeedbackValue was applied in the block and again in request.body, and it is not idempotent: a quoted "1" became a number and a quoted "null" dropped the field. The tool body covers every path including Copilot, so the block call is removed. Jira declared commentBody and newStatus, which no tool emits, while the real keys were undeclared; status and assignee are objects declared as string. LinkedIn's postUrl doc named a URN family the regex never checks.
1 parent 161a2b0 commit eb1abb2

10 files changed

Lines changed: 193 additions & 37 deletions

File tree

apps/sim/blocks/blocks/jira.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,15 +1412,28 @@ Return ONLY the comment text - no explanations.`,
14121412
outputs: {
14131413
// Common outputs across all Jira operations
14141414
ts: { type: 'string', description: 'Timestamp of the operation' },
1415+
success: { type: 'boolean', description: 'Whether the operation succeeded' },
14151416

1416-
// jira_retrieve (read) outputs
1417+
// jira_retrieve (read) outputs — the transformed issue is spread at the top
1418+
// level, so every field below sits alongside the raw `issue` object
14171419
issueKey: { type: 'string', description: 'Issue key (e.g., PROJ-123)' },
14181420
summary: { type: 'string', description: 'Issue summary/title' },
14191421
description: { type: 'string', description: 'Issue description content' },
14201422
created: { type: 'string', description: 'Issue creation date' },
14211423
updated: { type: 'string', description: 'Issue last update date' },
1422-
status: { type: 'string', description: 'Issue status name' },
1423-
assignee: { type: 'string', description: 'Issue assignee display name or account ID' },
1424+
status: {
1425+
type: 'json',
1426+
description: 'Issue status object with id, name, description, and statusCategory',
1427+
},
1428+
statusName: { type: 'string', description: 'Issue status name (e.g., Open, In Progress)' },
1429+
assignee: {
1430+
type: 'json',
1431+
description: 'Assigned user object with accountId, displayName, and emailAddress',
1432+
},
1433+
assigneeName: {
1434+
type: 'string',
1435+
description: 'Assignee display name or account ID',
1436+
},
14241437

14251438
// jira_write (create) outputs
14261439
url: { type: 'string', description: 'URL to the created/accessed issue' },
@@ -1447,8 +1460,11 @@ Return ONLY the comment text - no explanations.`,
14471460

14481461
// jira_add_comment, jira_update_comment outputs
14491462
commentId: { type: 'string', description: 'Comment ID' },
1450-
commentBody: { type: 'string', description: 'Comment text content' },
1451-
author: { type: 'string', description: 'Comment author display name' },
1463+
body: { type: 'string', description: 'Comment text content' },
1464+
author: {
1465+
type: 'json',
1466+
description: 'Comment author object with accountId, displayName, and emailAddress',
1467+
},
14521468

14531469
// jira_get_attachments outputs
14541470
attachments: {
@@ -1478,7 +1494,11 @@ Return ONLY the comment text - no explanations.`,
14781494

14791495
// jira_transition_issue outputs
14801496
transitionId: { type: 'string', description: 'Applied transition ID' },
1481-
newStatus: { type: 'string', description: 'New status after transition' },
1497+
transitionName: { type: 'string', description: 'Applied transition name' },
1498+
toStatus: {
1499+
type: 'json',
1500+
description: 'Target status after transition, with id and name',
1501+
},
14821502

14831503
// jira_create_issue_link outputs
14841504
linkId: { type: 'string', description: 'Created link ID' },

apps/sim/blocks/blocks/langsmith.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { toError } from '@sim/utils/errors'
22
import { LangsmithIcon } from '@/components/icons'
33
import { AuthMode, type BlockConfig, type BlockMeta, IntegrationType } from '@/blocks/types'
44
import type { LangsmithResponse } from '@/tools/langsmith/types'
5-
import { parseLangsmithFeedbackValue } from '@/tools/langsmith/utils'
65

76
export const LangsmithBlock: BlockConfig<LangsmithResponse> = {
87
type: 'langsmith',
@@ -421,7 +420,7 @@ Common patch fields: outputs, end_time, status, error`,
421420
key: params.key,
422421
sessionId: params.feedback_session_id || params.session_id,
423422
score: parseScore(params.score),
424-
value: parseLangsmithFeedbackValue(params.value),
423+
value: params.value,
425424
comment: params.comment,
426425
correction: parseJsonValue(params.correction, 'correction'),
427426
feedbackSourceType: params.feedbackSourceType || undefined,
@@ -476,7 +475,7 @@ Common patch fields: outputs, end_time, status, error`,
476475
feedback_session_id: {
477476
type: 'string',
478477
description:
479-
'UUID of the tracing project (session) the feedback belongs to. Optional — LangSmith accepts feedback without it',
478+
"UUID of the tracing project (session) the feedback belongs to. LangSmith documents it as required and its SDK warns (or errors, on SmithDB-only deployments) without it. Left optional here so blocks saved against deployments that still resolve the run server-side keep working.",
480479
},
481480
session_name: { type: 'string', description: 'Session name' },
482481
status: { type: 'string', description: 'Run status' },
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { JiraBlock } from '@/blocks/blocks/jira'
6+
import * as jiraTools from '@/tools/jira'
7+
import type { ToolConfig } from '@/tools/types'
8+
9+
/**
10+
* Output names the block declares for its webhook trigger payload rather than
11+
* for a tool result. No Jira tool emits these, by design.
12+
*/
13+
const TRIGGER_ONLY_OUTPUTS = new Set([
14+
'event_type',
15+
'issue_id',
16+
'issue_key',
17+
'project_key',
18+
'project_name',
19+
'issue_type_name',
20+
'priority_name',
21+
'status_name',
22+
'assignee_name',
23+
'assignee_email',
24+
'reporter_name',
25+
'reporter_email',
26+
'comment_id',
27+
'comment_body',
28+
'worklog_id',
29+
'time_spent',
30+
'changelog',
31+
'sprint',
32+
'version',
33+
'jira',
34+
'user',
35+
'webhook',
36+
])
37+
38+
const toolOutputNames = new Set<string>()
39+
for (const tool of Object.values(jiraTools) as ToolConfig[]) {
40+
if (!tool?.id?.startsWith('jira_')) continue
41+
for (const name of Object.keys(tool.outputs ?? {})) toolOutputNames.add(name)
42+
}
43+
44+
const declaredOutputs = Object.keys(JiraBlock.outputs ?? {})
45+
46+
describe('jira block outputs match what the tools actually emit', () => {
47+
it('reads a non-trivial set of tool outputs', () => {
48+
expect(toolOutputNames.size).toBeGreaterThan(20)
49+
})
50+
51+
it('declares no phantom output that no Jira tool ever emits', () => {
52+
const phantoms = declaredOutputs.filter(
53+
(name) => !TRIGGER_ONLY_OUTPUTS.has(name) && !toolOutputNames.has(name)
54+
)
55+
expect(phantoms).toEqual([])
56+
})
57+
58+
it.each(['body', 'toStatus', 'transitionName', 'success', 'statusName', 'assigneeName'])(
59+
'declares %s, which Jira tools emit',
60+
(name: string) => {
61+
expect(toolOutputNames.has(name)).toBe(true)
62+
expect(declaredOutputs).toContain(name)
63+
}
64+
)
65+
})

apps/sim/tools/langsmith/create_feedback.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const langsmithCreateFeedbackTool: ToolConfig<
3838
required: false,
3939
visibility: 'user-or-llm',
4040
description:
41-
'UUID of the tracing project (session) the feedback belongs to. Optional — in the LangSmith OpenAPI spec only `key` is required by POST /api/v1/feedback, and the official SDK sends feedback without a session_id. Supply it when you have it: some LangSmith deployments cannot locate the run without it. Never guess it — use the sessionId reported by a preceding langsmith_get_run for the same run, or look the project up with GET /api/v1/sessions.',
41+
'UUID of the tracing project (session) the feedback belongs to. LangSmith documents this as required: "POST /api/v1/feedback now requires a session_id field in the request body. It was previously optional." The official SDK warns when it is omitted and raises outright on SmithDB-only deployments, because some LangSmith deployments cannot locate the run without it. Only `key` is in the endpoint\'s OpenAPI required array, so omitting session_id will not fail request validation — but always supply it when you have it. Never guess it — use the sessionId reported by a preceding langsmith_get_run for the same run, or look the project up with GET /api/v1/sessions.',
4242
},
4343
key: {
4444
type: 'string',

apps/sim/tools/langsmith/langsmith.test.ts

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -549,23 +549,29 @@ describe('langsmith feedback session_id documentation', () => {
549549
})
550550
})
551551

552-
describe('langsmith feedback session_id is not enforced by LangSmith', () => {
552+
describe('langsmith feedback session_id documents both halves of the truth', () => {
553553
const toolDescription = langsmithCreateFeedbackTool.params.sessionId.description ?? ''
554554
const blockDescription = LangsmithBlock.inputs?.feedback_session_id?.description ?? ''
555555
const feedbackSessionSubBlock = LangsmithBlock.subBlocks.find(
556556
(subBlock) => subBlock.id === 'feedback_session_id'
557557
)
558558

559-
it('does not tell the model the OpenAPI spec marks session_id as required', () => {
560-
expect(toolDescription).not.toMatch(/documents it as required|session_id is required/i)
559+
it('never tells the model the SDK omits session_id, which is the failing shape', () => {
560+
expect(toolDescription).not.toMatch(/sends feedback without|sdk (?:omits|does not send)/i)
561561
})
562562

563-
it('states that only key is required by POST /api/v1/feedback', () => {
564-
expect(toolDescription).toMatch(/only `?key`? is required/i)
563+
it('says LangSmith documents session_id as required', () => {
564+
expect(toolDescription).toMatch(/documents (?:this|it) as required/i)
565+
expect(toolDescription).toMatch(/warns|raises/i)
565566
})
566567

567-
it('does not claim in the block inputs that LangSmith requires it', () => {
568-
expect(blockDescription).not.toMatch(/required by langsmith/i)
568+
it('still explains that only key is in the OpenAPI required array', () => {
569+
expect(toolDescription).toMatch(/only `?key`? is in the .*required array/i)
570+
})
571+
572+
it('does not claim in the block inputs that LangSmith accepts feedback without it', () => {
573+
expect(blockDescription).not.toMatch(/accepts feedback without it/i)
574+
expect(blockDescription).toMatch(/documents it as required/i)
569575
})
570576

571577
it('leaves the feedback Session ID subBlock optional so saved blocks keep validating', () => {
@@ -605,3 +611,51 @@ describe('langsmith batch post/patch shape guard', () => {
605611
expect((body.post as Record<string, unknown>[])[0]).toMatchObject({ name: 'run-a' })
606612
})
607613
})
614+
615+
describe('langsmith feedback value is coerced exactly once', () => {
616+
const mapBlockParams = (value: string): Record<string, unknown> =>
617+
(
618+
LangsmithBlock.tools.config!.params as (p: Record<string, unknown>) => Record<string, unknown>
619+
)({
620+
operation: 'langsmith_create_feedback',
621+
apiKey: 'test-key',
622+
runId: 'run-1',
623+
key: 'correctness',
624+
value,
625+
})
626+
627+
const bodyFromBlock = (value: string): Record<string, unknown> =>
628+
resolveBody(
629+
langsmithCreateFeedbackTool as never,
630+
mapBlockParams(value) as unknown as LangsmithCreateFeedbackParams
631+
)
632+
633+
it.each([
634+
['"1"', '1'],
635+
['"true"', 'true'],
636+
['"null"', 'null'],
637+
])(
638+
'leaves a JSON-quoted %s untouched in the block param mapper',
639+
(typed: string, _expected: string) => {
640+
expect(mapBlockParams(typed).value).toBe(typed)
641+
}
642+
)
643+
644+
it.each([
645+
['"1"', '1'],
646+
['"true"', 'true'],
647+
['"null"', 'null'],
648+
])(
649+
'keeps a JSON-quoted %s a string on the wire instead of double-coercing it',
650+
(typed: string, expected: string) => {
651+
const body = bodyFromBlock(typed)
652+
expect(body).toHaveProperty('value')
653+
expect(body.value).toBe(expected)
654+
}
655+
)
656+
657+
it('still coerces an unquoted numeric value exactly once through the block path', () => {
658+
expect(bodyFromBlock('1').value).toBe(1)
659+
expect(bodyFromBlock('true').value).toBe(true)
660+
})
661+
})

apps/sim/tools/langsmith/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ export interface LangsmithCreateFeedbackParams {
114114
key: string
115115
score?: number
116116
/**
117-
* Declared to the model as `type: 'string'`, but the block's param mapper may
118-
* have already coerced it with `parseLangsmithFeedbackValue`, so the runtime
119-
* value is any member of the union LangSmith stores — never `null`, which the
117+
* Declared to the model as `type: 'string'` and handed to this tool as one:
118+
* `parseLangsmithFeedbackValue` runs once, inside `request.body`, widening it
119+
* to any member of the union LangSmith stores — never `null`, which the
120120
* parser drops. The response echoes the full union, `null` included.
121121
*/
122122
value?: Exclude<LangsmithFeedbackValue, null>

apps/sim/tools/langsmith/utils.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ export type LangsmithFeedbackValue = string | number | boolean | Record<string,
4747
* throws and the `catch` returns the original string.
4848
* - An array literal stays a string, since the schema has no array member.
4949
*
50-
* Idempotent: a value already coerced by the block layer is returned unchanged.
50+
* Not idempotent, and deliberately called exactly once — from
51+
* {@link file://./create_feedback.ts}'s `request.body`, which every surface
52+
* (block, model tool call, Copilot) routes through. Applying it twice would
53+
* unwrap a JSON-quoted string a second time, which is precisely how a user
54+
* forces a categorical label that looks scalar: `"1"` would become the number
55+
* `1`, `"true"` the boolean `true`, and `"null"` would be dropped entirely.
5156
*/
5257
export const parseLangsmithFeedbackValue = (value: unknown): LangsmithFeedbackValue | undefined => {
5358
if (value === undefined || value === null) return undefined

apps/sim/tools/linkedin/share_post.test.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,15 +144,24 @@ describe('linkedInSharePostTool.outputs', () => {
144144
expect(Object.keys(linkedInSharePostTool.outputs ?? {}).sort()).toEqual(['postId', 'postUrl'])
145145
})
146146

147-
it('names the ugcPost URN family that /v2/ugcPosts actually returns', () => {
148-
const descriptions = [
149-
linkedInSharePostTool.outputs?.postId?.description ?? '',
150-
linkedInSharePostTool.outputs?.postUrl?.description ?? '',
151-
]
152-
153-
for (const description of descriptions) {
154-
expect(description).toContain('ugcPost')
155-
expect(description).not.toMatch(/\bshare\b/i)
156-
}
147+
it('attributes the ugcPost id only to what LinkedIn documents: the create header', () => {
148+
const postId = linkedInSharePostTool.outputs?.postId?.description ?? ''
149+
150+
expect(postId).toContain('x-restli-id')
151+
expect(postId).toContain('ugcPost')
152+
expect(postId).not.toMatch(/never|always/i)
153+
})
154+
155+
it('does not promise postUrl is gated on the ugcPost family the regex never checks', () => {
156+
const postUrl = linkedInSharePostTool.outputs?.postUrl?.description ?? ''
157+
158+
expect(postUrl).not.toMatch(/ugcPost family/i)
159+
expect(postUrl).toContain('urn:li:')
160+
})
161+
162+
it('does not claim postUrl is absent only when postId is', () => {
163+
const postUrl = linkedInSharePostTool.outputs?.postUrl?.description ?? ''
164+
165+
expect(postUrl).toMatch(/or did not carry/i)
157166
})
158167
})

apps/sim/tools/linkedin/share_post.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,13 @@ export const linkedInSharePostTool: ToolConfig<SharePostParams, SharePostRespons
176176
postId: {
177177
type: 'string',
178178
description:
179-
'The `urn:li:ugcPost:` URN of the created post, read from the `x-restli-id` response header. Absent when LinkedIn omits that header.',
179+
'The URN of the created post, read from the `x-restli-id` response header, which LinkedIn documents as carrying the ugcPost id. Reported as received. Absent when LinkedIn omits that header.',
180180
optional: true,
181181
},
182182
postUrl: {
183183
type: 'string',
184184
description:
185-
'LinkedIn URL of the created post. Viewable by an authorized LinkedIn member — not a guaranteed public permalink. Absent when the `x-restli-id` header was missing or did not carry a `urn:li:` URN of the ugcPost family.',
185+
'LinkedIn URL of the created post. Viewable by an authorized LinkedIn member — not a guaranteed public permalink. Absent when the `x-restli-id` header was missing or did not carry a `urn:li:<entityType>:<id>` URN.',
186186
optional: true,
187187
},
188188
},

apps/sim/tools/linkedin/types.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,19 @@ export interface SharePostParams {
4343
export interface SharePostResponse extends ToolResponse {
4444
output: {
4545
/**
46-
* The `urn:li:ugcPost:` URN of the created post, from the `x-restli-id` response header;
47-
* absent if LinkedIn omits the header. `/v2/ugcPosts` returns a ugcPost URN, never a
48-
* `urn:li:share:` URN from the legacy Shares API.
46+
* The URN of the created post, from the `x-restli-id` response header; absent if LinkedIn
47+
* omits the header. LinkedIn documents the create response's `x-restli-id` as carrying the
48+
* ugcPost id. It is not a guarantee about every id `/v2/ugcPosts` ever hands back: the
49+
* finder examples in the same reference return `urn:li:share:` ids for existing posts, so
50+
* the value is reported as received rather than assumed to be a `urn:li:ugcPost:` URN.
4951
*/
5052
postId?: string
5153
/**
5254
* LinkedIn's `feed/update/<urn>` permalink for the created post. LinkedIn documents this URL
5355
* as viewable by an authorized member, so it is not guaranteed to resolve for the public or
54-
* for signed-out visitors. Absent whenever {@link SharePostResponse.output.postId} is.
56+
* for signed-out visitors. Absent whenever {@link SharePostResponse.output.postId} is, and
57+
* also when `postId` is present but is not shaped like a `urn:li:` URN — the header is
58+
* server-controlled, so it is only interpolated into a URL after that check.
5559
*/
5660
postUrl?: string
5761
}

0 commit comments

Comments
 (0)