feat(vscode) - Adding additional csharp lsp server functionality#8828
feat(vscode) - Adding additional csharp lsp server functionality#8828lambrianmsft wants to merge 46 commits intoAzure:ccastrotrejo/csharpLSPServerfrom
Conversation
… 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
…ded temporary shouldExtractOrUpdate method
…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
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:
|
| 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 aneeds-pr-updatelabel. - 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) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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?
…AppsUX into ccastrotrejo/csharpLSPServer
|
|
||
| // 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; |
There was a problem hiding this comment.
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
…tion, update tests
…o reduce azure auth time, updated connectionTable to look if a selected connection is inside the connections.json, updated flow to prevent hanging after use connectors from azure is selected initally
…brianmsft/LogicAppsUX into ccastrotrejo/csharpLSPServer
…ic operation for connectionView
… to azure, updated lsp server with correct codelens location, fixed connection insertion locations
…sUX into ccastrotrejo/csharpLSPServer
…that displays manage or create connection depending on connection.json
…brianmsft/LogicAppsUX into ccastrotrejo/csharpLSPServer
…d for the first time, debug session now attaches properly, and run history webview refreshes.
Commit Type
Risk Level
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
Workflows runs can now be opened and new workflows can be added
Test Plan
Contributors
Screenshots/Videos