From 0f4e2a697e9f3a60e7e8b81730d8548e4ddf5ab9 Mon Sep 17 00:00:00 2001 From: Marcel Reuss Date: Thu, 20 Aug 2026 09:22:06 +0200 Subject: [PATCH] fix: keep diagnostics off stdout Reported from real use: a task that reads `sputnik completion bash` to compare it against the installed file got a discovery warning as line one of the script. $ sputnik completion bash | head -2 Skipped task 'list' in .../ProbeListTask.php: the name is reserved ... # This file is part of the Symfony package. The warning would have been written into the completion file. --silent and -q do not help - they suppress the script along with the warning, because both were on the same stream. Diagnostics now go to stderr, where the redacting decorator already masks secrets. Looking into it turned up the same class of bug in a place nobody had tried: `list --format=json` was never parseable. Three separate things landed on stdout around the JSON - the discovery warning, our header before it, and the "Available tasks" section after it. So for any list format other than the default txt, and for --raw, the decoration is left out entirely: those exist to be read by something else. completion bash stdout: # This file is part of the Symfony package. stderr: Skipped task 'list' in ... list --format=json valid JSON, 10 commands list --raw the raw command list, nothing around it list header and grouped task section, unchanged The last line has its own test, because moving decoration behind a condition is how you lose it. --- docs/cli.md | 16 ++++++++ src/Console/Application.php | 26 ++++++++++--- tests/E2E/SputnikBinaryTest.php | 68 +++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 6 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index fb109cf..5c07d44 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -44,6 +44,16 @@ Values are automatically coerced: `true`/`false` to bool, numeric strings to int Available on both direct task commands (`sputnik deploy -D ...`) and the run command (`sputnik run deploy -D ...`). +### `--format` on `list` + +`list` takes Symfony's `--format` (`txt`, `xml`, `json`, `md`) and `--raw`. For +anything other than the default `txt`, Sputnik leaves the output alone: no +header, no grouped task section, so the result is exactly what a parser expects. + +```bash +sputnik list --format=json | jq '.commands[].name' +``` + ### `-v` / `--verbose` Show additional output including log messages and stack traces on errors. @@ -152,6 +162,12 @@ built-in command - rename the task or give it a group prefix The warning appears on every run, not only the one that filled the container cache -- the task stays missing until someone renames it. +It is written to **stderr**, along with every other diagnostic Sputnik emits +about itself. That keeps stdout usable as data: `sputnik completion bash > file` +writes only the script, and `sputnik list --format=json` parses. Redirect stderr +if you want it gone -- `--silent` and `-q` are the wrong tool, they suppress the +payload with it. + `init` is different: a project task may take it, and then the built-in scaffold is no longer reachable. Scaffolding a project happens once, while a project command called `init` may well be a daily one, so the project wins. diff --git a/src/Console/Application.php b/src/Console/Application.php index 25c2179..a19aa1f 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -8,6 +8,7 @@ use Sputnik\Task\TaskMetadata; use Symfony\Component\Console\Application as BaseApplication; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; final class Application extends BaseApplication @@ -70,20 +71,33 @@ public function doRun(InputInterface $input, OutputInterface $output): int $commandName = $this->getCommandName($input); $isList = $commandName === null || $commandName === 'list'; - // Reported here, not while the application is assembled: -v is parsed by - // run(), so verbosity is not known any earlier. + // Anything else is a format meant for a machine - json, xml, md - or the + // raw list used to embed a command runner. Decoration would land in the + // middle of it. + $isReadableList = $isList + && $input->getParameterOption('--format', 'txt') === 'txt' + && !$input->hasParameterOption('--raw'); + + // Diagnostics go to stderr: `completion bash > file` and + // `list --format=json` both put stdout into something that parses it, and + // --silent cannot separate the two - it removes the payload as well. + // + // Reported here rather than while the application is assembled, because + // -v is parsed by run() and verbosity is not known any earlier. + $diagnostics = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output; + foreach ($this->discoveryWarnings as $warning) { - $output->writeln('' . $warning . ''); + $diagnostics->writeln('' . $warning . ''); } if ($output->isVerbose()) { foreach ($this->discoveryNotices as $notice) { - $output->writeln('' . $notice . ''); + $diagnostics->writeln('' . $notice . ''); } } // Show our header instead of Symfony's "AppName version" for list - if ($isList) { + if ($isReadableList) { $output->writeln(\sprintf( "\xF0\x9F\x9B\xB0 Sputnik %s │ %s │ %s", self::VERSION, @@ -95,7 +109,7 @@ public function doRun(InputInterface $input, OutputInterface $output): int $result = parent::doRun($input, $output); - if ($isList && $this->taskDiscovery instanceof TaskDiscovery) { + if ($isReadableList && $this->taskDiscovery instanceof TaskDiscovery) { $this->renderTaskList($output); } diff --git a/tests/E2E/SputnikBinaryTest.php b/tests/E2E/SputnikBinaryTest.php index f38d7dd..19139a3 100644 --- a/tests/E2E/SputnikBinaryTest.php +++ b/tests/E2E/SputnikBinaryTest.php @@ -874,6 +874,74 @@ public function testAMissingWorkingDirSaysSoInsteadOfFailingOnItsCache(): void $this->assertStringNotContainsString('mkdir', $output, 'The cache is a symptom, not the problem'); } + public function testCompletionScriptIsNotPollutedByDiagnostics(): void + { + // The completion script is written to a file. A warning on stdout landed + // in it as line one, and --silent/-q cannot help: they suppress the + // script along with the warning. + $this->scaffoldProject([ + 'list' => <<<'PHP' + #[Task(name: 'list', description: 'Collides with a built-in')] + final class ListTask implements TaskInterface + { + public function __invoke(TaskContext $ctx): TaskResult + { + return TaskResult::success(); + } + } + PHP, + ]); + + $result = $this->sputnik(['completion', 'bash'], $this->tempDir); + + $this->assertStringStartsWith('#', ltrim($result->getOutput())); + $this->assertStringNotContainsString('Skipped task', $result->getOutput()); + $this->assertStringContainsString('Skipped task', $result->getErrorOutput()); + } + + public function testListInAMachineReadableFormatIsParseable(): void + { + $this->scaffoldProject([ + 'deploy' => <<<'PHP' + #[Task(name: 'deploy', description: 'Deploy it')] + final class DeployTask implements TaskInterface + { + public function __invoke(TaskContext $ctx): TaskResult + { + return TaskResult::success(); + } + } + PHP, + ]); + + $result = $this->sputnik(['list', '--format=json'], $this->tempDir); + + $this->assertSame(0, $result->getExitCode()); + $this->assertIsArray(json_decode($result->getOutput(), true), 'The header and the task section used to surround the JSON'); + } + + public function testTheDefaultListKeepsItsHeaderAndTaskSection(): void + { + $this->scaffoldProject([ + 'deploy' => <<<'PHP' + #[Task(name: 'deploy', description: 'Deploy it')] + final class DeployTask implements TaskInterface + { + public function __invoke(TaskContext $ctx): TaskResult + { + return TaskResult::success(); + } + } + PHP, + ]); + + $output = $this->sputnik(['list'], $this->tempDir)->getOutput(); + + $this->assertStringContainsString('Sputnik', $output); + $this->assertStringContainsString('Available tasks', $output); + $this->assertStringContainsString('deploy', $output); + } + private function sputnik(array $args, ?string $cwd = null): Process { $process = new Process(