refactor(fx-core): delegate manifest template resolution to app-manifest

iclanton · iclanton · commit 23a1773dc70b · 2026-07-06T21:57:10.000Z
Now that the resolver lives in @microsoft/app-manifest (reached transitively via
@microsoft/teamsfx-api), remove the duplicated logic from fx-core and delegate:

- component/utils/common.ts re-exports expandEnvironmentVariable and
  getEnvironmentVariables from @microsoft/teamsfx-api, so existing call sites are
  unchanged.
- envFunctionUtils.expandVariableWithFunction delegates its loop to
  expandFileFunctionMacros, keeping only the telemetry event and mapping the
  resolver's plain errors to localized UserError / fx-core FileNotFoundError via
  toFxError. getResolvedManifest is unchanged and keeps composing these primitives.

Update the affected tests to drive resolution through real temp files instead of
fs mocks, since file reads now happen inside app-manifest.
diff --git a/packages/fx-core/src/component/utils/common.ts b/packages/fx-core/src/component/utils/common.ts
@@ -18,8 +18,6 @@ import { getLocalizedString } from "../../common/localizeUtils";
 import { SummaryConstant } from "../configManager/constant";
 import { EOL } from "os";
 
-const placeholderRegex = /\${{ *[a-zA-Z_][a-zA-Z0-9_]* *}}/g;
-
 /**
  * check parameter, throw error if value is null or undefined
  * @param name parameter name
@@ -128,44 +126,10 @@ export async function wrapSummary(
   }
 }
 
-// Expand environment variables in content. The format of referencing environment variable is: ${{ENV_NAME}}
-export function expandEnvironmentVariable(
-  content: string,
-  envs?: { [key in string]: string }
-): string {
-  const placeholders = content.match(placeholderRegex);
-  if (placeholders) {
-    for (const placeholder of placeholders) {
-      const envName = placeholder.slice(3, -2).trim(); // removes `${{` and `}}`
-      const envValue = envs ? envs[envName] : process.env[envName];
-      if (envName === "APP_NAME_SUFFIX") {
-        if (envValue !== undefined && envValue !== null) {
-          content = content.replace(placeholder, envValue);
-        }
-      } else {
-        if (envValue) {
-          content = content.replace(placeholder, envValue);
-        }
-      }
-    }
-  }
-
-  return content;
-}
-
-/**
- * Expand environment variables in content. The format of referencing environment variable is: ${{ENV_NAME}}
- * @return An array of environment variables
- */
-export function getEnvironmentVariables(content: string): string[] {
-  const placeholders = content.match(placeholderRegex);
-  if (placeholders) {
-    const variables = placeholders.map((placeholder) => placeholder.slice(3, -2).trim()); // removes `${{` and `}}`)
-    // remove duplicates
-    return [...new Set(variables)];
-  }
-  return [];
-}
+// Manifest env-variable (`${{ENV_NAME}}`) expansion lives in @microsoft/app-manifest
+// (re-exported via @microsoft/teamsfx-api) so hosts can resolve manifests without
+// fx-core. Re-exported here to keep the existing import path for fx-core call sites.
+export { expandEnvironmentVariable, getEnvironmentVariables } from "@microsoft/teamsfx-api";
 
 export function getAbsolutePath(relativeOrAbsolutePath: string, projectPath: string): string {
   relativeOrAbsolutePath = relativeOrAbsolutePath || "";
diff --git a/packages/fx-core/src/component/utils/envFunctionUtils.ts b/packages/fx-core/src/component/utils/envFunctionUtils.ts
@@ -8,12 +8,18 @@ import {
   Result,
   UserError,
   UserErrorOptions,
+  // Host-agnostic manifest-template resolution lives in @microsoft/app-manifest and
+  // is re-exported by @microsoft/teamsfx-api. fx-core stays the home for telemetry,
+  // localized messages, and FxError mapping; the resolution logic lives one layer down.
+  ManifestType,
+  expandFileFunctionMacros,
+  UnsupportedFileFormatError as ManifestUnsupportedFileFormatError,
+  InvalidFunctionError as ManifestInvalidFunctionError,
+  InvalidFunctionParameterError as ManifestInvalidFunctionParameterError,
+  ReadFileError as ManifestReadFileError,
+  FileNotFoundError as ManifestFileNotFoundError,
 } from "@microsoft/teamsfx-api";
-import path from "path";
-import fs from "fs-extra";
-import stripBom from "strip-bom";
 import { FileNotFoundError } from "../../error";
-import { expandEnvironmentVariable } from "./common";
 import { getLocalizedString } from "../../common/localizeUtils";
 import { DriverContext } from "../driver/interface/commonArgs";
 
@@ -26,145 +32,64 @@ enum TelemetryPropertyKey {
   functionCount = "function-count",
 }
 
-export enum ManifestType {
-  TeamsManifest = "teams-manifest",
-  PluginManifest = "plugin-manifest",
-  DeclarativeCopilotManifest = "declarative-copilot-manifest",
-  ApiSpec = "api-spec",
-  EmbeddedKnowledgeFile = "embedded-knowledge-file",
-}
+// Re-exported for existing importers (ManifestUtils, PluginManifestUtils, utils, createAppPackage).
+export { ManifestType };
 
-export async function expandVariableWithFunction(
-  content: string,
-  ctx: DriverContext,
-  envs: { [key in string]: string } | undefined,
-  isJson: boolean,
-  manifestType: ManifestType,
-  fromPath: string
-): Promise<Result<string, FxError>> {
-  const regex = /\$\[ *[a-zA-Z][a-zA-Z]*\([^\]]*\) *\]/g;
-  const matches = content.match(regex);
-
-  if (!matches) {
-    return ok(content); // no function
-  }
-  let count = 0;
-  for (const placeholder of matches) {
-    const processedRes = await processFunction(
-      placeholder.slice(2, -1).trim(),
-      ctx,
-      envs,
-      fromPath
+// Map a plain error thrown by @microsoft/app-manifest's resolver to the localized
+// FxError surface fx-core drivers expect, emitting the same diagnostic logs as before.
+function toFxError(e: unknown, ctx: DriverContext): FxError {
+  if (e instanceof ManifestUnsupportedFileFormatError) {
+    ctx.logProvider.error(
+      getLocalizedString("core.envFunc.unsupportedFile.errorLog", e.filePath, "txt")
     );
-    if (processedRes.isErr()) {
-      return err(processedRes.error);
-    }
-    let value = processedRes.value;
-    if (isJson && value) {
-      value = JSON.stringify(value).slice(1, -1);
-    }
-    if (value) {
-      count += 1;
-      content = content.replace(placeholder, value);
-    }
-  }
-
-  if (count > 0) {
-    ctx.telemetryReporter.sendTelemetryEvent(telemetryEvent, {
-      [TelemetryPropertyKey.manifestType]: manifestType.toString(),
-      [TelemetryPropertyKey.functionCount]: count.toString(),
-    });
+    return new UnsupportedFileFormatError(ctx.platform);
   }
-  return ok(content);
-}
-
-async function processFunction(
-  content: string,
-  ctx: DriverContext,
-  envs: { [key in string]: string } | undefined,
-  path: string
-): Promise<Result<string, FxError>> {
-  const firstTrimmedContent = content.trim();
-  if (!firstTrimmedContent.startsWith("file(") || !firstTrimmedContent.endsWith(")")) {
+  if (e instanceof ManifestInvalidFunctionError) {
     ctx.logProvider.error(
-      getLocalizedString("core.envFunc.unsupportedFunction.errorLog", firstTrimmedContent, "file")
+      getLocalizedString("core.envFunc.unsupportedFunction.errorLog", e.token, "file")
     );
-    return err(new InvalidFunctionError(ctx.platform));
+    return new InvalidFunctionError(ctx.platform);
   }
-
-  // file()
-  const trimmedParameter = content.slice(5, -1).trim();
-  if (trimmedParameter[0] === "'" && trimmedParameter[trimmedParameter.length - 1] === "'") {
-    // static string as function parameter
-    const res = await readFileContent(
-      trimmedParameter.substring(1, trimmedParameter.length - 1),
-      ctx,
-      envs,
-      path
+  if (e instanceof ManifestInvalidFunctionParameterError) {
+    ctx.logProvider.error(
+      getLocalizedString("core.envFunc.invalidFunctionParameter.errorLog", e.token, "file")
     );
-    return res;
-  } else if (trimmedParameter.startsWith("${{") && trimmedParameter.endsWith("}}")) {
-    // env variable inside
-    const resolvedParameter = expandEnvironmentVariable(trimmedParameter, envs);
-
-    const res = readFileContent(resolvedParameter, ctx, envs, path);
-    return res;
-  } else if (trimmedParameter.startsWith("file(") && trimmedParameter.endsWith(")")) {
-    // nested function inside
-    const processsedRes = await processFunction(trimmedParameter, ctx, envs, path);
-
-    if (processsedRes.isErr()) {
-      return err(processsedRes.error);
-    }
-
-    const readFileRes = await readFileContent(processsedRes.value, ctx, envs, path);
-    return readFileRes;
-  } else {
-    // invalid content inside function
+    return new InvalidFunctionParameter(ctx.platform);
+  }
+  if (e instanceof ManifestReadFileError) {
     ctx.logProvider.error(
-      getLocalizedString("core.envFunc.invalidFunctionParameter.errorLog", trimmedParameter, "file")
+      getLocalizedString("core.envFunc.readFile.errorLog", e.filePath, e.cause?.toString())
     );
-    return err(new InvalidFunctionParameter(ctx.platform));
+    return new ReadFileError(ctx.platform, e.filePath);
   }
+  if (e instanceof ManifestFileNotFoundError) {
+    return new FileNotFoundError(source, e.filePath);
+  }
+  throw e;
 }
 
-async function readFileContent(
-  filePath: string,
+export async function expandVariableWithFunction(
+  content: string,
   ctx: DriverContext,
   envs: { [key in string]: string } | undefined,
+  isJson: boolean,
+  manifestType: ManifestType,
   fromPath: string
 ): Promise<Result<string, FxError>> {
-  const ext = path.extname(filePath);
-  if (ext.toLowerCase() !== ".txt" && ext.toLowerCase() !== ".md") {
-    ctx.logProvider.error(
-      getLocalizedString("core.envFunc.unsupportedFile.errorLog", filePath, "txt")
-    );
-    return err(new UnsupportedFileFormatError(ctx.platform));
+  let resolved: { content: string; functionCount: number };
+  try {
+    resolved = await expandFileFunctionMacros(content, isJson, { envs, fromPath });
+  } catch (e) {
+    return err(toFxError(e, ctx));
   }
 
-  const absolutePath = getAbsolutePath(filePath, fromPath);
-  if (await fs.pathExists(absolutePath)) {
-    try {
-      let fileContent = await fs.readFile(absolutePath, "utf8");
-      fileContent = stripBom(fileContent);
-      let processedFileContent = expandEnvironmentVariable(fileContent, envs);
-      processedFileContent = processedFileContent.replace(/\r\n/g, "\n");
-      return ok(processedFileContent);
-    } catch (e) {
-      ctx.logProvider.error(
-        getLocalizedString("core.envFunc.readFile.errorLog", absolutePath, e?.toString())
-      );
-      return err(new ReadFileError(ctx.platform, absolutePath));
-    }
-  } else {
-    return err(new FileNotFoundError(source, filePath));
+  if (resolved.functionCount > 0) {
+    ctx.telemetryReporter.sendTelemetryEvent(telemetryEvent, {
+      [TelemetryPropertyKey.manifestType]: manifestType.toString(),
+      [TelemetryPropertyKey.functionCount]: resolved.functionCount.toString(),
+    });
   }
-}
-
-function getAbsolutePath(relativeOrAbsolutePath: string, fromPath: string): string {
-  return path.isAbsolute(relativeOrAbsolutePath)
-    ? relativeOrAbsolutePath
-    : path.join(path.dirname(fromPath), relativeOrAbsolutePath);
+  return ok(resolved.content);
 }
 
 class UnsupportedFileFormatError extends UserError {
diff --git a/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts b/packages/fx-core/tests/component/driver/teamsApp/manifestUtils.test.ts
@@ -10,6 +10,7 @@ import {
 } from "@microsoft/teamsfx-api";
 import fs from "fs-extra";
 import mockedEnv, { RestoreFn } from "mocked-env";
+import os from "os";
 import path from "path";
 import { assert, vi } from "vitest";
 import {
@@ -527,13 +528,18 @@ describe("trimManifestShortName", () => {
 describe("resolveLocFile", () => {
   const sandbox = vi;
   let mockedEnvRestore: RestoreFn;
+  let tmpDir: string | undefined;
 
-  afterEach(() => {
+  afterEach(async () => {
     if (mockedEnvRestore) {
       mockedEnvRestore();
     }
     vi.restoreAllMocks();
     vi.restoreAllMocks();
+    if (tmpDir) {
+      await fs.remove(tmpDir);
+      tmpDir = undefined;
+    }
   });
 
   it("returns error when loc file doesn't exist", async () => {
@@ -582,21 +588,17 @@ describe("resolveLocFile", () => {
   });
 
   it("resolves $[file(...)] when context is provided", async () => {
-    vi.spyOn(fs, "pathExists").mockImplementation(async (filePath) => {
-      return filePath === "loc_file_path" || filePath === "instruction.txt";
-    });
-    vi.spyOn(fs, "readFile").mockImplementation(((filePath: number | fs.PathLike) => {
-      if (filePath === "loc_file_path") {
-        return Promise.resolve(
-          JSON.stringify({
-            name: {
-              short: "$[file('instruction.txt')]",
-            },
-          })
-        );
-      }
-      return Promise.resolve("localized short name");
-    }) as any);
+    tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "resolveloc-"));
+    const locFilePath = path.join(tmpDir, "loc_file.json");
+    await fs.writeFile(
+      locFilePath,
+      JSON.stringify({
+        name: {
+          short: "$[file('instruction.txt')]",
+        },
+      })
+    );
+    await fs.writeFile(path.join(tmpDir, "instruction.txt"), "localized short name");
 
     const context: any = {
       platform: Platform.VSCode,
@@ -608,7 +610,7 @@ describe("resolveLocFile", () => {
       },
     };
 
-    const locFile = await manifestUtils.resolveLocFile("loc_file_path", context);
+    const locFile = await manifestUtils.resolveLocFile(locFilePath, context);
 
     assert.isTrue(locFile.isOk());
     if (locFile.isOk()) {
diff --git a/packages/fx-core/tests/component/util/envFunctionUtils.test.ts b/packages/fx-core/tests/component/util/envFunctionUtils.test.ts
