Skip to content

Conversation

@lambrianmsft
Copy link
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

Our current use of bundle feed is using index-v2.json which has not been updated in a while. Updating to use and pull the latest version from index.json

Impact of Change

  • Users:
  • None.
  • Developers:
  • None.
  • System:
  • Slightly better system performance as we are reducing an api call for getting the default version range.

Test Plan

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

Contributors

Screenshots/Videos

Copilot AI review requested due to automatic review settings February 11, 2026 00:29
@github-actions
Copy link

github-actions bot commented Feb 11, 2026

🤖 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: fix(vscode): Updated bundle feed to use index.json instead of index-v2.json
  • Issue: None — title is clear, follows conventional commit-style scope, and succinctly describes the change.
  • Recommendation: Keep as-is. If you want to be extra explicit, you could include the impacted module (e.g., apps/vs-code-designer) but it's optional.

Commit Type

  • Properly selected (fix).
  • Note: Exactly one option selected in the PR body, which is correct.

⚠️ Risk Level

  • Assessment: The PR body marks this as Low risk and the code changes are consistent with that (small surface area: two files changed, focused behavior change to feed URL and related test additions). I advise risk:low.
  • Issue: There is no repository risk label present (expected labels like risk:low, risk:medium, or risk:high). The PR currently has the label needs-pr-update but not a risk label that matches the body.
  • Recommendation: Add the appropriate risk label matching the PR body (e.g., risk:low). Remove needs-pr-update once you have updated the PR body/labels.

What & Why

  • Current: "Our current use of bundle feed is using index-v2.json which has not been updated in a while. Updating to use and pull the latest version from index.json"
  • Issue: None — brief and adequate.
  • Recommendation: Good. If desired, add a one-line note about why switching to index.json is preferable (e.g., updated feed format / more frequent updates) but not required.

⚠️ Impact of Change

  • Issue: The Impact section is present and mostly fine, but the Developers/System entries could be slightly expanded to note the function signature changes that occurred in the diff.
  • Recommendation:
    • Users: None (as claimed).
    • Developers: Mention that addDefaultBundle signature was changed (context parameter removed) and getLatestVersionRange changed from an async function that fetched a feed to a synchronous getter returning the defaultVersionRange constant — ensure all callers were updated accordingly.
    • System: Keep the note about slight performance improvement due to reduced API calls.

Test Plan

  • Assessment: Unit tests were added/updated in the diff (apps/vs-code-designer/src/app/utils/test/bundleFeed.test.ts) to cover getLatestVersionRange, addDefaultBundle, and downloadExtensionBundle scenarios — this matches the "Unit tests added/updated" checkbox in the PR body.
  • Recommendation: Run CI and confirm new tests pass. If these changes affect any E2E flows (unlikely but possible), consider adding or running E2E tests as a follow-up.

⚠️ Contributors

  • Assessment: Section is blank.
  • Recommendation: If others (PMs, designers, reviewers) contributed, add their GitHub handles. Blank is acceptable but consider crediting collaborators.

⚠️ Screenshots/Videos

  • Assessment: Not applicable for this non-UI change; blank is fine.
  • Recommendation: No action required.

Summary Table

Section Status Recommendation
Title Keep as-is.
Commit Type No change needed.
Risk Level ⚠️ Add the matching repo label (e.g., risk:low). Remove needs-pr-update after update.
What & Why Good.
Impact of Change ⚠️ Call out the changed function signatures and ensure callers updated.
Test Plan Unit tests present; ensure CI passes.
Contributors ⚠️ Optional: add contributors if any.
Screenshots/Videos ⚠️ Not applicable.

Final Notes & Action Items

  1. Please add the appropriate risk label matching the PR body (recommended: risk:low). The PR currently lacks a risk label — add one and remove needs-pr-update once done.
  2. Verify that all call sites were updated for the changed function signatures:
    • addDefaultBundle(context, hostJson) -> addDefaultBundle(hostJson) (context parameter removed). Search the repo for usages and update them as needed.
    • getLatestVersionRange(context) -> getLatestVersionRange() (now synchronous and returns defaultVersionRange). Ensure any callers expecting a Promise were updated.
  3. Confirm CI passes for the added unit tests and any integration pipelines that might rely on the old async behavior.
  4. (Optional) Expand the Impact section in the PR body to mention the function signature changes so reviewers can easily spot possible breaking changes for developers.

Please update the PR with the label and any notes about call-site updates, then re-submit. Thank you for the clear title, tests, and concise PR body — overall this looks good and low risk.


Last updated: Wed, 11 Feb 2026 00:41:03 GMT

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the VS Code designer’s extension-bundle feed usage to rely on index.json (instead of index-v2.json) and avoids fetching the default bundle version range from the feed by using the in-repo constant.

Changes:

  • Switch workflow bundle feed URL to .../index.json and update the “latest bundle version” selection logic accordingly.
  • Remove the network-based “latest version range” lookup; getLatestVersionRange() and addDefaultBundle() now use defaultVersionRange.
  • Add unit tests covering the new getLatestVersionRange() and addDefaultBundle() behavior.

Reviewed changes

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

File Description
apps/vs-code-designer/src/app/utils/bundleFeed.ts Switches to index.json, removes index-v2.json feed usage for default version range, and adjusts version selection from feed results.
apps/vs-code-designer/src/app/utils/test/bundleFeed.test.ts Adds tests for getLatestVersionRange() and addDefaultBundle().

Comment on lines +25 to 31
async function getWorkflowBundleFeed(context: IActionContext): Promise<string[]> {
const envVarUri: string | undefined = process.env.FUNCTIONS_EXTENSIONBUNDLE_SOURCE_URI;
const baseUrl: string = envVarUri || 'https://cdn.functions.azure.com/public';
const url = `${baseUrl}/ExtensionBundles/${extensionBundleId}/index-v2.json`;
const url = `${baseUrl}/ExtensionBundles/${extensionBundleId}/index.json`;

return getJsonFeed(context, url);
}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

getWorkflowBundleFeed is typed to return Promise<string[]>, but it directly returns getJsonFeed(context, url). getJsonFeed is constrained to T extends Record<string, any> (i.e., JSON object), so this is very likely a type error and/or a runtime mismatch if index.json is an object (as the existing IBundleFeed model suggests). Consider fetching index.json into a typed object (e.g., IBundleFeed or a dedicated IIndexJsonFeed), then explicitly deriving the version list from a known property (and keep the function return type aligned with what index.json actually contains).

Copilot uses AI. Check for mistakes.
Comment on lines +162 to 165
const feedVersions: string[] = await getWorkflowBundleFeed(context);
for (const bundleVersion of feedVersions) {
latestFeedBundleVersion = semver.gt(latestFeedBundleVersion, bundleVersion) ? latestFeedBundleVersion : bundleVersion;
}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

downloadExtensionBundle assumes every entry returned from the feed is a valid semver and passes it directly to semver.gt. If any non-semver value makes it into the returned list (e.g., because the feed shape changes or contains metadata), semver.gt can throw and the whole download path will fall into the catch and silently skip updating. Consider filtering/validating feed entries with semver.valid (and/or extracting versions from a well-defined feed object) before comparing.

Copilot uses AI. Check for mistakes.
Comment on lines 56 to 62
/**
* Gets latest bundle extension version range.
* @param {IActionContext} context - Command context.
* @returns {Promise<string>} Returns lates version range.
* @returns {string} Returns latest version range.
*/
export async function getLatestVersionRange(context: IActionContext): Promise<string> {
const feed: IBundleFeed = await getBundleFeed(context, undefined);
return feed.defaultVersionRange;
export function getLatestVersionRange(): string {
return defaultVersionRange;
}
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

getLatestVersionRange now returns the defaultVersionRange constant unconditionally, but its name/JSDoc still reads like it computes the latest value. This is misleading for callers and makes it easy to assume it’s dynamically sourced. Consider renaming it (e.g., to reflect that it returns the default/fallback range) and/or updating the doc comment to explicitly state it no longer queries the feed.

Copilot uses AI. Check for mistakes.
Comment on lines +428 to +429
logLevel: {
default: 'Information',
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

In the "should preserve other host.json properties" test, hostJson is typed as IHostJsonV2 but the logging object you provide uses logLevel.default, which doesn’t match the IHostJsonV2['logging'] shape (it expects applicationInsights.samplingSettings...). This will fail type-checking; either use a property that exists on IHostJsonV2 (e.g., managedDependency, extensions, or the supported logging.applicationInsights structure) or relax the test’s typing for that specific case.

Suggested change
logLevel: {
default: 'Information',
applicationInsights: {
samplingSettings: {
isEnabled: true,
},

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant