feat(safe-outputs): allow model-selected PR reviewers

.github/workflows/ado-script.yml

@@ -8,6 +8,12 @@ on:
       - "src/compile/extensions/ado_script.rs"
       - "src/ado_proxy/**"
       - "src/main.rs"
+      - "src/execute.rs"
+      - "src/safe_outputs/**"
+      - "src/secure.rs"
+      - "src/validate.rs"
+      - "src/compile/common.rs"
+      - "src/compile/types.rs"
       - "Cargo.toml"
       - "Cargo.lock"
       - ".github/workflows/ado-script.yml"
@@ -24,6 +30,12 @@ on:
       - "src/compile/extensions/ado_script.rs"
       - "src/ado_proxy/**"
       - "src/main.rs"
+      - "src/execute.rs"
+      - "src/safe_outputs/**"
+      - "src/secure.rs"
+      - "src/validate.rs"
+      - "src/compile/common.rs"
+      - "src/compile/types.rs"
       - "Cargo.toml"
       - "Cargo.lock"
       - ".github/workflows/ado-script.yml"

docs/safe-outputs.md

@@ -30,6 +30,13 @@ safe-outputs:
       - agent-created
     work-items:
       - 12345
+  update-pr:
+    allowed-operations:
+      - add-reviewers
+    allowed-reviewers:
+      - "user@example.com"
+    max-reviewers: 3
+    max: 2
 ```
 
 Safe output configurations are passed to Stage 3 execution and used when processing safe outputs.
@@ -609,7 +616,7 @@ Multi-call operations preflight repository policy, filters, capability
 switches, and requested values before writing, so a denied later value cannot
 leave a partial mutation.
 
-### Temporary IDs and approval
+### Temporary IDs, approval, and staged mode
 
 Use a gh-aw-compatible temporary ID to refer to a newly created issue before
 its real number exists:
@@ -642,6 +649,13 @@ safe-outputs:
     target-repo: octo-org/octo-repo
 ```
 
+They must also have exactly the same effective `staged` setting. A staged
+create previews issue creation and cannot supply a live issue to a consumer;
+conversely, a live create followed by a staged consumer splits creation from
+its preview-only follow-up. The compiler resolves the section-level
+`safe-outputs.staged` default and any per-tool override before comparing every
+configured consumer.
+
 The `ado-aw-safe-outputs` build-summary tab shows each proposal using the exact
 tool name, repository, target IDs, requested state, labels/users/field values,
 and a sanitized body excerpt. Reviewed GitHub proposals are grouped under
@@ -886,7 +900,12 @@ job — that create is scoped by its own configuration — so they are not check
 against `target`. A temporary ID that cannot be traced to such a create is
 rejected before any request is sent. When both tools are configured they must
 have the same effective `require-approval` setting, so temporary-ID state stays
-within a single SafeOutputs job.
+within a single SafeOutputs job. `create-work-item` and every configured
+temporary-ID consumer must also have exactly the same effective `staged`
+setting: staged creation cannot produce the live work item required by a
+consumer, while staging only the consumer splits a live creation from its
+preview-only follow-up. Section-level defaults and per-tool overrides are
+resolved before comparison.
 
 ```json
 {"title":"Investigate build failure","description":"Detailed failure report long enough for validation."}
@@ -960,9 +979,9 @@ call returns the temporary ID used by the second:
 - `max` - Maximum assignments per run (default: 1).
 
 When both create and assign are configured, they must have the same effective
-`require-approval` setting so temporary-ID state remains in one SafeOutputs
-job. Unresolved, duplicate, reversed, or failed-create references fail before
-assignment.
+`require-approval` and `staged` settings so temporary-ID state remains in one
+SafeOutputs job and preview behavior stays consistent. Unresolved, duplicate,
+reversed, or failed-create references fail before assignment.
 
 `Agency` and `GitHub Copilot` are reserved non-assignable identities. They are
 rejected case-insensitively in static `create-work-item.assignee`,
@@ -1102,6 +1121,11 @@ This hybrid approach combines:
 
 Note: The source branch name is auto-generated from a sanitized version of the PR title plus a unique suffix (e.g., `agent/fix-bug-in-parser-a1b2c3`). This format is human-readable while preventing injection attacks.
 
+The tool response includes a generated temporary PR ID such as `#aw_a1b2c3`.
+The agent can pass that value as `pull_request_id` to later `update-pr` calls in
+the same SafeOutputs job. The ID is generated by the MCP server and is not an
+input to `create-pull-request`.
+
 **Configuration options (front matter):**
 - `target-branch` - Target (base) branch the PR merges into (default: "main"). A
   plain literal branch name, applied to every repo unless overridden below.
@@ -1146,7 +1170,7 @@ Note: The source branch name is auto-generated from a sanitized version of the P
 - `protected-files` - Controls whether manifest/CI files (e.g., `package-lock.json`, `.github/`, `*.lock`) can be modified: `"blocked"` (default, reject changes to these files) or `"allowed"` (permit all files)
 - `excluded-files` - Glob patterns for files to strip from the patch before applying (e.g., `["*.lock", "dist/**"]`)
 - `allowed-labels` - Allowlist of labels the agent is permitted to apply. If empty (default), any labels are accepted.
-- `reviewers` - List of reviewer emails to add
+- `reviewers` - List of reviewer emails or Azure DevOps user IDs to add
 - `labels` - List of labels to apply
 - `work-items` - List of work item IDs to link
 - `fallback-record-branch` - When PR creation fails, record the pushed branch name and target branch in the failure response so operators can manually create the PR (default: true)
@@ -1277,7 +1301,7 @@ safe-outputs:
 Updates pull request metadata (reviewers, labels, auto-complete, vote, description).
 
 **Agent parameters:**
-- `pull_request_id` - The PR ID to update (required)
+- `pull_request_id` - A positive numeric PR ID, a quoted positive numeric ID, or a temporary ID (`#aw_...`) returned by an earlier `create-pull-request` call in the same SafeOutputs job (required)
 - `operation` - Update operation: `add-reviewers`, `add-labels`, `set-auto-complete`, `vote`, or `update-description` (required)
 - `reviewers` - Reviewer emails (required for `add-reviewers`)
 - `labels` - Label names (required for `add-labels`)
@@ -1291,12 +1315,48 @@ safe-outputs:
   update-pr:
     allowed-operations: []          # Optional — restrict which operations are permitted (empty = all)
     allowed-repositories: []        # Optional — restrict which repos can be updated
+    allowed-reviewers: []           # Optional — non-empty list restricts reviewers; empty or ["*"] permits any valid reviewer
+    max-reviewers: 3                # Maximum reviewers in one add-reviewers call (default: 3)
     allowed-votes: []               # REQUIRED for vote operation — empty rejects all votes
     delete-source-branch: true      # For set-auto-complete (default: true)
     merge-strategy: "squash"        # For set-auto-complete: squash, noFastForward, rebase, rebaseMerge
     max: 1                          # Maximum per run (default: 1)
 ```
 
+When `allowed-reviewers` is omitted or empty, any otherwise-valid reviewer is
+permitted, matching gh-aw's reviewer policy. A non-empty list restricts
+reviewers using case-insensitive exact matching; `["*"]` is an explicit
+unrestricted form. Non-GUID reviewer values must also exactly match an Azure
+DevOps identity email, account name, or display name; fuzzy Identity Picker
+results are not selected. Reviewer identity or API failures return a warning
+with structured `added` and `failed` arrays. Invalid configuration, disallowed
+reviewers, and unresolved PR references fail before reviewer writes begin.
+
+Temporary PR references are resolved in safe-output proposal order, so
+`create-pull-request` must appear before its `update-pr` entries. They are
+in-memory references scoped to one SafeOutputs job: automatic and manually
+reviewed safe outputs execute in separate jobs and cannot share a temporary ID.
+When both tools are configured, the compiler therefore requires them to have
+the same effective `require-approval` setting.
+
+The two tools must also have the same effective `staged` setting. A staged
+`create-pull-request` previews creation instead of producing the live PR that
+`update-pr` would modify, while staging only `update-pr` would preview updates
+after live creation. The compiler rejects both split-process configurations.
+Section-level `safe-outputs.staged` defaults and per-tool `staged` overrides are
+resolved before this comparison.
+Each follow-up call counts against `update-pr.max`.
+
+Example agent call sequence:
+
+```json
+{"title":"Update dependencies","description":"Refresh dependencies and related tests."}
+{"pull_request_id":"#aw_a1b2c3","operation":"add-reviewers","reviewers":["user@example.com"]}
+```
+
+The first line represents the `create-pull-request` call; use the actual
+temporary ID returned by that call in the later `update-pr` call.
+
 ### link-work-items
 Links two Azure DevOps work items together.
 

scripts/ado-script/src/executor-e2e/__tests__/ado-rest.test.ts

@@ -19,6 +19,125 @@ describe("AdoRest.workItemTypeExists", () => {
     vi.unstubAllGlobals();
   });
 
+  describe("AdoRest.resolveIdentityId", () => {
+    afterEach(() => {
+      vi.unstubAllGlobals();
+    });
+
+    it("verifies GUID identities through the identityIds query", async () => {
+      const fetchMock = stubFetch(
+        () =>
+          new Response(
+            JSON.stringify({
+              value: [
+                { id: "01234567-89AB-CDEF-0123-456789ABCDEF" },
+              ],
+            }),
+            {
+              status: 200,
+              headers: { "content-type": "application/json" },
+            },
+          ),
+      );
+
+      await expect(
+        new AdoRest(options).resolveIdentityId(
+          "01234567-89ab-cdef-0123-456789abcdef",
+        ),
+      ).resolves.toBe("01234567-89AB-CDEF-0123-456789ABCDEF");
+      expect(fetchMock.mock.calls[0]?.[0]).toBe(
+        "https://vssps.dev.azure.com/org/_apis/identities?identityIds=01234567-89ab-cdef-0123-456789abcdef&api-version=7.1",
+      );
+    });
+
+    it.each([
+      {
+        name: "missing",
+        value: [],
+      },
+      {
+        name: "duplicate",
+        value: [
+          { id: "01234567-89ab-cdef-0123-456789abcdef" },
+          { id: "01234567-89ab-cdef-0123-456789abcdef" },
+        ],
+      },
+      {
+        name: "mismatched",
+        value: [{ id: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" }],
+      },
+    ])("rejects a $name GUID identity response", async ({ value }) => {
+      stubFetch(
+        () =>
+          new Response(JSON.stringify({ value }), {
+            status: 200,
+            headers: { "content-type": "application/json" },
+          }),
+      );
+
+      await expect(
+        new AdoRest(options).resolveIdentityId(
+          "01234567-89ab-cdef-0123-456789abcdef",
+        ),
+      ).resolves.toBeUndefined();
+    });
+
+    it("encodes the identity query and accepts one case-insensitive exact match", async () => {
+      const fetchMock = stubFetch(
+        () =>
+          new Response(
+            JSON.stringify({
+              value: [
+                {
+                  id: "reviewer-id",
+                  displayName: "Near Match",
+                  properties: {
+                    Mail: { $value: "REQUESTER+E2E@example.com" },
+                  },
+                },
+              ],
+            }),
+            {
+              status: 200,
+              headers: { "content-type": "application/json" },
+            },
+          ),
+      );
+
+      await expect(
+        new AdoRest(options).resolveIdentityId("requester+e2e@example.com"),
+      ).resolves.toBe("reviewer-id");
+      expect(fetchMock.mock.calls[0]?.[0]).toBe(
+        "https://vssps.dev.azure.com/org/_apis/identities?searchFilter=General&filterValue=requester%2Be2e%40example.com&api-version=7.1",
+      );
+    });
+
+    it("rejects ambiguous exact matches", async () => {
+      stubFetch(
+        () =>
+          new Response(
+            JSON.stringify({
+              value: [
+                { id: "one", providerDisplayName: "owner@example.com" },
+                {
+                  id: "two",
+                  properties: { Account: { $value: "OWNER@example.com" } },
+                },
+              ],
+            }),
+            {
+              status: 200,
+              headers: { "content-type": "application/json" },
+            },
+          ),
+      );
+
+      await expect(
+        new AdoRest(options).resolveIdentityId("owner@example.com"),
+      ).resolves.toBeUndefined();
+    });
+  });
+
   describe("AdoRest authentication", () => {
     afterEach(() => {
       vi.unstubAllGlobals();