From 22edf5f0e9c21d3b187a50b582ad352343a64130 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Sat, 24 Jan 2026 17:27:34 +0100 Subject: [PATCH] feat(stack): add `hooks` subcommand for status display Add a new `stack hooks` command that displays the status of both git hooks and Claude hooks. The existing `stack setup` command becomes an alias for `stack hooks --setup`. - `mergify stack hooks` shows detailed status of all hooks - `mergify stack hooks --setup` installs/upgrades hooks - `mergify stack hooks --setup --force` forces wrapper reinstall - `mergify stack setup` works as before (backwards compatible) - `mergify stack setup --check` shows status (deprecated in favor of `hooks`) Co-Authored-By: Claude Opus 4.5 Change-Id: I45910d02a514ed45f8b6d0becf58d36dce2a51f5 Claude-Session-Id: 3dabd187-059b-40b7-9a52-a2542a27c752 --- mergify_cli/stack/cli.py | 113 ++++++++++++- mergify_cli/stack/setup.py | 194 +++++++++++++++------- mergify_cli/tests/stack/test_setup.py | 225 ++++++++++++++++++++++++++ 3 files changed, 474 insertions(+), 58 deletions(-) diff --git a/mergify_cli/stack/cli.py b/mergify_cli/stack/cli.py index 5e886bf..98e10ec 100644 --- a/mergify_cli/stack/cli.py +++ b/mergify_cli/stack/cli.py @@ -2,6 +2,7 @@ import asyncio import os +from typing import Any from urllib import parse import click @@ -96,7 +97,109 @@ def github_server_to_context( ) -@stack.command(help="Configure the required git commit-msg hooks") # type: ignore[untyped-decorator] +def _print_hooks_status(status: dict[str, Any]) -> None: + """Print hooks status in a formatted table.""" + needs_setup = False + needs_force = False + + # Git hooks section + console.print("\nGit Hooks Status:\n") + git_hooks = status["git_hooks"] + + for hook_name, info in git_hooks.items(): + console.print(f" {hook_name}:") + + wrapper_status = info["wrapper_status"] + wrapper_path = info["wrapper_path"] + + if wrapper_status == stack_setup_mod.WrapperStatus.INSTALLED: + console.print(f" Wrapper: [green]installed[/] ({wrapper_path})") + elif wrapper_status == stack_setup_mod.WrapperStatus.LEGACY: + console.print( + " Wrapper: [yellow]legacy[/] (needs --force to migrate)", + ) + needs_force = True + else: # MISSING + console.print(" Wrapper: [red]not installed[/]") + needs_setup = True + + script_path = info["script_path"] + if info["script_installed"]: + if info["script_needs_update"]: + console.print(f" Script: [yellow]needs update[/] ({script_path})") + needs_setup = True + else: + console.print(f" Script: [green]up to date[/] ({script_path})") + else: + console.print(" Script: [red]not installed[/]") + needs_setup = True + + console.print() + + # Claude hooks section + console.print("Claude Hooks Status:\n") + claude_hooks = status["claude_hooks"] + + for script_name, script_info in claude_hooks["scripts"].items(): + console.print(f" {script_name}:") + if script_info["installed"]: + if script_info["needs_update"]: + console.print( + f" Script: [yellow]needs update[/] ({script_info['path']})", + ) + needs_setup = True + else: + console.print( + f" Script: [green]up to date[/] ({script_info['path']})", + ) + else: + console.print(" Script: [red]not installed[/]") + needs_setup = True + console.print() + + console.print(" settings.json:") + if claude_hooks["settings_installed"]: + console.print( + f" Hook: [green]configured[/] ({claude_hooks['settings_path']})", + ) + else: + console.print(" Hook: [red]not configured[/]") + needs_setup = True + console.print() + + if needs_setup or needs_force: + console.print("Run 'mergify stack hooks --setup' to install/upgrade hooks.") + if needs_force: + console.print( + "Run 'mergify stack hooks --setup --force' to force reinstall wrappers.", + ) + else: + console.print("[green]All hooks are up to date.[/]") + + +@stack.command(help="Show git hooks status and manage installation") # type: ignore[untyped-decorator] +@click.option( + "--setup", + "do_setup", + is_flag=True, + help="Install or upgrade hooks", +) +@click.option( + "--force", + "-f", + is_flag=True, + help="Force reinstall wrappers (use with --setup)", +) +@utils.run_with_asyncio +async def hooks(*, do_setup: bool, force: bool) -> None: + if do_setup: + await stack_setup_mod.stack_setup(force=force) + else: + status = await stack_setup_mod.get_hooks_status() + _print_hooks_status(status) + + +@stack.command(help="Configure git hooks (alias for 'stack hooks --setup')") # type: ignore[untyped-decorator] @click.option( "--force", "-f", @@ -106,11 +209,15 @@ def github_server_to_context( @click.option( "--check", is_flag=True, - help="Only check hook status without making changes", + help="Check status only (use 'stack hooks' instead)", ) @utils.run_with_asyncio async def setup(*, force: bool, check: bool) -> None: - await stack_setup_mod.stack_setup(force=force, check_only=check) + if check: + status = await stack_setup_mod.get_hooks_status() + _print_hooks_status(status) + else: + await stack_setup_mod.stack_setup(force=force) @stack.command(help="Edit the stack history") # type: ignore[untyped-decorator] diff --git a/mergify_cli/stack/setup.py b/mergify_cli/stack/setup.py index b5fbbbb..b6adcb8 100644 --- a/mergify_cli/stack/setup.py +++ b/mergify_cli/stack/setup.py @@ -17,9 +17,16 @@ import enum import importlib.resources +import importlib.resources.abc import json import pathlib import shutil +from typing import TYPE_CHECKING +from typing import Any + + +if TYPE_CHECKING: + from collections.abc import Iterator from mergify_cli import console from mergify_cli import utils @@ -48,11 +55,56 @@ def _get_claude_hooks_dir() -> pathlib.Path: return pathlib.Path.home() / ".config" / "mergify-cli" / "claude-hooks" +def _get_claude_hook_scripts() -> Iterator[importlib.resources.abc.Traversable]: + """Iterate over Claude hook script files in package resources.""" + claude_hooks_src = importlib.resources.files(__package__).joinpath("claude_hooks") + for src_file in claude_hooks_src.iterdir(): + if src_file.name.endswith(".sh"): + yield src_file + + +def _claude_script_needs_update( + dest_file: pathlib.Path, + src_file: importlib.resources.abc.Traversable, +) -> bool: + """Check if a Claude hook script needs to be updated by comparing content.""" + if not dest_file.exists(): + return True + installed_content = dest_file.read_text(encoding="utf-8") + new_content = src_file.read_text(encoding="utf-8") + return installed_content != new_content + + +def _get_hook_command(hook: dict[str, object]) -> str: + """Safely extract command from Claude hook structure, handling empty lists.""" + hooks_list = hook.get("hooks", []) + if not hooks_list or not isinstance(hooks_list, list): + return "" + first_hook = hooks_list[0] + if not isinstance(first_hook, dict): + return "" + command = first_hook.get("command", "") + return command if isinstance(command, str) else "" + + def _get_claude_settings_file() -> pathlib.Path: """Get the global Claude settings file path.""" return pathlib.Path.home() / ".claude" / "settings.json" +def _read_claude_settings() -> dict[str, Any]: + """Read and parse Claude settings.json, returning empty dict on failure.""" + settings_file = _get_claude_settings_file() + if not settings_file.exists(): + return {} + try: + result: dict[str, Any] = json.loads(settings_file.read_text(encoding="utf-8")) + except (json.JSONDecodeError, OSError): + return {} + else: + return result + + def _get_wrapper_status(hook_path: pathlib.Path, hook_name: str) -> WrapperStatus: """Check the status of a hook wrapper.""" if not hook_path.exists(): @@ -96,7 +148,6 @@ def _install_git_hook( hook_name: str, *, force: bool = False, - check_only: bool = False, ) -> None: """Install or upgrade a git hook with the sourcing architecture. @@ -120,18 +171,6 @@ def _install_git_hook( ), ) - if check_only: - # Just report status - if wrapper_status == WrapperStatus.MISSING: - console.log(f"Hook not installed: {hook_name}") - elif wrapper_status == WrapperStatus.LEGACY: - console.log(f"Legacy hook found: {hook_name} (use --force to migrate)") - elif _script_needs_update(script_path, new_script_file): - console.log(f"Hook script needs update: {hook_name}") - else: - console.log(f"Hook is up to date: {hook_name}") - return - # Create mergify-hooks directory managed_dir.mkdir(exist_ok=True) @@ -157,7 +196,7 @@ def _install_git_hook( else: console.print( f"Found legacy hook: {hook_name}\n" - f"Run 'mergify stack setup --force' to migrate to new format.", + f"Run 'mergify stack hooks --setup --force' to migrate to new format.", style="yellow", ) @@ -176,22 +215,10 @@ def _install_claude_hooks() -> None: claude_hooks_dir.mkdir(parents=True, exist_ok=True) # Install hook scripts - claude_hooks_src = importlib.resources.files(__package__).joinpath("claude_hooks") - for src_file in claude_hooks_src.iterdir(): - if not src_file.name.endswith(".sh"): - continue - + for src_file in _get_claude_hook_scripts(): dest_file = claude_hooks_dir / src_file.name - # Check if update needed by comparing content directly - # Use read_text() on Traversable to handle zip-packaged installations - needs_update = True - if dest_file.exists(): - installed_content = dest_file.read_text(encoding="utf-8") - new_content = src_file.read_text(encoding="utf-8") - needs_update = installed_content != new_content - - if needs_update: + if _claude_script_needs_update(dest_file, src_file): console.log(f"Updating Claude hook script: {src_file.name}") # Use as_file() context manager for safe copying from package resources with importlib.resources.as_file(src_file) as src_path: @@ -203,16 +230,7 @@ def _install_claude_hooks() -> None: # Install/update Claude settings settings_file = _get_claude_settings_file() settings_file.parent.mkdir(parents=True, exist_ok=True) - - if settings_file.exists(): - try: - existing_settings = json.loads( - settings_file.read_text(encoding="utf-8"), - ) - except json.JSONDecodeError: - existing_settings = {} - else: - existing_settings = {} + existing_settings = _read_claude_settings() if "hooks" not in existing_settings: existing_settings["hooks"] = {} @@ -232,17 +250,6 @@ def _install_claude_hooks() -> None: existing_hooks = existing_settings["hooks"].get("SessionStart", []) - def _get_hook_command(hook: dict[str, object]) -> str: - """Safely extract command from hook structure, handling empty lists.""" - hooks_list = hook.get("hooks", []) - if not hooks_list or not isinstance(hooks_list, list): - return "" - first_hook = hooks_list[0] - if not isinstance(first_hook, dict): - return "" - command = first_hook.get("command", "") - return command if isinstance(command, str) else "" - # Check if our hook is already installed (by checking the command) already_installed = any( _get_hook_command(hook) == hook_script_path for hook in existing_hooks @@ -270,21 +277,98 @@ def _get_hook_command(hook: dict[str, object]) -> str: console.log("Installation of Claude settings.json hook") -async def stack_setup(*, force: bool = False, check_only: bool = False) -> None: +def _get_claude_hooks_status() -> dict[str, Any]: + """Get detailed status of Claude hooks for display. + + Returns: + Dictionary with 'scripts' and 'settings' status info. + """ + claude_hooks_dir = _get_claude_hooks_dir() + settings_file = _get_claude_settings_file() + + # Check script status + scripts_status = {} + for src_file in _get_claude_hook_scripts(): + dest_file = claude_hooks_dir / src_file.name + installed = dest_file.exists() + needs_update = ( + _claude_script_needs_update(dest_file, src_file) if installed else False + ) + + scripts_status[src_file.name] = { + "installed": installed, + "needs_update": needs_update, + "path": str(dest_file), + } + + # Check settings.json status + hook_script_path = str(claude_hooks_dir / "session-start.sh") + existing_settings = _read_claude_settings() + existing_hooks = existing_settings.get("hooks", {}).get("SessionStart", []) + settings_installed = any( + _get_hook_command(hook) == hook_script_path for hook in existing_hooks + ) + + return { + "scripts": scripts_status, + "settings_installed": settings_installed, + "settings_path": str(settings_file), + } + + +async def get_hooks_status() -> dict[str, Any]: + """Get detailed status of all hooks for display. + + Returns: + Dictionary with 'git_hooks' and 'claude_hooks' status info. + """ + hooks_dir = pathlib.Path(await utils.git("rev-parse", "--git-path", "hooks")) + managed_dir = hooks_dir / "mergify-hooks" + + git_hooks = {} + for hook_name in _get_git_hook_names(): + wrapper_path = hooks_dir / hook_name + script_path = managed_dir / f"{hook_name}.sh" + + wrapper_status = _get_wrapper_status(wrapper_path, hook_name) + script_installed = script_path.exists() + script_needs_update = False + + if script_installed: + new_script_file = str( + importlib.resources.files(__package__).joinpath( + f"hooks/scripts/{hook_name}.sh", + ), + ) + script_needs_update = _script_needs_update(script_path, new_script_file) + + git_hooks[hook_name] = { + "wrapper_status": wrapper_status, + "script_installed": script_installed, + "script_needs_update": script_needs_update, + "wrapper_path": str(wrapper_path), + "script_path": str(script_path), + } + + return { + "git_hooks": git_hooks, + "claude_hooks": _get_claude_hooks_status(), + } + + +async def stack_setup(*, force: bool = False) -> None: """Set up git hooks for the stack workflow. Args: force: If True, overwrite wrappers even if user modified them - check_only: If True, only check status without making changes """ hooks_dir = pathlib.Path(await utils.git("rev-parse", "--git-path", "hooks")) for hook_name in _get_git_hook_names(): - _install_git_hook(hooks_dir, hook_name, force=force, check_only=check_only) + _install_git_hook(hooks_dir, hook_name, force=force) - if not check_only: - # Install Claude hooks for session ID tracking (global) - _install_claude_hooks() + # Install Claude hooks for session ID tracking (global) + _install_claude_hooks() async def ensure_hooks_updated() -> None: diff --git a/mergify_cli/tests/stack/test_setup.py b/mergify_cli/tests/stack/test_setup.py index 3f4f6af..d02907f 100644 --- a/mergify_cli/tests/stack/test_setup.py +++ b/mergify_cli/tests/stack/test_setup.py @@ -4,6 +4,8 @@ import subprocess import typing +from click.testing import CliRunner + from mergify_cli.stack import setup from mergify_cli.stack.changes import CHANGEID_RE @@ -172,3 +174,226 @@ def test_new_commit_after_amend_gets_new_change_id( assert second_change_id != first_change_id, ( "Each commit should have a unique Change-Id" ) + + +async def test_get_hooks_status_no_hooks( + git_mock: test_utils.GitMock, + tmp_path: pathlib.Path, +) -> None: + """Test get_hooks_status when no hooks are installed.""" + hooks_dir = tmp_path / ".git" / "hooks" + hooks_dir.mkdir(parents=True) + git_mock.mock("rev-parse", "--git-path", "hooks", output=str(hooks_dir)) + + status = await setup.get_hooks_status() + + # Should have git_hooks and claude_hooks sections + assert "git_hooks" in status + assert "claude_hooks" in status + + git_hooks = status["git_hooks"] + assert "commit-msg" in git_hooks + assert "prepare-commit-msg" in git_hooks + + # All git hooks should show as not installed + for info in git_hooks.values(): + assert info["wrapper_status"] == setup.WrapperStatus.MISSING + assert info["script_installed"] is False + + +async def test_get_hooks_status_installed_hooks( + git_mock: test_utils.GitMock, + tmp_path: pathlib.Path, +) -> None: + """Test get_hooks_status when hooks are installed.""" + import importlib.resources + import shutil + + hooks_dir = tmp_path / ".git" / "hooks" + hooks_dir.mkdir(parents=True) + managed_dir = hooks_dir / "mergify-hooks" + managed_dir.mkdir(parents=True) + + git_mock.mock("rev-parse", "--git-path", "hooks", output=str(hooks_dir)) + + # Install hooks + for hook_name in ("commit-msg", "prepare-commit-msg"): + # Install wrapper + wrapper_source = str( + importlib.resources.files("mergify_cli.stack").joinpath( + f"hooks/wrappers/{hook_name}", + ), + ) + shutil.copy(wrapper_source, hooks_dir / hook_name) + + # Install script + script_source = str( + importlib.resources.files("mergify_cli.stack").joinpath( + f"hooks/scripts/{hook_name}.sh", + ), + ) + shutil.copy(script_source, managed_dir / f"{hook_name}.sh") + + status = await setup.get_hooks_status() + + # All git hooks should show as installed and up to date + git_hooks = status["git_hooks"] + for info in git_hooks.values(): + assert info["wrapper_status"] == setup.WrapperStatus.INSTALLED + assert info["script_installed"] is True + assert info["script_needs_update"] is False + + +async def test_get_hooks_status_outdated_script( + git_mock: test_utils.GitMock, + tmp_path: pathlib.Path, +) -> None: + """Test get_hooks_status when script is outdated.""" + import importlib.resources + import shutil + + hooks_dir = tmp_path / ".git" / "hooks" + hooks_dir.mkdir(parents=True) + managed_dir = hooks_dir / "mergify-hooks" + managed_dir.mkdir(parents=True) + + git_mock.mock("rev-parse", "--git-path", "hooks", output=str(hooks_dir)) + + # Install wrapper + wrapper_source = str( + importlib.resources.files("mergify_cli.stack").joinpath( + "hooks/wrappers/commit-msg", + ), + ) + shutil.copy(wrapper_source, hooks_dir / "commit-msg") + + # Install script with different (old) content + script_path = managed_dir / "commit-msg.sh" + script_path.write_text("#!/bin/sh\n# old script content\n") + + status = await setup.get_hooks_status() + + git_hooks = status["git_hooks"] + assert git_hooks["commit-msg"]["wrapper_status"] == setup.WrapperStatus.INSTALLED + assert git_hooks["commit-msg"]["script_installed"] is True + assert git_hooks["commit-msg"]["script_needs_update"] is True + + +def test_hooks_command_shows_status( + git_repo_with_hooks: pathlib.Path, +) -> None: + """Test that 'stack hooks' command shows status.""" + import os + + from mergify_cli.cli import cli + + os.chdir(git_repo_with_hooks) + + runner = CliRunner() + result = runner.invoke(cli, ["stack", "hooks"]) + + assert result.exit_code == 0 + # Git hooks section + assert "Git Hooks Status:" in result.output + assert "commit-msg:" in result.output + assert "prepare-commit-msg:" in result.output + assert "Wrapper:" in result.output + # Claude hooks section + assert "Claude Hooks Status:" in result.output + assert "settings.json:" in result.output + + +def test_hooks_command_setup_flag( + tmp_path: pathlib.Path, +) -> None: + """Test that 'stack hooks --setup' installs hooks.""" + import os + + from mergify_cli.cli import cli + + # Create a git repo without hooks + subprocess.run( + ["git", "init", "--initial-branch=main"], + check=True, + cwd=tmp_path, + ) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + check=True, + cwd=tmp_path, + ) + subprocess.run( + ["git", "config", "user.name", "Test User"], + check=True, + cwd=tmp_path, + ) + + os.chdir(tmp_path) + + runner = CliRunner() + result = runner.invoke(cli, ["stack", "hooks", "--setup"]) + + assert result.exit_code == 0 + + # Verify hooks were installed + hooks_dir = tmp_path / ".git" / "hooks" + assert (hooks_dir / "commit-msg").exists() + assert (hooks_dir / "prepare-commit-msg").exists() + assert (hooks_dir / "mergify-hooks" / "commit-msg.sh").exists() + assert (hooks_dir / "mergify-hooks" / "prepare-commit-msg.sh").exists() + + +def test_setup_command_check_flag( + git_repo_with_hooks: pathlib.Path, +) -> None: + """Test that 'stack setup --check' shows status (alias behavior).""" + import os + + from mergify_cli.cli import cli + + os.chdir(git_repo_with_hooks) + + runner = CliRunner() + result = runner.invoke(cli, ["stack", "setup", "--check"]) + + assert result.exit_code == 0 + assert "Git Hooks Status:" in result.output + assert "commit-msg:" in result.output + + +def test_setup_command_without_flags( + tmp_path: pathlib.Path, +) -> None: + """Test that 'stack setup' installs hooks (backwards compatibility).""" + import os + + from mergify_cli.cli import cli + + # Create a git repo without hooks + subprocess.run( + ["git", "init", "--initial-branch=main"], + check=True, + cwd=tmp_path, + ) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + check=True, + cwd=tmp_path, + ) + subprocess.run( + ["git", "config", "user.name", "Test User"], + check=True, + cwd=tmp_path, + ) + + os.chdir(tmp_path) + + runner = CliRunner() + result = runner.invoke(cli, ["stack", "setup"]) + + assert result.exit_code == 0 + + # Verify hooks were installed + hooks_dir = tmp_path / ".git" / "hooks" + assert (hooks_dir / "commit-msg").exists() + assert (hooks_dir / "prepare-commit-msg").exists()