feat(safe-outputs): add explicit work item assignment

[jamesadevine]

Summary

#1881 has landed on main. This branch now merges that GitHub issue mutation family with the Azure DevOps work-item assignment stack.

• adds opt-in assign-work-item with scoped numeric targets, same-run temporary-ID resolution, allowed/blocked identity policy, and reserved-identity protection
• makes create-work-item allocate, durably persist, and return MCP-owned #aw_... IDs before responding; GitHub creation remains agent-supplied and honors require-temporary-id
• preserves all GitHub mutation handlers, repository-aware approval summaries, executor dispatch/budgets, schemas, compiler validation, docs, and E2E contracts
• shares only temporary-reference parsing, approval-lane parity, and resolved-reference registry primitives without changing domain behavior

The combined path was also validated by successful live Azure DevOps run 632439.

Closes #1899.

Tests

• cargo check --tests
• cargo test — 3,415 passed, 2 ignored
• cargo clippy --all-targets -- -D warnings
• git diff --check
• npm test -- --maxWorkers=1 — 90 files, 1,140 tests passed
• npm run typecheck
• npm run build:approval-summary
• npm run build:executor-e2e
• targeted Rust router/schema/work-item/create-work-item/result/config-injection tests — 61 passed
• targeted approval-summary and executor-E2E tests — 6 files, 134 tests passed

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

[jamesadevine]

@copilot merge main

[Copilot]

@copilot merge main

Merged main into this branch in commit a42f69c9.

[jamesadevine]

/review

[github-actions]

✅ Rust Code Quality Reviewer completed the Rust code quality review.

🦀 Rust code quality review by Rust Code Quality Reviewer

[github-actions]

Compiler-contract review

Checked drift (bundle/codegen/lock/smoke), front-matter grammar, safe-output tool wiring, typed IR, and docs sync for the new assign-work-item tool.

Findings:

Documentation sync — AGENTS.md architecture tree is stale (1 finding).
The PR adds src/safe_outputs/assign_work_item.rs as a new safe-output tool module (registered in mod.rs, execute.rs dispatch, inspect/catalog.rs, and both docs/safe-outputs.md/docs/front-matter.md), but AGENTS.md's src/safe_outputs/ file listing (around line 203) was not updated to include it. That tree is the authoritative map every future agent/contributor reads to orient itself in this module, and it currently omits the new tool file even though every sibling module (create_work_item.rs, comment_on_work_item.rs, etc.) is listed alphabetically. Please add assign_work_item.rs in its alphabetical slot (between add_pr_comment.rs and comment_on_work_item.rs).

Everything else checked out cleanly:

• No .lock.yml/codegen/bundle drift — pr-meta.json's files list is consistent with a source-only change (no gate/fact IR touched, no .github/workflows/*.md changed).
• New assign-work-item tool is fully wired: CONFIGURED_ONLY_TOOLS/ALL_KNOWN_SAFE_OUTPUTS registries, dispatch macro, budget registration, catalog description, and MCP tool schema/tests all line up.
• AssignWorkItemParams.work_item_id/assignee are semantically validated (Validate impl, reserved-identity denylist, reject_pipeline_injection) rather than needing a src/secure.rs newtype — these are ADO identity/ID values, not file paths, git refs, or artifact names, so a raw String/enum is appropriate here.
• require-approval consistency check between create-work-item and assign-work-item is a good defensive addition given the shared temporary-ID state lives in one SafeOutputs job.
• Front-matter and safe-outputs docs were both updated for the new tool and the assignee-fallback removal.

No merge-blocking issues found.

🏗️ Compiler contract review by Compiler Contract Reviewer · auto · 47.1 AIC · ⌖ 2.05 AIC · ⊞ 12.1K
_{Comment /review to run again}

[github-actions]

TypeScript review — scripts/ado-script/

Reviewed the assign-work-item executor-e2e scenario, the WIQL cleanup lookup, and the approval-summary field addition. No blocking issues found:

• resolveWorkItemAssignee correctly guards against unexpanded ADO macros ($(...)) and reserved identities before use.
• findWorkItemByTitle's WIQL query properly escapes single quotes in the title before interpolation.
• assignedIdentityValues safely narrows the untyped System.AssignedTo REST field before comparison, and numResult/getWorkItem results are validated before use.
• New tests (work-item-scenarios.test.ts) cover the assignee-resolution fallback chain, the temporary-ID handoff, the assertion path, and cleanup-by-title recovery.

Nothing here rises to a merge-blocking defect from a TypeScript-quality perspective (async/error-handling, type-safety, secrets). Comment-only.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer · auto · 32.9 AIC · ⌖ 2.4 AIC · ⊞ 11.2K
_{Comment /review to run again}

[github-actions]

Reviewed the diff for security regressions (scope: this PR only, not a full audit).

No security regressions found. Notable hardening in this change:

• normalize_work_item_assignee rejects the reserved Agency/GitHub Copilot identities before the allowed/blocked lists are consulted, so an author cannot allow-list around the hard-deny.
• Numeric (pre-existing) work-item targets now require an explicit target: "*" or exact ID — closing the previous unscoped-assignment gap.
• Temporary IDs are generated server-side by the MCP and rejected as unknown field if the agent supplies one in create-work-item params (#[serde(deny_unknown_fields)]), preventing spoofed temporary-ID collisions.
• Assignee/blocked-pattern values are run through reject_pipeline_injection and sanitize_config.
• The write-capable PATCH to Azure DevOps still runs only in Stage 3 (execute_impl), gated by ctx.tool_configs.contains_key("assign-work-item").

Nothing else in the diff weakens validation, widens the network allowlist, or moves a write path into Stage 1. Security-neutral to positive; no blocking issues.

🔒 Security review by PR Security Reviewer · auto · 62.4 AIC · ⌖ 3.06 AIC · ⊞ 11.5K
_{Comment /review to run again}

[github-actions]

Solid test coverage overall for the new assign-work-item feature and the create-work-item temporary-ID handoff — the identity-policy, temporary-ID parsing, MCP concurrency, and TS scenario tests are all behavior-focused rather than implementation mirrors. Three gaps left as inline comments: the execute_impl "not configured" / numeric-target-rejection paths in assign_work_item.rs are untested end-to-end, the Stage-3 duplicate-temporary_id guard in create_work_item.rs has no round-trip test, and the new assign-work-item.target validator's rejection branches in common.rs aren't exercised. None of these are merge-blocking — advisory only.

🧪 Test quality analysis by Test Quality Sentinel · auto · 92.7 AIC · ⌖ 1.87 AIC · ⊞ 11.1K
_{Comment /review to run again}

[github-actions]

Rust code quality review — no merge-blocking issues found

Reviewed the Rust-side changes in this PR: the new assign-work-item safe output (src/safe_outputs/assign_work_item.rs), the create-work-item temporary-ID generation/collision-avoidance path (src/mcp.rs, src/safe_outputs/create_work_item.rs), the shared resolved_work_items map (src/safe_outputs/result.rs), and the secure.rs temporary-ID validation refactor.

💡 Notes

• create_work_item.rs now fails loudly instead of silently defaulting a missing/invalid id to 0 — a real hardening, not a regression.
• The create_work_item_proposal_lock correctly scopes the collision-avoidance retry loop to create-work-item only; assign-work-item does not need it since it does not mint new IDs.
• AssignWorkItemResult is correctly wired into register_budgets! and CONFIGURED_ONLY_TOOLS/ALL_KNOWN_SAFE_OUTPUTS.
• The assign_work_item MCP handler discards the write_safe_output_file error via let _ = ..., but this matches ~8 other existing call sites in mcp.rs, so it is a pre-existing pattern rather than a new defect introduced by this PR.
• A stray blank line was added inside CreateWorkItemParams (after the tags field) — purely cosmetic, not worth a blocking comment.

The rust-critic sub-agent invocation for this run did not receive usable diff content (an internal tooling issue on my end, not a PR defect) and its output was discarded; this review is based on a manual pass over the full Rust diff.

Scope: this review covers Rust engineering quality only (src/**, ado-aw-derive/**, tests/**, Cargo.toml). Front-matter/safe-output schema and generated-artifact concerns are covered by the compiler-contract reviewer.

🦀 Rust code quality review by Rust Code Quality Reviewer · auto · 134.6 AIC · ⌖ 12.9 AIC · ⊞ 11.3K
_{Comment /review to run again}