Repository navigation
Resolve successfully handled /issue triage command comments - #39235
Conversation
Gate exact command minimization on trusted validation receipts and complete native safe-output success while keeping reports artifact-only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 39235Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 39235" |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Disable native custom-job failure reports alongside the existing artifact-only triage reporting policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The security-sensitive completion path lacks automated tests for its success and rejection cases.
1 open finding
What changed in this PR
Adds trusted post-processing that marks successfully handled /issue triage command comments as resolved.
Changes:
- Adds a guarded completion job and validation receipt.
- Revalidates command identity, permissions, issue state, and native operation results.
- Disables public failed-job reports and documents the behavior.
| File | Description |
|---|---|
.github/workflows/issue-triage.md |
Defines the completion job and safety gates. |
.github/workflows/issue-triage.lock.yml |
Regenerates the compiled workflow. |
.github/scripts/IssueTriage.ps1 |
Validates receipts and minimizes the command. |
.github/docs/issue-triage.md |
Documents completion and failure behavior. |
🧠 Review effort: Balanced
| } | ||
| } | ||
|
|
||
| function Complete-TriageRequest { |
There was a problem hiding this comment.
No new Pester fixtures are included in this follow-up. The compiler/hash, PowerShell parser and actionlint checks cover syntax/configuration, not the runtime success and rejection cases listed here. Live completion behavior is still unverified until deployment, so I am leaving this thread open rather than claiming the requested coverage exists.
Auto-replied by the GitHub Copilot app
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
Reviewed exact head cdb48762793ca59854923c2d975bc61ea39a89d2 using 3 independent reviewers with adversarial consensus. The independent reviewer panel checked the four changed files, receipt provenance and binding, completion eligibility, live identity/permission checks, and pinned gh-aw v0.86.2 output semantics.
Finding
.github/scripts/IssueTriage.ps1:241-250,261-265 and .github/workflows/issue-triage.md:135-147, completion relies on successful native item counters, then re-fetches the issue only to check identity and open state. In the pinned removal handler, a non-404 deletion error is collected in failedLabels but the item still returns success: true; the upstream test explicitly confirms this behavior. The status classifier does not turn failedLabels into a failed/warning item.
For example, one validated remove_labels item requests two removals: the first succeeds and the second fails. Processed/applied/succeeded counts can all be 1, matching the receipt, while failure/warning counts remain zero. All new completion gates therefore pass and the command is collapsed as resolved although a requested removal remains unapplied. This contradicts the documented guarantee that partial/incomplete outcomes remain visible; the upstream failure handling is pre-existing, but resolving such an incomplete command is introduced here.
Before minimizing, verify that every validation.requestedAdditions label is present and every validation.requestedRemovals label is absent from the refetched issue. Add a Complete-stage fixture with a valid one-item receipt, two requested removals, and one label still present; assert that no minimization mutation occurs.
Flagged by: 3/3 reviewers after dispute; final severity uses the lowest independently assigned supporting severity.
Coverage and limits
The existing Pester-coverage review thread remains unresolved; this is a concrete false-success path rather than a duplicate general coverage request. No completion tests are added in this PR. Review was static: the PowerShell AST and scoped diff checks passed, but no hosted completion run, API failure reproduction, or test suite was executed. The gh-aw CLI was unavailable, so the author's compile/hash results were not independently rerun. No approval or merge-readiness conclusion is implied.
Scope: code review only. CI/check status and merge readiness were not evaluated; use the repository's CI-analysis workflow for that assessment.
Require trusted validated additions to be present and removals absent on the freshly fetched issue, rejecting invalid label shapes and partial native removals. Preserve empty deltas for genuine no-op completion. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Fixed the partial-removal case in ada0dbe. Before minimizing the command, completion now checks that every trusted validated addition is present and every removal absent in the freshly fetched issue. Partial failures or reversals reflected in that fetched snapshot leave the command visible. This is a point-in-time check, not an atomic guarantee against label changes between the fetch and minimization; completion does not change or roll back labels. The earlier Pester run 37671994169 had 2538 passing tests and three model-pin assertion failures. The exact same three failures and totals occurred on unchanged main run 37549105032; the five relevant test/input Git blobs are identical across that main baseline, this PR base and the tested head. Those failures are pre-existing, not introduced here. The new head CI is still running, and deployed command minimization is not yet verified. |
There was a problem hiding this comment.
🟡 Changes recommended
The completion flow lacks automated coverage, and its documentation overstates the concurrency guarantee.
2 open findings
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
Reviewed exact head ada0dbe3316b3955a693041ad5a0b72061a01c00 using 3 independent reviewers with adversarial consensus. No additional actionable defects found after adjudication. The independent reviewer panel examined all four changed files and necessary dependencies, including receipt provenance/binding, live permission and command identity checks, native output/detection gates, trusted completion-job isolation, and the new exact-label checks.
The previously reported partial-removal false-success path is addressed: completion now requires every validated addition to be present and every requested removal to be absent in the freshly fetched issue label snapshot, rather than trusting pinned gh-aw v0.86.2 item-success counters alone. Review covered additions-only, removals-only, combined operations, pure no-op and withheld-only receipts, malformed/duplicate/opposing names, and operation-count mismatches. No new overblocking defect was identified in those paths.
Existing feedback and limits: The Pester-coverage request and documentation concurrency-wording thread remain unresolved and are not duplicated here. Label verification is a point-in-time check, not an atomic guarantee against changes between the issue fetch and minimization. No automated completion fixtures are added in this head. Independent verification was static; PowerShell AST parsing and scoped diff checks passed, but no test suite, API-failure reproduction, or hosted end-to-end completion run was executed. The gh-aw compiler/hash checks were not independently rerun. This clean additional-findings conclusion does not resolve the outstanding feedback or constitute approval.
Scope: code review only. CI/check status and merge readiness were not evaluated; use the repository's CI-analysis workflow for that assessment.
Document that unmet label state in the fetched snapshot prevents command minimization, but later concurrent label changes are not atomically excluded. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The security-sensitive completion flow still lacks automated coverage for its success and rejection paths.
1 open finding
1 resolved since last review
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
Reviewed exact head 79d289d7baada3a2ff68f5a3bf7b444944274dc0 using 3 independent reviewers with adversarial consensus. No additional actionable defects found after adjudication. The independent reviewer panel examined all four changed files and necessary dependencies: trusted completion-job isolation, receipt/context/run bindings, live requester and comment identity checks, native operation/detection gates, and exact label postconditions for additions, removals, combined operations, pure no-op and withheld-only results.
The prior partial-removal false-success finding remains addressed by checking the fetched label snapshot rather than relying on native item-success counters alone. Configured pinned gh-aw v0.86.2 no-op handler behavior was cross-checked; an initial concern about a zero processed count was retracted because this workflow enables the handler, which records a successful item. The documentation now accurately states that label verification is point-in-time and not atomic with the later minimization mutation; the concurrency-wording thread is resolved.
Coverage and limits: The existing Pester-coverage request remains open and is not duplicated or resolved by this review. No automated completion fixtures are added. Independent verification was static, including PowerShell AST parsing and scoped diff checks; no test suite, API-failure reproduction, or hosted end-to-end completion run was executed. Compiler/hash checks were not independently rerun. The clean additional-findings conclusion does not remove the outstanding coverage limitation or constitute approval.
Scope: code review only. CI/check status and merge readiness were not evaluated; use the repository's CI-analysis workflow for that assessment.
|
@dalexsoto stamp |


Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Focused follow-up to #39201: collapse only the exact successfully handled
/issue triageinvocation comment using GitHub'sRESOLVEDclassifier, matching the existing/issue trace-regressionexperience. Label explanations remain Actions-artifact-only; no public triage report or status comments are restored. Native custom-job failure reports are explicitly disabled so completion failures cannot create public report noise.A separate fresh hosted completion job uses trusted
github.shatooling and step-scopedGITHUB_TOKEN. It requires successful activation/agent/detection/native safe-output processing, positive matching native operation counts, and zero cancelled/deferred/failed/skipped/warning items. An explicitly successful detector verdict is required; its legitimate no-op skip is accepted only with a trustednoopvalidation receipt. The retained validator receipt binds the operation count, issue, requester, triggering comment, context hash, run ID and first attempt. Validated no-change and withheld-only native no-ops are supported.Completion also verifies actual label postconditions: every exact canonical name in the trusted validated additions must be present, and every requested removal absent, in the freshly fetched issue label objects. It rejects malformed label arrays/objects, duplicate or opposing requested names, and operation-count mismatches. Pure no-op receipts explicitly retain empty addition/removal arrays. This closes the pinned v0.86.2 removal handler's false-success gap (
success: truecan coexist withfailedLabels, which native item status does not classify as failure). This is a point-in-time postcondition check: malformed or unmet label state detected in that fetched snapshot leaves the command visible. The subsequent command checks and separate minimization mutation are not atomic with the label read, so a concurrent label change after that snapshot can still occur before the command collapses. Completion never mutates or rolls back labels.Before the fixed GraphQL mutation, trusted completion rechecks default-branch infrastructure, current requester permission, the exact open non-PR issue, comment association, human author/account ID, comment/node IDs, exact body and creation/update timestamps. Manual/staged invocations, bots, other commands/comments, edits and job reruns cannot minimize a comment. Mutation failures or unconfirmed results produce an explicit Actions warning without rolling back successful labels. No agent permission, credential, model pin, shared import or native WTD3 behavior changes.
Validation (gh-aw v0.86.2; rerun after the exact-outcome fix; no pipeline tests or synthetic fixtures added):
gh aw compile issue-triage --strict --no-check-update— passed, one workflow, zero warnings; generated lock reviewed and unchanged by the script/docs follow-up.gh aw hash-frontmatter .github/workflows/issue-triage.md— passed; actual hash including current imports matches generated metadata:c848d6ca0861ac24b0a3d0286e6b6d49a2028ebde30f0b3680db1057823aa00b.pwsh -NoProfile -Command '$metadata = ((Get-Content .github/workflows/issue-triage.lock.yml -TotalCount 1) -replace "^# gh-aw-metadata: ", "") | ConvertFrom-Json; $hash = & gh aw hash-frontmatter .github/workflows/issue-triage.md; if ($LASTEXITCODE -ne 0 -or $hash.Trim() -cne $metadata.frontmatter_hash) { throw "Frontmatter/import hash mismatch" }; "Compiled frontmatter/import hash matches: $hash"'— passed.pwsh -NoProfile -Command '$tokens = $null; $errors = $null; $null = [System.Management.Automation.Language.Parser]::ParseFile((Join-Path $PWD ".github/scripts/IssueTriage.ps1"), [ref]$tokens, [ref]$errors); if ($errors.Count) { $errors | Format-List; exit 1 }; "Whole-script PowerShell AST: passed"'— passed.actionlint -format '{{json .}}' .github/workflows/issue-triage.lock.yml | jq -e 'length > 0 and all(.[]; .kind == "syntax-check" and (.message | test("^unexpected key \"queue\" for \"concurrency\" section\\. expected one of \"cancel-in-progress\", \"group\"$")))'— passed the exact known compatibility-exception filter. Raw actionlint reports only the two existingqueue: maxincompatibilities; no other diagnostics.git diff --check— passed, including the subsequent documentation-only point-in-time clarification.test "$(git diff --name-only)" = '.github/docs/issue-triage.md'— passed before that clarification commit, confirming no implementation or workflow configuration changes. No compiler rerun or tests were needed for the wording-only correction.These are local syntax/configuration checks and pinned-source inspection, not runtime tests or deployed end-to-end minimization verification. No historical comments were hidden, real label runs dispatched, or unmerged infrastructure deployed. Future live verification requires merge and a separately bounded approved command-testing scope.
Issues Fixed
No separately filed issue. User-requested completion-UX follow-up to #39201, including the partial-removal false-success finding raised during review.