Add TypeSpec break-check pipeline for ARM SDKs#38604
Draft
wxl534 wants to merge 30 commits into
Draft
Conversation
wxl534
requested review from
a team,
JialinHuang803,
MaryGao,
benbp,
mikeharder,
qiaozha and
weshaggard
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Azure Pipelines workflow to proactively detect TypeSpec emitter regressions for ARM (management-plane) JS SDKs by regenerating packages against a newer @azure-tools/typespec-ts and reporting build/API-surface diffs.
Changes:
- Added a new
typespec-break-checkpipeline that (1) resolves an emitter version, (2) regenerates ARM packages in a matrix, (3) runs build verification, (4) summarizes results, and (optionally) opens an automated PR with generated diffs. - Added two supporting Node scripts: a regeneration/build runner and an API-surface breaking-change detector for
review/*.api.md. - Updated the emitter toolchain inputs (
eng/emitter-package*.json) and enhanced the matrix-splitting script with aMaxPackagescap.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/typespec-break-check.yml | New pipeline wiring for matrix regeneration, build verification, breaking-change detection, and optional PR creation. |
| eng/pipelines/scripts/regenerate-runner.js | New runner to discover ARM packages, run tsp-client regeneration concurrently, and optionally build packages with logging/artifacts. |
| eng/pipelines/scripts/breaking-change-detector.js | New script to diff API Extractor api.md surfaces vs baseline and summarize breaking/potential/additive/moved signals. |
| eng/emitter-package.json | Updates emitter-package dependencies (including a newer @azure-tools/typespec-ts pre-release). |
| eng/emitter-package-lock.json | Regenerated lockfile to match updated emitter-package dependencies. |
| eng/common/scripts/New-RegenerateMatrix.ps1 | Adds -MaxPackages support to limit matrix size. |
Files not reviewed (1)
- eng/emitter-package-lock.json: Language not supported
Comment on lines
+14
to
+21
| paths: | ||
| include: | ||
| - eng/emitter-package.json | ||
| - eng/common/scripts/TypeSpec-* | ||
| - eng/common/tsp-client/ | ||
| - eng/pipelines/typespec-break-check.yml | ||
| - eng/pipelines/scripts/regenerate-runner.js | ||
| - eng/pipelines/scripts/breaking-change-detector.js |
| if [ "$MODE" = "api-md" ]; then | ||
| git diff --binary -- ':(glob)sdk/**/review/*.api.md' > "$PATCH_FILE" | ||
| else | ||
| git diff --binary -- sdk/ > "$PATCH_FILE" |
|
|
||
| const start = Date.now(); | ||
| const buildTimeoutMs = getBuildTimeoutForPackage(pkg.pkg); | ||
| const build = await runCommand("pnpm", ["build", "--filter", filterName], sdkRoot, buildTimeoutMs); |
| "@azure/core-lro", "@azure/logger", "@azure/core-paging" | ||
| ]; | ||
| const coreArgs = ["build"]; | ||
| for (const f of coreFilters) { coreArgs.push("--filter", f); } |
| @@ -0,0 +1,643 @@ | |||
| # eng/pipelines/typespec-break-check.yml | |||
Member
There was a problem hiding this comment.
It would be better to change the filename, for example, to "sdk-regenerate," similar to how the Go pipeline file is named.
|
|
||
| // Per-package timeout overrides (in ms) for known monster packages whose | ||
| // tsp compile / emit takes much longer than the global default. | ||
| const PACKAGE_TIMEOUT_OVERRIDES = { |
Member
There was a problem hiding this comment.
I would prefer to avoid service-specific logic, as it makes the code harder to maintain. Could you clarify why we are setting different timeouts for various packages, or why a timeout is necessary?
| } | ||
|
|
||
| // Fallback: use tsp-location.yaml | ||
| const tspLocationPath = path.join(pkgDir, "tsp-location.yaml"); |
Member
There was a problem hiding this comment.
If a package already contains a tsp-location.yaml file, why not use it to generate the SDK directly? If the tsp-location.yaml is invalid or the SDK generation fails with this file, we could simply display a warning message and move on.
| // of that emitter, preserving the indentation level of its other children. | ||
| // Returns the patched content; returns original if the emitter block can't | ||
| // be located (caller treats that as a no-op + warning). | ||
| function patchTspConfigApiVersion(content, apiVersion) { |
Member
There was a problem hiding this comment.
I am wondering if this method is needed. I recall that we don't have to specify the api-version in the tspconfig.yaml file explicitly to change the api-version. Also, could you try generating the SDKs with tsp-location.yaml using the default api-version (without specifying anything) and check how many SDKs have their api-versions changed? If it's only a few, we may not need this extra step.
| @@ -0,0 +1,839 @@ | |||
| #!/usr/bin/env node | |||
| // Breaking Change Detector for Azure SDK for JS | |||
Member
There was a problem hiding this comment.
Why do we need this detector? There is already logic in @azure-tools/js-sdk-release-tools, which is what populates the "Breaking Changes" sections of every ARM SDK's CHANGELOG.md today. I think the CHANGELOG.md diff in the PR should be sufficient.
wxl534
added a commit
to wxl534/azure-sdk-for-js
that referenced
this pull request
May 27, 2026
…-changelog Comment #3 — Generate via tsp-location.yaml only - Remove buildSpecIndex() and the 7 YAML helpers it used (~160 LOC of spec-repo scanning code in regenerate-runner.js). Each SDK's spec location is now resolved exclusively from its own tsp-location.yaml, matching what mentor (Jialin Huang) asked for: the regeneration contract is "what tsp-location.yaml points at", nothing more. - Add readTspLocation() + resolvePackageFromTspLocation() helpers. - classifyPackages() and classifyFromDirectoryList() now return { packages, skippedNoTspLocation } so packages that lack a usable tsp-location.yaml are explicitly surfaced rather than silently dropped. - Flip the matrix-gen filter from -OnlyTypeSpec false to true. This reuses the existing Test-Path "$_/tsp-location.yaml" check in New-RegenerateMatrix.ps1 (the same filter all .NET emitter pipelines use) so the runner only sees packages that can actually be regenerated. - Add a Setup-stage PowerShell scan that records every arm-* dir without tsp-location.yaml into matrix_artifacts/skipped-no-tsp-location.json, and have the Summary aggregator print them under "Packages skipped — no usable tsp-location.yaml" with an on-boarding callout. This is the list the spec/SDK team needs to act on. Comment #5 — Replace breaking-change-detector.js with update-changelog - Delete eng/pipelines/scripts/breaking-change-detector.js (1839 LOC of custom diff logic). - Use the official @azure-tools/js-sdk-release-tools 'update-changelog' CLI (vendored at eng/tools/js-sdk-release-tools) — same tool the SDK release process uses. Baseline is now the latest npm-published version per package, which is a stricter regression baseline than diffing against git HEAD. - regenerate-runner.js: add installChangelogTool() (runs once per shard) and runUpdateChangelog() (invoked after each successful pnpm build). result.json gains a 'changelog' block with breakingPackages / failedPackages / withChanges counts. - sdk-regenerate.yml aggregator: read result.changelog.* and emit a "Changelog Signal (update-changelog)" section listing breaking and failed packages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wxl534
added a commit
that referenced
this pull request
May 27, 2026
Mentor asked us to verify whether the metadata.json-driven
api-version pinning machinery actually reduces noise in the
generated diff, vs. just emitting against whatever the spec's
tspconfig.yaml declares.
This adds a new pipeline parameter, DisableApiVersionPinning
(default: false), wired through to the runner as
--disableApiVersionPinning. When true:
* runner short-circuits the entire metadata.json lookup
+ tspconfig patching block in processPackage()
* pinClass becomes "disabled" for every package
* result.json apiVersionPinning.disabledMode is true
* Summary stage prints a prominent A/B-TEST MODE banner
instead of the usual pinning coverage breakdown, so it
is obvious which run is which when comparing two PRs.
To run the A/B: trigger the pipeline twice — once with
DisableApiVersionPinning unchecked (current behaviour),
once with it checked. Compare the "Packages with Breaking
Changes" counts in the two Summary outputs. If they are
roughly equal, pinning is not pulling its weight and we
can rip out the metadata.json plumbing in a follow-up.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds eng/pipelines/sdk-regenerate.yml and eng/pipelines/scripts/regenerate-runner.js to regenerate all ARM TypeSpec SDK packages with the latest emitter and surface breaking changes via CHANGELOG.md aggregation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wxl534
force-pushed
the
feature/break-check
branch
from
1d327a7 to
b3764e9
Compare
| - eng/common/tsp-client/ | ||
| - eng/pipelines/sdk-regenerate.yml | ||
| - eng/pipelines/scripts/regenerate-runner.js | ||
| - eng/pipelines/scripts/breaking-change-detector.js |
Member
There was a problem hiding this comment.
Should this line be removed?
Contributor
|
@wxl534 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
| const CHANGELOG_TOOL_DIR = "eng/tools/js-sdk-release-tools"; | ||
| let changelogToolReady = false; | ||
|
|
||
| async function installChangelogTool() { |
Member
There was a problem hiding this comment.
The runner duplicates eng/scripts/update-changelog-content.ps1 (the same wrapper that spec-gen-sdk already calls via eng/swagger_to_sdk_config.json#updateChangelogContentScript). I s it possible to replace the inline npm ci + npm exec in installChangelogTool/runUpdateChangelog with a single pwsh -File eng/scripts/update-changelog-content.ps1 -SdkRepoPath … -PackagePath … call?
| nodeVersion: '22.x' | ||
|
|
||
| stages: | ||
| - stage: BreakCheck |
Member
There was a problem hiding this comment.
I would prefer to rename BreakCheck to another one.
| -OnlyTypeSpec $true ` | ||
| -DirectoryFilterPattern '$(effFilter)' | ||
|
|
||
| Write-Host "===== Scan ARM packages without tsp-location.yaml =====" |
Member
There was a problem hiding this comment.
What are the packages without tsp-location.yaml used for?
Member
|
Since there are so many lines in both files, I am asking Copilot to check if any existing templates or scripts can be used. You may want to leverage the existing code and avoid adding any duplicate code. Duplication / reuse auditI walked through every step in
Per-step duplication / reuse auditLegend: ✅ already reuses existing script · 🟡 reusable but partially duplicated · 🔴 fully reimplements something that exists · 🆕 truly new logic (no existing equivalent) · 🐛 emitter-bug workaround that doesn't belong in this pipeline
Summary table (size impact)
Both files together would land around 400–500 lines, comparable to the Go regen pipeline and the .NET emitter CI. The runner could shrink to ~150 lines of "matrix-shard wrapper that calls Suggested landing order
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Packages impacted by this PR
None (pipeline-only change under
eng/pipelines/).Issues associated with this PR
Related: Azure/autorest.typescript#3916
Describe the problem that is addressed by this PR
Existing smoke tests use fixed sample
.tspfiles, which cannot detect realregressions when
@azure-tools/typespec-tsis updated. We need a pipelinethat regenerates real ARM SDK packages against the latest emitter and
surfaces breaking changes early.
What are the possible designs available to address the problem?
Adds
eng/pipelines/typespec-break-check.ymland supporting scripts:tsp-location.yamlfallback)comparing previous emitter vs current emitter
api.mdreview files)Are there test cases added in this PR?
No — this PR adds a CI pipeline and its supporting scripts; the pipeline
itself is the test harness.
Provide a list of related PRs
N/A
Command used to generate this PR
N/A
Checklists