diff --git a/.github/wiki b/.github/wiki index 0e0484634..546706f28 160000 --- a/.github/wiki +++ b/.github/wiki @@ -1 +1 @@ -Subproject commit 0e0484634c2fe11cdb158f61755db93295e31f5d +Subproject commit 546706f28694f18150a1aec0622da9be7f4ee710 diff --git a/CHANGELOG.md b/CHANGELOG.md index 68bcfe463..872df1fa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Keep packaged `.agents` payloads exportable and synchronize packaged skills and agents with repository-relative symlink targets so consumer repositories no longer receive broken absolute machine paths (#188) +- Rewrite drifted Git hooks by removing the previous target first, restore the intended `0o755` executable mode, and report unwritable hook replacements cleanly when `.git/hooks` stays locked (#190) ## [1.20.0] - 2026-04-23 diff --git a/docs/commands/git-hooks.rst b/docs/commands/git-hooks.rst index 5c98896b6..c9fe4e269 100644 --- a/docs/commands/git-hooks.rst +++ b/docs/commands/git-hooks.rst @@ -11,6 +11,8 @@ The ``git-hooks`` command installs the hook templates maintained in 1. Copies hook files from a source directory to the target hooks directory 2. Sets executable permissions on copied hooks +3. Replaces drifted hooks defensively by removing the previous target before + recopying it Usage ----- @@ -44,6 +46,11 @@ Options ``--interactive`` Prompt before replacing a drifted Git hook. +When a hook still cannot be rewritten because the target remains locked or +unwritable, the command logs a clear error for that hook, continues processing +the remaining hooks, and exits non-zero so ``dev-tools:sync`` reports the hook +install problem clearly instead of aborting mid-copy. + ``--json`` Emit a structured machine-readable payload instead of the normal terminal output. @@ -77,4 +84,5 @@ Exit Codes * - 0 - Success. Hooks installed successfully. * - 1 - - Failure. Copy error. + - Failure. Drift detected in ``--check`` mode or one or more hooks could + not be rewritten automatically. diff --git a/docs/running/specialized-commands.rst b/docs/running/specialized-commands.rst index b2fc76c17..7807ffbf1 100644 --- a/docs/running/specialized-commands.rst +++ b/docs/running/specialized-commands.rst @@ -461,7 +461,9 @@ Installs packaged Fast Forward Git hooks. Important details: - copies hook files from source to target directory; -- sets executable permissions on copied hooks; +- sets executable permissions on copied hooks with ``0o755``; +- removes an existing drifted hook before recopying it so stale target + permissions do not block replacements; - ``--source`` defaults to ``resources/git-hooks``; - ``--target`` defaults to ``.git/hooks``; - ``--no-overwrite`` preserves existing hook files. diff --git a/src/Console/Command/GitHooksCommand.php b/src/Console/Command/GitHooksCommand.php index 8c3888ab7..cdd8ade55 100644 --- a/src/Console/Command/GitHooksCommand.php +++ b/src/Console/Command/GitHooksCommand.php @@ -31,6 +31,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Filesystem\Exception\IOExceptionInterface; use Symfony\Component\Filesystem\Path; /** @@ -129,7 +130,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int ->files() ->in($sourcePath); - $status = self::SUCCESS; + $checkFailure = false; + $installFailure = false; foreach ($files as $file) { $hookPath = Path::join($targetPath, $file->getRelativePathname()); @@ -180,7 +182,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } if ($check) { - $status = self::FAILURE; + $checkFailure = true; continue; } @@ -203,8 +205,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } } - $this->filesystem->copy($file->getRealPath(), $hookPath, $overwrite || $interactive); - $this->filesystem->chmod($hookPath, 755, 0o755); + if (! $this->installHook($file->getRealPath(), $hookPath, $overwrite || $interactive, $input)) { + $installFailure = true; + + continue; + } $this->success( 'Installed {hook_name} hook.', @@ -216,7 +221,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } - if (self::FAILURE === $status) { + if ($checkFailure) { return $this->failure( 'One or more Git hooks require synchronization updates.', $input, @@ -227,6 +232,17 @@ protected function execute(InputInterface $input, OutputInterface $output): int ); } + if ($installFailure) { + return $this->failure( + 'One or more Git hooks could not be installed automatically.', + $input, + [ + 'target' => $targetPath, + ], + $targetPath, + ); + } + return $this->success( 'Git hook synchronization completed successfully.', $input, @@ -248,4 +264,42 @@ private function shouldReplaceHook(string $hookPath): bool return $this->getIO() ->askConfirmation(\sprintf('Replace drifted Git hook %s? [y/N] ', $hookPath), false); } + + /** + * Installs a single hook and rewrites drifted targets defensively. + * + * @param string $sourcePath the packaged hook path + * @param string $hookPath the target repository hook path + * @param bool $replaceExisting whether an existing hook SHOULD be removed first + * @param InputInterface $input the originating command input + * + * @return bool true when the hook was installed successfully + */ + private function installHook(string $sourcePath, string $hookPath, bool $replaceExisting, InputInterface $input): bool + { + try { + if ($replaceExisting && $this->filesystem->exists($hookPath)) { + $this->filesystem->remove($hookPath); + } + + $this->filesystem->copy($sourcePath, $hookPath, false); + $this->filesystem->chmod(files: $hookPath, mode: 0o755); + + return true; + } catch (IOExceptionInterface $exception) { + $this->logger->error( + 'Failed to install {hook_name} hook automatically. Remove or unlock {hook_path} and rerun git-hooks.', + [ + 'input' => $input, + 'hook_name' => $this->filesystem->basename($hookPath), + 'hook_path' => $hookPath, + 'error' => $exception->getMessage(), + 'file' => $exception->getPath() ?? $hookPath, + 'line' => null, + ], + ); + + return false; + } + } } diff --git a/tests/Console/Command/GitHooksCommandTest.php b/tests/Console/Command/GitHooksCommandTest.php index 2081b9f6a..4343b32a9 100644 --- a/tests/Console/Command/GitHooksCommandTest.php +++ b/tests/Console/Command/GitHooksCommandTest.php @@ -39,6 +39,7 @@ use Symfony\Component\Config\FileLocatorInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Filesystem\Exception\IOException; use Symfony\Component\Finder\Finder; use function Safe\mkdir; @@ -164,9 +165,9 @@ public function executeWillCopyPackagedHooks(): void ->willReturn('/app/.git/hooks'); $this->filesystem->exists('/app/.git/hooks/post-merge') ->willReturn(false); - $this->filesystem->copy(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge', true) + $this->filesystem->copy(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge', false) ->shouldBeCalledOnce(); - $this->filesystem->chmod('/app/.git/hooks/post-merge', 755, 0o755) + $this->filesystem->chmod('/app/.git/hooks/post-merge', 0o755) ->shouldBeCalledOnce(); $this->logger->log('info', 'Installed {hook_name} hook.', Argument::type('array')) ->shouldBeCalledOnce(); @@ -327,6 +328,97 @@ public function executeWillSkipReplacingHookWhenInteractiveConfirmationIsDecline self::assertSame(GitHooksCommand::SUCCESS, $this->executeCommand()); } + /** + * @return void + */ + #[Test] + public function executeWillRemoveDriftedHookBeforeReplacingIt(): void + { + $this->input->getOption('source') + ->willReturn('resources/git-hooks'); + $this->input->getOption('target') + ->willReturn('.git/hooks'); + $this->input->getOption('no-overwrite') + ->willReturn(false); + + $this->fileLocator->locate('resources/git-hooks') + ->willReturn($this->sourceDirectory); + $this->finderFactory->create() + ->willReturn(new Finder()) + ->shouldBeCalledOnce(); + $this->filesystem->getAbsolutePath('.git/hooks') + ->willReturn('/app/.git/hooks'); + $this->filesystem->exists('/app/.git/hooks/post-merge') + ->willReturn(true); + $this->fileDiffer->diff(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge') + ->willReturn(new FileDiff(FileDiff::STATUS_CHANGED, 'Changed summary', null)) + ->shouldBeCalledOnce(); + $this->filesystem->remove('/app/.git/hooks/post-merge') + ->shouldBeCalledOnce(); + $this->filesystem->copy(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge', false) + ->shouldBeCalledOnce(); + $this->filesystem->chmod('/app/.git/hooks/post-merge', 0o755) + ->shouldBeCalledOnce(); + $this->logger->log('info', 'Installed {hook_name} hook.', Argument::type('array')) + ->shouldBeCalledOnce(); + $this->logger->log('info', 'Git hook synchronization completed successfully.', Argument::type('array')) + ->shouldBeCalledOnce(); + + self::assertSame(GitHooksCommand::SUCCESS, $this->executeCommand()); + } + + /** + * @return void + */ + #[Test] + public function executeWillReportInstallFailureWhenReplacementStillCannotBeWritten(): void + { + $this->input->getOption('source') + ->willReturn('resources/git-hooks'); + $this->input->getOption('target') + ->willReturn('.git/hooks'); + $this->input->getOption('no-overwrite') + ->willReturn(false); + + $this->fileLocator->locate('resources/git-hooks') + ->willReturn($this->sourceDirectory); + $this->finderFactory->create() + ->willReturn(new Finder()) + ->shouldBeCalledOnce(); + $this->filesystem->getAbsolutePath('.git/hooks') + ->willReturn('/app/.git/hooks'); + $this->filesystem->exists('/app/.git/hooks/post-merge') + ->willReturn(true); + $this->fileDiffer->diff(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge') + ->willReturn(new FileDiff(FileDiff::STATUS_CHANGED, 'Changed summary', null)) + ->shouldBeCalledOnce(); + $this->filesystem->remove('/app/.git/hooks/post-merge') + ->shouldBeCalledOnce(); + $this->filesystem->copy(Argument::containingString('/post-merge'), '/app/.git/hooks/post-merge', false) + ->willThrow(new IOException('Target file could not be opened for writing.', 0, null, '/app/.git/hooks/post-merge')) + ->shouldBeCalledOnce(); + $this->filesystem->basename('/app/.git/hooks/post-merge') + ->willReturn('post-merge') + ->shouldBeCalledOnce(); + $this->logger->error( + 'Failed to install {hook_name} hook automatically. Remove or unlock {hook_path} and rerun git-hooks.', + Argument::that( + static fn(array $context): bool => $context['input'] instanceof InputInterface + && 'post-merge' === $context['hook_name'] + && '/app/.git/hooks/post-merge' === $context['hook_path'] + && '/app/.git/hooks/post-merge' === $context['file'] + && null === $context['line'] + && str_contains($context['error'], 'Target file could not be opened for writing.') + ), + )->shouldBeCalledOnce(); + $this->logger->error('One or more Git hooks could not be installed automatically.', Argument::type('array')) + ->shouldBeCalledOnce(); + $this->filesystem->chmod(Argument::cetera()) + ->shouldNotBeCalled(); + + self::assertSame(GitHooksCommand::FAILURE, $this->executeCommand()); + } + /** * @return int */