From 4cc48cbab863fd5fb26f2e515723a253ec230ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Say=C3=A3o=20Lobato=20Abreu?= Date: Wed, 21 Oct 2026 14:04:36 -0300 Subject: [PATCH] fix: keep missing sync targets read-only in preview modes --- .changelog/read-only-missing-sync-targets.md | 6 ++ docs/commands/copy-resource.rst | 9 +++ docs/commands/git-hooks.rst | 9 +++ src/Console/Command/CopyResourceCommand.php | 14 ++++ src/Console/Command/GitHooksCommand.php | 15 ++++ .../Command/CopyResourceCommandTest.php | 69 ++++++++++++++++++- tests/Console/Command/GitHooksCommandTest.php | 43 +++++++++++- tests/Console/Command/SyncCommandTest.php | 34 ++++++++- 8 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 .changelog/read-only-missing-sync-targets.md diff --git a/.changelog/read-only-missing-sync-targets.md b/.changelog/read-only-missing-sync-targets.md new file mode 100644 index 0000000000..be644c66d2 --- /dev/null +++ b/.changelog/read-only-missing-sync-targets.md @@ -0,0 +1,6 @@ +--- +category: fixed +type: patch +--- + +Keep resource and Git hook synchronization dry-run/check modes read-only when destinations are missing, reporting planned creations and failing checks without creating directories, copying files, rendering installed hooks or changing permissions. diff --git a/docs/commands/copy-resource.rst b/docs/commands/copy-resource.rst index e214100bf3..4f9f0f5a9d 100644 --- a/docs/commands/copy-resource.rst +++ b/docs/commands/copy-resource.rst @@ -90,3 +90,12 @@ Exit Codes - Success. All resources were copied or skipped. * - 1 - Failure. Missing required options or copy error. + +Preview safety +-------------- + +Missing targets are reported as planned creations. ``--dry-run`` returns success +and ``--check`` returns failure without creating files or parent directories, +including when overwrite or interactive options are also supplied. When both +preview flags are supplied, ``--check`` determines the failure status. Existing +targets are compared without replacement or permission changes. diff --git a/docs/commands/git-hooks.rst b/docs/commands/git-hooks.rst index c9fe4e269e..5982b182dc 100644 --- a/docs/commands/git-hooks.rst +++ b/docs/commands/git-hooks.rst @@ -86,3 +86,12 @@ Exit Codes * - 1 - Failure. Drift detected in ``--check`` mode or one or more hooks could not be rewritten automatically. + +Preview safety +-------------- + +Missing targets are reported as planned creations. ``--dry-run`` returns success +and ``--check`` returns failure without creating files or parent directories, +including when overwrite or interactive options are also supplied. When both +preview flags are supplied, ``--check`` determines the failure status. Existing +targets are compared without replacement or permission changes. diff --git a/src/Console/Command/CopyResourceCommand.php b/src/Console/Command/CopyResourceCommand.php index 26a32c821c..51637e30b5 100644 --- a/src/Console/Command/CopyResourceCommand.php +++ b/src/Console/Command/CopyResourceCommand.php @@ -228,6 +228,20 @@ private function copyFile( InputInterface $input, OutputInterface $output, ): int { + if (($dryRun || $check) && ! $this->filesystem->exists($targetPath)) { + $this->log( + 'Resource {target_path} is missing and would be created.', + $input, + [ + 'source_path' => $sourcePath, + 'target_path' => $targetPath, + ], + LogLevel::NOTICE, + ); + + return $check ? self::FAILURE : self::SUCCESS; + } + if (! $overwrite && ! $dryRun && ! $check && ! $interactive && $this->filesystem->exists($targetPath)) { return $this->success( 'Skipped existing resource {target_path}.', diff --git a/src/Console/Command/GitHooksCommand.php b/src/Console/Command/GitHooksCommand.php index aa903c8f25..d83004e5eb 100644 --- a/src/Console/Command/GitHooksCommand.php +++ b/src/Console/Command/GitHooksCommand.php @@ -149,6 +149,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int $renderedSourceContents = $this->hookContentRenderer->render($sourceContents, $projectPath); $hookPath = Path::join($targetPath, $file->getRelativePathname()); + if (($dryRun || $check) && ! $this->filesystem->exists($hookPath)) { + $this->log( + 'Git hook {hook_path} is missing and would be installed.', + $input, + [ + 'hook_name' => $file->getFilename(), + 'hook_path' => $hookPath, + ], + LogLevel::NOTICE, + ); + $checkFailure = $checkFailure || $check; + + continue; + } + if (! $overwrite && ! $dryRun && ! $check && ! $interactive && $this->filesystem->exists($hookPath)) { $this->log( 'Skipped existing {hook_name} hook.', diff --git a/tests/Console/Command/CopyResourceCommandTest.php b/tests/Console/Command/CopyResourceCommandTest.php index 1e86200e76..e90272e6d1 100644 --- a/tests/Console/Command/CopyResourceCommandTest.php +++ b/tests/Console/Command/CopyResourceCommandTest.php @@ -33,6 +33,7 @@ use FastForward\DevTools\Path\DevToolsPathResolver; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; use Prophecy\Argument; @@ -468,9 +469,71 @@ public function executeWillSkipReplacingDriftedFileWhenInteractiveConfirmationIs self::assertSame(CopyResourceCommand::SUCCESS, $this->executeCommand()); } - /** - * @return int - */ + #[Test] + #[DataProvider('missingTargetPreviewModes')] + public function executeWillNotCreateMissingTargetsInPreviewModes(bool $directory, bool $dryRun, bool $check): void + { + $source = $directory ? $this->sourceDirectory : $this->sourceDirectory . '/nested/example.yml'; + $target = $this->sourceDirectory . '/missing/output'; + $destination = $directory ? $target . '/nested/example.yml' : $target; + $this->input->getOption('source')->willReturn($source); + $this->input->getOption('target')->willReturn($target); + $this->input->getOption('overwrite')->willReturn(true); + $this->input->getOption('interactive')->willReturn(true); + $this->input->getOption('dry-run')->willReturn($dryRun); + $this->input->getOption('check')->willReturn($check); + $this->fileLocator->locate($source)->willReturn($source); + $this->filesystem->getAbsolutePath($target)->willReturn($target); + $this->filesystem->exists($destination)->willReturn(false); + $this->finderFactory->create()->willReturn(new Finder()); + $this->filesystem->copy(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->mkdir(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->dumpFile(Argument::cetera())->shouldNotBeCalled(); + $this->io->askQuestion(Argument::cetera())->shouldNotBeCalled(); + $this->logger->log('notice', 'Resource {target_path} is missing and would be created.', Argument::type('array')) + ->shouldBeCalledOnce(); + + self::assertSame($check ? CopyResourceCommand::FAILURE : CopyResourceCommand::SUCCESS, $this->executeCommand()); + self::assertDirectoryDoesNotExist($this->sourceDirectory . '/missing'); + } + + #[Test] + public function executeWillCheckAllFilesWithoutWritingWhenDirectoryContainsExistingAndMissingTargets(): void + { + $extraSource = $this->sourceDirectory . '/nested/second.yml'; + file_put_contents($extraSource, 'second'); + try { + $target = $this->sourceDirectory . '/missing'; + $this->input->getOption('source')->willReturn($this->sourceDirectory); + $this->input->getOption('target')->willReturn($target); + $this->input->getOption('overwrite')->willReturn(false); + $this->input->getOption('check')->willReturn(true); + $this->fileLocator->locate($this->sourceDirectory)->willReturn($this->sourceDirectory); + $this->filesystem->getAbsolutePath($target)->willReturn($target); + $this->filesystem->exists($target . '/nested/example.yml')->willReturn(true); + $this->filesystem->exists($target . '/nested/second.yml')->willReturn(false); + $this->finderFactory->create()->willReturn(new Finder()); + $this->fileDiffer->diff($this->sourceDirectory . '/nested/example.yml', $target . '/nested/example.yml') + ->willReturn(new FileDiff(FileDiff::STATUS_UNCHANGED, 'Unchanged'))->shouldBeCalledOnce(); + $this->filesystem->copy(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->mkdir(Argument::cetera())->shouldNotBeCalled(); + + self::assertSame(CopyResourceCommand::FAILURE, $this->executeCommand()); + self::assertDirectoryDoesNotExist($target); + } finally { + unlink($extraSource); + } + } + + public static function missingTargetPreviewModes(): iterable + { + foreach ([false, true] as $directory) { + yield [$directory, true, false]; + yield [$directory, false, true]; + yield [$directory, true, true]; + } + } + private function executeCommand(): int { $reflectionMethod = new ReflectionMethod($this->command, 'execute'); diff --git a/tests/Console/Command/GitHooksCommandTest.php b/tests/Console/Command/GitHooksCommandTest.php index 25757d5ae3..93d69b5be5 100644 --- a/tests/Console/Command/GitHooksCommandTest.php +++ b/tests/Console/Command/GitHooksCommandTest.php @@ -34,6 +34,7 @@ use FastForward\DevTools\Environment\RuntimeEnvironment; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesTrait; use PHPUnit\Framework\TestCase; @@ -510,9 +511,45 @@ public function executeWillReportInstallFailureWhenReplacementStillCannotBeWritt self::assertSame(GitHooksCommand::FAILURE, $this->executeCommand()); } - /** - * @return int - */ + #[Test] + #[DataProvider('missingHookPreviewModes')] + public function executeWillNotInstallMissingHooksInPreviewModes(bool $rendered, bool $dryRun, bool $check): void + { + $target = $this->sourceDirectory . '/missing/hooks'; + $this->input->getOption('source')->willReturn($this->sourceDirectory); + $this->input->getOption('target')->willReturn($target); + $this->input->getOption('no-overwrite')->willReturn(false); + $this->input->getOption('interactive')->willReturn(true); + $this->input->getOption('dry-run')->willReturn($dryRun); + $this->input->getOption('check')->willReturn($check); + $this->fileLocator->locate($this->sourceDirectory)->willReturn($this->sourceDirectory); + $this->filesystem->getAbsolutePath($target)->willReturn($target); + $this->filesystem->exists($target . '/post-merge')->willReturn(false); + $this->filesystem->readFile($this->sourceDirectory . '/post-merge') + ->willReturn($rendered ? '#!/bin/sh\n' . HookContentRenderer::MANAGED_GRUMPHP_CONFIG_PLACEHOLDER : '#!/bin/sh'); + $this->finderFactory->create()->willReturn(new Finder()); + $this->filesystem->copy(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->dumpFile(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->chmod(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->remove(Argument::cetera())->shouldNotBeCalled(); + $this->filesystem->mkdir(Argument::cetera())->shouldNotBeCalled(); + $this->io->askQuestion(Argument::cetera())->shouldNotBeCalled(); + $this->logger->log('notice', 'Git hook {hook_path} is missing and would be installed.', Argument::type('array')) + ->shouldBeCalledOnce(); + + self::assertSame($check ? GitHooksCommand::FAILURE : GitHooksCommand::SUCCESS, $this->executeCommand()); + self::assertDirectoryDoesNotExist($this->sourceDirectory . '/missing'); + } + + public static function missingHookPreviewModes(): iterable + { + foreach ([false, true] as $rendered) { + yield [$rendered, true, false]; + yield [$rendered, false, true]; + yield [$rendered, true, true]; + } + } + private function executeCommand(): int { $reflectionMethod = new ReflectionMethod($this->command, 'execute'); diff --git a/tests/Console/Command/SyncCommandTest.php b/tests/Console/Command/SyncCommandTest.php index 03ecd3e829..46d69f9e51 100644 --- a/tests/Console/Command/SyncCommandTest.php +++ b/tests/Console/Command/SyncCommandTest.php @@ -30,6 +30,7 @@ use FastForward\DevTools\Environment\RuntimeEnvironment; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesTrait; use PHPUnit\Framework\TestCase; @@ -192,9 +193,36 @@ public function executeWillPropagateJsonOutputFormatToSubCommands(): void self::assertSame(SyncCommand::SUCCESS, $this->executeCommand()); } - /** - * @return int - */ + #[Test] + #[DataProvider('readOnlyModes')] + public function executeWillForwardReadOnlyModeToEveryQueuedCommand(string $mode, int $status): void + { + $this->input->getOption($mode)->willReturn(true); + $commandLines = []; + $this->processQueue->add(Argument::type(Process::class), false, false, Argument::cetera()) + ->will(static function (array $arguments) use (&$commandLines): void { + $commandLines[] = $arguments[0]->getCommandLine(); + })->shouldBeCalledTimes(10); + $this->processQueue->add(Argument::type(Process::class), false, true, Argument::cetera())->shouldNotBeCalled(); + $this->processQueue->run($this->output->reveal())->willReturn($status); + + self::assertSame($status, $this->executeCommand()); + foreach ($commandLines as $commandLine) { + self::assertStringContainsString('--' . $mode, $commandLine); + self::assertStringNotContainsString("'wiki'", $commandLine); + self::assertStringNotContainsString("'skills'", $commandLine); + self::assertStringNotContainsString("'agents'", $commandLine); + } + self::assertCount(3, array_filter($commandLines, static fn(string $line): bool => str_contains($line, "'copy-resource'"))); + self::assertCount(1, array_filter($commandLines, static fn(string $line): bool => str_contains($line, "'git-hooks'"))); + } + + public static function readOnlyModes(): iterable + { + yield 'dry-run' => ['dry-run', SyncCommand::SUCCESS]; + yield 'check' => ['check', SyncCommand::FAILURE]; + } + private function executeCommand(): int { return (new ReflectionMethod($this->command, 'execute'))