fix(extensions): gate experiments in before hooks, prompt for upgrades, and default uninstall to false in ext:migrate

src/commands/ext-migrate.ts

@@ -2,6 +2,7 @@ import * as clc from "colorette";
 import { checkMinRequiredVersion } from "../checkMinRequiredVersion";
 import { Command } from "../command";
 import { needProjectId } from "../projectUtils";
+import * as experiments from "../experiments";
 import {
   ensureExtensionsApiEnabled,
   ensureInstanceSpec,
@@ -37,6 +38,14 @@ export const command = new Command("ext:migrate")
   .option("--ext-instance <instanceId>", "extension instance ID to migrate")
   .option("-e, --extension <extensionRef>", "extension reference or name to migrate")
   .option("-f, --force", "force update and migration without prompting")
+  .before(() => {
+    experiments.assertEnabled(
+      "extMigrationFeatures",
+      "migrate an extension instance to a function kit",
+    );
+    experiments.assertEnabled("kits", "migrate an extension instance to a function kit");
+    experiments.assertEnabled("secretEnvParams", "migrate an extension instance to a function kit");
+  })
   .before(requireConfig)
   .before(requirePermissions, [
     "firebaseextensions.instances.list",
@@ -67,6 +76,13 @@ export const command = new Command("ext:migrate")
       `Selected instance ${clc.bold(plan.instanceId)} (${plan.kitPackage}) for migration.`,
     );
 
+    plan.instance = await ensureInstanceSpec(plan.instance);
+    if (!plan.instance.config?.source?.spec) {
+      throw new FirebaseError(
+        `Could not load extension specification for ${clc.bold(plan.instanceId)}. Unable to export configuration.`,
+      );
+    }
+
     plan.instance = await ensureInstanceUpToDate(projectId, plan.instance, options);
 
     if (plan.instance.state !== "ACTIVE") {
@@ -76,13 +92,6 @@ export const command = new Command("ext:migrate")
       );
     }
 
-    plan.instance = await ensureInstanceSpec(plan.instance);
-    if (!plan.instance.config?.source?.spec) {
-      throw new FirebaseError(
-        `Could not load extension specification for ${clc.bold(plan.instanceId)}. Unable to export configuration.`,
-      );
-    }
-
     const exportedEnvs = functionsEnvFromInstance(plan.instance);
 
     await migrateSecrets(plan.instance, { force: options.force });
@@ -129,7 +138,7 @@ export const command = new Command("ext:migrate")
 
     const shouldUninstall = await confirm({
       message: `Functions kit ${kitInstanceId} successfully deployed. After checking function logs to verify that your backend is performing correctly, you should uninstall extension instance ${plan.instanceId}. Uninstall it now?`,
-      default: true,
+      default: false,
       nonInteractive: options.nonInteractive,
       force: options.force,
     });

src/commands/functions-kits-install.spec.ts

@@ -32,29 +32,23 @@ describe("functions:kits:install", () => {
   });
 
   describe("command configuration", () => {
-    it("should have requireConfig and requireAuth as before hooks", () => {
-      expect(originalBefores).to.deep.equal([
-        { fn: requireConfig, args: [] },
-        { fn: requireAuth, args: [] },
-      ]);
+    it("should assert kits experiment and require config/auth in before hooks", () => {
+      expect(originalBefores).to.have.lengthOf(3);
+      originalBefores[0].fn();
+      expect(assertEnabledStub).to.have.been.calledWith("kits", "install a function kit");
+      expect(originalBefores[1]).to.deep.equal({ fn: requireConfig, args: [] });
+      expect(originalBefores[2]).to.deep.equal({ fn: requireAuth, args: [] });
     });
-  });
 
-  describe("command action", () => {
-    it("should assert that kits experiment is enabled", async () => {
+    it("should fail if kits experiment is disabled", () => {
       assertEnabledStub.throws(new FirebaseError("kits experiment disabled"));
-
-      await expect(
-        command.runner()({
-          package: "@firebase-function-kits/firestore-bigquery-export",
-          cwd: "/mock/project",
-          nonInteractive: true,
-        }),
-      ).to.be.rejectedWith(FirebaseError, "kits experiment disabled");
-
-      expect(assertEnabledStub).to.have.been.calledWith("kits", "install a function kit");
+      expect(() => {
+        originalBefores[0].fn();
+      }).to.throw(FirebaseError, "kits experiment disabled");
     });
+  });
 
+  describe("command action", () => {
     it("should throw an error if not in a Firebase project directory", async () => {
       await expect(
         command.runner()({

src/commands/functions-kits-install.ts

@@ -15,6 +15,9 @@ export interface FunctionsKitsInstallOptions extends Options {
 
 export const command = new Command("functions:kits:install")
   .description("install a function kit into your project")
+  .before(() => {
+    experiments.assertEnabled("kits", "install a function kit");
+  })
   .before(requireConfig)
   .before(requireAuth)
   .withForce()
@@ -29,8 +32,6 @@ export const command = new Command("functions:kits:install")
   )
   .option("--no-configure", "skip parameter prompting and configuration during installation")
   .action(async (options: FunctionsKitsInstallOptions): Promise<void> => {
-    experiments.assertEnabled("kits", "install a function kit");
-
     if (!options.config) {
       throw new FirebaseError("Not in a Firebase project directory (firebase.json not found).");
     }

src/extensions/migrate.spec.ts

@@ -358,6 +358,12 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
     });
   });
   describe("ensureInstanceUpToDate", () => {
+    let confirmStub: sinon.SinonStub;
+
+    beforeEach(() => {
+      confirmStub = sandbox.stub(prompt, "confirm").resolves(true);
+    });
+
     it("should return original instance when instance is already up to date", async () => {
       sandbox.stub(extensionsApi, "getExtensionVersion").resolves({
         name: "firebase/firestore-send-email@0.1.14",
@@ -368,9 +374,10 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
       const updated = await migrateModule.ensureInstanceUpToDate("test-project", mockInstance1);
 
       expect(updated).to.equal(mockInstance1);
+      expect(confirmStub).to.not.have.been.called;
     });
 
-    it("should automatically attempt upgrade when a newer version exists", async () => {
+    it("should prompt user and upgrade when a newer version exists and user confirms", async () => {
       sandbox.stub(extensionsApi, "getExtension").resolves({
         latestVersion: "0.1.15",
       } as unknown as Extension);
@@ -384,9 +391,79 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
 
       await migrateModule.ensureInstanceUpToDate("test-project", mockInstance1);
 
+      expect(confirmStub).to.have.been.calledWithMatch({
+        message: sinon.match(/on version 0.1.18, but the latest version is 0.1.15/),
+        default: true,
+      });
       expect(getExtVersionStub).to.have.been.called;
     });
 
+    it("should continue with current version if user declines upgrade", async () => {
+      confirmStub.resolves(false);
+      sandbox.stub(extensionsApi, "getExtension").resolves({
+        latestVersion: "0.1.19",
+      } as unknown as Extension);
+      const updateSpy = sandbox.spy(updateHelper, "update");
+      const warnSpy = sandbox.spy(utils, "logLabeledWarning");
+
+      const result = await migrateModule.ensureInstanceUpToDate("test-project", mockInstance1);
+
+      expect(result).to.equal(mockInstance1);
+      expect(updateSpy).to.not.have.been.called;
+      expect(warnSpy).to.have.been.calledWithMatch(
+        "extensions",
+        /Continuing migration with extension instance email-1 on outdated version 0\.1\.18\./,
+      );
+    });
+
+    it("should pass force option to confirm prompt when --force is specified", async () => {
+      sandbox.stub(extensionsApi, "getExtension").resolves({
+        latestVersion: "0.1.15",
+      } as unknown as Extension);
+      sandbox.stub(extensionsApi, "getExtensionVersion").resolves({
+        name: "firebase/firestore-send-email@0.1.15",
+        ref: "firebase/firestore-send-email@0.1.15",
+        spec: { name: "firestore-send-email", version: "0.1.15", params: [] },
+      } as unknown as ExtensionVersion);
+      sandbox.stub(updateHelper, "update").resolves({} as unknown as ExtensionInstance);
+      sandbox.stub(extensionsApi, "getInstance").resolves(mockInstance1);
+
+      await migrateModule.ensureInstanceUpToDate("test-project", mockInstance1, {
+        force: true,
+      });
+
+      expect(confirmStub).to.have.been.calledWithMatch({
+        force: true,
+      });
+    });
+
+    it("should resolve currentVersion from instance.config.extensionVersion if spec version is missing", async () => {
+      const instanceWithoutSpec: ExtensionInstance = {
+        ...mockInstance1,
+        config: {
+          ...mockInstance1.config,
+          extensionVersion: "0.1.14",
+          source: undefined,
+        },
+      };
+      sandbox.stub(extensionsApi, "getExtension").resolves({
+        latestVersion: "0.1.15",
+      } as unknown as Extension);
+      sandbox.stub(extensionsApi, "getExtensionVersion").resolves({
+        name: "firebase/firestore-send-email@0.1.15",
+        ref: "firebase/firestore-send-email@0.1.15",
+        spec: { name: "firestore-send-email", version: "0.1.15", params: [] },
+      } as unknown as ExtensionVersion);
+      sandbox.stub(updateHelper, "update").resolves({} as unknown as ExtensionInstance);
+      sandbox.stub(extensionsApi, "getInstance").resolves(mockInstance1);
+
+      await migrateModule.ensureInstanceUpToDate("test-project", instanceWithoutSpec);
+
+      expect(confirmStub).to.have.been.calledWithMatch({
+        message: sinon.match(/on version 0.1.14, but the latest version is 0.1.15/),
+      });
+    });
+
     it("should merge systemParams into currentParams when prompting for new parameters", async () => {
       sandbox.stub(extensionsApi, "getExtension").resolves({
         latestVersion: "0.1.15",
@@ -441,7 +518,7 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
     });
 
     it("should prompt user when extension reference cannot be parsed and throw if user declines", async () => {
-      sandbox.stub(prompt, "confirm").resolves(false);
+      confirmStub.resolves(false);
       const invalidRefInstance = {
         ...mockInstance1,
         config: { ...mockInstance1.config, extensionRef: "invalid-ref-format" },
@@ -453,7 +530,7 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
     });
 
     it("should prompt user when extension reference cannot be parsed and continue if user accepts", async () => {
-      sandbox.stub(prompt, "confirm").resolves(true);
+      confirmStub.resolves(true);
       const invalidRefInstance = {
         ...mockInstance1,
         config: { ...mockInstance1.config, extensionRef: "invalid-ref-format" },
@@ -707,7 +784,7 @@ describe("ext:migrate core logic (Unique Veneer)", () => {
           message: sinon.match(
             /Functions kit email-1 successfully deployed.*uninstall extension instance email-1/,
           ),
-          default: true,
+          default: false,
         }),
       );
 

src/extensions/migrate.ts

@@ -10,7 +10,7 @@ import {
   logLabeledSuccess,
   logLabeledWarning,
 } from "../utils";
-import { logPrefix } from "./extensionsHelper";
+import { ensureInstanceSpec, logPrefix } from "./extensionsHelper";
 import { confirm, select } from "../prompt";
 import * as extensionsApi from "./extensionsApi";
 import * as refs from "./refs";
@@ -347,7 +347,11 @@ export async function ensureInstanceUpToDate(
   try {
     const parsed = refs.parse(rawRef);
     baseRef = refs.toExtensionRef(parsed);
-    currentVersion = parsed.version || instance.config.source?.spec?.version;
+    currentVersion =
+      parsed.version ||
+      instance.config?.source?.spec?.version ||
+      instance.config?.extensionVersion ||
+      instance.extensionVersion;
   } catch (err: unknown) {
     logger.debug(`[ensureInstanceUpToDate] Could not parse extension reference '${rawRef}':`, err);
     logLabeledWarning(
@@ -375,6 +379,20 @@ export async function ensureInstanceUpToDate(
     return instance;
   }
 
+  const shouldUpgrade = await confirm({
+    message: `Extension instance ${clc.bold(instanceId)} is on version ${clc.bold(currentVersion)}, but the latest version is ${clc.bold(latestVersion)}. An upgrade is strongly recommended before migrating to avoid breaking changes. Upgrade it now?`,
+    default: true,
+    nonInteractive: options?.nonInteractive,
+    force: options?.force,
+  });
+  if (!shouldUpgrade) {
+    logLabeledWarning(
+      logPrefix,
+      `Continuing migration with extension instance ${clc.bold(instanceId)} on outdated version ${clc.bold(currentVersion)}.`,
+    );
+    return instance;
+  }
+
   logLabeledBullet(
     logPrefix,
     `Upgrading extension instance ${clc.bold(instanceId)} from version ${clc.bold(currentVersion)} to ${clc.bold(latestVersion)} to ensure a smooth migration...`,
@@ -439,7 +457,7 @@ export async function ensureInstanceUpToDate(
   }
 
   const updatedInstance = await extensionsApi.getInstance(projectId, instanceId);
-  return updatedInstance ?? instance;
+  return updatedInstance ? await ensureInstanceSpec(updatedInstance) : instance;
 }
 
 /**