From 80c52aa87a38045532428cb2c753337d60ecf3a7 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:06:19 -0700 Subject: [PATCH 1/5] test: cover is_dir() OSError guard in _full_scandir_discovery (#846) Add test_full_scandir_is_dir_oserror_skips_entry to verify that a root-level DirEntry whose is_dir() raises OSError (e.g. broken symlink or EACCES on lstat) is silently skipped rather than crashing discovery. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/copilot_usage/test_parser.py | 51 +++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index 554c21a4..d271bd2b 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -11,7 +11,7 @@ from datetime import UTC, datetime from pathlib import Path from typing import SupportsIndex, overload -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest from pydantic import ValidationError @@ -629,6 +629,55 @@ def _bomb(path: str | os.PathLike[str]) -> Iterator[os.DirEntry[str]]: assert len(result) == 1 assert result[0][0].parent.name == "sess-good" + def test_full_scandir_is_dir_oserror_skips_entry(self, tmp_path: Path) -> None: + """Skip a root-level entry whose ``is_dir()`` raises ``OSError``. + + Simulates a broken symlink or ``EACCES`` on ``lstat`` by wrapping + ``os.scandir`` so that one entry's ``is_dir()`` raises ``OSError``. + The faulting entry must be silently skipped, not crash discovery. + """ + good = tmp_path / "sess-good" + _write_events(good / "events.jsonl", _START_EVENT) + bad = tmp_path / "sess-bad" + _write_events(bad / "events.jsonl", _START_EVENT) + + original_scandir = os.scandir + + def _patched_scandir( + path: str | os.PathLike[str], + ) -> Iterator[os.DirEntry[str]]: + ctx = original_scandir(path) + if str(path) != str(tmp_path): + return ctx # type: ignore[return-value] + + class _WrappedCtx: + """Context manager that wraps scandir entries.""" + + def __enter__(self) -> Iterator[os.DirEntry[str]]: + entries: list[os.DirEntry[str]] = list(original_scandir(path)) + wrapped: list[os.DirEntry[str]] = [] + for e in entries: + if e.name == "sess-bad": + m = MagicMock(spec=os.DirEntry) + m.name = e.name + m.path = e.path + m.is_dir.side_effect = OSError("lstat failed") + wrapped.append(m) + else: + wrapped.append(e) + return iter(wrapped) + + def __exit__(self, *a: object) -> None: + pass + + return _WrappedCtx() # type: ignore[return-value] + + with patch("copilot_usage.parser.os.scandir", side_effect=_patched_scandir): + _, result = _discover_with_identity(tmp_path) + + assert len(result) == 1 + assert result[0][0].parent.name == "sess-good" + # --------------------------------------------------------------------------- # _discover_with_identity — linear scan (issue #773) From 692f580781a7b759b3f2662c18aadeab93181683 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:15:46 -0700 Subject: [PATCH 2/5] fix: address review comments - Remove eager original_scandir(path) call that leaked a ScandirIterator when the root branch returns _WrappedCtx instead - Use 'with original_scandir(path) as it:' in __enter__ to deterministically close the iterator Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/copilot_usage/test_parser.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index d271bd2b..5dee17cd 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -646,15 +646,15 @@ def test_full_scandir_is_dir_oserror_skips_entry(self, tmp_path: Path) -> None: def _patched_scandir( path: str | os.PathLike[str], ) -> Iterator[os.DirEntry[str]]: - ctx = original_scandir(path) if str(path) != str(tmp_path): - return ctx # type: ignore[return-value] + return original_scandir(path) # type: ignore[return-value] class _WrappedCtx: """Context manager that wraps scandir entries.""" def __enter__(self) -> Iterator[os.DirEntry[str]]: - entries: list[os.DirEntry[str]] = list(original_scandir(path)) + with original_scandir(path) as it: + entries: list[os.DirEntry[str]] = list(it) wrapped: list[os.DirEntry[str]] = [] for e in entries: if e.name == "sess-bad": From 3d079e53178f09db0d17bf5cfc576fd01a4c0f93 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:26:05 -0700 Subject: [PATCH 3/5] fix: address review comments Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/copilot_usage/test_parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index 5dee17cd..02429259 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -10,7 +10,7 @@ from collections.abc import Iterator from datetime import UTC, datetime from pathlib import Path -from typing import SupportsIndex, overload +from typing import SupportsIndex, cast, overload from unittest.mock import MagicMock, patch import pytest @@ -662,7 +662,7 @@ def __enter__(self) -> Iterator[os.DirEntry[str]]: m.name = e.name m.path = e.path m.is_dir.side_effect = OSError("lstat failed") - wrapped.append(m) + wrapped.append(cast(os.DirEntry[str], m)) else: wrapped.append(e) return iter(wrapped) From 0e8ca024b41fdfdf0255df9edf6d8693657d69a4 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:38:05 -0700 Subject: [PATCH 4/5] fix: address review comments Keep ScandirIterator open for the lifetime of the _WrappedCtx context manager so DirEntry.is_dir() calls don't fail on DT_UNKNOWN filesystems (where closing the iterator invalidates the dir fd needed for lstat). The iterator is now yielded lazily and closed in __exit__, matching the real os.scandir lifecycle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/copilot_usage/test_parser.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index 02429259..acf877da 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -650,25 +650,32 @@ def _patched_scandir( return original_scandir(path) # type: ignore[return-value] class _WrappedCtx: - """Context manager that wraps scandir entries.""" + """Context manager keeping the scandir iterator open.""" - def __enter__(self) -> Iterator[os.DirEntry[str]]: - with original_scandir(path) as it: - entries: list[os.DirEntry[str]] = list(it) - wrapped: list[os.DirEntry[str]] = [] - for e in entries: + def __init__(self) -> None: + self._it: Iterator[os.DirEntry[str]] | None = None + + def _iter_wrapped_entries(self) -> Iterator[os.DirEntry[str]]: + if self._it is None: + return + for e in self._it: if e.name == "sess-bad": m = MagicMock(spec=os.DirEntry) m.name = e.name m.path = e.path m.is_dir.side_effect = OSError("lstat failed") - wrapped.append(cast(os.DirEntry[str], m)) + yield cast(os.DirEntry[str], m) else: - wrapped.append(e) - return iter(wrapped) + yield e + + def __enter__(self) -> Iterator[os.DirEntry[str]]: + self._it = original_scandir(path) + return self._iter_wrapped_entries() def __exit__(self, *a: object) -> None: - pass + if self._it is not None: + self._it.close() # type: ignore[union-attr] + self._it = None return _WrappedCtx() # type: ignore[return-value] From b87b0caf05485b35e70164bd44e2aebf79d23558 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Tue, 7 Apr 2026 23:46:28 -0700 Subject: [PATCH 5/5] fix: address review comments Change _patched_scandir return type from Iterator to AbstractContextManager[Iterator[os.DirEntry[str]]] so both branches type-check without # type: ignore[return-value] comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/copilot_usage/test_parser.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index acf877da..a18f3056 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -8,6 +8,7 @@ import os import time from collections.abc import Iterator +from contextlib import AbstractContextManager from datetime import UTC, datetime from pathlib import Path from typing import SupportsIndex, cast, overload @@ -645,9 +646,9 @@ def test_full_scandir_is_dir_oserror_skips_entry(self, tmp_path: Path) -> None: def _patched_scandir( path: str | os.PathLike[str], - ) -> Iterator[os.DirEntry[str]]: + ) -> AbstractContextManager[Iterator[os.DirEntry[str]]]: if str(path) != str(tmp_path): - return original_scandir(path) # type: ignore[return-value] + return original_scandir(path) class _WrappedCtx: """Context manager keeping the scandir iterator open.""" @@ -677,7 +678,7 @@ def __exit__(self, *a: object) -> None: self._it.close() # type: ignore[union-attr] self._it = None - return _WrappedCtx() # type: ignore[return-value] + return _WrappedCtx() with patch("copilot_usage.parser.os.scandir", side_effect=_patched_scandir): _, result = _discover_with_identity(tmp_path)