-
Notifications
You must be signed in to change notification settings - Fork 3.3k
v0.5.86: server side copilot, copilot mcp, error notifications, jira outputs destructuring, slack trigger improvements #3178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
waleedlatif1
commented
Feb 10, 2026
- fix(notifications): throw notification on runtime errors, move predeploy checks to update in deploy modal (fix(notifications): throw notification on runtime errors, move predeploy checks to update in deploy modal #3172)
- improvement(jsm): destructured outputs for jsm, jira, and added 1password integration (improvement(jsm): destructured outputs for jsm, jira, and added 1password integration #3174)
- fix(slack): resolve file metadata via files.info when event payload is partial (fix(slack): resolve file metadata via files.info when event payload is partial #3176)
- feat(copilot): copilot mcp + server side copilot execution (feat(copilot): copilot mcp + server side copilot execution #3173)
- fix(mcp): harden notification system against race conditions (fix(mcp): harden notification system against race conditions #3168)
- improvement(preview): added trigger mode context for deploy preview (improvement(preview): added trigger mode context for deploy preview #3177)
fix(notifications): throw notification on runtime errors, move predeploy checks to update in deploy modal
…word integration (#3174) * improvement(jsm): destructured outputs for jsm, jira, and added 1password integration * update 1password to support cloud & locally hosted * updated & tested 1pass * added an additional wandConfig for OnePassword & jira search issues * finished jira * removed unused route * updated types * restore old outputs * updated types
* v0 * v1 * Basic ss tes * Ss tests * Stuff * Add mcp * mcp v1 * Improvement * Fix * BROKEN * Checkpoint * Streaming * Fix abort * Things are broken * Streaming seems to work but copilot is dumb * Fix edge issue * LUAAAA * Fix stream buffer * Fix lint * Checkpoint * Initial temp state, in the middle of a refactor * Initial test shows diff store still working * Tool refactor * First cleanup pass complete - untested * Continued cleanup * Refactor * Refactor complete - no testing yet * Fix - cursor makes me sad * Fix mcp * Clean up mcp * Updated mcp * Add respond to subagents * Fix definitions * Add tools * Add tools * Add copilot mcp tracking * Fix lint * Fix mcp * Fix * Updates * Clean up mcp * Fix copilot mcp tool names to be sim prefixed * Add opus 4.6 * Fix discovery tool * Fix * Remove logs * Fix go side tool rendering * Update docs * Fix hydration * Fix tool call resolution * Fix * Fix lint * Fix superagent and autoallow integrations * Fix always allow * Update block * Remove plan docs * Fix hardcoded ff * Fix dropped provider * Fix lint * Fix tests * Fix dead messages array * Fix discovery * Fix run workflow * Fix run block * Fix run from block in copilot * Fix lint * Fix skip and mtb * Fix typing * Fix tool call * Bump api version * Fix bun lock * Nuke bad files
* fix(mcp): harden notification system against race conditions - Guard concurrent connect() calls in connection manager with connectingServers Set - Suppress post-disconnect notification handler firing in MCP client - Clean up Redis event listeners in pub/sub dispose() - Add tests for all three hardening fixes (11 new tests) * updated tests * plugged in new mcp event based system and create sse route to publish notifs * ack commetns * fix reconnect timer * cleanup when running onClose * fixed spacing on mcp settings tab * keep error listeners before quiet in redis
…3177) * improvement(preview): added trigger mode context for deploy preview * use existing helper * enhance disabled mode for subblocks * update * update all subblocks to allow scrolling in read only mode * updated short and long input to match others, reverted triggerutils change
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Too many files changed for review. ( |
| const response = await fetch(url, { | ||
| method: 'GET', | ||
| headers: getJsmHeaders(accessToken), | ||
| }) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 10 hours ago
In general, to fix this kind of issue you must ensure that any user-influenced data used in an outbound request URL is (a) strictly validated against an allow‑list of safe characters and patterns, and (b) treated as a path segment (or query parameter) using proper encoding rather than raw string concatenation. Hostname and scheme should never come directly from user input.
In this file, the best minimal fix is to normalize and encode the serviceDeskId and requestTypeId before building the URL, using a conservative allow‑list at the call site. That way, even if validateAlphanumericId were relaxed in the future, the URL construction here would still treat these values as opaque path segments and disallow characters that could alter the URL structure. We can do this by adding a small local helper (e.g. sanitizePathSegment) that enforces a strict regex (letters, digits, hyphen, underscore) and then applies encodeURIComponent, and then use that sanitized form in the url string. This does not change intended functionality if the IDs are already alphanumeric, but it guarantees that malformed input cannot introduce /, .., ?, #, : or similar into the path.
Concretely in apps/sim/app/api/tools/jsm/requesttypefields/route.ts:
- Define a local
sanitizePathSegmentfunction near the top of the file (beforePOST) that:- Checks the value is a non-empty string.
- Verifies it matches
/^[A-Za-z0-9_-]+$/. - Throws an error (or returns a safe default) if invalid.
- Returns
encodeURIComponent(value).
- In
POST, after the existing validations onserviceDeskIdandrequestTypeId, computeconst safeServiceDeskId = sanitizePathSegment(serviceDeskId)andconst safeRequestTypeId = sanitizePathSegment(requestTypeId). - Use these safe variables when building
urlinstead of the raw inputs.
We then slightly adjust error handling to convert any thrown error from sanitizePathSegment into a 400 JSON response. No new imports are required; we rely on built‑in encodeURIComponent.
-
Copy modified lines R11-R25 -
Copy modified lines R73-R83 -
Copy modified line R85
| @@ -8,6 +8,21 @@ | ||
|
|
||
| const logger = createLogger('JsmRequestTypeFieldsAPI') | ||
|
|
||
| function sanitizePathSegment(value: unknown, fieldName: string): string { | ||
| if (typeof value !== 'string' || value.length === 0) { | ||
| throw new Error(`${fieldName} must be a non-empty string`) | ||
| } | ||
|
|
||
| // Strict allow-list: prevent path traversal or URL-structure injection. | ||
| if (!/^[A-Za-z0-9_-]+$/.test(value)) { | ||
| throw new Error( | ||
| `${fieldName} contains invalid characters. Only letters, numbers, hyphen and underscore are allowed.` | ||
| ) | ||
| } | ||
|
|
||
| return encodeURIComponent(value) | ||
| } | ||
|
|
||
| export async function POST(request: NextRequest) { | ||
| const auth = await checkInternalAuth(request) | ||
| if (!auth.success || !auth.userId) { | ||
| @@ -55,8 +70,19 @@ | ||
| return NextResponse.json({ error: requestTypeIdValidation.error }, { status: 400 }) | ||
| } | ||
|
|
||
| let safeServiceDeskId: string | ||
| let safeRequestTypeId: string | ||
| try { | ||
| safeServiceDeskId = sanitizePathSegment(serviceDeskId, 'serviceDeskId') | ||
| safeRequestTypeId = sanitizePathSegment(requestTypeId, 'requestTypeId') | ||
| } catch (e: unknown) { | ||
| const message = e instanceof Error ? e.message : 'Invalid path parameter' | ||
| logger.error('Invalid path parameter for JSM request type fields:', { error: message }) | ||
| return NextResponse.json({ error: message }, { status: 400 }) | ||
| } | ||
|
|
||
| const baseUrl = getJsmApiBaseUrl(cloudId) | ||
| const url = `${baseUrl}/servicedesk/${serviceDeskId}/requesttype/${requestTypeId}/field` | ||
| const url = `${baseUrl}/servicedesk/${safeServiceDeskId}/requesttype/${safeRequestTypeId}/field` | ||
|
|
||
| logger.info('Fetching request type fields from:', url) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 6 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ...(contexts && contexts.length > 0 ? { context: contexts } : {}), | ||
| ...(chatId ? { chatId } : {}), | ||
| ...(processedFileContents.length > 0 ? { fileAttachments: processedFileContents } : {}), | ||
| ...(integrationTools.length > 0 ? { integrationTools } : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Payload key renamed from tools to integrationTools
High Severity
The old code sent tool definitions to the Go backend under the key tools (via { tools: integrationTools }), but buildCopilotRequestPayload uses JavaScript shorthand property syntax { integrationTools }, which sends them under the key integrationTools instead. The chat route's logging still references requestPayload.tools (which will always be undefined), confirming this rename was unintentional. The Go copilot backend likely won't find any tools in the payload.
Additional Locations (1)
| ...(integrationTools.length > 0 ? { integrationTools } : {}), | ||
| ...(credentials ? { credentials } : {}), | ||
| ...(commands && commands.length > 0 ? { commands } : {}), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing conversationId in request payload breaks multi-turn chat
High Severity
The old code included conversationId in the payload sent to the Go backend (via ...(effectiveConversationId ? { conversationId: effectiveConversationId } : {})). The chat route still computes effectiveConversationId but never passes it to buildCopilotRequestPayload, and the function doesn't accept or include it. Without this, the Go backend can't resume prior conversation state on subsequent messages, breaking multi-turn chat continuity.
Additional Locations (1)
| fileAttachments, | ||
| commands, | ||
| chatId, | ||
| } = params |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accepted implicitFeedback parameter silently dropped from payload
Medium Severity
buildCopilotRequestPayload accepts implicitFeedback in its BuildPayloadParams interface but never destructures or uses it. The old code injected implicit feedback as a system message into the conversation. Now, any implicit feedback provided by the client is silently ignored, potentially degrading response quality without any error.
| ...(integrationTools.length > 0 ? { integrationTools } : {}), | ||
| ...(credentials ? { credentials } : {}), | ||
| ...(commands && commands.length > 0 ? { commands } : {}), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing stream flag in main orchestrator payload
Medium Severity
The old payload included stream: true and streamToolCalls: true when sending to the Go backend. buildCopilotRequestPayload omits both fields. Notably, orchestrateSubagentStream explicitly adds stream: true to its payload, but the main orchestrateCopilotStream does not. If the Go backend checks this field to decide its response format, the SSE parser in runStreamLoop will fail on a non-streaming response.
| provider: 'vertex', | ||
| model: envModel, | ||
| apiKey: env.COPILOT_API_KEY, | ||
| vertexProject: env.VERTEX_PROJECT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Title generation race can push to closed stream
Medium Severity
The generateChatTitle promise fires asynchronously (.then(...)) and calls pushEvent after the title resolves. However, if the orchestrator finishes and the finally block calls controller.close() before the title promise resolves, the pushEvent call will attempt to enqueue on a closed controller. The old code had a try-catch around controller.enqueue, but pushEvent's catch only sets clientDisconnected — the controller.close() in finally no longer has a try-catch wrapper either, compounding this race.
| headless: true, | ||
| chatId, | ||
| source: 'mcp', | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MCP build bypasses tool/credential loading for Go backend
Medium Severity
The handleBuildToolCall in the MCP copilot route constructs a request payload for orchestrateCopilotStream without including integration tool definitions or user credentials. The main chat route uses buildCopilotRequestPayload which fetches OAuth tokens and tool schemas for build mode. The MCP build path skips this entirely, so the Go backend won't have access to available tool schemas or user credentials needed to execute integration tools.

