fix(cli): keep a rejected --concurrency value off stdout - #201
Open
ethanj wants to merge 1 commit into
Open
Conversation
`quickstart --json` promises a single parseable envelope on stdout. It did not deliver one when `--concurrency` was invalid: the value is parsed while the CLI assembles the command's arguments, so the rejection was written before `quickstartCommand` could enable quiet mode, landing ahead of the envelope. The command still exited 0, so an automated caller read a successful-looking stream that failed to parse. Quiet mode could never have suppressed this. Argument expressions are evaluated before the callee's body runs, so nothing the command does on entry is early enough. The channel is the problem, not the timing: stdout is the data channel, and a diagnostic about invalid input belongs on stderr. `output.note` already serves exactly that purpose and has eighteen callers. Routing it there fixes every command that takes the flag, not just quickstart, and keeps the warning visible rather than trading a corrupt envelope for a silent one. The existing unit test asserted the rejection appeared on stdout, so it encoded the defect and had to change. It now pins both halves of the contract: stderr carries the message, stdout stays empty.
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.
Fixes #191, reported by @graysoncooper with a diagnosis that pointed straight at the cause.
What
quickstart --jsonpromises a single parseable envelope on stdout. It did not deliver one when--concurrencywas invalid:Exit code is 0, so an automated caller sees success and then fails to parse.
Why it happened
The value is parsed while the CLI assembles the command's arguments, at
cli.ts:389, and quiet mode is enabled later inside the command body atquickstart.ts:147. Argument expressions are evaluated before the callee runs, so quiet mode was never early enough to suppress it. Moving thesetQuietcall earlier cannot fix this, because there is no point inside the command that precedes its own arguments.The channel was the real problem. stdout is the data channel, and a complaint about invalid input belongs on stderr.
output.notealready exists for exactly that and has eighteen callers, so the fix is to use it.That also fixes
compile,refreshandwatch, which take the same flag, and it keeps the warning visible rather than trading a corrupt envelope for a silent one.Tests
test/quickstart-json-stdout.test.tsdrives the real CLI throughdist/and asserts both halves of the contract: stdout parses whole, and stderr still names the rejected value. No credentials needed, since the provider failure is reported inside the envelope, which is the shape a caller has to read anyway.Both tests were mutation-tested against the committed fix. Restoring the stdout write turns both red, and deleting the diagnostic altogether also turns both red, so a fix that simply silenced the warning would not pass.
The two other warnings in
concurrency.tsare left alone deliberately: they run inside the command, where quiet mode governs them correctly.