Integrate ARM semantic review with auto-signoff by AkhilaIlla · Pull Request #46921 · Azure/azure-rest-api-specs

AkhillaIlla (AkhilaIlla) · 2026-10-01T04:53:02Z

Summary

publish a head-bound ARM Semantic Review result from the ARM API Reviewer
require a Passed semantic result before Universal ARM Auto-Signoff proceeds
route scoped or truncated reviews to manual signoff and reject stale workflow results

Validation

compiled arm-api-review.md with gh-aw v0.86.2
passed the full .github test suite, formatting, TypeScript checks, and targeted lint

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-10-01T04:54:03Z

Next Steps to Merge

✅ All automated merging requirements have been met! To get your PR merged, see aka.ms/azsdk/specreview/merge.

Comment generated by summarize-checks workflow run.

Timothee Guerin (timotheeguerin)

Two issues should be addressed before merge: preserve head-bound semantics when correlation artifacts are missing, and reuse GitHub API responses across semantic finalization and auto-signoff.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Identify reviewer runs by their stable workflow path so dynamic run names do not suppress status artifacts. Fail visibly for unexpected workflow sources and cover the behavior with regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Source PR, head SHA, and run-attempt correlation from trusted workflow artifacts and environment data instead of model output. Keep the model result semantic-only and add regression coverage for malformed model identifiers.

Embed the Critic instructions, protocol, and input template into the checkout-free runtime agent. Scope incomplete-review issue creation to the canonical repository and add structured semantic and auto-signoff decision logging with regression coverage.

- Resolve a Pending ARM Semantic Review status from the conclusion job when the record job never publishes (noop, agent or detection failure, cancel). - Split error outcomes: only scoped or oversized reviews are published as "Manual review required" and add ARMManualSignoffRequired; incomplete or failed reviews are "Review incomplete" and add no label. - Give PR events the trigger gate skips a run-scoped concurrency group so they cannot cancel an in-flight review. - Check for a newer run immediately before publishing the status. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Skipped PR events no longer get a run-scoped concurrency group. The extra condition duplicated the trigger gate for a narrow, low-impact case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Document every helper, type, and constant in the ARM semantic review policy, publisher, and universal auto-signoff code. - Simplify validation: drop duplicate correlation, PR-head, and newer-run checks; publish a fixed Review incomplete description and log error details instead of echoing them into the status. - Two-tier PR size check: aggregate counters first, then the specification/ subset only when the totals exceed the cap. - Skip the conclusion finalizer when the record job already published. - Document the trusted-artifact helpers and drop the ARM-specific path guard from the generic context code. - Add tests for the not-ready, non-Error, unset output path, and unknown status branches. - Add the arm-semantic-review skill describing the design, contract, invariants, and debugging steps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- Replace the long documentation/ implementation guide with a short README next to the code, and point the skill at it. - Use a YAML block sequence for the Universal workflow's upstream list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Timothee Guerin (timotheeguerin) · 2026-10-08T17:53:45Z

    );
  }

+  if (


is this related to this pr functionality? It seems this might just be a part of this larger fix I have here which prevent our workflow from running on closed PRs
#46996

Agreed, this is cross-cutting and overlaps #46996, so I removed it from this PR in dd85a0f (update-labels.ts and its test are back to main). The guard changed behavior for every producer that publishes a head-sha= artifact. Universal already re-checks that the PR is open and still at the evaluated head before it emits a label action, so the main case stays covered. A head-SHA check in Update Labels can follow as a separate change, ideally on top of your getOpenPullRequest. I noted the small remaining race (a push between Universal's check and Update Labels) in the skill's known limitations.

Timothee Guerin (timotheeguerin) · 2026-10-08T18:01:01Z

+  if (pullRequest.changed_files < 1) {
+    return true;
+  }
+  const changedFiles = await github.paginate(github.rest.pulls.listFiles, {


We probably don't need to paginate here if you already going to be above the limit of files after the first page

Done in dd85a0f. The size check costs one pulls.get call, which is all most PRs need. Only when the PR's totals are over the limits does it list files, and it now stops paginating at the first page where the specification/ changes are already over 50 files or 5,000 lines (more pages can only add to the totals). The rules are two small pure helpers in arm-semantic-review.ts and the workflow only does the API calls and logging. Tests cover both paths (stops after page 1, and keeps going when the extra files are outside specification/).

Timothee Guerin (timotheeguerin) · 2026-10-08T18:01:03Z

+        path: "${{ runner.temp }}/arm-semantic-review/empty.txt"
+        if-no-files-found: error
+        overwrite: true
+    - name: Upload ARM semantic review issue number


Can these two correlation values share one encoded artifact? Both steps upload the same empty file, so combining them removes one upload-artifact startup and network round trip per review.

Done in dd85a0f. pre_activation now uploads one artifact named head-sha=<sha>;issue-number=<n>, saving an upload-artifact step per review. ; is valid in artifact names. parseWorkflowRunArtifactInputs splits each name on ; before reading the key=value pairs, so existing single-value artifacts parse unchanged and a combined name goes through the same SHA and PR-number validation. That parser was the only consumer of these names. Tests cover a combined artifact, an invalid SHA inside one, and a conflict with a separate artifact.

Ravi Eda (ravimeda) · 2026-10-08T22:50:04Z

+      run_attempt: ${{ steps.resolve_target_pr.outputs.run_attempt }}
+      target_head_sha: ${{ steps.resolve_target_pr.outputs.target_head_sha }}
      target_pr_number: ${{ steps.resolve_target_pr.outputs.target_pr_number }}
+  # Pre-activation posts a Pending `ARM Semantic Review` status, but the record job


What happens where there is an exception? Can the workflow remain stuck in Pending state?

Mostly no, and I closed the realistic gaps in dd85a0f. Where an exception goes: before the agent, no pending has been posted yet, so nothing is stuck. In the agent, detection, or the record job's validation, the status becomes Review incomplete, either from the record job or from the conclusion finalizer (which only acts on a Pending this exact run owns). New: both github-script steps that write the status now use retries: 3, so a one-off API error no longer leaves Pending. It can still remain if the conclusion job does not run, the finalizer's calls fail even after retries, the runner/GitHub is lost mid-run, or after a re-run on a new push leaves conflicting head-sha= artifacts. Nothing signs off in any of those cases, because only success does. They are listed under known limitations in the skill.

Ravi Eda (ravimeda) · 2026-10-08T22:51:53Z

+    return true;
+  }
+  const specificationFiles = changedFiles.filter((file) =>
+    file.filename.startsWith("specification/"),


What if a file was renamed/moved?

Good catch, fixed in dd85a0f. The file list reports a renamed or moved file's new path in filename and its old path in previous_filename, so a file moved out of specification/ was missed. A file now counts as a specification change if either path is under specification/ (counted once, so a rename within specification/ is not doubled). This is in addSpecificationChanges in arm-semantic-review.ts, with tests for moved in, moved out, renamed within, and renames that never touch specification/.

Ravi Eda (ravimeda) · 2026-10-08T22:54:13Z

+          type: string
+      steps:
+        - uses: actions/checkout@v7
+          with:


I see a persist-credentials: false being used in other spots. Better to apply that here too.

Done in dd85a0f. persist-credentials: false is now set on the record job's checkout and on the conclusion pre-step checkout. The recompiled lock file shows it once on each, with no duplicate key.

- Verify the reviewer's scope claim with one pure helper pair in arm-semantic-review.ts (checkPullRequestSize, addSpecificationChanges). Only a "full, complete" claim is verified; a PR within the totals needs no file list, otherwise only specification/ changes are counted, stopping at the first page already over the limits. A rename counts by its old or new path. The status states the reason, and each step logs what it saw. - Upload one head-sha=<sha>;issue-number=<n> artifact instead of two, and split names on ";" in the shared artifact parser. - Retry transient API errors in the record and finalizer steps, and set persist-credentials: false on their checkouts. - Drop the Update Labels stale-head guard from this PR; it changes every producer that publishes a head-sha artifact and overlaps #46996. - Update the skill and README to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Integrate ARM semantic review with auto-signoff

98da746

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AkhillaIlla (AkhilaIlla) requested review from a team, Evan Hissey (MSEvanhi), Suhas Rao (ARM) (raosuhas) and Tejaswi Salaigari (tejaswiMinnu) as code owners October 1, 2026 04:53

Timothee Guerin (timotheeguerin) requested changes Oct 1, 2026

View reviewed changes

Comment thread .github/workflows/src/context.ts Outdated

Comment thread .github/workflows/src/arm-auto-signoff/arm-semantic-review-workflow.ts Outdated

Akhila Illa and others added 2 commits October 5, 2026 10:05

Isolate ARM semantic status publication

a620337

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Address semantic review feedback

a4acd7d

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Timothee Guerin (timotheeguerin) reviewed Oct 5, 2026

View reviewed changes

Comment thread documentation/arm-semantic-review-auto-signoff-implementation.md Outdated

Timothee Guerin (timotheeguerin) reviewed Oct 5, 2026

View reviewed changes

Comment thread .github/workflows/arm-universal-auto-signoff.yaml Outdated

Timothee Guerin (timotheeguerin) reviewed Oct 5, 2026

View reviewed changes

Comment thread .github/workflows/arm-semantic-review-status.yaml Outdated

Timothee Guerin (timotheeguerin) reviewed Oct 5, 2026

View reviewed changes

Comment thread .github/workflows/arm-semantic-review-status.yaml Outdated

Akhila Illa and others added 9 commits October 6, 2026 10:42

implmentation cleanup, optimizations

1e42a2d

Use trusted ARM review correlation metadata

d0f96d5

Source PR, head SHA, and run-attempt correlation from trusted workflow artifacts and environment data instead of model output. Keep the model result semantic-only and add regression coverage for malformed model identifiers.

Merge main into akhilailla/implement-arm-semantic-review

99c8922

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Revert concurrency rule to the simple version

e6b44ef

Skipped PR events no longer get a run-scoped concurrency group. The extra condition duplicated the trigger gate for a narrow, low-impact case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Recompile ARM API review lock after merging main

672c8bb

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AkhillaIlla (AkhilaIlla) requested review from a team, Arthur Ma (ArthurMa1978), Crystal YU (chunyu3), Haoling Dong (haolingdong-msft) and Renhe Li (lirenhe) as code owners October 8, 2026 05:59

Timothee Guerin (timotheeguerin) and others added 2 commits October 8, 2026 13:49

Merge branch 'main' into akhilailla/implement-arm-semantic-review

f3673b7

Timothee Guerin (timotheeguerin) reviewed Oct 8, 2026

View reviewed changes

Ravi Eda (ravimeda) reviewed Oct 8, 2026

View reviewed changes

Suhas Rao (ARM) (raosuhas) approved these changes Oct 9, 2026

View reviewed changes

                   );
                 }
+                if (

Conversation

AkhillaIlla (AkhilaIlla) commented Oct 1, 2026

Summary

Validation

Uh oh!

github-actions Bot commented Oct 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Next Steps to Merge

Uh oh!

Timothee Guerin (timotheeguerin) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Oct 1, 2026 •

edited

Loading