feat(vscode) - Adding additional csharp lsp server functionality#8828
Open
lambrianmsft wants to merge 49 commits into
Open
feat(vscode) - Adding additional csharp lsp server functionality#8828lambrianmsft wants to merge 49 commits into
lambrianmsft wants to merge 49 commits into
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
Contributor
🤖 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 | Use conventional commit style; capitalize "C#" and make scope explicit. | |
| Commit Type | ✅ | Keep feature — only one selected which is correct. |
| Risk Level | ❌ | Add risk:high label on PR and update PR body Risk Level to High (advised). |
| What & Why | Expand to describe LSP, runtime, UI, packaging, and test surface. | |
| Impact of Change | ❌ | Fill Users/Developers/System with concrete impacts from diff. |
| Test Plan | Check Unit tests box, describe test runs, and environment. | |
| Contributors | Add contributor list or note none. | |
| Screenshots/Videos | Add screenshots for UI/UX changes or explain omission. |
Summary and next steps
- Advised risk level based on the code diff: High. This is higher than the PR body’s selected "Medium" — please update the PR body and add a GitHub label (e.g.,
risk:high) to match. The changes touch the LSP, runtime startup/validation, packaging (binary assets and nupkg), connection persistence logic, and many UI flows — these affect both developer and user-facing paths and thus justify the higher risk. - Please update the PR with the following (minimum):
- Update the PR title to be clearer and follow conventional commit casing (example provided above).
- Add a risk label on the GitHub PR (risk:high) and update the Risk Level section to "High" or provide a detailed justification if you want to keep "Medium".
- Expand "What & Why" to include the major areas changed (LSP, codeful logic, runtime/start, connection view, packaging, tests).
- Fill Impact of Change: provide bullet points for Users, Developers, and System with specifics (see recommendations above). Mention any migration or configuration impacts, e.g., new extension bundle version, new nupkg asset, or new behaviors requiring environment changes.
- Update Test Plan: mark Unit tests added/updated (they exist in the diff) and detail how E2E/manual testing was performed and on which OS/environments. Mention any platform-specific notes (PowerShell scripts / Windows child process differences).
- Add Contributors or a short note if intentionally blank.
- Add screenshots/videos for UI changes (Overview, Connection view) if possible.
- Add the
risk:highlabel to the PR and, optionally, call out any mitigations (feature flags, canary rollout, steps to revert, testing checklist) to help reviewers and release managers.
If you update the PR body/title and add the risk label, re-request review and I will re-check the updated PR metadata and validate the requested fields. Thank you for the thorough implementation and tests — once the PR metadata is aligned with the actual changes the submission will be much easier to approve.
Last updated: Wed, 06 May 2026 08:17:28 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) { |
Contributor
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); |
Contributor
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?
This was referenced Feb 24, 2026
…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; |
Contributor
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
This was referenced Apr 6, 2026
This was referenced Apr 14, 2026
This was referenced Apr 25, 2026
This was referenced May 2, 2026
Route codeful projects through the existing Overview page from Program.cs, with a workflow dropdown that switches run history, run trigger, callback info, and agent URL state for the selected workflow. Remove the separate codeful run history command, route, webview app, and shared route constants. Hide the per-designer Run history view for codeful workflows while preserving codeless workflow.json Overview behavior. Add reducer coverage for centralized codeful workflow metadata and targeted callback info updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the default stateful codeful template to avoid the unsupported trigger-body expression that crashes the worker during startup while keeping the MSN Weather sample action and response body. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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