diff --git a/CLAUDE.md b/CLAUDE.md index e976693f..30742bbc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,10 @@ uv run forge worker # Print Forge version uv run forge version +uv run forge version --json + +# Run Forge as a module +python -m forge version # Build container podman build -t forge-dev:latest containers/ diff --git a/docs/developer-guide.md b/docs/developer-guide.md index e05d0ab7..b4b8ac13 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -723,17 +723,31 @@ For workflows paused at `review_response_gate` (due to contested comments): ### Forge Version Command -To print the currently installed Forge package version, run: +To print the currently installed Forge package version in plain text, run: ```bash uv run forge version ``` -This will print the package version in the format `Forge v` (e.g., `Forge v1.0.0`) and exit with a success status code. +This will print the package version in the format `Forge v` (e.g., `Forge v2.0.0`) and exit with a success status code. + +To print the version information as a JSON object, run: + +```bash +uv run forge version --json +``` + +This will print the version metadata in compact JSON format (e.g., `{"version": "2.0.0"}`) directly to standard output. + +Alternatively, you can run Forge as a module using the `python -m` option: + +```bash +python -m forge version +``` ### Worker logs -The worker logs to stdout. Useful log entries to grep for: +The worker logs strictly to stderr to prevent log messages from polluting standard output. Useful log entries to grep for: ```bash # Watch for a specific ticket diff --git a/src/forge/__main__.py b/src/forge/__main__.py new file mode 100644 index 00000000..9a8e2512 --- /dev/null +++ b/src/forge/__main__.py @@ -0,0 +1,8 @@ +"""Main entry point for forge when run as a module.""" + +import sys + +from forge.cli import main + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/forge/cli.py b/src/forge/cli.py index 0318d8e8..0aa7d9ee 100644 --- a/src/forge/cli.py +++ b/src/forge/cli.py @@ -13,11 +13,34 @@ def setup_logging(verbose: bool = False) -> None: """Configure logging for CLI usage.""" + root_logger = logging.getLogger() + # Clear any pre-existing logging handlers registered on the root logger + for handler in list(root_logger.handlers): + root_logger.removeHandler(handler) + level = logging.DEBUG if verbose else logging.INFO - logging.basicConfig( - level=level, - format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", - ) + root_logger.setLevel(level) + + # Instantiate and attach a new logging.StreamHandler(sys.stderr) + handler = logging.StreamHandler(sys.stderr) + handler.setLevel(level) + + formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s") + handler.setFormatter(formatter) + + root_logger.addHandler(handler) + + # Ensure all auxiliary loggers and standard console handlers default strictly to stderr + for logger_obj in list(logging.root.manager.loggerDict.values()): + if isinstance(logger_obj, logging.Logger): + for h in list(logger_obj.handlers): + if isinstance(h, logging.StreamHandler) and ( + h.stream is sys.stdout or h.stream == sys.stdout + ): + if logger_obj.propagate: + logger_obj.removeHandler(h) + else: + h.stream = sys.stderr async def _get_compiled_workflow_for_ticket(ticket_key: str): @@ -1423,11 +1446,16 @@ async def cmd_smoke_test(_args: argparse.Namespace) -> int: return await run_smoke_test(settings) -async def cmd_version(_args: argparse.Namespace) -> int: +async def cmd_version(args: argparse.Namespace) -> int: """Print the installed Forge package version.""" from forge import __version__ - print(f"Forge v{__version__}") + if getattr(args, "json", False): + import json + + print(json.dumps({"version": __version__})) + else: + print(f"Forge v{__version__}") return 0 @@ -1553,10 +1581,15 @@ def main(argv: list[str] | None = None) -> int: ) # version command - subparsers.add_parser( + version_parser = subparsers.add_parser( "version", help="Print the installed Forge package version", ) + version_parser.add_argument( + "--json", + action="store_true", + help="Print version information as a JSON object", + ) # test-skill subparser group test_skill_parser = subparsers.add_parser( diff --git a/tests/unit/test_cli_version.py b/tests/unit/test_cli_version.py index f3797459..a287b1f3 100644 --- a/tests/unit/test_cli_version.py +++ b/tests/unit/test_cli_version.py @@ -1,6 +1,7 @@ """Unit tests for the forge version CLI command.""" import argparse +from typing import Any from unittest.mock import AsyncMock, patch import pytest @@ -14,7 +15,7 @@ class TestCLIVersionParserAndRouting: @patch("forge.cli.cmd_version", new_callable=AsyncMock) @patch("forge.cli.setup_logging") - def test_routing_version(self, _mock_setup_logging, mock_cmd): + def test_routing_version(self, _mock_setup_logging: Any, mock_cmd: Any) -> None: """Calling main(['version']) routes to cmd_version.""" mock_cmd.return_value = 0 code = main(["version"]) @@ -24,10 +25,383 @@ def test_routing_version(self, _mock_setup_logging, mock_cmd): assert args.command == "version" @pytest.mark.asyncio - async def test_cmd_version_execution(self, capsys): + async def test_cmd_version_execution(self, capsys: Any) -> None: """cmd_version prints the correct version string and exits with 0.""" args = argparse.Namespace() code = await cmd_version(args) assert code == 0 captured = capsys.readouterr() assert f"Forge v{__version__}" in captured.out + + @patch("forge.cli.cmd_version", new_callable=AsyncMock) + @patch("forge.cli.setup_logging") + def test_routing_version_json(self, _mock_setup_logging: Any, mock_cmd: Any) -> None: + """Calling main(['version', '--json']) routes to cmd_version with args.json=True.""" + mock_cmd.return_value = 0 + code = main(["version", "--json"]) + assert code == 0 + mock_cmd.assert_called_once() + args = mock_cmd.call_args[0][0] + assert args.command == "version" + assert args.json is True + + @pytest.mark.asyncio + async def test_cmd_version_json_execution(self, capsys: Any) -> None: + """cmd_version with json=True prints compact JSON and exits with 0.""" + import json + + args = argparse.Namespace(json=True) + code = await cmd_version(args) + assert code == 0 + captured = capsys.readouterr() + + # Verify it has exactly one trailing newline and is valid JSON + assert captured.out.endswith("\n") + assert captured.out.count("\n") == 1 + + data = json.loads(captured.out.strip()) + assert data == {"version": __version__} + assert captured.err == "" + + @pytest.mark.asyncio + async def test_cmd_version_logging_isolation(self, capsys: Any) -> None: + """When verbose logging is set, log output is routed to stderr, and only JSON goes to stdout.""" + import json + import logging + + from forge.cli import setup_logging + + # Clean up existing handlers to start fresh + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + try: + setup_logging(verbose=True) + logger = logging.getLogger("test_cli_version") + logger.info("This is an info log message") + logger.debug("This is a debug log message") + + args = argparse.Namespace(json=True) + code = await cmd_version(args) + assert code == 0 + + captured = capsys.readouterr() + + # Assert stdout has ONLY the json payload + stdout_lines = captured.out.strip().split("\n") + assert len(stdout_lines) == 1 + data = json.loads(stdout_lines[0]) + assert data == {"version": __version__} + + # Assert stderr has the log messages + assert "This is an info log message" in captured.err + assert "This is a debug log message" in captured.err + finally: + # Restore handlers and level + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + def test_setup_logging_clears_handlers_and_routes_to_stderr(self) -> None: + """setup_logging clears existing handlers and attaches a StreamHandler(sys.stderr) with correct level and formatter.""" + import logging + import sys + + from forge.cli import setup_logging + + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + # Add a dummy handler to verify it gets cleared + dummy_handler = logging.NullHandler() + root_logger.addHandler(dummy_handler) + assert dummy_handler in root_logger.handlers + + try: + # Test non-verbose logging setup + setup_logging(verbose=False) + assert dummy_handler not in root_logger.handlers + assert len(root_logger.handlers) == 1 + handler = root_logger.handlers[0] + assert isinstance(handler, logging.StreamHandler) + assert handler.stream is sys.stderr + assert root_logger.level == logging.INFO + assert handler.level == logging.INFO + assert handler.formatter is not None + assert handler.formatter._fmt == "%(asctime)s - %(name)s - %(levelname)s - %(message)s" + + # Test verbose logging setup + setup_logging(verbose=True) + assert len(root_logger.handlers) == 1 + handler = root_logger.handlers[0] + assert isinstance(handler, logging.StreamHandler) + assert handler.stream is sys.stderr + assert root_logger.level == logging.DEBUG + assert handler.level == logging.DEBUG + finally: + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + def test_setup_logging_configures_auxiliary_loggers(self) -> None: + """setup_logging ensures auxiliary loggers with StreamHandler(sys.stdout) are rerouted to sys.stderr.""" + import logging + import sys + + from forge.cli import setup_logging + + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + # Set up an auxiliary logger with a stdout handler + aux_logger = logging.getLogger("test_auxiliary_logger") + aux_handler = logging.StreamHandler(sys.stdout) + aux_logger.addHandler(aux_handler) + + # Keep track of old state of the auxiliary logger + old_aux_handlers = list(aux_logger.handlers) + old_aux_propagate = aux_logger.propagate + + try: + # First, check behavior when propagate is True + aux_logger.propagate = True + setup_logging(verbose=False) + + # The stdout handler should have been removed because propagate is True + assert aux_handler not in aux_logger.handlers + + # Re-add and set propagate to False + aux_logger.addHandler(aux_handler) + aux_logger.propagate = False + + setup_logging(verbose=False) + + # The stdout handler stream should have been redirected to sys.stderr + assert aux_handler in aux_logger.handlers + assert aux_handler.stream is sys.stderr + + finally: + # Restore + aux_logger.handlers.clear() + for h in old_aux_handlers: + # Make sure to reset stream if we mutated it + if isinstance(h, logging.StreamHandler): + h.stream = sys.stdout + aux_logger.addHandler(h) + aux_logger.propagate = old_aux_propagate + + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + @pytest.mark.asyncio + async def test_cmd_version_no_json_attribute_defaults_to_text(self, capsys: Any) -> None: + """When args does not contain a 'json' attribute, cmd_version defaults to plain text.""" + args = argparse.Namespace() + code = await cmd_version(args) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + + @pytest.mark.asyncio + async def test_cmd_version_json_explicit_false(self, capsys: Any) -> None: + """When args has 'json' explicitly set to False, cmd_version prints plain text.""" + args = argparse.Namespace(json=False) + code = await cmd_version(args) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + + @patch("forge.cli.setup_logging") + def test_main_version_json_isolated(self, mock_setup_logging: Any, capsys: Any) -> None: + """Calling main(['-v', 'version', '--json']) prints the correct compact json to stdout and exits 0.""" + import json + + code = main(["-v", "version", "--json"]) + assert code == 0 + mock_setup_logging.assert_called_once_with(True) + captured = capsys.readouterr() + + # Verify it has exactly one trailing newline and is valid JSON + assert captured.out.endswith("\n") + assert captured.out.count("\n") == 1 + + data = json.loads(captured.out.strip()) + assert data == {"version": __version__} + assert captured.err == "" + + @patch("forge.cli.setup_logging") + def test_main_version_plain_text(self, _mock_setup_logging: Any, capsys: Any) -> None: + """Calling main(['version']) prints the correct plain text to stdout and exits 0.""" + code = main(["version"]) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + + @patch("forge.cli.setup_logging") + def test_main_version_verbose_plain_text(self, _mock_setup_logging: Any, capsys: Any) -> None: + """Calling main(['-v', 'version']) prints the correct plain text to stdout and exits 0.""" + code = main(["-v", "version"]) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + + def test_default_version_stream_isolation(self, capsys: Any) -> None: + """Execute main(['version']) and verify stdout contains exactly 'Forge v' while stderr remains empty.""" + import logging + + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + try: + code = main(["version"]) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + assert captured.err == "" + finally: + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + def test_verbose_version_stream_isolation(self, capsys: Any) -> None: + """Execute main(['-v', 'version']) and assert that stdout has only the version payload, while stderr captures verbose logging messages.""" + import logging + + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + try: + code = main(["-v", "version"]) + assert code == 0 + captured = capsys.readouterr() + assert captured.out == f"Forge v{__version__}\n" + # Since verbose is enabled, some debug/verbose logs must be captured on stderr + assert captured.err != "" + finally: + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + def test_verbose_json_version_stream_isolation(self, capsys: Any) -> None: + """Verify that main(['-v', 'version', '--json']) prints a clean, parseable JSON payload on stdout and all auxiliary logs on stderr.""" + import json + import logging + + root_logger = logging.getLogger() + old_handlers = list(root_logger.handlers) + old_level = root_logger.level + root_logger.handlers.clear() + + try: + code = main(["-v", "version", "--json"]) + assert code == 0 + captured = capsys.readouterr() + + # Verify stdout contains exactly the clean JSON payload with a single trailing newline + assert captured.out.endswith("\n") + assert captured.out.count("\n") == 1 + data = json.loads(captured.out.strip()) + assert data == {"version": __version__} + + # Verify stderr captures verbose logging messages + assert captured.err != "" + finally: + root_logger.handlers.clear() + for h in old_handlers: + root_logger.addHandler(h) + root_logger.setLevel(old_level) + + def test_subprocess_version_plain_text(self) -> None: + """Verify stream separation for 'python -m forge version' under actual subprocess execution.""" + import os + import subprocess + import sys + + env = os.environ.copy() + # Add src/ to PYTHONPATH to be absolutely sure the module can be imported + if "PYTHONPATH" in env: + env["PYTHONPATH"] = f"src{os.pathsep}{env['PYTHONPATH']}" + else: + env["PYTHONPATH"] = "src" + + res = subprocess.run( + [sys.executable, "-m", "forge", "version"], + capture_output=True, + text=True, + env=env, + ) + + assert res.returncode == 0 + assert res.stdout == f"Forge v{__version__}\n" + assert res.stderr == "" + + def test_subprocess_version_verbose_plain_text(self) -> None: + """Verify stream separation for 'python -m forge -v version' under actual subprocess execution.""" + import os + import subprocess + import sys + + env = os.environ.copy() + if "PYTHONPATH" in env: + env["PYTHONPATH"] = f"src{os.pathsep}{env['PYTHONPATH']}" + else: + env["PYTHONPATH"] = "src" + + res = subprocess.run( + [sys.executable, "-m", "forge", "-v", "version"], + capture_output=True, + text=True, + env=env, + ) + + assert res.returncode == 0 + assert res.stdout == f"Forge v{__version__}\n" + # Since verbose is enabled, some debug/verbose logs must be captured on stderr, but stdout remains clean + assert res.stderr != "" + + def test_subprocess_version_verbose_json(self) -> None: + """Verify stream separation for 'python -m forge -v version --json' under actual subprocess execution.""" + import json + import os + import subprocess + import sys + + env = os.environ.copy() + if "PYTHONPATH" in env: + env["PYTHONPATH"] = f"src{os.pathsep}{env['PYTHONPATH']}" + else: + env["PYTHONPATH"] = "src" + + res = subprocess.run( + [sys.executable, "-m", "forge", "-v", "version", "--json"], + capture_output=True, + text=True, + env=env, + ) + + assert res.returncode == 0 + + # Verify stdout contains exactly the clean JSON payload with a single trailing newline + assert res.stdout.endswith("\n") + assert res.stdout.count("\n") == 1 + data = json.loads(res.stdout.strip()) + assert data == {"version": __version__} + + # Verify stderr captures verbose logging messages, but stdout remains free of logs + assert res.stderr != "" diff --git a/tests/unit/workflow/nodes/test_implement_work.py b/tests/unit/workflow/nodes/test_implement_work.py index 1b752d06..f4c65ffc 100644 --- a/tests/unit/workflow/nodes/test_implement_work.py +++ b/tests/unit/workflow/nodes/test_implement_work.py @@ -1,6 +1,7 @@ """Tests for the generic task-first implementation node.""" from types import SimpleNamespace +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -9,7 +10,7 @@ from forge.workflow.stations.implementation_input import NoPendingImplementationWork -def resolved_task(): +def resolved_task() -> SimpleNamespace: artifact = { "id": "jira:TASK-1:task", "kind": "task", @@ -41,7 +42,7 @@ async def test_implements_resolved_task_and_marks_normalized_work_complete() -> jira.close = AsyncMock() git = MagicMock() - async def execute(state, *_args, **_kwargs): + async def execute(state: dict[str, Any], *_args: Any, **_kwargs: Any) -> dict[str, Any]: return {**state, "last_error": None, "commit_info": {"committed": True}} with ( @@ -71,6 +72,10 @@ async def execute(state, *_args, **_kwargs): AsyncMock(side_effect=lambda _state, _jira, prompt: prompt), ), patch("forge.workflow.nodes.implement_work.post_status_comment", AsyncMock()), + patch( + "forge.workflow.nodes.implement_work.ContainerRunner", + return_value=MagicMock(), + ), patch( "forge.workflow.nodes.implement_work.run_and_persist_execution", AsyncMock(side_effect=execute),