Azure DevOps plugin build can ignore npm failures and package stale dependencies

[jozkee]

Description

The Azure DevOps extension build script does not stop when npm or npx exits with a nonzero code. This can make CI report success and package stale dependencies from an existing node_modules directory.

This was exposed by the adm-zip 0.6.0 -> 0.6.1 update in #7763. The new tarball URL in the dotnet-public-npm feed returned HTTP 401, but the bump PR's CI build still passed.

Evidence

• Bump adm-zip from 0.6.0 to 0.6.1 in /src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/TypeScript/azure-devops-report/tasks/PublishAIEvaluationReport #7763 CI build 1605832 was reported successful: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1605832

• Its Build Azure DevOps plugin log contains:

  npm error code E401
  npm error Unable to authenticate, your authentication token seems to be invalid.
  

• The same underlying npm failure later caused build 1608282 to fail: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1608282&view=logs&j=0bc77094-9fcd-5c38-f6e4-27d2ae131589&t=206655b7-7ebc-5b58-0ce4-544602a53fe7

• Builds 1608831 and 1608835 failed the same way. Builds 1608856 and 1609097 logged E401 but were again reported successful.

Root cause

src/Libraries/Microsoft.Extensions.AI.Evaluation.Reporting/TypeScript/azure-devops-report/build.ps1 invokes several native commands without checking $LASTEXITCODE:

npm ci --omit=dev
npm ci
npx tsc -b
npx vite build
npx tfx-cli extension create ...

$ErrorActionPreference does not make ordinary native-command failures terminating errors. After the first npm ci failed, the script continued.

Whether the task ultimately appeared green depended on workspace state:

1. npm ci --omit=dev attempted to clean node_modules and then failed with E401.
2. On agents where Windows EPERM errors left enough of the old directory in place, Copy-Item ./node_modules succeeded and stale dependencies were packaged. Later commands succeeded, so the task was green.
3. When cleanup removed the directory, the subsequent Copy-Item failed and exposed the problem.

The pipeline already uses workspace: clean: all and checkout clean: true, but those settings cannot replace command-level failure handling and do not protect against locked files or state created earlier in the same job.

Impact

• Dependency-update PRs can receive false-green CI.
• A VSIX can be built with dependency versions that do not match package-lock.json.
• Feed/authentication failures can be hidden until a later unrelated PR happens to run on a cleaner agent.
• Results are nondeterministic based on agent filesystem state.

Proposed fix

1. Fail immediately after every npm/npx invocation when $LASTEXITCODE is nonzero, preferably through a shared local helper that throws.
2. Remove node_modules and dist before building, and fail explicitly if either directory cannot be cleaned.
3. Do not copy or package any output unless the corresponding npm ci completed successfully.
4. Add a regression test using a stub npm that exits nonzero and verify that the script exits nonzero without producing a VSIX.

The public-feed visibility problem for adm-zip@0.6.1 also needs correction, but it is separate from ensuring the build cannot report false success.

Acceptance criteria

• Any nonzero npm or npx exit code immediately fails Build Azure DevOps plugin.
• Locked/stale node_modules or dist directories cause a clear failure rather than reuse.
• No VSIX is produced after a failed dependency install or build command.
• Automated coverage reproduces a native-command failure and verifies a nonzero script exit.