Skip to content

Commit f8ae0f5

Browse files
FrankLiu4138claude
andcommitted
fix(upgrade): check extension version before calling gotoAgentMode
- checkOrInstallAppModExtensionForUpgrade now verifies extension version >= 1.13.0 (MIN_APPMOD_VERSION) before proceeding - If extension is outdated, triggers update and sends telemetry - Track install/update success rate via extensionInstall telemetry - Add source property (cve/upgrade) to upgrade command telemetry - Notification button and message body now distinguish three states: up-to-date, outdated, not-installed - Rename display name to "GitHub Copilot modernization" - Extract pure functions (getExtensionState, buildNotificationContent) for unit testability - Add comprehensive unit tests with real release version numbers Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
1 parent e9ae840 commit f8ae0f5

6 files changed

Lines changed: 278 additions & 35 deletions

File tree

.squire/.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Ignore everything in .squire/ except this file
2+
*
3+
!.gitignore

src/constants.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,13 @@ export namespace ExtensionName {
3636
export const APP_MODERNIZATION_FOR_JAVA = "vscjava.migrate-java-to-azure";
3737
// Java upgrade extension is merged into app modernization extension
3838
export const APP_MODERNIZATION_UPGRADE_FOR_JAVA = APP_MODERNIZATION_FOR_JAVA;
39-
export const APP_MODERNIZATION_EXTENSION_NAME = "GitHub Copilot app modernization";
39+
export const APP_MODERNIZATION_EXTENSION_NAME = "GitHub Copilot modernization";
4040
}
4141

4242
export namespace Upgrade {
4343
export const PACKAGE_ID_FOR_JAVA_RUNTIME = "java:*";
44+
/** Minimum version of the appmod extension that supports gotoAgentMode command */
45+
export const MIN_APPMOD_VERSION = "1.14.0";
4446
}
4547

4648
/**

src/upgrade/display/notificationManager.ts

Lines changed: 72 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
// Licensed under the MIT license.
33

44
import { commands, ExtensionContext, extensions, window } from "vscode";
5+
import * as semver from "semver";
56
import { UpgradeReason, type IUpgradeIssuesRenderer, type UpgradeIssue } from "../type";
6-
import { buildCVENotificationMessage, buildFixPrompt, buildNotificationMessage } from "../utility";
7+
import { buildCVENotificationMessage, buildFixPrompt, buildNotificationMessage, type ExtensionState } from "../utility";
78
import { Commands } from "../../commands";
89
import { Settings } from "../../settings";
910
import { instrumentOperation, sendInfo } from "vscode-extension-telemetry-wrapper";
10-
import { ExtensionName } from "../../constants";
11+
import { ExtensionName, Upgrade } from "../../constants";
1112
import { CveUpgradeIssue } from "../cve";
1213

1314
const KEY_PREFIX = 'javaupgrade.notificationManager';
@@ -17,6 +18,8 @@ const BUTTON_TEXT_UPGRADE = "Upgrade Now";
1718
const BUTTON_TEXT_FIX_CVE = "Fix Now";
1819
const BUTTON_TEXT_INSTALL_AND_UPGRADE = "Install Extension and Upgrade";
1920
const BUTTON_TEXT_INSTALL_AND_FIX_CVE = "Install Extension and Fix";
21+
const BUTTON_TEXT_UPDATE_AND_UPGRADE = "Update Extension and Upgrade";
22+
const BUTTON_TEXT_UPDATE_AND_FIX_CVE = "Update Extension and Fix";
2023
const BUTTON_TEXT_NOT_NOW = "Not Now";
2124

2225
const SECONDS_IN_A_DAY = 24 * 60 * 60;
@@ -26,6 +29,61 @@ function getNowTs() {
2629
return Number(new Date()) / 1000;
2730
}
2831

32+
export type { ExtensionState } from "../utility";
33+
34+
export interface NotificationContent {
35+
message: string;
36+
upgradeButtonText: string;
37+
fixCVEButtonText: string;
38+
}
39+
40+
export function getExtensionState(extensionVersion: string | undefined): ExtensionState {
41+
if (!extensionVersion) {
42+
return "not-installed";
43+
}
44+
if (semver.gte(extensionVersion, Upgrade.MIN_APPMOD_VERSION)) {
45+
return "up-to-date";
46+
}
47+
return "outdated";
48+
}
49+
50+
export function buildNotificationContent(
51+
issues: UpgradeIssue[],
52+
extensionState: ExtensionState,
53+
): NotificationContent {
54+
const cveIssues = issues.filter(
55+
(i): i is CveUpgradeIssue => i.reason === UpgradeReason.CVE
56+
);
57+
const nonCVEIssues = issues.filter(
58+
(i) => i.reason !== UpgradeReason.CVE
59+
);
60+
const hasCVEIssue = cveIssues.length > 0;
61+
62+
const message = hasCVEIssue
63+
? buildCVENotificationMessage(cveIssues, extensionState)
64+
: buildNotificationMessage(nonCVEIssues[0], extensionState);
65+
66+
let upgradeButtonText: string;
67+
let fixCVEButtonText: string;
68+
69+
switch (extensionState) {
70+
case "up-to-date":
71+
upgradeButtonText = BUTTON_TEXT_UPGRADE;
72+
fixCVEButtonText = BUTTON_TEXT_FIX_CVE;
73+
break;
74+
case "outdated":
75+
upgradeButtonText = BUTTON_TEXT_UPDATE_AND_UPGRADE;
76+
fixCVEButtonText = BUTTON_TEXT_UPDATE_AND_FIX_CVE;
77+
break;
78+
case "not-installed":
79+
upgradeButtonText = BUTTON_TEXT_INSTALL_AND_UPGRADE;
80+
fixCVEButtonText = BUTTON_TEXT_INSTALL_AND_FIX_CVE;
81+
break;
82+
}
83+
84+
return { message, upgradeButtonText, fixCVEButtonText };
85+
}
86+
2987
class NotificationManager implements IUpgradeIssuesRenderer {
3088
private hasShown = false;
3189
private context?: ExtensionContext;
@@ -42,16 +100,6 @@ class NotificationManager implements IUpgradeIssuesRenderer {
42100
return;
43101
}
44102

45-
// Filter to only CVE issues and cast to CveUpgradeIssue[]
46-
const cveIssues = issues.filter(
47-
(i): i is CveUpgradeIssue => i.reason === UpgradeReason.CVE
48-
);
49-
const nonCVEIssues = issues.filter(
50-
(i) => i.reason !== UpgradeReason.CVE
51-
);
52-
const hasCVEIssue = cveIssues.length > 0;
53-
const issue = hasCVEIssue ? cveIssues[0] : nonCVEIssues[0];
54-
55103
if (!this.shouldShow()) {
56104
return;
57105
}
@@ -61,28 +109,27 @@ class NotificationManager implements IUpgradeIssuesRenderer {
61109
}
62110
this.hasShown = true;
63111

64-
const hasExtension = !!extensions.getExtension(ExtensionName.APP_MODERNIZATION_UPGRADE_FOR_JAVA);
65-
const prompt = buildFixPrompt(issue);
112+
const ext = extensions.getExtension(ExtensionName.APP_MODERNIZATION_UPGRADE_FOR_JAVA);
113+
const extensionState = getExtensionState(ext?.packageJSON?.version);
114+
const { message, upgradeButtonText, fixCVEButtonText } = buildNotificationContent(issues, extensionState);
66115

67-
let notificationMessage = "";
116+
const hasCVEIssue = issues.some(i => i.reason === UpgradeReason.CVE);
117+
const issue = hasCVEIssue
118+
? issues.find((i): i is CveUpgradeIssue => i.reason === UpgradeReason.CVE)!
119+
: issues.find(i => i.reason !== UpgradeReason.CVE)!;
120+
const prompt = buildFixPrompt(issue);
68121

69-
if (hasCVEIssue) {
70-
notificationMessage = buildCVENotificationMessage(cveIssues, hasExtension);
71-
} else {
72-
notificationMessage = buildNotificationMessage(issue, hasExtension);
73-
}
74-
const upgradeButtonText = hasExtension ? BUTTON_TEXT_UPGRADE : BUTTON_TEXT_INSTALL_AND_UPGRADE;
75-
const fixCVEButtonText = hasExtension ? BUTTON_TEXT_FIX_CVE : BUTTON_TEXT_INSTALL_AND_FIX_CVE;
76122
sendInfo(operationId, {
77123
operationName: "java.dependency.upgradeNotification.show",
124+
extensionState,
78125
});
79126

80127
const buttons = hasCVEIssue
81128
? [fixCVEButtonText, BUTTON_TEXT_NOT_NOW]
82129
: [upgradeButtonText, BUTTON_TEXT_NOT_NOW];
83130

84131
const selection = await window.showInformationMessage(
85-
notificationMessage,
132+
message,
86133
...buttons
87134
);
88135
sendInfo(operationId, {
@@ -93,7 +140,8 @@ class NotificationManager implements IUpgradeIssuesRenderer {
93140
switch (selection) {
94141
case fixCVEButtonText:
95142
case upgradeButtonText: {
96-
commands.executeCommand(Commands.JAVA_UPGRADE_WITH_COPILOT, prompt);
143+
const source = selection === fixCVEButtonText ? "cve" : "upgrade";
144+
commands.executeCommand(Commands.JAVA_UPGRADE_WITH_COPILOT, prompt, source);
97145
break;
98146
}
99147
case BUTTON_TEXT_NOT_NOW: {

src/upgrade/upgradeManager.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { Settings } from "../settings";
1313
import assessmentManager, { getDirectDependencies } from "./assessmentManager";
1414
import { checkOrInstallAppModExtensionForUpgrade, checkOrPopupToInstallAppModExtensionForModernization } from "./utility";
1515

16-
const DEFAULT_UPGRADE_PROMPT = "Upgrade Java project dependency to latest version.";
16+
const DEFAULT_UPGRADE_PROMPT = "Upgrade Java runtime and frameworks to the latest stable version.";
1717

1818

1919
function shouldRunCheckup() {
@@ -25,7 +25,11 @@ class UpgradeManager {
2525
notificationManager.initialize(context);
2626

2727
// Upgrade project
28-
context.subscriptions.push(instrumentOperationAsVsCodeCommand(Commands.JAVA_UPGRADE_WITH_COPILOT, async (promptText?: string) => {
28+
context.subscriptions.push(instrumentOperationAsVsCodeCommand(Commands.JAVA_UPGRADE_WITH_COPILOT, async (promptText?: string, source?: string) => {
29+
sendInfo("", {
30+
operationName: "java.dependency.upgrade.execute",
31+
source: source ?? "unknown",
32+
});
2933
await checkOrInstallAppModExtensionForUpgrade(ExtensionName.APP_MODERNIZATION_UPGRADE_FOR_JAVA);
3034
const promptToUse = promptText ?? DEFAULT_UPGRADE_PROMPT;
3135
await commands.executeCommand(Commands.GOTO_AGENT_MODE, { prompt: promptToUse, useCustomAgent: true });

src/upgrade/utility.ts

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,20 @@ function findEolDate(currentVersion: string, eolDate: Record<string, string>): s
2222
return null;
2323
}
2424

25-
export function buildNotificationMessage(issue: UpgradeIssue, hasExtension: boolean): string {
25+
export type ExtensionState = "up-to-date" | "outdated" | "not-installed";
26+
27+
function getActionWord(extensionState: ExtensionState, verb: string): string {
28+
switch (extensionState) {
29+
case "up-to-date":
30+
return verb;
31+
case "outdated":
32+
return `update ${ExtensionName.APP_MODERNIZATION_EXTENSION_NAME} extension and ${verb}`;
33+
case "not-installed":
34+
return `install ${ExtensionName.APP_MODERNIZATION_EXTENSION_NAME} extension and ${verb}`;
35+
}
36+
}
37+
38+
export function buildNotificationMessage(issue: UpgradeIssue, extensionState: ExtensionState): string {
2639
const {
2740
packageId,
2841
currentVersion,
@@ -31,7 +44,7 @@ export function buildNotificationMessage(issue: UpgradeIssue, hasExtension: bool
3144
packageDisplayName
3245
} = issue;
3346

34-
const upgradeWord = hasExtension ? "upgrade" : `install ${ExtensionName.APP_MODERNIZATION_EXTENSION_NAME} extension and upgrade`;
47+
const upgradeWord = getActionWord(extensionState, "upgrade");
3548

3649
if (packageId === Upgrade.PACKAGE_ID_FOR_JAVA_RUNTIME) {
3750
return `This project is using an older Java runtime (${currentVersion}). Would you like to ${upgradeWord} it to the latest LTS version?`;
@@ -51,7 +64,7 @@ export function buildNotificationMessage(issue: UpgradeIssue, hasExtension: bool
5164
}
5265
}
5366

54-
export function buildCVENotificationMessage(issues: CveUpgradeIssue[], hasExtension: boolean): string {
67+
export function buildCVENotificationMessage(issues: CveUpgradeIssue[], extensionState: ExtensionState): string {
5568

5669
if (issues.length === 0) {
5770
return "No CVE issues found.";
@@ -81,7 +94,7 @@ export function buildCVENotificationMessage(issues: CveUpgradeIssue[], hasExtens
8194
CVESeverityDistribution: severityText,
8295
});
8396

84-
const fixWord = hasExtension ? "fix" : `install ${ExtensionName.APP_MODERNIZATION_EXTENSION_NAME} extension and fix`;
97+
const fixWord = getActionWord(extensionState, "fix");
8598

8699
if (issues.length === 1) {
87100
return `${severityText} CVE vulnerability is detected in this project. Would you like to ${fixWord} it now?`;
@@ -102,7 +115,7 @@ export function buildFixPrompt(issue: UpgradeIssue): string {
102115
return `upgrade ${packageDisplayName} to ${suggestedVersionName}`;
103116
}
104117
case UpgradeReason.CVE: {
105-
return `fix all critical and high-severity CVE vulnerabilities in this project by invoking #appmod-validate-cves-for-java`;
118+
return `fix all CVE vulnerabilities in this project`;
106119
}
107120
}
108121
}
@@ -155,10 +168,37 @@ export async function checkOrPopupToInstallAppModExtensionForModernization(
155168

156169
export async function checkOrInstallAppModExtensionForUpgrade(
157170
extensionIdToCheck: string): Promise<void> {
158-
if (extensions.getExtension(extensionIdToCheck)) {
159-
return;
171+
const ext = extensions.getExtension(extensionIdToCheck);
172+
173+
if (ext) {
174+
const installedVersion = ext.packageJSON?.version;
175+
if (installedVersion && semver.gte(installedVersion, Upgrade.MIN_APPMOD_VERSION)) {
176+
return;
177+
}
178+
// Extension is installed but too old — update it
179+
sendInfo("", {
180+
operationName: "java.dependency.upgrade.extensionOutdated",
181+
installedVersion,
182+
minRequiredVersion: Upgrade.MIN_APPMOD_VERSION,
183+
});
160184
}
161185

162-
await commands.executeCommand("workbench.extensions.installExtension", ExtensionName.APP_MODERNIZATION_FOR_JAVA);
186+
const action = ext ? "update" : "install";
187+
try {
188+
await commands.executeCommand("workbench.extensions.installExtension", ExtensionName.APP_MODERNIZATION_FOR_JAVA);
189+
sendInfo("", {
190+
operationName: "java.dependency.upgrade.extensionInstall",
191+
action,
192+
result: "success",
193+
});
194+
} catch (e) {
195+
sendInfo("", {
196+
operationName: "java.dependency.upgrade.extensionInstall",
197+
action,
198+
result: "failure",
199+
error: e instanceof Error ? e.message : String(e),
200+
});
201+
throw e;
202+
}
163203
await checkOrPromptToEnableAppModExtension("upgrade");
164204
}

0 commit comments

Comments
 (0)