Skip to content

feat(vscode): Add codeful sdk resolution from local nuget packages, remove old dependencies#9295

Open
andrew-eldridge wants to merge 2 commits into
mainfrom
aeldridge/lspServerSdkUpdate
Open

feat(vscode): Add codeful sdk resolution from local nuget packages, remove old dependencies#9295
andrew-eldridge wants to merge 2 commits into
mainfrom
aeldridge/lspServerSdkUpdate

Conversation

@andrew-eldridge

@andrew-eldridge andrew-eldridge commented Jun 18, 2026

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

  • Adds utils to resolve codeful sdk version and local nupkg path based on codeful .csproj
  • Removes nupkg from bundled 'LSPServer' dependencies and all references
  • Removes nuget.config with dependencies/LSPServer package source for codeful projects

Impact of Change

  • Users: Correct version of sdk passed to LSP server in all cases - can now reference any version of the sdk from public/private feeds in codeful projects
  • Developers: Add utils for sdk resolution, clean up old code
  • System: N/A

Test Plan

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

Contributors

@andrew-eldridge

@andrew-eldridge andrew-eldridge added the VSCode Issues or PRs specific to VS Code extension label Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🤖 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): Add codeful sdk resolution from local nuget packages, remove old dependencies
  • Issue: No blocking issue. The title is descriptive and clearly indicates the scope of the change.
  • Recommendation: No change required.

Commit Type

  • Current selection is incomplete: only feature is checked, which is valid for this change, but the PR body is missing the required explicit risk selection and should not be considered complete as submitted.
  • Recommendation: Keep only feature checked unless the change truly spans multiple commit types.

Risk Level

  • No risk level selected in the PR body.
  • Advised risk level: medium — this change removes legacy SDK/dependency behavior, changes language server startup logic, and alters project creation/publish flows. That is broader than a minor low-risk change.
  • Recommendation: Check one risk level only. Suggested: Medium.

What & Why

  • Current: The body explains the change clearly: resolving SDK version/path from the project, removing bundled nupkg dependencies, and removing the old nuget.config flow.
  • Issue: No blocking issue.
  • Recommendation: You may optionally expand this slightly with the user-facing reason, but it is sufficient as-is.

⚠️ Impact of Change

  • Partially complete and could be clearer. The sections are present, but the impact description should mention the removal of bundled SDK assets and the new dependency resolution path so reviewers can understand what changes for users and the runtime.
  • Recommendation:
    • Users: Mention that codeful projects now resolve the SDK from the project/NuGet cache instead of the VSIX-bundled package, which may affect restore/setup expectations.
    • Developers: Mention the renamed/removed dependency-install and cache-invalidation flow.
    • System: Mention that language server startup now depends on project SDK resolution and NuGet cache availability.

Test Plan

  • Passes. Unit tests were added/updated in the diff, so the test plan requirement is satisfied.
  • Recommendation: No E2E tests are required here because unit tests are present.

Contributors

  • Current: @andrew-eldridge
  • Issue: No blocking issue.
  • Recommendation: Add additional contributors if anyone else contributed materially to the design or implementation.

Screenshots/Videos

  • Not required. This PR appears to be non-visual.
  • Recommendation: None.

Summary Table

Section Status Recommendation
Title No change required
Commit Type Keep only one commit type checked; feature is appropriate
Risk Level Select exactly one risk level; recommended: Medium
What & Why No change required
Impact of Change ⚠️ Add clearer user/developer/system impact details
Test Plan Unit tests present; no E2E needed
Contributors Add others only if applicable
Screenshots/Videos Not required for this change

Please update the PR body to select a risk level and tighten the impact section, then resubmit. Thank you!


Last updated: Fri, 19 Jun 2026 00:07:50 GMT

@github-actions

Copy link
Copy Markdown
Contributor

🤖 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): Add codeful sdk resolution from local nuget packages, remove old dependencies
  • Issue: The title is clear and sufficiently descriptive.
  • Recommendation: No change needed.

⚠️ Commit Type

  • Properly selected (feature).
  • Only one commit type is checked, which is correct.

Risk Level

  • No risk level is selected in the PR body, but this PR clearly changes core SDK resolution, package handling, and language-server startup behavior.
  • Recommendation: Select exactly one risk level. Based on the diff, High is the safest choice.

What & Why

  • Current:
    • Adds utils to resolve codeful sdk version and local nupkg path based on codeful .csproj
    • Removes nupkg from bundled 'LSPServer' dependencies and all references
    • Removes nuget.config with dependencies/LSPServer package source for codeful projects
  • Issue: Clear and concise.
  • Recommendation: No change needed.

⚠️ Impact of Change

  • The section is populated and understandable, but it could be a bit more specific about the user-facing behavior changes and the removal of bundled SDK assets.
  • Recommendation:
    • Users: Mention that C# workflow authoring now resolves the SDK from the project/NuGet cache rather than bundled dependencies.
    • Developers: Note that startup/publish/build paths no longer rely on the old invalidation flow and that tests were updated accordingly.
    • System: Mention reduced packaged assets and a shift in dependency resolution at runtime.

Test Plan

  • Unit tests were added/updated in the diff, so the test plan passes.
  • No E2E tests are required for this PR because unit coverage exists.

Contributors

  • Contributors are listed (@andrew-eldridge).
  • No issue found.

⚠️ Screenshots/Videos

  • Not provided, but this appears to be a non-visual change, so screenshots are not required.

Summary Table

Section Status Recommendation
Title
Commit Type
Risk Level Select exactly one risk level; High is recommended for this change.
What & Why
Impact of Change ⚠️ Add more explicit user/developer/system impact details.
Test Plan
Contributors
Screenshots/Videos

Please update the PR body to select a single risk level (recommended: High) and optionally expand the impact section for clarity. Once that is fixed, this PR body will comply with the expected template.


Last updated: Thu, 18 Jun 2026 23:26:58 GMT

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage check completed. See workflow run for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the VS Code designer’s codeful workflow/LSP integration to stop shipping a fixed Microsoft.Azure.Workflows.Sdk nupkg inside the extension and instead resolve the SDK nupkg from the user’s project/NuGet caches, while removing the legacy nuget.config + dependency-folder SDK flow.

Changes:

  • Add SDK resolution utilities to locate the Microsoft.Azure.Workflows.Sdk version from the project’s .csproj and resolve the corresponding cached .nupkg path.
  • Remove legacy “extension-shipped SDK nupkg” behavior: delete the nuget.config template, stop copying the SDK into runtime dependencies, and remove related cache invalidation hooks.
  • Rename/install path updates around the LSP server install step (installLSPSDKinstallLSPServer) and update affected tests.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
apps/vs-code-designer/src/constants.ts Removes legacy lspDirectory constant used by the old extension-shipped SDK path.
apps/vs-code-designer/src/assets/CodefulProjectTemplate/ProgramFile Removes workflow builder placeholder section from the template.
apps/vs-code-designer/src/assets/CodefulProjectTemplate/nuget Deletes the legacy nuget.config template that pointed to extension SDK packages.
apps/vs-code-designer/src/app/utils/sdkResolution.ts New utility to resolve SDK version from .csproj and locate the cached .nupkg.
apps/vs-code-designer/src/app/utils/languageServerProtocol.ts Removes SDK copy logic; keeps only LSP server extraction and renames installer API.
apps/vs-code-designer/src/app/utils/codeful.ts Removes legacy SDK cache invalidation flow; retains codeful project detection/parsing utilities.
apps/vs-code-designer/src/app/utils/test/sdkResolution.test.ts Adds unit tests for SDK resolution behavior and fallbacks.
apps/vs-code-designer/src/app/utils/test/languageServerProtocol.test.ts Updates tests to match new “install server only” behavior.
apps/vs-code-designer/src/app/utils/test/codeful.test.ts Removes tests for deleted SDK cache invalidation functionality.
apps/vs-code-designer/src/app/languageServer/languageServer.ts Switches SDK nupkg discovery to resolveSdkFromProject and updates arg construction.
apps/vs-code-designer/src/app/languageServer/test/languageServer.test.ts Updates tests to mock project-based SDK resolution instead of scanning dependency folders.
apps/vs-code-designer/src/app/commands/publishCodefulProject.ts Removes call to the deleted SDK cache invalidation helper.
apps/vs-code-designer/src/app/commands/createWorkflow/createCodefulWorkflow/createCodefulWorkflowSteps/codefulWorkflowCreateStep.ts Removes nuget.config creation/merging step for codeful project creation.
apps/vs-code-designer/src/app/commands/createWorkflow/createCodefulWorkflow/createCodefulWorkflowSteps/test/codefulWorkflowCreateStep.test.ts Removes tests for deleted nuget.config behavior.
apps/vs-code-designer/src/app/commands/createNewCodeProject/CodeProjectBase/CreateLogicAppWorkspace.ts Stops generating nuget.config and removes Program.cs workflow builder substitution.
apps/vs-code-designer/src/app/commands/createNewCodeProject/CodeProjectBase/test/CreateLogicAppWorkspace.test.ts Updates template expectations (no workflow builder placeholder, no nuget.config).
apps/vs-code-designer/src/app/commands/buildCustomCodeFunctionsProject.ts Removes call to the deleted SDK cache invalidation helper.
apps/vs-code-designer/src/app/commands/binaries/validateAndInstallBinaries.ts Renames the LSP installation step to installLSPServer and updates telemetry/progress text.
apps/vs-code-designer/src/app/commands/binaries/test/validateAndInstallBinaries.test.ts Updates mocks/assertions for installLSPServer.
apps/vs-code-designer/src/app/commands/test/publishCodefulProject.test.ts Updates mocks/assertions after removing SDK cache invalidation from publish flow.

Comment thread apps/vs-code-designer/src/app/utils/sdkResolution.ts
Comment thread apps/vs-code-designer/src/app/utils/sdkResolution.ts
Comment thread apps/vs-code-designer/src/app/languageServer/languageServer.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated VSCode Issues or PRs specific to VS Code extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants