ARM API Reviewer: Copilot agent for Azure REST API spec PR reviews

[ravimeda]

ARM API Reviewer: Copilot agent for Azure REST API spec PR reviews

Why

ARM API reviewers manually inspect spec PRs against the Resource Provider Contract (RPC), the Azure REST API Guidelines, and repository conventions. Common issues -- missing provisioningState, incorrect DELETE response codes, secrets in GET responses, malformed examples -- are caught late and require multiple review round-trips.

To build a comprehensive rule set, review comments from tens of thousands of PRs across both repos were retrieved using GitHub GraphQL and analyzed with Copilot to identify the most frequent guidance given to spec authors. The resulting patterns were codified into structured instruction files that supplement the canonical sources (RPC, Azure REST API Guidelines).

This PR introduces an AI-powered reviewer that runs inside VS Code as a Copilot agent, designed for ARM API reviewers to use during their weekly review rotation.

What it does

The agent operates in read-only PR review mode. It fetches PRs from GitHub, produces a structured report with exact line numbers, rule IDs, and fix suggestions, and can post review comments with reviewer approval. It does not modify files.

Workflow: The reviewer points the agent at a PR (public or private repo). The agent reviews the changed spec files and presents findings in chat. The reviewer validates which findings to post as PR comments. Before posting, the agent checks existing comments to avoid duplicates and posts at the exact file and line. The reviewer can then update labels to manage the PR queue.

Key capabilities:

• Validates OpenAPI (Swagger), TypeSpec, and example files against 100+ codified rule IDs (58 RPC rule IDs covering PUT, PATCH, DELETE, GET, POST, and async operations, plus 38 additional rule IDs for property design, secret detection, naming, examples, schema correctness, What-If noise prevention, preflight compliance, Azure Policy compatibility, template deployment, and TypeSpec-specific patterns)
• Classifies every finding as [NEW] (introduced in this PR) or [EXISTING] (pre-existing debt) by comparing against the previous API version
• Detects secrets proactively: inspects every string property for names, descriptions, or examples suggesting credentials
• Analyzes readme.md suppressions across API versions: flags accidentally dropped or unjustified new suppressions
• Reconciles against existing PR comments to avoid duplicates on repeat reviews
• Proposes PR label changes (ARMChangesRequested / WaitForARMFeedback)
• Human-gated: findings are presented in chat first; PR comments and labels are posted only with explicit reviewer approval

What it does NOT do

• Does not replace human ARM reviewers. It augments them by handling mechanical checks.
• Does not generate SDKs or author new TypeSpec projects.
• Does not modify specification files. It is read-only.
• Does not review local files or uncommitted changes. It operates on PRs only.
• Does not post PR comments without human approval.

Files changed

20 new files, 5 modified files.

File	Status	Description
.github/agents/arm-api-reviewer.agent.md	New	Agent definition: persona, PR resolution rules, review scope, 8-step workflow (identify files, load rules, compare versions, review, classify, cross-file checks, report, post comments, update labels), comment tracking marker, and PR label management
.github/instructions/armapi-review.instructions.md	Modified	ARM control-plane review rules: 26 sections covering resource paths, resource models, PUT/PATCH/DELETE contracts, async operations, secret handling, property design, ARG compatibility, suppressions, tenant-level APIs, Azure Policy compatibility, template deployment/What-If/preflight, availability zones, extended locations, systemData, managedBy, schema evolution, and versioning (58 RPC rule IDs + 38 additional rule IDs)
.github/instructions/openapi-review.instructions.md	Modified	Generic OpenAPI review rules: 24 sections covering file structure, versioning, security definitions, naming, schema conventions, enums, error handling, pagination, LRO, common-types, examples, and more
.github/instructions/typespec-review.instructions.md	New	TypeSpec review rules: 7 sections covering project structure, namespace/versioning/model rules, secret detection, suppressions, anti-patterns, ARM-specific TypeSpec rules, and tspconfig validation
.github/instructions/typespec-project.instructions.md	Modified	Aligned with review instruction references
.github/skills/azure-api-review/SKILL.md	New	Shared review skill manifest: indexes 15 cross-cutting reference files used by all three instruction files
.github/skills/azure-api-review/references/secret-detection.md	New	SEC-SECRET-DETECT: proactive secret/credential detection signals and keyword list
.github/skills/azure-api-review/references/property-mutability.md	New	OAPI027/020/029/030/031/034: write-only prohibition, conditional read-only, immutability tolerance and enforcement, clear field ownership
.github/skills/azure-api-review/references/provisioning-state.md	New	RPC-Async-V1-02/03: provisioningState requirements, terminal states, transition rules
.github/skills/azure-api-review/references/naming-conventions.md	New	Naming/casing rules for properties, models, enums, operations, Azure terminology, common property names (createdAt, lastModifiedAt), and resource identifier naming (Id suffix)
.github/skills/azure-api-review/references/enum-best-practices.md	New	Extensible enum requirements and boolean-to-enum guidance
.github/skills/azure-api-review/references/tracked-resource-lifecycle.md	New	RPC-Put-V1-22/RPC-Get-V1-05/RPC003: mandatory CRUD and list operations, resource move, singleton/constrained-collection patterns, x-ms-azure-resource placement
.github/skills/azure-api-review/references/policy-compatibility.md	New	PLCY001--PLCY009: Azure Policy compatibility rules -- property granularity, read-only post-creation properties, CSV avoidance, free-form objects, PUT semantics, collection GET requirements
.github/skills/azure-api-review/references/template-deployment.md	New	TD001--TD003: ARM Template Deployment engine compatibility -- PUT for provisioning, idempotency, preflight support
.github/skills/azure-api-review/references/availability-zones.md	New	Zones property contract: request/response semantics, zone immutability, x-ms-mutability annotation, zone discoverability, cross-subscription move
.github/skills/azure-api-review/references/field-ownership.md	New	OAPI024/025/026: value preservation rules -- array ordering, data type preservation, no casing/format normalization
.github/skills/azure-api-review/references/what-if-preflight-compliance.md	New	WHATIF-001--005/PREFLIGHT-001--005: What-If noise prevention (false deletes, false creates, PATCH-on-PUT) and preflight validation contract (HTTP 200, error format, TLE handling, no existence checks)
.github/skills/azure-api-review/references/lro-final-state-via.md	New	LRO polling behavior and final-state-via decision table for PUT/PATCH/DELETE/POST -- when to specify and anti-patterns
.github/skills/azure-api-review/references/suppression-review-criteria.md	New	RPC-SUPPRESS-GA/RPC-SUPPRESS-SCOPE: suppression approval/rejection decision framework for GA vs. preview, escalation criteria
.github/skills/azure-api-review/references/linter-rule-coverage.md	New	Linter rule ID to instruction file mapping (130+ rules) for avoiding duplicate findings with CI linters
.github/skills/azure-api-review/references/design-decisions.md	New	DD-001--DD-010: grey-area design trade-off frameworks (10 decision matrices) for ambiguous review scenarios
.github/copilot-instructions.md	Modified	Updated repo-level Copilot instructions to reference review instruction files and the shared skill
.github/copilot-review-instructions.md	New	Copilot Code Review instructions for inline PR comments on spec files
.github/cspell.yaml	Modified	Added review-related terms to the spellcheck dictionary
documentation/api-reviewer-agent.md	New	User guide: prerequisites, setup, usage examples, comment tracking marker, report format, and scope/limitations

Getting started

See documentation/api-reviewer-agent.md for setup and usage.

Next steps

We plan to gather feedback from reviewers, refine the instructions and skills based on that feedback, and once we have confidence in quality, enable automatic reviews through Copilot Code Review in both repos.

[github-actions]

Next Steps to Merge

Next steps that must be taken to merge this PR:

• ❌ The required check named Protected Files has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide

Comment generated by summarize-checks workflow run.

[ravimeda]

Below is one example of the review

Field	Value
PR	Azure/azure-rest-api-specs#41721
Branch	jushi/public-api-specs-0324
Previous version	None — new service/resource types
Changed files	28 (all added)

Two new TypeSpec projects under specification/monitor/resource-manager/Microsoft.Monitor/:

• Slis — CRUD resource (ProxyResource) for SLI management
• SliSignalPreview — POST action endpoint for signal preview queries

---

Blocking Issues — New (must fix before merge)

These issues were introduced in this PR and must be resolved.

1. [NEW] [Section 6 — Types & Formats / TSP-4.5]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp — L72

   Issue: streamingRuleLastUpdatedTimestamp is typed as string but the example value is
   "2025-03-01T00:00:00Z" — a date-time. In the generated OpenAPI it appears as "type": "string"
   without "format": "date-time", which means SDKs will treat it as an opaque string instead of a
   proper DateTime.

   Fix: Change streamingRuleLastUpdatedTimestamp?: string; to streamingRuleLastUpdatedTimestamp?: utcDateTime;

2. [NEW] [Section 8.1 — Prefer Enums Over Booleans]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp — L75

   Issue: enableAlert is typed as boolean. Booleans do not version
   well — what today is a two-state switch often needs a third state
   (e.g., Inherited, Unknown). This is a new API so now is the time to
   get the type right.

   Fix: Replace enableAlert: boolean; with an extensible union:

   @doc("Specifies whether alerts are enabled for this SLI.")
   union AlertState {
     @doc("Alerts are enabled.") Enabled: "Enabled",
     @doc("Alerts are disabled.") Disabled: "Disabled",
     string,
   }

   Then use alertState: AlertState; on the resource.

3. [NEW] [Section 8.1 / TSP-4.5] specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp — L82–L86

   Issue: ExecutionState.state is typed as plain string. The example
   value "Succeeded" indicates a finite set of known states. A plain string
   gives no SDK discoverability — users won't know which values are valid.

   Fix: Define an extensible union for execution states:

   union ExecutionStateValue {
     @doc("Execution succeeded.") Succeeded: "Succeeded",
     @doc("Execution failed.") Failed: "Failed",
     @doc("Execution is in progress.") InProgress: "InProgress",
     string,
   }

   Then change state: string; to state: ExecutionStateValue;

4. [NEW] [RPC-Patch-V1-03 / Section 4]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp
   — L139–L159

   Issue: The Sli resource has many writable properties (description,
   category, evaluationType, enableAlert, sliProperties,
   baselineProperties, destinationAmwAccounts) but no PATCH/Update
   operation. Without PATCH, customers must resend the entire resource
   body via PUT to change a single property. While Sli is a ProxyResource
   (not tracked), ARM best practice requires PATCH for resources with
   multiple writable properties.

   Fix: Add a PATCH operation to the Slis interface using an update model:

   @doc("Updates an SLI resource.")
   @route(ServiceGroupRoute)
   update is ArmResourcePatchSync<Sli, SliResource, ServiceGroupParameters>;

5. [NEW] [Section 2.4 / Section 6 — Resource References]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp
   — L90–L100

   Issue: AmwAccount.resourceId and AmwAccount.identity are typed as
   plain string but hold ARM resource IDs (per the example:
   /subscriptions/.../providers/microsoft.monitor/accounts/...). These
   should be typed or annotated to indicate they are ARM resource IDs so
   SDKs and tooling can validate format.

   Fix: Use Azure.Core.armResourceIdentifier or at minimum add @doc
   clarifying the expected format and add a regex pattern
   (e.g., @pattern("^/subscriptions/.*")). Similarly update
   sourceAmwAccountResourceId and sourceAmwAccountManagedIdentity in
   sliProperties.tsp (both Slis and SliSignalPreview).

6. [NEW] [TSP-4.2 — Documentation] All .tsp files across both projects

   Issue: All TypeSpec files use @doc("...") decorator instead of
   /** ... */ doc comments. Per TSP-4.2, documentation MUST use
   /** ... */ format. This is a systematic issue across all 10 TypeSpec
   files in the PR.

   Fix: Convert all @doc("...") decorators to /** ... */ doc comments. For example, change:

   @doc("Represents an SLI resource within the ProviderHub.")
   model Sli is ProxyResource<SliResource> {

   to:

   /** Represents an SLI resource within the ProviderHub. */
   model Sli is ProxyResource<SliResource> {

---

Warnings — New (should fix)

1. [NEW] [Section 17 — Descriptions]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sliProperties.tsp
   — L186 and L197 (same in SliSignalPreview)

   Issue: TemporalAggregationType and SpatialAggregationType have
   identical descriptions: "Defines the available aggregation types."
   This is confusing — they describe different aggregation dimensions
   (temporal vs. spatial).

   Fix: Update descriptions to:

   • TemporalAggregationType: "Defines the available temporal aggregation types for time-based analysis."
   • SpatialAggregationType: "Defines the available spatial aggregation types for cross-dimensional analysis."

2. [NEW] [Section 6 — Types & Formats]
   specification/monitor/resource-manager/Microsoft.Monitor/SliSignalPreview/kqlmQueryResult.tsp
   — L13

   Issue: KqlmQueryResult.requestId is string? but the example value
   "12345678-1234-1234-1234-123456789012" is a UUID. Using uuid type
   would provide format validation.

   Fix: Change requestId?: string; to requestId?: uuid;
   (import @typespec/http if needed for uuid scalar).

3. [NEW] [Section 8.3 — Use Enums for Finite Value Sets]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp
   — L82–L86

   Issue: ExecutionState.message is always present in examples (even as
   empty string ""), yet marked optional (message?: string). If the
   service always returns it, consider making it required.

   Fix: Verify with the service team whether message is always returned. If yes, change to message: string;.

4. [NEW] [Section 4.3 — Enums vs. Unions]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/sli.tsp
   — L104–L115
   (and SliSignalPreview signalPreview.tsp L32–L38)

   Issue: Category and EvaluationType unions use anonymous string
   members (e.g., "Availability" instead of
   Availability: "Availability"). While this generates correct OpenAPI,
   named variants improve SDK ergonomics — users get named constants
   instead of raw strings.

   Fix: Use named variants:

   union Category {
     /** Indicates availability-related metrics. */
     Availability: "Availability",
     /** Indicates latency-related metrics. */
     Latency: "Latency",
     string,
   }

5. [NEW] [Section 7 — Enumerations]
   specification/monitor/resource-manager/Microsoft.Monitor/SliSignalPreview/signalPreview.tsp
   — L17–L27

   Issue: PreviewType enum values use lowercase ("goodsignal",
   "totalsignal", "signal", "slo") while Azure convention prefers
   PascalCase for enum values. This may cause confusion with the PascalCase
   member names (GoodSignal, TotalSignal).

   Fix: If the service already uses these lowercase wire-format values
   and cannot change, document this decision. Otherwise, prefer PascalCase
   values ("GoodSignal", "TotalSignal", "Signal", "Slo").

6. [NEW] [Section 5.4 — Operations Interface]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/service.tsp
   — L11 and SliSignalPreview/service.tsp — L11

   Issue: Both services suppress missing-operations-endpoint with
   "Operations API was moved to its own service." Verify that the
   Operations API exists in a separate TypeSpec project under
   Microsoft.Monitor. If it doesn't exist, this suppression is masking
   a real requirement.

   Fix: Confirm the Operations API is defined elsewhere and link to the specific spec file in the suppression reason.

7. [NEW] [TSP-1.1 — Directory Placement]
   specification/monitor/resource-manager/Microsoft.Monitor/Slis/ and
   SliSignalPreview/

   Issue: ARM TypeSpec folder names SHOULD end with .Management
   (e.g., Slis.Management). The existing Microsoft.Monitor projects in
   this repository may use a different convention, but for new projects the
   recommended pattern should be followed.

   Fix: Confirm whether the existing Microsoft.Monitor service follows
   this naming convention. If other services under Microsoft.Monitor also
   don't use .Management, this is acceptable for consistency. Otherwise,
   rename to Slis.Management and SliSignalPreview.Management.

---

Suggestions (optional improvements)

1. Code duplication across projects — Both Slis and
   SliSignalPreview duplicate many types (Signal, SignalSource,
   Condition, ScalarFunction, SamplingType, ConditionOperator,
   SpatialAggregation, TemporalAggregation, BaselineProperties,
   Baseline, EvaluationCalculationType, WindowUptimeCriteria,
   WindowUptimeCriteriaComparator, EvaluationType,
   ServiceGroupBaseParameters). Consider extracting shared types into a
   common TypeSpec library to avoid drift.

2. HmsDuration custom scalar —
   SliSignalPreview/kqlmQueryResult.tsp defines a custom HmsDuration
   scalar for HH:MM:SS format. Consider using the standard duration
   scalar (ISO 8601) if the service can accept both formats, as it
   provides better SDK support.

3. Versions enum doc comment — In service.tsp L18, the version
   doc @doc("API Version 2025-03-01-preview") could be a /** ... */
   comment per TSP-4.2.

---

Breaking Change Analysis

Field	Value
Previous version	None (new service)
Current version	2025-03-01-preview
Breaking changes found	0 (N/A — first version)

---

Summary

Category	Count
Files reviewed	28
Previous version compared	N/A — new service
New blocking issues	6
Existing blocking issues	0
New warnings	7
Existing warnings	0
Suggestions	3

The most critical blocking issues are #1
(streamingRuleLastUpdatedTimestamp type), #2 (enableAlert boolean),
#3 (ExecutionState.state plain string), #4 (missing PATCH
operation), #5 (ARM resource ID properties typed as plain string),
and #6 (@doc() instead of /** */ doc comments). These affect SDK
quality, API evolvability, and customer usability. Since this is a
brand-new preview API, now is the ideal time to fix these before any
customers adopt the API.
#Closed

[ravimeda]

Merged the changes from #41181
@raosuhas fyi #Closed

[ravimeda]

Next steps after the PR merges

• Create an article under https://armwiki.azurewebsites.net/rp_onboarding/process/api_review.html describing the usage of the ARM API Reviewer agent
• Create a GitHub action that will run the ARM API Reviewer agent on new PRs and new commits to a PR #Closed

[haolingdong-msft]

Thanks @ravimeda for the PR, I love the idea, just leave some comments for improvements.

[haolingdong-msft]

Thanks for the PR! LGTM. I'm good as long as the prompts are update to date.