Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(mcp): route object params to JSON editor, keep invalid drafts out of tool args#5570
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
09ab34b
fix(mcp): route object-typed params to JSON editor, keep invalid draf…
waleedlatif1 0f7b3a5
fix(mcp): reset invalid JSON drafts on schema change, not just tool c…
waleedlatif1 fda94bc
fix(mcp): key draft reset off both schema sources, not the resolved t…
waleedlatif1 5a0466d
fix(mcp): sign whole schema for draft reset; invalidate drafts on ext…
waleedlatif1 261f296
fix(mcp): restore comma-separated array input; stop spurious draft re…
waleedlatif1 72bdb1f
fix(mcp): reset drafts on genuine live schema refresh, not just its f…
waleedlatif1 16e8021
fix(mcp): compare live schema against last-known-non-empty, not prior…
waleedlatif1 9cd9120
fix(mcp): re-baseline live schema tracker fresh on every tool switch
waleedlatif1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
112 changes: 107 additions & 5 deletions
112 ...l/components/editor/components/sub-block/components/mcp-dynamic-args/mcp-dynamic-args.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import { useCallback } from 'react' | ||
| import { useCallback, useState } from 'react' | ||
| import { Combobox, FieldDivider, Label, Slider, Switch } from '@sim/emcn' | ||
| import { createLogger } from '@sim/logger' | ||
| import { useParams } from 'next/navigation' | ||
| @@ -42,6 +42,27 @@ function requiresJsonValue(paramSchema: any): boolean { | ||
| ) | ||
| } | ||
| /** | ||
| * Stable signature of an entire tool schema, for detecting whether the effective | ||
| * param shape has changed (independent of object identity). Signs the whole schema | ||
| * rather than cherry-picking fields (e.g. just `properties`) so a refresh that only | ||
| * changes `required`, or any other schema-level field, isn't silently missed. | ||
| */ | ||
| function schemaSignature(schema: unknown): string { | ||
| return schema ? JSON.stringify(schema) : '' | ||
| } | ||
| /** | ||
| * True when text looks like an attempted JSON array/object literal (starts with `[` | ||
| * or `{`), as opposed to plain freeform text. Used to tell an in-progress, incomplete | ||
| * JSON literal (which must not persist until valid — see `requiresJsonValue`) apart | ||
| * from the comma-separated/plain-text shorthand array params also accept. | ||
| */ | ||
| function looksLikeJsonLiteral(text: string): boolean { | ||
| const trimmed = text.trim() | ||
| return trimmed.startsWith('[') || trimmed.startsWith('{') | ||
| } | ||
| interface McpDynamicArgsProps { | ||
| blockId: string | ||
| subBlockId: string | ||
| @@ -97,6 +118,58 @@ export function McpDynamicArgs({ | ||
| const selectedToolConfig = mcpTools.find((tool) => tool.id === selectedTool) | ||
| const toolSchema = cachedSchema || selectedToolConfig?.inputSchema | ||
| /** | ||
| * Draft text for JSON-value params (object/array/non-primitive-enum) whose current | ||
| * edit isn't valid JSON yet, paired with a signature of the persisted value it was | ||
| * typed against. Keeping this out of toolArgs means the stored argument is always | ||
| * either the last valid parsed value or untouched — never malformed text that could | ||
| * reach tool execution. A draft is only displayed while its baseline still matches | ||
| * the live persisted value, so an external change to that value (undo/redo, a diff | ||
| * baseline switch, a collaborator's edit) can't be shadowed by stale draft text. | ||
| * Drafts also reset wholesale on either of two independent triggers: | ||
| * - the selected tool or the cached `_toolSchema` snapshot changes (this pair | ||
| * always drives `toolSchema` whenever a cached snapshot exists) — the live | ||
| * schema tracker is also re-baselined to the new tool's current signature | ||
| * here (even if still empty), so a tool switch never leaves the *previous* | ||
| * tool's signature behind to be misread as a "refresh" once the new tool's | ||
| * schema loads a moment later, or | ||
| * - for the *same* tool, the live discovered schema's signature changes to a | ||
| * different non-empty value than the last non-empty value actually observed | ||
| * — a genuine re-discovery/refresh. Comparing against the last known | ||
| * non-empty value (rather than merely the previous render's value) means a | ||
| * schema that transiently disappears and reappears — e.g. `mcpTools` | ||
| * refetching — still resets drafts if it comes back different, while a | ||
| * plain empty → non-empty transition (the initial async load) does not, | ||
| * since there is no prior non-empty value for this tool to compare against. | ||
| */ | ||
| const [invalidJsonDrafts, setInvalidJsonDrafts] = useState< | ||
| Record<string, { text: string; baseline: string }> | ||
| >({}) | ||
| const toolAndCachedSchemaKey = `${selectedTool ?? ''}|${schemaSignature(cachedSchema)}` | ||
| const liveSchemaSignature = schemaSignature(selectedToolConfig?.inputSchema) | ||
| const [prevToolAndCachedSchemaKey, setPrevToolAndCachedSchemaKey] = | ||
| useState(toolAndCachedSchemaKey) | ||
| const [lastNonEmptyLiveSchemaSignature, setLastNonEmptyLiveSchemaSignature] = | ||
| useState(liveSchemaSignature) | ||
| if (prevToolAndCachedSchemaKey !== toolAndCachedSchemaKey) { | ||
| setInvalidJsonDrafts({}) | ||
| setPrevToolAndCachedSchemaKey(toolAndCachedSchemaKey) | ||
| setLastNonEmptyLiveSchemaSignature(liveSchemaSignature) | ||
| } else { | ||
| const nextLastNonEmptyLiveSchemaSignature = | ||
| liveSchemaSignature !== '' ? liveSchemaSignature : lastNonEmptyLiveSchemaSignature | ||
| if (nextLastNonEmptyLiveSchemaSignature !== lastNonEmptyLiveSchemaSignature) { | ||
| const isGenuineLiveRefresh = | ||
| liveSchemaSignature !== '' && | ||
| lastNonEmptyLiveSchemaSignature !== '' && | ||
| liveSchemaSignature !== lastNonEmptyLiveSchemaSignature | ||
| if (isGenuineLiveRefresh) { | ||
| setInvalidJsonDrafts({}) | ||
| } | ||
| setLastNonEmptyLiveSchemaSignature(nextLastNonEmptyLiveSchemaSignature) | ||
| } | ||
| } | ||
| const currentArgs = useCallback(() => { | ||
| if (isPreview && previewValue) { | ||
| if (typeof previewValue === 'string') { | ||
| @@ -158,7 +231,7 @@ export function McpDynamicArgs({ | ||
| if (paramSchema.maxLength && paramSchema.maxLength > 100) return 'long-input' | ||
| return 'short-input' | ||
| } | ||
| if (paramSchema.type === 'array') return 'long-input' | ||
| if (paramSchema.type === 'array' || paramSchema.type === 'object') return 'long-input' | ||
| return 'short-input' | ||
| } | ||
| @@ -270,8 +343,17 @@ export function McpDynamicArgs({ | ||
| case 'long-input': { | ||
| const config = createParamConfig(paramName, paramSchema, 'long-input') | ||
| const needsJsonValue = requiresJsonValue(paramSchema) | ||
| const valueSignature = JSON.stringify(value ?? null) | ||
| const draft = invalidJsonDrafts[paramName] | ||
| const activeDraft = | ||
| needsJsonValue && draft && draft.baseline === valueSignature ? draft.text : undefined | ||
| const displayValue = | ||
| typeof value === 'string' || value == null ? value || '' : JSON.stringify(value) | ||
| activeDraft !== undefined | ||
| ? activeDraft | ||
| : typeof value === 'string' || value == null | ||
| ? value || '' | ||
| : JSON.stringify(value) | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return ( | ||
| <LongInput | ||
| key={`${paramName}-long`} | ||
| @@ -282,14 +364,34 @@ export function McpDynamicArgs({ | ||
| rows={4} | ||
| value={displayValue} | ||
| onChange={(newValue) => { | ||
| if (!requiresJsonValue(paramSchema)) { | ||
| if (!needsJsonValue) { | ||
| updateParameter(paramName, newValue) | ||
| return | ||
| } | ||
| const clearDraft = () => | ||
| setInvalidJsonDrafts((prev) => { | ||
| if (!(paramName in prev)) return prev | ||
| const { [paramName]: _removed, ...rest } = prev | ||
| return rest | ||
| }) | ||
| if (newValue === '') { | ||
| updateParameter(paramName, '') | ||
| clearDraft() | ||
| return | ||
| } | ||
| try { | ||
| updateParameter(paramName, JSON.parse(newValue)) | ||
| clearDraft() | ||
| } catch { | ||
| updateParameter(paramName, newValue) | ||
| if (paramSchema.type === 'array' && !looksLikeJsonLiteral(newValue)) { | ||
| updateParameter(paramName, newValue) | ||
| clearDraft() | ||
| return | ||
| } | ||
| setInvalidJsonDrafts((prev) => ({ | ||
| ...prev, | ||
| [paramName]: { text: newValue, baseline: valueSignature }, | ||
| })) | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| }} | ||
| isPreview={isPreview} | ||
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.