Improve CI-fix Azure queue failure diagnostics

[PureWeen]

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

Preserve bounded, redacted Azure DevOps response details when the trusted CI-fix dispatcher receives a nontransient queue failure such as HTTP 400.

The first genuine bot PR canary proved native maui-pr integration but the dispatcher lost Azure's response body for all queue POST failures, leaving only 400 (Bad Request). This change extracts Azure's message and typeKey when available, bounds fallback response text, redacts common credential forms, and keeps the existing fail-closed behavior: one POST, no retry or reconciliation for nontransient failures.

Diagnostics-only readiness: this change is ready for human deployment, but it does not claim to restore Azure queue acceptance. The provider-side HTTP 400 root cause remains unproven and queueing is not restored. After human merge, the next legitimate automatic trusted dispatcher execution must supply the actual redacted Azure response before any payload, provider, or configuration change is justified. No manual dispatcher rerun or direct Azure queue POST is part of this validation.

Validation at exact head ba0fd0e822e2dce0fce48ee3eb4e9a1f74e2d5f1:

• Authored run using the confirmed Pester 5.9.0 manifest: 82 passed, 0 failed.
• Independent parent run using the existing Pester 6.2.0 artifact: 82 passed, 0 failed, 0 skipped.
• Independent focused patch review: no blocker found; exact two-file scope, clean head, open non-draft PR targeting main verified before and after the run.
• git diff --check: passed.

Issues Fixed

Refs #39223

[github-actions]

🚀 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/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 39243

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 39243"

[azure-pipelines]

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.

[Copilot]

🟡 Changes recommended

Valid JSON fallback responses can expose token fields written as JSON key-value pairs.

1 open finding

• Redact access_token in JSON fallback error bodies · New

What changed in this PR

Improves Azure DevOps queue-failure diagnostics while preserving existing fail-closed behavior.

Changes:

• Extracts, bounds, and redacts Azure error details.
• Adds tests for structured and plain-text failures.
• One JSON credential-redaction gap remains.

File	Description
.github/​scripts/​Queue-CiFixAzdoValidation.ps1	Formats queue failure diagnostics.
.github/​scripts/​Queue-CiFixAzdoValidation.Tests.ps1	Tests diagnostics and retry safety.

🧠 Review effort: Balanced

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[kubaflo]

/azp run

[azure-pipelines]

Azure Pipelines:
3 pipeline(s) were filtered out due to trigger conditions.

[github-actions]

Note

🔍 /review started

AzDO build 15608582 is running for android.

The s/agent-review-in-progress label stays on this PR while the run is active. The final recommendation and outcome labels are posted only after Gate, expert review, and Deep UI tests finish.

[MauiBot]

AI Review Summary

@PureWeen — new AI review results are available based on commit ba0fd0e.

---

🗂️ Review Sessions — click to expand

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

📋 Pre-Flight — Context & Validation

Pre-Flight Context

PR snapshot

• #39243, “Improve CI-fix Azure queue failure diagnostics,” is open, non-draft, targets main, and contains one commit at ba0fd0e822e2dce0fce48ee3eb4e9a1f74e2d5f1.
• Submitted scope: 2 files, +103/−1.
• GitHub reported mergeable_state: blocked and s/agent-review-in-progress when read; the reason and eventual outcome are unknown.
• This is context gathering only, not an exhaustive review or recommendation.

Reported problem

The trusted CI-fix dispatcher posts Azure DevOps build requests but, for nontransient failures such as HTTP 400, currently rethrows only the generic exception (400 (Bad Request)). Azure’s response body—potentially containing the actionable validation reason—is lost.

The PR describes this as a diagnostics-only change. It does not claim to fix Azure queue acceptance or identify the provider-side HTTP 400 root cause. The intended next evidence is a legitimate automatic dispatcher execution after the change becomes part of the trusted workflow revision.

The referenced #39223 reports a separate net11.0 CI build failure: Windows and macOS Debug builds encounter MSB4184 because VersionLessThan receives an empty version. #39243 does not modify that build failure; the issue appears to have supplied the automated CI-fix canary that exposed the queue-diagnostics problem. #39223 has no comments, and its property provenance remains unknown.

Changed files and mechanism

• .github/scripts/Queue-CiFixAzdoValidation.ps1

  • Adds Get-AzdoQueueFailureMessage.
  • Reads ErrorRecord.ErrorDetails.Message.
  • Attempts to parse JSON and, when present, emits the top-level Azure message plus optional typeKey.
  • Otherwise preserves the response text after flattening line breaks/tabs, redacting selected Bearer/Basic, token-assignment, and JWT forms, and truncating details to a default maximum of 1,024 characters.
  • Changes only the nontransient queue-POST catch path from a bare rethrow to the formatted diagnostic.
  • Existing transient handling remains structurally unchanged: timed-out/5xx POSTs are issued once and followed by exact-build reconciliation rather than retrying the POST.
  • The resulting message flows into Write-Error, per-pipeline failure results, JSON output, and the GitHub job summary.

• .github/scripts/Queue-CiFixAzdoValidation.Tests.ps1

  • Imports the new helper into the isolated Pester module.
  • Adds a simulated HTTP 400 test asserting Azure message/typeKey propagation, exactly one POST, and no duplicate lookup.
  • Adds a non-JSON test for Bearer/token redaction, truncation, and a bounded final message.

The invoking workflow, .github/workflows/ci-fix-azdo-validation.yml, runs on ubuntu-latest under PowerShell, checks out only the script from the trusted workflow revision, uses GitHub OIDC to obtain Azure DevOps credentials, and queues definitions for maui-pr, maui-pr-uitests, and maui-pr-devicetests. Consequently, PR-head execution alone does not establish that the new trusted dispatcher behavior works after deployment.

Platform-sensitive paths

• No Android application, handler, native platform, or product source is changed.
• The requested later validation platform is Android, but the modified behavior is platform-neutral GitHub/Azure infrastructure.
• The linked canary issue affects Windows and macOS Debug builds.
• The dispatcher can queue UI/device pipelines whose downstream behavior may be platform-specific, but this patch changes only queue-failure reporting.
• A PR comment reported Android review build 15608582 as running; its result was unavailable when context was gathered.

Validation requirements

• Gate: ⚠️ skipped because no tests were detected. This was not rerun. Although the submitted diff adds Pester tests, Gate did not recognize or execute them; whether these tests are wired into required CI is unknown.
• The PR description reports 82 passing tests under Pester 5.9.0 and independently under Pester 6.2.0, plus git diff --check; those are author-provided claims and were not independently executed here.
• Later validation should establish, on the actual Ubuntu/PowerShell workflow runtime, that Invoke-RestMethod populates ErrorDetails.Message in the expected form for the real Azure HTTP 400.
• Coverage should confirm that all emitted surfaces remain bounded and credential-safe while preserving the one-POST/no-reconciliation behavior for nontransient failures.
• Because the workflow executes a trusted default-branch copy, real integration evidence requires a legitimate post-deployment dispatcher event rather than a manual direct Azure queue POST.
• Android validation should not be treated as proof that the provider-side queue rejection is fixed; this PR only intends to expose its redacted response.

Remaining uncertainties

• The actual Azure response body and therefore the HTTP 400 root cause remain unknown.
• Queue acceptance is not restored by the submitted mechanism.
• An unresolved automated review thread reports a possible redaction gap for valid JSON fallback bodies without a top-level message, such as JSON-form token key/value pairs; the submitted tests cover plain-text assignment syntax but not that reported JSON form.
• Behavior for other real-world Azure response shapes, encodings, nested error objects, and credential formats is not established by the submitted tests.
• Current CI completion, Android build outcome, merge-block reason, and whether the Pester suite is enforced by required checks were unavailable.
• No dedicated expert-review conclusion is included here.

---

🔬 Code Review — Deep Analysis

Code Review — PR #39243

Independent Assessment

What this changes:
Adds Get-AzdoQueueFailureMessage in .github/scripts/Queue-CiFixAzdoValidation.ps1 and routes nontransient queue-POST failures through it (Invoke-AzdoPipelineQueue, line 827 in submitted head). The helper parses Azure error JSON (message/typeKey) when present, otherwise falls back to raw response text with newline flattening, regex redaction, and truncation.

Inferred motivation:
Make Azure DevOps 400-class queue failures actionable (include provider diagnostics) without changing one-POST/non-retry semantics for nontransient failures.

Dimension / Platform Trace

• Changed paths: .github/scripts/Queue-CiFixAzdoValidation.ps1, .github/scripts/Queue-CiFixAzdoValidation.Tests.ps1
• Activated dimensions (routing + always-active):
  • Input and Path Correctness (#31)
  • Build & MSBuild (#29, script/infrastructure path family)
  • Logic and Correctness (#5, always active)
  • Regression Prevention and Test Coverage (#6, always active)
  • Complexity Reduction (#16, always active)
• Platform impact: infrastructure/workflow logic; not product Android runtime code. Queues Android/Windows/macOS validation pipelines but this patch itself is platform-neutral PowerShell automation.

Reconciliation with PR Narrative

Author claims: Diagnostics-only improvement; preserves queue semantics; tests pass.
Agreement/disagreement: Agreed on semantics preservation. Disagreement on safety completeness: fallback redaction still misses common valid JSON key/value token forms when no top-level message exists.

Prior Review Reconciliation

Prior ❌ Error Finding	Source	Status	Evidence
JSON fallback can expose token fields ({"access_token":"..."})	Copilot inline comment #discussion_r4213831572	❌ Unresolved	Current code redacts access_token=... only (line 127), not JSON \"access_token\":\"...\"; no added test for JSON fallback token redaction in this PR.

Blast Radius Assessment

• Runs for all instances: Yes, for every nontransient queue POST failure path in CI-fix dispatcher.
• Startup impact: No app startup impact; workflow/automation runtime only.
• Static/shared state: No new shared mutable state; helper is pure message formatting.

CI Status

• Required-check result: undetermined (tool-unavailable via unauthenticated gh; PR issue thread shows /azp run just started)
• Classification: undetermined
• Action taken: capped confidence at low; no LGTM
• Gate evidence: trusted pre-flight states Gate skipped (no tests detected); not rerun per instruction.

Findings

❌ Error — JSON fallback redaction gap can leak token fields

In Get-AzdoQueueFailureMessage (.github/scripts/Queue-CiFixAzdoValidation.ps1:127), redaction covers assignment-style tokens (access_token=...) but not JSON key/value token syntax.
Concrete scenario: a valid JSON Azure error body without top-level message (e.g., {"access_token":"sensitive123","errors":[...]}) leaves $detail as raw JSON; current regexes do not redact the JSON property value, and the thrown error includes that token text (return "$exceptionMessage Azure response: $detail", line 139).
Coverage gap: added tests validate only (a) message/typeKey extraction and (b) non-JSON assignment redaction (Tests.ps1:1131-1182), not the JSON fallback token case.

Failure-Mode Probing

• If valid JSON has no message: $detail remains full response body; regex pass is the only protection. Since JSON token pair format is unhandled, sensitive value survives into error output.
• If nontransient 400 repeats across pipelines: failure formatter runs per pipeline result; same leakage pattern propagates into error strings/results summaries each time that response shape occurs.
• Retry/acceptance semantics: unchanged and still fail-closed; nontransient branch still no POST retry/reconciliation (confirmed by call path and tests).

External Output Contract

Consumer token/pattern	Producer location	Producer emission condition	Consumer assumption	Ordinary negative case	Downstream effect
message / typeKey extracted from JSON	Azure DevOps REST error response body carried in ErrorRecord.ErrorDetails.Message	Emitted when Azure includes those top-level fields	If absent, fallback redaction is sufficient for raw body	Valid JSON error without top-level message but containing token fields ({"access_token":"..."})	Token remains in formatted diagnostic because only key=value token syntax is redacted
Redaction regex `(?i)\b(...	access_token	...	token)=([^&\s]+)`	Fallback formatter in this script (line 127)	Matches assignment syntax only

Verdict: NEEDS_CHANGES

Confidence: low
Summary: The PR correctly improves diagnostic richness and preserves nontransient queue semantics, but it leaves a concrete fallback redaction defect for valid JSON bodies lacking top-level message. Prior review already flagged this and it remains unresolved at submitted head. CI/required-check status is currently undetermined from this environment, further capping confidence.

---

Evidence threshold for concrete pr-plus-reviewer refinement

Status: MET

Precise defect/mechanism:
Fallback branch in Get-AzdoQueueFailureMessage redacts assignment-style secrets but not JSON token key/value pairs; when JSON parses but lacks top-level message, raw JSON is emitted after incomplete redaction.

Exact minimal patch needed (do not implement here):

1. Extend fallback redaction to cover JSON credential fields (client_assertion, access_token, id_token, refresh_token, token) in quoted JSON key/value form (and escaped quote variants after stringification), while preserving current truncation bounds.
2. Keep existing message/typeKey preference behavior unchanged.

Exact minimal tests needed (do not implement here):

1. Add a unit test in Queue-CiFixAzdoValidation.Tests.ps1 where ErrorDetails.Message is valid JSON without top-level message and contains {"access_token":"sensitive123"}; assert output contains redacted value and does not contain sensitive123.
2. Add equivalent coverage for another key from the same set (e.g., client_assertion) to prevent single-key overfitting.

---

🛠️ Try-Fix — Analysis & Comparison

Alternative generation: not requested

Evidence-first mode intentionally omits routine Try-Fix attempts. This is not a failed
attempt, a completed alternative search, or evidence that alternatives have no value.
The expert review may still produce one evidence-backed PR refinement.

---

🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Trusted evidence-first context consumed; legacy Pre-Flight review was not repeated
Expert Review	❌ NEEDS_CHANGES (low confidence)	1 blocking correctness/security finding; prior inline finding remains unresolved
Gate	⚠️ SKIPPED	Trusted Gate reported no tests detected; Gate was not rerun
Try-Fix	Not requested	Evidence-first mode intentionally omitted routine alternative generation
Refinement	⚠️ VALIDATION BLOCKED	pr-plus-reviewer was implemented, but Pester was unavailable before tests executed
Report	✅ COMPLETE	Compared only the two real candidates

Code Review and Candidate Comparison

Candidate	Evidence	Regression validation	Assessment
pr	Preserves one-POST behavior and adds bounded Azure diagnostics, but quoted JSON token values can reach emitted errors	Gate skipped; author-reported Pester results were not independently reproduced here	Not acceptable as submitted because the expert finding is concrete and unresolved
pr-plus-reviewer	Adds ordinary and escaped JSON credential-field redaction plus focused tests for three credential keys	Blocked before execution because Pester is absent in the isolated candidate environment	Preferred implementation, but not demonstrated merge-ready until incorporated and successfully validated

Selected Fix: pr-plus-reviewer — it directly addresses the source-backed redaction defect that remains in the submitted PR. Because this is an unsubmitted refinement and its sole validation pass was blocked, selecting it still requires changes to the submitted PR and does not establish merge readiness.

Summary

The submitted change improves Azure queue-failure diagnostics without weakening nontransient queue semantics, but its fallback can expose credential values from valid JSON bodies that omit a top-level message. The expert NEEDS_CHANGES finding independently vetoes approval, and the skipped Gate provides no compensating regression evidence. Routine Try-Fix was not requested.

Root Cause

After JSON parsing, the helper replaces the raw response only when a top-level Azure message exists. Otherwise it emits the original JSON after applying patterns for authorization headers, assignment-style tokens, and JWTs. Those patterns do not match quoted key/value syntax such as "access_token":"sensitive123", so the secret can survive into the thrown error, pipeline result, JSON output, and job summary.

Fix Quality

The submitted structure is otherwise bounded and preserves the intended fail-closed queue behavior. The pr-plus-reviewer refinement surgically extends redaction to ordinary and stringified JSON credential fields and adds focused regression cases without changing queue/retry behavior. Its validation is unknown—not passing—because Pester was unavailable before test execution; the author must incorporate the refinement and run the focused Pester coverage before the candidate can be considered validated.

---

🧭 Next Steps — reviewer changes required

The reviewer-enhanced candidate identified changes that are not yet in the submitted PR.

Why: The refinement directly closes the submitted fix's source-backed JSON credential-redaction gap while preserving its bounded diagnostics and queue semantics. It wins the comparison, but the submitted PR still requires these changes and successful focused validation because the candidate's sole Pester pass was blocked before execution.

Address the actionable findings in this review before merging.

[kubaflo]

Copilot found something

[Copilot]

🔵 Needs a closer look

The trusted dispatcher changes appear sound, but the current Pester check is failing and other checks remain pending.

0 open findings

1 resolved since last review

• Redact access_token in JSON fallback error bodies

🧠 Review effort: Balanced