You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Merged main and regenerated the PlatformValidation lockfile importer with pnpm 11.24.0 to match the shared Vitest 5 catalog. Other importers and dependency snapshots are unchanged from main.
Local validation: frozen-lockfile install for the package dependency closure, targeted Turborepo build (20 packages), formatting check, TypeScript check, and Node/Chromium playback tests passed. The package lint script is intentionally a no-op. No live Azure tests were run.
Azure Pipelines:
Successfully started running 2 pipeline(s).
9 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
Adds a new Azure Resource Manager (management-plane) JavaScript/TypeScript SDK package for PlatformValidation, including build/CI wiring, generated client APIs (layered + “classic” operation groups), and runnable samples/tests.
In getNonExpandedValue, the ifEmpty handling is inverted. For operators ?/&, ifEmpty is \"=\", and the current condition (if (!ifEmpty)) prevents appending = for empty values, producing ?var instead of ?var=. Change the condition to append ifEmpty when it is non-empty (e.g., if (ifEmpty) { vals.push(ifEmpty); }). sdk/platformvalidation/arm-platformvalidation/src/static-helpers/urlTemplate.ts:1
The comment and implementation disagree with RFC 3986: unreserved characters include ALPHA / DIGIT / "-" / "." / "_" / "~" (underscore is missing, and alphanumerics are also unreserved). Either update the comment to match the intended limited normalization, or expand the logic/regex to include the full unreserved set. sdk/platformvalidation/arm-platformvalidation/tsp-location.yaml:1
additionalDirectories: is present with no value, which can parse as null and may break tooling that expects a sequence. If there are no additional directories, set an explicit empty list (e.g., additionalDirectories: []) or remove the key entirely. sdk/platformvalidation/arm-platformvalidation/src/restorePollerHelpers.ts:1
This constructs a RegExp from an unescaped string derived from the swagger path template suffix. If that suffix ever contains regex metacharacters (e.g., ., +, ?, (), matching behavior becomes incorrect and could be abused if templates are influenced by inputs. Escape the suffix before building the regex (treat it as a literal), or avoid regex and use a string-suffix comparison where appropriate. sdk/platformvalidation/arm-platformvalidation/src/api/platformValidationContext.ts:1
Importing JSON via with { type: \"json\" } and using a self-referential package specifier (@azure/arm-platformvalidation/package.json) can be fragile across build targets (CJS/ESM/browser) and tooling (tsc, bundlers, test runners). Consider switching to a repo-standard approach for user-agent versioning (commonly a relative JSON import or a build-time injected version) that’s known to work in all emitted formats. sdk/platformvalidation/arm-platformvalidation/src/static-helpers/pollingHelpers.ts:1
addApiVersionToUrl is duplicated in both pollingHelpers.ts and pagingHelpers.ts. To reduce drift and future bug surface, extract this into a shared helper (e.g., a small internal module under static-helpers/) and import it from both places. sdk/platformvalidation/arm-platformvalidation/test/public/sampleTest.spec.ts:1
This test file contains large commented-out blocks (recorder imports/hooks). If recorder-based tests aren’t ready, consider removing the commented code and keeping a minimal baseline test, or implement the beforeEach/afterEach with the recorder so the file demonstrates the intended public test pattern. sdk/platformvalidation/arm-platformvalidation/src/static-helpers/urlTemplate.ts:1
The RFC6570 template expansion logic is non-trivial (operators, modifiers, empty values, reserved encoding, arrays/objects). Add focused unit tests for expandUrlTemplate covering: named query expansions with empty string values (?var=), reserved vs non-reserved encoding (+/# and allowReserved), array/object expansions (* vs non-*), and prefix modifiers (:n).
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the new @azure/arm-platformvalidation package (first preview release, 1.0.0-beta.1). No tool issues found: package version, platformValidationContext.ts, and CHANGELOG.md are consistently 1.0.0-beta.1/preview, and the introduced API version (2026-07-01-preview) is consistent with the preview package version per the "first version may only be preview" rule. No public API design issues found: no _N/AutoGenerated/_ prefix naming problems, no improper unknown return types, no void return on get/list/create-or-update operations, and all public symbols in review/arm-platformvalidation-node.api.md are properly exported from src/index.ts.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Copilot reviewed 159 out of 160 changed files in this pull request and generated 1 comment.
The PR description references a different SpecRepo commit SHA (1276749...) than the one recorded here (395e3a0...). Also, additionalDirectories: is present but empty, which is ambiguous YAML and can confuse tooling that expects a list. Recommend: (1) ensure the recorded commit matches the pipeline/spec input used for this PR, and (2) set additionalDirectories: [] or remove the key entirely if unused. sdk/platformvalidation/arm-platformvalidation/src/static-helpers/urlTemplate.ts:1
The empty-string handling for named expansions is reversed: ifEmpty is \"=\" for ?/&, but the code appends it only when ifEmpty is falsy. This causes {?var} / {&var} with an empty value to emit var instead of var=. Fix by pushing ifEmpty when it is truthy (or use an explicit operator check) so empty query params are serialized correctly per RFC6570. sdk/platformvalidation/arm-platformvalidation/src/static-helpers/urlTemplate.ts:1
RFC 3986 unreserved characters include _ in addition to - . ~ (i.e., ALPHA / DIGIT / - / . / _ / ~). The comment and regex currently omit _, so %5F will not be normalized even though it is unreserved. Update the documentation and the regex to include underscore. sdk/platformvalidation/arm-platformvalidation/src/static-helpers/pollingHelpers.ts:1
Using searchParams.get(\"api-version\") can incorrectly treat an existing-but-empty api-version= as missing (because get() returns an empty string, which is falsy), resulting in a duplicate api-version parameter. Prefer urlObj.searchParams.has(\"api-version\") to detect presence reliably. The same pattern exists in src/static-helpers/pagingHelpers.ts. sdk/platformvalidation/arm-platformvalidation/test/public/sampleTest.spec.ts:1
This test file contains large commented-out blocks for recorder wiring and hooks. This makes the test suite harder to maintain and can mask intended test coverage gaps. Recommendation: either fully implement the recorder-based test pattern (beforeEach/afterEach + createRecorder) or remove the commented code and keep a minimal, purposeful test. sdk/platformvalidation/arm-platformvalidation/README.md:1
This is an ARM management-plane SDK; users typically do not need a resource-specific endpoint, and the default ARM endpoint is used unless overridden. The current wording can mislead users into searching for a per-resource endpoint. Suggest adjusting the text to explain the default ARM endpoint behavior, and mention options.endpoint only as an advanced override (e.g., sovereign clouds or custom ARM endpoints).
# Azure PlatformValidation client library for JavaScript
The Owner role grants all Azure Resource Manager control-plane actions, so the parenthetical claim is incorrect for this management client. Recommend the least-privileged role that includes the required Microsoft.PlatformValidation actions without asserting that Owner cannot authorize the calls.
The create model permits properties to be omitted, and both planConfigurationUri and planConfigurationJson are optional even though their documentation says exactly one is mandatory during creation. Consequently createOrUpdate(..., { location }) type-checks but is guaranteed to be rejected by the service. Model the create input so exactly one configuration source is required (while retaining a separate permissive update model) in TypeSpec, then regenerate.
Preserve request options on continuation-page requests
Continuation-page requests drop the operation's request options: only the initial request receives its abortSignal, custom headers, timeout, and other request settings. All nine generated list operations use this helper, so cancellation or per-call settings stop applying after page one. Thread the original operation options into buildPagedAsyncIterator and apply them to GET/POST continuation requests; this should also be fixed in the emitter.
Add a documented, compile-checked operation example
The README has no Examples section or snippet demonstrating any PlatformValidation operation; the only snippets create a client or enable logging. Add at least one operation snippet to test/snippets.spec.ts and reference it from an Examples section so the new SDK's primary workflows are documented and compile-checked.
Regenerate the PlatformValidation importer with pnpm 11.24.0 to match main's Vitest 5 testing catalog. Preserve all other lockfile entries from main.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6f75acd6-b422-40b6-9131-9d233e89dfc4
One or more custom setup steps configured for this repository failed during this Copilot code review run:
Install library dependencies
Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.
Note
You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.
✅ All CI checks have passed — this PR is ready for review!
The SDK engineer will review this PR shortly. Once it's approved, you can go ahead and merge it. To make things easier, feel free to enable auto-merge so it gets auto merged right after approval.
For more details, see the [management SDK release process]((eng.ms/redacted)
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Sovereign-cloud authentication currently requests tokens for the public ARM audience, and the README misstates the client’s required constructor inputs.
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
auto-releaseWhen merged to main, release pipelines for this PR's changed packages trigger automatically.MgmtThis issue is related to a management-plane library.mgmt-review-addedManagement SDK review completedSelf-Service ReleasePR for self-service release
6 participants
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.
Configurations: 'specification/platformvalidation/resource-manager/Microsoft.PlatformValidation/PlatformValidation/tspconfig.yaml', and CommitSHA: 'c7569077719927adf8835d79ef8d0763dd3d492d' in SpecRepo: 'https://github.com/Azure/azure-rest-api-specs' Pipeline run: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=6890416 Refer to https://eng.ms/docs/products/azure-developer-experience/develop/sdk-release/sdk-release-prerequisites to prepare for SDK release. Release plan link: https://azsdk-releaseplan-dashboard-hveph5aqhhcfhtgu.westus-01.azurewebsites.net/?releaseplan=36144 Submitted by: shivangirana@microsoft.com
CI lockfile repair
Merged main and regenerated the PlatformValidation lockfile importer with pnpm 11.24.0 to match the shared Vitest 5 catalog. Other importers and dependency snapshots are unchanged from main.
Local validation: frozen-lockfile install for the package dependency closure, targeted Turborepo build (20 packages), formatting check, TypeScript check, and Node/Chromium playback tests passed. The package lint script is intentionally a no-op. No live Azure tests were run.