feat(safeoutputs): add GitHub issue mutation family

[jamesadevine]

Summary

• port gh-aw's GitHub issue mutation family as eleven GitHub-qualified, configured-only safe outputs
• add shared fail-closed repository, filter, temporary-ID, REST/GraphQL, and Stage 3 App-permission infrastructure
• extend approval summaries, documentation, compiler coverage, and executor E2E scenarios for the complete family

Closes#1871

Test plan

• cargo check --tests
• cargo test (3,395 passed, 1 ignored)
• cargo clippy --all-targets -- -D warnings
• changed-file Rust formatting check and git diff --check
• npm test -- --maxWorkers=1 in scripts/ado-script (1,130 passed)
• npm run typecheck
• npm run build:approval-summary
• npm run build:executor-e2e
• live GitHub mutation scenarios require the documented E2E credentials and were not run locally

[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]

/review

[github-actions]

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

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection results could not be parsed.

Review the workflow run logs for details.

[github-actions]

TypeScript review — scripts/ado-script/ (this PR's scope only)

Reviewed the diff to approval-summary/index.ts, approval-summary/render.ts, executor-e2e/github-client.ts, executor-e2e/runner.ts, and executor-e2e/scenarios/github-issue.ts.

No merge-blocking defects found. The new code is careful about the concerns that matter most here:

• parseRepositoryPolicies / repository-resolution logic in render.ts fails closed on malformed JSON, non-string fields, and array/object type mismatches — good defensive parsing of compiler-provided data.
• REST/GraphQL error paths in github-client.ts consistently throw with call context (method + path + status) rather than swallowing failures.
• New branches (mutation family scenarios, repository policy resolution, empty-value rendering) are matched by substantial new test coverage in render.test.ts and the scenario test suite.

One non-blocking observation: createRepoLabel and createMilestone in github-client.ts hand-roll their own fetch/error-text logic instead of going through the githubJson<T> helper introduced earlier in the same file, duplicating the "read body text on failure" pattern. Not a functional issue (this is test-harness code, not shipped in ado-script.zip), just a minor consistency nit for a future cleanup pass.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer · auto · 63.1 AIC · ⌖ 12 AIC · ⊞ 11.1K
_{Comment /review to run again}

[github-actions]

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection results could not be parsed.

Review the workflow run logs for details.

REQUEST_CHANGES not needed. Reviewed the Rust-side changes in this PR (new github_api.rs/github_issue_common.rs shared infra and the eleven new safe_outputs/*.rs GitHub issue-mutation tools, plus the create_github_issue.rs/agentic_pipeline.rs refactors).

Findings: no merge-blocking defects.

• Error handling is consistently anyhow-based with .context()/ensure!/bail!; the switch from ctx.get_tool_config(...).ok().unwrap_or_default() to ctx.get_tool_config(...)? in create_github_issue.rs is a real improvement (config shape errors now surface instead of silently defaulting).
• repository.split_once('/').expect(...) calls in github_api.rs/github_issue_common.rs are all preceded by a regex-enforced validate_github_repository/target_repo_regex check guaranteeing exactly one /, so the invariant holds.
• Repository dedupe/ownership helpers (dedupe_github_repositories, github_app_owner_for_repositories, github_app_repository_names) use insertion-order Vecs rather than HashMap iteration, preserving deterministic output for generated config/YAML.
• resolved_github_issues: Arc<Mutex<HashMap<...>>> accessors never hold the lock across an .await.
• One apparent defect turned out to be a false positive worth flagging for transparency: the pre-fetched diff shows .header(AUTHORIZATION, format!("******", self.token)) in github_api.rs, which would be a compile error (unused format arg). I checked the actual file on disk and the real code is format!("Bearer {}", self.token) — the diff-fetch tooling appears to redact secret-shaped literals for display. Not a code issue, just noting it in case the redaction masks other findings in future reviews.

Process note: the rust-critic sub-agent produced no usable output across three attempts (empty responses each turn), so this review reflects only my own manual pass over the diff; its findings were discarded per the sub-agent contract.

No inline comments posted — nothing rose to a postable, diff-scoped defect.

🦀 Rust code quality review by Rust Code Quality Reviewer · auto · 148.9 AIC · ⊞ 11.2K
_{Comment /review to run again}

[jamesadevine]

@copilot merge main

[Copilot]

@copilot merge main

Merged main into this branch in commit 4887d526 and validated with cargo check --tests.

[jamesadevine]

/review

[github-actions]

✅ Test Quality Sentinel completed the test quality analysis.

🧪 Test quality analysis by Test Quality Sentinel

[github-actions]

Compiler contract review

Walked the full drift surface for this PR (57 files, new GitHub issue-mutation safe-output family):

• Registry completeness: all 11 new tools (add-github-issue-labels, assign-github-issue-milestone, assign-github-issue-to-user, close-github-issue, comment-on-github-issue, hide-github-issue-comment, link-github-sub-issue, remove-github-issue-labels, set-github-issue-field, unassign-github-issue-from-user, update-github-issue) are consistently wired through safe_outputs/mod.rs, mcp.rs tool handlers, execute.rs dispatch table, inspect/catalog.rs descriptions (with a new catalog-completeness test), and the shared GITHUB_ISSUE_SAFE_OUTPUT_TOOLS / GITHUB_TEMPORARY_ID_CONSUMERS registries in types.rs.
• Docs sync: docs/safe-outputs.md, README.md, and AGENTS.md (bundle directory tree) all reflect the new files/tools.
• Newtype usage: new params (AddGithubIssueLabelsParams, etc.) consistently use GithubIssueNumber and existing validated helpers rather than raw String identifiers.
• Ado-script bundle drift: no .js bundle changes expected/flagged (gitignored generated artefacts).
• Codegen drift: no changes to filter_ir.rs/Fact enum in this diff, so types.gen.ts/fact-catalog.gen.json absence is expected.
• Lock drift: no .github/workflows/*.md changes in this PR.

No compiler-contract violations found. Nice, thorough threading of the new tool family through every registry this project depends on.

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

[github-actions]

TypeScript review (scripts/ado-script/)

Reviewed approval-summary/index.ts, approval-summary/render.ts, executor-e2e/github-client.ts, executor-e2e/runner.ts, and executor-e2e/scenarios/github-issue.ts for async/error handling, type safety on external data, and secret handling. Also ran an independent ts-critic pass on the same diff.

No high-confidence defects found:

• All new JSON/GraphQL parsing (parseRepositoryPolicies, githubJson, listRepositoryIssueFields, getIssueFieldValue, etc.) validates shape before use and fails closed with a descriptive <unresolved: ...> marker or thrown Error rather than trusting external input.
• Network calls consistently go through ghFetch/githubJson with AbortSignal.timeout and explicit non-2xx handling; no unawaited promises or forEach-with-async patterns.
• No tokens or secrets are interpolated into thrown errors or log lines — error messages only include GitHub's response body/status.
• Non-null assertions (e.g. resolved[0]!, reference.canonical!) are all preceded by explicit guards in the same function, so they're not blind casts on unchecked external data.

Nothing else to flag — clean, well-guarded diff.

🟦 TypeScript code quality review by TypeScript Code Quality Reviewer · auto · 69.7 AIC · ⌖ 2.6 AIC · ⊞ 11.3K
_{Comment /review to run again}

[github-actions]

This PR (github issue mutation family) was reviewed for security regressions across the shared GitHub API client (github_api.rs), repository/target policy (github_issue_common.rs), and the mutation tools (comment, update, close, assign/unassign, hide-comment, link-sub-issue, labels, milestone, issue-field). Findings:\n\n- Repository selection consistently routes through resolve_github_repository/resolve_github_issue_target, which enforces the target-repo/allowed-repos allowlist for every path, including close-github-issue.duplicate_of explicit-repository and URL forms — no way to reach an unconfigured repo.\n- All agent-controlled config strings (labels, assignees, prefixes, repository refs, reasons) pass through reject_pipeline_injection before use, and result fields are sanitized via SanitizeContent/sanitize_config before serialization.\n- Capability checks (validate_github_target_capability) and required-labels/required-title-prefix filters are resolved against live GitHub state before the first write in every tool, preventing a stale/forged local view from bypassing policy.\n- The GitHub write token stays confined to Stage 3 executor code (ExecutionContext::github_token), matching the documented three-stage boundary; the Authorization header is redacted the same way in both source and tests, so no plaintext token leak was found.\n\nNo exploitable injection, token exposure, or validation weakening was found in the reviewed diff; it is security-neutral to hardening. Given the size (57 files, ~16.5k additions dominated by new, additive Rust/TS code), I focused on the highest-risk sinks (HTTP client construction, repository/target resolution, and each mutation tool's write path) rather than test-only executor-e2e harness code.

🔒 Security review by PR Security Reviewer · auto · 115.5 AIC · ⌖ 2.8 AIC · ⊞ 11.6K
_{Comment /review to run again}

[github-actions]

Test Quality Review

Reviewed the new/changed test files for this PR (Rust unit tests in src/safe_outputs/*.rs, TypeScript executor-e2e and approval-summary tests, and tests/compiler_tests.rs additions).

No test-quality issues found. This PR is unusually thorough on the testing front:

• Every new safe-output tool (assign-github-issue-milestone, assign-github-issue-to-user, close-github-issue, comment-on-github-issue, hide-github-issue-comment, link-github-sub-issue, set-github-issue-field, update-github-issue, add/remove-github-issue-labels, unassign-github-issue-from-user) has dedicated unit tests covering: param validation (including injection payloads like ##vso[task.complete] and $(TOKEN)), strict-config deserialization/rejection of unknown keys, policy filter ordering (all preflight reads happen before any write), pagination, dry-run behaviour, and sanitization of hostile GitHub API error bodies.
• The executor-e2e harness (runner.test.ts) adds a real regression test for repeated priorEntries occurrences (link-github-sub-issue parent+child), and github-client.test.ts covers GraphQL introspection, product-error surfacing, and pagination for the new REST/GraphQL helpers.
• The approval-summary render.test.ts additions specifically test hostile-repository sanitization, temporary-ID-to-repository resolution ordering (including the negative case where a create-github-issue occurs after the consumer and must NOT resolve), and empty-value markers.
• No existing assertions were weakened, loosened, or #[ignore]d — the diffs against previously-modified test files (e.g. set_github_issue_type.rs, tests/compiler_tests.rs) are purely rustfmt reflow plus additive fields/tests.
• tests/compiler_tests.rs additions correctly assert both automatic and gated (SafeOutputs_Reviewed) job placement for the new GitHub App token scoping tests.

No inline comments needed.

🧪 Test quality analysis by Test Quality Sentinel · auto · 114.8 AIC · ⌖ 2.52 AIC · ⊞ 11.2K
_{Comment /review to run again}

[github-actions]

Rust code-quality review — no merge-blocking issues found

Reviewed the Rust-only portion of this diff (src/compile/{agentic_pipeline,common,types}.rs, src/execute.rs, src/mcp.rs, src/inspect/catalog.rs, src/safe_outputs/**, tests/compiler_tests.rs) covering the new github_api.rs / github_issue_common.rs shared infrastructure and the eleven new GitHub-issue safe-output modules.

What I checked:

• Error handling: consistent anyhow::Result + .context()/ensure!/bail! usage; every expect() I found is guarded by a preceding validation (e.g. split_once('/') after validate_github_repository, Number::from_f64 after an explicit is_finite() check) rather than reachable from unchecked user input.
• The GithubClient::send builds the Authorization header via format!("Bearer {}", self.token) — I initially misread this as a literal "******" placeholder from a masked terminal render, but git show on the actual blob confirms the real bearer-token format string is correct. No credential-leak/broken-auth bug here.
• Pagination (get_paginated) enforces a MAX_PAGES bound and re-validates same-origin on every Link: rel="next" hop before following it — good defense against SSRF via a malicious redirect chain.
• validate_github_issue_outputs_config's refactor from two hardcoded tools to a GITHUB_ISSUE_SAFE_OUTPUT_TOOLS loop preserves the original target-repo/mutation-filter/approval-compatibility checks (verified create-github-issue and set-github-issue-type are still members of that table and routed through the same validation calls).
• No lossy casts, no blocking calls in async paths, no unsynchronized shared mutable state observed in the changed lines.

Most of the diff outside the new files is mechanical cargo fmt reflow. Nothing here meets the bar for an inline comment. Compiler-contract-specific concerns (front-matter schema, generated YAML, docs sync) are left to the Compiler Contract Reviewer.

rust-critic sub-agent returned no findings on this diff.

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