[Fix] Adding default sdk version incase config inspect returns null by ssreerama · Pull Request #21449 · microsoft/vscode-mssql

ssreerama · 2026-03-04T20:49:48Z

Description

Pipeline generated VSIX file does not returns the config defaults (investigating what could be the reason).

Adding a default fallback version and comments to make it sync with the release version.
Could not repro this in local run but pipeline generated vsix repro it.

Code Changes Checklist

New or updated unit tests added
All existing tests pass (npm run test)
Code follows contributing guidelines
Telemetry/logging updated if relevant
No regressions or UX breakage

Reviewers: Please read our reviewer guidelines

Before: Was able to repro with the 1.5.8version install

After: (locally, always working without the fallback version)

Copilot

Pull request overview

This PR addresses a packaging-only issue where VS Code configuration defaults for sqlDatabaseProjects.microsoftBuildSqlVersion are not available (via config.inspect) in the pipeline-generated VSIX, causing the SDK version to resolve to empty/undefined. The change adds an explicit fallback so the extension can still pick a usable Microsoft.Build.Sql SDK version at runtime.

Changes:

Added a hardcoded default Microsoft.Build.Sql SDK version constant (2.1.0) with guidance to keep it in sync with package.json.
Updated getMicrosoftBuildSqlVersion() to fall back to the hardcoded value when config.inspect(...).defaultValue is missing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-03-04T21:00:42Z

PR Changes

Category	Target Branch	PR Branch	Difference
vscode-mssql VSIX	6400 KB	6386 KB	⚪ -14 KB ( 0% )
sql-database-projects VSIX	7024 KB	7025 KB	⚪ 1 KB ( 0% )
data-workspace VSIX	535 KB	535 KB	⚪ 0 KB ( 0% )

codecov-commenter · 2026-03-04T21:06:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.83%. Comparing base (8deebc1) to head (690cff7).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #21449   +/-   ##
=======================================
  Coverage   72.83%   72.83%           
=======================================
  Files         325      325           
  Lines       96419    96419           
  Branches     5411     5411           
=======================================
  Hits        70227    70227           
  Misses      26192    26192

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Benjin · 2026-03-05T20:22:30Z

-    return defaultValue;
+
+    // Read the default value from package.json
+    const extension = vscode.extensions.getExtension(sqldbproj.extension.vsCodeName);


I don't love this method as a work-around. Reading config should be trivial, so let's just do this correctly instead.

done, as per the discussion i have added the fallback version constant and comments for future understanding.

Benjin · 2026-03-05T20:39:30Z

Why does this method need to take in a config string? Is it ever anything different from sqlDatabaseProjects.microsoftBuildSqlVersion?

user can set any version from vscode settings, so we are getting the version from the config file and then if the version is not semver then we fall back to default (constant now)

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T22:58:39Z

-    const defaultValue = config.inspect<string>(microsoftBuildSqlVersionKey)?.defaultValue ?? "";
-    return defaultValue;
+
+    // Fall back to the hardcoded constant if config value is unavailable or invalid


The docstring describes a 3-step resolution order (including “package.json default value”), but the implementation always returns the hardcoded constant for any invalid/blank value and never attempts to read inspect(...).defaultValue. This creates a drift risk if the package.json default changes but FALLBACK_MICROSOFT_BUILD_SQL_VERSION isn’t updated. Consider restoring a guarded config.inspect(...).defaultValue fallback (validate with semver.valid) and only use FALLBACK_MICROSOFT_BUILD_SQL_VERSION when inspect is missing/invalid.

Suggested change

// Fall back to the hardcoded constant if config value is unavailable or invalid

// If the user value is missing/invalid, try the package.json default (if present and valid)

const inspected = config.inspect<string>(microsoftBuildSqlVersionKey);

if (inspected && typeof inspected.defaultValue === "string") {

const defaultValue = inspected.defaultValue.trim();

if (defaultValue && semver.valid(defaultValue)) {

return defaultValue;

}

}

// Fall back to the hardcoded constant if both config and package.json default are unavailable or invalid

Copilot · 2026-03-05T22:58:39Z

+        test("Should fall back to FALLBACK_MICROSOFT_BUILD_SQL_VERSION when configured value is invalid or empty", async function (): Promise<void> {
+            // Test with invalid semver
+            await vscode.workspace
+                .getConfiguration(DBProjectConfigurationKey)
+                .update(
+                    constants.microsoftBuildSqlVersionKey,
+                    "not-a-valid-version",
+                    vscode.ConfigurationTarget.Global,
+                );
+            let result = getMicrosoftBuildSqlVersion();
+            expect(result).to.equal(FALLBACK_MICROSOFT_BUILD_SQL_VERSION);
+
+            // Test with empty config
+            await vscode.workspace
+                .getConfiguration(DBProjectConfigurationKey)
+                .update(
+                    constants.microsoftBuildSqlVersionKey,
+                    undefined,
+                    vscode.ConfigurationTarget.Global,
+                );
+            result = getMicrosoftBuildSqlVersion();
+            expect(result).to.equal(FALLBACK_MICROSOFT_BUILD_SQL_VERSION);
+        });


The “empty config” case uses update(..., undefined, ...), which typically removes the user override and allows config.get(...) to return the package.json default (not “empty”). This assertion will become incorrect if the package.json default ever differs from FALLBACK_MICROSOFT_BUILD_SQL_VERSION, and it doesn’t accurately test a blank/empty user value. Suggest changing this to explicitly set an empty string (e.g., "") to validate the “blank” path, or else assert the package default behavior for the “unset” scenario.

Copilot · 2026-03-05T22:58:39Z

+            await vscode.workspace
+                .getConfiguration(DBProjectConfigurationKey)
+                .update(
+                    constants.microsoftBuildSqlVersionKey,
+                    undefined,
+                    vscode.ConfigurationTarget.Global,
+                );


These tests write to vscode.ConfigurationTarget.Global, which can leak state if the test process is interrupted or a failure prevents cleanup. Prefer writing to ConfigurationTarget.Workspace (or a workspace-folder target) to keep tests isolated and avoid modifying the developer/machine global settings during test runs.

Adding default sdk version incase config inspect returns null

88b4491

ssreerama requested review from Benjin, aasimkhan30, allancascante, caohai, kburtram, laurenastrid1, lewis-sanchez and manujoseph85 as code owners March 4, 2026 20:49

Copilot AI review requested due to automatic review settings March 4, 2026 20:49

ssreerama self-assigned this Mar 4, 2026

Copilot started reviewing on behalf of ssreerama March 4, 2026 20:50 View session

ssreerama added this to the March 2026 Release milestone Mar 4, 2026

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Comment thread extensions/sql-database-projects/src/tools/netcoreTool.ts Outdated

Comment thread extensions/sql-database-projects/src/tools/netcoreTool.ts Outdated

ssreerama linked an issue Mar 4, 2026 that may be closed by this pull request

[Bug]: New project results in sqlproj with blank SDK version #21444

Closed

17 tasks

get the fallback version from package.json during runtime

17b48e6

ssreerama requested a review from Copilot March 4, 2026 21:58

Copilot started reviewing on behalf of ssreerama March 4, 2026 21:58 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

updating tests

c0a304a

ssreerama requested a review from Copilot March 5, 2026 00:37

Copilot started reviewing on behalf of ssreerama March 5, 2026 00:38 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Benjin reviewed Mar 5, 2026

View reviewed changes

adding fall back version

7045a76

ssreerama requested a review from Benjin March 5, 2026 22:24

removing the parameter that can be accessed from constants directly

690cff7

Copilot AI review requested due to automatic review settings March 5, 2026 22:51

Benjin approved these changes Mar 5, 2026

View reviewed changes

Copilot AI reviewed Mar 5, 2026

View reviewed changes

Copilot started reviewing on behalf of ssreerama March 5, 2026 23:28 View session

ssreerama merged commit b032adb into main Mar 6, 2026
6 of 7 checks passed

ssreerama deleted the sai/mar/sdkEmptyIssue branch March 6, 2026 00:03

-    // Fall back to the hardcoded constant if config value is unavailable or invalid
+    // If the user value is missing/invalid, try the package.json default (if present and valid)
+    const inspected = config.inspect<string>(microsoftBuildSqlVersionKey);
+    if (inspected && typeof inspected.defaultValue === "string") {
+        const defaultValue = inspected.defaultValue.trim();
+        if (defaultValue && semver.valid(defaultValue)) {
+            return defaultValue;
+        }
+    }
+    // Fall back to the hardcoded constant if both config and package.json default are unavailable or invalid

Conversation

ssreerama commented Mar 4, 2026

Description

Code Changes Checklist

Reviewers: Please read our reviewer guidelines

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Changes

Uh oh!

codecov-commenter commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Benjin Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

ssreerama Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Benjin Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

ssreerama Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Mar 4, 2026 •

edited

Loading

codecov-commenter commented Mar 4, 2026 •

edited

Loading