Automate Architecture Board review process with GitHub Forms + Actions#10037
Automate Architecture Board review process with GitHub Forms + Actions#10037samvaity wants to merge 18 commits into
Conversation
- arch-board-review.yml: GitHub Forms template for API review/sign-off requests with structured language selection, per-language APIView/PR/README fields, and verification checklists - namespace-review.yml: Unified namespace review form for data plane and management plane, with plane-type routing (mgmt -> Arthur, data -> archboard) - arch-board-triage.yml: GitHub Actions workflow to auto-validate API review submissions, apply language labels, and label ready-for-review or needs-info - namespace-review-triage.yml: GitHub Actions workflow to route namespace reviews and validate namespace proposals Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add end-of-line anchor and multiline flag so 'C' only matches the exact checkbox line '- [X] C' and not '- [X] C++' etc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The workflow now actually fetches each URL (HEAD then GET fallback) to verify it returns HTTP 200. Invalid/unreachable links are flagged. Validation details are shown in collapsible sections on the comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of blindly trusting any apiview.dev URL, validates that the
URL contains a review ID pattern (/Review/{hex} or query params).
URLs like 'https://apiview.dev/' without a review ID are rejected.
Future Level 2: use APIView token API (GET /api/reviews/resolve)
to verify the review actually exists and has a diff revision.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
API Review template: - Match tone and structure of existing Azure SDK templates - Add section headers (## Contacts, ## About the Service, etc.) - Separate samples into a dedicated link field (not just a checkbox) - Add guidance about Tier-1/Tier-2 language groupings - Reference review process guidelines inline - Better placeholders with real examples Namespace Review template: - Professional naming conventions in table format - Include Java Maven group+package convention - Match title format to existing: 'Board Review: Namespace Review <service>' - Resource provider uses Microsoft.X format Workflow: - Update checklist validation for renamed fields - Add samples link validation (URL check or 'Uploaded in APIView') - Update diff revision checkbox text to match new template Config: - Add config.yml to disable blank issues - Add contact links for review guidelines and arch board email Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Each language now renders as one compact section instead of 5+ separate H3 headers. Format: ### .NET - APIView: <url> - Samples: <url or 'Uploaded in APIView'> - PR: <url> - README: <url> - [x] Diff revision is selected in APIView - [x] CI checks are passing on the PR Updated triage workflow parser to extract labeled URLs from textarea sections instead of separate input fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use placeholder (ghost text) only so unselected languages render as empty '_No response_' instead of showing blank checklist templates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Leverages GitHub Issue Forms (YAML-based form templates) instead of plain markdown templates. Key improvements: - Structured form fields: dropdowns, checkboxes, validated inputs - Service teams get a guided form experience instead of editing markdown - Add render: markdown to textarea fields for rich preview - Clearer field descriptions to guide submitters Docs: https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The following templates are superseded by the new Forms-based versions: - adp_api_review_and_signoff.md → arch-board-review.yml - adp_mgmt_namespace_review.md → namespace-review.yml Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…descriptions - Rename dropdown option to 'Release Sign-off' to match actual process - Add persistent description text above each language textarea listing all required artifacts (APIView, Samples, PR, README, CI checks) - Description stays visible even while typing, solving the UX concern that placeholder text disappears on focus - Remove old markdown templates (already done in prior commit) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes all issues found in code review:
1. Remove render:markdown from textareas — it wrapped content in code
fences which broke the workflow's regex-based URL extraction
2. Move diff revision and CI checks to proper GitHub Forms checkboxes
instead of requiring users to type '- [x]' in a textarea. The form
now enforces these as required checkboxes with a clean click UX
3. Fix duplicate bot comments — both workflows now upsert a single
comment using an HTML marker instead of creating new comments on
every edit
4. Fix namespace review header mismatch — workflow was looking for
headers like '.NET Namespace' but forms render labels as-is ('.NET')
5. Tighten plane-type regex — now scopes to content between header
and next section instead of greedy cross-section matching
6. Add Tier-2 language validation — checks that Additional Languages
textarea has content when C/C++/Android/iOS is selected
7. Enable blank issues — config.yml no longer blocks non-templated
issues for other contributors
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Architecture Board intake process by replacing legacy, manual email/markdown issue templates with GitHub Issue Forms and automated triage workflows that label/route requests and validate required inputs.
Changes:
- Add GitHub Issue Forms for Architecture Board API review/sign-off requests and for unified namespace reviews (data + management plane).
- Add GitHub Actions workflows to triage new/edited issues: route namespace reviews, apply labels/assignees, and validate required artifacts/checkbox confirmations.
- Remove legacy markdown-based Architecture Board issue templates and add a template chooser configuration.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/arch-board-triage.yml | Automates parsing language selections, validating artifacts/confirmations, and applying ready-for-review/needs-info with a bot comment. |
| .github/workflows/namespace-review-triage.yml | Routes namespace review issues by plane type, applies plane labels, and validates required per-language namespace entries. |
| .github/ISSUE_TEMPLATE/arch-board-review.yml | New Issue Form for API review / release sign-off requests with per-language artifact sections and confirmations. |
| .github/ISSUE_TEMPLATE/namespace-review.yml | New Issue Form for unified namespace reviews across planes with conventions reference tables and required fields. |
| .github/ISSUE_TEMPLATE/config.yml | Adds template chooser configuration and contact links for review guidance/help. |
| .github/ISSUE_TEMPLATE/adp_mgmt_namespace_review.md | Removes legacy management-plane namespace review markdown template. |
| .github/ISSUE_TEMPLATE/adp_api_review_and_signoff.md | Removes legacy API review/sign-off markdown template. |
1. SSRF protection: Add hostname allowlist (github.com, apiview.dev, learn.microsoft.com, azure.github.io) to prevent runners from probing internal/link-local endpoints via user-supplied URLs 2. Stale label removal: Language labels and plane-type labels are now synced on edit — deselecting a language or changing plane type removes the old label instead of only adding new ones 3. Fix Tier-1/Tier-2 guidance: Go is Tier-1 (not Tier-2), use '.NET' instead of 'C#' to match form field names 4. Fix unknown plane type: planeLabel now shows 'Unknown Plane' instead of defaulting to 'Data Plane' when detection fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub handles are the only actionable format on GitHub — needed for @mentions, assignments, and notifications. MS alias can be resolved from GitHub handle via internal endpoint if needed for Teams/email. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Based on team feedback: architects don't block on CI — they review API shape regardless. Diff revision remains required (wrong diffs are a real problem). CI checkbox is now optional in the form and produces a warning note instead of blocking ready-for-review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove Track 2 reference (no longer relevant after 7 years) - Fix Go tier: Tier-1 for mgmt plane only, clarify in guidance text - Rename REST APIs field to 'REST APIs / TypeSpec' - Remove 'Name of the Client Library' field (redundant with title) - Remove C, Android, iOS language options; add Rust - Update Additional Languages section to 'C++, Rust' - Swap namespace dropdown: Management Plane first (80% of reviews) - Fix JS mgmt convention: arm-[resource-provider-name] with hyphens - Add m-nash as secondary mgmt plane namespace reviewer - Update title placeholder to '<service name>' - Update workflow language map and tier-2 list to match Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add PROCESS.md documenting both review processes and approval lifecycle - Label-based approval tracking: per-language namespace and API approval labels - Approval labels are manually applied by architects, automation auto-closes - Remove APIView triage checks table (APIView is being sunset) - Remove future enhancements section (approval labels are current scope) - Remove language pairing sentence per Jonathan's feedback - Remove Review Type dropdown (API Review vs Release Sign-off distinction removed) - Template renamed to just 'API Review' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f76bae2 to
a0041f8
Compare
- New workflow triggers on issue labeled events
- API Review: closes when all {lang}-api-approved labels present
- Namespace (mgmt): closes when mgmt-namespace-ready applied
- Namespace (data): closes when all {lang}-namespace-approved present
- Validates label was applied by authorized architect per language
- Unauthorized labels are automatically removed with a comment
- Authorized approvers: dotnet (jsquire, m-nash), java (JonathanGiles,
alzimmermsft), python (xirzec), typescript (xirzec, ArcturusZhang),
go (ArcturusZhang, jhendrixMSFT, RickWinter), cpp (ArcturusZhang,
RickWinter, LarryOsterman), rust (heaths, ArcturusZhang, RickWinter,
LarryOsterman), mgmt (ArthurMa1978, m-nash)
- PROCESS.md: namespace is one-time, API review required per GA only
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
|
||
| This document describes the two review processes automated by the GitHub Forms templates and triage workflows in this directory. | ||
|
|
||
| ## Overview |
There was a problem hiding this comment.
Can you add a list of all labels that exists in the overview? This way we get a centralized list, something like:
- mgmt-namespace-review: Auto-applied by flow if Mgmt name review is detected
- data-namespace-review: Auto-applied by flow if DP name review is detected
- ready-for-review: Auto-applied when all checks are validated
- mgmt-namespace-ready: Manually applied by ARM approver after review completion
- etc.
Edit: I see there is a table at the end, I feel I would want to see it at the top
| - Detects plane type (Management or Data) from the dropdown | ||
| - **Management Plane**: auto-assigns @ArthurMa1978 and @m-nash, applies `mgmt-namespace-review` label | ||
| - **Data Plane**: applies `data-namespace-review` label (goes to full archboard) | ||
| - Validates all 5 language namespace proposals are provided (.NET, Java, Go, JavaScript, Python) |
There was a problem hiding this comment.
Should we validate the exact language that the service team asks for? Are we always validated all?
Do we care that Go is not a tier1 language in dataplane?
There was a problem hiding this comment.
We require all 5 because namespace consistency across languages matters even if a team ships .NET only today, the Java/Python/etc. package names should be decided upfront so they're consistent when another team picks them up later. This matches how it was done manually.
For Go in data plane the Go package name still needs to follow a convention and be consistent with the other languages, even if it's not Tier-1 for review purposes.
| |-------------|---------------|---------------------|---------| | ||
| | Namespace (per language) | `{lang}-namespace-approved` | Language architect | `java-namespace-approved`, `dotnet-namespace-approved` | | ||
| | Namespace (mgmt gate) | `mgmt-namespace-ready` | Arthur/Michael | Management plane namespace approval | | ||
| | API Review (per language) | `{lang}-api-approved` | Language architect | `java-api-approved`, `dotnet-api-approved` | |
There was a problem hiding this comment.
Is API review for ARM our architects?
| │ | ||
| ▼ | ||
| ┌─────────────────┐ | ||
| │ Namespace Review │ "What should the library be called?" |
There was a problem hiding this comment.
Is separate namespace review request required or can namespace review be part of tspconfig.yaml PR review when emitter configuration is changed? Spec PR with emitter config change can be labelled as namespace review required using a GitHub action and blocks PR merge until approver adds the label to mark it as approved.
There was a problem hiding this comment.
A separate namespace review issue is needed because it's an independent architectural decision, involves different reviewers, and ensures clear tracking and approval. Bundling it with a spec PR could cause confusion and make the process less reliable.
That said, we can add an automation to link the two as a future enhancement could detect emitter config changes in a spec PR and comment "Have you filed a namespace review?" with a pre-filled link to the form.
I think this process aligns with no to little adjustment from today's process so it should be easier to adapt and follow, I think.
Authorized approvers updated per Laurent's feedback: - Python: add kashifkhan - TypeScript: add maorleger - Remove RickWinter and ArcturusZhang (Shanghai/mgmt covered by Arthur) - ArthurMa1978/m-nash can approve API reviews on mgmt plane issues PROCESS.md restructured per Laurent's request: - Labels Reference section moved to top with all labels listed - Split into auto-applied vs architect-applied tables - Shows specific authorized approvers per language - Documents mgmt plane API review approval rule Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| ### Architect-applied approval labels (validated by `approval-close.yml`) | ||
|
|
||
| | Label | Authorized approvers | Purpose | | ||
| |-------|---------------------|---------| | ||
| | `dotnet-api-approved` | jsquire, m-nash | .NET API surface approved | | ||
| | `java-api-approved` | JonathanGiles, alzimmermsft | Java API surface approved | | ||
| | `python-api-approved` | xirzec, kashifkhan | Python API surface approved | | ||
| | `typescript-api-approved` | xirzec, maorleger | TypeScript API surface approved | | ||
| | `go-api-approved` | jhendrixMSFT | Go API surface approved | | ||
| | `cpp-api-approved` | LarryOsterman | C++ API surface approved | | ||
| | `rust-api-approved` | heaths, LarryOsterman | Rust API surface approved | | ||
| | `{lang}-namespace-approved` | Same as above per language | Namespace approved | | ||
| | `mgmt-namespace-ready` | ArthurMa1978, m-nash | Management plane namespace approved | |
There was a problem hiding this comment.
Are these labels in the specs repo, or the language repos? How are we tracking who the authorized approvers are?
There was a problem hiding this comment.
These labels are in the azure-sdk repo (where this PR lives). The authorized approvers are maintained in the approval-close.yml workflow. It validates the GitHub user who applied the label against a per-language allowlist and automatically removes unauthorized labels.
|
|
||
| ### Flow | ||
|
|
||
| 1. Service team files an **API Review** issue using the form |
There was a problem hiding this comment.
Where is this form? What are the required fields? Do we expect service teams to provide APIView links? With APIView going away, most likely the submission of this form would trigger the creation of the API Review PR, and it would need to be updated once the PR is created.
There was a problem hiding this comment.
The form is arch-board-review.yml in this PR. You can preview it on the fork: https://github.com/samvaity/azure-sdk/issues/new?template=arch-board-review.yml
Currently the form asks for APIView links, PR links, README links, and samples per language. With APIView going away, these fields will evolve. The form can be updated to ask for the GitHub Review PR link instead. The template is just YAML, easy to adjust.
There was a problem hiding this comment.
Follow-up: To be clear, the API review form (arch-board-review.yml) is a bridge. The Power Apps scheduler that created API review issues is being shut down, and the PR review system is not live yet. Once your pipeline creates review PRs and auto-assigns architects, we retire this form.
| 3. If `needs-info`: service team edits the issue (updates APIView link with new revision, fixes missing items) → workflow re-validates automatically | ||
| 4. Once `ready-for-review`: architects review async in APIView | ||
| 5. Back-and-forth happens on the **same issue** — service team edits, bot re-validates, architects re-review | ||
| 6. Architects approve per language → apply per-language labels (e.g., `java-api-approved`, `dotnet-api-approved`) |
There was a problem hiding this comment.
Why is this necessary? If the API is already approved in APIView, why would they need to add this label? In the future, when there's no APIView, the fact that the Review PR is approved would mean the API is approved--again, no need to add a label here. And the approved hash would be stored in the ADO (though it could be stored here...) and the release process needs to validate this approved hash, not merely look for a "api-approved" label.
Otherwise, the label gets added, then a new commit sneaks in, and unapproved changes get released.
There was a problem hiding this comment.
The label serves as the approval signal that works independently of any specific review tool. With APIView going away and reviews moving to GitHub PRs, we need a durable, tool-agnostic way to track per-language approval. The label says 'I, the architect for this language, have reviewed and approved.'
You raise a valid point about post-approval changes. In the future, this could be enhanced to verify against a commit hash or PR approval status rather than just a label. For now, the authorized approver validation (only designated architects can apply the label) provides the guardrail.
There was a problem hiding this comment.
Follow-up: After discussing with the team, we are aligning on this direction. For API review, you are right that the approved hash in ADO is the durable signal and per-language labels are redundant once your PR review system goes live. We are treating the API review template as a bridge for intake while your system is still in progress. Once GitHub PR reviews go live, we retire arch-board-review.yml and the associated labels. Namespace review stays as a separate workflow since it is explicitly out of scope for your proposal.
| 4. Once `ready-for-review`: architects review async in APIView | ||
| 5. Back-and-forth happens on the **same issue** — service team edits, bot re-validates, architects re-review | ||
| 6. Architects approve per language → apply per-language labels (e.g., `java-api-approved`, `dotnet-api-approved`) | ||
| 7. Automation closes issue when all required API approval labels are present ✅ |
There was a problem hiding this comment.
If the point of opening the issue is to track the release of GA version X, it makes more sense to me to close this issue once the package is released, not merely approved.
There was a problem hiding this comment.
Good thought. For now, this issue tracks the review/approval lifecycle specifically: 'has the API been approved by the right architects?' Once approved, the release itself is tracked separately (release plans, package publishing). We could add a 'released' label later that triggers final closure if we want to extend the lifecycle to cover shipping too.
| - Posts a bot comment with ✅/❌ details per language | ||
| 3. If `needs-info`: service team edits the issue (updates APIView link with new revision, fixes missing items) → workflow re-validates automatically | ||
| 4. Once `ready-for-review`: architects review async in APIView | ||
| 5. Back-and-forth happens on the **same issue** — service team edits, bot re-validates, architects re-review |
There was a problem hiding this comment.
I'm not sure what review would take place on an issue? The discussion of an API would take place on the API Review PR, not an issue.
There was a problem hiding this comment.
The issue is the tracking envelope. It collects the links, validates artifacts are present, tracks which languages have been approved. The actual review discussion happens wherever the API is reviewed (currently APIView, moving to GitHub PRs). The back-and-forth on the issue is about updating links and re-validating, not the API discussion itself.
| - Applies `ready-for-review` or `needs-info` label | ||
| - Posts a bot comment with ✅/❌ details per language | ||
| 3. If `needs-info`: service team edits the issue (updates APIView link with new revision, fixes missing items) → workflow re-validates automatically | ||
| 4. Once `ready-for-review`: architects review async in APIView |
There was a problem hiding this comment.
In the not to distant future, these reviews will be happening async in Github PRs.
There was a problem hiding this comment.
Understood. This issue serves as the intake and tracking mechanism for the review request. The actual API review discussion can happen in the GitHub Review PR (or wherever it moves to). The issue tracks: which languages need approval, who has approved, and when everything is complete.
| - Pull Request link (HTTP reachability check) | ||
| - README link (HTTP reachability check) | ||
| - Validates Tier-2 languages (C++, Rust) have content in the Additional Languages section | ||
| - Checks required confirmation: diff revision selected in APIView |
There was a problem hiding this comment.
...or GitHub Review PR (which is always a diff)
There was a problem hiding this comment.
Good call. When reviews move to GitHub PRs, the diff is inherent in the PR itself. This checkbox can be replaced with a link to the GitHub Review PR at that point.
| - APIView link (with review ID format check) | ||
| - Samples link (or "Uploaded in APIView") |
There was a problem hiding this comment.
These should account for the new future where APIs are reviewed in GitHub, not APIView. I'll take it as a lookup for myself how to handle samples (though right now I assume they will be part of the API review PR).
There was a problem hiding this comment.
Agreed. These fields are designed for the current APIView-based workflow but will need to evolve. When reviews move to GitHub PRs, the form can be updated to ask for the Review PR link instead of APIView link, and samples could be validated as part of the PR itself. The template is just YAML so these changes are straightforward.
|
|
||
| ### What it does | ||
|
|
||
| Reviews the API surface (methods, models, parameters) before a new **GA release** with API changes. Beta/preview releases do not require API review. This review is required for each new GA version — unlike namespace review, it is not a one-time process. |
There was a problem hiding this comment.
So do service teams have to create release plans AND create these issues? I'm not super familiar with this wider process.
There was a problem hiding this comment.
This issue replaces the email to azsdkarch-help@microsoft.com that service teams currently send. It's specifically for the architecture board review process (namespace + API surface approval). Release plans are a separate tracking mechanism. This issue replaces the manual intake process that Ronnie was doing.
Architecture Board Review Automation
Replaces the manual email-based Architecture Board review process with GitHub Forms templates and automated triage workflows.
What this PR adds
3 GitHub Forms templates:
arch-board-review.yml— API Review request form (replaces email to azsdkarch-help@microsoft.com)namespace-review.yml— Unified namespace review form (Management + Data Plane)config.yml— Template chooser with blank issues enabled3 GitHub Actions workflows:
arch-board-triage.yml— Validates API review submissions (URLs, artifacts, confirmations)namespace-review-triage.yml— Routes namespace reviews and validates proposalsapproval-close.yml— Watches for architect approval labels, validates authorized approvers, auto-closes issuesDocumentation:
PROCESS.md— Documents both review processes, approval label taxonomy, and authorizationHow it works
API Review flow
ready-for-revieworneeds-infolabel + posts validation summaryjava-api-approved)Namespace Review flow
mgmt-namespace-reviewdata-namespace-reviewfor archboardSecurity
Approval label taxonomy
{lang}-api-approved{lang}-namespace-approvedmgmt-namespace-ready