Integrate ARM semantic review with auto-signoff

.github/skills/arm-semantic-review/SKILL.md

@@ -0,0 +1,200 @@
+---
+name: arm-semantic-review
+description: "Maintain, review, and debug the ARM Semantic Review commit status that connects the ARM API Reviewer (gh-aw) to Universal Auto-Signoff. USE FOR: changes to .github/workflows/arm-api-review.md or its lock file's status plumbing, arm-semantic-review.ts, arm-semantic-review-workflow.ts, arm-universal-auto-signoff.ts, the record_arm_semantic_review safe output, an ARM Semantic Review status that is stuck Pending or wrong, ARMAutoSignedOff-Test or ARMManualSignoffRequired label questions. DO NOT USE FOR: reviewing ARM API specifications themselves (use azure-api-review), TypeSpec authoring, or general GitHub Actions review (use engineering-code-review)."
+---
+
+# ARM Semantic Review
+
+The ARM API Reviewer is an AI agent. Auto-signoff must not trust it directly, so its verdict is
+turned into one **commit status**, `ARM Semantic Review`, on the reviewed SHA. Universal
+Auto-Signoff reads that status alongside LintDiff and Avocado.
+
+Short overview next to the code:
+[README.md](../../workflows/src/arm-auto-signoff/README.md).
+
+## Mental model
+
+```text
+START   pre_activation          post status PENDING, upload one head-sha=<sha>;issue-number=<n> artifact
+MIDDLE  agent (read-only AI)    review the PR, then request record_arm_semantic_review
+END     record job (trusted)    validate the 4 values, post SUCCESS / FAILURE / ERROR
+SAFETY  conclusion job          if no result was posted, turn this run's PENDING into ERROR
+READ    Universal Auto-Signoff  re-check the PR is open at the evaluated head, read the newest status
+                                on that SHA plus labels -> label action
+APPLY   Update Labels           add or remove the label as emitted
+```
+
+The model proposes. Trusted code validates and publishes. Any doubt ends as "not Passed".
+
+## Files
+
+| File                                                                                                    | Role                                                                         |
+| ------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- |
+| [arm-api-review.md](../../workflows/arm-api-review.md)                                                  | Source of the reviewer workflow. Edit this, never the lock file.             |
+| [arm-api-review.lock.yml](../../workflows/arm-api-review.lock.yml)                                      | Generated by `gh aw compile`. Must match the `.md`.                          |
+| [arm-semantic-review.ts](../../workflows/src/arm-auto-signoff/arm-semantic-review.ts)                   | Pure policy: validate the result, map it to a status, read statuses. No I/O. |
+| [arm-semantic-review-workflow.ts](../../workflows/src/arm-auto-signoff/arm-semantic-review-workflow.ts) | I/O: publish the status, finalize an unpublished one.                        |
+| [arm-universal-auto-signoff.ts](../../workflows/src/arm-auto-signoff/arm-universal-auto-signoff.ts)     | Reads statuses and labels, decides label actions.                            |
+| [context.ts](../../workflows/src/context.ts)                                                            | `extractInputs` and the trusted-artifact parser shared by all consumers.     |
+
+## The status contract
+
+Context `ARM Semantic Review`, bound to the reviewed SHA. `target_url` is
+`.../actions/runs/<id>/attempts/<n>`, which identifies the run that wrote it.
+
+| State     | Description starts with    | Meaning                                         | Adds `ARMManualSignoffRequired`? |
+| --------- | -------------------------- | ----------------------------------------------- | -------------------------------- |
+| `pending` | `ARM API semantic review`  | Review in progress                              | No                               |
+| `success` | `Passed:`                  | Full, complete review, 0 Blocking findings      | No                               |
+| `failure` | `Changes requested:`       | Full, complete review with Blocking findings    | No                               |
+| `error`   | `Manual review required: ` | Scoped review, or `specification/` over the cap | **Yes**                          |
+| `error`   | `Review incomplete: `      | Failed, cancelled, unreadable, or unvalidated   | No                               |
+
+Only `success` can lead to signoff. The two `error` outcomes share a state and are told apart by
+the description prefix, so keep the prefixes in `arm-semantic-review.ts` as the single source.
+`Manual review required` is a property of the PR. `Review incomplete` is transient, so a re-run
+can still pass and it must never add the sticky label.
+
+## Validation, and why each check exists
+
+Trusted inputs (never from the model): the PR number and head SHA come from the run's correlation
+artifact, named `head-sha=<sha>;issue-number=<n>` (`;` joins the pairs so one upload carries both),
+and the attempt from `GITHUB_RUN_ATTEMPT`.
+
+| Check                                                                         | Where                                  | If it fails               |
+| ----------------------------------------------------------------------------- | -------------------------------------- | ------------------------- |
+| Trusted artifacts present, full 40-hex SHA, no conflicting duplicates         | `context.ts`                           | Nothing published / throw |
+| Output file exists and is JSON                                                | publisher `try`                        | `Review incomplete`       |
+| Exactly one `record_arm_semantic_review` item                                 | `parseSemanticReviewResult`            | `Review incomplete`       |
+| `blocking_count` is a digit string; the other three fields are in closed sets | `parseSemanticReviewResult`            | `Review incomplete`       |
+| A `complete` review must have reason `none`                                   | `parseSemanticReviewResult`            | `Review incomplete`       |
+| PR size within the cap, checked independently of the model's `scope`          | `requiresManualReview` (workflow file) | `Manual review required`  |
+| Incomplete, then scoped, then Blocking, then Passed (first match wins)        | `evaluateSemanticReview`               | n/a                       |
+
+### Verifying the model's scope
+
+The model reports `scope: full|scoped`, but it is untrusted. For a **full, complete** claim, the
+trusted code re-derives scope with the reviewer prompt's rule: more than 50 `specification/` files
+or 5,000 `specification/` lines needs a human. The rules are pure helpers in
+`arm-semantic-review.ts` (`checkPullRequestSize` and `addSpecificationChanges`); the workflow file's
+`getOversizeReason` does the API calls and logging. Any other claim (scoped, incomplete) already
+withholds signoff, so the check is skipped.
+
+1. Zero changed files (GitHub could not compute the diff) or 3,000 or more requires manual review
+   without listing files. 3,000 is the most files GitHub's file-listing API will return, so a PR
+   that large cannot be proven fully covered.
+2. If the PR's totals are within the cap, its `specification/` subset is too: no file list, so
+   most PRs cost one `pulls.get` call.
+3. Otherwise count only the `specification/` files, and stop paginating as soon as the count is
+   over the cap. A renamed or moved file counts when either its old or its new path is under
+   `specification/`.
+
+The status says why, for example `Manual review required: specification/ changes exceed 50 files
+or 5000 lines`. Reasons are fixed strings, never model output. Each step logs what it saw.
+Deliberately **not** validated, because it adds code without protecting signoff:
+
+- the PR being open or still at the reviewed SHA (the status is SHA-bound and Universal re-checks
+  the live head);
+- a "newer run exists" check before publishing (a late same-SHA write is stale but valid for that
+  SHA);
+- re-validating the attempt, PR, or SHA inside the parser (already validated upstream).
+
+Error text from a failed validation goes to the run log only. The published description is the
+fixed string `Review incomplete: result could not be validated`, so runner paths and API errors
+never reach a public status.
+
+## Invariants: do not break these
+
+1. The model never supplies the SHA, PR number, or attempt.
+2. Only `success` authorizes signoff; every unknown, missing, or errored case is "not Passed".
+3. `statuses: write` stays in trusted jobs only (`pre_activation`, the record job, `conclusion`).
+   Never give it to the `agent` job, and Universal stays `statuses: read`.
+4. The finalizer publishes only when this exact run and attempt owns the newest `pending` status.
+5. `Review incomplete` never adds `ARMManualSignoffRequired`.
+6. Do not check out or run PR code in any of these jobs (`pull_request_target`).
+
+## Behavior in the edge cases
+
+| Situation                                                    | Result                                                                                                        |
+| ------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- |
+| New commit pushed during a review                            | An eligible push cancels the run (`cancel-in-progress`). The old SHA's status is never read for the new head. |
+| `/arm-review` while a run is in flight                       | Cancels the old run and starts a new one on the same SHA.                                                     |
+| Agent `noop`, failure, timeout, detection failure, or cancel | The `conclusion` finalizer turns this run's `pending` into `Review incomplete`.                               |
+| Transient GitHub API error while publishing                  | `Review incomplete`, no sticky label. A re-run can pass.                                                      |
+| Critic unavailable                                           | `Review incomplete: critic-unavailable`, labels unchanged.                                                    |
+| Scoped or oversized PR                                       | `Manual review required`, adds `ARMManualSignoffRequired`.                                                    |
+| Non-team-member actor                                        | No `pending`, no artifacts, no status. Signoff stays blocked.                                                 |
+| Event the gate skips (other label, other comment)            | The run is skipped. Those events get a private concurrency lane so they cannot cancel a review.               |
+
+## Known limitations
+
+- Update Labels applies the emitted label action without re-checking the PR head. Universal checks
+  just before emitting, so a push in the short window between the two can leave the pilot label
+  stale until the next evaluation. A stale-head guard in Update Labels is a separate, cross-cutting
+  change (it affects every producer that publishes a `head-sha=` artifact).
+- A same-SHA re-run posts `pending` but does not revoke an existing `ARMAutoSignedOff-Test`
+  label until Universal next runs. This matters only once enforcement is on.
+- Universal's label filter does not include `ARMChangesRequested` or `WaitForARMFeedback`, so
+  adding them does not trigger an immediate re-evaluation.
+- The newest status named `ARM Semantic Review` wins; its creator is not verified.
+- If a re-run resolves a different head SHA than the first attempt, two `head-sha=` artifacts can
+  conflict and the parser throws, which can leave `Pending`.
+- `blocking_count` is the model's verdict. The Critic and threat detection are the only checks on
+  it, which is an accepted trade-off.
+- A `Pending` status is resolved in almost every failure, but can still remain when the
+  `conclusion` job does not run, the finalizer's own API calls still fail after their retries,
+  the runner or GitHub is lost mid-run, or the finalizer cannot read the head SHA (the
+  conflicting-artifacts case above). Nothing signs off in any of these cases, because only
+  `success` does.
+- A push to a draft PR, or to one without `WaitForARMFeedback`, joins the PR concurrency lane
+  before the gate skips it, so it can cancel a review that is already running.
+
+## Changing the reviewer workflow
+
+1. Edit `arm-api-review.md`, then regenerate the lock file with the same gh-aw version recorded in
+   the lock file header (`gh aw compile arm-api-review`). Confirm `git diff` shows only intended
+   changes. The compiler may also prune unrelated entries in `.github/aw/actions-lock.json`;
+   revert those.
+2. A custom job cannot run after the agent: gh-aw rejects it as a dependency cycle. Post-agent
+   work goes in the built-in `conclusion` job via `jobs.conclusion.pre-steps`.
+3. `pre_activation` is the only place that can post `pending` before the agent starts.
+4. Keep the concurrency expression and the `if:` trigger gate consistent.
+5. The size cap exists in the prompt (Trigger Validation) and in `arm-semantic-review.ts`. Change
+   both together.
+
+## Verify a change
+
+```bash
+cd .github
+npx vitest run workflows/test/arm-auto-signoff workflows/test/arm-api-review-workflow.test.ts workflows/test/context.test.ts
+npx tsc --noEmit
+npx oxlint workflows
+npx oxfmt --check workflows
+```
+
+The tests assert the compiled lock file (job order, permissions, the finalizer condition), so a
+stale lock file fails them.
+
+Live check on a test PR (ideally in `azure-rest-api-specs-pr`), then compare to the tables above:
+
+```bash
+gh api repos/Azure/azure-rest-api-specs/commits/<head-sha>/statuses \
+  --jq '.[] | select(.context=="ARM Semantic Review") | [.created_at,.state,.description,.target_url]'
+```
+
+The `pending` status and the `head-sha=<sha>;issue-number=<n>` artifact appear on the run created
+by `pre_activation`. Try: a clean review, `skip-arm-review`, a push mid-run, two `/arm-review`
+comments in a row, and cancelling a run.
+
+## Debug a status that is stuck or wrong
+
+1. List the statuses for the head SHA (command above). Which `target_url` is newest, and is it
+   the run you expect?
+2. Open that run. Did `record_arm_semantic_review` run? It needs the agent and detection to
+   succeed and an emitted `record_arm_semantic_review` item.
+3. If it did not run, check whether the `conclusion` finalizer step ran. It is skipped when the
+   record job succeeded, and it acts only on a `pending` this exact run owns.
+4. If the description is `Review incomplete: result could not be validated`, read the warning in
+   the record job's log for the real reason.
+5. If Universal did not act, check its run log: it prints the event, correlation, labels, and the
+   status it read. A "state/head mismatch" means the PR head moved on.