Skip to content

[main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade - #10565

Open
dotnet-maestro[bot] wants to merge 9 commits into
mainfrom
darc-main-eeca6b06-f0f3-411c-941f-0cdebef1e441
Open

[main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade#10565
dotnet-maestro[bot] wants to merge 9 commits into
mainfrom
darc-main-eeca6b06-f0f3-411c-941f-0cdebef1e441

Conversation

@dotnet-maestro

@dotnet-maestro dotnet-maestro Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

This pull request updates the following dependencies

From https://github.com/dotnet/arcade

From https://dev.azure.com/devdiv/DevDiv/_git/vs-code-coverage

…811.2

On relative base path root
Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.XliffTasks From Version 11.0.0-beta.26410.10 -> To Version 11.0.0-beta.26411.2
Copilot AI balanced review requested due to automatic review settings August 12, 2026 02:02
@github-actions
github-actions Bot enabled auto-merge (squash) August 12, 2026 02:02

@microsoft-github-policy-service microsoft-github-policy-service Bot 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.

Auto-approve

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.

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Updates the Arcade toolset and extends Helix job monitoring to optionally control per-test result attachments, alongside a reliability fix in the GitHub App installation lookup script.

Changes:

  • Bumped Microsoft.DotNet.Arcade.Sdk / templating toolset versions to 11.0.0-beta.26411.2 (and updated SHAs).
  • Added testResultAttachmentMode parameter to the Helix job monitor template and forwarded it to the monitoring tool.
  • Fixed PowerShell GitHub App installation enumeration and added deterministic handling for multiple matching installations.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
global.json Updates Arcade SDK version used by the repo toolset.
eng/common/core-templates/job/helix-job-monitor.yml Adds a new parameter and forwards it as a CLI flag to control test result attachments.
eng/common/Get-GitHubAppToken.ps1 Fixes handling of top-level JSON arrays and adds explicit validation for multiple installation matches.
eng/Versions.props Aligns templating package version with updated Arcade toolset.
eng/Version.Details.xml Updates Arcade dependency versions and SHAs to match the toolset bump.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.


organization='${{ parameters.organization }}'
repository='${{ parameters.repository }}'
testResultAttachmentMode='${{ parameters.testResultAttachmentMode }}'
Comment on lines +245 to +247
if [ -n "$testResultAttachmentMode" ]; then
toolArgs+=( --test-result-attachment-mode "$testResultAttachmentMode" )
fi
exit 1
}
$installation = $matchingInstallations[0]
Write-Host "Using installation $($installation.id) for '$($installation.account.login)'."
…code-coverage build 20260811.4

On relative base path root
Microsoft.Testing.Extensions.CodeCoverage From Version 18.11.0-preview.26410.2 -> To Version 18.11.0-preview.26411.4
@dotnet-maestro dotnet-maestro Bot changed the title [main] Update dependencies from dotnet/arcade [main] Update dependencies from devdiv/DevDiv/vs-code-coverage, dotnet/arcade Aug 12, 2026
Copilot AI review requested due to automatic review settings August 12, 2026 03:07

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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

eng/common/core-templates/job/helix-job-monitor.yml:247

  • The testResultAttachmentMode parameter has an explicit default (Failed) and a constrained values: list, so it will always be non-empty for valid template invocations. The -n conditional is therefore redundant and adds branching that can drift over time. Consider always appending --test-result-attachment-mode unconditionally (or, if you want an escape hatch, make it explicit via an additional boolean like enableTestResultAttachmentsArg).
      if [ -n "$organization" ]; then toolArgs+=( --organization "$organization" ); fi
      if [ -n "$repository" ];   then toolArgs+=( --repository   "$repository" );   fi
      if [ -n "$testResultAttachmentMode" ]; then
        toolArgs+=( --test-result-attachment-mode "$testResultAttachmentMode" )
      fi

eng/common/Get-GitHubAppToken.ps1:143

  • Write-Host is hard to control/redirect in automation and can add noise to stdout for callers. Prefer Write-Verbose (with -Verbose opt-in) or Write-Information so pipeline consumers can suppress or route it without affecting standard output.
Write-Host "Using installation $($installation.id) for '$($installation.account.login)'."

…812.6

On relative base path root
Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.XliffTasks From Version 11.0.0-beta.26410.10 -> To Version 11.0.0-beta.26412.6
Copilot AI review requested due to automatic review settings August 13, 2026 02:02

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.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

eng/common/tools.sh:74

  • Using ${var:-default} makes an explicitly provided empty value indistinguishable from an unset value, so callers can no longer clear warn_not_as_error by setting it to empty. If clearing is still intended to be supported, switch to ${warn_not_as_error-default} (no colon) so that only an unset variable gets the default, while an explicitly empty string remains empty.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/tools.ps1:43

  • This logic treats an explicitly provided empty string the same as 'not provided' and forces the default NU1901–NU1904 list, removing the ability to intentionally clear warnNotAsError. If empty is a valid user choice, base the default solely on whether the variable/parameter was provided (e.g., Test-Path variable: or $PSBoundParameters.ContainsKey(...) at the entrypoint), rather than also requiring $warnNotAsError to be truthy.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

eng/common/build.sh:46

  • The usage text documents --msbuildMultiThreaded / --mt, but the actual option parsing only recognizes -msbuildmultithreaded / -mt. This makes the help output inaccurate and will cause documented invocations to fail. Either update the usage text to match the accepted flags, or update the case patterns to accept the documented --... forms as well.
  echo "  --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"

eng/common/build.sh:205

  • The usage text documents --msbuildMultiThreaded / --mt, but the actual option parsing only recognizes -msbuildmultithreaded / -mt. This makes the help output inaccurate and will cause documented invocations to fail. Either update the usage text to match the accepted flags, or update the case patterns to accept the documented --... forms as well.
    -msbuildmultithreaded|-mt)

…code-coverage build 20260812.2

On relative base path root
Microsoft.Testing.Extensions.CodeCoverage From Version 18.11.0-preview.26410.2 -> To Version 18.11.0-preview.26412.2
Copilot AI review requested due to automatic review settings August 13, 2026 03:40

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.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Suppressed comments (3)

eng/common/tools.sh:8

  • NormalizeBoolArg only translates 1/0; values like True, FALSE, or true/false with different casing will pass through unchanged. Since later checks compare with == true (e.g., $msbuild_multi_threaded), passing --mt True (or similar) will not enable the feature. Normalize input case (and optionally accept common spellings like true/false) so comparisons are reliable.
function NormalizeBoolArg {
  case "${1:-}" in
    1) echo true ;;
    0) echo false ;;
    *) echo "${1:-}" ;;
  esac
}

eng/common/tools.sh:74

  • Using ${var:-default} makes an explicitly-empty warn_not_as_error fall back to the default, which removes the ability to intentionally opt out (e.g., forcing NU1901-NU1904 to be treated as errors) by setting it to empty. If opt-out is intended/needed, use an “unset only” defaulting pattern so an explicitly-empty value is respected.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/tools.ps1:43

  • The condition -and $warnNotAsError treats an explicitly-empty value as “not provided” and forces the default, which prevents callers from intentionally clearing the list. If empty is meant to be a valid override (to treat all warnings as errors), consider keying only on variable presence (or a separate explicit opt-out flag) rather than truthiness.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

Comment thread eng/common/build.sh
echo " --pipelinesLog Promote msbuild errors/warnings to Azure Pipelines timeline issues; defaults to on in CI (short: -pl)"
echo " --prepareMachine Prepare machine for CI run, clean up processes after build"
echo " --nodeReuse <value> Sets nodereuse msbuild parameter ('true' or 'false')"
echo " --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"
Comment thread eng/common/build.sh
node_reuse=$2
shift
;;
-msbuildmultithreaded|-mt)
…813.5

On relative base path root
Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.XliffTasks From Version 11.0.0-beta.26410.10 -> To Version 11.0.0-beta.26413.5
Copilot AI review requested due to automatic review settings August 14, 2026 02:02

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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Suppressed comments (5)

eng/common/tools.sh:74

  • Using ${var:-default} forces the NU1901-NU1904 default even when the caller explicitly provides an empty value (e.g., --warnNotAsError \"\"), making it impossible to opt out of the new default. If opting out is a supported/necessary scenario, consider tracking whether the option was explicitly specified (similar to the PowerShell PSBoundParameters pattern) and only applying the default when it was not specified.
# Specifies semi-colon delimited list of warning codes that should not be treated as errors.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/build.ps1:182

  • This removes the previous opt-in path via the MSBUILD_NODEREUSE_ENABLED environment variable on CI. If anything external still relies on that env var, this is a breaking behavior change. If backward-compatibility is required, consider honoring MSBUILD_NODEREUSE_ENABLED=1 when -nodeReuse was not explicitly provided (or document/deprecate the env var removal where consumers will see it).
    # Node reuse isn't used on CI unless it was explicitly requested via -nodeReuse.
    if (-not $PSBoundParameters.ContainsKey('nodeReuse')) {
      $nodeReuse = $false
    }

eng/common/core-templates/steps/publish-logs.yml:8

  • Because this template is under eng/common (typically shared), defaulting enableInternalRuntimes to true means any non-public Azure DevOps project will try to use the internal runtime feed and the dotnetbuilds-internal-container-read-token-base64 secret. In projects that don't define that secret, the task can fail at runtime. Consider making the default false (opt-in), or further guarding on presence of the secret variable/known internal project identifiers before adding -runtimeSourceFeedKey.
  enableInternalRuntimes: true

eng/common/core-templates/steps/publish-logs.yml:36

  • Because this template is under eng/common (typically shared), defaulting enableInternalRuntimes to true means any non-public Azure DevOps project will try to use the internal runtime feed and the dotnetbuilds-internal-container-read-token-base64 secret. In projects that don't define that secret, the task can fail at runtime. Consider making the default false (opt-in), or further guarding on presence of the secret variable/known internal project identifiers before adding -runtimeSourceFeedKey.
    ${{ if and(parameters.enableInternalRuntimes, ne(variables['System.TeamProject'], 'public')) }}:
      arguments: -InputPath '$(System.DefaultWorkingDirectory)/PostBuildLogs'
        -BinlogToolVersion '${{parameters.BinlogToolVersion}}'
        -TokensFilePath '$(System.DefaultWorkingDirectory)/eng/BinlogSecretsRedactionFile.txt'
        -runtimeSourceFeed https://ci.dot.net/internal
        -runtimeSourceFeedKey '$(dotnetbuilds-internal-container-read-token-base64)'
        '$(System.AccessToken)'
        ${{parameters.CustomSensitiveDataList}}

eng/common/tools.sh:9

  • NormalizeBoolArg only normalizes 1/0 and passes through other values unchanged. Since later checks compare strictly to true (lowercase), inputs like True/FALSE will silently behave as false. If these values can originate from user input or environment variables, consider normalizing case as well (e.g., mapping TRUE/Truetrue, FALSE/Falsefalse) to make behavior more robust.
function NormalizeBoolArg {
  case "${1:-}" in
    1) echo true ;;
    0) echo false ;;
    *) echo "${1:-}" ;;
  esac
}

Comment thread eng/common/build.sh
echo " --pipelinesLog Promote msbuild errors/warnings to Azure Pipelines timeline issues; defaults to on in CI (short: -pl)"
echo " --prepareMachine Prepare machine for CI run, clean up processes after build"
echo " --nodeReuse <value> Sets nodereuse msbuild parameter ('true' or 'false')"
echo " --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"
Comment thread eng/common/build.sh
node_reuse=$2
shift
;;
-msbuildmultithreaded|-mt)
…code-coverage build 20260813.3

On relative base path root
Microsoft.Testing.Extensions.CodeCoverage From Version 18.11.0-preview.26410.2 -> To Version 18.11.0-preview.26413.3
Copilot AI review requested due to automatic review settings August 14, 2026 03:04

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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (5)

eng/common/build.sh:46

  • The usage text advertises the short option as --mt, but the argument parser only recognizes -mt. Either update the usage string to say (short: -mt) or extend the case pattern to accept --mt as well to match the help text.
  echo "  --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"

eng/common/build.sh:205

  • The usage text advertises the short option as --mt, but the argument parser only recognizes -mt. Either update the usage string to say (short: -mt) or extend the case pattern to accept --mt as well to match the help text.
    -msbuildmultithreaded|-mt)
      msbuild_multi_threaded=$2
      shift

eng/common/tools.sh:4

  • The comment is inaccurate: PowerShell [bool] parameters can bind from string values like "true"/"false" in addition to 1/0. Consider adjusting the comment to reflect the actual reason for normalization (e.g., consistent cross-shell invocation and explicit support for 1/0), without claiming [bool] only binds 1/0.
# Normalizes the value of a boolean build argument. Accepts 1/0 in addition to true/false so that
# the same value works with the PowerShell scripts, whose [bool] parameters only bind 1/0.
function NormalizeBoolArg {

eng/common/tools.sh:74

  • Using ${warn_not_as_error:-...} makes it impossible to intentionally clear the list (an explicitly empty value will still be replaced by the default). If consumers need a way to opt out of exempting NU1901-NU1904, consider introducing an explicit disable value (e.g., none) or switching the defaulting semantics to only apply when the variable is truly unset (and then ensure wrappers don’t pre-initialize it to '').
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/tools.ps1:43

  • This logic treats an explicitly provided empty string as “not provided” and forces the default list, which removes the ability to intentionally set “no warnings are exempted.” If you want to preserve the ability to clear it, consider defaulting based on whether the caller provided the parameter (e.g., via $PSBoundParameters.ContainsKey('warnNotAsError')) rather than truthiness, keeping behavior consistent with the Bash scripts.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

…814.2

On relative base path root
Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Templating , Microsoft.DotNet.XliffTasks From Version 11.0.0-beta.26410.10 -> To Version 11.0.0-beta.26414.2
Copilot AI review requested due to automatic review settings August 15, 2026 02:01
…code-coverage build 20260814.1

On relative base path root
Microsoft.Testing.Extensions.CodeCoverage From Version 18.11.0-preview.26410.2 -> To Version 18.11.0-preview.26414.1

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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (5)

eng/common/tools.sh:74

  • This makes it impossible to explicitly clear warn_not_as_error (e.g., to treat NU1901-NU1904 as errors) because :- substitutes the default even when the caller sets an empty string. Consider defaulting only when the value was not provided (e.g., track whether the CLI arg/env var was set), so callers can intentionally override to an empty value when desired.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/tools.ps1:43

  • This prevents callers from intentionally setting an empty warnNotAsError (PowerShell treats empty string as falsey, so it falls back to the default). If the goal is ‘default unless explicitly specified’, consider using a parameter-bound check (e.g., $PSBoundParameters.ContainsKey('warnNotAsError')) in the invoking script, or otherwise carry a separate ‘was specified’ flag so an explicit empty value is respected.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

eng/common/build.sh:46

  • The usage text advertises --msbuildMultiThreaded and --mt, but the argument parser pattern shown only matches -msbuildmultithreaded and -mt (single dash, different casing). This will mislead users and may break documented invocations. Align the help text and the accepted switch patterns (either document the actual accepted forms, or support the documented --msbuildMultiThreaded/--mt forms in the parser).
  echo "  --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"

eng/common/build.sh:205

  • The usage text advertises --msbuildMultiThreaded and --mt, but the argument parser pattern shown only matches -msbuildmultithreaded and -mt (single dash, different casing). This will mislead users and may break documented invocations. Align the help text and the accepted switch patterns (either document the actual accepted forms, or support the documented --msbuildMultiThreaded/--mt forms in the parser).
    -msbuildmultithreaded|-mt)
      msbuild_multi_threaded=$2
      shift
      ;;

eng/common/build.ps1:83

  • This help text indicates only 1/0 are valid values, but the parameter type is [bool] and typically accepts true/false as well. To avoid confusion, consider documenting both accepted forms (e.g., true/false and 1/0) or matching the help text to the actual supported input conventions across the scripts.
  Write-Host "  -msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('1' or '0') (short: -mt)"

Copilot AI review requested due to automatic review settings August 15, 2026 02:03

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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

Suppressed comments (7)

eng/common/tools.ps1:43

  • This defaulting logic prevents callers from explicitly clearing warn-not-as-error (e.g., passing an empty string) because empty becomes the default list again. If the intent is to allow 'no exclusions', default only when the variable is not set (or when the parameter wasn’t provided), not when it’s set to empty.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

eng/common/tools.sh:74

  • Using ${var:-default} makes an explicitly-empty warn_not_as_error indistinguishable from 'unset', so callers can’t intentionally set it to an empty string (i.e., no exclusions). If that capability is required, prefer defaulting only when the variable is truly unset (e.g., ${var-default}), or introduce an explicit sentinel to mean 'use defaults'.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/build.sh:46

  • The usage text advertises --msbuildMultiThreaded / --mt, but the argument parser matches -msbuildmultithreaded / -mt (single dash, different casing). This makes the documented flags not work as written. Align the usage string and the case patterns (including dash count and capitalization) so the documented invocation is actually accepted.
  echo "  --msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('true' or 'false') (short: --mt)"

eng/common/build.sh:205

  • The usage text advertises --msbuildMultiThreaded / --mt, but the argument parser matches -msbuildmultithreaded / -mt (single dash, different casing). This makes the documented flags not work as written. Align the usage string and the case patterns (including dash count and capitalization) so the documented invocation is actually accepted.
    -msbuildmultithreaded|-mt)
      msbuild_multi_threaded=$2
      shift
      ;;

eng/common/tools.sh:9

  • This normalizer handles 1/0 but leaves other inputs untouched. Since downstream checks are case-sensitive (e.g., == true), values like True/FALSE will not behave as expected. Consider normalizing common boolean spellings/casing (e.g., mapping True/False to lowercase) to make bash behavior consistent with typical user input and PowerShell bool conventions.
function NormalizeBoolArg {
  case "${1:-}" in
    1) echo true ;;
    0) echo false ;;
    *) echo "${1:-}" ;;
  esac
}

eng/common/core-templates/job/helix-job-monitor.yml:222

  • Because testResultAttachmentMode has a non-empty default (Failed) and its allowed values are non-empty, the -n check will always pass. Either remove the conditional for simpler control flow, or (if the goal is backward compatibility with older monitor tool versions) gate the arg on whether the parameter was explicitly set rather than on non-emptiness.
      testResultAttachmentMode='${{ parameters.testResultAttachmentMode }}'

eng/common/core-templates/job/helix-job-monitor.yml:247

  • Because testResultAttachmentMode has a non-empty default (Failed) and its allowed values are non-empty, the -n check will always pass. Either remove the conditional for simpler control flow, or (if the goal is backward compatibility with older monitor tool versions) gate the arg on whether the parameter was explicitly set rather than on non-emptiness.
      if [ -n "$testResultAttachmentMode" ]; then
        toolArgs+=( --test-result-attachment-mode "$testResultAttachmentMode" )
      fi

Copilot AI review requested due to automatic review settings August 18, 2026 02:02
…code-coverage build 20260817.1

On relative base path root
Microsoft.Testing.Extensions.CodeCoverage From Version 18.11.0-preview.26410.2 -> To Version 18.11.0-preview.26417.1

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.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

eng/common/tools.sh:4

  • The comment is inaccurate: PowerShell [bool] parameters can bind true/false as well; they don't 'only bind 1/0'. Update the wording to reflect that 1/0 is additionally supported (often for compatibility with callers), rather than being the only accepted format.
# Normalizes the value of a boolean build argument. Accepts 1/0 in addition to true/false so that
# the same value works with the PowerShell scripts, whose [bool] parameters only bind 1/0.

eng/common/tools.sh:74

  • Using ${var:-default} makes it impossible to intentionally clear warn_not_as_error by setting it to an empty string (it will fall back to the default). If callers need an explicit opt-out, default only when the variable is unset (not when set-but-empty), or introduce an explicit switch/value convention to disable the defaults.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
warn_not_as_error="${warn_not_as_error:-NU1901;NU1902;NU1903;NU1904}"

eng/common/tools.ps1:43

  • This logic also prevents intentionally clearing warnNotAsError (passing '' still falls back to the NU1901-NU1904 default because of -and $warnNotAsError). If an explicit empty value should mean 'no exclusions', default only when the variable is not present, and treat an empty-but-present value as an override.
# Defaults to NuGet Audit warning codes NU1901-NU1904.
[string]$warnNotAsError = if ((Test-Path variable:warnNotAsError) -and $warnNotAsError) { $warnNotAsError } else { 'NU1901;NU1902;NU1903;NU1904' }

eng/common/msbuild.sh:41

  • The --mt alias reads like a flag, but the parser requires a value and does shift 2. If a caller uses --mt without an argument, this will mis-parse args (and can error on shift). Consider supporting --mt as a valueless flag (set msbuild_multi_threaded=true) and keeping --msbuildmultithreaded <true|false> (or --msbuildmultithreaded=<...>) for explicit values.
    --msbuildmultithreaded|--mt)
      msbuild_multi_threaded=$2
      shift 2
      ;;

eng/common/build.sh:207

  • Similarly here, -mt is treated as requiring a value in $2, but the alias strongly suggests 'just enable it'. For consistency with the PowerShell experience (where -mt can be used without a value), consider making -mt set msbuild_multi_threaded=true when no explicit value is provided, and only consume $2 when it exists and is a boolean.
    -msbuildmultithreaded|-mt)
      msbuild_multi_threaded=$2
      shift
      ;;

eng/common/tools.sh:612

  • This change removes support for enabling -mt via the prior environment variable (MSBUILD_MT_ENABLED) that older automation may rely on. If backwards compatibility is required, consider honoring the env var as a fallback (while still preferring the new explicit switch), or documenting the migration path clearly.
  # Build with MSBuild's multi-threaded mode.
  local mt_switch=""
  if [[ "$msbuild_multi_threaded" == true ]]; then
    mt_switch="-mt"
  fi

eng/common/tools.ps1:818

  • As in tools.sh, this drops the previous MSBUILD_MT_ENABLED env-var control path. If existing pipelines set that env var today, -mt will silently stop being enabled. Consider supporting the env var as a fallback (or adding a clear deprecation period) to avoid breaking existing CI configurations.
  # Build with MSBuild's multi-threaded mode.
  if ($msbuildMultiThreaded) {
    $cmdArgs += ' -mt'

eng/common/build.ps1:83

  • The usage text implies only 1/0 are accepted, but the parameter is [bool] and typically accepts $true/$false (and true/false) as well. Consider updating the help text to reflect all supported inputs and to match the bash help (true/false).
  Write-Host "  -msbuildMultiThreaded <value> Sets MSBuild's multi-threaded mode, i.e. the -mt switch ('1' or '0') (short: -mt)"

Comment on lines +29 to 45
${{ if and(parameters.enableInternalRuntimes, ne(variables['System.TeamProject'], 'public')) }}:
arguments: -InputPath '$(System.DefaultWorkingDirectory)/PostBuildLogs'
-BinlogToolVersion '${{parameters.BinlogToolVersion}}'
-TokensFilePath '$(System.DefaultWorkingDirectory)/eng/BinlogSecretsRedactionFile.txt'
-runtimeSourceFeed https://ci.dot.net/internal
-runtimeSourceFeedKey '$(dotnetbuilds-internal-container-read-token-base64)'
'$(System.AccessToken)'
${{parameters.CustomSensitiveDataList}}
${{ else }}:
arguments: -InputPath '$(System.DefaultWorkingDirectory)/PostBuildLogs'
-BinlogToolVersion '${{parameters.BinlogToolVersion}}'
-TokensFilePath '$(System.DefaultWorkingDirectory)/eng/BinlogSecretsRedactionFile.txt'
'$(System.AccessToken)'
${{parameters.CustomSensitiveDataList}}
continueOnError: true
condition: always()

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant