Automatic Sync: Merge changes from stable/mi to main - #2406
Automatic Sync: Merge changes from stable/mi to main#2406choreo-cicd wants to merge 164 commits into
Conversation
Replace VALID_FILE_EXTENSIONS / VALID_SPECIAL_FILE_NAMES with BLOCKED_BINARY_EXTENSIONS so the agent can read/write any text file (including extensionless ones like Dockerfile, Makefile, .gitignore) while still blocking archives, executables, office docs, images, etc. Shell sandbox now enforces the same deny-list on mutation paths. The @-mention picker keeps its own short UX allow-list, decoupled from the security gate. Also enable XML validation for .dbs data services.
Trim filePath in getReadFileKind() to keep classification consistent with validateReadableFilePath(), and update shell sandbox error message to accurately describe all mutation types (write/edit/delete/rename).
Loads a root-level AGENTS.md (CLAUDE.md / Cursor convention) as a tracked context block. Re-injected only on content drift via the existing session-context hash system, so it does not waste tokens every turn. Files over 30 KB are truncated before being shipped to the model. The block carries an explicit truncation banner so the agent knows context is missing (and can file_read the tail on demand), and a persistent in-chat warning segment tells the user. Truncation warnings fire only when the block is actually being injected, so an unchanged large file does not re-warn each turn.
- agent.ts: make `saveAgentsMdWarning` failure non-fatal. The live event still fires so the user sees the warning this turn; only the JSONL replay across reconnects is sacrificed on persistence failure. - prompt.ts: neutralize embedded `<system-reminder>` / `<user_query>` tags in user-authored AGENTS.md so they can't break out of the surrounding envelope. Replaces the angle brackets with U+27E8 / U+27E9. - prompt.ts: fix the `SessionContextBlockHashes.agentsMd` doc comment to describe what the code actually hashes (surfaced bytes + truncation metadata) instead of the old "full untruncated raw bytes" claim. - AIChatFooter.tsx: splice the warning before the optimistic streaming placeholder. Appending at the end displaced the placeholder, causing subsequent content_block / thinking_* events to land in the warning bubble instead of the assistant message. - eventToMessageConverter.ts: drop `(event as any)` cast now that `warningMessage` is a typed field on AgentEvent / ChatHistoryEvent.
Fix trivy detected vulnerabilities
Support the startOnLoad attribute for tasks in MI 4.1.0
Instruct agent-mode to flip pom.xml <packaging> to jar and declare synapse-core when creating a class mediator, otherwise the CApp ships without the compiled jar and deployment silently fails. Fixes wso2/product-integrator#1631
file_write and file_edit now append a <system-reminder> to the tool result whenever the touched file is a class mediator java source (src/main/java/**/*.java), telling the agent to verify root pom.xml packaging is "jar" and synapse-core is declared. The prompt-level guideline alone can be missed; a per-tool reminder makes it harder to skip. Related to wso2/product-integrator#1631
Read the project root pom.xml and skip the class mediator reminder when <packaging>jar</packaging> is already set, so the agent is not prompted to fix something that is already correct. When packaging is something else (typically the default "pom"), the reminder reports the current value to make the required change unambiguous.
Prepare for the MI 4.0.2 release
…ging Fix copilot-created class mediators not packed in CApp
- Clarify file_read offset/limit are line-based in AGENTS.md guidance - Use Math.ceil for KB rounding and improve truncation warning copy - Add aria-hidden to decorative warning icon - Dedup context_warning on panel reconnect/event replay
Mirror the POSIX shell sandbox check so the agent cannot mutate foo.jar, foo.dll, etc. via PowerShell on Windows.
Switch agent file tools to a binary deny-list
Fix trivy detected vulnerabilities
Sync latest changes from release/mi-4.0.2
Merge "micro-integrator-4.0.2" into "stable/mi"
…ails-1640 Fix agent-mode codegen guardrails (operator precedence, scope, connectors, variables)
Mirror of wso2/mi-vscode#1515 for the release/mi-4.1.1 line of this repo. The MI Copilot proxy blocks Opus models and returns a 400 with an explanatory body, but the chat error card only showed the SDK's generic status text, and WSO2 users could still select Opus in Settings. Settings (WSO2/MI_INTEL login only): - Disable both the Main Agent and Sub-Agent model switches (greyed out) with a note: "Sign in with your own Anthropic API key or AWS Bedrock to change models." BYOK (Anthropic key / Bedrock) is unaffected. - The Main switch always shows the non-Opus default; the high-intelligence warning is suppressed on the plan. Backend: - Clamp the main agent to Sonnet for MI_INTEL when an 'opus' preset carried over from a prior BYOK session, so it can't 400 every turn. Custom model IDs are left untouched. Error surfacing: - Add getDisplayErrorMessage() which extracts the upstream provider/proxy message from APICallError responseBody/data (walking the cause chain) instead of the bare HTTP status text, and wire it into the agent's stream 'error' path, execution catch, and the rpc-manager fallback catch. Mirrors: wso2/mi-vscode#1515
Add support to the binds-to attribute in APIs
Fix back button functionality issue
Mirror the review fixes applied to the mi-vscode PR so both repos carry an identical change set: stream_guard.ts: - Extract shared extractProviderErrorMessageFromBody() that handles APICallError.data as a raw string, surfaces JSON-quoted string bodies, and guards JSON.parse against oversized bodies (MAX_PARSEABLE_BODY_LENGTH) on the already-failing path. agent.ts: - Prefer abortReason over the caught error when building the user-facing message, so watchdog-wrapped errors keep the upstream provider detail. SettingsPanel.tsx / AICodeGenerator.tsx: - Add a byokResolved flag; gate model-switch locking on it and keep the controls neutral (disabled, no lock note, no warning) while resolution is pending. Reset byokResolved before each lookup and settle it on the failure path so the UI can't leak stale state or hang pending. - Make Reset to defaults respect the locked plan: on MI Copilot it no longer rewrites the (locked) model presets, closing the hidden model-switch path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AICodeGenerator.tsx: - Clear isByok/isAwsBedrock (not just byokResolved) and run the re-gate before the rpcClient guard, so a failed or rpcClient-less lookup can't settle with a prior session's stale auth flags and wrongly unlock the model controls. SettingsPanel.tsx: - Use modelControlsDisabled (locked plan OR pending resolution) instead of isMiCopilotPlan for both handleResetDefaults and isDefault, so Reset to defaults can't rewrite presets before the plan is known. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y-errors Lock model switching on WSO2 plan and surface proxy error messages
Merge "micro-integrator-4.1.1" into "stable/mi"
Rename inbound-connectors to inbound-endpoints
Fix table values loading issue in inbound-endpoints
Update vscode-car-plugin version
Fix nested param manager rendering issue on edit
Merge "micro-integrator-4.1.2" into "stable/mi"
|
|
📝 WalkthroughWalkthroughChangesDependency Version Bumps
Estimated code review effort: 3 (Moderate) | ~25 minutes MI Extension & Visualizer Feature Work
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant AddArtifact
participant MCPServerWizard
participant MiDiagramRpcManager
participant MCPServerToolsForm
AddArtifact->>MCPServerWizard: open MCP server form
MCPServerWizard->>MiDiagramRpcManager: getMcpUsedInboundPorts
MCPServerWizard->>MiDiagramRpcManager: createEmptyLocalEntry + createInboundEndpoint
MiDiagramRpcManager-->>MCPServerWizard: created endpoint paths
MCPServerWizard->>MCPServerToolsForm: open MCPServerFromAPIsForm with editData
MCPServerToolsForm->>MiDiagramRpcManager: buildMcpToolsXml / updateMcpInboundEndpoint
sequenceDiagram
participant Agent
participant ChatHistoryManager
participant AIChatFooter
Agent->>Agent: read/truncate AGENTS.md via readAgentsMd
Agent->>ChatHistoryManager: saveAgentsMdWarning(warningMessage)
Agent-->>AIChatFooter: emit context_warning event
AIChatFooter->>AIChatFooter: render ContextWarningSegment
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)common/config/rush/pnpm-config.jsonFile contains syntax errors that prevent linting: Line 1: JSON standard does not allow comments.; Line 5: End of file expected; Line 8: JSON standard does not allow comments.; Line 20: End of file expected; Line 20: End of file expected; Line 20: End of file expected; Line 20: End of file expected; Line 22: JSON standard does not allow comments.; Line 36: JSON standard does not allow comments.; Line 46: JSON standard does not allow comments.; Line 61: JSON standard does not allow comments.; Line 79: JSON standard does not allow comments.; Line 91: End of file expected; Line 91: End of file expected; Line 91: End of file expected; Line 94: End of file expected; Line 96: JSON standard does not allow comments.; Line 108: End of file expected; Line 108: End of file expected; Line 109: Expected a property but instead found '// "ignoreMissing": [" ... [truncated 344 characters] ... an object, or a literal but instead found '// "fsevents"'.; Line 154: End of file expected; Line 155: End of file expected; Line 158: JSON standard does not allow comments.; Line 170: End of file expected; Line 170: End of file expected; Line 171: Expected a property but instead found '// "request": "*"'.; Line 170: End of file expected; Line 171: End of file expected; Line 175: JSON standard does not allow comments.; Line 182: End of file expected; Line 182: End of file expected; Line 182: End of file expected; Line 182: End of file expected; Line 184: JSON standard does not allow comments.; Line 196: End of file expected; Line 196: End of file expected; Line 197: Expected a property but instead found '// "dependencies": {'.; Line 196: End of file expected; Line 197: End of file expected Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
workspaces/mi/mi-extension/src/ai-features/auth.ts (1)
599-606: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStray period breaks the error message.
Line 602 reads "...are only available via the global. inference profile in the commercial AWS partition...". The period after "global" is a leftover artifact and garbles the sentence.
✏️ Proposed fix
throw new Error( `AWS region "${region}" is not supported. The Anthropic models on Bedrock ` + - `(Haiku 4.5, Sonnet 4.6, Opus 4.8) are only available via the global. ` + + `(Haiku 4.5, Sonnet 4.6, Opus 4.8) are only available via the global ` + `inference profile in the commercial AWS partition — GovCloud and China ` + `partitions are not supported. Use a commercial region like us-east-1 or eu-west-1.` );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/auth.ts` around lines 599 - 606, The error text in auth.ts has a stray period in the region validation message, causing “global. inference profile” to read incorrectly. Update the Error message in the region check inside the auth.ts block to remove that extra punctuation and keep the sentence continuous, preserving the rest of the warning and the existing unsupported-region logic.workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts (1)
66-102: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd the explorer focus guard to
goToAddArtifact()andaddArtifact()
openFormFromArtifacts()callsgoToAddArtifact()directly, andaddArtifact()is also invoked without a priorgoToOverview()in several paths. Reuse the samefocusProjectExplorer()/retry flow here to avoid the same tree-visibility flake.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts` around lines 66 - 102, The Project Explorer interaction methods are missing the same visibility/retry guard used by goToOverview, which can make the tree flaky when called directly. Update goToAddArtifact() and addArtifact() to first focus the MI Project Explorer via focusProjectExplorer(), then wait/retry for the project root locator to become visible before hovering and clicking the Add Artifact action. Reuse the existing projectExplorerRoot and locator flow so openFormFromArtifacts() and other direct call paths are protected the same way as goToOverview().
🟡 Minor comments (12)
workspaces/mi/mi-visualizer/src/views/AIPanel/constants.ts-30-30 (1)
30-30: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse a separate cap for images.
MAX_FILE_SIZEis enforced on both files and images inhandleFileAttach, so 10 MB images can pass local validation and then be rejected by Anthropic’s per-image limit. Keep this constant for non-image uploads or add an image-specific limit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/AIPanel/constants.ts` at line 30, `MAX_FILE_SIZE` is currently used for both file attachments and images in `handleFileAttach`, which can let oversized images pass local validation and fail later against Anthropic’s image limit. Update the AIPanel constants and attach flow to use a separate image-specific cap while keeping `MAX_FILE_SIZE` for non-image uploads; refer to `MAX_FILE_SIZE` and `handleFileAttach` when wiring the new limit.workspaces/mi/mi-extension/CHANGELOG.md-38-38 (1)
38-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTypo: "Automatons" → "Automations".
Fixed: Tooling Allow tryout feature for Event Integrations and Automatons (
#1453) — should read "Automations" to match product terminology.✏️ Proposed fix
-Fixed: Tooling Allow tryout feature for Event Integrations and Automatons ([`#1453`](https://github.com/wso2/mi-vscode/issues/1453)) +Fixed: Tooling Allow tryout feature for Event Integrations and Automations ([`#1453`](https://github.com/wso2/mi-vscode/issues/1453))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/CHANGELOG.md` at line 38, Update the changelog entry text to use the correct product term “Automations” instead of “Automatons” in the listed feature description. Locate the existing release note entry in CHANGELOG.md and replace the typo so the wording matches the rest of the product terminology.workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/index.tsx-615-631 (1)
615-631: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard against repeated
onErrorfiring if the fallback icon also fails.If
connector.iconUrl/connectorFailoverIconUrlis also broken,onErrorwill keep re-firing on the same failed URL. Clear the handler after the first fallback attempt.🖼️ Proposed fix
onError={(e) => { const target = e.target as HTMLImageElement; + target.onerror = null; target.src = connector.iconUrl || connector.connectorFailoverIconUrl; }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/index.tsx` around lines 615 - 631, The image fallback in ConnectionForm’s ConnectionCard icon can loop if the replacement URL also fails. Update the img onError handler to stop after the first fallback attempt by clearing the handler before switching the src, using the existing ConnectionCard/icon render logic and the connection.iconUrl, connector.iconUrl, and connector.connectorFailoverIconUrl references to locate it.workspaces/mi/mi-extension/src/ai-features/agent-mode/agents/main/prompt.ts-593-617 (1)
593-617: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTruncation can split a multi-byte UTF-8 character.
readAgentsMdcuts the file at a fixed byte offset (AGENTS_MD_MAX_BYTES) and then decodes withbuffer.subarray(0, bytesRead).toString('utf8'). If the cut lands mid-codepoint, the tail character is silently replaced with U+FFFD in the content that's injected into the prompt and referenced by the truncation footer — a safe but confusing edge case for non-ASCII AGENTS.md files.🩹 Proposed fix: trim back to the last valid UTF-8 boundary
const fd = fs.openSync(agentsMdPath, 'r'); try { const buffer = Buffer.allocUnsafe(AGENTS_MD_MAX_BYTES); const bytesRead = fs.readSync(fd, buffer, 0, AGENTS_MD_MAX_BYTES, 0); - const content = buffer.subarray(0, bytesRead).toString('utf8'); + let end = bytesRead; + // Back off from a truncated multi-byte UTF-8 sequence at the tail + // (continuation bytes are 0x80-0xBF; a lead byte >= 0xC0 needs more + // bytes than remain). + while (end > 0 && (buffer[end - 1] & 0xC0) === 0x80) { + end--; + } + const content = buffer.subarray(0, end).toString('utf8'); return { content, truncated: true, originalSize }; } finally { fs.closeSync(fd); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/agent-mode/agents/main/prompt.ts` around lines 593 - 617, The truncation logic in readAgentsMd can split a multi-byte UTF-8 character when reading only AGENTS_MD_MAX_BYTES bytes, which can inject replacement characters into the prompt content. Update the oversized-file path to trim the read buffer back to the last valid UTF-8 boundary before decoding, then return that safely decoded content with truncated=true. Keep the fix localized to readAgentsMd and its buffer.subarray/toString handling.workspaces/mi/mi-visualizer/src/views/AIPanel/component/AIChatMessage.tsx-344-345 (1)
344-345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExclude context warnings from feedback eligibility.
isContextWarningnow renders a system warning, buthasAnswerContentstill treats its text as answer content. A warning-only final assistant message can show the feedback bar.Proposed fix
segment.isCompactSummary || segment.isFileChanges || segment.isPlan || - segment.isThinking + segment.isThinking || + segment.isContextWarning🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/AIPanel/component/AIChatMessage.tsx` around lines 344 - 345, The feedback eligibility logic in AIChatMessage should ignore context-warning segments, since isContextWarning now renders system text but hasAnswerContent still counts it as answer content. Update the answer-content check to exclude segment.isContextWarning so a warning-only final assistant message does not qualify for feedback, while keeping normal assistant content behavior unchanged.workspaces/mi/mi-extension/src/ai-features/copilot/message-utils.ts-120-132 (1)
120-132: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject mismatched PDF data URI media types.
stripDataUriPrefix()accepts anydata:*;base64,prefix, so a file markedapplication/pdfcan pass validation withdata:image/png;base64,...and then be sent as a PDF block. Validate the prefix media type when present.Proposed fix
-function stripDataUriPrefix(content: string): string { +function stripDataUriPrefix(content: string, expectedMediaType?: string): string | null { const match = content.match(/^data:[^;]+;base64,([\s\S]+)$/); - return match ? match[1] : content; + if (!match) { + return content; + } + const mediaType = content.match(/^data:([^;]+);base64,/i)?.[1]?.toLowerCase(); + if (expectedMediaType && mediaType !== expectedMediaType.toLowerCase()) { + return null; + } + return match[1]; }- else if (file.mimetype === "application/pdf" && !isValidBase64(stripDataUriPrefix(file.content))) { + else if (file.mimetype === "application/pdf") { + const pdfBase64 = stripDataUriPrefix(file.content, "application/pdf"); + if (!pdfBase64 || !isValidBase64(pdfBase64)) { + warnings.push(`Invalid base64 encoding: ${file.name}`); + } + } - warnings.push(`Invalid base64 encoding: ${file.name}`); - }Also applies to: 282-282
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/copilot/message-utils.ts` around lines 120 - 132, The stripDataUriPrefix helper currently removes any data:*;base64, prefix without checking the declared media type, which can let mismatched content slip through as a PDF. Update stripDataUriPrefix and the call path in message-utils.ts so that when a data URI is present it validates the prefix media type against the expected document type before stripping it, and rejects or preserves mismatched prefixes instead of normalizing them. Use the stripDataUriPrefix symbol and the PDF message handling code nearby to keep the validation aligned with the existing inline-data flow.workspaces/mi/mi-visualizer/src/views/Forms/RegistryResourceForm.tsx-45-46 (1)
45-46: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTrailing backslash isn't normalized to
/, risking mixed-separator paths and duplicate-check bypass.When
resourceDirPath/pathends with\(only reachable on Windows per thevalidatePathSeparatorschema rule), bothformatResourcePathandformatRegistryPathappend the filename directly onto the existing\-terminated string instead of normalizing to/. This can produce a mixed-separator path that won't match the/-delimited paths returned from the backend (resourcePaths/registryPaths), silently defeating the "Resource already exists" duplicate check.🐛 Suggested fix (normalize separators up front)
const formatResourcePath = (resourceDirPath: string) => { let resPath = 'resources:'; - resPath = PATH_SEPARATORS.some(sep => resourceDirPath.startsWith(sep)) ? resPath + resourceDirPath.substring(1) : resPath + resourceDirPath; + const normalizedDirPath = resourceDirPath.replace(/\\/g, '/'); + resPath = normalizedDirPath.startsWith('/') ? resPath + normalizedDirPath.substring(1) : resPath + normalizedDirPath; if (createOptionValue) { - PATH_SEPARATORS.some(sep => resPath.endsWith(sep)) ? resPath = resPath + getValues("resourceName") + getFileExtension(getValues('templateType')) - : resPath = resPath + '/' + getValues("resourceName") + getFileExtension(getValues('templateType')); + resPath = resPath.endsWith('/') ? resPath + getValues("resourceName") + getFileExtension(getValues('templateType')) + : resPath + '/' + getValues("resourceName") + getFileExtension(getValues('templateType')); } else { const filename = getValues("filePath").split(/[/\\]/).pop(); - PATH_SEPARATORS.some(sep => resPath.endsWith(sep)) ? resPath = resPath + filename : resPath = resPath + '/' + filename; + resPath = resPath.endsWith('/') ? resPath + filename : resPath + '/' + filename; } return resPath; }Apply the analogous fix to
formatRegistryPath.Also applies to: 277-306
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/RegistryResourceForm.tsx` around lines 45 - 46, Normalize trailing path separators before building paths in both formatResourcePath and formatRegistryPath, since a Windows-style trailing backslash can leave mixed separators and bypass the duplicate check against resourcePaths/registryPaths. Update the path formatting logic in RegistryResourceForm so resourceDirPath/path is converted to a consistent "/"-delimited form up front, then append the filename using that normalized base. Make the same fix in both helpers referenced by PATH_SEPARATORS and the path formatting block to keep duplicate detection reliable.workspaces/mi/syntax-tree/src/syntax-tree-interfaces.ts-1158-1158 (1)
1158-1158: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake
APIResource.bindsTooptional
workspaces/mi/syntax-tree/src/syntax-tree-interfaces.ts:1158— the resource schema doesn’t definebindsTo, and the form code already handles it as missing/empty, so this should bebindsTo?: stringto match the actual contract.🔧 Suggested fix
- bindsTo: string; + bindsTo?: string;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/syntax-tree/src/syntax-tree-interfaces.ts` at line 1158, Make APIResource.bindsTo optional in syntax-tree-interfaces.ts so the type matches the actual resource contract and existing form handling. Update the APIResource interface field from required to optional, and ensure any related references to APIResource or bindsTo continue treating it as missing/empty when not provided.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/index.tsx-227-230 (1)
227-230: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTitle and button text never reflect edit mode.
FormView title(line 230) andcreateButtonLabel(line 227) always say "Create MCP Server"/"Creating..." even wheneditDatais provided.♻️ Proposed fix
- <FormView title="Create MCP Server" onClose={handleClose}> + <FormView title={editData ? "Edit MCP Server" : "Create MCP Server"} onClose={handleClose}>- const createButtonLabel = submitting ? 'Creating...' : (portDiscoveryLoading || depState === 'idle' || depState === 'checking') ? 'Checking...' : 'Create MCP Server'; + const createButtonLabel = submitting ? (editData ? 'Updating...' : 'Creating...') : (portDiscoveryLoading || depState === 'idle' || depState === 'checking') ? 'Checking...' : (editData ? 'Update MCP Server' : 'Create MCP Server');Also applies to: 319-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/index.tsx` around lines 227 - 230, The MCPServerForm is not switching its UI copy when editData is present, so both the FormView title and createButtonLabel still use create-mode text. Update the conditional text in MCPServerForm (around createButtonLabel and the FormView title) to derive labels from edit mode versus create mode, using editData to choose “Edit/Update” copy and keeping the existing “Create/Creating” copy for new servers. Also check the related submit/secondary button text near the later referenced section so all displayed actions stay consistent with the current mode.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/index.tsx-63-66 (1)
63-66: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPort field lacks valid-range bounds.
Only
required/integerchecks are applied; nothing constrains the value to a valid TCP port range, allowing e.g. negative numbers or values above 65535 to pass client-side validation.🐛 Proposed fix
port: yup.number() .typeError('Port must be a number') .required('Port is required') - .integer('Port must be an integer'), + .integer('Port must be an integer') + .min(1, 'Port must be between 1 and 65535') + .max(65535, 'Port must be between 1 and 65535'),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/index.tsx` around lines 63 - 66, The MCP server form port validation in MCPServerForm currently only checks required and integer values, so invalid TCP ports can still pass. Update the yup schema for the port field in the form validation to add explicit minimum and maximum bounds so only valid port numbers are accepted, using the existing port validator chain in the MCPServerForm component.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddAPIToolDialog.tsx-224-236 (1)
224-236: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUnguarded
convertMcpJsonSchema/pickMcpJsonFileRPC calls can produce unhandled promise rejections.
validateSchemaawaitsconvertMcpJsonSchemawith no try/catch, and it's invoked fire-and-forget from the schemaonChangehandler (Line 460) and fromhandleImportFile(Line 294, which also has its own unguardedpickMcpJsonFile()call). If the RPC rejects, this becomes an unhandled rejection with no error surfaced to the user (the field just silently fails to validate/import).🛡️ Proposed fix
const validateSchema = async (id: string, value: string): Promise<boolean> => { if (!value.trim()) { clearErrors(`items.${id}.inputSchema` as const); return true; } - const { schema } = await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: value }); - if (schema === null) { - setError(`items.${id}.inputSchema` as const, { message: INVALID_MCP_SCHEMA_MESSAGE }); - return false; - } - clearErrors(`items.${id}.inputSchema` as const); - return true; + try { + const { schema } = await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: value }); + if (schema === null) { + setError(`items.${id}.inputSchema` as const, { message: INVALID_MCP_SCHEMA_MESSAGE }); + return false; + } + clearErrors(`items.${id}.inputSchema` as const); + return true; + } catch (error) { + console.error('Schema validation failed', error); + setError(`items.${id}.inputSchema` as const, { message: INVALID_MCP_SCHEMA_MESSAGE }); + return false; + } };Also applies to: 290-295, 459-461
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddAPIToolDialog.tsx` around lines 224 - 236, Guard the RPC calls in AddAPIToolDialog’s validateSchema and handleImportFile so rejected promises do not become unhandled rejections. Wrap rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema and pickMcpJsonFile in try/catch, surface a user-facing error via setError or existing import error handling, and ensure the schema onChange path and import flow both await or safely handle the async result from validateSchema/handleImportFile.workspaces/mi/mi-visualizer/src/views/Overview/ProjectStructureView.tsx-269-287 (1)
269-287: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle MCP servers that only have an inbound endpoint path. The row can render from
inboundEndpoint.path, butgoToMcpServerToolsreturns immediately whenlocalEntry.pathis missing, so clicking those entries does nothing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Overview/ProjectStructureView.tsx` around lines 269 - 287, The goToMcpServerTools handler currently exits early when localEntry.path is missing, which breaks MCP server rows that render from inboundEndpoint.path only. Update goToMcpServerTools in ProjectStructureView to accept inboundEndpoint.path as a valid source for navigation, derive the serverName and documentUri from whichever path is available, and keep passing both localEntryPath and inboundEndpointPath in customProps for MACHINE_VIEW.MCPServerFromAPIsForm.
🧹 Nitpick comments (16)
workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx (1)
68-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared import logic into a hook.
importWithOpenAPIis now nearly identical toimportWithProtoinImportConnectionFromProto.tsx(same state, same result-check/error-handling flow). Given the codebase already introduces shared hooks for connector concerns (e.g.,useConnectorDependency), consider extracting this into auseSpecImport-style hook parameterized by file path/type to avoid duplicated logic across both files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/ConnectionForm/ImportConnectionFromOpenAPI.tsx` around lines 68 - 90, The import flow in importWithOpenAPI duplicates the same state transitions, result checks, and error handling already present in importWithProto, so extract that shared logic into a reusable hook such as useSpecImport. Move the common async workflow into the hook, parameterize it with the spec source details (for example file path/type and success callback), and update ImportConnectionFromOpenAPI to use the shared hook instead of keeping a separate copy of the logic.workspaces/mi/mi-extension/src/ai-features/agent-mode/agents/main/prompt.ts (1)
902-931: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for the new injection-defense / truncation logic.
neutralizePromptEnvelopeTagsandbuildAgentsMdBlockTextare new, security-relevant pure functions (prompt envelope-tag neutralization, truncation-footer wording) with no visible test coverage. Since they gate whether user-authored AGENTS.md content can break out of the<system-reminder>wrapper, a couple of focused unit tests (tag-breakout attempt, truncated vs. non-truncated, cleared status) would guard against regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/agent-mode/agents/main/prompt.ts` around lines 902 - 931, Add focused unit tests for the new prompt-safety helpers so regressions are caught early. Cover neutralizePromptEnvelopeTags with a breakout attempt that contains system-reminder and user_query tags, and verify buildAgentsMdBlockText in the cleared, truncated, and non-truncated paths. Use the existing helper names neutralizePromptEnvelopeTags and buildAgentsMdBlockText to locate the logic, and assert the returned text contains the expected neutralized tags, truncation footer, or cleared-message behavior.workspaces/mi/mi-extension/src/ai-features/agent-mode/attachment-utils.ts (1)
91-108: 📐 Maintainability & Code Quality | 🔵 TrivialMinor duplication between
stripDataUriPrefixandtoImageContentPart.Both functions parse the same
data:<mediaType>;base64,<payload>pattern independently. Consider havingtoImageContentPartreusestripDataUriPrefixplus a small mediaType extraction, or extract a sharedparseDataUri()helper returning{ mediaType?, data }.♻️ Possible consolidation
-function toImageContentPart(imageDataUri: string): { type: "image"; image: string; mediaType?: string } { - const match = imageDataUri.match(/^data:([^;]+);base64,([\s\S]+)$/); - if (match) { - return { type: "image", image: match[2], mediaType: match[1] }; - } - // Fallback: pass through unchanged (already-raw base64 or unexpected format). - return { type: "image", image: imageDataUri }; -} +function parseDataUri(value: string): { mediaType?: string; data: string } { + const match = value.match(/^data:([^;]+);base64,([\s\S]+)$/); + return match ? { mediaType: match[1], data: match[2] } : { data: value }; +} + +function stripDataUriPrefix(content: string): string { + return parseDataUri(content).data; +} + +function toImageContentPart(imageDataUri: string): { type: "image"; image: string; mediaType?: string } { + const { mediaType, data } = parseDataUri(imageDataUri); + return { type: "image", image: data, mediaType }; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/agent-mode/attachment-utils.ts` around lines 91 - 108, `stripDataUriPrefix` and `toImageContentPart` both parse the same data URI format separately, so consolidate this logic in `attachment-utils.ts`. Update `toImageContentPart` to reuse `stripDataUriPrefix` for the payload and add a shared helper (or equivalent) to extract `mediaType`, so the `data:<mediaType>;base64,<payload>` handling lives in one place and both functions stay consistent.workspaces/mi/mi-extension/src/ai-features/agent-mode/tools/types.ts (1)
66-115: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftAdd content-based binary detection before text reads/writes
The deny-list still treats extensionless and unlisted files as text, so a binary artifact with an unknown extension can be opened or overwritten as UTF-8. Keep the image/PDF multimodal paths, but add byte-level sniffing before the text path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/ai-features/agent-mode/tools/types.ts` around lines 66 - 115, The current extension-only check in hasBlockedBinaryExtension still lets extensionless or unknown binary files be treated as text, so add content-based sniffing before any text read/write path. Update the file handling flow in the agent tools around file_read and file_write/file_edit to inspect initial bytes/magic signatures and route binaries to the blocked/multimodal path even when the extension is not in BLOCKED_BINARY_EXTENSIONS. Keep the existing image/PDF special cases intact, but ensure the new check runs before the UTF-8 text path is chosen.workspaces/mi/mi-diagram/src/components/Form/GigaParamManager/ParameterManager.tsx (1)
112-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared expression-type lists.
ParameterManager,FormGenerator, andDynamicFieldsHandlerall hard-code the sameOrExpressiongrouping, including the legacystringOrExpresionalias. Move this to one shared helper so the categories don’t drift; keep the alias only if older saved forms still depend on it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-diagram/src/components/Form/GigaParamManager/ParameterManager.tsx` around lines 112 - 117, The expression-type grouping is duplicated across ParameterManager, FormGenerator, and DynamicFieldsHandler, which risks the lists drifting out of sync. Extract the shared OrExpression categories, including the legacy stringOrExpresion alias if it is still needed for backward compatibility, into one common helper and have ParameterManager use that shared source instead of its local EXPRESSION_INPUT_TYPES array.workspaces/mi/mi-extension/src/debugger/debugHelper.ts (1)
437-442: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueMinor:
pidnon-null assertion could throw if spawn failed to obtain a pid.
activeBuildProcess.pid!assumespidis always defined. If the underlying process failed to spawn (e.g., ENOENT),pidcould beundefined, andtreeKillmay throw or behave unexpectedly. Low risk givenactiveBuildProcessis only set after a successfulspawn()call, but a guard would be more defensive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` around lines 437 - 442, The abortBuildAndRun helper is using a non-null assertion on activeBuildProcess.pid, which can be unsafe if the process never spawned correctly. Update abortBuildAndRun to defensively check that activeBuildProcess and its pid are both defined before calling treeKill, and only clear activeBuildProcess after a valid kill attempt; use the abortBuildAndRun and activeBuildProcess symbols to locate the change.workspaces/mi/mi-visualizer/src/Hooks.tsx (1)
54-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRegister the download-progress listener in an effect. The current body-level subscription runs on every render, so each progress update can add another handler. Move it into
useEffectwith[rpcClient], and dispose the listener on cleanup ifonNotificationreturns a disposable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/Hooks.tsx` around lines 54 - 56, The download-progress subscription is being registered directly in the Hooks component body, so it runs on every render and can accumulate duplicate handlers. Move the rpcClient.onDownloadProgress registration into a useEffect tied to [rpcClient], and keep the setDownloadProgress callback inside that effect. If onDownloadProgress/onNotification returns a disposable or unsubscribe handle, store it and invoke cleanup in the effect return to avoid leaking listeners.workspaces/mi/mi-visualizer/src/MainPanel.tsx (1)
177-330: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant per-case
keyprops now dead code.
getUniqueKey(...)keys set onResourceView,SequenceView,ProxyView,DataServiceView, etc. are now overwritten byReact.cloneElement(..., { key: viewKey })at Line 445, sinceviewKeyis applied uniformly to every view. Not a functional bug (viewKey is a superset), but the per-case keys are effectively unused now.♻️ Optional cleanup
- key={getUniqueKey(visualizerState.stNode, visualizerState.documentUri)}(remove from each case now redundant with the outer
cloneElementkey, or drop thecloneElementwrapper for cases that already set an equivalent key.)Also applies to: 444-446
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/MainPanel.tsx` around lines 177 - 330, The per-case getUniqueKey(...) key props in MainPanel’s view switch are now redundant because React.cloneElement later applies a uniform viewKey to every rendered view. Clean up the affected cases such as ResourceView, SequenceView, ProxyView, and DataServiceView by either removing the local key props or, if you want to preserve them, stop wrapping the result in cloneElement for those views so only one key source remains.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddAPIToolDialog.tsx (1)
318-336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify the convoluted
flatMap(...).map(fn => fn())construction, and guard its ownconvertMcpJsonSchemacall.This can be a plain
Promise.all(opIds.map(async opId => {...})). It also makes its own unguardedconvertMcpJsonSchemacall (Line 326) with no try/catch, so a rejection here fails the whole submit silently (unhandled rejection, form stuck).♻️ Proposed simplification
- const selectedOperations = await Promise.all(opIds.flatMap(opId => { - const operation = selectedAPI?.operations.find((op: APIOperation) => op.id === opId); - if (!operation) return []; - return [async () => { - const item = data.items?.[opId]; - const raw = item?.inputSchema?.trim() || ''; - let converted: string | null = null; - if (raw) { - const { schema } = await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: raw }); - converted = schema; - } - return { - id: opId, - customName: item?.customName || getDefaultName(operation), - description: item!.description.trim(), - inputSchema: converted || EMPTY_MCP_SCHEMA, - }; - }]; - }).map(fn => fn())); + const validOpIds = opIds.filter(opId => selectedAPI?.operations.some((op: APIOperation) => op.id === opId)); + const selectedOperations = await Promise.all(validOpIds.map(async opId => { + const operation = selectedAPI!.operations.find((op: APIOperation) => op.id === opId)!; + const item = data.items?.[opId]; + const raw = item?.inputSchema?.trim() || ''; + let converted: string | null = null; + if (raw) { + try { + const { schema } = await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: raw }); + converted = schema; + } catch (error) { + console.error('Schema conversion failed', error); + } + } + return { + id: opId, + customName: item?.customName || getDefaultName(operation), + description: item!.description.trim(), + inputSchema: converted || EMPTY_MCP_SCHEMA, + }; + }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddAPIToolDialog.tsx` around lines 318 - 336, The selectedOperations builder in AddAPIToolDialog is overly convoluted and leaves convertMcpJsonSchema unguarded. Replace the current flatMap(...).map(fn => fn()) pattern with a direct Promise.all over opIds using a single async mapper in AddAPIToolDialog, and wrap the rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema call in try/catch so a schema conversion failure falls back to EMPTY_MCP_SCHEMA instead of rejecting the whole submit flow.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/ToolsList.tsx (1)
213-217: 🚀 Performance & Scalability | 🔵 TrivialSame on-keystroke RPC validation pattern.
validateSchemaruns on everyonChange(Line 215), same as the other MCP tool dialogs — consider debouncing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/ToolsList.tsx` around lines 213 - 217, The `validateSchema` call in `ToolsList` is still firing on every `onChange`, so update the `setEditToolInputSchema` handler to debounce validation instead of running it on each keystroke. Use the existing `validateSchema` path in `ToolsList.tsx` and apply the same debouncing approach used in the other MCP tool dialogs so schema checks happen after typing settles.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/CreateScratchToolDialog.tsx (2)
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated tool-name sanitization logic.
This
sanitizeToolNamehelper duplicates the inline name-derivation logic inMCPServerToolsForm.tsx'sconfirmAddScratchTool(name lowercasing + character replacement +_toolsuffix). Since the collision check here validates against a name computed independently of the one actually used to create the sequence, any future change to one algorithm without the other risks silent collision-detection mismatches. Consider extracting a single shared helper (e.g. exported from this file or a small shared util) and reusing it in both places.♻️ Suggested direction
+// e.g. export from a shared module and import in both files +export const sanitizeToolName = (raw: string): string => raw.trim().toLowerCase() .replace(/[^a-z0-9]/g, '_') .replace(/_{2,}/g, '_') .replace(/^_+|_+$/, '');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/CreateScratchToolDialog.tsx` around lines 60 - 64, The tool-name sanitization logic is duplicated between sanitizeToolName in CreateScratchToolDialog and confirmAddScratchTool in MCPServerToolsForm, which can cause mismatch between collision checks and the actual created sequence name. Extract the name-derivation into a single shared helper and reuse it in both places so both the collision validation and tool creation use the exact same lowercasing, replacement, and suffix rules.
225-235: 🚀 Performance & Scalability | 🔵 TrivialSame on-keystroke RPC validation pattern as
AddSequenceToolDialog.tsx.
validateSchemais called on everyonChange(line 233) without debouncing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/CreateScratchToolDialog.tsx` around lines 225 - 235, The `CreateScratchToolDialog` input schema field is triggering `validateSchema` on every keystroke via the `TextArea` `onChange` handler, which mirrors the same RPC-heavy pattern seen in `AddSequenceToolDialog`. Update this to debounce or otherwise throttle the `validateSchema` call so validation runs after typing pauses instead of on every change. Keep the logic in the `register('inputSchema', ...)` handler and preserve the existing schema validation behavior.workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddSequenceToolDialog.tsx (1)
299-310: 🚀 Performance & Scalability | 🔵 TrivialSchema validation fires an RPC on every keystroke.
convertMcpJsonSchemais invoked on everyonChangeof the schema textarea (Line 307) with no debounce, so each keystroke triggers a full RPC round-trip to the extension host. The same pattern recurs inCreateScratchToolDialog.tsxandToolsList.tsx. Consider debouncing the validation call (e.g., 300-500ms) to reduce chattiness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/AddSequenceToolDialog.tsx` around lines 299 - 310, The schema validation path in the textarea is firing on every keystroke, causing repeated RPC calls from the `onChange` handler wired through `register(..., { onChange: e => validateSchema(seq.id, e.target.value) })`. Update `validateSchema` usage in `AddSequenceToolDialog` to debounce the `convertMcpJsonSchema`-triggering validation, and apply the same debounce pattern in `CreateScratchToolDialog` and `ToolsList` where the same validation flow exists. Keep the validation behavior intact, but delay execution by a short interval so typing does not spam the extension host.workspaces/mi/mi-visualizer/src/views/ServiceDesigner/index.tsx (1)
326-326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
bindsTostring-parsing logic.The comma-split/trim/filter logic here duplicates
getApiBindsToOptions's underlying parsing (form.ts, viabindsToToList) and the equivalent logic inResourceForm.tsx'sbindsToToParamConfig. Consider exporting and reusing a singlebindsToToList/stringToListhelper fromform.tsacross all three call sites to avoid drift if the delimiter or normalization rules ever change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/ServiceDesigner/index.tsx` at line 326, The bindsTo parsing in ServiceDesigner is duplicated and should reuse the shared list conversion logic instead of repeating split/trim/filter here. Update the ServiceDesigner component to call the same helper used by getApiBindsToOptions and ResourceForm.tsx, ideally exporting bindsToToList or a generic stringToList from form.ts, so all three call sites stay consistent if the parsing rules change.workspaces/mi/mi-visualizer/src/views/Forms/ResourceForm.tsx (1)
171-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate “Inbound Endpoint” ParamField definition.
The
ParamFieldconfig for the bindings AutoComplete is defined twice: once insidebindsToToParamConfig(Lines 178-187) and again inline in the JSX (Lines 476-484). Since the JSX always overridesparamFieldsfrombindsToParams, theparamFieldscomputed insidebindsToToParamConfigis effectively dead/unused at render time — onlyparamValuesfrom that helper survives. This drift risk (e.g. someone updates one definition but not the other) can be avoided by extracting a singlebuildBindsToParamField(options: string[]): ParamFieldhelper reused in both places.♻️ Suggested consolidation
+const buildBindsToParamField = (options: string[]): ParamField => ({ + id: 0, + type: "AutoComplete", + label: "Inbound Endpoint", + values: options, + isRequired: true, + allowItemCreate: false, +}); + const bindsToToParamConfig = (bindsTo: string, options: string[]): ParamConfig => { - const paramFields: ParamField[] = [ - { - id: 0, - type: "AutoComplete", - label: "Inbound Endpoint", - values: options, - isRequired: true, - allowItemCreate: false, - }, - ]; return { - paramFields, + paramFields: [buildBindsToParamField(options)], ... }; };And in the JSX:
<ParamManager paramConfigs={{ ...bindsToParams, - paramFields: [ - { - id: 0, - type: "AutoComplete", - label: "Inbound Endpoint", - values: bindsToOptions, - isRequired: true, - allowItemCreate: false, - }, - ], + paramFields: [buildBindsToParamField(bindsToOptions)], }}Also applies to: 469-491
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Forms/ResourceForm.tsx` around lines 171 - 200, The “Inbound Endpoint” ParamField is duplicated between bindsToToParamConfig and the JSX that builds bindsToParams, and the helper’s paramFields are effectively overwritten at render time. Refactor ResourceForm so there is a single shared builder for this field, such as a reusable buildBindsToParamField(options) helper, and use it both in bindsToToParamConfig and where the JSX currently defines the AutoComplete field. Keep the helper’s ParamConfig focused on paramValues and avoid maintaining two separate ParamField definitions.workspaces/mi/mi-visualizer/src/views/Diagram/Resource.tsx (1)
55-61: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against stale/out-of-order responses in the bindsTo fetch effect.
The async IIFE fetching
bindsToOptionsondocumentUrichange has no guard against out-of-order resolution — ifdocumentUrichanges again before the previous fetch resolves, a stale response can overwrite fresher state.🔒 Suggested fix
useEffect(() => { + let cancelled = false; (async () => { - setBindsToOptions(await getApiBindsToOptions(rpcClient, documentUri)); + const options = await getApiBindsToOptions(rpcClient, documentUri); + if (!cancelled) { + setBindsToOptions(options); + } })(); + return () => { cancelled = true; }; }, [documentUri]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-visualizer/src/views/Diagram/Resource.tsx` around lines 55 - 61, The bindsToOptions fetch in Resource’s useEffect can apply stale results if an older getApiBindsToOptions call resolves after a newer documentUri change. Update the effect to guard against out-of-order async completion, using the existing useEffect and setBindsToOptions flow in Resource.tsx. Add a cancellation/active flag or similar cleanup so only the latest documentUri request can update state, and ignore results from superseded requests.
| // Mirror the POSIX analyzer: block mutations targeting binary file paths | ||
| // (matches the file-tool deny-list). Basename is compared case-insensitively | ||
| // via `hasBlockedBinaryExtension`. We use `path.win32.basename` to handle | ||
| // both `/` and `\` separators in PowerShell tokens. | ||
| const blockedBinaryMutationPaths = dedupe( | ||
| extractedPathTokens.mutationPathTokens.filter( | ||
| (token) => hasBlockedBinaryExtension(path.win32.basename(stripWrappingQuotes(token))) | ||
| ) | ||
| ); | ||
| if (blockedBinaryMutationPaths.length > 0) { | ||
| blocked = true; | ||
| reasons.push( | ||
| `Mutating binary file paths is blocked (write/edit/delete/rename; matches the file-tool deny-list). ` + | ||
| `Blocked path(s): ${blockedBinaryMutationPaths.join(', ')}. ` + | ||
| `Use directory-level operations (e.g. \`Remove-Item -Recurse dist\`) instead of targeting binary files by name.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate PowerShell analyzer tests and add/verify a case where a variable target
# points to a deny-listed binary path and must be blocked, not approval-only.
rg -n -C3 "analyzePowerShellCommand|unresolvedPathLikeTokens|Remove-Item|blockedBinaryMutationPaths" \
workspaces/mi/mi-extension/src/ai-features/agent-modeRepository: wso2/vscode-extensions
Length of output: 12084
Block unresolved PowerShell mutation targets. Variable targets like Remove-Item $p can hide a deny-listed binary path and currently fall through to unresolvedPathLikeTokens, which only adds an approval reason. Mirror the POSIX branch’s fail-closed handling so ambiguous mutation targets are blocked, not just approved.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/mi/mi-extension/src/ai-features/agent-mode/tools/shell_sandbox_powershell.ts`
around lines 902 - 918, The PowerShell mutation analyzer in
shell_sandbox_powershell.ts currently lets unresolved mutation targets like
Remove-Item $p fall into unresolvedPathLikeTokens and only adds an approval
reason, which should be blocked instead. Update the mutation-path handling
around extractedPathTokens and blockedBinaryMutationPaths to mirror the POSIX
fail-closed behavior: detect ambiguous or unresolved mutation targets during
write/edit/delete/rename analysis, mark them as blocked, and add a rejection
reason instead of approving them when the target cannot be resolved.
| activeBuildProcess = buildProcess; | ||
| showServerOutputChannel(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Aborted builds will show a misleading "build failed" message.
The close handler now fires unconditionally (previously gated by shouldCopyTarget) and reports failure whenever code !== 0. When abortBuildAndRun() kills the process via SIGKILL on disconnect, Node reports code === null (not 0) on exit/close, so this handler will show "${project} build failed" even though the build was intentionally aborted by the user — contradicting the disconnect flow's intent to suppress error UI (_isDisconnecting is checked in debugAdapter.ts's executeTasks().catch(), but not here).
🛠️ Proposed fix: track abort state and suppress the message
+let isBuildAborted = false;
+
export function abortBuildAndRun(): void {
if (activeBuildProcess) {
+ isBuildAborted = true;
treeKill(activeBuildProcess.pid!, 'SIGKILL');
activeBuildProcess = undefined;
}
} buildProcess.on('close', async (code) => {
- if (code === 0) {
+ if (isBuildAborted) {
+ isBuildAborted = false;
+ } else if (code === 0) {
vscode.window.showInformationMessage(`${path.basename(project)} build was successful`);
} else {
vscode.window.showErrorMessage(`${path.basename(project)} build failed`);
}
});Also applies to: 225-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-extension/src/debugger/debugHelper.ts` around lines 216 -
217, The build process close handler in debugHelper.ts is now treating every
non-zero/undefined exit as a failure, which causes aborted builds from
abortBuildAndRun() to report a misleading “build failed” message. Update the
close/exit handling around activeBuildProcess and showServerOutputChannel so it
can detect an intentional abort (for example via a flag set when SIGKILL is sent
on disconnect) and suppress the failure UI in that case. Keep the failure
message only for genuine build errors, and align the logic with the disconnect
flow already handled in debugAdapter.ts.
| deleteApiMetadata(fileUri); | ||
| } | ||
| refreshViewsAfterDelete(fileUri); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the reported lines
sed -n '430,520p' workspaces/mi/mi-extension/src/project-explorer/activate.ts
# Locate the helper definitions and all call sites
rg -n "deleteApiMetadata|refreshViewsAfterDelete|deleteSwagger|fileUri" workspaces/mi/mi-extension/src/project-explorer/activate.ts workspaces/mi/mi-extension/src -g '!**/node_modules/**'Repository: wso2/vscode-extensions
Length of output: 17784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the helper signatures and the tree item builders that feed DELETE_PROJECT_EXPLORER_ITEM
sed -n '1,120p' workspaces/mi/mi-extension/src/project-explorer/activate.ts
sed -n '600,660p' workspaces/mi/mi-extension/src/util/fileOperations.ts
sed -n '300,340p' workspaces/mi/mi-extension/src/util/swagger.ts
# Find where project explorer TreeItems attach command arguments or info.path
rg -n "DELETE_PROJECT_EXPLORER_ITEM|info\.path|command:.*DELETE_PROJECT_EXPLORER_ITEM|arguments:\s*\[" workspaces/mi/mi-extension/src -g '!**/node_modules/**'Repository: wso2/vscode-extensions
Length of output: 12733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the project explorer tree-item builders around the delete-related entries
sed -n '420,460p' workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.ts
sed -n '930,960p' workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.ts
# Also inspect any MCP-related tree item construction for context, if present
sed -n '860,900p' workspaces/mi/mi-extension/src/project-explorer/project-explorer-provider.tsRepository: wso2/vscode-extensions
Length of output: 3911
Normalize fileUri before cleanup calls. fileUri can be a Uri, and deleteSwagger, deleteApiMetadata, and refreshViewsAfterDelete all expect a string path. Convert once to filePath and pass that through so API metadata cleanup and the post-delete refresh run on the right resource.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-extension/src/project-explorer/activate.ts` around lines 488
- 490, Normalize the resource before cleanup in the delete flow: in the code
around deleteSwagger, deleteApiMetadata, and refreshViewsAfterDelete, convert
fileUri to a string path once (for example, a filePath variable) and pass that
consistently to all three calls. This ensures the cleanup and refresh logic in
activate.ts targets the correct resource regardless of whether fileUri is a Uri
or string.
| if (!localEntryPath || !inboundEndpointPath) { | ||
| window.showErrorMessage('Could not determine MCP server paths'); | ||
| return; | ||
| } | ||
|
|
||
| const confirmation = await window.showWarningMessage( | ||
| `Are you sure you want to delete MCP Server - ${item.label}?`, | ||
| { modal: true }, | ||
| 'Yes' | ||
| ); | ||
|
|
||
| if (confirmation === 'Yes') { | ||
| try { | ||
| // Delete local entry file | ||
| await vscode.workspace.fs.delete(Uri.file(localEntryPath), { recursive: true, useTrash: true }); | ||
|
|
||
| // Delete inbound endpoint file if it exists | ||
| if (inboundEndpointPath) { | ||
| await vscode.workspace.fs.delete(Uri.file(inboundEndpointPath), { recursive: true, useTrash: true }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Allow MCP server deletion when only the local-entry config exists.
inboundEndpoint is optional in MCPServerTreeItem, but this branch refuses to delete unless inboundEndpointPath is present. That leaves local-entry-only or partially-created MCP servers undeletable from the explorer.
Proposed fix
- if (!localEntryPath || !inboundEndpointPath) {
+ if (!localEntryPath) {
window.showErrorMessage('Could not determine MCP server paths');
return;
}
@@
- // Delete inbound endpoint file if it exists
if (inboundEndpointPath) {
- await vscode.workspace.fs.delete(Uri.file(inboundEndpointPath), { recursive: true, useTrash: true });
+ try {
+ await vscode.workspace.fs.delete(Uri.file(inboundEndpointPath), { recursive: true, useTrash: true });
+ } catch {
+ // The inbound endpoint is optional; continue deleting the MCP config.
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!localEntryPath || !inboundEndpointPath) { | |
| window.showErrorMessage('Could not determine MCP server paths'); | |
| return; | |
| } | |
| const confirmation = await window.showWarningMessage( | |
| `Are you sure you want to delete MCP Server - ${item.label}?`, | |
| { modal: true }, | |
| 'Yes' | |
| ); | |
| if (confirmation === 'Yes') { | |
| try { | |
| // Delete local entry file | |
| await vscode.workspace.fs.delete(Uri.file(localEntryPath), { recursive: true, useTrash: true }); | |
| // Delete inbound endpoint file if it exists | |
| if (inboundEndpointPath) { | |
| await vscode.workspace.fs.delete(Uri.file(inboundEndpointPath), { recursive: true, useTrash: true }); | |
| } | |
| if (!localEntryPath) { | |
| window.showErrorMessage('Could not determine MCP server paths'); | |
| return; | |
| } | |
| const confirmation = await window.showWarningMessage( | |
| `Are you sure you want to delete MCP Server - ${item.label}?`, | |
| { modal: true }, | |
| 'Yes' | |
| ); | |
| if (confirmation === 'Yes') { | |
| try { | |
| // Delete local entry file | |
| await vscode.workspace.fs.delete(Uri.file(localEntryPath), { recursive: true, useTrash: true }); | |
| if (inboundEndpointPath) { | |
| try { | |
| await vscode.workspace.fs.delete(Uri.file(inboundEndpointPath), { recursive: true, useTrash: true }); | |
| } catch { | |
| // The inbound endpoint is optional; continue deleting the MCP config. | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-extension/src/project-explorer/activate.ts` around lines 686
- 705, The MCP server delete flow in activate.ts is too strict because it
requires both localEntryPath and inboundEndpointPath, even though
inboundEndpoint is optional in MCPServerTreeItem. Update the deletion guard so
it only blocks when localEntryPath is missing, then keep deleting the local
entry file and delete the inbound endpoint file conditionally only when
inboundEndpointPath exists. Use the existing delete branch around
window.showWarningMessage and vscode.workspace.fs.delete to ensure
local-entry-only or partially created MCP servers can still be removed.
| taskProperties: [], | ||
| startOnLoad: jsonData.task["@_"]["startOnLoad"] !== undefined ? | ||
| String(jsonData.task["@_"]["startOnLoad"]) : undefined |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return the parsed task response even when there are no task properties.
startOnLoad is added to response, but tasks without jsonData.task.property still fall through to the default empty response below, so the edit form loses the parsed task fields.
Proposed fix
- if (jsonData.task.property) {
+ if (jsonData.task.property) {
response.taskProperties = Array.isArray(jsonData.task.property) ?
jsonData.task.property.map((prop: any) => ({
@@
- resolve(response);
}
- resolve({
- name: '',
- group: '',
- implementation: '',
- pinnedServers: '',
- triggerType: 'simple',
- triggerCount: 1,
- triggerInterval: 1,
- triggerCron: '',
- taskProperties: []
- });
+ resolve(response);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| taskProperties: [], | |
| startOnLoad: jsonData.task["@_"]["startOnLoad"] !== undefined ? | |
| String(jsonData.task["@_"]["startOnLoad"]) : undefined | |
| if (jsonData.task.property) { | |
| response.taskProperties = Array.isArray(jsonData.task.property) ? | |
| jsonData.task.property.map((prop: any) => ({ | |
| @@ | |
| } | |
| resolve(response); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-extension/src/rpc-managers/mi-diagram/rpc-manager.ts` around
lines 1852 - 1854, The parsed task data is being discarded for tasks that have
no properties because the branch in rpc-manager.ts under the task parsing logic
still falls through to the default empty response. Update the task response
handling in the same parsing path so the object built with startOnLoad and other
parsed task fields is returned immediately even when jsonData.task.property is
missing, instead of being replaced by the fallback empty response.
| useEffect(() => { | ||
| const loadUsedPorts = async () => { | ||
| setPortDiscoveryLoading(true); | ||
| setPortDiscoveryError(null); | ||
| try { | ||
| const projectRootResp = await rpcClient.getMiDiagramRpcClient().getProjectRoot({ path: props.path }); | ||
| const { ports } = await rpcClient.getMiDiagramRpcClient().getMcpUsedInboundPorts({ | ||
| projectUri: projectRootResp.path, | ||
| }); | ||
| setUsedPorts(new Set(ports)); | ||
| setPortDiscoveryLoading(false); | ||
| } catch (err) { | ||
| console.error('[MCPServerForm] Port discovery error:', err); | ||
| setPortDiscoveryError(`Failed to check existing ports: ${err instanceof Error ? err.message : String(err)}`); | ||
| setPortDiscoveryLoading(false); | ||
| } | ||
| }; | ||
| loadUsedPorts(); | ||
| }, [rpcClient, props.path]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Editing an existing server can falsely flag its own port as "in use".
getMcpUsedInboundPorts is called without excludePath (lines 130-132), even though the request type supports it specifically for this scenario. When editing an existing MCP server, its own currently-bound port will be included in usedPorts, causing the submit-time check at line 162 to reject the unchanged port as "already in use by another inbound endpoint."
🐛 Proposed fix
const { ports } = await rpcClient.getMiDiagramRpcClient().getMcpUsedInboundPorts({
projectUri: projectRootResp.path,
+ excludePath: editData?.inboundEndpointPath,
});Also applies to: 162-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/index.tsx` around
lines 124 - 142, The port lookup in MCPServerForm is including the server being
edited, so its own bound port gets marked as used and the submit validation
rejects an unchanged port. Update loadUsedPorts in MCPServerForm to pass the
current server’s path as excludePath when calling getMcpUsedInboundPorts, using
the existing props/path data available in the component so the current endpoint
is omitted from usedPorts during edit flows.
| const onSubmit = async (data: any) => { | ||
| setSubmitting(true); | ||
| setError(null); | ||
| try { | ||
| const projectRootResp = await rpcClient.getMiDiagramRpcClient().getProjectRoot({ path: props.path }); | ||
| const projectDir = projectRootResp.path; | ||
| const localEntriesDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'local-entries').toString(); | ||
| const inboundEndpointsDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'inbound-endpoints').toString(); | ||
|
|
||
| const { xml } = await rpcClient.getMiDiagramRpcClient().buildMcpToolsXml({ projectRoot: projectDir, tools }); | ||
| const localEntryName = `${data.serverName}${MCP_CONFIG_NAME_SUFFIX}`; | ||
|
|
||
| await rpcClient.getMiDiagramRpcClient().createLocalEntry({ | ||
| directory: localEntriesDir, | ||
| name: localEntryName, | ||
| type: 'In-Line XML Entry', | ||
| value: xml, | ||
| URL: '', | ||
| getContentOnly: false, | ||
| }); | ||
|
|
||
| await rpcClient.getMiDiagramRpcClient().createInboundEndpoint({ | ||
| directory: inboundEndpointsDir, | ||
| attributes: { | ||
| name: `${data.serverName}`, | ||
| sequence: '', | ||
| onError: '', | ||
| class: inboundListenerClass, | ||
| }, | ||
| parameters: { | ||
| 'inbound.mcp.port': data.port, | ||
| 'inbound.http.port': data.port, | ||
| 'inbound.http.context': '/mcp', | ||
| 'mcp.tools.localentry': localEntryName, | ||
| 'inbound.behavior': 'listening', | ||
| 'inbound.cors.allow.origin': corsSettings.corsAllowOrigin, | ||
| 'inbound.cors.allow.methods': corsSettings.corsAllowMethods, | ||
| 'inbound.cors.allow.headers': corsSettings.corsAllowHeaders, | ||
| 'inbound.cors.expose.headers': corsSettings.corsExposeHeaders, | ||
| 'inbound.sse.keepalive.interval': corsSettings.keepAliveInterval, | ||
| }, | ||
| }); | ||
|
|
||
| rpcClient.getMiVisualizerRpcClient().showNotification({ | ||
| message: `MCP Server "${data.serverName}" created with ${tools.length} tool(s)`, | ||
| type: 'info', | ||
| }); | ||
| rpcClient.getMiVisualizerRpcClient().openView({ | ||
| type: EVENT_TYPE.OPEN_VIEW, | ||
| location: { view: MACHINE_VIEW.Overview }, | ||
| }); | ||
| } catch (err) { | ||
| setError(`Failed to create MCP Server: ${err instanceof Error ? err.message : String(err)}`); | ||
| } finally { | ||
| setSubmitting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Missing port-uniqueness validation in the create flow.
usedPorts is loaded (Line 247-251) and enforced in handleUpdate for edit mode (Line 444: usedPorts.has(portValue) && portValue !== originalPort), but onSubmit (create mode) never checks it before calling createInboundEndpoint. Users can create a new MCP server on a port that's already bound by another inbound endpoint in the same project, which will likely fail at MI runtime/deployment instead of being caught in the UI.
🐛 Suggested fix
const onSubmit = async (data: any) => {
setSubmitting(true);
setError(null);
try {
+ const portValue = Number(data.port);
+ if (usedPorts.has(portValue)) {
+ setFieldError('port', { message: `Port ${portValue} is already in use by another inbound endpoint in this project` });
+ setSubmitting(false);
+ return;
+ }
const projectRootResp = await rpcClient.getMiDiagramRpcClient().getProjectRoot({ path: props.path });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onSubmit = async (data: any) => { | |
| setSubmitting(true); | |
| setError(null); | |
| try { | |
| const projectRootResp = await rpcClient.getMiDiagramRpcClient().getProjectRoot({ path: props.path }); | |
| const projectDir = projectRootResp.path; | |
| const localEntriesDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'local-entries').toString(); | |
| const inboundEndpointsDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'inbound-endpoints').toString(); | |
| const { xml } = await rpcClient.getMiDiagramRpcClient().buildMcpToolsXml({ projectRoot: projectDir, tools }); | |
| const localEntryName = `${data.serverName}${MCP_CONFIG_NAME_SUFFIX}`; | |
| await rpcClient.getMiDiagramRpcClient().createLocalEntry({ | |
| directory: localEntriesDir, | |
| name: localEntryName, | |
| type: 'In-Line XML Entry', | |
| value: xml, | |
| URL: '', | |
| getContentOnly: false, | |
| }); | |
| await rpcClient.getMiDiagramRpcClient().createInboundEndpoint({ | |
| directory: inboundEndpointsDir, | |
| attributes: { | |
| name: `${data.serverName}`, | |
| sequence: '', | |
| onError: '', | |
| class: inboundListenerClass, | |
| }, | |
| parameters: { | |
| 'inbound.mcp.port': data.port, | |
| 'inbound.http.port': data.port, | |
| 'inbound.http.context': '/mcp', | |
| 'mcp.tools.localentry': localEntryName, | |
| 'inbound.behavior': 'listening', | |
| 'inbound.cors.allow.origin': corsSettings.corsAllowOrigin, | |
| 'inbound.cors.allow.methods': corsSettings.corsAllowMethods, | |
| 'inbound.cors.allow.headers': corsSettings.corsAllowHeaders, | |
| 'inbound.cors.expose.headers': corsSettings.corsExposeHeaders, | |
| 'inbound.sse.keepalive.interval': corsSettings.keepAliveInterval, | |
| }, | |
| }); | |
| rpcClient.getMiVisualizerRpcClient().showNotification({ | |
| message: `MCP Server "${data.serverName}" created with ${tools.length} tool(s)`, | |
| type: 'info', | |
| }); | |
| rpcClient.getMiVisualizerRpcClient().openView({ | |
| type: EVENT_TYPE.OPEN_VIEW, | |
| location: { view: MACHINE_VIEW.Overview }, | |
| }); | |
| } catch (err) { | |
| setError(`Failed to create MCP Server: ${err instanceof Error ? err.message : String(err)}`); | |
| } finally { | |
| setSubmitting(false); | |
| } | |
| }; | |
| const onSubmit = async (data: any) => { | |
| setSubmitting(true); | |
| setError(null); | |
| try { | |
| const portValue = Number(data.port); | |
| if (usedPorts.has(portValue)) { | |
| setFieldError('port', { message: `Port ${portValue} is already in use by another inbound endpoint in this project` }); | |
| setSubmitting(false); | |
| return; | |
| } | |
| const projectRootResp = await rpcClient.getMiDiagramRpcClient().getProjectRoot({ path: props.path }); | |
| const projectDir = projectRootResp.path; | |
| const localEntriesDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'local-entries').toString(); | |
| const inboundEndpointsDir = path.join(projectDir, 'src', 'main', 'wso2mi', 'artifacts', 'inbound-endpoints').toString(); | |
| const { xml } = await rpcClient.getMiDiagramRpcClient().buildMcpToolsXml({ projectRoot: projectDir, tools }); | |
| const localEntryName = `${data.serverName}${MCP_CONFIG_NAME_SUFFIX}`; | |
| await rpcClient.getMiDiagramRpcClient().createLocalEntry({ | |
| directory: localEntriesDir, | |
| name: localEntryName, | |
| type: 'In-Line XML Entry', | |
| value: xml, | |
| URL: '', | |
| getContentOnly: false, | |
| }); | |
| await rpcClient.getMiDiagramRpcClient().createInboundEndpoint({ | |
| directory: inboundEndpointsDir, | |
| attributes: { | |
| name: `${data.serverName}`, | |
| sequence: '', | |
| onError: '', | |
| class: inboundListenerClass, | |
| }, | |
| parameters: { | |
| 'inbound.mcp.port': data.port, | |
| 'inbound.http.port': data.port, | |
| 'inbound.http.context': '/mcp', | |
| 'mcp.tools.localentry': localEntryName, | |
| 'inbound.behavior': 'listening', | |
| 'inbound.cors.allow.origin': corsSettings.corsAllowOrigin, | |
| 'inbound.cors.allow.methods': corsSettings.corsAllowMethods, | |
| 'inbound.cors.allow.headers': corsSettings.corsAllowHeaders, | |
| 'inbound.cors.expose.headers': corsSettings.corsExposeHeaders, | |
| 'inbound.sse.keepalive.interval': corsSettings.keepAliveInterval, | |
| }, | |
| }); | |
| rpcClient.getMiVisualizerRpcClient().showNotification({ | |
| message: `MCP Server "${data.serverName}" created with ${tools.length} tool(s)`, | |
| type: 'info', | |
| }); | |
| rpcClient.getMiVisualizerRpcClient().openView({ | |
| type: EVENT_TYPE.OPEN_VIEW, | |
| location: { view: MACHINE_VIEW.Overview }, | |
| }); | |
| } catch (err) { | |
| setError(`Failed to create MCP Server: ${err instanceof Error ? err.message : String(err)}`); | |
| } finally { | |
| setSubmitting(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/MCPServerToolsForm.tsx`
around lines 476 - 532, The create flow in onSubmit currently skips the same
port-uniqueness check that handleUpdate uses, so a new MCP server can be created
with a port already present in usedPorts. Add a validation step in onSubmit
before buildMcpToolsXml/createInboundEndpoint that checks the submitted port
against usedPorts and blocks submission when it is already taken. Reuse the
existing port parsing/validation pattern from handleUpdate and surface the error
via setError so the user sees the conflict before any RPC calls are made.
| const saveEdit = async () => { | ||
| if (!editingTool) return; | ||
| if (schemaError) return; | ||
|
|
||
| const normalizedSchema = editToolInputSchema.trim() | ||
| ? (await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: editToolInputSchema })).schema | ||
| : null; | ||
|
|
||
| const updatedTools = tools.map(t => { | ||
| if (t.id !== editingTool.id) return t; | ||
| return { ...t, name: editToolName.trim() || t.name, description: editToolDescription, inputSchema: normalizedSchema || t.inputSchema }; | ||
| }); | ||
| onSave(updatedTools); | ||
| setEditingTool(null); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clearing the input schema in Edit Tool has no effect.
When a user empties the schema textarea and saves, normalizedSchema resolves to null (Line 99-101), but Line 105's normalizedSchema || t.inputSchema falls back to the tool's old schema instead of clearing it. Since the Save button doesn't require a non-empty schema, users have no way to remove a previously-set schema. Compare with AddSequenceToolDialog.tsx/CreateScratchToolDialog.tsx, which use converted || EMPTY_MCP_SCHEMA for the equivalent empty case.
🐛 Suggested fix
+import { EMPTY_MCP_SCHEMA, INVALID_MCP_SCHEMA_MESSAGE } from '../../../constants';
...
const updatedTools = tools.map(t => {
if (t.id !== editingTool.id) return t;
- return { ...t, name: editToolName.trim() || t.name, description: editToolDescription, inputSchema: normalizedSchema || t.inputSchema };
+ return { ...t, name: editToolName.trim() || t.name, description: editToolDescription, inputSchema: editToolInputSchema.trim() ? (normalizedSchema ?? t.inputSchema) : EMPTY_MCP_SCHEMA };
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const saveEdit = async () => { | |
| if (!editingTool) return; | |
| if (schemaError) return; | |
| const normalizedSchema = editToolInputSchema.trim() | |
| ? (await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: editToolInputSchema })).schema | |
| : null; | |
| const updatedTools = tools.map(t => { | |
| if (t.id !== editingTool.id) return t; | |
| return { ...t, name: editToolName.trim() || t.name, description: editToolDescription, inputSchema: normalizedSchema || t.inputSchema }; | |
| }); | |
| onSave(updatedTools); | |
| setEditingTool(null); | |
| }; | |
| const saveEdit = async () => { | |
| if (!editingTool) return; | |
| if (schemaError) return; | |
| const normalizedSchema = editToolInputSchema.trim() | |
| ? (await rpcClient.getMiDiagramRpcClient().convertMcpJsonSchema({ input: editToolInputSchema })).schema | |
| : null; | |
| const updatedTools = tools.map(t => { | |
| if (t.id !== editingTool.id) return t; | |
| return { ...t, name: editToolName.trim() || t.name, description: editToolDescription, inputSchema: editToolInputSchema.trim() ? (normalizedSchema ?? t.inputSchema) : EMPTY_MCP_SCHEMA }; | |
| }); | |
| onSave(updatedTools); | |
| setEditingTool(null); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-visualizer/src/views/Forms/MCPServerForm/ToolsList.tsx`
around lines 95 - 109, The edit flow in saveEdit currently falls back to the
existing tool schema, so clearing the schema textarea never removes it. Update
the updatedTools mapping for the editingTool case so an empty schema is
preserved as an explicit cleared value instead of using normalizedSchema ||
t.inputSchema. Use the same empty-schema handling pattern as
AddSequenceToolDialog and CreateScratchToolDialog, and keep the logic tied to
saveEdit, editingTool, and normalizedSchema.
| const projectDetails = await rpcClient.getMiVisualizerRpcClient().getProjectDetails(); | ||
| const runtimeVersion = projectDetails.primaryDetails.runtimeVersion.value; | ||
| setIsStartOnLoadSupported(compareVersions(runtimeVersion, RUNTIME_VERSION_410) === 0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm compareVersions semantics (returns -1/0/1?) and check how it's used elsewhere for version gating.
rg -n -B2 -A2 'compareVersions\(' workspaces/mi --type=ts
# Locate the implementation to confirm return value semantics.
fd -a commons.ts workspaces/mi/mi-diagram/src/utils
# Confirm RUNTIME_VERSION_410 definition/usage pattern.
rg -n 'RUNTIME_VERSION_410' workspaces/mi --type=tsRepository: wso2/vscode-extensions
Length of output: 45184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the compareVersions implementation in both relevant places.
sed -n '92,150p' workspaces/mi/mi-diagram/src/utils/commons.ts
printf '\n----\n'
sed -n '1007,1065p' workspaces/mi/mi-extension/src/util/onboardingUtils.ts
printf '\n==== TaskForm surrounding logic ====\n'
sed -n '240,340p' workspaces/mi/mi-visualizer/src/views/Forms/TaskForm.tsx
printf '\n==== Search for startOnLoad usage ====\n'
rg -n 'startOnLoad|isStartOnLoadSupported|RUNTIME_VERSION_410' workspaces/mi/mi-visualizer/src/views/Forms/TaskForm.tsx workspaces/mi/mi-visualizer/src -g '!**/node_modules/**'Repository: wso2/vscode-extensions
Length of output: 12453
Use a minimum-version gate for startOnLoad
compareVersions(runtimeVersion, RUNTIME_VERSION_410) === 0 only enables this for exactly 4.1.0; 4.1.1+ will still hide the checkbox and drop startOnLoad from the task request.
Suggested fix
- setIsStartOnLoadSupported(compareVersions(runtimeVersion, RUNTIME_VERSION_410) === 0);
+ setIsStartOnLoadSupported(compareVersions(runtimeVersion, RUNTIME_VERSION_410) >= 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const projectDetails = await rpcClient.getMiVisualizerRpcClient().getProjectDetails(); | |
| const runtimeVersion = projectDetails.primaryDetails.runtimeVersion.value; | |
| setIsStartOnLoadSupported(compareVersions(runtimeVersion, RUNTIME_VERSION_410) === 0); | |
| const projectDetails = await rpcClient.getMiVisualizerRpcClient().getProjectDetails(); | |
| const runtimeVersion = projectDetails.primaryDetails.runtimeVersion.value; | |
| setIsStartOnLoadSupported(compareVersions(runtimeVersion, RUNTIME_VERSION_410) >= 0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-visualizer/src/views/Forms/TaskForm.tsx` around lines 267 -
269, The `startOnLoad` gating in `TaskForm` is using an exact version match, so
versions newer than 4.1.0 are incorrectly treated as unsupported. Update the
version check around `getProjectDetails`, `runtimeVersion`, and
`setIsStartOnLoadSupported` to use a minimum-version comparison so `4.1.0` and
any later runtime versions still enable the checkbox and preserve `startOnLoad`
in the task request.
| const deleteMCPServer = (entry: any) => { | ||
| const inboundPath = entry.inboundEndpoint?.path; | ||
| const localEntryPath = entry.localEntry?.path; | ||
|
|
||
| const confirmed = window.confirm(`Are you sure you want to delete MCP Server "${entry.name}"? This action cannot be undone.`); | ||
| if (!confirmed) { | ||
| return; | ||
| } | ||
|
|
||
| if (inboundPath) { | ||
| rpcClient.getMiDiagramRpcClient().deleteArtifact({ path: inboundPath, enableUndo: false }); | ||
| } | ||
| if (localEntryPath) { | ||
| rpcClient.getMiDiagramRpcClient().deleteArtifact({ path: localEntryPath, enableUndo: false }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether window.confirm/alert is used elsewhere in the mi-visualizer/mi-extension webview code,
# and whether any webview panel option enables modals.
rg -nP '\bwindow\.(confirm|alert)\s*\(' workspaces/mi --type=ts --type=tsx
rg -nP 'allow-modals|retainContextWhenHidden|WebviewPanelOptions' workspaces/mi/mi-extension/srcRepository: wso2/vscode-extensions
Length of output: 610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant webview panel setup =="
sed -n '1,140p' workspaces/mi/mi-extension/src/visualizer/webview.ts
printf '\n== project structure view around delete flow ==\n'
sed -n '250,340p' workspaces/mi/mi-visualizer/src/views/Overview/ProjectStructureView.tsx
printf '\n== search for dialog primitives in mi-visualizer ==\n'
rg -n "AddAPIToolDialog|Dialog|Alert|confirm\\(" workspaces/mi/mi-visualizer/src workspaces/mi/mi-extension/srcRepository: wso2/vscode-extensions
Length of output: 45794
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== webview html generation snippets =="
rg -n "sandbox|allow-modals|webview.*html|iframe" workspaces/mi/mi-extension/src/visualizer/webview.ts workspaces/mi/mi-extension/src/runtime-services-panel/webview.ts workspaces/mi/mi-extension/src/ai-features/webview.ts workspaces/mi/mi-extension/src/swagger/webview.tsRepository: wso2/vscode-extensions
Length of output: 753
window.confirm() won’t work here
deleteMCPServer relies on a modal confirm, but this webview is created without modal support. The delete action will return early every time, so MCP servers can’t be deleted from this view. Use the existing Dialog pattern instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/mi/mi-visualizer/src/views/Overview/ProjectStructureView.tsx`
around lines 310 - 325, The delete flow in deleteMCPServer currently uses
window.confirm, which does not work in this webview and blocks deletion. Replace
it with the existing Dialog-based confirmation pattern used in
ProjectStructureView so the user can confirm deletion without relying on browser
modal support. Update the deleteMCPServer entry point and its surrounding
state/handlers to open the Dialog, and only call
rpcClient.getMiDiagramRpcClient().deleteArtifact after the dialog confirms.
6 participants
This PR syncs changes from stable/mi to the main branch.
Automated PR created by GitHub Actions.
Summary by CodeRabbit
New Features
Bug Fixes