Skip to content

Add human review gates to code review skill - #19434

Open
David Pine (IEvangelist) wants to merge 1 commit into
mainfrom
ievangelist-code-review-human-gates
Open

Add human review gates to code review skill#19434
David Pine (IEvangelist) wants to merge 1 commit into
mainfrom
ievangelist-code-review-human-gates

Conversation

@IEvangelist

Copy link
Copy Markdown
Member

Description

Issue #19370 exposed a gap in our AI-assisted review workflow. PR #18858 changed a public npm registry to an internal feed, propagated that choice into customer-facing CLI and generated artifacts, changed supply-chain checks, and added tests that asserted the new internal default. Material behavior also changed after the last human approval. The review process found several local issues but did not challenge whether internal build infrastructure should become shipped customer behavior.

This PR keeps the code-review skill's existing high-confidence findings model and adds a separate, provenance-neutral semantic risk pass. A single sensitive trigger now requires human review:

  • Trust, security, or supply-chain boundary changes.
  • Internal infrastructure flowing into customer-facing behavior or generated artifacts.
  • Customer-facing defaults, fallbacks, or source-precedence changes.
  • Scope mismatches where dependency, security, generated-file, mechanical, or refactoring work also changes production behavior.
  • Material changes added after the latest human approval.

Triggered changes appear in a distinct Human review required section rather than being presented as proven defects. Each item must identify the exact evidence, affected surfaces, implications, required human validation, and whether an existing approval covers the current head. The skill will not recommend or submit APPROVE until the user explicitly confirms that the listed human review occurred.

The review guidance also now requires tracing sensitive values to shipped sinks, separating repository infrastructure from product behavior, comparing removed security controls with their replacements, preferring outcome-oriented boundary validation over implementation-shaped tests, and checking approval freshness. Issue #19370 is included as a calibration case so the guidance generalizes to registries, mirrors, endpoints, credentials, and security controls without special-casing npm.

docs/contributing.md now describes the new human-review escalation and approval gate.

Note

These Copilot skills are living guidance. If these triggers become too noisy or over-escalate routine changes, we can continue to tune the categories, wording, and threshold based on real review experience.

Validation

Related to #19370

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
      • This is a skill and contributor-documentation change; it was validated structurally and against the historical regression scenario.
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 17, 2026 13:49
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19434

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19434"

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 17, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants