Conversation
|
Joseph Fonseka hevayo@gmail.com seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical 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 (4)
workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentFormView/sections/ComponentFormGenDetailsSection.tsx (1)
216-225: Lowercase "credential" for consistency.The error message is clear and actionable, but "Credential" should be lowercase per standard English conventions.
Apply this diff:
- <span>Selected Credential does not have sufficient permissions to access the repository.</span> + <span>Selected credential does not have sufficient permissions to access the repository.</span>workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentDetailsView/ComponentDetailsView.tsx (1)
135-162: Memory leak: async callback lacks cleanup mechanism.The
onSuccesscallback is now async and includes a 10-second delay before refetching queries. If the component unmounts during this delay, the promise will still resolve and attempt to refetch, potentially causing memory leaks or operating on stale data.Additionally, the hard-coded 10-second delay is a magic number that may not be reliable across different deployment scenarios.
Apply this pattern to add cleanup:
+const abortControllerRef = React.useRef<AbortController | null>(null); + +useEffect(() => { + return () => { + // Cleanup on unmount + if (abortControllerRef.current) { + abortControllerRef.current.abort(); + } + }; +}, []); const buildListQueryData = useGetBuildList(deploymentTrack, component, project, organization, { onSuccess: async (builds) => { setHasOngoingBuilds(builds.some((item) => item.status?.conclusion === "")); if (buildDetailsPanel?.open && buildDetailsPanel?.build) { const matchingItem = builds.find((item) => item.status?.runId === buildDetailsPanel?.build?.status?.runId); if (matchingItem) { setBuildDetailsPanel((state) => ({ ...state, build: matchingItem })); } buildLogsQueryData.refetch(); } const hasPrevSucceedBuilds = prevBuildList.filter((item) => item.status.conclusion === "success").length > 0; if (!hasPrevSucceedBuilds && builds.length > 0 && builds[0].status?.conclusion === "success" && envs.length > 0) { + abortControllerRef.current = new AbortController(); + const signal = abortControllerRef.current.signal; + // have a new succeeded build, which should be auto deployed - await new Promise((resolve) => setTimeout(resolve, 10000)); + await new Promise((resolve, reject) => { + const timeoutId = setTimeout(resolve, 10000); + signal.addEventListener('abort', () => { + clearTimeout(timeoutId); + reject(new Error('Aborted')); + }); + }).catch(() => { + // Aborted, do nothing + return; + }); + + if (signal.aborted) return; + if (getTypeForDisplayType(component.spec?.type) === ChoreoComponentType.ApiProxy) { queryClient.refetchQueries({ queryKey: queryKeys.getProxyDeploymentInfo( component, organization, envs[0], component?.apiVersions?.find((item) => item.latest), ), }); } else { queryClient.refetchQueries({ queryKey: queryKeys.getDeploymentStatus(deploymentTrack, component, organization, envs[0]) }); } } setPrevBuildList(builds); },workspaces/wso2-platform/wso2-platform-extension/src/git/git.ts (1)
415-421: Critical bug inclonefunction: returned path doesn't match actual clone location whenskipCreateSubPathis trueThe current implementation always generates a subfolder name from the repository URL and runs collision detection, but when
skipCreateSubPathistrue, Git clones directly intooptions.parentPath(via"."). However, the function returnsfolderPath(the computed URL-derived subfolder path), creating a mismatch: the caller receives a path that was never created.Additionally, subfolder name collision checks run unnecessarily when
skipCreateSubPathistrue.Fix by making
folderPathreflect the actual working tree location and only generating a subfolder name whenskipCreateSubPathisfalse:- async clone(url: string, options: ICloneOptions, cancellationToken?: CancellationToken): Promise<string> { - const baseFolderName = - decodeURI(url) - .replace(/[\/]+$/, "") - .replace(/^.*[\/\\]/, "") - .replace(/\.git$/, "") || "repository"; - let folderName = baseFolderName; - let folderPath = path.join(options.parentPath, folderName); - let count = 1; - - while (count < 20 && (await new Promise((c) => exists(folderPath, c)))) { - folderName = `${baseFolderName}-${count++}`; - folderPath = path.join(options.parentPath, folderName); - } + async clone(url: string, options: ICloneOptions, cancellationToken?: CancellationToken): Promise<string> { + let folderPath = options.parentPath; + + if (!options.skipCreateSubPath) { + const baseFolderName = + decodeURI(url) + .replace(/[\/]+$/, "") + .replace(/^.*[\/\\]/, "") + .replace(/\.git$/, "") || "repository"; + let folderName = baseFolderName; + folderPath = path.join(options.parentPath, folderName); + let count = 1; + + while (count < 20 && (await new Promise((c) => exists(folderPath, c)))) { + folderName = `${baseFolderName}-${count++}`; + folderPath = path.join(options.parentPath, folderName); + } + } await mkdirp(options.parentPath); @@ -526,8 +540,7 @@ try { const command = ["clone", url.includes(" ") ? encodeURI(url) : url]; - if(options.skipCreateSubPath){ - command.push(".") - }else{ - command.push(folderPath) - } + command.push(options.skipCreateSubPath ? "." : folderPath); command.push("--progress");workspaces/wso2-platform/wso2-platform-extension/src/cmds/open-in-console-cmd.ts (1)
182-186: Bug: Incorrect component reference in URL construction.When
selectedOption?.type === "component", the code usesparams?.component.metadata.handlerbut this code path is reached whenparams?.componentwas not provided (we're in theelsebranch at line 103). The correct reference should beselectedOption?.item?.data.metadata.handler.Apply this diff to fix the incorrect reference:
if (selectedOption?.type === "project") { env.openExternal(Uri.parse(`${projectBaseUrl}/home`)); } else if (selectedOption?.type === "component") { - env.openExternal(Uri.parse(`${projectBaseUrl}/components/${params?.component.metadata.handler}/overview`)); + env.openExternal(Uri.parse(`${projectBaseUrl}/components/${selectedOption?.item?.data?.metadata?.handler}/overview`)); }
🟠 Major comments (18)
workspaces/wso2-platform/wso2-platform-webviews/src/components/FormElements/Dropdown/Dropdown.tsx-55-55 (1)
55-55: Form state not updated whenonChangeRootis provided.The current logic
onChangeRoot || onChangecompletely bypasses react-hook-form'sonChangewhen an external handler is provided. This means the form field value won't be tracked and validation won't trigger.Apply this diff to ensure both handlers are called:
- onChange={onChangeRoot || onChange} + onChange={(e) => { + onChange(e); + onChangeRoot?.(e); + }}workspaces/wso2-platform/wso2-platform-extension/src/utils.ts-433-439 (1)
433-439: JWT parsing may fail: base64url vs base64 encoding mismatch.JWTs use base64url encoding (with
-and_instead of+and/), butatobexpects standard base64. This can cause decoding failures for tokens containing those characters.Replace with proper base64url decoding:
export const parseJwt = (token: string): { iss: string } | null => { try { - return JSON.parse(atob(token.split(".")[1])); + const base64Url = token.split(".")[1]; + if (!base64Url) return null; + const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/'); + return JSON.parse(atob(base64)); } catch (e) { return null; } };Alternatively, consider using the
jwt-decodepackage already in dependencies for more robust handling.workspaces/ballerina/ballerina-core/src/utils/identifier-utils.ts-29-39 (1)
29-39: Missing return value for unmatched modules causes implicitundefinedreturn.The function declares return type
SCOPEbut returns nothing when no module matches, resulting inundefined. This is a type mismatch that could cause runtime issues for callers expecting a validSCOPEvalue.Either update the return type to reflect the possibility of
undefined, or add a default return:-export function findScopeByModule(moduleName: string): SCOPE { +export function findScopeByModule(moduleName: string): SCOPE | undefined { if (AI_AGENT_MODULE === moduleName) { return SCOPE.AI_AGENT; } else if (INTEGRATION_API_MODULES.includes(moduleName)) { return SCOPE.INTEGRATION_AS_API; } else if (EVENT_INTEGRATION_MODULES.includes(moduleName)) { return SCOPE.EVENT_INTEGRATION; } else if (FILE_INTEGRATION_MODULES.includes(moduleName)) { return SCOPE.FILE_INTEGRATION; } + return undefined; }workspaces/wso2-platform/wso2-platform-extension/scripts/download-choreo-cli.js-353-366 (1)
353-366: Missing assets result in incomplete ZIP without explicit failure.When an asset is not found in the release, the code logs a warning and resolves successfully. This means the combined ZIP could be created with missing platform binaries, which would cause runtime failures later when users on those platforms try to use the CLI.
Consider either:
- Failing the entire download if any expected asset is missing
- Or tracking which assets were successfully downloaded and validating before creating the ZIP
async function downloadAllAssets(releaseData, expectedAssetNames, tempDir) { + const missingAssets = []; const downloadPromises = expectedAssetNames.map(assetName => { const asset = releaseData.assets?.find(a => a.name === assetName); if (!asset) { - console.warn(`Warning: Choreo CLI Asset not found: ${assetName}`); - return Promise.resolve(); + missingAssets.push(assetName); + return Promise.resolve(null); } return downloadAsset(asset, tempDir); }); await Promise.all(downloadPromises); + + if (missingAssets.length > 0) { + throw new Error(`Missing required CLI assets: ${missingAssets.join(', ')}`); + } }workspaces/wso2-platform/wso2-platform-extension/src/choreo-rpc/cli-install.ts-81-89 (1)
81-89: Use a dedicated zip library instead of shell commands to avoid potential command injection issues.The paths
COMBINED_ZIP_PATHandCHOREO_TMP_DIRare interpolated directly into shell commands using template literals. While single quotes provide some protection, paths containing special characters (backticks, quotes, or spaces fromext.context.extensionPath) could cause command failure or unexpected behavior. Other workspaces in this repository (mi-extension, ballerina-extension) already useadm-zipandunzipperlibraries for zip extraction, which is more secure and cross-platform reliable.Consider adopting the same approach here by using
adm-zipfor consistency with the rest of the codebase and to eliminate shell command dependencies entirely..github/workflows/publish-vsix.yml-92-93 (1)
92-93: Potential bug: Repository path may be malformed in release creation step.The repository value for
wso2-platformis nowwso2/vscode-extensions, but line 119 prependswso2/to the repo output:https://api.github.com/repos/wso2/${{ steps.repo.outputs.repo }}/releases/$idThis would result in
wso2/wso2/vscode-extensions, which is invalid. Other extensions (e.g.,ballerina) use just the repo name without the org prefix (e.g.,ballerina-vscode).Consider one of these fixes:
Option 1: Remove the
wso2/prefix from the repo output:elif [ "${{ github.event.inputs.extension }}" == "wso2-platform" ]; then - echo "repo=wso2/vscode-extensions" >> $GITHUB_OUTPUT + echo "repo=vscode-extensions" >> $GITHUB_OUTPUTOption 2: Update line 119 to not prepend
wso2/:- https://api.github.com/repos/wso2/${{ steps.repo.outputs.repo }}/releases/$id` + https://api.github.com/repos/${{ steps.repo.outputs.repo }}/releases/$id`Option 1 is safer as it maintains consistency with other extensions and doesn't affect line 114.
workspaces/ballerina/ballerina-visualizer/src/views/AIPanel/SettingsPanel/index.tsx-74-81 (1)
74-81: Add error handling and remove console.log.Two issues:
- The
console.logon line 78 should be removed or replaced with proper logging for production code.- If
getLoginMethod()fails, the promise rejection is unhandled, which could cause runtime warnings and leaveshouldShowLogoutButtonin an incorrect state.Apply this diff:
rpcClient .getAiPanelRpcClient() .getLoginMethod() .then((loginMethod) => { - console.log("Login Method: ", loginMethod); setShouldShowLogoutButton(loginMethod !== LoginMethod.DEVANT_ENV); + }) + .catch((error) => { + console.error("Failed to get login method:", error); + // Default to showing logout button on error + setShouldShowLogoutButton(true); });workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-in-with-code-cmd.ts-45-50 (1)
45-50: Add error handling forgetCurrentRegion()to prevent incomplete login.If
getCurrentRegion()throws an error,loginSuccesswill never be called, potentially leaving the authentication state inconsistent. The outer try-catch block won't catch errors in the async.then()callback.Apply this diff to handle errors and ensure login completes:
- ext.clients.rpcClient.signInWithAuthCode(authCode).then(async (userInfo) => { - if (userInfo) { - const region = await ext.clients.rpcClient.getCurrentRegion(); - authStore.getState().loginSuccess(userInfo, region); - } - }); + try { + const userInfo = await ext.clients.rpcClient.signInWithAuthCode(authCode); + if (userInfo) { + const region = await ext.clients.rpcClient.getCurrentRegion(); + authStore.getState().loginSuccess(userInfo, region); + } + } catch (regionError: any) { + getLogger().warn(`Failed to fetch region: ${regionError.message}`); + // Fall back to login without region if region fetch fails + const userInfo = await ext.clients.rpcClient.signInWithAuthCode(authCode); + if (userInfo) { + authStore.getState().loginSuccess(userInfo, undefined); + } + }Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-core/src/utils.ts-288-303 (1)
288-303: Add input validation and handle null returns appropriately at call sites.The
buildGitURLfunction can returnnullfor unknown providers, but several callers (e.g.,ComponentFormView.tsxlines usingrepoUrl) do not explicitly check for null before using the result. WhileWebviewRPC.tsdemonstrates proper null handling with an explicit check (if (!_repoUrl || !_repoUrl.startsWith("https://"))), other usages assign the result directly without validation.Additionally, the function accepts
organdrepoNameas unvalidated strings. These should be trimmed and validated to prevent malformed URLs from being generated.Consider adding input validation to
buildGitURL:export const buildGitURL = (org: string, repoName: string, provider: string, withDotGitSuffix?: boolean, serverUrl?: string): string | null => { + if (!org?.trim() || !repoName?.trim()) { + return null; + } + switch (provider) { case GitProvider.GITHUB: return `https://github.com/${org}/${repoName}${withDotGitSuffix ? ".git" : ""}`;Ensure all call sites explicitly validate the returned value before use, similar to the pattern in
WebviewRPC.ts.Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-extension/src/devant-utils.ts-54-54 (1)
54-54: IncorrectUri.from()usage.
workspace.workspaceFolders[0].uriis already aUriinstance.Uri.from()expects aUriComponentsobject (withscheme,path, etc.), not aUri. This should use the uri directly or call.fsPathon it.- const componentPath = Uri.from(workspace.workspaceFolders[0].uri).fsPath; + const componentPath = workspace.workspaceFolders[0].uri.fsPath;workspaces/ballerina/ballerina-extension/src/features/ai/service/connection.ts-66-68 (1)
66-68: Possible null pointer dereference oncredentials.
getAccessToken()can returnundefined(as per its return typePromise<AuthCredentials | undefined>), butcredentials.loginMethodis accessed without a null check. This will throw a runtime error if credentials is undefined.Apply this diff to add a null check:
const credentials = await getAccessToken(); + if (!credentials) { + console.warn("No credentials available"); + return undefined; + } const loginMethod = credentials.loginMethod;workspaces/ballerina/ballerina-extension/src/features/ai/service/connection.ts-170-176 (1)
170-176: Add null check for credentials in ANTHROPIC_KEY branch.
getAccessToken()can returnundefined, butcredentials.secretsis accessed directly without a null check.Apply this diff:
} else if (loginMethod === LoginMethod.ANTHROPIC_KEY) { const credentials = await getAccessToken(); + if (!credentials) { + throw new Error('Anthropic API key credentials not found'); + } const secrets = credentials.secrets as AnthropicKeySecrets; cachedAnthropic = createAnthropic({ baseURL: "https://api.anthropic.com/v1", apiKey: secrets.apiKey, });workspaces/ballerina/ballerina-visualizer/src/views/BI/PackageOverview/index.tsx-521-521 (1)
521-521: Type mismatch:devantMetadatacan be undefined.
DevantDashboarddeclaresdevantMetadata: DevantMetadataas required, butuseQueryreturnsdata: DevantMetadata | undefined. At line 963,devantMetadatamay be undefined when passed toDevantDashboard.Either guard the render or make the prop optional:
-function DevantDashboard({ projectStructure, handleDeploy, goToDevant, devantMetadata }: { projectStructure: ProjectStructure, handleDeploy: () => void, goToDevant: () => void, devantMetadata: DevantMetadata }) { +function DevantDashboard({ projectStructure, handleDeploy, goToDevant, devantMetadata }: { projectStructure: ProjectStructure, handleDeploy: () => void, goToDevant: () => void, devantMetadata?: DevantMetadata }) {Or wrap the render with a guard:
{isInDevant && + devantMetadata && <DevantDashboard projectStructure={projectStructure} handleDeploy={handleDeploy} goToDevant={goToDevant} devantMetadata={devantMetadata} /> }Also applies to: 958-964
workspaces/ballerina/ballerina-extension/src/features/ai/utils.ts-35-38 (1)
35-38: Operator precedence issue causing potentially unintended behavior.The
||operator has lower precedence than the ternary?:, so these expressions parse as:config.get('rootUrl') || (isDevantDev ? devUrl : prodUrl)This means if
config.get()returns any truthy value, that value is used regardless of environment. If this is intentional, add parentheses for clarity. If you want the config to override only when explicitly set:-export const BACKEND_URL: string = config.get('rootUrl') || isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_ROOT_URL : process.env.BALLERINA_ROOT_URL; -export const AUTH_ORG: string = config.get('authOrg') || isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_ORG : process.env.BALLERINA_AUTH_ORG; -export const AUTH_CLIENT_ID: string = config.get('authClientID') || isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_CLIENT_ID : process.env.BALLERINA_AUTH_CLIENT_ID; -export const AUTH_REDIRECT_URL: string = config.get('authRedirectURL') || isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_REDIRECT_URL : process.env.BALLERINA_AUTH_REDIRECT_URL; +export const BACKEND_URL: string = config.get('rootUrl') || (isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_ROOT_URL : process.env.BALLERINA_ROOT_URL); +export const AUTH_ORG: string = config.get('authOrg') || (isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_ORG : process.env.BALLERINA_AUTH_ORG); +export const AUTH_CLIENT_ID: string = config.get('authClientID') || (isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_CLIENT_ID : process.env.BALLERINA_AUTH_CLIENT_ID); +export const AUTH_REDIRECT_URL: string = config.get('authRedirectURL') || (isDevantDev ? process.env.BALLERINA_DEV_COPLIOT_AUTH_REDIRECT_URL : process.env.BALLERINA_AUTH_REDIRECT_URL);workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentFormView/sections/ComponentFormRepoInitSection.tsx-278-287 (1)
278-287: Incorrect variable checked in credential dropdown onChange.At line 280, the code checks
credential === createNewCredTextbut should checkvalue === createNewCredText. Thecredentialvariable is the current form value, not the newly selected value.onChange={(e) => { const value = (e.target as HTMLSelectElement).value; - if (credential === createNewCredText) { + if (value === createNewCredText) { form.setValue("credential", ""); ChoreoWebViewAPI.getInstance().openExternalChoreo(`organizations/${organization.handle}/settings/credentials`); } else { form.setValue("credential", value); } }}workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentFormView/sections/ComponentFormRepoInitSection.tsx-134-145 (1)
134-145: Branch selection logic may incorrectly prefer "master" over "main".The if-else structure at lines 136-142 has a logic issue: if
mainexists it setsmain, but then ifmasterexists (line 139), it overwrites withmasterdue to the separateifstatement. Additionally, theelseat line 141 only applies whenmasterdoesn't exist.useEffect(() => { if (branches?.length > 0 && !branches.includes(form.getValues("branch"))) { if (branches.includes("main")) { form.setValue("branch", "main", { shouldValidate: true }); - } - if (branches.includes("master")) { + } else if (branches.includes("master")) { form.setValue("branch", "master", { shouldValidate: true }); } else { form.setValue("branch", branches[0], { shouldValidate: true }); } } }, [branches]);workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentFormView/sections/ComponentFormRepoInitSection.tsx-97-103 (1)
97-103: Inverted conditional logic for credential selection.The condition at line 98 appears inverted. It sets the first credential when
form.getValues("credential")is truthy OR when the current credential is not in the list. The intended behavior seems to be setting a default when NO credential is set OR when current is invalid.useEffect(() => { - if (gitCredentials.length > 0 && (form.getValues("credential") || !gitCredentials.some((item) => item.id === form.getValues("credential")))) { + if (gitCredentials.length > 0 && (!form.getValues("credential") || !gitCredentials.some((item) => item.id === form.getValues("credential")))) { form.setValue("credential", gitCredentials[0]?.id); } else if (gitCredentials.length === 0 && form.getValues("credential") !== "") { form.setValue("credential", ""); } }, [gitCredentials]);workspaces/wso2-platform/wso2-platform-extension/src/cmds/commit-and-push-to-git-cmd.ts-110-140 (1)
110-140: URL parsing may throw on malformed remote URLs.
new URL(repoUrl)at line 124 will throw aTypeErrorif the URL is malformed. Since remotes can have various URL formats (e.g., SSHgit@github.com:org/repo.git), this should be wrapped in a try-catch or validated first.if (!matchingRemote && remotes[0].fetchUrl) { const repoUrl = remotes[0].fetchUrl; const parsed = parseGitURL(repoUrl); if (parsed) { const [repoOrg, repoName, provider] = parsed; - const urlObj = new URL(repoUrl); + let urlObj: URL; + try { + urlObj = new URL(repoUrl); + } catch { + window.showErrorMessage("Invalid repository URL format"); + return; + } await enrichGitUsernamePassword(
🟡 Minor comments (14)
workspaces/wso2-platform/wso2-platform-webviews/src/components/FormElements/Dropdown/Dropdown.tsx-58-72 (1)
58-72: Remove unnecessary Fragment to fix React key warning.The Fragment wrapper
<>is unnecessary since only one element is conditionally rendered, and it causes React key warnings because the key should be on the immediate child of.map(), not nested inside a Fragment.Apply this diff to simplify the rendering:
- {items?.map((item, index) => ( - <> - {typeof item !== "string" && item.type === "separator" ? ( - <VSCodeOption disabled className="h-[1px] bg-vsc-foreground" key={`separator-${index}`} value={`separator-${index}`} /> - ) : ( - <VSCodeOption - key={typeof item === "string" ? item : item?.value} - value={typeof item === "string" ? item : item.value} - className="p-1" - > - {typeof item === "string" ? item : item?.label || item.value} - </VSCodeOption> - )} - </> - ))} + {items?.map((item, index) => + typeof item !== "string" && item.type === "separator" ? ( + <VSCodeOption disabled className="h-[1px] bg-vsc-foreground" key={`separator-${index}`} value={`separator-${index}`} /> + ) : ( + <VSCodeOption + key={typeof item === "string" ? item : item?.value} + value={typeof item === "string" ? item : item.value} + className="p-1" + > + {typeof item === "string" ? item : item?.label || item.value} + </VSCodeOption> + ) + )}workspaces/ballerina/ballerina-extension/.env.example-15-15 (1)
15-15: Add blank line at end of file.The file should end with a newline for consistency with standard conventions.
Add a blank line after line 15:
BALLERINA_DEV_COPILOT_ASK_API_KEY= +Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-webviews/src/components/BreadCrumb/BreadCrumb.tsx-36-46 (1)
36-46: Fix duplicate React keys for breadcrumb items and separatorsYou're assigning
key={item.label}to both the item element (line 35 or 41) and the separator span (line 45), which creates duplicate keys among siblings in thenodesarray. This will trigger React reconciliation issues and "duplicate key" warnings, and assumeslabelis always unique across items.Use distinct, stable keys per element:
- <VSCodeLink key={item.label} className="text-sm text-vsc-foreground opacity-70" onClick={item.onClick}> + <VSCodeLink + key={`breadcrumb-item-${index}-${item.label}`} + className="text-sm text-vsc-foreground opacity-70" + onClick={item.onClick} + > {item.label} </VSCodeLink>, ... - nodes.push(<span key={item.label} className={classNames("text-sm", index + 1 < items.length && "opacity-70")}>{item.label}</span>); + nodes.push( + <span + key={`breadcrumb-item-${index}-${item.label}`} + className={classNames("text-sm", index + 1 < items.length && "opacity-70")} + > + {item.label} + </span> + ); ... - nodes.push(<span key={item.label} className="text-sm opacity-70">/</span>); + nodes.push( + <span + key={`breadcrumb-separator-${index}`} + className="text-sm opacity-70" + > + / + </span> + );workspaces/ballerina/ballerina-extension/src/features/devant/activator.ts-117-122 (1)
117-122: Handle the case when user cancels the quick pick dialog.When the user cancels the quick pick,
selectedScopewill beundefined, which then gets cast toSCOPE. This can lead tointegrationTypebeingundefinedand causing issues downstream when passed toICreateComponentCmdParams.} else { const selectedScope = await window.showQuickPick([...scopeSet], { placeHolder: "Multiple types of artifacts detected. Please select the artifact type to be deployed", }); - integrationType = selectedScope as SCOPE; + if (!selectedScope) { + return; // User cancelled the selection + } + integrationType = selectedScope as SCOPE; }workspaces/wso2-platform/wso2-platform-extension/src/devant-utils.ts-42-48 (1)
42-48: UseclearIntervalinstead ofclearTimeoutin dispose handler.The interval is created with
setIntervalon line 31, but the dispose handler usesclearTimeouton line 45. While this may work in most JavaScript environments, using the matching function is correct and clearer.ext.context.subscriptions.push({ dispose: () => { if (intervalId) { - clearTimeout(intervalId); + clearInterval(intervalId); } }, });workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts-163-170 (1)
163-170: Handle undefinedregionto prevent invalid cache keys.If
authStore.getState().state.regionisundefinedor empty, the resulting key would be"undefined-orgHandle"or"-orgHandle", leading to cache key collisions or data corruption across different regions.Consider adding a fallback or validation:
const getRootKey = (orgHandle: string) => { - const region = authStore.getState().state.region; + const region = authStore.getState().state.region || 'default'; const env = ext.choreoEnv; let orgRegionHandle = `${region}-${orgHandle}`; if (env !== "prod") { orgRegionHandle = `${env}-${orgRegionHandle}`; } return orgRegionHandle; };workspaces/ballerina/ballerina-visualizer/src/views/BI/PackageOverview/index.tsx-538-538 (1)
538-538: Remove debug console.log statement.This debug log should be removed before merging to production.
- console.log(">>> devantMetadata", devantMetadata);workspaces/wso2-platform/wso2-platform-extension/src/extension.ts-111-112 (1)
111-112: Stale or incomplete comment.The comment says "skip showing this if cloud sts env is available" but there's no conditional logic implementing this skip. Either implement the skip or remove the misleading comment.
workspace.onDidChangeConfiguration(async ({ affectsConfiguration }: ConfigurationChangeEvent) => { if (affectsConfiguration("WSO2.WSO2-Platform.Advanced.ChoreoEnvironment") || affectsConfiguration("WSO2.WSO2-Platform.Advanced.RpcPath")) { - // skip showing this if cloud sts env is available const selection = await window.showInformationMessage(workspaces/ballerina/ballerina-extension/src/utils/ai/auth.ts-348-368 (1)
348-368: Misleading comment about credential storage.The comment on line 360-361 states "this is in-memory only for DEVANT_ENV", but
exchangeStsToken(called on line 358) actually persists credentials viastoreAuthCredentials. The comment should be removed or corrected to avoid confusion.// Exchange for new Bearer token const newSecrets = await exchangeStsToken(newStsToken); - // Update stored credentials (this is in-memory only for DEVANT_ENV) - // Note: checkDevantEnvironment already handles the storage, so we just return the token - return newSecrets.accessToken;workspaces/ballerina/ballerina-extension/src/utils/ai/auth.ts-316-341 (1)
316-341: Add request timeout for network resilience.The
axios.postcall to exchange STS tokens lacks a timeout configuration. This could cause the extension to hang indefinitely if the exchange endpoint is unresponsive.const response = await axios.post(getDevantExchangeUrl(), { choreo_sts_token: choreoStsToken }, { headers: { 'Content-Type': 'application/json' - } + }, + timeout: 30000 // 30 second timeout });workspaces/wso2-platform/wso2-platform-extension/src/cmds/commit-and-push-to-git-cmd.ts-52-62 (1)
52-62: Non-null assertion on potentially undefined property.The
displayNameproperty at line 55 uses a non-null assertion (!) which could cause runtime errors ifmetadata.displayNameis undefined.- label: item?.component?.metadata?.displayName!, + label: item?.component?.metadata?.displayName ?? item?.component?.metadata?.name ?? "Unknown",workspaces/wso2-platform/wso2-platform-webviews/src/views/ComponentFormView/ComponentFormView.tsx-183-186 (1)
183-186: Type-unsafe cache access and potential stale data.Line 185 accesses the query cache directly with
queryClient.getQueryData()which returnsunknownand may returnundefinedor stale data. The type assertion tostring[]is implicit and unsafe.-const branchesCache: string[] = queryClient.getQueryData(queryKeys.getGitBranches(repoUrl, organization, "", true)); +const branchesCache: string[] | undefined = queryClient.getQueryData(queryKeys.getGitBranches(repoUrl, organization, "", true));Also ensure
branchesCacheis validated before use at line 194 and 199 since it could be undefined.Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-extension/src/cmds/commit-and-push-to-git-cmd.ts-203-210 (1)
203-210: Conflicting optional chaining with non-null assertion.At line 205,
org?.id?.toString()!uses optional chaining but then applies a non-null assertion, which is contradictory and could mask issues iforgoridis undefined.- orgId: org?.id?.toString()!, + orgId: org.id.toString(),Since
orgis required by the function signature (org: Organization), the optional chaining is unnecessary here.workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts-329-367 (1)
329-367: Missing error handling forgetHEAD()returning undefined.If
repo.getHEAD()returns an object wherehead.nameisundefined, the code at line 339 will callrepo.getCommit(undefined), which may cause unexpected behavior. Add a check or fallback.const head = await repo.getHEAD(); -if (head.name) { +if (head?.name) { const commit = await repo.getCommit(head.name);
| - name: Create a release in wso2/vscode-extensions repo | ||
| if: ${{ github.event.inputs.wso2-platform == 'true' }} | ||
| uses: ./.github/actions/release | ||
| with: | ||
| repo: wso2/platform-vscode | ||
| repo: wso2/vscode-extensions | ||
| name: wso2-platform | ||
| token: ${{ secrets.CHOREO_BOT_TOKEN }} | ||
| chatAPI: ${{ steps.chat.outputs.chatAPI }} |
There was a problem hiding this comment.
Add missing required input threadId.
The release action requires a threadId input, but it's not provided in this step. This will cause the workflow to fail during execution.
Based on static analysis hints, apply this diff:
- name: Create a release in wso2/vscode-extensions repo
if: ${{ github.event.inputs.wso2-platform == 'true' }}
uses: ./.github/actions/release
with:
repo: wso2/vscode-extensions
name: wso2-platform
token: ${{ secrets.CHOREO_BOT_TOKEN }}
chatAPI: ${{ steps.chat.outputs.chatAPI }}
+ threadId: ${{ github.run_id }}📝 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.
| - name: Create a release in wso2/vscode-extensions repo | |
| if: ${{ github.event.inputs.wso2-platform == 'true' }} | |
| uses: ./.github/actions/release | |
| with: | |
| repo: wso2/platform-vscode | |
| repo: wso2/vscode-extensions | |
| name: wso2-platform | |
| token: ${{ secrets.CHOREO_BOT_TOKEN }} | |
| chatAPI: ${{ steps.chat.outputs.chatAPI }} | |
| - name: Create a release in wso2/vscode-extensions repo | |
| if: ${{ github.event.inputs.wso2-platform == 'true' }} | |
| uses: ./.github/actions/release | |
| with: | |
| repo: wso2/vscode-extensions | |
| name: wso2-platform | |
| token: ${{ secrets.CHOREO_BOT_TOKEN }} | |
| chatAPI: ${{ steps.chat.outputs.chatAPI }} | |
| threadId: ${{ github.run_id }} |
🧰 Tools
🪛 actionlint (1.7.9)
148-148: missing input "threadId" which is required by action "Release vsix to github and notify" defined at "./.github/actions/release". all required inputs are "chatAPI", "name", "repo", "threadId", "token"
(action)
🤖 Prompt for AI Agents
.github/workflows/release-vsix.yml around lines 146 to 153: the release action
invocation is missing the required threadId input which will cause the workflow
to fail; add the threadId input using the chat step output (e.g. threadId: ${{
steps.chat.outputs.threadId }}) alongside the existing chatAPI input so the
action receives the required value.
| BALLERINA_DEFAULT_COPLIOT_CODE_API_KEY= | ||
| BALLERINA_DEFAULT_COPLIOT_ASK_API_KEY= |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n "workspaces/ballerina/ballerina-extension/.env.example"Repository: wso2/vscode-extensions
Length of output: 708
🏁 Script executed:
rg -n 'COPLIOT|COPILOT' workspaces/ballerina/ --type-add 'env:*.example' -t env -t js -t ts -t py -t java -t goRepository: wso2/vscode-extensions
Length of output: 5643
🏁 Script executed:
git ls-files "workspaces/ballerina/" | head -30Repository: wso2/vscode-extensions
Length of output: 2275
🏁 Script executed:
rg -n 'BALLERINA_DEFAULT_COPLIOT|BALLERINA_DEFAULT_COPILOT' workspaces/ballerina/ -t js -t ts -t py -t javaRepository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
sed -n '33,40p' workspaces/ballerina/ballerina-extension/src/features/ai/utils.tsRepository: wso2/vscode-extensions
Length of output: 935
🏁 Script executed:
rg -n 'BALLERINA_DEV_COPLIOT|BALLERINA_DEFAULT_COPLIOT' workspaces/ballerina/ballerina-extension/src --type ts --type jsRepository: wso2/vscode-extensions
Length of output: 994
🏁 Script executed:
rg -n 'BALLERINA_ROOT_URL|BALLERINA_AUTH_ORG' workspaces/ballerina/ballerina-extension/src --type ts --type js -lRepository: wso2/vscode-extensions
Length of output: 131
Fix typo in Copilot variable names: "COPLIOT" → "COPILOT".
The misspelled environment variable names appear in both .env.example and the code that consumes them. Update .env.example and src/features/ai/utils.ts to use the correct spelling.
In .env.example (lines 6–7, 10–15):
BALLERINA_DEFAULT_COPLIOT_*→BALLERINA_DEFAULT_COPILOT_*BALLERINA_DEV_COPLIOT_*→BALLERINA_DEV_COPILOT_*
In src/features/ai/utils.ts (lines 35–38), update the corresponding environment variable references to match the corrected names.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 6-6: [UnorderedKey] The BALLERINA_DEFAULT_COPLIOT_CODE_API_KEY key should go before the BALLERINA_ROOT_URL key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The BALLERINA_DEFAULT_COPLIOT_ASK_API_KEY key should go before the BALLERINA_DEFAULT_COPLIOT_CODE_API_KEY key
(UnorderedKey)
| actions: assign({ | ||
| loginMethod: (_ctx, event) => event.data.loginMethod, | ||
| userToken: (_ctx, event) => ({ token: event.data.token }), | ||
| userToken: (_ctx, event) => ({ credentials: event.data }), | ||
| errorMessage: (_ctx) => undefined, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd workspaces/ballerina && rg -n 'userToken\.credentials' --type=ts -A 2 -B 2Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
cd workspaces/ballerina && rg -n 'userToken' --type=ts -A 1 -B 1 | grep -E '(credentials|token)' | head -40Repository: wso2/vscode-extensions
Length of output: 358
🏁 Script executed:
cd workspaces/ballerina/ballerina-extension/src && fd . -name "*.ts" -type f | xargs grep -l 'userToken\|checkToken\|getTokenAfterAuth' | head -20Repository: wso2/vscode-extensions
Length of output: 296
🏁 Script executed:
cd workspaces/ballerina && rg -n 'userToken' --type=ts -B 2 -A 2 | head -80Repository: wso2/vscode-extensions
Length of output: 6275
🏁 Script executed:
cd workspaces/ballerina/ballerina-extension/src && grep -n "checkToken\|getTokenAfterAuth" aiMachine.ts --type=tsRepository: wso2/vscode-extensions
Length of output: 186
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts | sed -n '60,90p'Repository: wso2/vscode-extensions
Length of output: 1479
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts | sed -n '250,275p'Repository: wso2/vscode-extensions
Length of output: 1246
🏁 Script executed:
rg -n "type AIUserToken\|interface AIUserToken" workspaces/ballerina --type=ts -A 5Repository: wso2/vscode-extensions
Length of output: 48
🏁 Script executed:
cd workspaces/ballerina/ballerina-extension/src && grep -n "checkToken\|getTokenAfterAuth" aiMachine.ts | head -20Repository: wso2/vscode-extensions
Length of output: 111
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts | sed -n '1,50p'Repository: wso2/vscode-extensions
Length of output: 2399
🏁 Script executed:
rg -n "AIUserToken" workspaces/ballerina/ballerina-core/src/state-machine-types.ts -A 3 -B 1Repository: wso2/vscode-extensions
Length of output: 306
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-core/src/state-machine-types.ts | sed -n '400,430p'Repository: wso2/vscode-extensions
Length of output: 937
🏁 Script executed:
cd workspaces/ballerina/ballerina-extension/src/views/ai-panel && grep -n "checkToken\|getTokenAfterAuth" utils.ts aiMachine.ts 2>/dev/null | head -30Repository: wso2/vscode-extensions
Length of output: 725
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts | sed -n '351,380p'Repository: wso2/vscode-extensions
Length of output: 1193
🏁 Script executed:
cat -n workspaces/ballerina/ballerina-extension/src/views/ai-panel/utils.ts | sed -n '30,80p'Repository: wso2/vscode-extensions
Length of output: 2191
🏁 Script executed:
rg -n "AuthCredentials" workspaces/ballerina/ballerina-core/src/state-machine-types.ts -B 2 -A 5 | head -40Repository: wso2/vscode-extensions
Length of output: 457
Fix userToken shape at line 75 to match getTokenAfterAuth response handling.
At line 75, userToken is assigned { credentials: event.data }, wrapping the entire AuthCredentials object. However, at line 262, it correctly extracts { credentials: event.data.credentials } where event.data.credentials is the secrets property.
Since AuthCredentials is a discriminated union containing both loginMethod and secrets, line 75 should destructure similarly:
userToken: (_ctx, event) => ({ credentials: event.data.secrets, loginMethod: event.data.loginMethod })
This ensures userToken.credentials contains only the secrets, not the full AuthCredentials object, matching the AIUserToken interface definition.
🤖 Prompt for AI Agents
In workspaces/ballerina/ballerina-extension/src/views/ai-panel/aiMachine.ts
around lines 73 to 77, the assigned userToken wraps the entire AuthCredentials
object (userToken: { credentials: event.data }) which makes credentials contain
the full AuthCredentials instead of the secrets; change the assignment to
destructure event.data and set userToken to include credentials:
event.data.secrets and loginMethod: event.data.loginMethod so
userToken.credentials holds only the secrets and matches the AIUserToken shape
used elsewhere.
| const credentials = await checkToken(); | ||
| if (credentials.loginMethod === LoginMethod.BI_INTEL) { | ||
| const logoutURL = getLogoutUrl(); | ||
| vscode.env.openExternal(vscode.Uri.parse(logoutURL)); | ||
| } |
There was a problem hiding this comment.
Potential null pointer error when accessing credentials.loginMethod.
checkToken() can return undefined, but line 70 accesses credentials.loginMethod without a null check, which will throw a runtime error.
if (isUserLogout) {
const credentials = await checkToken();
- if (credentials.loginMethod === LoginMethod.BI_INTEL) {
+ if (credentials?.loginMethod === LoginMethod.BI_INTEL || credentials?.loginMethod === LoginMethod.DEVANT_ENV) {
const logoutURL = getLogoutUrl();
vscode.env.openExternal(vscode.Uri.parse(logoutURL));
}
}📝 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 credentials = await checkToken(); | |
| if (credentials.loginMethod === LoginMethod.BI_INTEL) { | |
| const logoutURL = getLogoutUrl(); | |
| vscode.env.openExternal(vscode.Uri.parse(logoutURL)); | |
| } | |
| const credentials = await checkToken(); | |
| if (credentials?.loginMethod === LoginMethod.BI_INTEL || credentials?.loginMethod === LoginMethod.DEVANT_ENV) { | |
| const logoutURL = getLogoutUrl(); | |
| vscode.env.openExternal(vscode.Uri.parse(logoutURL)); | |
| } |
🤖 Prompt for AI Agents
In workspaces/ballerina/ballerina-extension/src/views/ai-panel/utils.ts around
lines 69 to 73, checkToken() can return undefined but the code accesses
credentials.loginMethod directly; add a null/undefined guard before accessing
properties (for example: if (!credentials) return or handle the unauthenticated
case), or use optional chaining (credentials?.loginMethod) and only call
getLogoutUrl()/openExternal when credentials exists and loginMethod ===
LoginMethod.BI_INTEL, ensuring you handle the unauthenticated branch explicitly.
| // Check if devant.editor extension is active | ||
| const isDevantEditor = vscode.commands.executeCommand('getContext', 'devant.editor') !== undefined; | ||
|
|
There was a problem hiding this comment.
Incorrect async handling: executeCommand returns a Promise, not the context value.
vscode.commands.executeCommand returns a Thenable<T>, so comparing it directly to undefined will always evaluate to true (the Promise object is never undefined). The context value is never actually retrieved.
Either make getWebviewContent async and await the result, or use the synchronous context API:
- // Check if devant.editor extension is active
- const isDevantEditor = vscode.commands.executeCommand('getContext', 'devant.editor') !== undefined;
+ // Check if devant.editor extension is active using getContext API
+ const isDevantEditor = vscode.extensions.getExtension('wso2.devant-editor') !== undefined;Or if you need to check a VS Code context key set via setContext:
// Store the context value when it's set, or check extension activation differently
const devantExtension = vscode.extensions.getExtension('wso2.devant-editor');
const isDevantEditor = devantExtension?.isActive ?? false;🤖 Prompt for AI Agents
In workspaces/ballerina/ballerina-extension/src/views/visualizer/webview.ts
around lines 149-151, the code calls
vscode.commands.executeCommand('getContext', 'devant.editor') and compares the
returned Thenable to undefined — executeCommand returns a Promise, so the
context value is never retrieved; fix by handling the async result (make
getWebviewContent async and await the executeCommand call, then use the awaited
boolean value) or alternatively check the extension activation synchronously via
vscode.extensions.getExtension('wso2.devant-editor')?.isActive and assign that
boolean to isDevantEditor.
| - **Deploy Builds**: Deploy your component builds to any chosen [environments](https://wso2.com/choreo/docs/choreo-concepts/environments/). | ||
| - **Test Services**: Verify the functionality of publicly exposed services. | ||
| - **Monitor Components**: Access runtime logs to monitor your deployed components. | ||
| - **Connect Locally to Dependencies**: Link your app to dependent connections while developing. See [guide](https://wso2.com/choreo/docs/develop-components/connect-to-remote-dependencies-while-developing/). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
curl -s -o /dev/null -w "%{http_code}" "https://wso2.com/choreo/docs/develop-components/connect-to-remote-dependencies-while-developing/"Repository: wso2/vscode-extensions
Length of output: 69
Documentation link returns HTTP 403 and is not publicly accessible.
The link at line 20 (https://wso2.com/choreo/docs/develop-components/connect-to-remote-dependencies-while-developing/) returns an HTTP 403 Forbidden status code. Users following this link will encounter an access error. Verify the URL is correct and the documentation page is publicly available before merging.
🤖 Prompt for AI Agents
In workspaces/choreo/choreo-extension/README.md around line 20, the
documentation link returns HTTP 403 and is not publicly accessible; verify the
URL and either replace it with the correct public documentation URL (or an
updated path on wso2.com), or remove the link and substitute a short in-repo
note explaining how to link dependencies locally (or link to an alternative
public resource or archived copy). Update the README to use the working URL or
the inline guidance, and ensure the link is tested before committing.
| export interface ICommitAndPuhCmdParams extends ICmdParamsBase { | ||
| componentPath: string; | ||
| } |
There was a problem hiding this comment.
Fix typo in interface name.
The interface name ICommitAndPuhCmdParams has a typo – it should be ICommitAndPushCmdParams (missing 's' in "Push").
Apply this diff to fix the typo:
-export interface ICommitAndPuhCmdParams extends ICmdParamsBase {
+export interface ICommitAndPushCmdParams extends ICmdParamsBase {
componentPath: string;
}📝 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.
| export interface ICommitAndPuhCmdParams extends ICmdParamsBase { | |
| componentPath: string; | |
| } | |
| export interface ICommitAndPushCmdParams extends ICmdParamsBase { | |
| componentPath: string; | |
| } |
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-core/src/types/cmd-params.ts around
lines 18-20, the exported interface name ICommitAndPuhCmdParams contains a typo
and should be renamed to ICommitAndPushCmdParams; rename the interface
declaration to ICommitAndPushCmdParams and update any references/imports across
the codebase to use the corrected name to avoid type errors.
| "@vscode/vsce": "^3.7.0", | ||
| "@wso2/playwright-vscode-tester": "workspace:*", | ||
| "axios": "^1.12.0", | ||
| "axios": "^1.9.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd workspaces/wso2-platform/wso2-platform-extension && sed -n '230,236p' package.jsonRepository: wso2/vscode-extensions
Length of output: 277
🌐 Web query:
axios 1.9.0 1.12.0 security vulnerabilities changelog
💡 Result:
Summary — security-related items for axios 1.9.0 and 1.12.0
-
axios 1.9.0: affected by a DoS vulnerability where a crafted data: URI can cause unbounded memory allocation (CVE‑2025‑58754 / “lack of data size check”); versions >=1.0.0 and <1.12.0 are vulnerable. [1][2][3]
-
axios 1.12.0: includes the patch that fixes the data: URI DoS (CVE‑2025‑58754). Release notes show 1.12.x as the patched series; security advisories point to v1.12.0 as the patched version. [1][3][4]
References
- GitHub Advisory (GHSA / CVE advisory) — “Axios is vulnerable to DoS attack through lack of data size check” (patched in 1.12.0). [1]
- NVD entry for CVE‑2025‑58754. [2]
- Snyk vulnerability notes for axios (shows <1.12.0 vulnerable; 1.12.0+ fixed). [3]
- axios GitHub Releases (v1.12.x release entries). [4]
Revert axios to ^1.12.0 — version 1.9.0 is vulnerable to CVE-2025-58754.
The downgrade to ^1.9.0 introduces a known DoS vulnerability (CVE-2025-58754) affecting axios versions <1.12.0, where a crafted data: URI can cause unbounded memory allocation. Version 1.12.0 includes the fix for this critical issue.
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/package.json around line 233
the axios dependency was set to "^1.9.0", which is vulnerable to CVE-2025-58754;
update the axios entry back to "^1.12.0" in package.json, then run your package
manager (npm/yarn/pnpm) to update node_modules and regenerate the lockfile, run
the test suite and dependency audit (npm audit or equivalent) to confirm the
vulnerability is resolved, and commit the updated package.json and lockfile.
| commands.executeCommand( | ||
| "vscode.open", | ||
| ` ${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Leading space in URL template literal creates invalid URL.
Line 127 has a leading space before the template: ${extensionName... which will produce URLs like https://... with a leading space, causing the URL to be invalid.
if (res === "Sign Up") {
commands.executeCommand(
"vscode.open",
- ` ${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`,
+ `${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`,
);
}📝 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.
| commands.executeCommand( | |
| "vscode.open", | |
| ` ${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`, | |
| ); | |
| } | |
| commands.executeCommand( | |
| "vscode.open", | |
| `${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts around
lines 125 to 129, there is a leading space inside the template literal used for
the URL (` ` ${extensionName...`) which produces an invalid URL with a leading
space; remove the extra space so the template literal starts immediately with
the URL (e.g., `${extensionName === "Devant" ? ...}/signup`) to ensure
commands.executeCommand receives a well-formed URL string.
| export const useGetAuthorizedGitOrgs = (orgId: string, provider: string, credRef = "", options?: UseQueryOptions<GetAuthorizedGitOrgsResp>) => | ||
| useQuery<GetAuthorizedGitOrgsResp>( | ||
| queryKeys.getAuthorizedGitOrgs(orgId, provider, credRef), | ||
| () => ChoreoWebViewAPI.getInstance().getChoreoRpcClient().getAuthorizedGitOrgs({ orgId, credRef }), | ||
| options, | ||
| ); |
There was a problem hiding this comment.
Missing provider parameter in RPC call.
The provider parameter is included in the query key (line 164) but is not passed to the getAuthorizedGitOrgs RPC call (line 165). This will cause:
- Incorrect caching behavior - queries with different providers but same
orgId/credRefwill return cached responses from a different provider - Potentially incorrect API responses if the backend requires the provider
export const useGetAuthorizedGitOrgs = (orgId: string, provider: string, credRef = "", options?: UseQueryOptions<GetAuthorizedGitOrgsResp>) =>
useQuery<GetAuthorizedGitOrgsResp>(
queryKeys.getAuthorizedGitOrgs(orgId, provider, credRef),
- () => ChoreoWebViewAPI.getInstance().getChoreoRpcClient().getAuthorizedGitOrgs({ orgId, credRef }),
+ () => ChoreoWebViewAPI.getInstance().getChoreoRpcClient().getAuthorizedGitOrgs({ orgId, provider, credRef }),
options,
);📝 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.
| export const useGetAuthorizedGitOrgs = (orgId: string, provider: string, credRef = "", options?: UseQueryOptions<GetAuthorizedGitOrgsResp>) => | |
| useQuery<GetAuthorizedGitOrgsResp>( | |
| queryKeys.getAuthorizedGitOrgs(orgId, provider, credRef), | |
| () => ChoreoWebViewAPI.getInstance().getChoreoRpcClient().getAuthorizedGitOrgs({ orgId, credRef }), | |
| options, | |
| ); | |
| export const useGetAuthorizedGitOrgs = (orgId: string, provider: string, credRef = "", options?: UseQueryOptions<GetAuthorizedGitOrgsResp>) => | |
| useQuery<GetAuthorizedGitOrgsResp>( | |
| queryKeys.getAuthorizedGitOrgs(orgId, provider, credRef), | |
| () => ChoreoWebViewAPI.getInstance().getChoreoRpcClient().getAuthorizedGitOrgs({ orgId, provider, credRef }), | |
| options, | |
| ); |
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-webviews/src/hooks/use-queries.tsx
around lines 162 to 167, the query key includes the provider but the provider
argument is not passed to the getAuthorizedGitOrgs RPC call; update the RPC call
to pass the provider parameter (e.g., include provider in the
getAuthorizedGitOrgs({ orgId, provider, credRef }) call) so query caching and
backend behavior align with the query key.
Add new apis to wso2 platform extension
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (1)
559-567: New required fieldserverUrlon CredentialItem breaks RPC contract.Adding
serverUrlas a required field is a breaking change for the RPC endpointsgetCredentials()andgetCredentialDetails(). The backend must be updated to includeserverUrlin allCredentialItemresponses returned by these endpoints. Without coordination with the backend implementation, this will cause type mismatches for any client code consuming these RPC calls.
♻️ Duplicate comments (1)
workspaces/wso2-platform/wso2-platform-extension/package.json (1)
233-233: [DUPLICATE] Revert axios to ^1.12.0 — version 1.9.0 is vulnerable to CVE-2025-58754.This critical security issue was already flagged in a previous review. The downgrade to
^1.9.0reintroduces a known DoS vulnerability where a crafted data: URI can cause unbounded memory allocation. Version 1.12.0 includes the fix for this critical issue and must be restored.Apply this diff to fix the vulnerability:
- "axios": "^1.9.0", + "axios": "^1.12.0",After applying this change, regenerate your lockfile (pnpm install) and run
pnpm auditto confirm the vulnerability is resolved.
🧹 Nitpick comments (1)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (1)
24-43: Minor style inconsistency in method signature.Line 34 uses function type syntax (
getDevantConsoleUrl: () => Promise<string>) while all other methods use method signature syntax (getStsToken(): Promise<string>). Consider aligning for consistency.- getDevantConsoleUrl: () => Promise<string>; + getDevantConsoleUrl(): Promise<string>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts(4 hunks)workspaces/wso2-platform/wso2-platform-extension/package.json(8 hunks)workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (1)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (3)
IWso2PlatformExtensionAPI(24-43)ComponentKind(131-139)openClonedDirReq(45-52)
🔇 Additional comments (10)
workspaces/wso2-platform/wso2-platform-extension/package.json (6)
21-22: Verify language model agent integration.The new activation event
onLanguageModel:agentindicates language model/AI agent integration. Ensure that the extension properly implements the language model provider API and handles activation when a language model is available.Please confirm:
- The extension exports a language model provider (if required by the activation event)
- The related implementation exists in the extension source code
- The feature is documented and properly wired end-to-end
128-133: Verify git commit/push command implementation and security.The new
wso2.wso2-platform.push-to-gitcommand is properly declared. Ensure that:
- The command handler is implemented and properly handles git authentication, authorization, and error cases
- Sensitive data (credentials, tokens) is not logged or exposed during git operations
- The command validates repository state and user permissions before attempting to push
171-172: Verify configuration scope change for ChoreoEnvironment.The scope of
WSO2.WSO2-Platform.Advanced.ChoreoEnvironmentchanged fromwindowtomachine. This means the setting will now apply at the user/machine level rather than per-workspace.Please confirm:
- This scope change is intentional and aligns with how the environment should be configured (machine-wide vs. workspace-specific)
- There is no migration guidance needed for existing users with workspace-level settings
- No unintended side effects in multi-workspace or team collaboration scenarios
191-196: Verify MCP server provider implementation.The new
mcpServerDefinitionProvidersentry declares a Choreo MCP Server. Ensure that the MCP server provider is properly implemented in the extension code and follows VSCode's MCP integration guidelines.
220-221: Verify download-choreo-cli script implementation.The postbuild script now runs
download-choreo-clifirst. Ensure that the script:
- Handles network failures and timeouts gracefully
- Validates downloaded CLI integrity (checksums/signatures if applicable)
- Does not block the build indefinitely
- Provides clear error messages if download fails
261-261: Approve @iarna/toml dependency addition.The addition of
@iarna/tomlfor TOML configuration file parsing is appropriate and well-maintained. Ensure that TOML files are parsed safely and that file input validation is in place.workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (2)
29-33: Clean defensive implementation.The helper correctly uses optional chaining and nullish coalescing for safe handling of undefined/null values. The type assertion after the truthy filter is appropriate here.
35-44: LGTM!The public API methods are well-implemented with appropriate null handling and delegation patterns.
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (2)
569-607: LGTM!The new interfaces (
SubscriptionItem,GithubRepository,GithubOrganization,GitRepoMetadata) are well-structured and provide clear type definitions for Git and subscription data modeling.
188-189: No backward compatibility issues detected for the new required fields inBuildKind.status.The
clusterIdandbuildReffields are part of a response type populated by backend RPC methods (getBuilds,createBuild). No code in the codebase manually constructsBuildKindobjects, so adding these required fields is not a breaking change—the backend must simply ensure it always provides these fields in status responses.
|
|
||
| // Context state subscriptions | ||
| public subscribeContextState = (callback: (state: ContextItemEnriched | undefined)=>void) => contextStore.subscribe((state)=>callback(state.state?.selected)); | ||
| public subscribeDirComponents = (fsPath: string, callback: (comps: ComponentKind[])=>void) => contextStore.subscribe((state)=>callback(this.getComponentsOfDir(fsPath, state.state.components))); |
There was a problem hiding this comment.
Missing optional chaining may cause runtime error.
state.state.components lacks the optional chaining used elsewhere (e.g., line 37 uses state?.components, line 51 uses state.state?.selected). If state.state is null/undefined, this will throw.
Apply this diff to add consistent null-safety:
- public subscribeDirComponents = (fsPath: string, callback: (comps: ComponentKind[])=>void) => contextStore.subscribe((state)=>callback(this.getComponentsOfDir(fsPath, state.state.components)));
+ public subscribeDirComponents = (fsPath: string, callback: (comps: ComponentKind[])=>void) => contextStore.subscribe((state)=>callback(this.getComponentsOfDir(fsPath, state.state?.components)));📝 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.
| public subscribeDirComponents = (fsPath: string, callback: (comps: ComponentKind[])=>void) => contextStore.subscribe((state)=>callback(this.getComponentsOfDir(fsPath, state.state.components))); | |
| public subscribeDirComponents = (fsPath: string, callback: (comps: ComponentKind[])=>void) => contextStore.subscribe((state)=>callback(this.getComponentsOfDir(fsPath, state.state?.components))); |
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts
around line 52, the subscribeDirComponents method directly accesses
state.state.components which can throw if state.state is null/undefined; update
the access to use optional chaining (e.g., state.state?.components) and ensure
the callback receives a safe value (pass an empty array or undefined through
getComponentsOfDir if components is missing) so the subscription no longer
throws on null state.
…g WSO2AuthenticationProvider - Removed all references to authStore and replaced them with ext.authProvider for user authentication state management. - Updated commands related to user login, logout, and context management to utilize the new authentication provider. - Ensured that user information retrieval and context updates are consistent with the new authentication flow. - Deleted the authStore implementation and adjusted related stores and utilities to maintain functionality.
…ensions into devant-main-sync
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… devant-main-sync
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical 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 (3)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.ts (1)
328-355: Fix null-safety + contract mismatch ingetUserInfoForCmd(avoiduserInfo!and returnnullon cancel).Today this can fall through and return
userInfo!whileuserInfois stillundefined(e.g., user dismisses the first modal), which breaksPromise<UserInfo | null>and can crash callers. Also Line 338 duplicates the sameif.export const getUserInfoForCmd = async (message: string): Promise<UserInfo | null> => { - let userInfo = ext.authProvider?.getState().state.userInfo; + let userInfo = ext.authProvider?.getState?.()?.state?.userInfo ?? null; const extensionName = webviewStateStore.getState().state.extensionName; if (!userInfo) { const loginSelection = await window.showInformationMessage( `You are not logged into ${extensionName}.`, { modal: true, detail: `Please login to continue and ${message}` }, "Login", ); - if (loginSelection === "Login") { - if (loginSelection === "Login") { - await commands.executeCommand(CommandIds.SignIn); - } + if (loginSelection !== "Login") { + return null; + } + + await commands.executeCommand(CommandIds.SignIn); userInfo = await waitForLogin(); + if (!userInfo) { + return null; + } const response = await window.showInformationMessage( `Successfully logged into ${extensionName}`, { modal: true, detail: `Do you want to continue and ${message}` }, "Continue", ); - if (response === "Continue") { - return userInfo; - } - return null; - } + return response === "Continue" ? userInfo : null; } - return userInfo!; + return userInfo; };workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (2)
154-170: Add loginMethod validation before accessing accessToken from credentials.secrets.The code assumes credentials follow the BI_INTEL schema without checking
credentials.loginMethodfirst. For other authentication methods (ANTHROPIC_KEY, DEVANT_ENV, AWS_BEDROCK), the secrets object has a different structure and may lack anaccessTokenproperty, causing the function to resolve withundefined. This should match the pattern used inbi-diagram/rpc-manager.ts(lines 723–726), which validates the login method before casting:async getAccessToken(): Promise<string> { return new Promise(async (resolve, reject) => { try { const credentials = await getAccessToken(); if (!credentials) { reject(new Error("Access Token is undefined")); return; } + if (credentials.loginMethod !== LoginMethod.BI_INTEL) { + reject(new Error("Access token is only available for BI Intelligence authentication")); + return; + } const secrets = credentials.secrets as BIIntelSecrets; const accessToken = secrets.accessToken; resolve(accessToken); } catch (error) { reject(error); } }); }
20-63: Add loginMethod guard before castingcredentials.secretstoBIIntelSecrets.In
getAccessToken()(line 163), the code castscredentials.secrets as BIIntelSecretswithout verifying the login method. TheAuthCredentialstype definessecretsas a union of four different secret types:BIIntelSecrets | AnthropicKeySecrets | DevantEnvSecrets | AwsBedrockSecrets. If the stored credentials use a non-BI_INTEL login method (ANTHROPIC_KEY, DEVANT_ENV, or AWS_BEDROCK), the cast is invalid andsecrets.accessTokenwill be undefined, causing a runtime error. Add a guard to ensurecredentials.loginMethod === LoginMethod.BI_INTELbefore accessingsecrets.accessToken.The
BIIntelSecretsimport is correctly exported from@wso2/ballerina-coreandgetProjectUuid()is properly implemented.
♻️ Duplicate comments (2)
workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts (1)
124-128: Leading space in URL template literal creates invalid URL.This issue was previously flagged: Line 126 has a leading space before the template (
${extensionName...) which produces invalid URLs like" https://..."with a leading space. Additionally, if the config URL isundefined, it will produce" undefined/signup".Apply this fix:
if (res === "Sign Up") { + const consoleUrl = extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl; + if (!consoleUrl) { + w.showErrorMessage("Configuration error: console URL not available."); + return; + } commands.executeCommand( "vscode.open", - ` ${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/signup`, + `${consoleUrl}/signup`, ); }workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (1)
51-52:subscribeDirComponents: probably safe as-is, but please re-verify hydration/nullability assumptions.
Given the store shape ({ state: initialState, ... }),state.stateshould always exist; if that’s guaranteed, no change needed.
🟠 Major comments (20)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-out-cmd.ts-28-30 (1)
28-30: Handle undefined authProvider explicitly to prevent silent sign-out failure.The optional chaining on
ext.authProvider?.getState().logout()causes a silent no-op ifauthProvideris undefined. When a user triggers the sign-out command, they expect to be logged out or receive an error message—silent failure leaves them without feedback and potentially still authenticated.Apply this diff to handle the undefined case explicitly:
try { isRpcActive(ext); - ext.authProvider?.getState().logout(); + if (!ext.authProvider) { + throw new Error("Authentication provider is not initialized"); + } + ext.authProvider.getState().logout(); } catch (error: any) {workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts-85-88 (1)
85-88: Guard against undefined config values in URL construction.If
ext.config?.billingConsoleUrlisundefined, the template literal will produce an invalid URL like"undefined/cloud/devant/upgrade". Thecommands.executeCommandwill attempt to open this malformed URL.Apply this pattern to both occurrences (lines 85-88 and 99-102):
if (res === "Upgrade") { + const billingUrl = ext.config?.billingConsoleUrl; + if (!billingUrl) { + w.showErrorMessage("Configuration error: billing console URL not available."); + return; + } commands.executeCommand( "vscode.open", - `${ext.config?.billingConsoleUrl}/cloud/${extensionName === "Devant" ? "devant" : "choreo"}/upgrade`, + `${billingUrl}/cloud/${extensionName === "Devant" ? "devant" : "choreo"}/upgrade`, ); }Also applies to: 99-102
workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts-136-139 (1)
136-139: Guard against undefined config values in URL construction.If
ext.config?.devantConsoleUrlorext.config?.choreoConsoleUrlisundefined, the command will attempt to open"undefined"as a URL.Apply this fix:
if (res === `Open ${extensionName} Console`) { + const consoleUrl = extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl; + if (!consoleUrl) { + w.showErrorMessage("Configuration error: console URL not available."); + return; + } - commands.executeCommand("vscode.open", extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl); + commands.executeCommand("vscode.open", consoleUrl); }workspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.ts-19-23 (1)
19-23: Add timeout mechanism towaitForLogin()to prevent indefinite hangs.The
waitForLogin()function (wso2-auth-provider.ts:398-406) creates a Promise that only resolves whenstate.userInfobecomes truthy but has no timeout or rejection path. If the SignIn flow fails or the user closes the browser, callers likegetUserInfoForCmd()(line 341) will hang indefinitely with no way to recover. Additionally, the subscription callback is never unsubscribed, creating a potential memory leak.Consider wrapping
waitForLogin()with a timeout using the existingwithTimeout()utility fromsrc/utils.ts, or implementPromise.race()with a timeout that rejects after a reasonable duration (e.g., 10 minutes). Also ensure the subscription is properly cleaned up if the Promise times out or is cancelled.workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts-332-348 (1)
332-348: Add error handling for getCommit operation.Line 335 calls
repo.getCommit(head.name)without a try-catch. If the commit fetch fails (e.g., repository corruption, missing commit object), it will throw an unhandled error. Consider wrapping this call in the existing try-catch or adding specific error handling.Apply this diff to add error handling:
const repo = newGit.open(gitRoot, dotGit); const head = await repo.getHEAD(); if (head.name) { - const commit = await repo.getCommit(head.name); - try { + try { + const commit = await repo.getCommit(head.name); await window.withProgress( { title: "Updating cloud editor with newly created component...", location: ProgressLocation.Notification }, () =>workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts-228-235 (1)
228-235: Fix case-sensitivity inconsistency in component name uniqueness check.The uniqueness logic uses
toLowerCase()for comparison (lines 229–235), but component name lookups elsewhere in the codebase (uri-handlers.ts, data-cache-store.ts) use case-sensitive exact matching. This creates inconsistency: a user cannot create "mycomponent" if "MyComponent" exists, yet backend queries searching for "mycomponent" will not find "MyComponent". Align the uniqueness check to use case-sensitive comparison matching the rest of the codebase, or confirm the backend API expects case-insensitive names and apply consistent normalization throughout.workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts-390-398 (1)
390-398: Add validation for console URL configuration before constructing URLs.The code at line 395 constructs a URL using
ext.config?.devantConsoleUrlorext.config?.choreoConsoleUrl. SincegetConfigFromCli()inextension.tsline 100 lacks error handling,ext.configcould be undefined if the RPC call fails. While optional chaining prevents exceptions, the resulting URL would be invalid (e.g.,"undefined/organizations/..."). Either add error handling around the RPC initialization or validate that the config values are non-empty before constructing the URL.workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-579-674 (1)
579-674: Refactor complex handler into smaller, testable functions.This handler performs multiple complex operations (Git initialization, URL construction, credential enrichment, file system manipulation, Ballerina.toml updates, cloning, moving files, and pushing) in a single ~95-line function. This makes it difficult to test, debug, and maintain. Additionally, there's no cleanup mechanism if operations fail partway through.
Consider refactoring into smaller functions:
async function validateAndBuildGitUrl(params: CloneRepositoryIntoCompDirReq): Promise<URL> { /* ... */ } async function updateBallerinaToml(cwd: string, org: OrgInfo, componentName: string): Promise<void> { /* ... */ } async function cloneRepository(git: Git, url: string, params: CloneParams): Promise<string> { /* ... */ } async function moveWorkspaceFiles(sourcePath: string, destPath: string): Promise<void> { /* ... */ } async function commitAndPush(git: Git, repoPath: string, message: string, branch: string): Promise<void> { /* ... */ } messenger.onRequest(CloneRepositoryIntoCompDir, async (params: CloneRepositoryIntoCompDirReq) => { try { const url = await validateAndBuildGitUrl(params); await updateBallerinaToml(params.cwd, params.org, params.componentName); const clonedPath = await cloneRepository(newGit, url.href, params); const newPath = await moveWorkspaceFiles(params.cwd, join(clonedPath, params.subpath)); await commitAndPush(newGit, newPath, `Add source for...`, params.repo.branch); return newPath; } catch (error) { // Cleanup logic here throw error; } });This improves:
- Testability (each function can be unit tested)
- Readability (clear separation of concerns)
- Error handling (easier to add cleanup logic)
- Reusability (functions can be used elsewhere)
workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-612-629 (1)
612-629: Remove or document commented-out code.A large block of code (~17 lines) is commented out with a TODO note. Commented code should either be removed or, if it's intended for future implementation, tracked in an issue rather than left in the codebase.
Remove the commented code and track the feature request in your issue tracker, or document why this code is preserved and when it will be enabled.
workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-600-610 (1)
600-610: Fix mixed sync/async filesystem operations.The code mixes synchronous (
existsSync) and asynchronous (fs.promises.readFile,fs.promises.writeFile) filesystem operations, which is inconsistent and potentially error-prone. Additionally, Line 606 could fail ifcomponentNameisnull.Use consistent async operations and add proper validation:
// if ballerina toml exists, need to update the org and name const balTomlPath = join(params.cwd, "Ballerina.toml"); -if (existsSync(balTomlPath)) { +if (fs.existsSync(balTomlPath)) { + if (!params.componentName) { + throw new Error("Component name is required for Ballerina.toml update"); + } const fileContent = await fs.promises.readFile(balTomlPath, "utf-8"); const parsedToml: any = toml.parse(fileContent); if (parsedToml?.package) { parsedToml.package.org = params.org.handle; - parsedToml.package.name = params.componentName?.replaceAll("-", "_"); + parsedToml.package.name = params.componentName.replaceAll("-", "_"); } const updatedTomlContent = toml.stringify(parsedToml); await fs.promises.writeFile(balTomlPath, updatedTomlContent, "utf-8"); }workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-585-598 (1)
585-598: Strengthen URL validation and error handling.The URL validation only checks if it starts with
https://but doesn't validate the URL structure. Additionally, credential enrichment (line 594) has no error handling if it fails.Enhance validation and add error handling:
const _repoUrl = buildGitURL(params.repo.orgHandler, params.repo.repo, params.repo.provider, true, params.repo.serverUrl); if (!_repoUrl || !_repoUrl.startsWith("https://")) { - throw new Error("failed to parse git details"); + throw new Error(`Invalid repository URL: ${_repoUrl}. Only HTTPS URLs are supported.`); } -const urlObj = new URL(_repoUrl); + +let urlObj: URL; +try { + urlObj = new URL(_repoUrl); +} catch (error) { + throw new Error(`Failed to parse repository URL: ${error instanceof Error ? error.message : String(error)}`); +} const parsed = parseGitURL(_repoUrl); if (parsed) { const [repoOrg, repoName, provider] = parsed; - await enrichGitUsernamePassword(params.org, repoOrg, repoName, provider, urlObj, _repoUrl, params.repo.secretRef || ""); + try { + await enrichGitUsernamePassword(params.org, repoOrg, repoName, provider, urlObj, _repoUrl, params.repo.secretRef || ""); + } catch (error) { + throw new Error(`Failed to enrich Git credentials: ${error instanceof Error ? error.message : String(error)}`); + } +} else { + throw new Error("Failed to parse Git repository details from URL"); }workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-192-200 (1)
192-200: Add URL validation before opening external links.If
ext.configor the console URL properties are undefined,vscode.Uri.parsewill receive an empty string, potentially leading to unexpected behavior or a broken user experience.Add validation before attempting to open the external URL:
messenger.onRequest(OpenExternalChoreo, (choreoPath: string) => { + const baseUrl = (webviewStateStore.getState().state.extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl); + if (!baseUrl) { + window.showErrorMessage("Console URL is not configured"); + return; + } vscode.env.openExternal( vscode.Uri.joinPath( - vscode.Uri.parse( - (webviewStateStore.getState().state.extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl) || "", - ), + vscode.Uri.parse(baseUrl), choreoPath, ), ); });Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-297-308 (1)
297-308: Validate configuration before constructing GitHub URLs.If
ext.configor nested properties (ghApp.authUrl,ghApp.clientId,ghApp.installUrl) are undefined, the URL construction will embedundefinedas a string, creating invalid URLs that will confuse users when opened externally.Add validation before constructing and opening URLs:
messenger.onRequest(TriggerGithubAuthFlow, async (orgId: string) => { + if (!ext.config?.ghApp.authUrl || !ext.config?.ghApp.clientId) { + window.showErrorMessage("GitHub App configuration is missing"); + return; + } const extName = webviewStateStore.getState().state.extensionName; const baseUrl = extName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl; + if (!baseUrl) { + window.showErrorMessage("Console URL is not configured"); + return; + } const redirectUrl = `${baseUrl}/ghapp`; const state = await _getGithubUrlState(orgId); const ghURL = Uri.parse(`${ext.config.ghApp.authUrl}?redirect_uri=${redirectUrl}&client_id=${ext.config.ghApp.clientId}&state=${state}`); await env.openExternal(ghURL); }); messenger.onRequest(TriggerGithubInstallFlow, async (orgId: string) => { + if (!ext.config?.ghApp.installUrl) { + window.showErrorMessage("GitHub App configuration is missing"); + return; + } const state = await _getGithubUrlState(orgId); const ghURL = Uri.parse(`${ext.config.ghApp.installUrl}?state=${state}`); await env.openExternal(ghURL); });workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-652-661 (1)
652-661: Add error handling and conflict resolution for file operations.The file copy loop (lines 656-660) lacks error handling and doesn't check for existing files in the destination, which could lead to data loss or unexpected overwrites.
Add proper error handling and validation:
// Move everything into cloned dir const cwdFiled = readdirSync(params.cwd); const newPath = join(clonedPath, params.subpath); fs.mkdirSync(newPath, { recursive: true }); for (const file of cwdFiled) { + // Skip the cloned directory itself to avoid recursion + if (join(params.cwd, file) === clonedPath) { + continue; + } const cwdFilePath = join(params.cwd, file); const destFilePath = join(newPath, file); + if (fs.existsSync(destFilePath)) { + getLogger().warn(`Destination file already exists, will be overwritten: ${destFilePath}`); + } + try { - fs.cpSync(cwdFilePath, destFilePath, { recursive: true }); + await fs.promises.cp(cwdFilePath, destFilePath, { recursive: true }); + } catch (error) { + throw new Error(`Failed to copy ${cwdFilePath} to ${destFilePath}: ${error instanceof Error ? error.message : String(error)}`); + } }Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts-162-170 (1)
162-170: Potentialundefinedin cache key when region is not set.If
ext.authProvider?.getState().state.regionisundefined(e.g., before authentication completes), the key will contain the literal string"undefined", producing keys like"undefined-orgHandle"or"env-undefined-orgHandle". This could cause:
- Data stored under malformed keys that become orphaned after login
- Cache misses when the region becomes available
Consider adding a guard or fallback:
const getRootKey = (orgHandle: string) => { const region = ext.authProvider?.getState().state.region; + if (!region) { + // Return orgHandle directly if region isn't available yet + return orgHandle; + } const env = ext.choreoEnv; let orgRegionHandle = `${region}-${orgHandle}`; if (env !== "prod") { orgRegionHandle = `${env}-${orgRegionHandle}`; } return orgRegionHandle; };workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts-38-48 (1)
38-48: PII persistence: confirm storing fulluserInfoin SecretStorage is acceptable.
Even if SecretStorage is encrypted, this increases retained personal/org data; consider storing only what you need to restore auth quickly (and refetch the rest).workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts-198-234 (1)
198-234: Bug risk:CLOUD_INITIAL_PROJECT_IDlikely won’t matchProject.iddue to string/number comparison.
matchingProject = projectsCache.find((item) => item.id === process.env.CLOUD_INITIAL_PROJECT_ID)(Line 212) will fail ifitem.idis numeric.- const matchingProject = projectsCache.find((item) => item.id === process.env.CLOUD_INITIAL_PROJECT_ID); + const matchingProject = projectsCache.find( + (item) => item.id?.toString() === process.env.CLOUD_INITIAL_PROJECT_ID, + );Also consider guarding
ext.clients.rpcClient.getProjects(...)with try/catch here (you do elsewhere).workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts-145-170 (1)
145-170: Logout should also clear persisted org/project caches and context selection.
Right nowresetState()clears auth, butdataCacheStoreandcontextStorecan retain previous-account org/project metadata (and selection) across sessions.public async logout(silent = false, skipClearSessions = false) { @@ // Clear local state this.resetState(); + // Clear derived/persisted app state (avoid cross-account bleed) + contextStore.getState().resetState(); + // If data-cache has a reset/clear API, call it here as well.Committable suggestion skipped: line range outside the PR's diff.
workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts-215-249 (1)
215-249: Single-account support is broken: old sessions are never removed from SecretStorage.
storeSession()says it removes existing sessions, but it only reports them as removed; it doesn’t delete them from storage, so sessions can accumulate andinitAuth()may restore the wrong one.private async storeSession(userInfo: UserInfo, region: "US" | "EU"): Promise<AuthenticationSession> { // Remove any existing sessions first (single account support) const existingSessions = await this.readSessions(); const removedSessions = [...existingSessions]; + if (removedSessions.length > 0) { + await this.secretStorage.delete(WSO2_SESSIONS_SECRET_KEY); + } @@ await this.storeSessions([session], sessionData);workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts-108-114 (1)
108-114:createSession()throwing will break VS Code's Accounts-based sign-in flow on first login.When a user initiates sign-in via the Accounts menu with
createIfNone: true, VS Code callsgetSessions()first. If no sessions exist, it callscreateSession(). SincecreateSession()throws an error instead of delegating to the RPC authentication flow, the sign-in will fail on first attempt. After successful RPC authentication,storeSession()stores the session, so subsequent calls work viagetSessions(). To support the standard Accounts UI flow, routecreateSession()to the RPC sign-in flow and return the stored session result instead of throwing.
🟡 Minor comments (4)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-out-cmd.ts-29-29 (1)
29-29: Consider allowing sign-out even when extension is uninitialized.The
isRpcActive(ext)check throws an exception if the extension hasn't fully initialized, which is caught and displayed to the user. However, unlike other commands that require RPC for their operations, sign-out should succeed regardless of extension state to ensure users aren't locked into authenticated sessions. Consider movingisRpcActive(ext)after the logout call, or removing it entirely from this command if the auth provider handles its own state checks.workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts-134-138 (1)
134-138: Line 138 returns undefined instead of AuthState, violating the request type contract.The
GetAuthStaterequest handler should return a validAuthStateobject, but currently returnsundefinedifauthProvideris falsy. Althoughext.authProvideris always initialized during extension activation (before webviews are created), the optional chaining should be paired with a default value to guarantee type safety. Use the nullish coalescing operator to return a default state:messenger.onRequest(GetAuthState, () => ext.authProvider?.getState().state ?? { userInfo: null, region: "US" as const });This matches the pattern already used in
PlatformExtensionApi.tsline 34.workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts-47-52 (1)
47-52: Non-null assertion after optional chaining can produceundefined.On line 49, the non-null assertion
!at the end doesn't preventundefinedfrom being assigned if any part of the optional chain evaluates toundefined. This will result inidpId: undefinedbeing sent in telemetry data, which could cause issues downstream.Apply this diff to handle the undefined case explicitly:
export function getCommonProperties(): { [key: string]: string } { return { - idpId: ext.authProvider?.getState().state?.userInfo?.userId!, + idpId: ext.authProvider?.getState().state?.userInfo?.userId ?? "", // check if the email ends with "@wso2.com" isWSO2User: ext.authProvider?.getState().state?.userInfo?.userEmail?.endsWith("@wso2.com") ? "true" : "false", }; }workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts-398-406 (1)
398-406:waitForLogin()leaks a subscription; unsubscribe after resolve.export const waitForLogin = async (): Promise<UserInfo> => { return new Promise((resolve) => { - ext.authProvider?.subscribe(({ state }) => { + const unsubscribe = ext.authProvider?.subscribe(({ state }) => { if (state.userInfo) { + unsubscribe?.(); resolve(state.userInfo); } }); }); };
🧹 Nitpick comments (9)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts (1)
170-170: Consider reusing the existinguserInfovariable.Line 47 already retrieves and line 48 validates
userInfo, so it's available and guaranteed to be non-null at this point. PassinguserInfodirectly is simpler and avoids re-fetching fromext.authProvider.Apply this diff to use the existing variable:
-updateContextFile(clonedResp[0].clonedPath, ext.authProvider?.getState().state.userInfo!, selectedProject, selectedOrg, projectCache); +updateContextFile(clonedResp[0].clonedPath, userInfo, selectedProject, selectedOrg, projectCache);workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (1)
369-371: Consider enabling component details view for Devant integrations.The component details view is only shown for non-Devant components. If users would benefit from seeing integration details in Devant mode, consider removing this restriction or clarifying the reasoning behind this design choice.
workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts (1)
19-36: Verify all filesystem imports are necessary.Multiple filesystem operations are imported, including both named imports and a wildcard import (
* as fs). This creates redundancy and may include unused imports.Consider:
- Using only the wildcard import (
import * as fs) for consistency, or- Verifying that named imports like
mkdtempSync,renameSync, andrmdirSyncare actually usedApply this pattern for cleaner imports:
-import { - copyFileSync, - existsSync, - mkdirSync, - mkdtempSync, - readFileSync, - readdirSync, - renameSync, - rmSync, - rmdirSync, - statSync, - unlinkSync, - writeFileSync, -} from "fs"; -import * as fs from "fs"; +import * as fs from "fs"; +import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync, statSync, unlinkSync, writeFileSync } from "fs";workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (2)
70-85: Indentation inconsistency in sign-in callback.The code block at lines 83-85 has inconsistent indentation - it appears to be inside the
if (userInfo)block but is not properly indented to match. This affects readability.if (userInfo) { if (contextStore?.getState().state?.selected) { const includesOrg = userInfo.organizations?.some((item) => item.handle === contextStore?.getState().state?.selected?.orgHandle); if (!includesOrg) { contextStore.getState().resetState(); } } - const region = await ext.clients.rpcClient.getCurrentRegion(); - await ext.authProvider?.getState().loginSuccess(userInfo, region); - window.showInformationMessage(`Successfully signed into ${extName}`); + const region = await ext.clients.rpcClient.getCurrentRegion(); + await ext.authProvider?.getState().loginSuccess(userInfo, region); + window.showInformationMessage(`Successfully signed into ${extName}`); }
267-267: Non-null assertion on potentially undefineduserInfo.The non-null assertion
ext.authProvider?.getState().state.userInfo!could fail ifauthProvideris undefined. WhileswitchContextAndOpenDiris only called after successful authentication, defensive handling would be safer.- const contextFilePath = updateContextFile(gitRoot, ext.authProvider?.getState().state.userInfo!, project, org, projectCache); + const userInfo = ext.authProvider?.getState().state.userInfo; + if (!userInfo) { + window.showErrorMessage("User information not available. Please log in and try again."); + return; + } + const contextFilePath = updateContextFile(gitRoot, userInfo, project, org, projectCache);workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (2)
60-79:refreshStatesequencing looks coherent after authProvider migration.
Only nit:items = await getEnrichedContexts(get().state?.items)(Line 69) could just pass the localitemsyou already computed to avoid any chance of reading a mid-update snapshot.
340-349: Component-id filtering: normalize ID types to avoid “no matches” due to string/number mismatch.- const filteredComps = components.filter((item) => item.metadata?.id === workspaceCompId); + const filteredComps = components.filter((item) => item.metadata?.id?.toString() === workspaceCompId);workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (1)
27-33:getComponentsOfDir: prefer a real type guard instead of casting.- return (components?.filter((item) => isSamePath(item?.componentFsPath, fsPath)) - ?.map((item) => item?.component) - ?.filter((item) => !!item) as ComponentKind[]) ?? [] + return ( + components + ?.filter((item) => isSamePath(item?.componentFsPath, fsPath)) + ?.map((item) => item?.component) + ?.filter((item): item is ComponentKind => item != null) ?? [] + );workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (1)
175-209:initAuth()logic is fine, but avoid calling fulllogout()(RPC signOut) when you just have “no session”.
IfgetUserInfo()returns null because there is no session, callingsignOut()on startup is unnecessary work/noise; consider a lightweight “local reset only” path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts(3 hunks)workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts(3 hunks)workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.ts(3 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts(7 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/refresh-directory-context-cmd.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-in-with-code-cmd.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-out-cmd.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts(6 hunks)workspaces/wso2-platform/wso2-platform-extension/src/extension.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/status-bar.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/stores/auth-store.ts(0 hunks)workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts(7 hunks)workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts(5 hunks)workspaces/wso2-platform/wso2-platform-extension/src/tarminal-handlers.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts(5 hunks)workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts(9 hunks)
💤 Files with no reviewable changes (1)
- workspaces/wso2-platform/wso2-platform-extension/src/stores/auth-store.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-in-with-code-cmd.ts
- workspaces/ballerina/ballerina-extension/src/rpc-managers/bi-diagram/rpc-manager.ts
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/(Diagram|DiagramCanvas|Controls)*.tsx : Store diagram zoom and offset positions in state; optionally cache them in localStorage for session persistence
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/status-bar.tsworkspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.tsworkspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.tsworkspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.tsworkspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/resources/icons/**/*.tsx : Create separate SVG icon components in src/resources/icons/ for all diagram icons and import them as React components
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.tsworkspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Use TypeScript 5.8.3 with strict type checking enabled for all source files
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.tsworkspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.tsworkspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.tsworkspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts
📚 Learning: 2025-12-12T13:11:41.252Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 1096
File: workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts:27-28
Timestamp: 2025-12-12T13:11:41.252Z
Learning: In workspaces/ballerina/ballerina-extension/src/views/ai-panel/activate.ts, when processing projectInfo.children in handleWorkspaceLevelAIPanel, the projectPath field is always available for child projects in the workspace, so additional filtering or type guards are not necessary.
Applied to files:
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.tsworkspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/DiagramContext.tsx : Use DiagramContext (React Context API) for state management across the diagram component, including expandedNodes and graphQLGroupOpen state
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/**/*.config.{js,ts} : Use minimatch-compatible glob patterns for file matching in build and test configuration files
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
📚 Learning: 2025-12-12T13:11:28.926Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 1096
File: workspaces/ballerina/ballerina-extension/src/features/project/cmds/add.ts:105-112
Timestamp: 2025-12-12T13:11:28.926Z
Learning: In workspaces/ballerina/ballerina-extension/src/features/project/cmds/add.ts, the `getPackage` function is only called in contexts where `projectInfo.children[].projectPath` is guaranteed to be available, so no filtering for undefined values is needed when mapping over children.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts
🧬 Code graph analysis (11)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/refresh-directory-context-cmd.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts (2)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/interfaces.ts (1)
ErrorCode(39-42)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/sign-out-cmd.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/status-bar.ts (2)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (1)
state(90-92)
workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (3)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (1)
UserInfo(62-70)workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (1)
contextStore(55-119)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/cmd-utils.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (4)
workspaces/ballerina/ballerina-core/src/rpc-types/ai-panel/rpc-type.ts (1)
getAccessToken(28-28)workspaces/ballerina/ballerina-rpc-client/src/rpc-clients/ai-panel/rpc-client.ts (1)
getAccessToken(130-132)workspaces/ballerina/ballerina-extension/src/features/natural-programming/utils.ts (1)
getAccessToken(513-524)workspaces/ballerina/ballerina-core/src/state-machine-types.ts (1)
BIIntelSecrets(359-362)
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (3)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (1)
state(90-92)workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts (1)
dataCacheStore(38-160)
workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (5)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (3)
IWso2PlatformExtensionAPI(24-43)ComponentKind(131-139)openClonedDirReq(45-52)workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (1)
contextStore(55-119)workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (1)
openClonedDir(145-173)workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (1)
state(90-92)
🔇 Additional comments (25)
workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts (1)
22-22: LGTM!The import of
extis necessary for the dynamic config-based URL resolution and auth provider access.workspaces/ballerina/ballerina-extension/src/rpc-managers/ai-panel/rpc-manager.ts (1)
118-145: ConfirmCLOUD_INITIAL_PROJECT_IDsemantics to avoid project-UUID collisions.
Hashing a shared env var could make different workspaces/projects report the same UUID; if downstream caches/telemetry assume uniqueness, that’s risky.Suggested hardening (still keeps your behavior, but avoids accidental whitespace / empty IDs and makes intent explicit):
- const cloudProjectId = process.env.CLOUD_INITIAL_PROJECT_ID; + const cloudProjectId = process.env.CLOUD_INITIAL_PROJECT_ID?.trim(); - if (cloudProjectId) { + if (cloudProjectId && cloudProjectId.length > 0) { projectIdentifier = cloudProjectId; } else { projectIdentifier = workspace.workspaceFolders[0].uri.fsPath; }workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (4)
62-62: Good practice: Synchronizing context store access.Waiting for the context store to load before proceeding prevents race conditions and ensures data consistency.
176-182: Git initialization check handles missing repos gracefully.The try-catch block correctly handles cases where Git is not initialized without disrupting the flow.
244-244: The logic is correct.isNewCodeServerCompistruewhen git is not initialized AND running in the Devant cloud editor, which appropriately identifies scenarios where a new repository needs to be initialized. The flag controls whether the form shows repository initialization details and performs repository setup—this behavior aligns with its purpose.
325-367: Add explicit check for devantEditor extension availability before executing cross-extension command.Line 355 executes
devantEditor.clearLocalStoragefrom an external extension without verifying it's loaded. The try-catch masks failures but doesn't prevent runtime errors. Add an explicit availability check like the choreo extension check at line 39 ofextensionVariables.ts:const isDevantEditorAvailable = !!extensions.getExtension("devantEditor");Then conditionally execute the command only if the extension is active, or handle the case where it's unavailable more gracefully than silent error logging.
workspaces/wso2-platform/wso2-platform-extension/src/webviews/WebviewRPC.ts (2)
277-282: LGTM - Environment check with appropriate user feedback.The environment check appropriately restricts the runtime logs feature to production and provides clear user feedback when the feature is unavailable.
401-404: LGTM - Proper null checks for auth state.The optional chaining and early return appropriately handle cases where the auth provider or organization information is unavailable.
workspaces/wso2-platform/wso2-platform-extension/src/cmds/refresh-directory-context-cmd.ts (1)
31-34: LGTM!The migration from direct auth-store access to
ext.authProvider?.getState().state.userInfois consistent with the broader authentication provider refactor. The null check on line 32 properly guards against undefined userInfo before proceeding with the context refresh.workspaces/wso2-platform/wso2-platform-extension/src/tarminal-handlers.ts (2)
29-31: LGTM on configuration expansion.The logic to support both
choreoandchoreoConnectconfigurations is sound. When both are present,config.choreotakes precedence via the||operator, which maintains backward compatibility.
67-67: Auth provider migration looks correct.The switch from auth-store to
ext.authProvider?.getState().state?.userInfois consistent with the broader refactor pattern.workspaces/wso2-platform/wso2-platform-extension/src/status-bar.ts (1)
14-25: LGTM!The migration to
ext.authProviderfor status bar state is well-implemented. The type change fromAuthState | nulltoAuthState | undefinedcorrectly reflects the optional nature of the provider, and the optional chaining onsubscribegracefully handles cases where the auth provider isn't initialized.workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts (1)
158-158: Storage key rename will invalidate existing cached data.The storage key changed from
"data-cache-zustand-storage-v1"to"data-cache-zustand-storage". Existing users will lose their cached projects, components, commits, and environments data upon upgrade. If this is intentional (e.g., due to incompatible structure changes), consider documenting it. Otherwise, maintain the versioned key or implement a migration.workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
36-40: LGTM on initialization logic.The constructor properly initializes environment detection flags:
isDevantCloudEditorcorrectly checks for the cloud STS token environment variableisChoreoExtInstalleduses the VS Code extensions API appropriatelyNote that
choreoEnvdefaults to"prod"here but is immediately overwritten inextension.tsviagetChoreoEnv(), which is fine since the constructor runs before extension activation.workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (1)
106-106: Good catch on the typo fix.The log message correction from "Githup" to "Github" improves log clarity.
workspaces/wso2-platform/wso2-platform-extension/src/extension.ts (2)
92-99: Conditional feature activation is well-structured.The feature flags (
isChoreoExtInstalled,isDevantCloudEditor) cleanly gate the activation of MCP and Devant-specific features. This ensures features only activate in appropriate environments.
111-118: Environment resolution cascade is comprehensive.The
getChoreoEnvfunction properly cascades through:
CHOREO_ENVenvironment variableCLOUD_ENVenvironment variable- VS Code workspace configuration
- Default to
"prod"This provides flexibility for different deployment scenarios.
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (4)
39-40: Good add:isSamePathalongsideisSubpathfor component path resolution.
155-168: Org/project enrichment now depends onext.authProviderstate — ensure it’s initialized beforerefreshState.
IfauthProvider.initAuth()can racecontextStore.refreshState(), you may get transient “missing org” items until the next refresh.
262-287: OK: org list now sourced from authProvider.
353-390: Path inclusion condition is stricter/safer now (exact match OR subpath + exists + dedupe).workspaces/wso2-platform/wso2-platform-extension/src/PlatformExtensionApi.ts (2)
19-25: Imports look consistent with the new API surface.
34-50: Nice: auth + context getters/subscriptions are now centralized and simple.workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (2)
67-85: Good:subscribe()immediately emits current state and returns an unsubscribe.
127-140:loginSuccessflow is reasonable (state → caches → refresh → persist → emit).
workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts
Outdated
Show resolved
Hide resolved
| if (ext.authProvider?.getState().state?.userInfo) { | ||
| ext.authProvider?.getState().logout(); |
There was a problem hiding this comment.
Incomplete optional chaining can cause runtime errors.
The optional chaining stops after authProvider but not after getState(). If authProvider exists but getState() returns null or undefined, accessing .state or .logout() will throw a TypeError.
Apply this pattern to all four occurrences (lines 46-47, 52-53, 58-59, 67-68):
- if (ext.authProvider?.getState().state?.userInfo) {
- ext.authProvider?.getState().logout();
+ if (ext.authProvider?.getState()?.state?.userInfo) {
+ ext.authProvider?.getState()?.logout();Also applies to: 52-53, 58-59, 67-68
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/error-utils.ts around
lines 46-47 (and similarly at 52-53, 58-59, 67-68), the optional chaining stops
at ext.authProvider but then calls getState() and accesses properties/methods
without guarding against getState() returning null/undefined; update each
occurrence to null-check getState() (e.g., use
ext.authProvider?.getState()?.state?.userInfo and
ext.authProvider?.getState()?.logout() or store the result of getState() in a
const and guard it) so that you only access .state or call .logout() when
getState() returns a defined object.
| // Initialize and register authentication provider | ||
| const authProvider = new WSO2AuthenticationProvider(context.secrets); | ||
| ext.authProvider = authProvider; | ||
| context.subscriptions.push( | ||
| authentication.registerAuthenticationProvider(WSO2_AUTH_PROVIDER_ID, "WSO2 Platform", authProvider, { | ||
| supportsMultipleAccounts: false, | ||
| }), | ||
| ); | ||
|
|
||
| // Subscribe to auth state changes | ||
| authProvider.subscribe(({ state }) => { | ||
| vscode.commands.executeCommand("setContext", "isLoggedIn", !!state.userInfo); | ||
| }); | ||
|
|
||
| await initRPCServer(); | ||
| await ext.clients.rpcClient.init(); | ||
| authProvider.getState().initAuth(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for initAuth method definition in the codebase
rg "initAuth" --type ts -B 2 -A 8Repository: wso2/vscode-extensions
Length of output: 4948
🏁 Script executed:
#!/bin/bash
# Get the complete initAuth implementation
rg -n "public async initAuth" --type ts -A 20 workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts
# Also check continueCreateComponent to see if it depends on auth state
rg -n "continueCreateComponent" --type ts -B 2 -A 15Repository: wso2/vscode-extensions
Length of output: 7501
Await initAuth() on line 90 — initAuth() is an async method that performs critical initialization (retrieving session data, updating auth state, refreshing context and data stores, and setting organization context). Calling it without await creates a race condition where continueCreateComponent() on line 91 may execute before authentication is properly initialized. This can cause component creation to proceed with incomplete or missing auth context.
Change:
authProvider.getState().initAuth();
continueCreateComponent();
To:
await authProvider.getState().initAuth();
continueCreateComponent();
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/extension.ts around
lines 74 to 90, authProvider.getState().initAuth() is called without awaiting
its asynchronous initialization which can cause race conditions; change the call
to await authProvider.getState().initAuth() so initialization completes before
proceeding, then call continueCreateComponent() after that await to ensure
authentication and context are fully ready.
…ode-extensions into devant-main-sync
Introduce AuthenticationProvider for Platform auth handling
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts(1 hunks)
🔇 Additional comments (18)
workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts (18)
50-58: LGTM!The constructor properly accepts
SecretStoragevia dependency injection and initializes the event emitters and disposables correctly.
67-72: LGTM!The subscribe pattern correctly provides immediate state and future updates. The returned cleanup function allows proper resource management.
77-85: LGTM!The method binding pattern ensures
thiscontext is preserved when methods are destructured or called separately.
97-106: LGTM!Standard implementation of the
getSessionsmethod with correct scope filtering logic.
112-114: LGTM!Appropriately throws an error since authentication is handled via RPC. The error message is clear and informative.
145-170: LGTM!The logout flow properly handles errors without blocking the complete signout process, and the
skipClearSessionsparameter correctly prevents recursion.
175-192: LGTM!The initialization flow correctly attempts to restore auth state from the RPC client and falls back to logout on error. The optional chaining at line 183 is defensive and appropriate for initialization where dependencies might not be fully ready.
198-232: LGTM!The single-account session management is correctly implemented, removing existing sessions before storing the new one. The session change event properly reflects both additions and removals.
238-252: LGTM!The session removal correctly prevents recursion by passing
skipClearSessions=trueto the logout method.
257-266: LGTM!Straightforward implementation that correctly clears all sessions and notifies listeners.
271-278: LGTM!Correctly implements single-account semantics by returning the first session when no ID is specified.
283-285: LGTM!The dispose method correctly cleans up event emitters. Not clearing sessions/state is appropriate as dispose focuses on resource cleanup rather than data modification.
290-299: LGTM!These backward-compatibility helpers provide a clean migration path from the previous auth implementation.
304-322: LGTM!Properly handles errors by logging and returning an empty array, preventing the application from crashing on corrupted storage.
327-351: LGTM!The dual-mode operation (add/update vs. filter) is correctly implemented and the error handling appropriately logs without throwing.
356-368: LGTM!Correctly reads and returns full session data with appropriate error handling.
373-375: LGTM!The session ID generation using timestamp and random string provides sufficient uniqueness for practical purposes.
47-47: Verify region constraint meets product roadmap requirements.The region is hardcoded to
"US" | "EU"throughout the platform (consistently applied in auth provider, RPC client, store types, and API exports). Ensure this aligns with current and planned deployment regions. If additional regions will be supported, update this constraint and add corresponding region detection logic.
| public async loginSuccess(userInfo: UserInfo, region: "US" | "EU") { | ||
| // Update local state | ||
| this._state = { userInfo, region }; | ||
|
|
||
| // Update related stores | ||
| dataCacheStore.getState().setOrgs(userInfo.organizations); | ||
| contextStore.getState().refreshState(); | ||
|
|
||
| // Store session in secure storage | ||
| await this.storeSession(userInfo, region); | ||
|
|
||
| // Notify subscribers | ||
| this._stateChangeEmitter.fire({ state: this._state }); | ||
| } |
There was a problem hiding this comment.
Add error handling to prevent inconsistent state.
The loginSuccess method updates multiple stores without error handling. If any operation fails (e.g., storeSession), the state could become inconsistent. Consider wrapping the operations in a try-catch block and reverting state on failure.
Additionally, contextStore.getState().refreshState() is an async operation that is not awaited, which could lead to race conditions if subsequent code depends on the refreshed state.
Apply this diff to improve error handling:
public async loginSuccess(userInfo: UserInfo, region: "US" | "EU") {
- // Update local state
- this._state = { userInfo, region };
-
- // Update related stores
- dataCacheStore.getState().setOrgs(userInfo.organizations);
- contextStore.getState().refreshState();
-
- // Store session in secure storage
- await this.storeSession(userInfo, region);
-
- // Notify subscribers
- this._stateChangeEmitter.fire({ state: this._state });
+ try {
+ // Update local state
+ this._state = { userInfo, region };
+
+ // Update related stores
+ dataCacheStore.getState().setOrgs(userInfo.organizations);
+ await contextStore.getState().refreshState();
+
+ // Store session in secure storage
+ await this.storeSession(userInfo, region);
+
+ // Notify subscribers
+ this._stateChangeEmitter.fire({ state: this._state });
+ } catch (error) {
+ getLogger().error("Error during login success", error);
+ // Revert state on failure
+ this._state = { userInfo: null, region: "US" };
+ throw error;
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts
around lines 127-140, loginSuccess updates multiple stores and calls async
refresh without awaiting or protecting against failures; wrap the sequence in a
try-catch, await contextStore.getState().refreshState(), await
this.storeSession(userInfo, region), and only set this._state and fire the
stateChange emitter after all awaited operations succeed; on any error, revert
any partially applied changes (restore previous this._state and rollback stores
if possible), log or rethrow the error so callers know the failure, and ensure
no inconsistent state remains.
| export const waitForLogin = async (): Promise<UserInfo> => { | ||
| return new Promise((resolve) => { | ||
| ext.authProvider?.subscribe(({ state }) => { | ||
| if (state.userInfo) { | ||
| resolve(state.userInfo); | ||
| } | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Critical memory leak: subscription is never disposed.
The waitForLogin function creates a subscription but never cleans it up. Once the promise resolves, the subscription remains active indefinitely, leading to a memory leak. Each call to this function creates a permanent subscription.
Apply this diff to fix the memory leak:
export const waitForLogin = async (): Promise<UserInfo> => {
return new Promise((resolve) => {
- ext.authProvider?.subscribe(({ state }) => {
+ const unsubscribe = ext.authProvider?.subscribe(({ state }) => {
if (state.userInfo) {
+ unsubscribe?.();
resolve(state.userInfo);
}
});
});
};🤖 Prompt for AI Agents
In
workspaces/wso2-platform/wso2-platform-extension/src/auth/wso2-auth-provider.ts
around lines 381 to 389, the waitForLogin function creates an authProvider
subscription but never disposes it; capture the Subscription returned by
ext.authProvider?.subscribe, and inside the callback call
subscription.unsubscribe() immediately before resolving the promise so the
handler is removed after the first userInfo is received; also guard against
ext.authProvider being undefined and consider rejecting or timing out the
promise if needed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (1)
392-400: Critical: Workspace configuration changes are not persisted.The code reads and modifies
workspaceContentbut never writes the changes back to disk. The new component folder will not be added to the workspace.Apply this diff to persist the workspace configuration:
if (workspace.workspaceFile) { const workspaceContent: WorkspaceConfig = JSON.parse(readFileSync(workspace.workspaceFile.fsPath, "utf8")); workspaceContent.folders = [ ...workspaceContent.folders, { name: createdComponent.metadata.name, path: path.normalize(path.relative(path.dirname(workspace.workspaceFile.fsPath), createParams.componentDir)), }, ]; + writeFileSync(workspace.workspaceFile.fsPath, JSON.stringify(workspaceContent, null, 2), "utf8"); + window.showInformationMessage(successMessage, `Open in ${extensionName}`).then(async (resp) => { + if (resp === `Open in ${extensionName}`) { + commands.executeCommand( + "vscode.open", + `${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/organizations/${org.handle}/projects/${extensionName === "Devant" ? project.id : project.handler}/components/${createdComponent.metadata.handler}/overview`, + ); + } + }); }Also add
writeFileSyncto the imports at the top of the file:-import { existsSync, readFileSync } from "fs"; +import { existsSync, readFileSync, writeFileSync } from "fs";
🧹 Nitpick comments (6)
workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts (1)
49-51: Auth state access is safe; consider minor refactor and confirmidpIdsemanticsThe use of optional chaining and
?? ""/"true"/"false"provides safe defaults and should behave well even when the user is not signed in. Two small follow‑ups to consider:
- To avoid calling
getState()twice and to improve readability, you could factor out the auth state once:const authState = ext.authProvider?.getState(); const userInfo = authState?.state?.userInfo; return { idpId: userInfo?.userId ?? "", isWSO2User: userInfo?.userEmail?.endsWith("@wso2.com") ? "true" : "false", };
idpIdis being populated fromuserInfo.userId. If downstream telemetry consumers expect a true IdP identifier (vs a user id), please double‑check that this mapping matches existing expectations; if this is already the established meaning, then the current change is fine.workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (4)
176-182: LGTM!The Git initialization check correctly handles cases where Git is not yet initialized, allowing the flow to continue with
isGitInitialized=false. The silent error handling is intentional and appropriate here.Optionally, consider logging the caught error for debugging purposes:
} catch (err) { - // ignore error + getLogger().debug("Git not initialized in selected directory", err); }
234-241: LGTM!The unique name generation logic correctly handles naming conflicts by appending numeric suffixes.
Consider adding a safety limit to prevent infinite loops in case of unexpected bugs:
let compInitialName = params?.name || dirName || selectedType; const existingNames = components.map((c) => c.metadata?.name?.toLowerCase?.()); const baseName = compInitialName; let counter = 1; -while (existingNames.includes(compInitialName.toLowerCase())) { +const MAX_ATTEMPTS = 1000; +while (existingNames.includes(compInitialName.toLowerCase()) && counter < MAX_ATTEMPTS) { compInitialName = `${baseName}-${counter}`; counter++; } +if (counter >= MAX_ATTEMPTS) { + throw new Error("Unable to generate unique component name"); +}
331-378: LGTM!The post-creation Git integration handles both Devant cloud editor and traditional flows with comprehensive error handling and appropriate logging.
Consider explicitly handling the case where
head.nameis undefined to provide clearer diagnostics:const head = await repo.getHEAD(); if (head.name) { const commit = await repo.getCommit(head.name); // ... update code server ... +} else { + getLogger().warn("HEAD name is undefined, skipping code server update"); }
401-409: Simplify complex URL construction.The URL construction spans multiple lines with nested ternary operators, making it difficult to read and maintain.
Consider extracting URL construction into a helper function or simplifying with variables:
+const consoleUrl = extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl; +const projectPath = extensionName === "Devant" ? project.id : project.handler; +const componentUrl = `${consoleUrl}/organizations/${org.handle}/projects/${projectPath}/components/${createdComponent.metadata.handler}/overview`; + window.showInformationMessage(successMessage, `Open in ${extensionName}`).then(async (resp) => { if (resp === `Open in ${extensionName}`) { - commands.executeCommand( - "vscode.open", - `${extensionName === "Devant" ? ext.config?.devantConsoleUrl : ext.config?.choreoConsoleUrl}/organizations/${org.handle}/projects/${extensionName === "Devant" ? project.id : project.handler}/components/${createdComponent.metadata.handler}/overview`, - ); + commands.executeCommand("vscode.open", componentUrl); } });workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (1)
104-113: Nit: use canonical “GitHub” casing in log messageSuper minor, but for log/search consistency you may want:
- getLogger().info("WSO2 Platform Github auth Callback hit"); + getLogger().info("WSO2 Platform GitHub auth Callback hit");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts(1 hunks)workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts(7 hunks)workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts(2 hunks)workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workspaces/wso2-platform/wso2-platform-extension/src/cmds/clone-project-cmd.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.{ts,tsx} : Define all constants (node types, sizing, spacing) in src/resources/constants.ts and import them where needed instead of hardcoding values
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts
📚 Learning: 2025-11-10T15:04:50.474Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:153-169
Timestamp: 2025-11-10T15:04:50.474Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate, independently deployable VSCode extensions in the wso2/vscode-extensions repository. Code duplication between these extensions is acceptable and often necessary to maintain their independence, rather than creating cross-extension dependencies.
Applied to files:
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts
🧬 Code graph analysis (3)
workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (2)
workspaces/wso2-platform/wso2-platform-core/src/types/common.types.ts (1)
UserInfo(62-70)workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts (1)
workspaces/wso2-platform/wso2-platform-extension/src/extensionVariables.ts (1)
ext(47-47)
workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (2)
workspaces/wso2-platform/wso2-platform-extension/src/stores/context-store.ts (2)
waitForContextStoreToLoad(398-414)contextStore(55-119)workspaces/wso2-platform/wso2-platform-extension/src/stores/data-cache-store.ts (1)
dataCacheStore(38-160)
🔇 Additional comments (7)
workspaces/wso2-platform/wso2-platform-extension/src/telemetry/utils.ts (1)
19-19: Importingextfor auth-backed telemetry is appropriateUsing
exthere to source auth state for telemetry aligns this utility with the extension’s single auth provider and looks correct, assumingextensionVariablesremains free of circular imports to this file.workspaces/wso2-platform/wso2-platform-extension/src/cmds/create-component-cmd.ts (4)
25-25: LGTM!The new imports (
ComponentKind,initGit,getLogger, andwaitForContextStoreToLoad) are all utilized appropriately throughout the file.Also applies to: 39-39, 41-42
62-62: LGTM!Awaiting the context store to load before accessing the selected project/organization prevents race conditions and ensures data consistency.
220-226: LGTM!The authentication validation before updating the context file is a good defensive check that prevents potential runtime errors and provides clear user feedback.
380-390: LGTM!The conditional UI handling appropriately distinguishes between Devant cloud editor and traditional flows, with the component ID stored in global state for cloud editor integration.
workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts (2)
19-30: ImportingUserInfofrom core aligns sign‑in with shared auth typesUsing the shared
UserInfointerface from@wso2/wso2-platform-corefor the sign-in result keeps this handler in sync with the core auth schema and improves type safety arounduserInfo.
39-44: ReusingisSamePath/openDirectoryutilities is a good consolidationPulling in
isSamePathfor workspace checks andopenDirectoryfor folder opening makes this module consistent with the shared utilities and avoids duplicating path/UX logic here.
| const orgId = contextStore?.getState().state?.selected?.org?.id?.toString(); | ||
| const callbackUrl = extName === "Devant" ? `${choreoEnvConfig.getDevantUrl()}/vscode-auth` : undefined; | ||
| const clientId = extName === "Devant" ? choreoEnvConfig.getDevantAsgardeoClientId() : undefined; | ||
| const userInfo = await ext.clients.rpcClient.signInWithAuthCode(authCode, region, orgId, callbackUrl, clientId); | ||
| let userInfo: UserInfo | undefined; | ||
| if (extName === "Devant") { | ||
| userInfo = await ext.clients.rpcClient.signInDevantWithAuthCode(authCode, region, orgId); | ||
| } else { | ||
| userInfo = await ext.clients.rpcClient.signInWithAuthCode(authCode, region, orgId); | ||
| } | ||
| if (userInfo) { | ||
| if (contextStore?.getState().state?.selected) { | ||
| const includesOrg = userInfo.organizations?.some((item) => item.handle === contextStore?.getState().state?.selected?.orgHandle); | ||
| if (!includesOrg) { | ||
| contextStore.getState().resetState(); | ||
| } | ||
| } | ||
| authStore.getState().loginSuccess(userInfo); | ||
| window.showInformationMessage(`Successfully signed into ${extName}`); | ||
| const region = await ext.clients.rpcClient.getCurrentRegion(); | ||
| await ext.authProvider?.getState().loginSuccess(userInfo, region); | ||
| window.showInformationMessage(`Successfully signed into ${extName}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TypeScript JavaScript optional chaining ?.getState().method() short-circuit behavior undefined
💡 Result:
Short answer: optional chaining only short-circuits at the spot where you put it. If you write obj?.getState().method(), the ?. only protects access to getState; the call to .method() will still run on whatever getState() returns — so if getState() returns null/undefined you'll get a TypeError. To protect the whole chain you must use ?. at each step that might be nullish, e.g.:
- obj?.getState()?.method?.() // safe: returns undefined if obj, getState(), or method is null/undefined.
- (obj?.getState).call(...) // different semantics; grouping breaks the single-chain short-circuit.
See MDN: optional chaining short-circuits at the operator and short-circuited expressions evaluate to undefined; grouping can stop the short-circuit and cause a TypeError. [1][2]
Sources:
[1] MDN Optional chaining — description and short-circuit behavior.
[2] MDN Optional chaining — examples (stacking and grouping notes).
Guard ext.authProvider properly before calling loginSuccess and avoid relying on partial optional chaining
The new Devant/non‑Devant sign‑in split and UserInfo typing look good, but this line is unsafe:
await ext.authProvider?.getState().loginSuccess(userInfo, region);Optional chaining only short-circuits at the ?. operator itself. If ext.authProvider is undefined, then ?.getState() returns undefined, and .loginSuccess(...) is still invoked on that undefined value, causing a runtime error. To guard the entire chain, you need ?. at each step or explicitly check the intermediate value.
Consider explicitly capturing the auth state before invoking loginSuccess, and failing gracefully if the auth provider is not initialized, e.g.:
- const region = await ext.clients.rpcClient.getCurrentRegion();
- await ext.authProvider?.getState().loginSuccess(userInfo, region);
- window.showInformationMessage(`Successfully signed into ${extName}`);
+ const region = await ext.clients.rpcClient.getCurrentRegion();
+ const authState = ext.authProvider?.getState();
+ if (!authState) {
+ getLogger().error("Auth provider is not initialized; cannot complete login");
+ window.showErrorMessage("Sign in failed: authentication provider is not initialized.");
+ return;
+ }
+ await authState.loginSuccess(userInfo, region);
+ window.showInformationMessage(`Successfully signed into ${extName}`);(Optionally, also rename the inner const region to currentRegion to avoid shadowing the earlier region derived from the URL, which improves readability.)
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts around
lines 69 to 86, the call uses optional chaining only on ext.authProvider (await
ext.authProvider?.getState().loginSuccess(...)) which can still attempt to call
loginSuccess on undefined; explicitly guard the whole chain by retrieving the
auth state into a const (e.g. const authState = ext.authProvider?.getState();)
and check authState before calling authState.loginSuccess(userInfo,
currentRegion); also avoid shadowing the earlier region by renaming the later
const region to currentRegion or similar, and if authState is missing handle the
failure path (log or show an error) instead of attempting the call.
| const projectCache = dataCacheStore.getState().getProjects(org?.handle); | ||
| const contextFilePath = updateContextFile(gitRoot, authStore.getState().state.userInfo!, project, org, projectCache); | ||
| const userInfo = ext.authProvider?.getState().state.userInfo; | ||
| if (!userInfo) { | ||
| window.showErrorMessage("User information is not available. Please sign in and try again."); | ||
| return; | ||
| } | ||
| const contextFilePath = updateContextFile(gitRoot, userInfo, project, org, projectCache); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JavaScript optional chaining ?. property access chain behavior undefined throws
💡 Result:
Short answer
- The optional chaining operator (?.) short-circuits when the left-hand reference is null or undefined and returns undefined instead of throwing. [1][2]
- It only treats nullish (null or undefined) as the short‑circuit condition — other errors or invalid uses still throw. [1][2]
Important details (concise)
- Property access: a?.b returns undefined if a is null or undefined; otherwise it returns a.b. [1][2]
- Bracket access: a?.[expr] behaves the same and won’t evaluate expr if a is nullish (side effects in expr won’t run). [1]
- Calls: f?.() returns undefined if f is nullish; but if f exists and is not a function, calling it throws TypeError. Also if the base object itself is not optional-chained, you can still get a TypeError (use a?.m?.()). [1]
- Parentheses break the chain: (a?.b).c will evaluate a?.b (possibly undefined) and then attempt .c — which can throw if the result is undefined. Optional chaining only short-circuits along a continuous chain. [1]
- Invalid/forbidden forms: you cannot use optional chaining as the left-hand side of an assignment (object?.prop = ...) or as a constructor target (new obj?.Ctor()). [1][2]
- Unsafe contexts: using the result of ?. where undefined is not allowed (e.g., 1 in obj?.prop, for-of, instanceof, destructuring) can cause runtime TypeError; linters flag these patterns. [3]
Spec references
- ECMAScript spec runtime semantics: optional chains return undefined when the base value is nullish; otherwise evaluation continues. [2]
- MDN docs and examples for behavior, short‑circuiting rules and caveats. [1]
Sources
[1] MDN — Optional chaining (?.).
[2] ECMAScript (ECMA-262) — Optional Chains (Runtime Semantics).
[3] ESLint rule no-unsafe-optional-chaining (explains unsafe contexts).
Unsafe optional chaining in userInfo access—fix the nested property chain
This code has a real safety issue:
const userInfo = ext.authProvider?.getState().state.userInfo;Optional chaining (?.) only short-circuits along a continuous chain. Here, .getState() is a method call that breaks the chain. If ext.authProvider is undefined, then ext.authProvider?.getState() returns undefined, and .state is immediately accessed on that undefined value—causing a runtime error before your !userInfo guard can run.
Fix by using a continuous optional chain:
- const projectCache = dataCacheStore.getState().getProjects(org?.handle);
- const userInfo = ext.authProvider?.getState().state.userInfo;
- if (!userInfo) {
+ const projectCache = dataCacheStore.getState().getProjects(org?.handle);
+ const authState = ext.authProvider?.getState();
+ const userInfo = authState?.state?.userInfo;
+ if (!userInfo) {
window.showErrorMessage("User information is not available. Please sign in and try again.");
return;
}
const contextFilePath = updateContextFile(gitRoot, userInfo, project, org, projectCache);This allows switchContextAndOpenDir to fail gracefully instead of crashing if the auth provider is not initialized.
🤖 Prompt for AI Agents
In workspaces/wso2-platform/wso2-platform-extension/src/uri-handlers.ts around
lines 266 to 273, the code uses unsafe optional chaining when calling a method
(ext.authProvider?.getState().state.userInfo) which can throw if
ext.authProvider is undefined because the method call breaks the chain; fix it
by guarding the call or using a continuous safe access pattern such as checking
ext.authProvider before calling getState (e.g. if (!ext.authProvider) handle
missing auth) or using ext.authProvider?.getState?.().state?.userInfo so every
step is optional, then keep the existing !userInfo early-return error path.
8 participants
Purpose
Goals
Approach
UI Component Development
npm run storybookfrom the root directory to view current components.Manage Icons
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes & Updates
✏️ Tip: You can customize this high-level summary in your review settings.