Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion workspaces/wso2-platform/wso2-platform-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"displayName": "WSO2 Platform",
"description": "Manage WSO2 Choreo and Devant projects in VS Code.",
"license": "Apache-2.0",
"version": "1.0.18",
"version": "1.0.19",
"cliVersion": "v1.2.212509091800",
"publisher": "wso2",
"bugs": {
Expand Down Expand Up @@ -176,6 +176,23 @@
"default": "",
"description": "The path to Choreo RPC server",
"scope": "window"
},
"WSO2.WSO2-Platform.Advanced.RpcArchitecture": {
"type": "string",
"enum": [
"",
"amd64",
"arm64"
],
"default": "",
"description": "The architecture of Choreo RPC server. Change only if you are running within a container with a different architecture than your host machine.",
"scope": "window"
},
"WSO2.WSO2-Platform.Advanced.SkipKeyring": {
"type": "boolean",
"default": false,
"description": "Enable only your system does not support keyring.",
"scope": "window"
Comment on lines +180 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify the SkipKeyring setting description.
Current text is grammatically unclear and can confuse users.

✏️ Proposed text fix
-          "description": "Enable only your system does not support keyring.",
+          "description": "Enable only if your system does not support keyring.",
📝 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.

Suggested change
"WSO2.WSO2-Platform.Advanced.RpcArchitecture": {
"type": "string",
"enum": [
"",
"amd64",
"arm64"
],
"default": "",
"description": "The architecture of Choreo RPC server. Change only if you are running within a container with a different architecture than your host machine.",
"scope": "window"
},
"WSO2.WSO2-Platform.Advanced.SkipKeyring": {
"type": "boolean",
"default": false,
"description": "Enable only your system does not support keyring.",
"scope": "window"
"WSO2.WSO2-Platform.Advanced.RpcArchitecture": {
"type": "string",
"enum": [
"",
"amd64",
"arm64"
],
"default": "",
"description": "The architecture of Choreo RPC server. Change only if you are running within a container with a different architecture than your host machine.",
"scope": "window"
},
"WSO2.WSO2-Platform.Advanced.SkipKeyring": {
"type": "boolean",
"default": false,
"description": "Enable only if your system does not support keyring.",
"scope": "window"
🤖 Prompt for AI Agents
In `@workspaces/wso2-platform/wso2-platform-extension/package.json` around lines
180 - 195, The "WSO2.WSO2-Platform.Advanced.SkipKeyring" setting description is
grammatically unclear; update the "description" string for the configuration
entry named WSO2.WSO2-Platform.Advanced.SkipKeyring so it clearly explains its
purpose (e.g., indicate it should be enabled when the user's system does not
support a keyring) and maintains same scope/type/default; locate the JSON object
for WSO2.WSO2-Platform.Advanced.SkipKeyring and replace the description value
with a concise, correct sentence.

}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import { exec } from "child_process";
import * as fs from "fs";
import { downloadCLI, getChoreoExecPath, getCliVersion } from "./cli-install";
import { installCLI, getChoreoExecPath, getCliVersion } from "./cli-install";
import { RPCClient } from "./client";
import { getLogger } from "../logger/logger";

Expand Down Expand Up @@ -58,7 +58,7 @@ export async function initRPCServer() {
const installed = await isChoreoCliInstalled();
if (!installed) {
getLogger().trace(`WSO2 Platform RPC version ${getCliVersion()} not installed`);
await downloadCLI();
await installCLI();
}

await RPCClient.getInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ export const getChoreoEnv = (): string => {
};

const getChoreoBinPath = () => {
return path.join(ext.context.globalStorageUri.fsPath, "choreo-cli-rpc", getCliVersion(), "bin");
const OS = os.platform();
const ARCH = getArchitecture();
return path.join(ext.context.extensionPath, "resources", "choreo-cli", getCliVersion(), OS, ARCH, "bin");
};

export const downloadCLI = async () => {
export const installCLI = async () => {
const OS = os.platform();
const ARCH = getArchitecture();
const CHOREO_BIN_DIR = getChoreoBinPath();
Expand Down Expand Up @@ -167,6 +169,10 @@ export const downloadCLI = async () => {
};

function getArchitecture() {
const arch = workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture");
if (arch) {
return arch;
}
Comment on lines 171 to +175
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Validate architecture overrides.
If a user manually sets an invalid value, the install will fail later with a less clear error. Consider validating allowed values early.

🛡️ Suggested validation
 function getArchitecture() {
 	const arch = workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture");
 	if (arch) {
+		const allowed = new Set(["amd64", "arm64", "arm"]);
+		if (!allowed.has(arch)) {
+			throw new Error(`Unsupported architecture override: ${arch}. Expected one of: ${[...allowed].join(", ")}`);
+		}
 		return arch;
 	}
 	const ARCH = os.arch();
📝 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.

Suggested change
function getArchitecture() {
const arch = workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture");
if (arch) {
return arch;
}
function getArchitecture() {
const arch = workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture");
if (arch) {
const allowed = new Set(["amd64", "arm64", "arm"]);
if (!allowed.has(arch)) {
throw new Error(`Unsupported architecture override: ${arch}. Expected one of: ${[...allowed].join(", ")}`);
}
return arch;
}
🤖 Prompt for AI Agents
In
`@workspaces/wso2-platform/wso2-platform-extension/src/choreo-rpc/cli-install.ts`
around lines 171 - 175, The getArchitecture function should validate the
user-configured value from
workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture")
against a small allowlist (e.g. valid values such as "x86_64" and "arm64" or
your project's canonical names); if the retrieved arch is present but not in the
allowlist, surface a clear early error (throw or show a user-facing error
message) or fall back to a documented default before proceeding so the installer
fails with a clear message; update getArchitecture to perform this check and use
the allowlist when deciding whether to return the configured value.

const ARCH = os.arch();
switch (ARCH) {
case "x64":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { ext } from "../extensionVariables";
import { getLogger } from "../logger/logger";
import { parseJwt } from "../utils";
import { getChoreoExecPath } from "./cli-install";
import { workspace } from "vscode";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Honor explicit false for SkipKeyring.
A user setting of false is currently treated the same as “unset,” allowing process.env.SKIP_KEYRING to override it. Consider distinguishing undefined vs false.

✅ Proposed fix
-		const skipKeyringConfig = workspace.getConfiguration().get<boolean>("WSO2.WSO2-Platform.Advanced.SkipKeyring");
+		const skipKeyringConfig = workspace.getConfiguration().get<boolean>("WSO2.WSO2-Platform.Advanced.SkipKeyring");
+		const skipKeyring =
+			skipKeyringConfig !== undefined
+				? skipKeyringConfig
+				: ext.isDevantCloudEditor
+					? true
+					: undefined;
 		this._serverProcess = spawn(executablePath, ["start-rpc-server"], {
 			env: {
 				...process.env,
-				SKIP_KEYRING: (skipKeyringConfig || ext.isDevantCloudEditor) ? "true" : (process.env.SKIP_KEYRING || ""),
+				SKIP_KEYRING: skipKeyring === undefined ? (process.env.SKIP_KEYRING || "") : (skipKeyring ? "true" : ""),
 				CHOREO_ENV: ext.choreoEnv,
 				CHOREO_REGION: region,
 			},
 		});

Also applies to: 47-52

🤖 Prompt for AI Agents
In
`@workspaces/wso2-platform/wso2-platform-extension/src/choreo-rpc/connection.ts`
at line 26, The code currently treats a user setting of skipKeyring the same as
unset, letting process.env.SKIP_KEYRING override an explicit false; update the
logic where you read the configuration (the
workspace.getConfiguration(...).get('skipKeyring') usage) so that you
distinguish undefined from false — only use process.env.SKIP_KEYRING when the
config value is strictly undefined, and preserve a boolean false when the user
explicitly sets it; apply this change to both places that reference
process.env.SKIP_KEYRING / the skipKeyring config in connection.ts.


export class StdioConnection {
private _connection: MessageConnection;
Expand All @@ -43,10 +44,11 @@ export class StdioConnection {
if (!region && process.env.CLOUD_STS_TOKEN && parseJwt(process.env.CLOUD_STS_TOKEN)?.iss?.includes(".eu.")) {
region = "EU";
}
const skipKeyringConfig = workspace.getConfiguration().get<boolean>("WSO2.WSO2-Platform.Advanced.SkipKeyring");
this._serverProcess = spawn(executablePath, ["start-rpc-server"], {
env: {
...process.env,
SKIP_KEYRING: ext.isDevantCloudEditor ? "true" : "",
SKIP_KEYRING: (skipKeyringConfig || ext.isDevantCloudEditor) ? "true" : (process.env.SKIP_KEYRING || ""),
CHOREO_ENV: ext.choreoEnv,
CHOREO_REGION: region,
},
Expand Down
Loading