Improve Azure DevOps authentication diagnostics

[Amaury Levé (Evangelink)]

Summary

• disable automatic redirects for Azure DevOps REST requests on supported transports
• report actionable SYSTEM_ACCESSTOKEN guidance for 401, 302, and browser opaqueredirect responses
• preserve WASI compatibility and report successful HTML responses by status/content type instead of surfacing JSON parse failures
• add localized diagnostics and regression coverage

Validation

• build.cmd -projects test\UnitTests\Microsoft.Testing.Extensions.UnitTests\Microsoft.Testing.Extensions.UnitTests.csproj -binaryLog
• 151 focused Azure DevOps publishing tests passed on net8.0

Fixes #10983

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.

Highlights:

• Resource disposal is correctly handled in both the HTML-content and auth-failure paths (explicit response.Dispose() before throw in the HTML path; finally block covers the auth path).
• OperatingSystem.IsWasi() has a polyfill in src/Polyfills/OperatingSystem.cs, so netstandard2.0 builds compile fine.
• .resx entries use proper {Locked} markers with sufficient context; .xlf files appear build-generated.
• New tests cover 401, 302, opaque-redirect, and HTML-response scenarios with correct assertions.

[Copilot]

Copilot review overview

🟡 Changes recommended

Throwing for successful HTML publish responses can requeue accepted batches and create duplicate Azure DevOps result rows.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	src/​Platform/​Microsoft.Testing.Extensions.AzureDevOpsReport/​AzureDevOpsTestResultsClient.Http.cs — This check also runs for PublishTestResultsWithSubResultsAsync, where throwing marks the batch as…

What changed in this PR

Improves Azure DevOps authentication diagnostics while preserving browser/WASI compatibility.

Changes:

• Disables supported automatic redirects and diagnoses authentication redirects.
• Reports unexpected successful HTML responses with status/content type.
• Adds localized resources and regression tests.

File	Description
AzureDevOpsTestResultsClient.Http.cs	Adds redirect and response diagnostics.
AzureDevOpsResources.resx	Adds diagnostic messages.
AzureDevOpsResources.cs.xlf	Updates Czech localization.
AzureDevOpsResources.de.xlf	Updates German localization.
AzureDevOpsResources.es.xlf	Updates Spanish localization.
AzureDevOpsResources.fr.xlf	Updates French localization.
AzureDevOpsResources.it.xlf	Updates Italian localization.
AzureDevOpsResources.ja.xlf	Updates Japanese localization.
AzureDevOpsResources.ko.xlf	Updates Korean localization.
AzureDevOpsResources.pl.xlf	Updates Polish localization.
AzureDevOpsResources.pt-BR.xlf	Updates Brazilian Portuguese localization.
AzureDevOpsResources.ru.xlf	Updates Russian localization.
AzureDevOpsResources.tr.xlf	Updates Turkish localization.
AzureDevOpsResources.zh-Hans.xlf	Updates Simplified Chinese localization.
AzureDevOpsResources.zh-Hant.xlf	Updates Traditional Chinese localization.
AzureDevOpsTestResultsClientHandlerTests.cs	Verifies redirects are disabled.
AzureDevOpsLivePublishingTests.cs	Covers authentication and HTML diagnostics.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

🧪 Expert test review — PR #10999

Grade	Test	Mutation	Notes	How to improve
A (90–100)	mod AzureDevOpsTestResultsClientHandlerTests. CreateHttpClientHandler_ WhenPlatformSupportsDecompression_ DisablesRedirectsAndOptsIntoGZipAndDeflate	2/2 killed	Asserts AllowAutoRedirect is disabled and guards the WASI-specific skip condition.	—
A (90–100)	new AzureDevOpsTestResultsClientHandlerTests. AzureDevOpsTestResultsClient_ AuthenticationFailure_ ReportsInvalidAccessTokenGuidance	3/3 killed	Data-driven over Redirect/Unauthorized precisely verifies IsAuthenticationFailure and message content.	—
A (90–100)	new AzureDevOpsTestResultsClientHandlerTests. AzureDevOpsTestResultsClient_ BrowserOpaqueRedirect_ ReportsInvalidAccessTokenGuidance	2/2 killed	Covers the status-code-0/opaqueredirect branch distinctly from the numeric-status DataRow cases.	—
A (90–100)	new AzureDevOpsTestResultsClientHandlerTests. AzureDevOpsTestResultsClient_ SuccessfulHtmlResponse_ ReportsStatusAndContentType	2/2 killed	Verifies both status code and content-type formatting for the unexpected-HTML-on-success guard.	—

_{This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Re-run with /review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 74.1 AIC · ⌖ 2.58 AIC · ⊞ 16.9K · [◷]( · ◷)

[github-actions]

🧪 Expert test review — PR #10999

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new AzureDevOpsLivePublishingTests. AzureDevOpsTestResultsClient_ AuthenticationFailure_ ReportsInvalidAccessTokenGuidance	2/2 killed	DataRow covers 302 and 401; asserts exact status token and both guidance strings.	—
A (90–100)	new AzureDevOpsLivePublishingTests. AzureDevOpsTestResultsClient_ BrowserOpaqueRedirect_ ReportsInvalidAccessTokenGuidance	1/1 killed	Covers the status==0/opaqueredirect branch the DataRow test above cannot reach.	—
A (90–100)	new AzureDevOpsLivePublishingTests. AzureDevOpsTestResultsClient_ SuccessfulHtmlResponse_ ReportsStatusAndContentType	1/1 killed	Exercises the throw-on-HTML path (default throwOnUnexpectedContentType=true) with exact message assertions.	—
A (90–100)	new AzureDevOpsLivePublishingTests. AzureDevOpsTestResultsClient_ PublishTestResults_ SuccessfulHtmlResponseReturnsNullAndReportsDiagnostic	1/1 killed	Covers the non-throwing/log-and-return-null branch, asserting both null result and logged diagnostic.	—
A (90–100)	mod AzureDevOpsTestResultsClientHandlerTests. CreateHttpClientHandler_ WhenPlatformSupportsDecompression_ DisablesRedirectsAndOptsIntoGZipAndDeflate	1/1 killed	New assertion on AllowAutoRedirect kills the added redirect-disabling mutation, guarded correctly for WASI.	—

Together these four new tests give solid coverage of AzureDevOpsTestResultsClient's HTML-content-type diagnostic and authentication-failure-guidance branches (throwing vs. non-throwing paths, numeric status vs. opaque-redirect status), and the modified handler test correctly asserts the new AllowAutoRedirect = false behavior. No high-confidence actionable findings — no inline suggestions posted.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 78.9 AIC · ⌖ 2.51 AIC · ⊞ 16.9K · [◷]( · ◷)

[Copilot]

Copilot review overview

🔵 Needs a closer look

A logger failure after an accepted publish can still cause the batch to be retried and duplicated.

Review tier: Balanced
Findings: None

Issues resolved since last review (1)

Severity	Finding
	src/​Platform/​Microsoft.Testing.Extensions.AzureDevOpsReport/​AzureDevOpsTestResultsClient.Http.cs — This check also runs for PublishTestResultsWithSubResultsAsync, where throwing marks the batch as… View resolved comment

Suppressed comments (1)

src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/AzureDevOpsTestResultsClient.Http.cs:123

• On this non-idempotent POST path, a failing logger provider can throw after Azure DevOps has already returned 2xx. Logger.LogAsync propagates provider failures, so the publisher then treats the accepted batch as failed and requeues it, potentially creating duplicate result rows. Make this diagnostic best-effort, as the surrounding publisher's TryLogWarning already does; a ThrowOnLog regression case would also protect this guarantee.

                                await _logger.LogWarningAsync(diagnostic).ConfigureAwait(false);

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation addresses the reported failure modes with appropriate platform guards, idempotency handling, localization, and regression coverage.

Review tier: Balanced
Findings: None

[github-actions]

🧪 Expert test review — PR #10999

Grade	Test	Mutation	Notes	How to improve
B (80–89)	new AzureDevOpsTestResultsClientTests. AzureDevOpsTestResultsClient_ PublishTestResults_ LoggerFailureDoesNotReplaySuccessfulHtmlResponse	1/1 killed	Only asserts IsNull; "does not replay" is proven only implicitly by queue exhaustion.	Assert the request was sent exactly once (e.g. track invocation count in the handler) to make the no-replay contract explicit.
A (90–100)	mod AzureDevOpsTestResultsClientHandlerTests. CreateHttpClientHandler_ WhenPlatformSupportsDecompression_ DisablesRedirectsAndOptsIntoGZipAndDeflate	4/4 killed	New assertion correctly verifies AllowAutoRedirect is disabled outside wasi.	—
A (90–100)	new AzureDevOpsTestResultsClientTests. AzureDevOpsTestResultsClient_ AuthenticationFailure_ ReportsInvalidAccessTokenGuidance	3/3 killed	Data-driven coverage of 302/401 with precise substring checks on status and guidance text.	—
A (90–100)	new AzureDevOpsTestResultsClientTests. AzureDevOpsTestResultsClient_ BrowserOpaqueRedirect_ ReportsInvalidAccessTokenGuidance	3/3 killed	Covers the browser opaqueredirect status-0 branch distinct from the HTTP-status branch.	—
A (90–100)	new AzureDevOpsTestResultsClientTests. AzureDevOpsTestResultsClient_ SuccessfulHtmlResponse_ ReportsStatusAndContentType	2/2 killed	Verifies the throwing diagnostic path (status code + content type) for the create-run request.	—
A (90–100)	new AzureDevOpsTestResultsClientTests. AzureDevOpsTestResultsClient_ PublishTestResults_ SuccessfulHtmlResponseReturnsNullAndReportsDiagnostic	3/3 killed	Confirms the non-idempotent publish path returns null and logs the diagnostic instead of throwing.	—

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 101.9 AIC · ⌖ 2.63 AIC · ⊞ 16.9K · [◷]( · ◷)

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation addresses the reported failure modes with focused regression coverage and no unresolved issues.

Review tier: Balanced
Findings: None

[github-actions]

🧵 Parallel-safety audit — PR #10999

Parallelization — assembly audited:

Test assembly	Scope	Workers	Analyzer coverage
Microsoft.Testing.Extensions.UnitTests	MethodLevel	0 (CPU count)	coverable once the parallel-safety analyzers ship (attribute-based opt-in via Program.cs)

This PR touches two test files in Microsoft.Testing.Extensions.UnitTests, which runs under [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] — every test method is its own concurrent chunk, so intra-class conflicts are live here.

Changes reviewed:

• AzureDevOpsLivePublishingTests.cs — adds 6 new [TestMethod]s covering AzureDevOps auth-failure guidance, opaque-redirect handling, and HTML-response diagnostics. Each test constructs its own QueueHttpMessageHandler, HttpClient, AzureDevOpsTestResultsClient, and (where used) CollectingLogger — no shared/static fields, no Environment.*, no filesystem paths, no [ResourceLock]/[DoNotParallelize] declarations added or removed.
• AzureDevOpsTestResultsClientHandlerTests.cs — renames one test and adds two assertions (AllowAutoRedirect, an OperatingSystem.IsWasi() guard) against a handler instance local to the test method.

Findings: A (global-state) 0 · B (paths) 0 · C (declaration) 0 · D (over-serialization) 0 — by severity: Critical 0 · High 0 · Warning 0 · Info 0.

No process-global state mutation, shared filesystem path, or resource-lock declaration issue found in the changed lines. All new/modified test bodies operate on per-test instances (handler, HttpClient, client, logger), which is exactly the pattern that stays safe under MethodLevel parallelism.

_{Advisory only — heuristic, non-blocking. Re-run with /parallel-audit. This audit answers "is it parallel-safe?"; for testability, smells, or flakiness see the detect-static-dependencies / test-smell-detection / test-anti-patterns analyses.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 57.5 AIC · ⌖ 4.16 AIC · ⊞ 24.8K · [◷]( · ◷)

[github-actions]

🧪 Expert test review — PR #10999

No new or modified test methods were identified in the changed regions
of this PR (the pre-step file/line-range extraction data was not available
in this run).

_{Re-run with /review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 31.1 AIC · ⌖ 2.46 AIC · ⊞ 16.9K · [◷]( · ◷)

[Youssef Fahmy (Youssef1313)]

Can you check with Helix folks how does this work for fork PRs where Helix is in a similar situation as MTP extensions?

[Amaury Levé (Evangelink)]

Can you check with Helix folks how does this work for fork PRs where Helix is in a similar situation as MTP extensions?

Correction: my previous reply was incomplete and misleading. I inspected the Helix task implementation but missed Arcade's authoritative security guidance.

Evidence:

• Arcade's current security documentation explicitly says: "It is recommended that you do NOT enable" Make secrets available to builds of forks.
• The current Helix task still emits the opposite recommendation and says its team evaluated that risk for its own public validation builds.
• This contradiction is longstanding: the documentation already contained the do-not-enable policy in March 2020, while the May 2020 Helix change preserved the opposing message when adding 401 handling.

Conclusion: the Helix task's embedded error text is not a reliable statement of current Arcade security policy and appears stale or narrowly team-specific. This PR now follows the documented safe default: do not expose secrets to untrusted fork code; publish from a separate trusted context or disable live publishing.

[Amaury Levé (Evangelink)]

/backport to rel/4.4

[github-actions]

Started backporting to rel/4.4: https://github.com/microsoft/testfx/actions/runs/33860538309