feat(vscode) - Adding additional csharp lsp server functionality#8828
feat(vscode) - Adding additional csharp lsp server functionality#8828lambrianmsft wants to merge 47 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 it concise and specific; use conventional commit style (e.g. feat(vscode): add C# LSP & codeful workflow support) |
|
| Commit Type | ✅ | commit type feature selected correctly |
| Risk Level | ❌ | Add matching risk label to the PR. I advise High due to large, cross-cutting changes — update the PR body or labels accordingly. |
| What & Why | Expand the short summary into a few bullets listing major areas changed and why. | |
| Impact of Change | ❌ | Complete Users/Developers/System bullets and mention upgrade/migration steps. |
| Test Plan | ❌ | Check Unit tests added/updated (many unit tests in diff). Provide runnable test checklist and ensure CI passes. |
| Contributors | Add contributor list (optional but recommended). | |
| Screenshots/Videos | Add if UI/UX changed; otherwise optional. |
Final notes and actions to take
-
Advised risk: High (higher than the PR body
Medium). Reason: This PR modifies many core paths (LSP server and packaged SDK, process detection and PowerShell script, build/packaging behavior, runtime start/validation, connection management and auth caching), adds binaries/nupkg assets, and makes behavior changes that will affect users and CI. Please either:- Update the PR risk to High in the body and add a
risk:highlabel to the PR, OR - If you believe the changes are not high risk, provide clear mitigation details in the PR body (feature gates, fallback-safe behavior, thorough E2E coverage, rollout plan) and still add a matching risk label (e.g.
risk:medium) with justification.
- Update the PR risk to High in the body and add a
-
Tests: The diff contains many new test files. Update the Test Plan to mark
Unit tests added/updatedchecked and list the test commands to run locally and in CI. Ensure all tests pass in CI and mention this in the PR. -
Body details to add or expand (copy-paste friendly suggestions):
- What & Why (short):
- "What: Add C# codeful workflow support and LSP improvements. Key changes: Program.cs generation and patching to add workflows, new Codeful workflow templates, LSP server and SDK packaging (added nupkg + LSP zip), improved completion filtering, new helper APIs (resolveConnectionEdit, resolveCurrentConnectionId), improved child process detection (PowerShell script + findChildProcess), caching of Azure connector details, telemetry/diagnostics, and many unit/integration tests."
- "Why: Provide first-class C# (codeful) workflow authoring and debugging in VS Code and reduce UX friction (faster connection pane, more reliable process detection)."
- Impact of Change: exact bullets for Users/Developers/System as shown earlier.
- Test Plan: list commands and which suites changed (files). Example:
- Unit tests:
pnpm -w testorpnpm --filter apps/vs-code-designer test(list changed test files under apps/vs-code-designer/src/**/test) - Manual: steps to create codeful workspace, add workflow, verify Program.cs updates, run debug attach flow, run overview/run history flows, test connection insertion on .cs file, verify auth caching behavior.
- Unit tests:
- What & Why (short):
-
Labels: Add a repository risk label to the PR (e.g.
risk:high) and ensure it matches the PR body. -
Title: Update to conventional commit style and make it specific. Example:
feat(vscode): C# LSP & codeful workflow support (Program.cs generation, LSP SDK, debug/runtime improvements)
Please update the PR title, add the missing risk label (or change the risk in the body), update the Test Plan to reflect unit tests added, and expand the What/Why and Impact sections with the concrete items above. Once the PR body is updated to include the recommended details and labels (and CI/tests are green), re-request review and we can re-evaluate. Thank you for the thorough work — this is a big feature and the more context you provide in the PR body the faster the reviewers can approve and the safer the roll-out will be.
Last updated: Thu, 23 Apr 2026 21:25:07 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
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