Merge branch 'main' into vikeshi26/arm-leases-batch-2026-05-22

Author: tejaswiMinnu

.github/instructions/armapi-review.instructions.md

@@ -505,6 +505,12 @@ The TypeSpec-required rule applies to all new ARM API versions. The full rule de
 
 - Properties that are arrays **MUST** have plural names (e.g., `scopes`, `rules`, `addresses`) -- not singular (e.g., `scope`, `rule`, `address`). Singular names suggest a single value, not a collection.
 
+### 8.3b Array Item Identifiers (`x-ms-identifiers`)
+
+- Arrays of objects in ARM responses **SHOULD** declare which item property uniquely identifies an item via the `x-ms-identifiers` extension. This is what enables ARM tooling (Resource Graph, Change Analysis, What-If) to track individual items across PUT/PATCH/GET responses without treating the array as opaque.
+- **In TypeSpec source, do NOT use `@extension("x-ms-identifiers", ...)`** to control this. `@extension` is forbidden in TypeSpec source for any reason. Use the `@key` decorator on the identity property of the item type, or the `@identifiers(#["<propertyName>"])` decorator on the array property. For arrays whose items have no logical identifier, use `@identifiers(#[])`. See `typespec-review.instructions.md` Section 6.5 (TSP-ARRAY-IDENTIFIERS) for the full rule with examples.
+- A `#suppress` on `missing-x-ms-identifiers` is acceptable **only** as a last resort and must include a real technical justification (no `FIXME`/`TODO`/`TBD`, no "matching another resource" — see `typespec-review.instructions.md` Section 4.1 Warning Suppression Policy).
+
 ### 8.4 Use Specific Types Instead of Generic Strings (ACTIVELY REVIEW)
 
 > **See also:** [`.github/skills/azure-api-review/references/naming-conventions.md`](../skills/azure-api-review/references/naming-conventions.md) for common property names (e.g., `createdAt`, `lastModifiedAt`, `deletedAt`) and resource identifier naming rules (use `Id` suffix, not `Uri` or `Name`).
@@ -1138,6 +1144,7 @@ When reviewing ARM resource-manager swagger files, verify:
 - ✅ Immutable properties: unchanged values accepted, changed values rejected with 400 (OAPI030, OAPI031)
 - ✅ Fields have clear ownership — server-owned (readOnly) or client-owned (preserved exactly); no server auto-updates of client fields (OAPI034)
 - ✅ Array ordering preserved in PUT/PATCH responses (OAPI024)
+- ✅ Arrays of objects declare `x-ms-identifiers` via TypeSpec `@key` or `@identifiers` (not `@extension`) — see TSP-ARRAY-IDENTIFIERS
 - ✅ Property value casing and formatting preserved — no normalization (OAPI026)
 
 ### List APIs

.github/instructions/typespec-review.instructions.md

@@ -290,6 +290,7 @@ Flag these issues when found:
 - **`@flattenProperty` on new APIs** — do not add new `@flattenProperty` decorators. Flattening creates SDK-breaking issues and is discouraged for new resource types and properties. Existing flattened properties may remain for backward compatibility.
 - **Spread-only model types as full models** — a model type used only for spreading (`...`) into other types **SHOULD** be declared as an `alias` instead. Using `model` for types that are only spread generates unnecessary types in the output. See [TypeSpec alias documentation](https://typespec.io/docs/language-basics/alias) (TypeSpec-BestPractice-01).
 - **Empty model literal `{}` as POST action body** -- when an `ArmResourceActionAsync` or `ArmResourceActionSync` POST action does not need a request body, use `void` instead of `{}`. An empty model literal triggers the `no-empty-model` lint rule, and suppressing it is the wrong fix. Use `void` and remove the suppression.
+- **`@extension(...)` in TypeSpec source** -- never add `@extension(...)` decorators to TypeSpec source for any reason. This includes `@extension("x-ms-identifiers", ...)`, `@extension("x-ms-mutability", ...)`, and any other OpenAPI extension. `@extension` bypasses TypeSpec's type system and emitter conventions. Use the appropriate built-in decorator instead: `@identifiers` / `@key` for `x-ms-identifiers`, `@visibility` for mutability, `@secret` for secret data, etc. (see TSP-ARRAY-IDENTIFIERS for the `x-ms-identifiers` case).
 
 ---
 
@@ -320,6 +321,47 @@ Flag these issues when found:
 - Properties whose names or doc comments clearly indicate numeric values (e.g., `numberOfCores`, `ram`, `vCpu`, `diskSizeGB`) **SHOULD** use `int32`, `int64`, or `float64` — not `string`.
 - If backward compatibility forces a string type for a numeric value, the doc comment **MUST** document the expected format and units.
 
+### 6.5 Array Identifiers — `x-ms-identifiers` (TSP-ARRAY-IDENTIFIERS)
+
+The `missing-x-ms-identifiers` linter rule fires on array-typed properties whose item models do not declare which property uniquely identifies an item. **Do not silence this rule with `@extension("x-ms-identifiers", [])`.** `@extension` is forbidden in TypeSpec source for any reason (see Section 5 anti-patterns).
+
+Use one of the following built-in TypeSpec decorators instead:
+
+- **Preferred — `@key` on the identity property of the item type.** The emitter automatically derives `x-ms-identifiers` from `@key`. Use this when the item model has a single, natural identity property:
+
+  ```tsp
+  model MoveRequest {
+    @key moveId: string;
+    from: string;
+    to: string;
+  }
+
+  model MoveCollection {
+    moves?: MoveRequest[];
+  }
+  ```
+
+- **Alternative — `@identifiers(#["<propertyName>", ...])` on the array property.** Use this when the identity property cannot be marked with `@key` on the item type (e.g., the item type is shared and `@key` would conflict, or identity is composed of multiple properties):
+
+  ```tsp
+  @identifiers(#["moveId"])
+  moves?: MoveRequest[];
+  ```
+
+- **Items genuinely have no identifier — `@identifiers(#[])` on the array property.** For arrays whose items are ordered result records, primitive values, or otherwise have no stable identity field, declare the empty identifier list explicitly. This satisfies the linter without a suppression and documents the design intent:
+
+  ```tsp
+  @identifiers(#[])
+  testResults?: TestResultRecord[];
+  ```
+
+**Forbidden patterns:**
+
+- `@extension("x-ms-identifiers", [])` — never add `@extension` in TypeSpec source for any reason. Replace with `@identifiers(#[])` (or `@key` on the item type).
+- `#suppress "@azure-tools/typespec-azure-resource-manager/missing-x-ms-identifiers" "..."` — suppression is **only** acceptable as a last resort when neither `@key` nor `@identifiers` is technically feasible (extremely rare). The suppression reason must still meet Section 4.1 rules (real technical justification, no `FIXME`/`TODO`/`TBD`, no "matching another resource"). For brand-new operations and models, suppression is never the right fix — use `@identifiers` or `@key`.
+
+When reviewing a new `missing-x-ms-identifiers` suppression, propose `@identifiers` or `@key` as the fix. **Never** suggest `@extension("x-ms-identifiers", ...)`.
+
 ---
 
 ## 7. `tspconfig.yaml` Additional Validation
@@ -396,6 +438,8 @@ When reviewing TypeSpec files, verify:
 - ✅ ARM resource ID properties use `armResourceIdentifier` not `string` (TSP-ARM-RESOURCE-ID)
 - ✅ No `@operationId` overrides — restructure interfaces instead
 - ✅ URI properties use `url` scalar type, not plain `string`
+- ✅ Array-typed properties declare item identity via `@key` (on item type) or `@identifiers` (on array property); use `@identifiers(#[])` when items have no identifier (TSP-ARRAY-IDENTIFIERS)
+- ✅ No `@extension(...)` decorators in TypeSpec source — never `@extension("x-ms-identifiers", ...)`, `@extension("x-ms-mutability", ...)`, etc.
 - ✅ Standard ARM base types used (no custom/private resource decorators)
 - ✅ Client customizations only in `client.tsp`
 - ✅ `tspconfig.yaml` references correct linter ruleset

.github/skills/evals/arm-api-reviewer/.vally.yaml

@@ -4,5 +4,5 @@ paths:
 
 suites:
   all:
-    description: "Full ARM API Reviewer eval suite (34 stimuli)"
+    description: "Full ARM API Reviewer eval suite (35 stimuli)"
     evals: ["evaluate/eval-*.yaml"]

.github/skills/evals/arm-api-reviewer/README.md

@@ -22,7 +22,7 @@ arm-api-reviewer/
 │   ├── eval-classification.yaml
 │   ├── eval-report-format.yaml
 │   └── eval-citation-and-parity.yaml
-├── fixtures/              # Test fixtures (32 files)
+├── fixtures/              # Test fixtures (34 files)
 │   ├── arm-openapi/       # ARM OpenAPI specs with seeded violations
 │   ├── examples/          # Example JSON files (good and bad)
 │   ├── readme/            # readme.md suppression files
@@ -31,7 +31,7 @@ arm-api-reviewer/
 └── README.md              # This file
 ```
 
-## Test Categories (34 test cases across 13 eval files)
+## Test Categories (35 test cases across 13 eval files)
 
 | ID     | Category                 | Count | Description                                                          |
 | ------ | ------------------------ | ----- | -------------------------------------------------------------------- |
@@ -41,7 +41,7 @@ arm-api-reviewer/
 | 04xxxx | Breaking changes         | 4     | Removed property, type change, enum narrowing, added required        |
 | 05xxxx | Suppression analysis     | 2     | Missing reason, security rule suppressions                           |
 | 06xxxx | Example file validation  | 2     | Bad resource ID, realistic secrets                                   |
-| 07xxxx | TypeSpec review          | 3     | Segment casing, secrets, anti-patterns                               |
+| 07xxxx | TypeSpec review          | 4     | Segment casing, secrets, anti-patterns, x-ms-identifiers             |
 | 08xxxx | Check Name Availability  | 1     | Custom CNA models, missing input validation                          |
 | 09xxxx | True negatives           | 3     | Clean spec, clean example, clean proxy resource                      |
 | 10xxxx | Classification           | 1     | NEW vs EXISTING issue tagging                                        |
@@ -51,15 +51,15 @@ arm-api-reviewer/
 
 ## Fixtures
 
-All 33 fixture files live in `fixtures/`. See
+All 34 fixture files live in `fixtures/`. See
 [`fixtures/README.md`](fixtures/README.md) for the complete catalog with
 descriptions, seeded violations, and guidance on reusing fixtures in other
 eval suites.
 
 - **15 ARM OpenAPI specs** in `arm-openapi/` -- 2 clean + 12 with seeded violations + 1 TypeSpec-generated
 - **3 example JSON files** in `examples/` -- 1 clean + 2 with issues
 - **2 readme.md files** in `readme/` -- suppression scenarios
-- **3 TypeSpec files** in `typespec/` -- segment/naming, secret/type, anti-pattern violations
+- **4 TypeSpec files** in `typespec/` -- segment/naming, secret/type, anti-pattern, x-ms-identifiers violations
 - **10 version-pair files** in `version-pairs/` -- 5 pairs for breaking change detection
 
 ## Quick Start
@@ -74,7 +74,7 @@ VS Code with GitHub Copilot active.
 ```powershell
 cd .github/skills/evals/arm-api-reviewer
 
-# Run the full suite (34 stimuli, sequential -- safest)
+# Run the full suite (35 stimuli, sequential -- safest)
 .\run-evals.ps1
 
 # Point to an existing evaluate clone instead of re-cloning
@@ -124,7 +124,7 @@ cd .github/skills/evals/arm-api-reviewer
 # (evaluate is a monorepo; the vally binary lives under packages/cli)
 export VALLY_CLI="/path/to/evaluate/packages/cli/dist/index.js"
 
-# Run the full suite (all 34 stimuli, 5 concurrent workers)
+# Run the full suite (all 35 stimuli, 5 concurrent workers)
 node $VALLY_CLI eval --suite all --verbose
 
 # Run a single category

.github/skills/evals/arm-api-reviewer/evaluate/eval-typespec.yaml

@@ -116,3 +116,46 @@ stimuli:
       - "Flagged ALL_CAPS enum value (FAILED) -- should use PascalCase (Failed)"
       - "Flagged default value on ipAllocationMethod that flows into PATCH model (RPC-Patch-V1-10)"
       - "Flagged @flattenProperty on new API -- flattening is discouraged for new resource types"
+
+  # 070004 -- x-ms-identifiers violations (TSP-ARRAY-IDENTIFIERS)
+  - name: detect-typespec-x-ms-identifiers-violations
+    environment:
+      files:
+        - src: "../fixtures/typespec/x-ms-identifiers-violations.tsp"
+          dest: "specification/identifierdemo/resource-manager/Microsoft.IdentifierDemo/main.tsp"
+    prompt: >
+      Review the following TypeSpec specification file for compliance with
+      Azure REST API Guidelines, ARM RPC rules, and TypeSpec-specific review
+      rules. This is a new service with no previous API version. Report all
+      findings with rule IDs, line numbers, and fix suggestions. Pay special
+      attention to how array properties declare x-ms-identifiers.
+
+      File: specification/identifierdemo/resource-manager/Microsoft.IdentifierDemo/main.tsp
+    constraints:
+      expect_skills:
+        - azure-api-review
+    graders:
+      - type: output-matches
+        config:
+          pattern: "(?i)@extension|x-ms-identifiers"
+      - type: output-matches
+        config:
+          pattern: "(?i)@identifiers|@key"
+      - type: output-matches
+        config:
+          pattern: "(?i)TSP-ARRAY-IDENTIFIERS|missing-x-ms-identifiers"
+      - type: output-matches
+        config:
+          pattern: "(?i)FIXME|placeholder"
+      - type: prompt
+    rubric:
+      - 'Flagged the @extension("x-ms-identifiers", []) on preflightResults as forbidden -- @extension MUST NOT be used in TypeSpec source for any reason (TSP-ARRAY-IDENTIFIERS)'
+      - 'Recommended replacing @extension("x-ms-identifiers", []) with @identifiers(#[]) for preflightResults'
+      - 'Flagged the @extension("x-ms-identifiers", ["nodeId"]) on nodes as forbidden -- must use @identifiers(#["nodeId"]) or @key on the item type'
+      - 'Flagged the #suppress reason containing "FIXME" on volumes as a placeholder-text violation (TSP-4.1 / Section 4.1 -- Warning Suppression Policy)'
+      - 'Flagged the vague "Matching AiGateway pattern" suppression reason on firewallRules -- suppressions must stand on their own with a real technical justification (TSP-4.1)'
+      - 'Recommended using @identifiers(#["volumeId"]) on volumes or @key on volumeId in the StorageVolume model instead of suppressing the rule'
+      - 'Did NOT flag the @identifiers(#["moveId"]) on pendingMoves -- this is the correct pattern (positive control)'
+      - 'Did NOT flag the @identifiers(#[]) on diagnosticSnapshots -- explicit empty identifier list is the correct pattern for arrays with no identity (positive control)'
+      - 'Did NOT flag customLabels -- the ClusterLabel item type uses @key on labelKey, which automatically derives x-ms-identifiers (positive control)'
+      - 'Never suggested @extension("x-ms-identifiers", ...) as a fix in any finding'

.github/skills/evals/arm-api-reviewer/fixtures/README.md

@@ -94,10 +94,11 @@ Each subdirectory contains a `stable-2024-01-01.json` (previous) and
 | `new-vs-existing/`         | Mixed classification      | `bar` has no description in both versions (EXISTING); `baz` is newly added without description (NEW). |
 | `added-required-property/` | Optional becomes required | `sku` property changes from optional to required in WidgetProperties between versions.                |
 
-### `typespec/` -- TypeSpec Specification Files (3 files)
+### `typespec/` -- TypeSpec Specification Files (4 files)
 
 | File                             | Violations                    | Description                                                                                                                                                                                                                                           |
 | -------------------------------- | ----------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `segment-casing-violations.tsp`  | @segment casing, naming, enum | @segment uses all-lowercase instead of camelCase (TSP-SEGMENT-CASE); PATCH named "patch" not "update" (TSP-PATCH-NAME); @operationId override; enum instead of union; plain string for ARM resource ID.                                               |
 | `secret-and-type-violations.tsp` | Secrets, type constraints     | Missing @secret on connectionString/adminPassword/primaryKey (SEC-SECRET-DETECT); #suppress for secret-prop; string instead of utcDateTime; string for numeric diskSizeGB (TSP-NUMERIC-TYPE); plain string for ARM resource ID (TSP-ARM-RESOURCE-ID). |
 | `anti-patterns.tsp`              | Common TypeSpec anti-patterns | Empty model `{}` instead of void for POST action; #suppress for no-empty-model; @flattenProperty on new API; default value flowing into PATCH; `\| null` on new property; underscore and ALL_CAPS enum values.                                        |
+| `x-ms-identifiers-violations.tsp` | x-ms-identifiers / @extension | `@extension("x-ms-identifiers", ...)` on array properties (forbidden -- use `@identifiers` or `@key`); `#suppress` of `missing-x-ms-identifiers` with FIXME placeholder text (TSP-4.1); vague "matching another resource" suppression (TSP-ARRAY-IDENTIFIERS). Also contains positive controls: `@identifiers(#["..."])`, `@identifiers(#[])`, and `@key` on item type. |