Skip to content

fix(vscode): prevent multiple Create clicks in convert-to-workspace webview#9148

Merged
lambrianmsft merged 6 commits into
Azure:mainfrom
lambrianmsft:lambrian/convert_to_workspace
May 12, 2026
Merged

fix(vscode): prevent multiple Create clicks in convert-to-workspace webview#9148
lambrianmsft merged 6 commits into
Azure:mainfrom
lambrianmsft:lambrian/convert_to_workspace

Conversation

@lambrianmsft
Copy link
Copy Markdown
Contributor

@lambrianmsft lambrianmsft commented May 7, 2026

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

When a user opened a logic app without a workspace and chose to convert it, clicking Create on the review page required multiple clicks before the webview actually proceeded. The button had no pending state so repeated clicks could queue duplicate create messages.

Fix:

  • Dispatch setLoading(true) on the first Create click so the button is immediately disabled (via disabled={isLoading}) and shows a spinner
  • Guard handleCreate with an early return when isLoading is already true
  • Replace hardcoded command strings with ExtensionCommand constants
  • Add retry-safe isCreateInProgress handling in workspaceWebviewCommandHandler so duplicate postMessages that arrive before the webview is disposed are silently dropped on the extension-host side, while failed create attempts reset the guard for retry
  • Stabilize Phase 4.8b conversion-create E2E setup with a fresh legacy project fixture, robust button locators, notification/quick-input handling, and explicit review-step validation

Tests:

  • React unit tests assert the convertToWorkspace flow sends ExtensionCommand.createWorkspaceStructure, sets isLoading=true on first click, and suppresses duplicate clicks while loading
  • Extension-host unit tests cover initialization, duplicate create suppression, retry after create failure, dialog/path message handling, and webview cache cleanup
  • Existing VS Code designer utility tests were expanded to cover bundle path working-directory propagation, Functions Core Tools command fallback behavior, and local designer/monitoring metadata paths
  • ExTester E2E test in workspaceConversionCreate.test.ts now:
    • Asserts the Create button is enabled before clicking
    • Clicks it exactly once using Selenium Actions API
    • Asserts the button enters pending/disabled state or workspace is created immediately (waitForSingleCreateClickToStart)
    • Fails if the .code-workspace file is not created
  • Added E2E_MODE=conversioncreateonly runner mode in run-e2e.js for targeted local and CI execution of Phase 4.8b with a fresh legacy-project fixture

Impact of Change

  • Users: Convert-to-workspace creation gives immediate pending feedback, prevents duplicate Create submissions, and still allows retry if creation fails.
  • Developers: Adds focused host/webview unit coverage and a deterministic Phase 4.8b runner path for iterating on workspace conversion behavior.
  • System: No dependency changes. The extension host now guards duplicate create messages and resets that guard only when creation does not complete successfully.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: Windows local VS Code extension unit tests and targeted ExTester Phase 4.8b

Validated test files/commands:

  • apps/vs-code-react/src/app/createWorkspace/test/createWorkspace.test.tsx (pnpm --filter vscode-react test:extension-unit -- createWorkspace)
  • apps/vs-code-designer/src/app/commands/shared/test/workspaceWebviewCommandHandler.test.ts
  • apps/vs-code-designer/src/app/commands/workflows/openDesigner/test/openDesignerForLocalProject.test.ts
  • apps/vs-code-designer/src/app/commands/workflows/openMonitoringView/test/openMonitoringViewForLocal.test.ts
  • apps/vs-code-designer/src/app/utils/test/bundleFeed.test.ts
  • apps/vs-code-designer/src/app/utils/funcCoreTools/test/funcVersion.test.ts (pnpm --filter vscode-designer test:extension-unit -- workspaceWebviewCommandHandler bundleFeed funcVersion openDesignerForLocalProject openMonitoringViewForLocal)
  • apps/vs-code-designer/src/test/ui/workspaceConversionCreate.test.ts (E2E_MODE=conversioncreateonly node src/test/ui/run-e2e.js)
  • apps/vs-code-designer/src/test/ui/run-e2e.js (npx tsup --config tsup.e2e.test.config.ts)

Contributors

Screenshots/Videos

When a user opened a logic app without a workspace and chose to convert
it, clicking Create on the review page required multiple clicks before
the webview actually proceeded. The button had no pending state so
repeated clicks could queue duplicate create messages.

Fix:
- Dispatch setLoading(true) on the first Create click so the button is
  immediately disabled (via disabled={isLoading}) and shows a spinner
- Guard handleCreate with an early return when isLoading is already true
- Replace hardcoded command strings with ExtensionCommand constants
- Add an isCreateInProgress flag in workspaceWebviewCommandHandler so
  duplicate postMessages that arrive before the webview is disposed are
  silently dropped on the extension-host side

Tests:
- New Vitest unit test asserts the convertToWorkspace flow sends
  createWorkspaceStructure command and sets isLoading=true on first click
- ExTester E2E test in workspaceConversionCreate.test.ts now:
  - Asserts the Create button is enabled before clicking
  - Clicks it exactly once using Selenium Actions API
  - Asserts the button enters pending/disabled state or workspace is
    created immediately (waitForSingleCreateClickToStart)
  - Fails if the .code-workspace file is not created (was previously
    only logging a diagnostic message)
- Added E2E_MODE=conversioncreateonly runner mode in run-e2e.js for
  targeted local and CI execution of Phase 4.8b

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 7, 2026 18:05
@lambrianmsft lambrianmsft changed the title fix: prevent multiple Create clicks in convert-to-workspace webview fix(vscode): prevent multiple Create clicks in convert-to-workspace webview May 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 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: fix(vscode): prevent multiple Create clicks in convert-to-workspace webview
  • Issue: None — the title is concise, follows conventional commit style, and clearly indicates the area and intent of the change.
  • Recommendation: Keep as-is. If you want to be slightly more explicit, you could add a short phrase about the user-facing symptom (e.g., "... (prevents duplicate submissions)") but that's optional.

Commit Type

  • Properly selected (fix).
  • Note: Only one type is selected, which is correct.

Risk Level

  • The PR body marks Medium and the PR has the label risk:medium — these match.
  • Assessment: The code changes are cross-cutting (webview/extension-host guards, e2e adjustments, func core tools handling, bundle feed changes and tests) and the Medium risk level is appropriate.

What & Why

  • Current: The body contains a clear "What & Why" explaining the duplicated-create-click symptom and the fixes applied (UI loading state, guard, extension-host suppression, stable E2E setup).
  • Issue: None.
  • Recommendation: None.

Impact of Change

  • Impact section present and clear.
  • Recommendation: Good — includes Users, Developers, and System notes.

Test Plan

  • Assessment: You claimed unit and E2E tests were added/updated. The code diff confirms new/updated unit tests and E2E test changes (createWorkspace unit tests, workspaceWebviewCommandHandler tests, openDesigner/openMonitoring tests, many e2e runner and test files). This matches the Test Plan selections.
  • Recommendation: Good. Optionally include in the body the exact commands to run the new tests locally (e.g., pnpm filter/test commands or the E2E runner invocation) to speed up reviewer verification.

⚠️ Contributors

  • The Contributors section is blank. This is allowed, but it's helpful to credit reviewers/PMs/designers who provided input.
  • Recommendation: Add any reviewers, PMs, designers, or test contributors if applicable.

⚠️ Screenshots/Videos

  • No screenshots/videos provided. This is acceptable for a behavioral/UI change that is well-covered by automated tests and described in the PR body.
  • Recommendation: Optional — if you want to make the reviewer experience faster, include a short GIF or screenshot of the create button entering pending state.

Summary Table

Section Status Recommendation
Title Keep as-is or optionally add (prevents duplicate submissions)
Commit Type None
Risk Level None
What & Why None
Impact of Change None
Test Plan Consider adding explicit run commands in the body
Contributors ⚠️ Add credit for contributors if any
Screenshots/Videos ⚠️ Optional: add a short screenshot/GIF of pending state

Final message
This PR passes the PR title/body checklist. The submitter's risk assessment (Medium) matches the code changes and labels; the test plan claims are substantiated by the diff (unit + E2E tests added/updated). The only suggested improvements are minor:

  • Add contributor credits (optional but recommended).
  • Optionally include the commands to run the new tests locally and/or a short screenshot/GIF showing the pending state for the Create button to help reviewers.

Please update the Contributors field if anyone else contributed, and consider adding the test commands to the Test Plan section. Thanks for the thorough description and the added tests — this looks ready for review/merge.


Last updated: Mon, 11 May 2026 23:57:39 GMT

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 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: fix(vscode): prevent multiple Create clicks in convert-to-workspace webview
  • Issue: Title is clear, follows conventional commit style, and describes the change scope and intent.
  • Recommendation: No change required. Good job using a descriptive conventional-commit-style title.

Commit Type

  • Properly selected (fix).
  • Note: Only one commit type is selected which is correct. The chosen type aligns with the changes (bug fix for double-click/duplicate actions).

Risk Level

  • The PR body selects Medium as the risk level, which is appropriate given the change affects workspace creation flows (UI + extension-host message handling + file creation). However, the PR is missing the corresponding GitHub label (e.g., risk:medium).
  • Recommendation: Add the repository label risk:medium to this PR so the label matches the Risk Level selected in the PR body. If the repo uses a different naming convention for risk labels, use the repo's canonical label but ensure it matches the PR body.

What & Why

  • Current: (Missing)
  • Issue: The PR body left the "What & Why" section blank. This section is required for context and to help reviewers quickly understand the motivation and user-visible behavior changes.
  • Recommendation: Add a short, focused explanation (2–4 sentences) describing what changed and why. Example based on the diff:
    • "What: Prevent duplicate workspace creation by disabling the Create button immediately and guarding duplicate create messages on the extension host. Replaced hardcoded command strings with ExtensionCommand constants and added tests.\n - Why: Repeated clicks could queue duplicate create messages and required multiple clicks for the UI to proceed; this fixes UX and prevents duplicate operations that could create inconsistent state."

Impact of Change

  • Issue: The Impact of Change section is empty. Reviewers need to know what users, developers, and systems are affected.
  • Recommendation: Populate this section based on the code changes. Suggested content:
    • Users: Clicking Create in the convert-to-workspace webview is now disabled/pending immediately on first click, preventing duplicate creates and improving UX.\n - Developers: Added ExtensionCommand constants usage (reduces magic strings), introduced isCreateInProgress flag in workspaceWebviewCommandHandler, and added setLoading in createWorkspaceSlice to track UI loading. Updated / added unit and E2E tests.\n - System: Minimal performance impact. Works with workspace creation flow and E2E runner: added a new E2E mode conversioncreateonly to the e2e test runner.

Test Plan

  • Assessment: The PR body did not mark any test checkboxes, but the code diff includes both a new unit test (createWorkspace.test.tsx) and E2E updates/additions (workspaceConversionCreate.test.ts and run-e2e.js). This is inconsistent.
  • Issue: The Test Plan section must reflect reality. If unit and/or E2E tests were added, the corresponding checkboxes must be marked and a short description of where/how the tests cover the change should be provided.
  • Recommendation: Update the Test Plan section to mark the appropriate boxes and briefly summarize the tests you added:
    • Mark Unit tests added/updated and note the test file apps/vs-code-react/src/app/createWorkspace/__test__/createWorkspace.test.tsx with a one-line description of the new test (asserts postMessage createWorkspaceStructure and setLoading(true)).
    • Mark E2E tests added/updated and mention the updated workspaceConversionCreate.test.ts and new targeted E2E mode added to run-e2e.js (conversioncreateonly) and what the E2E asserts (Create button enabled before click, click exactly once, button enters pending/disabled or .code-workspace is created).
    • If any manual testing was done, mark Manual testing completed and describe it briefly.

⚠️ Contributors

  • Assessment: Contributors section is blank. This is optional, but it's good practice to credit reviewers, PMs, designers, or helpers.
  • Recommendation: Optionally add contributors or leave blank. If people helped (reviewers, test authors), add them here.

⚠️ Screenshots/Videos

  • Assessment: Not applicable — no UI screenshot required. The change is UX-related but the E2E tests and unit tests are adequate. If you want, include a short GIF or screenshot demonstrating the disabled/pending Create button after the click for reviewers.
  • Recommendation: Optional; not required.

Summary Table

Section Status Recommendation
Title Keep as-is
Commit Type Keep as-is
Risk Level Add GitHub label risk:medium to match PR body
What & Why Add concise "What & Why" (2–4 sentences)
Impact of Change Fill Users/Developers/System impact (see suggestions)
Test Plan Mark Unit and E2E checkboxes and summarize tests added
Contributors ⚠️ Optional: add contributors if applicable
Screenshots/Videos ⚠️ Optional: add if you want to demonstrate UI

Final Notes & Next Steps

  • Overall the code changes look correct and appropriate: you added a defensive isCreateInProgress flag server-side, early-return guard and setLoading client-side, replaced magic strings with ExtensionCommand constants, and added both unit and E2E coverage. These combine to reduce the chance of duplicate workspace creation. From the diff, the changes are moderate in scope and affect workspace creation UX/flow — so risk:medium is appropriate and my advised risk is also risk:medium.
  • Please update the PR body to address the missing items listed above, specifically:
    1. Add a brief "What & Why" section describing behavior and motivation. Use the suggested text in this review as a starting point.\n 2. Fill out "Impact of Change" (Users/Developers/System) — use the recommendations above.\n 3. Update the "Test Plan" checkboxes to mark Unit tests and E2E tests added/updated and include a short note describing the tests and the new E2E mode name conversioncreateonly.\n 4. Add the proper GitHub label to match the selected risk level (e.g. risk:medium).\n 5. Optionally fill Contributors and add an illustrative screenshot/GIF if helpful.

Once those updates are made, re-request review and this checklist should be able to pass.


Last updated: Thu, 07 May 2026 18:08:27 GMT

Copy link
Copy Markdown
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

Note

Copilot was unable to run its full agentic suite in this review.

Prevents duplicate “Create” actions in the convert-to-workspace webview by introducing immediate loading/disabled UI and extension-side de-duping, and tightens automated coverage for the single-click flow.

Changes:

  • Disable “Create” immediately on first click (loading state + client-side guard) and use ExtensionCommand constants.
  • Drop duplicate create messages on the extension-host side while creation is in progress.
  • Add/adjust unit + E2E tests, plus a targeted conversioncreateonly E2E runner mode.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
apps/vs-code-react/src/app/createWorkspace/createWorkspace.tsx Adds loading guard/state and switches to ExtensionCommand constants for create messages.
apps/vs-code-react/src/app/createWorkspace/test/createWorkspace.test.tsx Adds a unit test for convert-to-workspace “Create” behavior and loading state.
apps/vs-code-designer/src/test/ui/workspaceConversionCreate.test.ts Strengthens E2E assertion that one click starts creation / enters pending state.
apps/vs-code-designer/src/test/ui/run-e2e.js Adds conversioncreateonly mode for running only the conversion-create phase.
apps/vs-code-designer/src/app/commands/shared/workspaceWebviewCommandHandler.ts Adds an extension-host de-dupe flag to ignore repeated create messages.

Comment thread apps/vs-code-react/src/app/createWorkspace/__test__/createWorkspace.test.tsx Outdated
Comment thread apps/vs-code-react/src/app/createWorkspace/__test__/createWorkspace.test.tsx Outdated
Comment thread apps/vs-code-react/src/app/createWorkspace/__test__/createWorkspace.test.tsx Outdated
Comment thread apps/vs-code-designer/src/test/ui/workspaceConversionCreate.test.ts Outdated
Comment thread apps/vs-code-designer/src/test/ui/run-e2e.js Outdated
After convert-to-workspace opens a brand-new VS Code window, `installBinaries` runs in the background and the workspace can have multiple folders. Two pre-existing issues then surfaced as `Could not find path to extension bundle`:

1. `getFunctionsCommand` threw when the global `funcCoreToolsBinaryPath` setting was not yet written by `setFunctionsCommand`, even though the binaries already existed on disk. It now self-heals by inspecting the local dependencies folder and falling back to the install location before throwing.

2. `getExtensionBundleFolder` always used `vscode.workspace.workspaceFolders[0]`, which after convert can be a non-Logic-App project (no host.json declaring the extension bundle). It now accepts an optional `workingDirectory` parameter and the designer/monitoring callers pass the resolved Logic App project path.

Also surfaces a friendlier 'dependencies are still installing' error when the func binary path isn't ready, instead of the generic bundle message.

Tests: extends `bundleFeed.test.ts` with optional working-directory and friendly-error coverage; new `funcVersion.test.ts` for the `getFunctionsCommand` self-heal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lambrianmsft and others added 4 commits May 11, 2026 10:42
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lambrianmsft lambrianmsft merged commit 09cb72c into Azure:main May 12, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants