Skip to content

[SDK Validation] replace spawnSync pwsh with simple-git in getChangedFiles to fix ENOBUFS on large PRs#40882

Merged
raych1 merged 1 commit intomainfrom
users/raych1/fix-sdk-validation-enobufs
Mar 2, 2026
Merged

[SDK Validation] replace spawnSync pwsh with simple-git in getChangedFiles to fix ENOBUFS on large PRs#40882
raych1 merged 1 commit intomainfrom
users/raych1/fix-sdk-validation-enobufs

Conversation

@raych1
Copy link
Member

@raych1 raych1 commented Feb 28, 2026

Problem

The spec-gen-sdk-runner's getChangedFiles function uses spawnSync to invoke PowerShell (pwsh) for git diff, which has a default maxBuffer of 1MB. For PRs with thousands of changed files, the output exceeds this buffer limit, causing an ENOBUFS error.

When this happens, runPowerShellScript returns undefined, which gets coalesced to an empty array (?? []). The pipeline then sees 0 changed files and skips SDK validation entirely — reporting a false-positive success.

Pipeline log

Error executing PowerShell script: Error: spawnSync pwsh ENOBUFS
No files changed in the PR
Changed files in the PR: 0
No relevant files changed under 'specification' folder in the PR

Fix

Replace the PowerShell-based getChangedFiles in eng/tools/spec-gen-sdk-runner/src/utils.ts with the existing getChangedFiles from @azure-tools/specs-shared/changed-files, which:

  • Uses simple-git (streaming spawn) instead of spawnSyncno buffer size limitation

… ENOBUFS on large PRs

The spec-gen-sdk-runner's getChangedFiles function used spawnSync to invoke
PowerShell for git diff, which has a default maxBuffer of 1MB. For PRs with
thousands of changed files (e.g. folder structure migrations), the output
exceeds this limit causing ENOBUFS. This silently returns undefined, resulting
in 0 changed files detected and a false-positive SDK validation pass.

Replace the PowerShell-based implementation with the existing
getChangedFiles from @azure-tools/specs-shared/changed-files, which uses
simple-git (streaming spawn) and has no buffer size limitation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Feb 28, 2026

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ The required check named Swagger Avocado has failed. Refer to the check in the PR's 'Checks' tab for details on how to fix it and consult the aka.ms/ci-fix guide


Comment generated by summarize-checks workflow run.

@raych1 raych1 marked this pull request as ready for review February 28, 2026 02:12
@raych1 raych1 self-assigned this Feb 28, 2026
@raych1 raych1 moved this from 🤔 Triage to 🔬 Dev in PR in Azure SDK EngSys 📆🎇 Feb 28, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a false-positive success scenario in spec-gen-sdk-runner where large PRs with thousands of changed files could trigger an ENOBUFS error from PowerShell's spawnSync-based getChangedFiles. The undefined return from the failed script was silently coalesced to an empty array, causing the SDK validation step to be skipped while reporting success. The fix replaces the PowerShell approach with the existing simple-git-based implementation from @azure-tools/specs-shared/changed-files, which uses streaming and has no buffer size limit.

Changes:

  • Replaced the PowerShell-based getChangedFiles in utils.ts with an async wrapper around the shared getChangedFilesShared from @azure-tools/specs-shared/changed-files
  • Propagated the async change through detectChangedSpecConfigFiles in spec-helpers.ts and its call site in commands.ts
  • Updated all tests to use mockResolvedValue and async/await to match the new async API

Reviewed changes

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

Show a summary per file
File Description
eng/tools/spec-gen-sdk-runner/src/utils.ts Replaces PowerShell spawnSync-based getChangedFiles with an async wrapper over the shared simple-git implementation
eng/tools/spec-gen-sdk-runner/src/spec-helpers.ts Adds async/await to detectChangedSpecConfigFiles to handle the now-async getChangedFiles
eng/tools/spec-gen-sdk-runner/src/commands.ts Adds await to the detectChangedSpecConfigFiles call
eng/tools/spec-gen-sdk-runner/test/spec-helpers.test.ts Updates all test cases to use mockResolvedValue and await for the async detectChangedSpecConfigFiles
eng/tools/spec-gen-sdk-runner/test/commands.test.ts Updates detectChangedSpecConfigFiles spy mocks from mockReturnValue to mockResolvedValue

@raych1 raych1 merged commit d52d255 into main Mar 2, 2026
66 of 69 checks passed
@raych1 raych1 deleted the users/raych1/fix-sdk-validation-enobufs branch March 2, 2026 17:32
@raych1 raych1 changed the title fix: replace spawnSync pwsh with simple-git in getChangedFiles to fix ENOBUFS on large PRs [SDK Validation] replace spawnSync pwsh with simple-git in getChangedFiles to fix ENOBUFS on large PRs Mar 3, 2026
@kurtzeborn kurtzeborn moved this from 🔬 Dev in PR to 🎊 Closed in Azure SDK EngSys 📆🎇 Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants