Quote --ignore-exit-code values so the quarantine pipeline does not shell-split on Unix - #13918
Conversation
…ll-split on Unix The scheduled quarantine pipeline (azure-pipelines/quarantine.yml, definition 344) sets RunQuarantinedTests=true, which adds '--ignore-exit-code 8;2' to the xUnit v3 test command so a quarantined-only run treats exit code 8 (zero tests ran) and 2 (test failures) as success. Arcade's XUnitV3.Runner.targets runs the assembled command via <Exec>, which on non-Windows executes through /bin/sh. The unquoted ';' was interpreted as a shell command separator, splitting the line into '<runner> ... --ignore-exit-code 8' and a second fragment '2 --results-directory ...'. The latter starts with the token '2', which is not a command, so the step failed with exit code 127 (MSB3073) on the Linux and macOS legs (first observed in build 1445333). Quoting the value keeps '8;2' as a single argument to --ignore-exit-code on both sh and cmd. Verified the parsing through /bin/sh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates unit test runner arguments used by the quarantine pipeline to ensure Unix shells don’t split the --ignore-exit-code value on ;, keeping Linux/macOS quarantine legs from failing when invoked via Arcade’s <Exec>.
Changes:
- Quote the
--ignore-exit-codemulti-exit-code value as"8;2"whenRunQuarantinedTests=true. - Expand the inline comment explaining why the quoting is required for
/bin/shexecution.
Show a summary per file
| File | Description |
|---|---|
| src/Directory.Build.targets | Quotes the --ignore-exit-code value for quarantined test runs and documents the shell-splitting rationale. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
✅ 24/24 dimensions clean — no findings.
Reviewed against all 24 dimensions (backwards compatibility, ChangeWave discipline, performance, test coverage, error messages, logging, string comparison, API surface, target authoring, design, cross-platform correctness, code simplification, concurrency, naming, SDK integration, idiomatic C#, file I/O, documentation, build infrastructure, scope/PR discipline, evaluation model integrity, correctness/edge cases, dependency management, security).
Summary: This is a correct, minimal fix for a real Unix shell-splitting bug in an opt-in CI scenario. The semicolon in 8;2 is a literal character in MSBuild property values and the double quotes are preserved through property expansion into the Exec command string. The added comment is excellent — it documents the "why" and prevents future regressions from well-intentioned quote removal.
Generated by Expert Code Review (on open) for issue #13918 · ● 1.3M
Updated [Microsoft.Build.Utilities.Core](https://github.com/dotnet/msbuild) from 18.8.2 to 18.9.6. <details> <summary>Release notes</summary> _Sourced from [Microsoft.Build.Utilities.Core's releases](https://github.com/dotnet/msbuild/releases)._ ## 18.9.6 ## What's Changed * [vs18.6] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13793 * [vs18.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13859 * [vs18.6] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13858 * [vs18.7] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13863 * CsWin32 follow-up: CLR metadata + TypeLib interop migration by @JeremyKuhne in dotnet/msbuild#13853 * Update vmr-sb-validation.yml for Azure Pipelines by @meghnave in dotnet/msbuild#13871 * Test: keep shell alive 15s in ToolTaskCanChangeCanonicalErrorFormat (#13734) by @jankratochvilcz in dotnet/msbuild#13878 * Add vs18.8 to merge-flow config by @OvesN in dotnet/msbuild#13877 * Stable branding for 18.8 release by @OvesN in dotnet/msbuild#13883 * Bump main to 18.9.0 after vs18.8 snap by @OvesN in dotnet/msbuild#13880 * Avoid checkout in insertion pipeline by @rainersigwald in dotnet/msbuild#13887 * Report actual launch path in MSB4216 for Runtime="NET" task host by @ViktorHofer in dotnet/msbuild#13889 * Migrate Tlblmp and AxImp to Multithreaded Execution by @AlesProkop in dotnet/msbuild#13708 * Replace ErrorUtilities assertion methods with Assumed API and BCL throw helpers by @DustinCampbell in dotnet/msbuild#13790 * Fix CLR_E_SHIM_RUNTIMELOAD in RAR's IMetaDataDispenser activation by @JeremyKuhne in dotnet/msbuild#13899 * [main] Update dependencies from nuget/nuget.client by @dotnet-maestro[bot] in dotnet/msbuild#13905 * [main] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13907 * [main] Update dependencies from dotnet/roslyn by @dotnet-maestro[bot] in dotnet/msbuild#13910 * [vs17.14] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13908 * [vs18.0] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13906 * Fix ToolTask output loss: increase EOF pipe timeout from 2s to 30s by @huulinhnguyen-dev in dotnet/msbuild#13767 * Improve symlink cycle condition by @GangWang01 in dotnet/msbuild#13901 * [vs18.6] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13904 * Add flaky-test detection and auto-fix agentic workflows by @ViktorHofer in dotnet/msbuild#13915 * Quote --ignore-exit-code values so the quarantine pipeline does not shell-split on Unix by @ViktorHofer in dotnet/msbuild#13918 * Fix flaky-test detector PR-evidence loss and raise scan limits by @ViktorHofer in dotnet/msbuild#13919 * Tighten the pr review agent by @JanKrivanek in dotnet/msbuild#13921 * Add environment variables for governance detection by @ViktorHofer in dotnet/msbuild#13920 * Change IsPackable to true and add IsShipping flag by @ViktorHofer in dotnet/msbuild#13924 * [vs18.7] Update dependencies from dotnet/arcade by @dotnet-maestro[bot] in dotnet/msbuild#13911 * Make flaky detector verify recurrence postdates the fix before commenting by @ViktorHofer in dotnet/msbuild#13930 * Fix AbsolutePath.GetCanonicalForm process state leak on Windows by @OvesN in dotnet/msbuild#13788 * Fix flaky detector: unblock dnceng feed, fail fast, and defer quarantine to a second run by @ViktorHofer in dotnet/msbuild#13936 * Update documentation for ImplicitUsings element by @drewnoakes in dotnet/msbuild#13900 * [vs18.6] Point OptProf bootstrapper at rel/stable instead of int.main by @AlesProkop in dotnet/msbuild#13923 * Add CS8618 suppressor for required MSBuild task properties by @AArnott in dotnet/msbuild#13926 * Tighten NodeLaunchData.EnvironmentOverrides nullability to IDictionary<string, string?>? by @OvesN with @Copilot in dotnet/msbuild#13815 * Fix ToolTask EOF wait to be STA-safe via CountdownEvent (MSB4018 in AspNetCompiler) by @YuliiaKovalova in dotnet/msbuild#13917 * [automated] Merge branch 'vs18.6' => 'vs18.7' by @github-actions[bot] in dotnet/msbuild#13941 * Localized file check-in by OneLocBuild Task: Build definition ID 9434: Build ID 14192258 by @dotnet-bot in dotnet/msbuild#13849 * Bumping to 10.0.8 runtime packages by @OvesN in dotnet/msbuild#13898 * Flaky-test workflow: reassure on empty PR list + drop local reproduction (quarantine-first) by @ViktorHofer in dotnet/msbuild#13938 * [Flaky Test] Un-quarantine 5 consistently-green tests by @github-actions[bot] in dotnet/msbuild#13952 * Flaky-test detector: open PRs ready-for-review; drop newly-filed-issues section from PR body by @ViktorHofer in dotnet/msbuild#13958 * [Flaky Test] Quarantine 4 flaky tests by @github-actions[bot] in dotnet/msbuild#13937 * Flaky-test: fix duplicate-issue bug by switching dedup key to a visible code-block key by @ViktorHofer in dotnet/msbuild#13963 * CsWin32 follow-up: WindowsNative + VS Setup Configuration + remaining hand-rolled interop by @JeremyKuhne in dotnet/msbuild#13872 * Add the reviewer release skill checking if the Learn article Change waves is updated by @GangWang01 in dotnet/msbuild#13840 * [main] Source code updates from dotnet/dotnet by @dotnet-maestro[bot] in dotnet/msbuild#13977 ... (truncated) Commits viewable in [compare view](dotnet/msbuild@v18.8.2...v18.9.6). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Problem
The scheduled quarantine pipeline (
azure-pipelines/quarantine.yml, definition 344) failed on its first real run — build 1445333 — on the Linux and macOS legs with:Root cause
RunQuarantinedTests=trueadds--ignore-exit-code 8;2to the xUnit v3 test command (so a quarantined-only run treats exit code8= zero tests ran and2= test failures as success — the per-test signal is the published TRX, not the exit code).Arcade's
XUnitV3.Runner.targetsassembles the command and runs it via<Exec>, which on non-Windows executes through/bin/sh. The unquoted;was interpreted by the shell as a command separator, splitting the line into:<runner> ... --ignore-exit-code 82 --results-directory ...The second fragment starts with the token
2, which is not a command, so the step failed with exit code 127 (command not found).Fix
Quote the value:
--ignore-exit-code "8;2". This keeps8;2as a single argument on both/bin/shandcmd.8;2is the correct semicolon-separated multi-code syntax for Microsoft.Testing.Platform; only the shell-level quoting was missing.Verified the parse through
/bin/sh:2: command not found--ignore-exit-codereceives a single arg8;2Windows was unaffected (
cmddoes not treat;as a command separator), but the quoted form is correct on both.Follow-up to #13915.