fix(server): skip cmd.exe for multi-line args on Windows - #4428
Closed
eden881 wants to merge 1 commit into
Closed
Conversation
Collaborator
|
Closing for now: this needs Windows before/after output showing that the child receives multiline arguments intact through both |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change
Reasoning
On Windows every
gh api graphqlcall the daemon makes fails, so anything backed by GraphQL is quietly missing from the changes pane - merge state, auto merge facts, PR timeline and comments, and the batched PR status poll.Nothing surfaces in the UI because
loadPullRequestGithubFactscatchesGitHubCommandErrorand returnsnull, so the pane just renders less than it should and you have no idea why.I found this on my own machine after a separate
ghproblem sent me intoC:\Users\<USER>\.paseo\daemon.log, which was full of:Column 35 is the end of
query PaseoBatchPullRequestStatus {, so GitHub only ever received the first line of the query.Timeline is basically:
createForgeCliRunnercallsexecCommand("gh", ...)with the bare binary nameshouldUseWindowsShellreturns true onwin32for a command with no path separator and no extension, so the spawn goes throughcmd.exe /d /s /ccmd.execan't carry a line break inside an argument. It drops everything after a\nand silently swallows a lone\r.quoteWindowsArgumenthandles& | ^ < > ( ) !but has nothing for line breaksghgets a one-line query, GitHub's parser hits EOF where it wants a field, and the whole poll failsSingle line calls like
gh pr view --json ...are fine which is what makes this hard to spot - PR status half works, so it shows as flaky instead of broken.The bare name shell routing only exists so
PATHEXTresolution findsgh.exe.libuvdoes that itself, so skipping the shell when an argument spans lines costs nothing.Goals
gh api graphqlreaches the CLI intact on Windows, and the same for theglab/tearunners that sharecreateForgeCliRunnerNon-goals
.cmdor.batlaunch - they can't run without the shell, so they keep it, and a multi line argument to one of those stays unsupportedshell:from a callerrunGitCommand, as it already passesshell: false, so it was never affected. I checked because I assumed multi line commit messages were broken too and they aren'tgh/glab/teaonly and leaves the trap in place for whoever passes a multiline argument nextQA
IN PROGRESS
I'll undraft the PR when this section is ready 🙂
Checklist
npm run typecheckpassesnpm run lintpassesnpm run formatpasses