diff --git a/CHANGELOG.md b/CHANGELOG.md index 96adf28..9603a75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and versions are tracked in the repo-root `VERSION` file. - Keep private runtime files and directories owner-only on POSIX, use inherited user-profile ACLs on Windows, and make history appends binary-safe across locking backends. +- Make terminal detection tolerate closed streams and record `COMSPEC` when + Windows has no `SHELL` environment variable. ## [0.2.0] - 2026-08-01 diff --git a/README.md b/README.md index 3f088fd..303304e 100644 --- a/README.md +++ b/README.md @@ -499,8 +499,9 @@ def test_command(tmp_path: Path) -> None: assert "hello Ada" in result.stdout ``` -The helper wraps Click's `CliRunner`, sets `HOME` when requested, and supplies -`cwd` to the invocation for the duration of the test. Calls that use +The helper wraps Click's `CliRunner`, sets `HOME` plus the relevant +`USERPROFILE`, `LOCALAPPDATA`, and `XDG_CACHE_HOME` values when requested, and +supplies `cwd` to the invocation for the duration of the test. Calls that use `cwd` are serialized and the caller's cwd is restored afterward, but this remains process-global: do not use it concurrently with code that changes cwd outside `invoke()` or from threads spawned by the invoked command. A diff --git a/lib/python/base_cli/history.py b/lib/python/base_cli/history.py index ac20615..fadd830 100644 --- a/lib/python/base_cli/history.py +++ b/lib/python/base_cli/history.py @@ -84,7 +84,7 @@ def build_finished_record( "project_root": compact_optional_path(context.project_root), "manifest": compact_optional_path(context.manifest_path), "workspace_root": compact_optional_path(context.workspace_root), - "shell": os.environ.get("SHELL"), + "shell": current_shell(), "scope": context.history_scope, "parent_run_id": context.history_parent_run_id, } @@ -286,6 +286,12 @@ def normalized_os() -> str: return system or platform.platform() +def current_shell() -> str | None: + """Return the active shell identifier across POSIX and Windows.""" + + return os.environ.get("SHELL") or os.environ.get("COMSPEC") + + def redact_history_argv(argv: list[str], sensitive_options: set[str]) -> list[str]: redacted = redact_argv(argv, sensitive_options) result: list[str] = [] diff --git a/lib/python/base_cli/logging.py b/lib/python/base_cli/logging.py index 89f5617..416e8ca 100644 --- a/lib/python/base_cli/logging.py +++ b/lib/python/base_cli/logging.py @@ -69,12 +69,12 @@ def _handler_formatter(formatter: logging.Formatter | None, *, use_color: bool) def _use_color(stream: TextIO) -> bool: - return ( - os.environ.get("BASE_CLI_COLOR") != "0" - and "NO_COLOR" not in os.environ - and hasattr(stream, "isatty") - and stream.isatty() - ) + if os.environ.get("BASE_CLI_COLOR") == "0" or "NO_COLOR" in os.environ: + return False + try: + return bool(stream.isatty()) + except (AttributeError, OSError, ValueError): + return False def secure_log_file_permissions(log_file: Path) -> None: diff --git a/lib/python/base_cli/output.py b/lib/python/base_cli/output.py index 753ae05..463ba58 100644 --- a/lib/python/base_cli/output.py +++ b/lib/python/base_cli/output.py @@ -30,7 +30,7 @@ def is_terminal(stream: TextIO | None = None) -> bool: candidate = stream if stream is not None else sys.stdout try: return bool(candidate.isatty()) - except (AttributeError, OSError): + except (AttributeError, OSError, ValueError): return False diff --git a/tests/test_history.py b/tests/test_history.py index 349af3e..e92f5d0 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -22,6 +22,18 @@ def locking(self, _fd: int, mode: int, size: int) -> None: class HistoryAppendTests(unittest.TestCase): + def test_current_shell_falls_back_to_comspec(self) -> None: + with mock.patch.dict("os.environ", {"COMSPEC": r"C:\Windows\System32\cmd.exe"}, clear=True): + self.assertEqual(history.current_shell(), r"C:\Windows\System32\cmd.exe") + + def test_current_shell_prefers_shell(self) -> None: + with mock.patch.dict( + "os.environ", + {"SHELL": "/bin/zsh", "COMSPEC": r"C:\Windows\System32\cmd.exe"}, + clear=True, + ): + self.assertEqual(history.current_shell(), "/bin/zsh") + def test_concurrent_appends_produce_complete_records(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: path = Path(tmpdir) / "history.jsonl" diff --git a/tests/test_logging.py b/tests/test_logging.py index 000ca20..3a7d52f 100644 --- a/tests/test_logging.py +++ b/tests/test_logging.py @@ -105,6 +105,20 @@ def test_configure_logger_honors_explicit_color_disable(self) -> None: self.assertNotIn("\033[", stream.getvalue()) + def test_configure_logger_handles_streams_that_reject_isatty(self) -> None: + class ClosedStream(io.StringIO): + def isatty(self) -> bool: + raise ValueError("stream is closed") + + stream = ClosedStream() + + with mock.patch.dict(os.environ, {}, clear=True): + logger = base_cli.configure_logger("closed-stream", None, debug=False, stream=stream) + logger.info("hello closed stream") + + self.assertNotIn("\033[", stream.getvalue()) + self.assertIn("hello closed stream", stream.getvalue()) + def test_configure_logger_uses_custom_formatter_for_file_handler(self) -> None: formatter = logging.Formatter("%(levelname)s:%(message)s") user_stream = io.StringIO() diff --git a/tests/test_output.py b/tests/test_output.py index a4aaab9..84b8437 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -18,6 +18,11 @@ def isatty(self) -> bool: return self.terminal +class _ClosedStream(io.StringIO): + def isatty(self) -> bool: + raise ValueError("stream is closed") + + RECORDS = ( {"name": "base", "path": "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/work/base"}, {"name": "demo,one", "path": "/work/demo\tone"}, @@ -26,6 +31,9 @@ def isatty(self) -> bool: class OutputTest(unittest.TestCase): + def test_closed_stream_is_not_treated_as_terminal(self) -> None: + self.assertEqual(resolve_output_format("text", stream=_ClosedStream()), "tsv") + def test_text_is_pretty_on_terminal(self) -> None: stream = _Stream(terminal=True)