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
Enforce rule-ID hyperlinks and chat<->PR parity in ARM API Reviewer instructions (#43181)
* Update review instructions and evaluation suite to enforce rule citation format and reviewer-posted parity
* Add cleanup instructions for local workspace after PR review
Copy file name to clipboardExpand all lines: .github/agents/arm-api-reviewer.agent.md
+19Lines changed: 19 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -371,6 +371,24 @@ After successfully posting review comments to the PR:
371
371
4. If the PR does not have the `WaitForARMFeedback` label, skip the removal step and only propose adding `ARMChangesRequested`.
372
372
5. Report to the human reviewer which labels were added and removed.
373
373
374
+
### Step 9: Clean Up Local Workspace
375
+
376
+
If the review required creating any local artifacts to inspect the PR (typically a Git worktree and/or local branch tracking the PR's head ref - for example, when the PR is too large for `gh pr diff` to handle), **clean these up at the end of the review**, after the comments have been posted (Step 7) and labels updated (Step 8).
377
+
378
+
1.**Inventory** anything created during this session:
379
+
- Local branches matching `pr-<number>` or similar patterns created by you.
380
+
- Worktrees created by you (e.g., `<repo>/../specs-pr-<number>`).
381
+
- Temporary files written to the workspace root (e.g., `review-payload.json`, `review-body.txt`).
382
+
2.**Confirm the user is not still using them.** If `git status` in the worktree shows uncommitted work, ask the user before removing.
4.**Do not** touch branches, worktrees, or files that pre-existed the review (e.g., the user's working branch, unrelated worktrees, the user's stashes).
388
+
5.**Report** what was cleaned up to the user.
389
+
390
+
This keeps the user's workspace tidy and prevents accumulation of stale `pr-*` branches across reviews.
391
+
374
392
## Constraints
375
393
376
394
-**Read-only.** This agent does not modify specification files. Its job is to flag issues and suggest fixes, not apply them.
@@ -381,6 +399,7 @@ After successfully posting review comments to the PR:
381
399
-**Clean specs get clean reports.** If after thorough review a specification has no blocking violations, explicitly state that no blocking issues were found. Do not downgrade compliant patterns into violations. For example: a spec that correctly uses common-types, has all required CRUD operations, includes `provisioningState` with the right terminal states, and follows naming conventions should receive a clean bill of health -- not a list of fabricated issues. The absence of findings is a valid review outcome.
382
400
-**Scope boundaries.** Do not review SDK code, pipeline configs, or infrastructure files. Only review specification artifacts (OpenAPI JSON, TypeSpec `.tsp`, `tspconfig.yaml`, examples, readmes for AutoRest config).
383
401
-**Always compare versions.** When a previous API version exists in the repository, load it and check for breaking changes. Do not skip this step.
402
+
-**Clean up after yourself.** Any local branches, worktrees, or temporary files you create to inspect a PR MUST be removed at the end of the review (see Step 9). Never leave stale `pr-*` branches or scratch JSON payloads in the user's workspace.
Copy file name to clipboardExpand all lines: .github/instructions/armapi-review.instructions.md
+59Lines changed: 59 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -27,6 +27,65 @@ This file contains **ARM control plane–specific** review rules that supplement
27
27
28
28
Flag every violation clearly with the file path, the **exact line number** (e.g., `line 42` or `line 10-15` for ranges), the JSON path (e.g., `$.definitions.Widget.properties.name`), the specific rule ID, and a concrete suggestion for how to fix it. Vague references like "near end of file" or "around line 50" are not acceptable -- always resolve the actual line number by reading the file content. Respond in markdown format.
29
29
30
+
### Rule Citation Format (REQUIRED for posted PR comments)
31
+
32
+
Every rule ID cited in a posted PR comment **MUST** be accompanied by a markdown hyperlink to the rule's authoritative location in this repository. A bare rule ID without a link is **not acceptable** -- reviewers and authors must be able to one-click navigate to the exact section that defines the rule.
33
+
34
+
**Format.** Use a markdown link whose visible text is the rule ID and whose target is a permanent GitHub URL (use the `main` branch ref) anchored to the section that defines the rule:
**Where to link.** Pick the most specific source for the rule:
41
+
42
+
- ARM RPC rules (e.g., `RPC-Put-V1-12`, `RPC-Async-V1-06`, `RPC-Patch-V1-10`) → link to the corresponding section in `.github/instructions/armapi-review.instructions.md`.
43
+
- Generic OpenAPI rules (e.g., `OAPI020`, `OAPI027`, `OAPI034`, `WHATIF-001`, `PLCY008`, `TSP-REQUIRED-V1`) → link to the rule's section in `.github/instructions/openapi-review.instructions.md`**or** to the dedicated reference file under `.github/skills/azure-api-review/references/*.md` when the rule has a full reference page (e.g., `property-mutability.md#oapi034`, `secret-detection.md`, `provisioning-state.md`).
44
+
- TypeSpec-only rules → link to the corresponding section in `.github/instructions/typespec-review.instructions.md`.
45
+
46
+
**Multiple rule IDs.** When a finding cites more than one rule ID (e.g., a combined ARG/Section/OAPI citation), each ID **MUST** be its own hyperlink:
**Anchor resolution.** GitHub auto-generates section anchors by lowercasing the heading, replacing spaces with `-`, and stripping punctuation. For example, the heading `### 8.20 Fields Must Have Clear Ownership — Server or Client (OAPI034)` becomes `#820-fields-must-have-clear-ownership--server-or-client-oapi034`. When in doubt, open the rendered file on GitHub and copy the link from the heading's anchor icon.
### Reviewer-Posted Parity (REQUIRED -- no divergence)
63
+
64
+
The set of findings posted to the GitHub PR **MUST** be **byte-for-byte identical** to the set of findings shown to the reviewer in chat. There **MUST** be no discrepancy in content, count, ordering, severity, rule IDs, links, code blocks, JSON examples, fix snippets, or the `<!-- posted-by: arm-api-reviewer-agent -->` marker.
65
+
66
+
**Hard rules.**
67
+
68
+
1.**Single source of truth.** Build the exact comment body **once** as the canonical text for each finding. The text rendered to the reviewer in chat and the text written into the GitHub review payload **MUST** come from that same string -- not a reconstructed, re-summarized, or shortened variant.
69
+
2.**Verbatim reproduction.** When constructing the GitHub review payload (`POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews` with inline `comments[]`), each comment's `body` field **MUST** contain the canonical finding text **unchanged**. Do **not** drop, paraphrase, collapse, or shorten:
70
+
- the rule ID hyperlinks,
71
+
- the JSON / TypeSpec / code blocks under **Fix:**,
72
+
- inline examples,
73
+
- file path / line number / JSON path citations,
74
+
- the trailing `<!-- posted-by: arm-api-reviewer-agent -->` marker.
75
+
3.**No re-authoring during payload assembly.** Heredoc rebuilds, JSON-string escaping, multi-finding consolidation, or any step that involves "rewriting the body inline" is **forbidden**. Generate each comment body once, store it, and reference the stored value when building the payload.
76
+
4.**Exact one-to-one mapping.** Every finding shown to the reviewer **MUST** map to exactly one inline comment in the posted review. The reviewer **MUST NOT** see N findings and the PR receive N-1 (something dropped) or N+1 (something added). Severity tags (`🔴 Blocking`, `🟡 Warning`, `🔵 Suggestion`) and `[NEW]`/`[EXISTING]` classifications **MUST** match.
77
+
5.**Post-post verification (REQUIRED).** Immediately after posting the review, the agent **MUST** fetch the live comment bodies (`GET /repos/{owner}/{repo}/pulls/comments/{id}` for each created comment) and verify, for every comment:
78
+
- body length matches the canonical text length (within normalisation tolerance for line endings only),
79
+
- the rule ID hyperlinks are present,
80
+
- any code-fence (` ``` `) blocks present in the canonical text are present in the posted body,
81
+
- the `<!-- posted-by: arm-api-reviewer-agent -->` marker is present.
82
+
If any check fails, the agent **MUST** PATCH the affected comment(s) (`PATCH /repos/{owner}/{repo}/pulls/comments/{id}`) to restore the canonical text and re-verify -- before reporting completion to the user.
83
+
6.**Failure handling.** If a finding cannot be posted as-is (e.g., GitHub API rejects the body, a line anchor cannot be resolved), the agent **MUST** report the discrepancy explicitly to the reviewer rather than silently posting a shortened or altered variant.
84
+
85
+
**Negative example (do NOT do):** Show the reviewer a finding with a JSON code-block under **Fix:**, then build a multi-comment payload heredoc that omits the code-block to keep the JSON string short.
86
+
87
+
**Positive example (DO):** Build each finding's body string once with hyperlinks, code blocks, and marker included; serialize that exact string into the `body` field of each `comments[]` entry in the review payload; after posting, re-fetch each comment and confirm the live body matches.
88
+
30
89
**False-positive avoidance.** If a spec is fully compliant with all ARM RPC rules -- has all required CRUD operations, correct response codes, provisioningState, systemData, x-ms-mutability on location, x-ms-pageable on list operations, x-ms-enum with modelAsString, descriptions on all elements, and proper security definitions -- state that no blocking issues were found. Do not fabricate violations or elevate process-level recommendations to blocking findings. Specifically:
31
90
32
91
-**Inline definitions vs. common-types `$ref`:** A spec that correctly defines ARM-standard shapes inline (ErrorResponse, SystemData, Operation, OperationListResult, TrackedResource, etc.) with all required fields is **functionally compliant**. Preferring `$ref` to common-types is a process recommendation, NOT a blocking error. Flag it as a **suggestion** only.
Copy file name to clipboardExpand all lines: .github/instructions/openapi-review.instructions.md
+33Lines changed: 33 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -22,6 +22,39 @@ When performing a code review on OpenAPI v2 (Swagger) JSON definition files in t
22
22
23
23
Flag every violation clearly with the file path, the **exact line number** (e.g., `line 42` or `line 10-15` for ranges), the JSON path (e.g., `$.definitions.Widget.properties.name`), the specific rule being violated, and a concrete suggestion for how to fix it. Vague references like "near end of file" or "around line 50" are not acceptable -- always resolve the actual line number by reading the file content. Respond in markdown format.
24
24
25
+
### Rule Citation Format (REQUIRED for posted PR comments)
26
+
27
+
Every rule ID cited in a posted PR comment **MUST** be accompanied by a markdown hyperlink to the rule's authoritative location in this repository. A bare rule ID without a link is **not acceptable** -- reviewers and authors must be able to one-click navigate to the exact section that defines the rule.
28
+
29
+
**Format.** Use a markdown link whose visible text is the rule ID and whose target is a permanent GitHub URL (use the `main` branch ref) anchored to the section that defines the rule:
**Where to link.** Pick the most specific source for the rule:
36
+
37
+
- Generic OpenAPI rules (e.g., `OAPI020`, `OAPI027`, `OAPI034`, `WHATIF-001`, `PLCY008`, `TSP-REQUIRED-V1`) → link to the rule's section in `.github/instructions/openapi-review.instructions.md`**or** to the dedicated reference file under `.github/skills/azure-api-review/references/*.md` when the rule has a full reference page (e.g., `property-mutability.md#oapi034`, `secret-detection.md`, `provisioning-state.md`).
38
+
- ARM RPC rules (e.g., `RPC-Put-V1-12`, `RPC-Async-V1-06`) → link to the corresponding section in `.github/instructions/armapi-review.instructions.md`.
39
+
- TypeSpec-only rules → link to the corresponding section in `.github/instructions/typespec-review.instructions.md`.
40
+
41
+
**Multiple rule IDs.** When a finding cites more than one rule ID, each ID **MUST** be its own hyperlink (e.g., `[OAPI034](...) / [WHATIF-001](...)`).
42
+
43
+
**Anchor resolution.** GitHub auto-generates section anchors by lowercasing the heading, replacing spaces with `-`, and stripping punctuation. When in doubt, open the rendered file on GitHub and copy the link from the heading's anchor icon.
44
+
45
+
### Reviewer-Posted Parity (REQUIRED -- no divergence)
46
+
47
+
The set of findings posted to the GitHub PR **MUST** be **byte-for-byte identical** to the set of findings shown to the reviewer in chat. There **MUST** be no discrepancy in content, count, ordering, severity, rule IDs, links, code blocks, examples, fix snippets, or the agent's posted-by marker.
48
+
49
+
**Hard rules.**
50
+
51
+
1.**Single source of truth.** Build each comment body **once** as the canonical text for that finding. The text rendered to the reviewer in chat and the text written into the GitHub review payload **MUST** come from that same string -- never a reconstructed or shortened variant.
52
+
2.**Verbatim reproduction.** When assembling the review payload (`POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews`), each `comments[].body`**MUST** contain the canonical text unchanged: rule-ID hyperlinks, code blocks, examples, citations, and the trailing posted-by HTML comment.
53
+
3.**No re-authoring during payload assembly.** Heredoc rebuilds, payload-time paraphrasing, or multi-finding consolidation that drops content is **forbidden**.
54
+
4.**Exact one-to-one mapping.** Every finding shown to the reviewer maps to exactly one posted inline comment. Severity tags and `[NEW]`/`[EXISTING]` classifications **MUST** match.
55
+
5.**Post-post verification (REQUIRED).** Immediately after posting, the agent **MUST** re-fetch each created comment (`GET /repos/{owner}/{repo}/pulls/comments/{id}`) and confirm body length, hyperlinks, code-fence blocks, and marker. On any mismatch, PATCH the comment to restore the canonical text and re-verify before reporting completion.
56
+
6.**Failure handling.** If a finding cannot be posted as-is, report the discrepancy explicitly to the reviewer instead of silently posting a shortened variant.
0 commit comments