From 8c2726dfcd23c0b5000b6e76074407bda3a644ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Tue, 27 Oct 2026 03:31:09 -0300 Subject: [PATCH] feat: capture lossless native process results --- .changelog/lossless-process-result.md | 7 +++ docs/architecture.md | 18 ++++++ src/Process/NativeProcessRunner.php | 11 +++- src/Process/ProcessResult.php | 16 +++++ src/Process/ResultProcessRunner.php | 21 +++++++ tests/Process/NativeProcessRunnerTest.php | 77 ++++++++++++++++++++++- 6 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 .changelog/lossless-process-result.md create mode 100644 src/Process/ProcessResult.php create mode 100644 src/Process/ResultProcessRunner.php diff --git a/.changelog/lossless-process-result.md b/.changelog/lossless-process-result.md new file mode 100644 index 0000000..1844612 --- /dev/null +++ b/.changelog/lossless-process-result.md @@ -0,0 +1,7 @@ +--- +category: added +type: minor +--- + +Expose optional lossless process capture with exact native exit codes and separate +stdout/stderr bytes while preserving existing transport and provider contracts. diff --git a/docs/architecture.md b/docs/architecture.md index f5f86d6..c55bdee 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -114,6 +114,24 @@ apply, including idempotent repeated boolean flags and both `--option=value` and `--option value`. Behavioral tests exercise the default preset, real rule overrides, help outside a checkout and invalid invocations. +### Optional lossless process capture + +`ResultProcessRunner` extends the existing process interface with `captureResult()`. +The native transport returns an immutable `ProcessResult` containing the exact +exit code and separate stdout/stderr bytes, including non-UTF-8 and ANSI bytes. +Nonzero exits remain results; startup, timeout and invalid-bound failures retain +their native exceptions. Capture uses the same credential filtering, explicit +working directory, argument vector and finite timeout policy, without invoking +the streaming output sink. + +Existing `ProcessRunner` implementations and provider overrides need no new +method. Existing `capture()` still throws on nonzero exits, and `run()` retains +its streaming behavior. The service provider and command output are unchanged. +This library capability does not parse tool output, infer a clean outcome or +expose CLI JSON. Callers must preserve unsupported output rather than extract a +JSON suffix, merge the two channels or treat empty diagnostics as success. +Check, fix and semantic-operation authority remain with their existing runners. + ## Implementation quality Keep methods focused on one responsibility and use early returns for rejected diff --git a/src/Process/NativeProcessRunner.php b/src/Process/NativeProcessRunner.php index d987717..0479349 100644 --- a/src/Process/NativeProcessRunner.php +++ b/src/Process/NativeProcessRunner.php @@ -8,7 +8,7 @@ use Symfony\Component\Process\Process; /** Native transport; callers own selection and operation policy. */ -final class NativeProcessRunner implements ProcessRunner +final class NativeProcessRunner implements ResultProcessRunner { /** Both captured and streamed configuration subprocesses receive the same credential policy. */ private const array ENVIRONMENT = ['GH_TOKEN' => false, 'GITHUB_TOKEN' => false, 'COMPOSER_AUTH' => false]; @@ -33,6 +33,15 @@ public function run(array $arguments, string $directory, float $timeout = 300): return $process->run($this->write); } + /** Captures both channels without invoking the streaming sink or interpreting native failure statuses. */ + public function captureResult(array $arguments, string $directory, float $timeout = 300): ProcessResult + { + $process = $this->createProcess($arguments, $directory, $timeout); + $status = $process->run(); + + return new ProcessResult($status, $process->getOutput(), $process->getErrorOutput()); + } + /** @param list $arguments Positive finite bounds never become Symfony's zero/null unlimited mode. */ private function createProcess(array $arguments, string $directory, float $timeout): Process { diff --git a/src/Process/ProcessResult.php b/src/Process/ProcessResult.php new file mode 100644 index 0000000..9d4c06f --- /dev/null +++ b/src/Process/ProcessResult.php @@ -0,0 +1,16 @@ + $arguments Literal executable and argument vector, without shell interpolation. + * @param string $directory Explicit child working directory; the caller's directory remains unchanged. + * @param positive-float $timeout Finite maximum seconds using the same transport bounds as run(). + * @return ProcessResult Exact native status and separate output bytes, without a clean/failure interpretation. + * @throws \InvalidArgumentException When the timeout is nonfinite or not positive. + * @throws \Symfony\Component\Process\Exception\RuntimeException When execution cannot complete, including startup or timeout failure. + */ + public function captureResult(array $arguments, string $directory, float $timeout = 300): ProcessResult; +} diff --git a/tests/Process/NativeProcessRunnerTest.php b/tests/Process/NativeProcessRunnerTest.php index 5aa9371..e688198 100644 --- a/tests/Process/NativeProcessRunnerTest.php +++ b/tests/Process/NativeProcessRunnerTest.php @@ -5,6 +5,7 @@ namespace FastForward\CodeStandards\Tests\Process; use FastForward\CodeStandards\Process\NativeProcessRunner; +use FastForward\CodeStandards\Process\ResultProcessRunner; use InvalidArgumentException; use PHPUnit\Framework\TestCase; use Symfony\Component\Process\Exception\ProcessTimedOutException; @@ -16,7 +17,7 @@ final class NativeProcessRunnerTest extends TestCase public function testFiniteTimeoutsApplyToCapturedAndStreamedProcesses(): void { $process = new NativeProcessRunner(); - foreach (['capture', 'run'] as $method) { + foreach (['capture', 'run', 'captureResult'] as $method) { try { $process->{$method}([PHP_BINARY, '-r', 'usleep(250000);'], sys_get_temp_dir(), 0.01); self::fail('A controlled long subprocess must time out.'); @@ -26,6 +27,7 @@ public function testFiniteTimeoutsApplyToCapturedAndStreamedProcesses(): void } $arguments = [PHP_BINARY, '-r', 'usleep(50000); echo "completed";']; self::assertSame('completed', $process->capture($arguments, sys_get_temp_dir(), 2)); + self::assertSame('completed', $process->captureResult($arguments, sys_get_temp_dir(), 2)->stdout); $output = ''; $streamed = new NativeProcessRunner(static function (string $type, string $bytes) use (&$output): void { $output .= $bytes; @@ -37,7 +39,7 @@ public function testFiniteTimeoutsApplyToCapturedAndStreamedProcesses(): void /** Zero, negative and nonfinite values cannot silently become Symfony's unlimited mode. */ public function testInvalidBoundsFailBeforeLaunchingEitherChannel(): void { - foreach (['capture', 'run'] as $method) { + foreach (['capture', 'run', 'captureResult'] as $method) { foreach ([0.0, -0.1, INF, -INF, NAN] as $timeout) { try { (new NativeProcessRunner())->{$method}([PHP_BINARY, '-r', 'echo "unexpected";'], sys_get_temp_dir(), $timeout); @@ -48,4 +50,75 @@ public function testInvalidBoundsFailBeforeLaunchingEitherChannel(): void } } } + + /** Binary bytes and nonzero statuses remain separate data and never reach the streaming output sink. */ + public function testResultCaptureRetainsExactBytesAndNativeStatuses(): void + { + $writes = []; + $runner = new NativeProcessRunner(static function (string $type, string $bytes) use (&$writes): void { + $writes[] = [$type, $bytes]; + }); + self::assertInstanceOf(ResultProcessRunner::class, $runner); + $source = <<<'PHP' + fwrite(STDOUT, "out\0\xFF\033[31m\n"); + fwrite(STDERR, "err\0\xFE\033[0m\n"); + exit((int) $argv[1]); + PHP; + + foreach ([0, 2, 255] as $status) { + $result = $runner->captureResult([PHP_BINARY, '-r', $source, (string) $status], sys_get_temp_dir()); + self::assertSame($status, $result->exitCode); + self::assertSame("out\0\xFF\033[31m\n", $result->stdout); + self::assertSame("err\0\xFE\033[0m\n", $result->stderr); + } + + self::assertSame([], $writes); + } + + /** Captured execution preserves the selected cwd/argv and the existing credential-removal policy. */ + public function testResultCaptureUsesTheExistingProcessBoundary(): void + { + $before = getcwd(); + $directory = realpath(sys_get_temp_dir()); + self::assertNotFalse($directory); + $previous = []; + foreach (['GH_TOKEN', 'GITHUB_TOKEN', 'COMPOSER_AUTH'] as $name) { + $previous[$name] = getenv($name); + putenv($name . '=synthetic-test-value'); + } + + try { + $result = (new NativeProcessRunner())->captureResult([ + PHP_BINARY, '-r', + 'echo getcwd(), "\n", $argv[1]; foreach (["GH_TOKEN", "GITHUB_TOKEN", "COMPOSER_AUTH"] as $name) { if (false !== getenv($name)) { exit(19); } }', + 'literal argument with spaces', + ], $directory); + } finally { + foreach ($previous as $name => $value) { + putenv(false === $value ? $name : $name . '=' . $value); + } + } + + self::assertSame(0, $result->exitCode); + self::assertSame($directory . "\nliteral argument with spaces", $result->stdout); + self::assertSame('', $result->stderr); + self::assertSame($before, getcwd()); + } + + /** Native startup errors propagate instead of becoming an invented successful process result. */ + public function testResultCaptureDoesNotInventAResultForStartupFailure(): void + { + $this->expectException(\Symfony\Component\Process\Exception\RuntimeException::class); + (new NativeProcessRunner())->captureResult( + [PHP_BINARY, '-r', 'echo "unexpected";'], + sys_get_temp_dir() . '/code-standards-absent-' . bin2hex(random_bytes(16)), + ); + } + + /** Existing captured execution still throws on a completed nonzero process instead of changing its API. */ + public function testLegacyCaptureRetainsItsFailureContract(): void + { + $this->expectException(\Symfony\Component\Process\Exception\ProcessFailedException::class); + (new NativeProcessRunner())->capture([PHP_BINARY, '-r', 'exit(2);'], sys_get_temp_dir()); + } }