Enable package manager lifecycle mappings - W-23773464

[peternhale]

Summary

• support npm, pnpm, and Yarn dependency installation, caching, and selected lockfile updates in reusable VS Code workflows
• expose lifecycle command mappings for install, lint, build, test, coverage, quality, and VSIX packaging
• retain existing npm/VSE defaults and document pnpm, Yarn, and direct-command adoption

Validation

• Prettier check for the changed action, workflows, and README
• Ruby YAML parsing for the changed action and workflows
• git diff --check
• npm run ext-change-detector -- --help
• Cross-checked 11 local direct workflow consumers; all use the preserved npm defaults and package-lock.json.

@W-23773464@

[iowillhoit]

Avoid using inputs in a run:, it can be a RCE risk. Even if the risk is low (we control the inputs) it is still a good habit to use.

Instead set the input on the env: and then read from there. Here is an example

[madhur310]

Overview
This PR adds support for npm, pnpm, and Yarn package managers to VS Code reusable workflows. The changes are well-structured and backward-compatible, but I've identified several issues that should be addressed before merging.

Critical Issues
🔴 .github/workflows/vscode-ci-template.yml:303 - Silent build failure
Issue: The BUILD_COMMAND can be empty, causing the build step to silently succeed without actually building anything.

• name: Build project
  env:
  BUILD_COMMAND: ${{ inputs.build-command || inputs.compile-command }}
  run: $BUILD_COMMAND
  Problem: When a caller passes build-command: '' to override the default, the shell executes an empty command which succeeds with exit code 0. The step shows as passed ✅ but no build actually runs.

Fix: Add validation before executing:

• name: Build project
  env:
  BUILD_COMMAND: ${{ inputs.build-command || inputs.compile-command }}
  run: |
  if [ -z "$BUILD_COMMAND" ]; then
  echo "Error: BUILD_COMMAND cannot be empty"
  exit 1
  fi
  $BUILD_COMMAND
  Or use direct expression: run: ${{ inputs.build-command || inputs.compile-command || 'npm run compile' }}

🟡 .github/workflows/vscode-publish-extensions.yml:953 - Lockfile validation false positive
Issue: The lockfile validation checks for ANY uncommitted changes, not just changes from the version bump step.

if ! git diff --quiet -- "$lockfile"; then
echo "$PACKAGE_MANAGER version bumps must not modify $lockfile"
exit 1
fi
Problem: If an earlier workflow step modifies yarn.lock, and then the version bump runs with npm (only touching package.json/package-lock.json), this validation incorrectly attributes the earlier modification to the version bump.

Fix: Capture lockfile state before version bump:

Before version bump

for lockfile in "${unexpected_lockfiles[@]}"; do
git hash-object "$lockfile" 2>/dev/null > /tmp/${lockfile}.hash || echo "missing" > /tmp/${lockfile}.hash
done

... version bump happens ...

After version bump

for lockfile in "${unexpected_lockfiles[@]}"; do
before=$(cat /tmp/${lockfile}.hash)
after=$(git hash-object "$lockfile" 2>/dev/null || echo "missing")
if [ "$before" != "$after" ]; then
echo "$PACKAGE_MANAGER version bumps must not create or modify $lockfile"
exit 1
fi
done
Breaking Changes
⚠️ .github/workflows/vscode-publish-extensions.yml:1052 - CBWeb publishing now opt-in
Change: The publish-to-cbweb job now requires inputs.publish-web-vsix: true to run.

if: needs.package.result == 'success' && inputs.publish-web-vsix
Impact: Existing callers (nightly.yml, publishVSCode.yml) don't pass this input, so CBWeb publishing will silently stop working.

Action Required:

Add documentation for the new publish-web-vsix input
Update existing callers to pass publish-web-vsix: true if they need CBWeb publishing
Consider making the default true for backward compatibility
Efficiency Issues
🟡 .github/workflows/vscode-ci-template.yml:272 - Wasted runner time
Issue: The quality job runs checkout even when there's no quality command to execute.

quality:
name: Quality Checks
needs: [test-matrix]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout code
uses: actions/checkout@v6
- name: Setup Node.js and install dependencies
if: inputs.quality-command != ''
Problem: Runner is allocated and code checked out (~10-25 seconds) before steps are skipped.

Fix: Add job-level conditional:

quality:
name: Quality Checks
if: inputs.quality-command != '' # Add this
needs: [test-matrix]
🟡 .github/workflows/vscode-package.yml:639 - Artifact path mismatch risk
Issue: The artifact-glob input defaults to packages/**/*.vsix but doesn't validate against extensions-root.

Problem: If a caller sets extensions-root: 'src' but forgets to override artifact-glob, the upload step finds 0 files, causing downstream failures.

Fix: Either derive artifact-glob from extensions-root or add validation:

artifact-glob:
default: "${{ inputs.extensions-root }}/**/*.vsix"
Design Concerns
🟡 .github/actions/setupNodeAndInstall/action.yml:54 - Brittle command matching
Issue: Install routing uses exact string matching:

if: inputs.package-manager == 'npm' && inputs.install-command == 'npm ci'
Problem: Common variations like npm ci --legacy-peer-deps or yarn install --frozen-lockfile bypass the optimized retry actions.

Recommendation: Use pattern matching or only check package-manager:

if: inputs.package-manager == 'npm' && startsWith(inputs.install-command, 'npm ci')
🟡 .github/workflows/vscode-release-explicit.yml:1225 - Removed graceful fallback
Previous behavior: Checked if bundle script exists before running
New behavior: Runs bundle-command unconditionally when not empty

Problem: Workflows fail hard instead of skipping when the bundle script doesn't exist.

Recommendation: Restore the existence check or document that callers must ensure scripts exist.

Minor Issues
🟡 .github/workflows/vscode-ci-template.yml:286 - No cache validation
Issue: Dynamic cache: ${{ inputs.package-manager }} accepts invalid values without error.

Problem: Typos or empty values cause silent caching failures, slowing builds 2-3x.

Recommendation: Add validation in setupNodeAndInstall action or document valid values clearly.

Summary
Overall, this is a well-designed refactor that adds important multi-package-manager support. The main concerns are:

Critical: Fix silent build failure when BUILD_COMMAND is empty
Important: Fix lockfile validation logic to avoid false positives
Breaking: Document CBWeb publishing opt-in change and update callers
Nice-to-have: Optimize quality job and improve command matching robustness
The changes are backward-compatible for most use cases, but existing users of CBWeb publishing will need to update their workflow calls.

[iowillhoit]

I'm sure you know this, but change all these back to main before merging. I have forgotten this before myself 😵‍💫

[iowillhoit]

I think this if is not quite right.

If you pass package-manager = pnpm but forget to pass in the custom install-command input, the default npm ci will be used here with pnpm

if using pnpm:
!npm (true) && !yarn (true) -> true
-> Uses pnpm "installer" with install command npm ci