Introduce Ballerina Security Scanner UI to VS Code Extension#2217
Open
sdmdg wants to merge 6 commits into
Open
Introduce Ballerina Security Scanner UI to VS Code Extension#2217sdmdg wants to merge 6 commits into
sdmdg wants to merge 6 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a Ballerina security scanner end-to-end: typed RPC contracts and re-exports, extension manifest and wiring, scanner utilities and rules, ScannerRpcManager and client, webview UI and Visualizer integration, pre-deploy gating and banners, and AI-agent security diagnostics tool with prompt integration. ChangesSecurity Scanner Feature Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts (1)
134-137:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
openAIWebviewWithPrompt: unreachablevscode.Uriguard introduces anulltype mismatch.
defaultPromptis typedAIPanelPrompt | undefined, so theinstanceof vscode.Uribranch is statically unreachable, and thenullit produces is not assignable toopenAIWebview'sdefaultprompt?: AIPanelPromptparameter (compile error withstrictNullChecks). Thevscode.Urifilter is already applied upstream inresolveDefaultPromptArg(line 90), making this guard redundant.🛠 Proposed fix
function openAIWebviewWithPrompt(defaultPrompt?: AIPanelPrompt): void { - const prompt = defaultPrompt instanceof vscode.Uri ? null : defaultPrompt; - openAIWebview(prompt); + openAIWebview(defaultPrompt); }🤖 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/ballerina/ballerina-extension/src/views/ai-panel/activate.ts` around lines 134 - 137, The guard using instanceof vscode.Uri in openAIWebviewWithPrompt is unreachable and introduces a null that doesn't match openAIWebview's AIPanelPrompt parameter; remove the vscode.Uri check and forward defaultPrompt directly to openAIWebview (i.e., let prompt = defaultPrompt or call openAIWebview(defaultPrompt)), relying on upstream resolveDefaultPromptArg to filter Uris; this keeps types aligned and fixes the strictNullChecks compile error.
🧹 Nitpick comments (20)
workspaces/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.ts (2)
147-150: ⚡ Quick win
ScanResponse.activeIssuesandexcludedIssuestyped asany[]— useScannerIssueContext[]instead.
ScannerIssueContextis already defined in this same file. Usingany[]erases all type safety for the core response payload, silencing TypeScript on malformed issue objects at every call site.♻️ Proposed fix
export interface ScanResponse extends BaseResponse { - activeIssues?: any[]; - excludedIssues?: any[]; + activeIssues?: ScannerIssueContext[]; + excludedIssues?: ScannerIssueContext[]; }🤖 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/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.ts` around lines 147 - 150, The ScanResponse interface currently types activeIssues and excludedIssues as any[] which loses type safety; change both properties to use the existing ScannerIssueContext[] type (i.e., activeIssues?: ScannerIssueContext[]; excludedIssues?: ScannerIssueContext[];) so callers get proper typing and compile-time checks; update any related imports/exports in this file if needed to ensure ScannerIssueContext is visible to ScanResponse.
54-61: ⚡ Quick winNaming inconsistencies in new public API interfaces.
Two violations in this new API surface:
ScannerExclusionContext.IssueContext(line 60): fields should becamelCase; this should beissueContext.includeIssueResponse/enableRuleResponse(lines 167, 173): interface names should bePascalCaseto match every other interface in the file (AddExclusionResponse,AddGlobalExclusionResponse, etc.); these should beIncludeIssueResponse/EnableRuleResponse.Since these are being introduced fresh in this PR, fixing the names now avoids a future breaking rename for all downstream consumers.
♻️ Proposed fix
export interface ScannerExclusionContext { filePath: string; lineHash: string; ruleId: string; symbol: string; isGlobalExclusion?: boolean; - IssueContext: ScannerIssueContext; + issueContext: ScannerIssueContext; }-export interface includeIssueResponse extends BaseResponse { +export interface IncludeIssueResponse extends BaseResponse { } -export interface enableRuleResponse extends BaseResponse { +export interface EnableRuleResponse extends BaseResponse { }Also applies to: 167-174
🤖 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/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.ts` around lines 54 - 61, Rename the two inconsistent public API identifiers: change the property ScannerExclusionContext.IssueContext to use camelCase (issueContext) and rename the interfaces includeIssueResponse and enableRuleResponse to PascalCase (IncludeIssueResponse and EnableRuleResponse); update all references/exports/usages of ScannerExclusionContext, IncludeIssueResponse and EnableRuleResponse (e.g., where they are imported, instantiated, or typed) so the new names are used everywhere to avoid breaking consumers.workspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts (4)
416-422: 💤 Low valueFunction naming violates JS convention.
AddSecurityToolPromptis exported with PascalCase but is neither a class nor a JSX component. Convention here would beaddSecurityToolPrompt(matching the rest of the agent helpers in this folder).🤖 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/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts` around lines 416 - 422, Rename the exported function AddSecurityToolPrompt to follow JS naming conventions (addSecurityToolPrompt) and update any references; specifically change the function declaration/export name for AddSecurityToolPrompt to addSecurityToolPrompt and ensure callers/imports use the new identifier as well, keeping the internal logic (isScannerActive check and return behavior) unchanged.
362-375: ⚡ Quick win
userInputRequiredCountis computed over the full list but onlyMAX_ISSUESare returned.When
filteredIssues.length > MAX_ISSUES, the message can say e.g. "8 issue(s) have autoFixEnabled=false" while only the first 50 are actually visible to the agent. Compute the count overissuesToReturn(or clarify the wording) so the agent's "STRICT RULE" guidance refers to issues it can actually see.♻️ Proposed fix
- const issuesToReturn = filteredIssues.slice(0, MAX_ISSUES); - const userInputRequiredCount = filteredIssues.filter((issue) => issue.requiresExplicitUserInput).length; + const issuesToReturn = filteredIssues.slice(0, MAX_ISSUES); + const userInputRequiredCount = issuesToReturn.filter((issue) => issue.requiresExplicitUserInput).length;🤖 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/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts` around lines 362 - 375, The "STRICT RULE" count is computed from filteredIssues but only issuesToReturn (sliced to MAX_ISSUES) are shown; update the logic so userInputRequiredCount is derived from issuesToReturn (or change the message to clarify it refers to the full set). Specifically, replace the filteredIssues.filter(...) call used to compute userInputRequiredCount with a filter over issuesToReturn (or adjust the outputMessage text) so the count matches what the agent can actually see; keep references to issuesToReturn, filteredIssues, userInputRequiredCount, and MAX_ISSUES to locate and update the code.
195-207: ⚡ Quick win
runWithLockcan deadlock ifawait currentMutexever rejects.If
currentMutexrejects (today it shouldn't, but a future change to howscanMutexis composed could introduce it),await currentMutexthrows before thetryblock, sorelease()is never invoked andnextMutexstays pending forever — every subsequent caller will be wedged. Defensively wrap theawait/task()in a try/finally that always releases.♻️ Hardened version
-async function runWithLock<T>(task: () => Promise<T>): Promise<T> { - const currentMutex = scanMutex; - let release: () => void; - const nextMutex = new Promise<void>(resolve => { release = resolve; }); - scanMutex = currentMutex.then(() => nextMutex); - - await currentMutex; - try { - return await task(); - } finally { - release!(); - } -} +async function runWithLock<T>(task: () => Promise<T>): Promise<T> { + const currentMutex = scanMutex; + let release!: () => void; + const nextMutex = new Promise<void>(resolve => { release = resolve; }); + scanMutex = currentMutex.then(() => nextMutex, () => nextMutex); + + try { + await currentMutex.catch(() => {/* don't propagate prior failures */}); + return await task(); + } finally { + release(); + } +}🤖 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/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts` around lines 195 - 207, The runWithLock function can deadlock if await currentMutex rejects because release() is never called; fix it by ensuring nextMutex is always resolved regardless of currentMutex outcome and that release() is invoked in a finally block: create nextMutex and its release as before, set scanMutex = currentMutex.then(() => nextMutex, () => nextMutex) to propagate resolution on both fulfillment and rejection, then await currentMutex.catch(() => undefined) (or wrap the await in try/catch) and execute the task inside a try/finally where the finally calls release!(); reference runWithLock, currentMutex, nextMutex, release and scanMutex when making the change.
99-115: 💤 Low valueUse
@iarna/tomlparser instead of regex for workspace packages.
readWorkspacePackagesFromTomlmanually parsesBallerina.tomlwith regex, which misses:
- comments inside the array (
# "pkg-a",)- single-quoted strings (
'pkg-a')- arrays with trailing commas and other complexities
[workspace]appearing in another section's value
@iarna/toml(already in dependencies and used across the codebase) properly handles TOML syntax. Replace the regex logic withparse()from@iarna/toml, similar to howtoml-utils.tshandles Config.toml parsing.🤖 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/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts` around lines 99 - 115, Replace the fragile regex parsing in readWorkspacePackagesFromToml with the `@iarna/toml` parser: read Ballerina.toml from tempProjectPath, call parse(tomlContent) (imported from '@iarna/toml'), then safely extract the array at parsed.workspace?.packages, coerce its elements to strings and return them; on any parse/read error or if workspace/packages are missing, return an empty array. Ensure you update imports to include parse from '@iarna/toml' and remove the regex-based extraction inside readWorkspacePackagesFromToml.workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts (4)
170-174: ⚡ Quick winUse
??for line/column defaults to preserve0.
params.issue.startLine || 0is mostly fine since0falls back to0, butendLine = params.issue.endLine || startLinewill incorrectly substitutestartLinewhenendLineis a legitimate0. Same forendColumn. Switch to nullish coalescing.♻️ Proposed fix
- const startLine = params.issue.startLine || 0; - const startCol = params.issue.startColumn || 0; - const endLine = params.issue.endLine || startLine; - const endCol = params.issue.endColumn || startCol; + const startLine = params.issue.startLine ?? 0; + const startCol = params.issue.startColumn ?? 0; + const endLine = params.issue.endLine ?? startLine; + const endCol = params.issue.endColumn ?? startCol;🤖 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/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts` around lines 170 - 174, The range construction uses logical OR which treats legitimate 0 values as missing; in the block that computes startLine/startCol/endLine/endCol (variables startLine, startCol, endLine, endCol in rpc-manager.ts) replace uses of "||" with the nullish coalescing operator "??" so defaults only apply for null/undefined (e.g. use params.issue.startLine ?? 0 and params.issue.endLine ?? startLine, same for startColumn/endColumn) before creating the vscode.Range to preserve 0 indices correctly.
543-549: 💤 Low valueUnnecessary defensive try/catch around
showErrorMessage.
vscode.window.showErrorMessagereturns aThenableand does not throw synchronously. The wrapping try/catch only obscures the control flow. Just call it directly (or.catch(...)if you want to log a UI failure).🤖 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/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts` around lines 543 - 549, The try/catch around vscode.window.showErrorMessage that swallows UI errors is unnecessary; remove the synchronous try/catch and call vscode.window.showErrorMessage(lsError) directly (or append .catch(err => console.error("[Scanner] Failed to show error message:", err)) if you want to log UI failures) in the rpc-manager.ts scan response handling so the ScanResponse control flow is not obscured.
207-244: 💤 Low value
findEnclosingArtifactPositionmixes O(N) traversal andanytyping — extract and tighten types.The triple-nested loop traverses every artifact and resource on every reveal call, and
bestMatch.positionis typed asany. While project structures are typically small, consider:
- Replacing
anywith the actual position shape returned (matches the function's return type),- Caching
path === filePathmatches by file path if reveal is called frequently.Not blocking; flagging as a maintainability follow-up.
🤖 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/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts` around lines 207 - 244, findEnclosingArtifactPosition currently uses an any-typed bestMatch.position and re-traverses the full projectStructure on every call; change bestMatch to use the actual Position interface (the same type returned by the function) instead of any, tighten types for projectStructure/projects/directoryMap/artifact/resources to reflect Position and artifact shapes, and introduce a simple filePath-to-Position cache keyed by filePath inside the RPC manager so that if the same filePath is requested repeatedly you return the cached enclosing position instead of walking projectStructure; keep the original traversal (iterating project.directoryMap → artifacts → artifact.resources) but move it into a private helper method (e.g., findPositionForPath) to make caching and typed returns straightforward and update references to resource.position and artifact.position accordingly so bestMatch.position is strongly typed.
190-193: 💤 Low valueMisleading error message in catch.
The single error message
Could not open file: ${params.filePath}is shown for any failure inside the try block — including failures ofexecuteCommand('ballerina.showVisualizer', ...)orentityDiagram, which have nothing to do with file opening. Either narrow the try blocks or includeerror.messagein the surfaced text.🤖 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/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts` around lines 190 - 193, The catch block around the reveal flow is surfacing a misleading static message; narrow the try blocks or include the real error text so failures in executeCommand('ballerina.showVisualizer', ...) or entityDiagram aren’t reported as "Could not open file". Update the code around reveal/open logic (references: params.filePath, vscode.window.showErrorMessage, console.error) to either wrap the file-open/reveal call in its own try/catch and show "Could not open file: ${params.filePath}" only there, and separately catch errors from executeCommand('ballerina.showVisualizer', ...) and entityDiagram, or keep the single catch but append error.message (or error.toString()) to the vscode.window.showErrorMessage and to console.error so the real failure is surfaced.workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts (2)
141-141: 💤 Low valueSeverity fallback to
'LOW'should come from the LS — confirm before release.The inline comment notes severity should be authoritative from LS. Defaulting silently to
'LOW'may misrepresent vulnerability impact in the UI (e.g., a HIGH issue rendered as LOW ifgetRuleSeverityreturns falsy for a new/unknown rule). Consider defaulting to a non-classifying value like'UNKNOWN'until the LS supplies severity, or fail loud in dev builds.🤖 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/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts` at line 141, The current fallback "severity: severity || 'LOW'" in scan-utils.ts silently demotes unknown severities; update the code that sets the severity (the variable named severity used when building the diagnostic/issue object in the scan-utils.ts helper) to use a non-classifying fallback such as 'UNKNOWN' instead of 'LOW' (or, optionally, throw or log loudly in dev builds), and ensure this change interacts with getRuleSeverity (or whatever LS severity provider you call) so the LS remains authoritative for final severity values.
105-112: 💤 Low valueFloating promise on
showInformationMessage.The notification chain (
.then(...)) is fire-and-forget. Any error from the user-action handler (e.g.,executeCommandrejection) is swallowed with no logging. Consider awaiting it inside the try block or attaching a.catchto surface failures via the output channel.🤖 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/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts` around lines 105 - 112, The current use of vscode.window.showInformationMessage(...).then(...) creates a floating promise that can swallow errors from the user-action handler (vscode.commands.executeCommand); update the logic in scan-utils.ts to await the showInformationMessage call inside the existing try block (or attach a .catch) and handle rejections by logging to the extension output channel or logger; specifically modify the call sites using showInformationMessage and executeCommand so any rejection from executeCommand is caught and reported (use try/catch around the awaited showInformationMessage/executeCommand or add a .catch that calls the extension output channel/logger).workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts (2)
27-39: 💤 Low value
columnis computed but ignored when creating a new panel.Line 28 computes
columnbased on whether an editor is active, butcreateWebviewalways passesvscode.ViewColumn.Beside(Line 51). The first-open path will not honor the active-editor branch. Either drop the unused calculation or passcolumnthrough tocreateWebview.🤖 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/ballerina/ballerina-extension/src/views/scanner/webview.ts` around lines 27 - 39, The computed local variable `column` in ScannerWebview.show is not used when creating a new panel, so update the new-panel creation path to pass that `column` into the panel creation instead of always using vscode.ViewColumn.Beside; specifically, change the call that constructs a new ScannerWebview (and the underlying createWebview factory/constructor) to accept and use the `column` value, or alternatively remove the unused `column` if you intentionally want Beside — modify ScannerWebview.show and the ScannerWebview constructor/createWebview to accept a column parameter (and ensure the constructor forwards it to createWebview) so the first-open path honors the active-editor logic.
1-7: ⚡ Quick winMissing license/copyright header.
This is a new file but does not carry the WSO2 Apache-2.0 header that the rest of the new files in this PR (e.g.,
activator.ts,ScannerOverview/index.tsx,usePreDeployScan.ts) include.🤖 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/ballerina/ballerina-extension/src/views/scanner/webview.ts` around lines 1 - 7, The file workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts is missing the WSO2 Apache-2.0 license header; add the same license/copyright header block used in the other new files in this PR (e.g., activator.ts, ScannerOverview/index.tsx, usePreDeployScan.ts) at the very top of webview.ts before any imports so it matches project convention and tooling; locate the file by the module symbols present (webview.ts containing imports for "vscode", "path", "extension", "getComposerWebViewOptions" and "getLibraryWebViewContent") and insert the identical header text used elsewhere in the repo.workspaces/ballerina/ballerina-extension/src/features/scanner/activator.ts (2)
142-158: 💤 Low valueDebounce timer can fire after disposal.
The watcher itself is disposed via
subscriptions, but the in-flightsetTimeout(Line 151) is not. On extension deactivation, a pending timer can still callRPCLayer._messenger.sendNotificationagainst a disposed messenger. ClearscannerContentChangedDebouncein a disposable that is registered alongside the watcher.♻️ Proposed fix
+ ballerinaExtInstance.context.subscriptions.push({ + dispose: () => { + if (scannerContentChangedDebounce) { + clearTimeout(scannerContentChangedDebounce); + scannerContentChangedDebounce = undefined; + } + } + });🤖 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/ballerina/ballerina-extension/src/features/scanner/activator.ts` around lines 142 - 158, The debounce timeout set in the scannerContentWatcher can fire after the watcher is disposed, risking calls to RPCLayer._messenger.sendNotification on a disposed messenger; to fix, ensure scannerContentChangedDebounce is cleared when the watcher is disposed by creating and registering a disposable alongside scannerContentWatcher that calls clearTimeout(scannerContentChangedDebounce) and sets scannerContentChangedDebounce = undefined (or null), so any pending timer is cancelled before extension deactivation; update the registration where scannerContentWatcher is pushed to subscriptions to also push this new disposable (referencing scannerContentWatcher, scannerContentChangedDebounce, RPCLayer._messenger.sendNotification, and ScannerWebview.viewType).
21-21: ⚡ Quick winUse a relative import for
BallerinaExtension.
import { BallerinaExtension } from 'src/core'relies on a non-standard path mapping. Other files in this folder import via relative paths (e.g.,../../RPCLayer), and using'src/core'will only resolve ifbaseUrl/pathsare configured intsconfig, which can break compilation, type-checking, or downstream consumers.♻️ Proposed fix
-import { BallerinaExtension } from 'src/core'; +import { BallerinaExtension } from '../../core';🤖 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/ballerina/ballerina-extension/src/features/scanner/activator.ts` at line 21, The import for BallerinaExtension should use a project-relative path instead of 'src/core'; replace "import { BallerinaExtension } from 'src/core'" in activator.ts with the correct relative import (e.g., import from '../../core') so TypeScript resolves without depending on non-standard baseUrl/paths; update any other imports in this module that use absolute 'src/...' to matching relative paths.workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx (3)
27-34: ⚡ Quick win
__SCANNER_PROJECT_PATH__is missing from theWindowinterface.The ambient declaration on Lines 27-34 covers
__SCANNER_ENABLED__,__SCANNER_VERSION_SUPPORTED__,__SCANNER_STATE__, and__SCANNER_DEPLOY_MODE__, but Line 595 reads(window as any).__SCANNER_PROJECT_PATH__— losing typing and forcing the cast. Adding it to the interface keeps the access type-safe and consistent with the others.♻️ Proposed fix
declare global { interface Window { __SCANNER_ENABLED__?: boolean; __SCANNER_VERSION_SUPPORTED__?: boolean; __SCANNER_STATE__?: 'NOT_FOUND' | 'INCOMPATIBLE' | 'SUPPORTED'; __SCANNER_DEPLOY_MODE__?: boolean; + __SCANNER_PROJECT_PATH__?: string; } } @@ - const projectPath = propProjectPath || (typeof window !== "undefined" ? String((window as any).__SCANNER_PROJECT_PATH__ || "") : ""); + const projectPath = propProjectPath || (typeof window !== "undefined" ? (window.__SCANNER_PROJECT_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/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx` around lines 27 - 34, Add the missing __SCANNER_PROJECT_PATH__ property to the global Window interface declaration so callers like (window as any).__SCANNER_PROJECT_PATH__ can be typed safely; update the declare global { interface Window { ... } } block to include __SCANNER_PROJECT_PATH__?: string (or string | undefined) and then remove any unnecessary (window as any) casts where the typed Window is available (e.g., usages in ScannerOverview index.tsx).
889-893: ⚡ Quick winAuto-pull effect omits its function dependency.
useEffect(() => { ... handlePullScannerTool(); }, [scannerState])callshandlePullScannerToolwithout listing it. BecausehandlePullScannerToolcloses oversetPullFailed/setPullSuccessandscannerRpcClient, future refactors that capture additional state (e.g., per-pull tokens) will silently use a stale closure. WraphandlePullScannerToolinuseCallbackand include it in the deps, or inline the body into the effect.🤖 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/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx` around lines 889 - 893, The effect that auto-pulls the scanner calls handlePullScannerTool without listing it in deps, which risks stale closures over setPullFailed/setPullSuccess/scannerRpcClient; wrap handlePullScannerTool in useCallback (including any dependencies it uses such as setPullFailed, setPullSuccess, scannerRpcClient, etc.) and then include that memoized handlePullScannerTool in the useEffect dependency array, or alternatively move the pull logic inline into the useEffect so all referenced variables are correctly listed in the deps.
1077-1143: ⚖️ Poor tradeoffExtract the duplicated NOT_FOUND / INCOMPATIBLE pull screens.
The two early-return blocks (Lines 1077-1109 and 1111-1143) are nearly identical, differing only in the title/subtitle strings ("Pulling/Pulled" vs "Updating/Updated"). Lift them into a single helper component parameterized by
stateto keep both flows in sync as styles or copy evolve.🤖 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/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx` around lines 1077 - 1143, There are two nearly identical early-return blocks for scannerState === 'NOT_FOUND' and 'INCOMPATIBLE'; extract them into a single helper React component (e.g., ScannerToolStatusScreen) that accepts props (mode or state, pullFailed, pullSuccess, handlePullScannerTool, globalStyles) and renders the shared JSX with the title/subtitle strings chosen based on the mode/state; then replace the two return blocks in the main component with calls to <ScannerToolStatusScreen state={'NOT_FOUND'|'INCOMPATIBLE'} pullFailed={pullFailed} pullSuccess={pullSuccess} handlePullScannerTool={handlePullScannerTool} globalStyles={globalStyles}>, keeping existing elements (Global, LoadingContent, DownloadIcon, LoadingTitle, LoadingSubtitle, Retry Button, LoadingText) and their conditional logic intact so styles and behavior remain identical.workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts (1)
117-136: ⚡ Quick winUse typed error codes instead of substring matching for scanner availability detection.
Classifying scanner availability by checking if
errorMsg.toLowerCase().includes("disabled" | "not ready" | "not found" | "not supported")at lines 121–124 is fragile — any wording change in the backend error messages (e.g., inrpc-manager.tsscanProject()) silently alters the policy from "deploy directly" to "block deploy with error banner".The
ScanResponseinterface has only string fields (errorMsg,error). Consider adding anerrorCode?: stringfield toBaseResponseso the frontend classification remains independent of error message text and stays in sync with the language-server contract.🤖 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/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts` around lines 117 - 136, Add a typed errorCode to the response contract and switch the availability check to it: add errorCode?: string to BaseResponse/ScanResponse types and ensure scanProject() (rpc-manager.ts) returns one of well-defined codes (e.g. "SCANNER_DISABLED", "SCANNER_NOT_READY", "SCANNER_NOT_FOUND", "SCANNER_NOT_SUPPORTED"); then in usePreDeployScan.ts replace the fragile substring checks on result.errorMsg with a switch/if on result.errorCode (checking the four codes and setting scannerNotActive = true when matched), falling back to the existing errorMsg logging/scanFailedError behavior when errorCode is absent.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@workspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.ts`:
- Around line 31-34: The JSDoc for revealSecurityIssue is stale and refers to
the BI diagram; update the doc comment above the revealSecurityIssue method to
accurately describe its behavior (e.g., "Navigate to or reveal the security
issue location in the editor for the given RevealSecurityIssueRequest") and
remove any BI/diagram wording so the comment matches the scanner API's purpose
and the RevealSecurityIssueRequest parameter.
In `@workspaces/ballerina/ballerina-extension/package.json`:
- Around line 1128-1130: The Explorer command entry for "ballerina.scan.project"
should be hidden when the scanner feature or project gating is false; update its
"when" clause to combine the existing file check with the scanner/project gating
used elsewhere (replace the current "resourceScheme == file" with something like
"resourceScheme == file && <existingScannerEnabledCondition>" where
<existingScannerEnabledCondition> is the same expression used in other places to
show scan UI), so the menu item only appears when the scanner is enabled and the
current resource is a Ballerina project/file.
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/activator.ts`:
- Around line 199-212: The call to scannerRpcManager.scanProject in the command
handler is unprotected and can throw, so wrap the await
scannerRpcManager.scanProject({ projectPath: projectRootUri.fsPath }) in a
try/catch inside the activator command callback; on error, append a descriptive
error line to outputChannel (e.g., `[ERROR] [SCAN] Exception: ${error}`) and
call vscode.window.showErrorMessage with a user-friendly message, then
return—preserve the existing scanError handling for RPC-returned errors and only
use the catch for thrown exceptions.
- Around line 109-168: The function activate registers scannerConfigWatcher,
scannerContentWatcher, and pullToolDisposable before checking langClient,
causing a disposable leak if langClient is missing; fix by either moving the
langClient presence check (the langClient = ballerinaExtInstance.langClient
check and early return) to occur before creating/registering
scannerConfigWatcher, scannerContentWatcher, and pullToolDisposable, or by
immediately pushing each created disposable (scannerConfigWatcher,
scannerContentWatcher, pullToolDisposable) into
ballerinaExtInstance.context.subscriptions right after creation so they are
cleaned up even if activate returns early; update references in activate and
ensure ScannerRpcManager and related setup still occur only when appropriate.
- Around line 32-68: The checkScanToolVersion function can hang because it uses
exec('bal tool list') without a timeout; update it to use execFile (or exec with
args) with a timeout option and handle the timeout by invoking
callback('NOT_FOUND'); specifically, replace the exec call in
checkScanToolVersion with execFile('bal', ['tool','list'], { timeout:
<reasonable-ms> }, ...) (or exec with { timeout: <ms> }), detect the timeout
error case (error && error.killed or error.code === 'ETIMEDOUT') and call
callback('NOT_FOUND') in that branch, keeping existing logic for other errors
and successful stdout parsing.
- Around line 191-194: The module currently destructures
isScannerVersionSupported at import time so it becomes a static binding and
doesn't reflect updates from setScannerVersionSupported(); change to live
accessors by converting the exported isScannerVersionSupported (in
scan-utils.ts) into a function getter (e.g., export function
isScannerVersionSupported(): boolean) and standardize isScannerConfigEnabled to
the same shape, then update activator.ts to call these as functions (e.g., if
(!isScannerVersionSupported()) { ... }) or alternatively replace the
destructured import with a namespace import (import * as scanUtils) and
reference scanUtils.isScannerVersionSupported() to ensure live binding; update
any usages and tests that reference the old variable name accordingly.
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts`:
- Line 33: Replace the exported mutable let bindings with explicit accessor
functions: remove exported variables isScannerVersionSupported and scannerState
and instead export getIsScannerVersionSupported(),
setIsScannerVersionSupported(value), getScannerState(), and
setScannerState(value) so consumers always read current values (live binding
behavior) when compiled to CommonJS; update any callers (e.g., rpc-manager.ts
checks that reference isScannerVersionSupported) to call
getIsScannerVersionSupported() (and similarly use getScannerState()) and keep
existing setter names (setScannerVersionSupported/setScannerState) consistent or
rename as above so the module maintains internal state but exposes only
getters/setters.
In
`@workspaces/ballerina/ballerina-extension/src/features/scanner/security-rules.ts`:
- Around line 83-85: The function getRuleSeverity currently declares a return
type of ScannerRuleSeverity but calls SEVERITY_BY_RULE_ID.get(ruleId) which
returns ScannerRuleSeverity | undefined; update getRuleSeverity to either (a)
reflect the actual possible undefined by changing its signature to return
ScannerRuleSeverity | undefined, or (b) return a safe default for unknown IDs
(e.g., return SEVERITY_BY_RULE_ID.get(ruleId) ?? ScannerRuleSeverity.LOW) so
callers always receive a non-undefined ScannerRuleSeverity; modify the function
body and signature accordingly and ensure references in scan-utils.ts handle the
updated type or rely on the chosen default.
In
`@workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts`:
- Around line 313-317: The early-exit in the disableRule code path returns
undefined because it uses a bare `return;`; change that to `return false;` so
the method (Promise<boolean>) consistently returns false on failure; locate the
branch where documentUri is resolved (calls to resolveDocumentUri(...) inside
disableRule) and replace `return;` with `return false;`, and scan other
early-exit branches in the same disableRule implementation to ensure they also
return false rather than undefined.
In `@workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts`:
- Around line 123-135: The inline script in loadedScript -> renderDiagrams sets
window.__SCANNER_PROJECT_PATH__ and window.__SCANNER_STATE__ by interpolating
JSON.stringify(...) which can allow injection (e.g. containing </script> or
U+2028/U+2029); fix by not directly embedding raw JSON into the inline script:
either escape the JSON string for HTML/script context (replace '<' with '\u003c'
and replace U+2028/U+2029 with their escaped forms) before injecting into
window.__SCANNER_PROJECT_PATH__ and window.__SCANNER_STATE__, or remove those
interpolations and instead send currentScannerState and projectPath via the
webview messaging API (panel.webview.postMessage) after the page loads and
assign them inside renderDiagrams from the received message.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/AgentStreamView/StreamEntry.tsx`:
- Around line 158-164: In the switch branch for "getSecurityVulnerabilities"
inside StreamEntry.tsx, handle the case where toolOutput?.success === false but
toolOutput?.message is missing by returning an explicit failure label instead of
falling through to the count logic; update the case in the StreamEntry component
(the "getSecurityVulnerabilities" branch) to check for success === false and, if
there's no message, return something like { label: "Security scan failed" } (or
similar failure text) before computing count so the UI and error icon correctly
reflect a failed scan.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/ToolCallGroupSegment.tsx`:
- Around line 162-164: The current branch in ToolCallGroupSegment uses
hasSecurityDiagnostics && !hasFile which also matches mixed groups; change the
guard so the security-only copy is used only when the group contains security
diagnostics and no other diagnostic types. Update the condition (in
ToolCallGroupSegment) to something like hasSecurityDiagnostics && !hasFile &&
!hasCompilationDiagnostics (or equivalently check the group's diagnostics types
to ensure every diagnostic is of the security kind), or compute a new boolean
(e.g., isSecurityOnly) by inspecting diagnostics and use that instead.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx`:
- Around line 671-771: rescanScan currently runs without cancellation so
switching projects causes in-flight scans to overwrite state; add a cancellation
token (e.g., an AbortController or a mutable ref like currentScanRef with a
unique id/boolean) created in the useEffect before calling rescanScan and
cleared in the effect cleanup, pass or reference that token inside rescanScan,
and at each await boundary and inside the workspace projects loop check the
token (bail early if aborted) before calling setters (setIssuesByProject,
setLocalIssues, setRescannedAt, setIsScanning, setCurrentScanningLabel,
setExpandedProjects, setScanError, etc.); also ensure the effect does not start
a new scan while the previous one is still active (use the token or a running
flag) so only the latest scan updates state.
- Line 715: The fallback projectName expression uses pPath.split(/[\/]/).pop()
which only splits on forward slashes and fails on Windows paths; update the
split regex to /[\\/]/ so it matches both backslashes and slashes (mirroring
other usages on lines 595/621/689) when computing projectName in the object that
sets projectName: project.projectName || pPath.split(...).pop() || "Project".
- Around line 686-725: The loop currently breaks on per-project scan failure
which leaves no entry for the failed project in newIssuesByProject and aborts
later scans; change the error path inside the for-loop (inside the try block
handling scanResponse/errorMsg) to record the failure on the project entry
instead of breaking: set newIssuesByProject[pPath] = { projectPath: pPath,
projectName: project.projectName || ..., active: [], excluded: [], rescannedAt:
new Date(), loading: false, errorMsg: errorMsg || 'Scan failed due to an unknown
error' }, add pPath to initialExpandSet if isFirstScan so the error pill is
visible, call setIssuesByProject({ ...newIssuesByProject }), optionally set a
top-level setScanError once per overall run but do NOT break so subsequent
projects continue scanning.
- Around line 765-771: The current useEffect re-registers
rpcClient.getScannerRpcClient().onScannerContentChanged on every projectPath
change causing listener accumulation; refactor by removing the
onScannerContentChanged registration from the projectPath-dependent effect and
add a new useEffect with an empty dependency array that registers the
onScannerContentChanged listener once and calls setResultsOutdated(true) when
triggered; keep the existing setResultsOutdated(false) and rescanScan(false)
logic in the projectPath-dependent effect (where they belong), and if the
scanner RPC ever adds an unsubscribe/cleanup API, use it in the empty-deps
effect to unregister the listener on unmount.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts`:
- Around line 158-178: The onDeploy() calls inside triggerScanAndDeploy are
awaited without try/catch, so a rejection leaves UI state inconsistent; wrap
each await onDeploy() in a try/catch inside triggerScanAndDeploy (the call sites
that currently do await onDeploy()) and in the catch clear/normalize the
banner/UI state (call setBannerVisible(false), setScanResult(null), and
setScanError(err) or set an appropriate error message) and then rethrow or
return a handled error as your flow requires; ensure you update the same
state-reset logic at every onDeploy() invocation so bannerVisible/scanResult
can't remain showing stale success after a deployment failure.
- Around line 88-93: The hook currently sets setIsScanning(true) only inside the
250ms bannerTimer callback, causing isScanning to remain false for short scans;
move setIsScanning(true) to run immediately when scanning starts (outside the
setTimeout) so consumers see the correct scanning state, keep the timeout solely
for showing the banner (setBannerVisible(true) and bannerDidShow) and leave
bannerTimer/bannerDidShow logic unchanged; ensure any teardown still sets
setIsScanning(false) when the scan completes.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/WorkspaceOverview/index.tsx`:
- Around line 861-871: handleDeployWithScan currently builds scanTargets from
all non-library projects, but handleDeploy only deploys undeployedProjectScopes;
change handleDeployWithScan to scope scanning to the same undeployed set.
Replace the scanTargets computation to map over undeployedProjectScopes (or
derive undeployedProjectScopes from projectCollection if needed) and produce the
same { projectPath, projectName } shape, preserving the projectName fallback
logic, then call preDeploy.triggerScanAndDeploy(scanTargets, handleDeploy) so
only projects that will be deployed are scanned.
---
Outside diff comments:
In `@workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts`:
- Around line 134-137: The guard using instanceof vscode.Uri in
openAIWebviewWithPrompt is unreachable and introduces a null that doesn't match
openAIWebview's AIPanelPrompt parameter; remove the vscode.Uri check and forward
defaultPrompt directly to openAIWebview (i.e., let prompt = defaultPrompt or
call openAIWebview(defaultPrompt)), relying on upstream resolveDefaultPromptArg
to filter Uris; this keeps types aligned and fixes the strictNullChecks compile
error.
---
Nitpick comments:
In `@workspaces/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.ts`:
- Around line 147-150: The ScanResponse interface currently types activeIssues
and excludedIssues as any[] which loses type safety; change both properties to
use the existing ScannerIssueContext[] type (i.e., activeIssues?:
ScannerIssueContext[]; excludedIssues?: ScannerIssueContext[];) so callers get
proper typing and compile-time checks; update any related imports/exports in
this file if needed to ensure ScannerIssueContext is visible to ScanResponse.
- Around line 54-61: Rename the two inconsistent public API identifiers: change
the property ScannerExclusionContext.IssueContext to use camelCase
(issueContext) and rename the interfaces includeIssueResponse and
enableRuleResponse to PascalCase (IncludeIssueResponse and EnableRuleResponse);
update all references/exports/usages of ScannerExclusionContext,
IncludeIssueResponse and EnableRuleResponse (e.g., where they are imported,
instantiated, or typed) so the new names are used everywhere to avoid breaking
consumers.
In
`@workspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts`:
- Around line 416-422: Rename the exported function AddSecurityToolPrompt to
follow JS naming conventions (addSecurityToolPrompt) and update any references;
specifically change the function declaration/export name for
AddSecurityToolPrompt to addSecurityToolPrompt and ensure callers/imports use
the new identifier as well, keeping the internal logic (isScannerActive check
and return behavior) unchanged.
- Around line 362-375: The "STRICT RULE" count is computed from filteredIssues
but only issuesToReturn (sliced to MAX_ISSUES) are shown; update the logic so
userInputRequiredCount is derived from issuesToReturn (or change the message to
clarify it refers to the full set). Specifically, replace the
filteredIssues.filter(...) call used to compute userInputRequiredCount with a
filter over issuesToReturn (or adjust the outputMessage text) so the count
matches what the agent can actually see; keep references to issuesToReturn,
filteredIssues, userInputRequiredCount, and MAX_ISSUES to locate and update the
code.
- Around line 195-207: The runWithLock function can deadlock if await
currentMutex rejects because release() is never called; fix it by ensuring
nextMutex is always resolved regardless of currentMutex outcome and that
release() is invoked in a finally block: create nextMutex and its release as
before, set scanMutex = currentMutex.then(() => nextMutex, () => nextMutex) to
propagate resolution on both fulfillment and rejection, then await
currentMutex.catch(() => undefined) (or wrap the await in try/catch) and execute
the task inside a try/finally where the finally calls release!(); reference
runWithLock, currentMutex, nextMutex, release and scanMutex when making the
change.
- Around line 99-115: Replace the fragile regex parsing in
readWorkspacePackagesFromToml with the `@iarna/toml` parser: read Ballerina.toml
from tempProjectPath, call parse(tomlContent) (imported from '@iarna/toml'),
then safely extract the array at parsed.workspace?.packages, coerce its elements
to strings and return them; on any parse/read error or if workspace/packages are
missing, return an empty array. Ensure you update imports to include parse from
'@iarna/toml' and remove the regex-based extraction inside
readWorkspacePackagesFromToml.
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/activator.ts`:
- Around line 142-158: The debounce timeout set in the scannerContentWatcher can
fire after the watcher is disposed, risking calls to
RPCLayer._messenger.sendNotification on a disposed messenger; to fix, ensure
scannerContentChangedDebounce is cleared when the watcher is disposed by
creating and registering a disposable alongside scannerContentWatcher that calls
clearTimeout(scannerContentChangedDebounce) and sets
scannerContentChangedDebounce = undefined (or null), so any pending timer is
cancelled before extension deactivation; update the registration where
scannerContentWatcher is pushed to subscriptions to also push this new
disposable (referencing scannerContentWatcher, scannerContentChangedDebounce,
RPCLayer._messenger.sendNotification, and ScannerWebview.viewType).
- Line 21: The import for BallerinaExtension should use a project-relative path
instead of 'src/core'; replace "import { BallerinaExtension } from 'src/core'"
in activator.ts with the correct relative import (e.g., import from
'../../core') so TypeScript resolves without depending on non-standard
baseUrl/paths; update any other imports in this module that use absolute
'src/...' to matching relative paths.
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts`:
- Line 141: The current fallback "severity: severity || 'LOW'" in scan-utils.ts
silently demotes unknown severities; update the code that sets the severity (the
variable named severity used when building the diagnostic/issue object in the
scan-utils.ts helper) to use a non-classifying fallback such as 'UNKNOWN'
instead of 'LOW' (or, optionally, throw or log loudly in dev builds), and ensure
this change interacts with getRuleSeverity (or whatever LS severity provider you
call) so the LS remains authoritative for final severity values.
- Around line 105-112: The current use of
vscode.window.showInformationMessage(...).then(...) creates a floating promise
that can swallow errors from the user-action handler
(vscode.commands.executeCommand); update the logic in scan-utils.ts to await the
showInformationMessage call inside the existing try block (or attach a .catch)
and handle rejections by logging to the extension output channel or logger;
specifically modify the call sites using showInformationMessage and
executeCommand so any rejection from executeCommand is caught and reported (use
try/catch around the awaited showInformationMessage/executeCommand or add a
.catch that calls the extension output channel/logger).
In
`@workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts`:
- Around line 170-174: The range construction uses logical OR which treats
legitimate 0 values as missing; in the block that computes
startLine/startCol/endLine/endCol (variables startLine, startCol, endLine,
endCol in rpc-manager.ts) replace uses of "||" with the nullish coalescing
operator "??" so defaults only apply for null/undefined (e.g. use
params.issue.startLine ?? 0 and params.issue.endLine ?? startLine, same for
startColumn/endColumn) before creating the vscode.Range to preserve 0 indices
correctly.
- Around line 543-549: The try/catch around vscode.window.showErrorMessage that
swallows UI errors is unnecessary; remove the synchronous try/catch and call
vscode.window.showErrorMessage(lsError) directly (or append .catch(err =>
console.error("[Scanner] Failed to show error message:", err)) if you want to
log UI failures) in the rpc-manager.ts scan response handling so the
ScanResponse control flow is not obscured.
- Around line 207-244: findEnclosingArtifactPosition currently uses an any-typed
bestMatch.position and re-traverses the full projectStructure on every call;
change bestMatch to use the actual Position interface (the same type returned by
the function) instead of any, tighten types for
projectStructure/projects/directoryMap/artifact/resources to reflect Position
and artifact shapes, and introduce a simple filePath-to-Position cache keyed by
filePath inside the RPC manager so that if the same filePath is requested
repeatedly you return the cached enclosing position instead of walking
projectStructure; keep the original traversal (iterating project.directoryMap →
artifacts → artifact.resources) but move it into a private helper method (e.g.,
findPositionForPath) to make caching and typed returns straightforward and
update references to resource.position and artifact.position accordingly so
bestMatch.position is strongly typed.
- Around line 190-193: The catch block around the reveal flow is surfacing a
misleading static message; narrow the try blocks or include the real error text
so failures in executeCommand('ballerina.showVisualizer', ...) or entityDiagram
aren’t reported as "Could not open file". Update the code around reveal/open
logic (references: params.filePath, vscode.window.showErrorMessage,
console.error) to either wrap the file-open/reveal call in its own try/catch and
show "Could not open file: ${params.filePath}" only there, and separately catch
errors from executeCommand('ballerina.showVisualizer', ...) and entityDiagram,
or keep the single catch but append error.message (or error.toString()) to the
vscode.window.showErrorMessage and to console.error so the real failure is
surfaced.
In `@workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts`:
- Around line 27-39: The computed local variable `column` in ScannerWebview.show
is not used when creating a new panel, so update the new-panel creation path to
pass that `column` into the panel creation instead of always using
vscode.ViewColumn.Beside; specifically, change the call that constructs a new
ScannerWebview (and the underlying createWebview factory/constructor) to accept
and use the `column` value, or alternatively remove the unused `column` if you
intentionally want Beside — modify ScannerWebview.show and the ScannerWebview
constructor/createWebview to accept a column parameter (and ensure the
constructor forwards it to createWebview) so the first-open path honors the
active-editor logic.
- Around line 1-7: The file
workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts is missing
the WSO2 Apache-2.0 license header; add the same license/copyright header block
used in the other new files in this PR (e.g., activator.ts,
ScannerOverview/index.tsx, usePreDeployScan.ts) at the very top of webview.ts
before any imports so it matches project convention and tooling; locate the file
by the module symbols present (webview.ts containing imports for "vscode",
"path", "extension", "getComposerWebViewOptions" and "getLibraryWebViewContent")
and insert the identical header text used elsewhere in the repo.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx`:
- Around line 27-34: Add the missing __SCANNER_PROJECT_PATH__ property to the
global Window interface declaration so callers like (window as
any).__SCANNER_PROJECT_PATH__ can be typed safely; update the declare global {
interface Window { ... } } block to include __SCANNER_PROJECT_PATH__?: string
(or string | undefined) and then remove any unnecessary (window as any) casts
where the typed Window is available (e.g., usages in ScannerOverview index.tsx).
- Around line 889-893: The effect that auto-pulls the scanner calls
handlePullScannerTool without listing it in deps, which risks stale closures
over setPullFailed/setPullSuccess/scannerRpcClient; wrap handlePullScannerTool
in useCallback (including any dependencies it uses such as setPullFailed,
setPullSuccess, scannerRpcClient, etc.) and then include that memoized
handlePullScannerTool in the useEffect dependency array, or alternatively move
the pull logic inline into the useEffect so all referenced variables are
correctly listed in the deps.
- Around line 1077-1143: There are two nearly identical early-return blocks for
scannerState === 'NOT_FOUND' and 'INCOMPATIBLE'; extract them into a single
helper React component (e.g., ScannerToolStatusScreen) that accepts props (mode
or state, pullFailed, pullSuccess, handlePullScannerTool, globalStyles) and
renders the shared JSX with the title/subtitle strings chosen based on the
mode/state; then replace the two return blocks in the main component with calls
to <ScannerToolStatusScreen state={'NOT_FOUND'|'INCOMPATIBLE'}
pullFailed={pullFailed} pullSuccess={pullSuccess}
handlePullScannerTool={handlePullScannerTool} globalStyles={globalStyles}>,
keeping existing elements (Global, LoadingContent, DownloadIcon, LoadingTitle,
LoadingSubtitle, Retry Button, LoadingText) and their conditional logic intact
so styles and behavior remain identical.
In
`@workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts`:
- Around line 117-136: Add a typed errorCode to the response contract and switch
the availability check to it: add errorCode?: string to
BaseResponse/ScanResponse types and ensure scanProject() (rpc-manager.ts)
returns one of well-defined codes (e.g. "SCANNER_DISABLED", "SCANNER_NOT_READY",
"SCANNER_NOT_FOUND", "SCANNER_NOT_SUPPORTED"); then in usePreDeployScan.ts
replace the fragile substring checks on result.errorMsg with a switch/if on
result.errorCode (checking the four codes and setting scannerNotActive = true
when matched), falling back to the existing errorMsg logging/scanFailedError
behavior when errorCode is absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3aa12d5e-9cf1-42c3-94b8-fca4dd7bcf57
⛔ Files ignored due to path filters (3)
workspaces/ballerina/ballerina-extension/resources/icons/shield-dark.svgis excluded by!**/*.svgworkspaces/ballerina/ballerina-extension/resources/icons/shield-light.svgis excluded by!**/*.svgworkspaces/ballerina/ballerina-extension/resources/icons/shield.svgis excluded by!**/*.svg
📒 Files selected for processing (28)
workspaces/ballerina/ballerina-core/src/index.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/rpc-type.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/RPCLayer.tsworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tool-registry.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/activator.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/security-rules.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-handler.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.tsworkspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.tsworkspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.tsworkspaces/ballerina/ballerina-extension/src/views/scanner/webview.tsworkspaces/ballerina/ballerina-rpc-client/src/BallerinaRpcClient.tsworkspaces/ballerina/ballerina-rpc-client/src/rpc-clients/scanner/rpc-client.tsworkspaces/ballerina/ballerina-visualizer/src/Visualizer.tsxworkspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/AgentStreamView/StreamEntry.tsxworkspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/ToolCallGroupSegment.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/PackageOverview/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/ScanBanner.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.tsworkspaces/ballerina/ballerina-visualizer/src/views/BI/WorkspaceOverview/index.tsx
Co-authored-by: Copilot <copilot@github.com>
- Added loading and error handling UI for scanner tool pulling in ScannerOverview. - Introduced a new hook `usePreDeployScan` to manage pre-deployment scanning logic. - Integrated pre-deployment scanning into WorkspaceOverview, allowing for scanning before deployment. - Updated UI components to reflect scan results, including active issues and severity filters. - Improved state management for scan results and error messages.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts`:
- Around line 186-197: The code currently hardcodes success: true for
non-errorMsg responses; instead preserve the backend's success flag by using the
response.success value when present (e.g., set success to response.success if
it's a boolean, otherwise default to true). Update the return object in the
block that maps rawActiveIssues/rawExcludedIssues (referencing response,
mapIssue, mapExclusion) to set success from response rather than always true so
a backend response with success: false is propagated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06487f7c-5763-4564-8990-0660bbf29889
⛔ Files ignored due to path filters (3)
workspaces/ballerina/ballerina-extension/resources/icons/shield-dark.svgis excluded by!**/*.svgworkspaces/ballerina/ballerina-extension/resources/icons/shield-light.svgis excluded by!**/*.svgworkspaces/ballerina/ballerina-extension/resources/icons/shield.svgis excluded by!**/*.svg
📒 Files selected for processing (28)
workspaces/ballerina/ballerina-core/src/index.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.tsworkspaces/ballerina/ballerina-core/src/rpc-types/scanner/rpc-type.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/RPCLayer.tsworkspaces/ballerina/ballerina-extension/src/extension.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tool-registry.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/activator.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.tsworkspaces/ballerina/ballerina-extension/src/features/scanner/security-rules.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-handler.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.tsworkspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.tsworkspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.tsworkspaces/ballerina/ballerina-extension/src/views/scanner/webview.tsworkspaces/ballerina/ballerina-rpc-client/src/BallerinaRpcClient.tsworkspaces/ballerina/ballerina-rpc-client/src/rpc-clients/scanner/rpc-client.tsworkspaces/ballerina/ballerina-visualizer/src/Visualizer.tsxworkspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/AgentStreamView/StreamEntry.tsxworkspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/ToolCallGroupSegment.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/PackageOverview/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/ScanBanner.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.tsworkspaces/ballerina/ballerina-visualizer/src/views/BI/WorkspaceOverview/index.tsx
✅ Files skipped from review due to trivial changes (3)
- workspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.ts
- workspaces/ballerina/ballerina-extension/src/features/scanner/security-rules.ts
- workspaces/ballerina/ballerina-core/src/rpc-types/scanner/rpc-type.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- workspaces/ballerina/ballerina-extension/src/features/ai/agent/tool-registry.ts
- workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts
- workspaces/ballerina/ballerina-extension/src/RPCLayer.ts
- workspaces/ballerina/ballerina-rpc-client/src/BallerinaRpcClient.ts
- workspaces/ballerina/ballerina-visualizer/src/Visualizer.tsx
- workspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.ts
- workspaces/ballerina/ballerina-extension/src/extension.ts
- workspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/ToolCallGroupSegment.tsx
- workspaces/ballerina/ballerina-visualizer/src/views/AIPanel/components/AgentStreamView/StreamEntry.tsx
- workspaces/ballerina/ballerina-visualizer/src/views/BI/WorkspaceOverview/index.tsx
- workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-handler.ts
- workspaces/ballerina/ballerina-core/src/rpc-types/scanner/interfaces.ts
- workspaces/ballerina/ballerina-core/src/index.ts
- workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/scanner/rpc-client.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/PackageOverview/index.tsx
- workspaces/ballerina/ballerina-extension/package.json
- workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/ScanBanner.tsx
- workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx
- workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts
- workspaces/ballerina/ballerina-extension/src/features/scanner/activator.ts
- workspaces/ballerina/ballerina-extension/src/views/scanner/webview.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts
- workspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/security-diagnostics.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts (1)
159-179: ⚡ Quick winReplace
anyparameter type with a more specific type.The
rawparameter is typed asany, which bypasses TypeScript's type checking. If the backend can return multiple shapes, define a union type or use a partial type to preserve some type safety while allowing flexibility.♻️ Suggested typing improvement
+type RawExclusionData = Partial<{ + IssueContext?: any; + issueContext?: any; + issue?: any; + context?: any; + symbol?: string; + lineHash?: string; + isGlobalExclusion?: boolean; + filePath?: string; + ruleId?: string; +}>; + -export function mapExclusion(raw: any): ScannerExclusionContext { +export function mapExclusion(raw: RawExclusionData): ScannerExclusionContext {🤖 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/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts` around lines 159 - 179, The parameter raw in mapExclusion is currently typed as any; replace it with a precise type (e.g., a union or Partial shape) that captures the possible backend payloads so TypeScript can validate properties like IssueContext/issueContext/issue/context, symbol, lineHash, filePath, ruleId, and isGlobalExclusion; update the function signature from mapExclusion(raw: any) to mapExclusion(raw: YourNewType) and define/export the new type (or union/Partial) near ScannerExclusionContext (or in the same module), and adjust callers if necessary; keep mapIssue(...) usage intact but ensure the new type includes the fields mapIssue expects.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts`:
- Around line 49-64: The runBalToolCommand function uses child_process.exec with
a dynamic command string which is a command-injection risk; change it to use
child_process.execFile and accept a fixed executable plus an arguments array
(e.g. executable "bal" and args ["tool","pull","scan"] or
["tool","update","scan"]), update runBalToolCommand to call execFile(executable,
args, ...) and adjust its callers that currently pass a single string (the
places invoking runBalToolCommand with "bal tool pull scan" or "bal tool update
scan") to pass the executable and args array instead so parameters are passed
safely.
- Around line 181-189: The function mapRawScanResponse currently dereferences
response.activeIssues and response.excludedIssues without guarding for an
undefined response; update mapRawScanResponse to first check if response is
falsy and return a safe default ScanResponse (e.g., { activeIssues: [],
excludedIssues: [] }) or assign const { activeIssues = [], excludedIssues = [] }
= response || {}; then continue mapping with activeIssues.map(mapIssue) and
excludedIssues.map(mapExclusion) so mapIssue and mapExclusion are only called on
arrays and no runtime error occurs when response is undefined.
---
Nitpick comments:
In `@workspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.ts`:
- Around line 159-179: The parameter raw in mapExclusion is currently typed as
any; replace it with a precise type (e.g., a union or Partial shape) that
captures the possible backend payloads so TypeScript can validate properties
like IssueContext/issueContext/issue/context, symbol, lineHash, filePath,
ruleId, and isGlobalExclusion; update the function signature from
mapExclusion(raw: any) to mapExclusion(raw: YourNewType) and define/export the
new type (or union/Partial) near ScannerExclusionContext (or in the same
module), and adjust callers if necessary; keep mapIssue(...) usage intact but
ensure the new type includes the fields mapIssue expects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 535a3270-95ed-4045-8a06-cbebce004e8c
📒 Files selected for processing (6)
workspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/features/scanner/scan-utils.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.tsworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsxworkspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- workspaces/ballerina/ballerina-core/src/rpc-types/scanner/index.ts
- workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/index.tsx
- workspaces/ballerina/ballerina-visualizer/src/views/BI/ScannerOverview/usePreDeployScan.ts
- workspaces/ballerina/ballerina-extension/src/rpc-managers/scanner/rpc-manager.ts
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Currently, relying on the CLI-based static code analysis tool introduces technical friction, especially for low-code integration developers. Vulnerabilities are often discovered too late in the lifecycle. To ensure a secure-by-design experience, we need to surface real-time security feedback natively within the IDE. This specific PR implements the frontend VS Code Extension UI required to display vulnerabilities, manage exclusions and seamlessly interact with the WSO2 Integrator Copilot.
Goals
This PR introduces the client-side UI and integration logic for the Ballerina Security Scanner:
Scan.tomlfile.ballerina.scanner.enable) to act as a feature flag, allowing users to opt in or completely disable the scanner as needed.Approach
Scan.toml.UI Component Development
npm run storybookfrom the root directory to view current components.Release note
Introduced the Ballerina Security Scanner Panel to the VS Code IDE. Developers can now view real-time security vulnerabilities, manage project exclusions via a visual interface, and automatically verify AI-generated code through WSO2 Copilot.
Documentation
https://docs.google.com/document/d/1-o9uuk4CcdkC0Q1EW4hosh8sMYARydHlHOSMB7SWxo4/edit?usp=sharing
Security checks
Related PRs
Test environment
Summary by CodeRabbit
New Features
UI
AI Integration
Configuration