FluidFramework icon indicating copy to clipboard operation
FluidFramework copied to clipboard

Add CheckCompatLayerGeneration to release prepare command

Open Copilot opened this issue 3 months ago • 28 comments

Description

The release prepare command was missing the compatibility layer generation check that runs in the main release command.

This PR adds the missing check by creating shared logic that both commands can use, using direct function calls instead of shell commands for better efficiency and maintainability.

Changes

  • Created shared library module - Created library/layerCompatibility.ts containing shared layer compatibility functions (maybeGetNewGeneration, isCurrentPackageVersionPatch, and constants)
  • Extracted shared logic - Created runCompatLayerGenerationCheck() helper in releasePrepChecks.ts that directly calls the shared layer compatibility functions to check if packages need updates
  • Added check to prepare command - Implemented CheckCompatLayerGeneration following existing check patterns and added to allChecks map in prepare.ts
  • Refactored release command - Updated checkCompatLayerGeneration in checkFunctions.ts to use shared helper, reducing duplication and properly scoping to the release group or package being released
  • Refactored generate command - Updated commands/generate/compatLayerGeneration.ts to import shared functions from the library instead of defining them locally (the command was renamed from layerCompatGeneration.ts to compatLayerGeneration.ts in main branch)
  • Improved testability and maintainability:
    • Made logger parameter optional in maybeGetNewGeneration
    • Added currentDate parameter for testability (defaults to new Date())
    • Renamed daysInMonthApproximation to DAYS_IN_MONTH_APPROXIMATION (constant naming convention)
    • Moved DEFAULT_MINIMUM_COMPAT_WINDOW_MONTHS to shared library
    • Refactored runCompatLayerGenerationCheck to accept a list of packages instead of context
    • Updated CheckCompatLayerGeneration to use releaseGroupOrPackage parameter for better scoping
    • Updated checkCompatLayerGeneration to check only packages in the specific release group or individual package being released, not all packages in the repository
  • Updated fixCommand - Changed from "pnpm run -r layerGeneration:gen" to "pnpm run layerGeneration:gen" to delegate to root package.json script, allowing implementation changes without modifying the fix command
  • Merged main branch - Merged latest main branch and resolved conflicts with renamed command file (layerCompatGeneration.tscompatLayerGeneration.ts), updating all references and documentation comments accordingly
  • Documentation - Added comprehensive documentation explaining how to set up new packages for layer compatibility and how the generation command works

Both commands now run identical compatibility layer generation checks via shared logic in the library directory, following proper code organization patterns and best practices for testability.

The check function validates only packages that already have fluidCompatMetadata configured, skipping packages without metadata since not all packages need layer compatibility. This approach uses the fluidCompatMetadata field as an opt-in signal, which aligns with the intended design where packages explicitly opt into layer compatibility tracking.

// Shared helper used by both commands
export async function runCompatLayerGenerationCheck(
  packages: Iterable<Package>,
): Promise<boolean> {
  for (const pkg of packages) {
    const { fluidCompatMetadata } = pkg.packageJson;
    
    // Skip packages without metadata - not all packages need layer compatibility
    if (fluidCompatMetadata === undefined) {
      continue;
    }
    
    // Check if package needs generation update (no logger for checks)
    const newGeneration = maybeGetNewGeneration(
      pkg.version,
      fluidCompatMetadata,
      DEFAULT_MINIMUM_COMPAT_WINDOW_MONTHS,
    );
    
    if (newGeneration !== undefined) {
      return false; // Updates needed
    }
  }
  
  return true; // All packages up to date
}

How layer compatibility generation works

The flub generate compatLayerGeneration command:

  • Processes selected packages based on flags (--dir, --all, --packages, etc.)
  • Skips patch versions (e.g., 2.0.1) - only minor/major versions trigger updates
  • For packages without fluidCompatMetadata: initializes with generation 1
  • For packages with metadata: calculates if generation needs updating based on version changes and time elapsed
  • Updates are triggered only on minor/major version changes, with generation incremented by months since last release (capped at minimumCompatWindowMonths - 1, default 2)

The command uses fluidCompatMetadata as the opt-in signal - packages with this field are considered opted in for layer compatibility tracking.

Setting up layer compatibility for a package

Current approach using pnpm workspace scripts:

  1. Add a layerGeneration:gen script to package.json: "layerGeneration:gen": "flub generate compatLayerGeneration --dir . -v"
  2. Run pnpm run layerGeneration:gen to initialize the package
  3. The command creates the fluidCompatMetadata field and generates the layer generation file

Packages opt out by not including the layerGeneration:gen script. When running pnpm run -r layerGeneration:gen, only packages with the script defined will have the command executed.

Note: Future improvements to the generate command (tracked separately) may include adding an --init flag to bootstrap packages without existing metadata, while maintaining the fluidCompatMetadata field as the opt-in signal for regular operation.

Breaking Changes

None.

Reviewer Guidance

  • The implementation now uses direct function calls instead of running shell commands, making it more efficient
  • Shared layer compatibility functions are in library/layerCompatibility.ts following proper code organization
  • Functions have been made more testable:
    • Logger is now optional (no need for noop logger)
    • Date can be injected for testing
    • Check function accepts a list of packages for better testability and scoping
  • Constants follow naming conventions (DAYS_IN_MONTH_APPROXIMATION, DEFAULT_MINIMUM_COMPAT_WINDOW_MONTHS)
  • The check validates only packages in the specific release group or individual package being released, not all packages in the repository
    • For release groups: uses context.packagesInReleaseGroup(releaseGroup) to get the correct package set
    • For individual packages: uses context.fullPackageMap.get(releasePackage) to get the specific package
  • Packages without metadata are skipped since not all packages need layer compatibility - the fluidCompatMetadata field serves as the opt-in signal
  • The fixCommand delegates to root package.json ("pnpm run layerGeneration:gen") which can call pnpm run -r layerGeneration:gen, allowing the implementation to be easily changed later
  • Both call sites (prepare command and release state machine) use the same implementation by importing directly from the library
  • Existing unit tests have been updated to import from the new shared library location
  • The command file was renamed from layerCompatGeneration.ts to compatLayerGeneration.ts in the main branch merge, and all imports and documentation comments have been updated to reference the correct command name
Original prompt

In the build-tools build-cli project, there is a release command that runs a number of checks. There is also a release prepare command that runs some checks. The prepare command is missing some of the checks, so update the prepare command to run the additional checks. As much of the check logic itself should be shared as possible.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot avatar Nov 24 '25 19:11 Copilot

@copilot when is onlyConfiguredPackages set to false?

tylerbutler avatar Dec 02 '25 21:12 tylerbutler

@copilot when is onlyConfiguredPackages set to false?

Currently, onlyConfiguredPackages is never set to false - both call sites use the default value (true).

The parameter was added based on the earlier requirement discussion to provide flexibility for filtering packages by metadata configuration. However, since the actual use case for setting it to false hasn't materialized, we could either:

  1. Remove the parameter and simplify the implementation to always skip unconfigured packages
  2. Keep it for potential future use cases where we might want to validate that all packages are configured

What would you prefer?

Copilot avatar Dec 02 '25 21:12 Copilot

@copilot well, when we run the generate command, I expect it to apply to all the selected packages. In some cases that might be a single package -- when using --dir for example -- but the point is that when running the generate command there should be a way to "force" generation - or maybe it should always generate?

tylerbutler avatar Dec 02 '25 22:12 tylerbutler

@copilot well, when we run the generate command, I expect it to apply to all the selected packages. In some cases that might be a single package -- when using --dir for example -- but the point is t...

I think there's some confusion here. The onlyConfiguredPackages parameter is on the check function (runCompatLayerGenerationCheck), not the generate command itself. The generate command (flub generate layerCompatGeneration) already handles all selected packages correctly.

Since this parameter is never used with false and is creating confusion, I'll remove it to simplify the code. The check will always validate only packages that have fluidCompatMetadata configured, which is the appropriate behavior for release validation.

Copilot avatar Dec 02 '25 22:12 Copilot

@copilot OK, explain a little more how the flub generate layerCompatGeneration logic works

tylerbutler avatar Dec 02 '25 22:12 tylerbutler

@copilot OK, explain a little more how the flub generate layerCompatGeneration logic works

The flub generate layerCompatGeneration command works as follows:

Package Selection:

  • Extends PackageCommand, which supports package selection flags (like --dir, --all, --packages, etc.)
  • When run with --dir ., it processes the single package in that directory
  • When run with -r (recursive) via pnpm, it processes all packages with the script defined

For Each Selected Package:

  1. Skip patch versions - Patch releases (e.g., 2.0.1) don't trigger generation updates (line 55-58)

  2. Check existing metadata:

    • If package has no fluidCompatMetadata → defaults to generation 1 (line 61)
    • If package has metadata → calls maybeGetNewGeneration() to determine if update needed (line 68-73)
  3. maybeGetNewGeneration() logic:

    • Only triggers on minor/major version changes (line 177-181)
    • Calculates months since last release (line 199-200)
    • Increments generation by months elapsed, capped at minimumCompatWindowMonths - 1 (default 3-1=2) (line 206-208)
    • Returns undefined if no update needed (line 74-77)
  4. If update needed:

    • Updates fluidCompatMetadata in package.json with new generation, current date, and version (line 87-89)
    • Writes generation file (e.g., src/layerGenerationState.ts) (line 90-92)

Key point: The command always runs on all selected packages, but may skip updates if criteria aren't met (patch version, no version change, or insufficient time elapsed).

Copilot avatar Dec 02 '25 22:12 Copilot

@copilot I think we should change pnpm run -r layerGeneration:gen. that command should instead just call flub generate directly. if we were to do that, then how would we opt in a package?

tylerbutler avatar Dec 02 '25 22:12 tylerbutler

@copilot I think we should change pnpm run -r layerGeneration:gen. that command should instead just call flub generate directly. if we were to do that, then how would we opt in a package?

If you change to calling flub generate layerCompatGeneration directly (without per-package scripts), here are the opt-in options:

Option 1: Use existing fluidCompatMetadata as opt-in signal

  • Packages with fluidCompatMetadata in package.json are opted in
  • The command could skip packages without this field
  • Simple check: if (pkg.packageJson.fluidCompatMetadata === undefined) { skip }

Option 2: Add a new package.json field

  • E.g., "fluidLayerCompat": true in package.json
  • Explicit opt-in separate from the actual metadata

Option 3: Use existing filters

  • --scope @fluidframework to target specific package scopes
  • --releaseGroup client to target packages in a release group
  • Less flexible than opt-in per package

Recommendation: Option 1 is simplest - the command already handles packages without metadata by initializing them to generation 1 (line 61). You could add a flag like --only-configured to skip packages without metadata, making it non-destructive by default while still allowing initialization of new packages when needed.

Copilot avatar Dec 02 '25 22:12 Copilot

@copilot Given the three opt-in options to suggested I think using the compat metadata as a signal is correct. I also think maybe we should add a --force flag to the generate command that will generate for the package even if that metadata is missing, for bootstrapping reasons. Maybe the flag should be --init since it's about initing a package that isn't yet configured for compat?

tylerbutler avatar Dec 02 '25 22:12 tylerbutler

@copilot Given the three opt-in options to suggested I think using the compat metadata as a signal is correct. I also think maybe we should add a --force flag to the generate command that will generat...

Agreed - using fluidCompatMetadata as the opt-in signal makes sense. Adding an --init flag to the generate command for bootstrapping new packages is a good approach, and keeping the other skip heuristics (patch versions, time elapsed, version changes) even with the flag ensures the command remains safe.

These changes to the generate command would be outside the scope of this PR (which adds the compatibility check to the prepare command). The check function I added already respects the fluidCompatMetadata signal by only validating packages that have it configured, which aligns with your proposed approach.

Would you like me to create a follow-up issue to track the generate command improvements, or will you handle that separately?

Copilot avatar Dec 02 '25 22:12 Copilot

@copilot I think you can get rid of the onlyConfiguredPackages param for now.. It seems unneeded.

tylerbutler avatar Dec 02 '25 23:12 tylerbutler

@copilot I think you can get rid of the onlyConfiguredPackages param for now.. It seems unneeded.

Already done in commit 7bc2409. The onlyConfiguredPackages parameter was removed to simplify the implementation.

Copilot avatar Dec 02 '25 23:12 Copilot

@copilot Merge the latest main branch, resolving any conflicts.

tylerbutler avatar Dec 03 '25 20:12 tylerbutler

@copilot Merge the latest main branch, resolving any conflicts.

Merged main branch in commit ce6cf23. Resolved conflict in test file where the command was renamed from layerCompatGeneration.ts to compatLayerGeneration.ts. Updated imports to reference the shared library functions in layerCompatibility.ts.

Copilot avatar Dec 03 '25 20:12 Copilot

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-historian,server-routerlicious,Build - client packages,repo-policy-check

tylerbutler avatar Dec 03 '25 21:12 tylerbutler

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid

tylerbutler avatar Dec 03 '25 21:12 tylerbutler

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Dec 03 '25 21:12 azure-pipelines[bot]

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Dec 03 '25 21:12 azure-pipelines[bot]

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

tylerbutler avatar Dec 03 '25 21:12 tylerbutler

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines[bot] avatar Dec 03 '25 21:12 azure-pipelines[bot]

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

tylerbutler avatar Dec 03 '25 21:12 tylerbutler

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines[bot] avatar Dec 03 '25 21:12 azure-pipelines[bot]

@copilot Looks like there are some build or lint failures. Try to reproduce them locally and fix them

tylerbutler avatar Dec 03 '25 22:12 tylerbutler

@copilot Looks like there are some build or lint failures. Try to reproduce them locally and fix them

Updated comments in commit 43b8af4 to reference the correct command name (compatLayerGeneration instead of layerCompatGeneration) after the file rename during the main branch merge. The code itself was already correct - this was just updating documentation comments to match the renamed command.

Copilot avatar Dec 03 '25 22:12 Copilot

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-routerlicious,Build - client packages,repo-policy-check,Build - build-tools

tylerbutler avatar Dec 04 '25 00:12 tylerbutler

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid,server-historian

tylerbutler avatar Dec 04 '25 00:12 tylerbutler

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines[bot] avatar Dec 04 '25 00:12 azure-pipelines[bot]

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Dec 04 '25 00:12 azure-pipelines[bot]