Add configs to manage CPU architecture for Choreo CLI and skip keyring for auth management#1376
Closed
Add configs to manage CPU architecture for Choreo CLI and skip keyring for auth management#1376
Conversation
Contributor
WalkthroughThis pull request bumps the extension version and introduces two new configuration options for controlling RPC architecture selection and keyring behavior. It refactors the CLI installation logic to use the extension's resources directory instead of global storage, renames the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@workspaces/wso2-platform/wso2-platform-extension/package.json`:
- Around line 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.
In
`@workspaces/wso2-platform/wso2-platform-extension/src/choreo-rpc/cli-install.ts`:
- Around line 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.
In
`@workspaces/wso2-platform/wso2-platform-extension/src/choreo-rpc/connection.ts`:
- 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.
Comment on lines
+180
to
+195
| "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" |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
171
to
+175
| function getArchitecture() { | ||
| const arch = workspace.getConfiguration().get<string>("WSO2.WSO2-Platform.Advanced.RpcArchitecture"); | ||
| if (arch) { | ||
| return arch; | ||
| } |
Contributor
There was a problem hiding this comment.
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.
| import { getLogger } from "../logger/logger"; | ||
| import { parseJwt } from "../utils"; | ||
| import { getChoreoExecPath } from "./cli-install"; | ||
| import { workspace } from "vscode"; |
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$subject
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
Chores