Skip to content

Assign per-language architects when a review is ready - #14

Closed
samvaity wants to merge 21 commits into
mainfrom
arch-board-assign-reviewers
Closed

samvaity wants to merge 21 commits into
mainfrom
arch-board-assign-reviewers

Conversation

@samvaity

@samvaity samvaity commented Jul 22, 2026 •

Copy link
Copy Markdown
Owner

What

When triage marks a board-review issue ready-for-review, automatically assign one architect per selected language so they are notified through GitHub's native notifications instead of having to watch the project board.

Why

The project board is visibility-only today - there is no assignment or notification. An architect recently missed a ready review (release-dated) because they were too busy to poll the board. Assignment routes each review to a real person through their existing GitHub notification preferences, with nothing custom to maintain.

How

  • New assign-reviewers.js: resolves one architect per selected language from the shared api-review-approvers.yml roster (round-robin by issue number), reusing the approval-check.js helpers. Management-plane issues also consider the management-plane approvers.
  • triage.js: on ready-for-review, assigns via addAssignees (this is the notification), then lists the assigned handles inside the existing "All materials verified" comment - a single comment, no extra ping.
  • Idempotent: re-edits of an already-assigned issue do not re-assign or re-notify.

Design note

Inspired by the config-driven, plane-aware pattern in Azure/azure-rest-api-specs#44664, but reuses our existing roster and helpers rather than copying check-label.js (which does label authorization, not assignment). Single source of truth for the architect list, no drift.

xirzec and others added 21 commits June 22, 2026 17:06
Co-authored-by: azure-sdk <azuresdk@microsoft.com>
Co-authored-by: Jeff Fisher <jeffish@microsoft.com>
…path (Azure#10077)

Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
)

* Update tsp-client Node engine requirement to >=22.0.0

* update package lock json

---------

Co-authored-by: Minh-Anh Phan <111523473+minhanh-phan@users.noreply.github.com>
Removed a deprecated entry for the networkcloud resource manager and updated related specifications.
Removed one assignee from the Namespace Review issue template.
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
)

* update tsp-client to 0.33.1

* bump min node to 22.13.0 - align with transitive deps

---------

Co-authored-by: Mike Harder <mharder@microsoft.com>
)

* Add pipeline step template to upload test result or llm-facing artifacts

* Make common llm-artifacts template support junit emitting repos

* Only set upload flag when artifacts are found

---------

Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
Co-authored-by: Chidozie Ononiwu <chononiw@microsoft.com>
…tions (Azure#10037)

* Add arch board review automation (Phase 1)
The new automated arch-board-review.yml template (merged in PR Azure#10037)
replaces this template. Both appeared in the issue chooser with similar
names causing confusion for service teams.
)

* Port Vally eval CI scripts from PowerShell to JavaScript

Rewrite all eval pipeline glue scripts as dependency-free ESM with node:test unit tests, under eng/common/scripts/eval/:

- collect-stimuli.js (+ lib/glob.js) <- Split-EvalSuite.ps1

- invoke-eval-shard.js + lib/verdict.js <- Invoke-EvalShard.ps1 + EvalVerdict.Helpers.ps1

- build-eval-summary.js <- Build-EvalSummary.ps1

- sync-eval-git-repo.js <- Sync-EvalGitRepo.ps1

- init-eval-git-fixtures.js <- Initialize-EvalGitFixtures.ps1

Delete all .ps1 + Pester tests; rewire vally-eval.yml to invoke the JS via node; update README/.vally.yaml/eval-comment references. 40 node:test cases pass.

* Split Vally eval pipeline into reusable templates

Extract the monolithic vally-eval.yml into modular, parameterized templates under eng/common/pipelines/templates (synced to other repos), leaving a thin repo-local entrypoint:

- steps/vally-build-mock.yml: build + publish the mock MCP DLL
- steps/vally-collect-stimuli.yml: glob eval files -> shard matrix (repo + optional common root)
- steps/vally-run-shard.yml: npm ci + download mock + prime git fixtures + run shard + publish JUnit/transcripts
- steps/vally-summarize.yml: aggregate shard JUnit + gate rollup
- stages/archetype-vally-eval.yml: Eval stage (BuildMock || Detect -> RunShard matrix, customMcpBuildJobs hook) + Summary stage

Drop the redundant global 'npm install -g @github/copilot-sdk': it is a locked transitive dep of @microsoft/vally-cli, so npm ci in eng/skill-eval already installs the executor (D9: pin via lockfile, no global installs).

Migrate all jobs to the managed pool (name: $(LINUXPOOL) / image: $(LINUXVMIMAGE) / os: linux); the repo-local entrypoint supplies the image.yml variables and the variable group.

* Generalize Vally eval templates for skill + live verticals; fix missing lib modules

Make the shared archetype/templates MCP-agnostic so the skill-compliance and
live workflow-scenario evals can reuse the same shard fan-out as the mock tier:

- collect-stimuli.js: derive file-mode shard names from the full relative path
  so per-skill <skill>/evals/trigger.eval.yaml files no longer collide.
- vally-build-mock.yml -> vally-build-mcp.yml: build a LIST of MCP servers and
  publish them under a common stage root.
- vally-collect-stimuli.yml: accept an explicit list of eval glob patterns.
- vally-run-shard.yml: rename mock* params to mcp*; optionally wrap the eval in
  AzureCLI@2 (live tier) so the agent's DevOps calls inherit the service
  connection, else run as a plain script (hermetic tiers).
- archetype-vally-eval.yml: swap mock-specific params for mcpBuilds/mcpArtifact/
  mcpStageRoot/patterns/azureServiceConnection and wire them through.

Add two thin entrypoints:
- eng/pipelines/skill-eval-shards.yml: sharded variant of skill-eval.yml
  (builds cli + mock, one shard per skill).
- eng/pipelines/live-eval.yml: nightly scheduled live run (builds cli,
  authenticated via opensource-api-connection).

Fix the pipeline ERR_MODULE_NOT_FOUND failure: the root .gitignore's Python
`lib/` rule silently excluded eng/common/scripts/eval/lib/{glob,verdict}.js, so
the pipeline checkout was missing them. Add a targeted negation and track the
two modules.

Defaults preserve the existing mock vertical; vally-eval.yml only updates the
renamed build-template trigger paths.

* polish and rename

* Switch Vally eval CI to 1ES; quiet teardown-noise warnings

Pipeline (1ES migration, localized to the archetype):
- archetype-vally-eval.yml now extends /eng/pipelines/templates/stages/1es-redirect.yml
  (chained extends, like archetype-sdk-tool-dotnet.yml). Adds Use1ESOfficial param.
- 1ES Official forbids the `publish:` shortcut, so the three artifact publishes move to
  job-level templateContext.outputs: MCP server artifact on BuildMcp; per-shard
  eval-result-$(shardName) (JUnit, always) and eval-debug-$(shardName) (transcripts, on
  failure) on the RunShard matrix job.
- Drop the publish steps from vally-build-mcp.yml / vally-run-shard.yml (keep the staging
  script steps) and remove the now-unused stageRoot/artifactName params from
  vally-build-mcp.yml. Clears the 17 [1ES PT] warnings; CG is opt-in under 1ES.

Shard gate (invoke-eval-shard.js):
- Demote the two "passing verdict" warnings (hadExecutionErrors and non-zero vally exit)
  from ##vso build warnings to plain log lines. Both are the same expected post-run
  agent-teardown noise and say nothing about the eval; the results.jsonl verdict is
  authoritative. Removes 8 confusing build annotations.
- Replace em-dash with ASCII hyphen in the task.logissue strings (rendered as mojibake
  in ADO annotations).

Earlier review fixes (carried in this PR):
- collect-stimuli.js: guard against area-mode shard-name collisions (matches file mode);
  add a regression test.
- vally-run-shard.yml: prime git fixtures from this shard's own evalArgs (--pattern per
  -e file) instead of the built-in defaults.

* Fix npm ci EPERM under 1ES by restoring via azure-sdk npm mirror

The 1ES Official managed image blocks direct egress to registry.npmjs.org,
so the shard's `npm ci` failed with EPERM ("rejected by your operating
system"), which cascaded into the collect-JUnit find errors. Restore the
Vally CLI through the allowlisted azure-sdk Azure Artifacts npm mirror via
an authenticated .npmrc (the same pattern typespec-author-eval-setup uses).

* Simplify eval archetype: build Mock+Cli MCP by default, drop never-overridden params and mcpBuilds

* Rework MCP build to useCustomMcp + runtime repo detection per review

* Copy Vally CLI pin to eng/common/vally-eval and repoint eval pipeline

Adds eng/common/vally-eval (renamed from skill-eval) so the pinned @microsoft/vally-cli lockfile syncs via eng/common. Repoints the Vally eval pipeline (vally-eval.yml, skill-eval-2.yml triggers; vally-run-shard skillEvalDir default; invoke-eval-shard comment) to the new path. eng/skill-eval is left intact for legacy GitHub Actions and the going-away skill-eval.yml; it can be removed later.

* Move eval-mcp-setup.yml step template into eng/common

Co-locates the custom-MCP override step template with the other vally-* eval step templates in eng/common/pipelines/templates/steps and repoints the archetype useCustomMcp branch + comments.

* Repoint eval-mcp-setup.yml references to eng/common path

* Eval CI: move entrypoints/templates to eng/common, rename vally-* templates to eval-*, extract generate-eval-matrix + eval-shard jobs, merge eval package.json, drop variable group, add maxParallel

* Eval CI: make eng/common templates MCP- and auth-agnostic

- Replace MCP build coupling with a single mcpSetupTemplate path param; move the actual builds to repo-local eng/pipelines/eval/mcp-mock.yml + mcp-live.yml and delete the shared eval-build-mcp.yml.

- Remove azureServiceConnection param; the live tier's AzureCLI@2 wrapping now lives in a repo-local evalInvokeTemplate (eval-invoke-live.yml). Default is the hermetic node runner eval-invoke.yml. This single run-time hook also covers chatbot/custom-MCP startup.

- Convert shardTimeoutInMinutes from a param to an entrypoint-declared variable (hermetic 20, live 45); document the required-variable contract in the archetype + shard job.

- Pass vallyRoot explicitly from every entrypoint; update trigger path filters for the new templates.

- Fix sync-eval-git-repo.js cold-cache clone: init + fetch <ref> + checkout FETCH_HEAD so branches, tags, and SHAs all resolve (matches the refresh path), not just the remote default branch.

* Simplify eval pipeline templates: inline non-varying parameters

Audit and remove pipeline parameters that no caller overrides, inlining the constant value instead. Make the archetype evalGlobs default explicit and verified equivalent to the collect-stimuli.js fallback. Fix latent shardBy and githubTokenVariable passthrough bugs.

* Flatten eval pipeline to stages -> jobs (steps inline)

Make archetype-eval.yml a pure orchestrator (only job-template references, no inline steps) and fold each single-use steps template into its owning job template. Add build-mcp.yml and eval-summarize.yml job templates; delete the merged eval-collect-stimuli, eval-run-shard, and eval-summarize step templates. Only the two swap-point step templates (mcpSetupTemplate, evalInvokeTemplate) remain. Update trigger path-filters to templates/jobs/**.

* Trim verbose comments across eval pipeline and scripts

Reduce oversized comment blocks to terse basic summaries: collapse multi-paragraph JS file headers, drop ported-from-PowerShell references and why/teardown rationale, and remove Phase/D2/D3 jargon from the pipeline YAML headers.

* Reject eval paths with whitespace; note JUnit parse coupling

Fail fast in collect-stimuli when a discovered eval path contains whitespace, so a bad path errors at matrix time instead of mis-splitting the space-delimited -e args downstream. Add a comment in build-eval-summary flagging the regex coupling to Vally's JUnit output.

* Make shard timeout a compile-time parameter

1ES requires job timeoutInMinutes to be an integer at compile time, so it cannot read a runtime variable macro. Thread shardTimeoutInMinutes as a number parameter (default 20) from each entrypoint through the archetype to the shard job; live tier sets 45.

* Map copilot PAT into GITHUB_TOKEN env for eval invoke steps

* Attach eval variable group to entrypoint pipelines so copilot PAT resolves

* Land eval cache on a local branch so worktree fixtures resolve the ref

* Consolidate eval invoke into one auth-conditional template and tidy shard/MCP seams

- Fold eval-invoke-live.yml into eval-invoke.yml via UseAzSdkAuthentication flag (AzureCLI@2 vs hermetic script); delete the live copy

- Derive npm --prefix from the script dir in invoke-eval-shard.js; drop the skill-eval-prefix arg/param everywhere

- Make vallyRoot required (no default) in archetype + shard; thread UseAzSdkAuthentication through

- Move checkout into the BuildMcp job; remove it from the MCP setup templates; make the example MCP template a mock/live if-else

- Add pre/post eval step hooks and blank-line spacing in eval-shard.yml

* Add eval hooks, parameterize gate threshold, require mcpSetupTemplate

- Add eval-hook-example.yml placeholder documenting pre/post-eval hooks; surface commented preEvalTemplate/postEvalTemplate in both entrypoints.
- Parameterize the pass-rate gate as a threshold param (default 0.8) threaded entrypoint -> archetype -> eval-shard -> eval-invoke; surface it explicitly per tier.
- Make mcpSetupTemplate required in archetype-eval.yml (example default was tools-repo specific).

* Consolidate MCP templates and add opt-in cross-repo checkout

- Merge mcp-mock.yml/mcp-live.yml into eval-mcp-setup.yml with an McpServer if/else selector threaded through build-mcp and the archetype.
- Add opt-in toolsRepo param (default '') so repos outside azure-sdk-tools can side-checkout the tools sources to build the MCP; no-op for the tools repo.

* Fix eval archetype: declare toolsRepo param and default mcpServer

The archetype forwarded toolsRepo/mcpServer without declaring toolsRepo (ADO 'Key not found' compile error) and left mcpServer without a default despite being optional. Declare toolsRepo (default '') and default mcpServer to ''.

* Address review feedback: portable lib/ gitignore, rerun-safe shard artifacts, TestType rename, exec.js note

* Always stage debug dir so failed()-gated transcript publish can't error on missing path

---------

Co-authored-by: helen229 <gaoh@microsoft.com>
When triage marks a board-review issue ready-for-review, resolve one
architect per selected language from the shared api-review-approvers.yml
roster (round-robin by issue number) and assign them. Assignment fires
GitHub's native notifications so architects are reached through their own
preferences. The resolved handles are shown in the existing "materials
verified" comment (single comment, no extra ping). Idempotent: re-edits
do not re-assign.

Also documents the assignment step and C++/Rust language labels in
ARCH-BOARD-REVIEW-PROCESS.md.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f922dad1-6173-4c65-b4bb-03f056572963
@samvaity
samvaity force-pushed the arch-board-assign-reviewers branch from 5f7346c to f5c7f8d Compare July 22, 2026 00:39
@samvaity

Copy link
Copy Markdown
Owner Author

Retargeting to Azure/azure-sdk (opened against the fork main by mistake).

@samvaity samvaity closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants