Skip to content

feat(vscode) - Adding additional csharp lsp server functionality#8828

Open
lambrianmsft wants to merge 46 commits intoAzure:ccastrotrejo/csharpLSPServerfrom
lambrianmsft:ccastrotrejo/csharpLSPServer
Open

feat(vscode) - Adding additional csharp lsp server functionality#8828
lambrianmsft wants to merge 46 commits intoAzure:ccastrotrejo/csharpLSPServerfrom
lambrianmsft:ccastrotrejo/csharpLSPServer

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

Adding ability to open overview page for a workflow, adding a new workflow, reducing the connection pane load time, and updating the package.

Impact of Change

  • Users:
    Workflows runs can now be opened and new workflows can be added
  • Developers:
  • System:

Test Plan

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

Contributors

Screenshots/Videos

… call times for checking child process, added options to add new codeful workflows to the project, optimzed the get-child-process.ps1 script, and finally added the overview page for codeful workflows and ability to view runs
…d to be passed as a path and added additional telemetry to capture this whenever it happens
…ger name isn't defined, we pull the default from the lsp server sdk. Disabled the create unit test from run for codeful workflows
…perience, prevent callback url from getting continuously pinged, updated sdk version
@github-actions
Copy link

github-actions bot commented Feb 19, 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) - Adding additional csharp lsp server functionality
  • Issue: The title is okay as a short summary, but it's vague given the breadth of changes in this PR (LSP server integration, codeful workflow support, many VS Code extension behaviors, runtime/process handling, PowerShell script changes, new assets and templates, and many tests). It should summarize the most impactful areas changed so reviewers and release notes can quickly understand scope.
  • Recommendation: Use a more specific, scoped title. Example: feat(vscode): Add C# LSP integration & codeful workflow support (overview, codegen, connection handling, tests)

Commit Type

  • Properly selected (feature) in the PR body.
  • Note: The PR contains many behavioral changes and new features; feature is appropriate.

Risk Level

  • Assessment: The PR body marks Medium risk, but based on the code diff this is a large, cross-cutting change touching: the LSP server integration and protocol, VS Code webviews and React app, project/template generation for C# codeful workflows, new binaries and nupkg assets, PowerShell script that inspects processes, design-time/runtime process management and caching, and many runtime behaviors. These all increase the potential for user-facing regressions and platform-specific failures (Windows PowerShell path, PID handling, taskkill). Therefore I advise: High risk.
  • Note: The repository is missing a risk label (e.g., risk:high) — please add one. If you intentionally consider this Medium, include an explicit rationale in the PR body explaining why the large scope should be Medium rather than High (for example: limited rollout strategy, feature flags, strong test coverage and canary). Otherwise, please update the PR body and label to High.

⚠️ What & Why

  • Current: "Adding ability to open overview page for a workflow, adding a new workflow, reducing the connection pane load time, and updating the package."
  • Issue: This is brief and does not enumerate the major functional areas changed, nor the backend/UX/packaging implications.
  • Recommendation: Replace or expand with a concise bullet list tied to key files/behaviors. Example content to paste:
    • Adds C# codeful workflow support (create codeful workflows, program/Program.cs updates, .csproj and nuget handling).
    • Integrates a local C# LSP server and adds LSP-related middleware + completion filtering.
    • Improves Connection View: atomic insert of local connections (connectionAndSetting), connections.json synchronization, connections lookup.
    • Updates design-time/runtime process detection and caching (PowerShell script updates, PID resolution changes, validation cache).
    • Adds many unit tests for new code paths and updates webview/React wiring (create workspace, designer, monitoring view).
    • Adds and updates extension assets (LSP zip, SDK nupkg, codeful templates).

Impact of Change

  • Issue: Impact section in PR body is sparse (Users filled but Developers and System left blank). Given the scope, you must clearly state who/what is affected.
  • Recommendation: Fill the Impact section with targeted statements. Example to paste:
    • Users:
      • Codeful workflows (.cs) are supported in the VS Code extension: users can create/open codeful workflows, run triggers, and debug locally. The Overview/Monitoring/Run History flows are updated to work with codeful projects.
      • Connection pane behavior changed: local connections are now written atomically with source edits; managed connections use connectionReferences. This can change how connection names appear in source files.
    • Developers:
      • New public/internal helpers and contracts: getCodefulWorkflowMetadata (LSP integration), resolveConnectionEdit/resolveCurrentConnectionId (connection editing), APIs in utils/codeful.ts and languageServer.ts. Update any consumers of ProjectType (renamed to codeful) and IWebviewProjectContext types (logicAppType is now typed as ProjectType). Also new extension context keys (e.g., azureLogicAppsStandard.isCodeful, azureLogicAppsStandard.codefulWorkflowFiles).
      • New tests added — please update CI if additional test runners or longer timeouts are required.
    • System / Runtime:
      • Design-time/runtime start/stop and process detection behavior changed (PowerShell script and child-process discovery, caching of validation). This can affect Windows environments and users running in dev containers; ensure the PowerShell script is acceptable in your security policy and consider elevated permission impacts.
      • New binary assets (LSPServer.zip and SDK nupkg) are added — validate licensing and distribution policy.

Test Plan

  • Assessment: The PR body claims "E2E tests added/updated" and "Manual testing completed" but did not check "Unit tests added/updated". The code diff shows many new/updated test files (unit tests) across the repo (multiple new test files under apps/vs-code-designer/src/**/test and libs tests). It appears primarily unit tests were added — please reconcile the test plan checkboxes.
  • Recommendation:
    • If you added unit tests, update the PR Test Plan checkboxes to include "Unit tests added/updated". List a few example test files in the PR body to make the review easier (e.g., apps/vs-code-designer/src/app/commands/workflows/languageServer/test/connectionView.test.ts, apps/vs-code-designer/src/app/commands/createNewCodeProject/CodeProjectBase/test/CreateLogicAppWorkspace.test.ts, libs/.../completionFilter.test.ts, etc.).
    • If you truly added E2E tests as well, list where they are and how to run them.
    • If manual testing is used for some areas (e.g., debugging & runtime interactions), give explicit steps and environments used (Windows/macOS/Linux, VS Code versions, whether design-time/runtime were run). Without this, reviewers can't reason about runtime stability.

⚠️ Contributors

  • Assessment: Blank in the PR body.
  • Recommendation: Please add contributors (engineers, PMs, designers) so they receive credit and so reviewers know who to ping for questions.

⚠️ Screenshots/Videos

  • Assessment: Blank. There are multiple UI changes (webview content, connection pane, monitoring/overview). Visual diffs would help reviewers.
  • Recommendation: Add screenshots or short GIFs showing: Overview for a codeful workflow, connection pane behavior before/after, and any new dialogs. If you intentionally omitted because there are no visual changes, say so.

Summary Table

Section Status Recommendation
Title ⚠️ Make the title more specific and mention primary areas changed (LSP, codeful workflows, connection behaviour).
Commit Type feature is correct for these changes.
Risk Level Change PR risk to High and add a risk:high label, or justify why this large change is Medium.
What & Why ⚠️ Expand to call out the main features and files affected (see suggested text).
Impact of Change Fill out Users/Developers/System with concrete impacts (see suggested bullets).
Test Plan Update checkboxes: Unit tests were added. Describe E2E/manual steps or add missing E2E tests.
Contributors ⚠️ Add contributors list or note if none.
Screenshots/Videos ⚠️ Add visuals for UX changes or explain omission.

Final Notes & Action Items

  • Risk: I advise upgrading the risk to High due to broad, cross-cutting changes (LSP, runtime, PowerShell, new binaries, templates). The PR is currently marked Medium — please either update the PR's Risk Level to High and add a corresponding label (risk:high) or provide a clear justification for keeping it Medium.
  • Tests: Update the Test Plan checkboxes to indicate Unit tests added/updated (they are present). If you also added E2E tests, point to them and provide instructions to run. Add manual test steps and environments used.
  • Impact: Expand the Impact of Change section with the recommended bullets; include potential platform-specific notes (Windows PowerShell behavior) and the new extension context keys / API surface changes that other parts of the extension or downstream consumers may need to accommodate.
  • Labels: Add a risk:* label matching the final selected risk. There currently is only a needs-pr-update label.
  • Contributors & Visuals: Add contributor credits and screenshots for webview/UI changes.

Please update the PR body and labels with the above changes and respond to any review comments. After you update the PR body and the risk label (or justify keeping Medium), I will re-check and provide further guidance. Thank you!


Last updated: Tue, 17 Mar 2026 22:10:15 GMT

} else {
// For codeless projects, build custom code functions if they exist
const customFolderExists = await fse.pathExists(path.join(logicAppNode.fsPath, libDirectory, customDirectory));
if (customFolderExists) {
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 this be if !customFolderExists? Since we would build in the case where lib/custom/* doesn't already exist for codeless, i.e. not already built

// For codeless projects, build custom code functions if they exist
const customFolderExists = await fse.pathExists(path.join(logicAppNode.fsPath, libDirectory, customDirectory));
if (customFolderExists) {
await buildCustomCodeFunctionsProject(actionContext, logicAppNode);
Copy link
Contributor

Choose a reason for hiding this comment

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

buildCustomCodeFunctionsProject is supposed to check whether the .net project is specifically a custom code functions project before building, part of that is to check for Microsoft.Azure.Workflows.Webjobs.Sdk in the .csproj, which I don't think will exist for codeful projects. Should there be a separate function for handling building codeful?


// Find the location to insert the new workflow builder
// Look for the "Build all workflows" comment or insert before host.Run()
const buildCallRegex = /(\s*)(\/\/ Build all workflows\s*\n(?:\s*\/\/.*\n)*\s*)(.*?)(\s*host\.Run\(\);)/s;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to avoid using a regex here to update Program.cs with a new workflow, and instead just overwrite Program.cs with one containing all workflows? We would need to track the workflows in a codeful project elsewhere, but we should probably avoid using regex as much as possible since it's bound to break in some cases

andrew-eldridge and others added 7 commits March 11, 2026 14:54
… to azure, updated lsp server with correct codelens location, fixed connection insertion locations
…that displays manage or create connection depending on connection.json
…d for the first time, debug session now attaches properly, and run history webview refreshes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants