Skip to content

Conversation

@lambrianmsft
Copy link
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

This moves the deploy command from using vscode wizards to a custom webview which allows more control and validation from the extension. We also display all relevant information so that users don't have to remember what they had input previously.

Impact of Change

  • Users:
    Users now see all content before deploying, and allow naming of resources when a new logic app is created.
  • Developers:
  • System:

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

Screenshots/Videos

Copilot AI review requested due to automatic review settings January 8, 2026 20:43
@github-actions
Copy link

github-actions bot commented Jan 8, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: feat(vscode): add "Deploy to Azure" webview and deploy flow (create Logic App without wizard)
  • Issue: Title is generally good — uses conventional commit prefix and describes the feature. It's a bit long and includes parentheses; consider making it slightly more concise and imperative.
  • Recommendation: Shorten and keep the important scope and outcome. Example: feat(vscode): add "Deploy to Azure" webview and create Logic App without wizard

Commit Type

  • Properly selected (feature).
  • Note: Only one commit type is selected which is correct.

Risk Level

  • Assessment: The PR is labeled risk:medium and the PR body selects Medium.
  • Issue: Based on the code diff this change adds a large new feature surface (new webview UI, React app pages, new redux slice, new API surface for listing subscriptions/locations/resources, new extension commands, new azure SDK packages, and a wizard-free create flow that can create resource groups, app service plans, storage accounts, and App Insights). This introduces cloud resource creation paths, new dependencies, and non-trivial runtime behavior in both extension and web UI.
  • Advised Risk Level: High — please update the label to risk:high and update the Risk Level section in the PR body accordingly.

What & Why

  • Current: "This moves the deploy command from using vscode wizards to a custom webview which allows more control and validation from the extension..."
  • Issue: This section is present and succinct. Good.
  • Recommendation: Optionally add one line listing the main capabilities (ex: "creates Logic App without wizard, supports creating RG/Plan/Storage/App Insights, validates names in the webview").

Impact of Change

  • Issue: This section is mostly empty for Developers and System. Given the size of changes, you must fill this out.
  • Recommendation: Fill the Impact section using concrete facts from the diff. Suggested content to paste/edit:
    • Users: New "Deploy to Azure" webview in VS Code. Users can create Logic App (Standard) without the wizard and deploy from a UI that validates names/locations and resource creation choices. They may see new prompts or automatic resource creations.
    • Developers: New extension command handlers (deploy, cancel_deploy, getFilteredLocations), new API surface (createLogicAppWithoutWizard, createChildWithoutPrompts), new redux state (deploySlice), and many new files in the React webview. New public dependencies added: @azure/arm-resources, @azure/arm-subscriptions. Developers must review packaging, build, and test hooks for the extension and webview.
    • System: Adds calls to Azure Resource Graph / ARM (creating RGs, plans, storage accounts, App Insights). New dependencies may require Node environment updates for CI (pnpm lock shows node >=20 for new packages). Consider deployment/bundling size, CI job changes, and telemetry/permission scopes for token usage.

Test Plan

  • Assessment: The PR body marks Manual testing completed and no unit/E2E tests. The diff shows many code additions and no tests.
  • Issue: When only manual testing is selected an explicit, detailed manual test plan / rationale is required. Additionally, for this scope you should add automated tests (unit and/or E2E). Without tests or a clear manual plan, this should fail.
  • Recommendation: Either add tests or expand the Test Plan with detailed manual verification steps. Minimum suggestions:
    • Unit tests: createLogicAppWithoutWizard behavior (validations, error handling), createChildWithoutPrompts validation logic, ApiService.getLocations/getLogicApps/getResourceGroups, storage name validation logic.
    • E2E / integration: a smoke test that runs the webview deploy flow against a test subscription (or a mocked backend) to cover creation of RG/plan/storage/AI and deployment flow.
    • Manual test steps (if you must rely on manual): provide explicit steps, environment pre-requisites, example subscription/resource names to use, logging/telemetry checks, and what to observe on success/failure. Example manual steps:
      1. Open extension with test subscription A and ensure you have a blank workspace.
      2. Open Deploy -> Select subscription A -> Create new Logic App -> enter a unique name X -> choose location Y and create resources -> click Deploy.
      3. Confirm RG, plan, storage and app insights are created in portal and the extension returns success.
      4. Repeat deploying to an existing Logic App and confirm correct behavior.
      5. Verify name validation errors (invalid storage name, duplicate app name) are shown in the webview.
  • If you keep manual testing only, add a short explanation why unit/E2E are not feasible and when/where tests will be added.

⚠️ Contributors

  • Assessment: Empty.
  • Recommendation: Add contributors (PMs, designers, reviewers) or include a note to credit others. This is optional but recommended.

⚠️ Screenshots/Videos

  • Assessment: Not provided.
  • Recommendation: This PR adds a significant UI (new webview / React pages). Please add at least a couple of screenshots or a short GIF/video showing the webview, the deploy flow, error states (name validation), and success state. This helps reviewers verify UX changes quickly.

Summary Table

Section Status Recommendation
Title Shorten slightly and keep key action & scope
Commit Type Good (feature)
Risk Level Change to risk:high in both label & PR body
What & Why Add a one-line capabilities summary (optional)
Impact of Change Fill Developer and System impacts (see suggested text)
Test Plan Add unit/E2E tests or a detailed manual test plan
Contributors ⚠️ Add credits/participants (recommended)
Screenshots/Videos ⚠️ Add screenshots/GIFs of the webview and flows

Final Notes & Action Items

  1. Change the risk label and the Risk Level section in the PR body to High (risk:high) — this PR introduces cloud resource creation, new SDK deps, and extension/server interactions that elevate risk.
  2. Update the Impact of Change section with the bullet points suggested above (Users / Developers / System). Be explicit about new permissions, new dependencies, and potential CI/node version changes.
  3. Expand the Test Plan: add unit tests and/or E2E tests. If you must rely on manual testing, add a detailed manual test matrix and explain why automated tests are missing and when they will be added.
  4. Add screenshots or a short video of the Deploy webview showing flows and validations.
  5. Address the existing needs-pr-update label by making the above updates and removing the label once ready.

Please update the PR title/body to address the items above and re-submit for review. If you want, paste the updated PR body here and I will re-check it against the template.


Note: I set the advised risk level to High because the diff creates/changes code paths that create Azure resources, adds new Azure SDK packages, modifies resource creation behavior (creates RG/Plan/Storage/AppInsights), and adds a large webview surface — these changes have cross-cutting impact and require careful testing and permission review.


Last updated: Thu, 08 Jan 2026 20:47:59 GMT

@lambrianmsft lambrianmsft changed the title feat(vscode) : Deploy Webview feat(vscode): add "Deploy to Azure" webview and deploy flow (create Logic App without wizard) Jan 8, 2026
@lambrianmsft lambrianmsft added the risk:medium Medium risk change with potential impact label Jan 8, 2026
state.isCreatingNewStorageAccount = action.payload === '__CREATE_NEW__';
},
setNewStorageAccountName(state, action: PayloadAction<string>) {
state.newStorageAccountName = action.payload;

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
in a security context.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request implements a significant UX improvement by migrating the Logic App deployment command from VS Code's native wizard interface to a custom webview. This change provides better control over the deployment process, enhanced validation, and persistent display of user inputs.

Key Changes

  • New deployment webview: Custom React-based UI for deployment with comprehensive validation
  • Enhanced state management: Added Redux slice for managing deployment state across subscription, resource group, Logic App, and dependent resource selections
  • Wizard-free Logic App creation: New code path to create Logic Apps programmatically without user prompts
  • Azure resource fetching: New API service methods to fetch subscriptions, Logic Apps, resource groups, locations, app service plans, and storage accounts

Reviewed changes

Copilot reviewed 20 out of 22 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pnpm-lock.yaml Added Azure ARM dependencies (@azure/arm-resources, @azure/arm-subscriptions) for resource management
libs/vscode-extension/src/lib/models/project.ts Added 'deploy' project name constant
libs/vscode-extension/src/lib/models/extensioncommand.ts Added deploy-related extension commands (deploy, cancel_deploy, getFilteredLocations)
apps/vs-code-react/src/stateWrapper.tsx Added navigation routing for deploy project
apps/vs-code-react/src/state/store.ts Registered deploySlice reducer in Redux store
apps/vs-code-react/src/state/deploySlice.ts Complete Redux slice for deployment state management with 21 state properties
apps/vs-code-react/src/run-service/types.ts Added logicApps resource type and deploy route
apps/vs-code-react/src/run-service/export/index.ts Added methods for fetching Logic Apps, locations, app service plans, storage accounts, and name availability checking
apps/vs-code-react/src/router/index.tsx Added DeployApp route
apps/vs-code-react/src/intl/messages.ts Added 271 new localization strings for deployment UI
apps/vs-code-react/src/intl/index.ts Exported deployMessages
apps/vs-code-react/src/app/deploy/deployStyles.ts Styling definitions for deployment UI
apps/vs-code-react/src/app/deploy/deploy.tsx Main deployment React component (853 lines) with form validation and Azure resource management
apps/vs-code-designer/src/extensionVariables.ts Added deploy webview key
apps/vs-code-designer/src/app/tree/subscriptionTree/subscriptionTreeItem.ts Added createChildWithoutPrompts method for programmatic Logic App creation (254 lines)
apps/vs-code-designer/src/app/commands/registerCommands.ts Changed deploy command from wizard to webview handler
apps/vs-code-designer/src/app/commands/deploy/deployWebview.ts New webview handler for deployment with message processing
apps/vs-code-designer/src/app/commands/deploy/deploy.ts Exported deploy function for reuse
apps/vs-code-designer/src/app/commands/createLogicApp/createLogicAppSteps/logicAppCreateStep.ts Added httpsOnly and vnetRouteAllEnabled properties to site config
apps/vs-code-designer/src/app/commands/createLogicApp/createLogicApp.ts Added createLogicAppWithoutWizard function for programmatic creation
apps/vs-code-designer/package.json Added Azure ARM dependencies
Localize/lang/strings.json Added localization keys and descriptions for deployment UI
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

reserved: context.newSiteOS === WebsiteOS.linux,
identity: context.customLocation ? undefined : { type: 'SystemAssigned' },
httpsOnly: true,
vnetRouteAllEnabled: false, // Enable VNet route all for enhanced security
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Inconsistent handling of vnetRouteAllEnabled property. The comment says "Enable VNet route all for enhanced security" but the value is set to false, which disables the feature. Either the comment is incorrect or the value should be true.

Suggested change
vnetRouteAllEnabled: false, // Enable VNet route all for enhanced security
vnetRouteAllEnabled: true, // Enable VNet route all for enhanced security

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +56
createHandler: async (actionContext: IActionContext, data: any) => {
if (data.createNew) {
// User wants to create a new Logic App without wizard prompts
const createContext: any = {
...actionContext,
newSiteName: data.newLogicAppName,
location: data.location,
newResourceGroupName: data.isCreatingNewResourceGroup ? data.resourceGroup : undefined,
resourceGroup: data.isCreatingNewResourceGroup ? undefined : { name: data.resourceGroup },
newPlanName: data.isCreatingNewAppServicePlan ? data.appServicePlan : undefined,
plan: data.isCreatingNewAppServicePlan ? undefined : { id: data.appServicePlan },
appServicePlanSku: data.appServicePlanSku || 'WS1',
newStorageAccountName: data.isCreatingNewStorageAccount ? data.storageAccount : undefined,
storageAccount: data.isCreatingNewStorageAccount ? undefined : { id: data.storageAccount },
createAppInsights: data.createAppInsights,
newAppInsightsName: data.appInsightsName,
};

// Create the Logic App using the wizard-free method
const node: SlotTreeItem = await createLogicAppWithoutWizard(
createContext,
data.subscriptionId,
true // Skip notification since we're deploying next
);

// Now deploy to the newly created Logic App
await deploy(actionContext, node.fullId, node.fullId);
} else {
// Deploy to existing Logic App
await deploy(actionContext, data.logicAppId, data.logicAppId);
}
},
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Missing input validation in deployViaWebview handler. The data object should be validated before being used to create resources. Required fields like subscriptionId, newLogicAppName, location, resourceGroup should be checked for existence and validity to prevent runtime errors.

Copilot uses AI. Check for mistakes.
Comment on lines +318 to +341
// Check storage account name availability (debounced)
useEffect(() => {
if (!newStorageAccountName || !isCreatingNew || !isCreatingNewStorageAccount || !selectedSubscription || storageAccountNameError) {
setStorageNameUnavailable(false);
setStorageNameMessage('');
return;
}

setIsCheckingStorageName(true);
const timeoutId = setTimeout(async () => {
try {
const result = await apiService.checkStorageAccountNameAvailability(selectedSubscription, newStorageAccountName);
setStorageNameUnavailable(!result.available);
setStorageNameMessage(result.message || '');
} catch (error) {
console.error('Error checking storage account name availability:', error);
} finally {
setIsCheckingStorageName(false);
}
}, 500); // 500ms debounce

return () => clearTimeout(timeoutId);
}, [newStorageAccountName, isCreatingNew, isCreatingNewStorageAccount, selectedSubscription, storageAccountNameError, apiService]);

Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Duplicate useEffect for storage account name availability check. Lines 294-316 and 318-340 contain identical logic. This will cause the availability check to run twice for every state change, doubling the API calls and potentially causing race conditions.

Suggested change
// Check storage account name availability (debounced)
useEffect(() => {
if (!newStorageAccountName || !isCreatingNew || !isCreatingNewStorageAccount || !selectedSubscription || storageAccountNameError) {
setStorageNameUnavailable(false);
setStorageNameMessage('');
return;
}
setIsCheckingStorageName(true);
const timeoutId = setTimeout(async () => {
try {
const result = await apiService.checkStorageAccountNameAvailability(selectedSubscription, newStorageAccountName);
setStorageNameUnavailable(!result.available);
setStorageNameMessage(result.message || '');
} catch (error) {
console.error('Error checking storage account name availability:', error);
} finally {
setIsCheckingStorageName(false);
}
}, 500); // 500ms debounce
return () => clearTimeout(timeoutId);
}, [newStorageAccountName, isCreatingNew, isCreatingNewStorageAccount, selectedSubscription, storageAccountNameError, apiService]);

Copilot uses AI. Check for mistakes.
Comment on lines +198 to +203
// Generate a simple GUID-like suffix
const generateGuid = () => {
return Math.random().toString(36).substring(2, 10);
};

const guid = generateGuid();
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The random GUID generation using Math.random() is not cryptographically secure and may produce collisions. Consider using a proper UUID library or crypto.randomUUID() for generating unique identifiers for Azure resources.

Suggested change
// Generate a simple GUID-like suffix
const generateGuid = () => {
return Math.random().toString(36).substring(2, 10);
};
const guid = generateGuid();
// Generate a simple GUID-like suffix using cryptographically secure random values
const generateSecureSuffix = () => {
const bytes = new Uint8Array(8);
crypto.getRandomValues(bytes);
return Array.from(bytes, (b) => b.toString(36).padStart(2, '0')).join('').substring(0, 8);
};
const guid = generateSecureSuffix();

Copilot uses AI. Check for mistakes.
* @param skipNotification - If true, skips the completion notification
* @returns The created Logic App tree item
*/
export async function createLogicAppWithoutWizard(
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't SubscriptionTreeItem.createChild already skip prompts if the values are provided in context? And if not, would it make sense to modify that method to skip prompts when not needed instead of adding this one?

};

// Create the Logic App using the wizard-free method
const node: SlotTreeItem = await createLogicAppWithoutWizard(
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't createLogicApp called from deploy if the logic app doesn't exist? Can this be refactored so that the logic app is either always created prior to deploy or always created within deploy function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-pr-update risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants