test: add failing tests for circular include detection in non-recursive mode (#553)

pdd/preprocess.py

@@ -197,11 +197,30 @@ def replace_include(match):
             console.print(f"[bold red]Error processing include:[/bold red] {str(e)}")
             _dbg(f"Error processing backtick include {file_path}: {e}")
             return f"```[Error processing include: {file_path}]```"
+    _expanded_prior = set()
+    _expanded_current = set()
+
+    def replace_include_tracked(match):
+        file_path = match.group(1).strip()
+        try:
+            full_path = get_file_path(file_path)
+            resolved = os.path.realpath(full_path)
+            if resolved in _expanded_prior:
+                raise ValueError(f"Circular include detected: {file_path} is already in the include chain")
+            _expanded_current.add(resolved)
+        except (FileNotFoundError, ValueError):
+            raise
+        except Exception:
+            pass
+        return replace_include(match)
+
     prev_text = ""
     current_text = text
     while prev_text != current_text:
         prev_text = current_text
-        current_text = re.sub(pattern, replace_include, current_text, flags=re.DOTALL)
+        _expanded_current = set()
+        current_text = re.sub(pattern, replace_include_tracked, current_text, flags=re.DOTALL)
+        _expanded_prior.update(_expanded_current)
     return current_text
 
 def process_xml_tags(text: str, recursive: bool, _seen: Optional[set] = None) -> str:
@@ -290,16 +309,35 @@ def replace_include(match):
             console.print(f"[bold red]Error processing include:[/bold red] {str(e)}")
             _dbg(f"Error processing XML include {file_path}: {e}")
             return f"[Error processing include: {file_path}]"
+    _expanded_prior = set()
+    _expanded_current = set()
+
+    def replace_include_tracked(match):
+        file_path = match.group(1).strip()
+        try:
+            full_path = get_file_path(file_path)
+            resolved = os.path.realpath(full_path)
+            if resolved in _expanded_prior:
+                raise ValueError(f"Circular include detected: {file_path} is already in the include chain")
+            _expanded_current.add(resolved)
+        except (FileNotFoundError, ValueError):
+            raise
+        except Exception:
+            pass
+        return replace_include(match)
+
     prev_text = ""
     current_text = text
     while prev_text != current_text:
         prev_text = current_text
+        _expanded_current = set()
         code_spans = _extract_code_spans(current_text)
         def replace_include_with_spans(match):
             if _intersects_any_span(match.start(), match.end(), code_spans):
                 return match.group(0)
-            return replace_include(match)
+            return replace_include_tracked(match)
         current_text = re.sub(pattern, replace_include_with_spans, current_text, flags=re.DOTALL)
+        _expanded_prior.update(_expanded_current)
     return current_text
 
 def process_pdd_tags(text: str) -> str:

tests/test_circular_includes.py

@@ -1,19 +1,42 @@
 """
-Tests for circular <include> detection in pdd/preprocess.py (Issue #521).
+Tests for circular <include> detection in pdd/preprocess.py (Issues #521, #553).
 
-The preprocessor has no cycle detection — circular includes recurse ~82 times
-until Python's recursion limit, then the broad `except Exception` swallows
-the RecursionError and returns corrupted output with exit code 0.
+Issue #521: Circular includes in the recursive path must be detected.
+Issue #553: Circular includes in the non-recursive (default) path must also
+be detected. The _seen set is only populated inside `if recursive:` blocks,
+so the non-recursive convergence loop has no cycle guard and loops forever.
 
 These tests verify that circular includes produce an error (exception or
 error marker in output), NOT silently corrupted content.
 """
 
 import os
+import signal
 import pytest
 from unittest.mock import patch, mock_open, MagicMock
 from pdd.preprocess import preprocess
 
+
+def _timeout_handler(signum, frame):
+    raise TimeoutError("Test timed out — possible infinite loop")
+
+
+def _with_timeout(seconds=10):
+    """Decorator that enforces a real timeout using SIGALRM (Unix only)."""
+    def decorator(func):
+        def wrapper(*args, **kwargs):
+            old_handler = signal.signal(signal.SIGALRM, _timeout_handler)
+            signal.alarm(seconds)
+            try:
+                return func(*args, **kwargs)
+            finally:
+                signal.alarm(0)
+                signal.signal(signal.SIGALRM, old_handler)
+        wrapper.__name__ = func.__name__
+        wrapper.__doc__ = func.__doc__
+        return wrapper
+    return decorator
+
 # Store original so we can restore
 _original_pdd_path = os.environ.get('PDD_PATH')
 
@@ -194,3 +217,135 @@ def test_diamond_includes_not_falsely_flagged(self):
             assert 'C' in result
             # D is included twice (via B and via C) — that's fine, not circular
             assert result.count('Shared') == 2
+
+
+class TestCircularIncludesNonRecursive:
+    """Issue #553: Circular includes must be detected in the non-recursive (default) path.
+
+    PR #528 added cycle detection for recursive=True, but the non-recursive path
+    (which uses a while-loop for convergence) never populates the _seen set,
+    so circular includes cause an infinite loop instead of raising ValueError.
+    """
+
+    def setup_method(self):
+        set_pdd_path('/mock/path')
+
+    def teardown_method(self):
+        if _original_pdd_path is not None:
+            os.environ['PDD_PATH'] = _original_pdd_path
+        elif 'PDD_PATH' in os.environ:
+            del os.environ['PDD_PATH']
+
+    @_with_timeout(10)
+    def test_circular_xml_includes_non_recursive_must_error(self):
+        """A→B→A circular XML include with recursive=False must raise ValueError, not loop forever."""
+        file_map = {
+            'a.txt': 'Hello\n<include>b.txt</include>',
+            'b.txt': 'World\n<include>a.txt</include>',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            # Buggy code: infinite loop in the while-loop → timeout → test fails
+            # Fixed code: ValueError("Circular include detected: ...") → test passes
+            with pytest.raises(ValueError, match="Circular include detected"):
+                preprocess(
+                    '<include>a.txt</include>',
+                    recursive=False,
+                    double_curly_brackets=False,
+                )
+
+    @_with_timeout(10)
+    def test_circular_backtick_includes_non_recursive_must_error(self):
+        """A→B→A circular backtick include with recursive=False must raise ValueError."""
+        file_map = {
+            'x.txt': 'Foo\n```<y.txt>```',
+            'y.txt': 'Bar\n```<x.txt>```',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            # Buggy code: infinite loop in process_backtick_includes while-loop
+            # Fixed code: ValueError raised
+            with pytest.raises(ValueError, match="Circular include detected"):
+                preprocess(
+                    '```<x.txt>```',
+                    recursive=False,
+                    double_curly_brackets=False,
+                )
+
+    @_with_timeout(10)
+    def test_self_referencing_include_non_recursive_must_error(self):
+        """A file that includes itself must be detected in non-recursive mode."""
+        file_map = {
+            'self.txt': 'Content\n<include>self.txt</include>',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            with pytest.raises(ValueError, match="Circular include detected"):
+                preprocess(
+                    '<include>self.txt</include>',
+                    recursive=False,
+                    double_curly_brackets=False,
+                )
+
+    @_with_timeout(10)
+    def test_three_file_cycle_non_recursive_must_error(self):
+        """A→B→C→A three-file cycle must be detected in non-recursive mode."""
+        file_map = {
+            'a.txt': 'AAA\n<include>b.txt</include>',
+            'b.txt': 'BBB\n<include>c.txt</include>',
+            'c.txt': 'CCC\n<include>a.txt</include>',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            with pytest.raises(ValueError, match="Circular include detected"):
+                preprocess(
+                    '<include>a.txt</include>',
+                    recursive=False,
+                    double_curly_brackets=False,
+                )
+
+    def test_non_circular_includes_non_recursive_still_work(self):
+        """Non-circular includes (A→B→C, no cycle) must still work in non-recursive mode."""
+        file_map = {
+            'top.txt': 'Top\n<include>mid.txt</include>',
+            'mid.txt': 'Mid\n<include>leaf.txt</include>',
+            'leaf.txt': 'Leaf',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            result = preprocess(
+                '<include>top.txt</include>',
+                recursive=False,
+                double_curly_brackets=False,
+            )
+
+            assert 'Top' in result
+            assert 'Mid' in result
+            assert 'Leaf' in result
+
+    def test_diamond_includes_non_recursive_not_falsely_flagged(self):
+        """Diamond pattern (A→B, A→C, B→D, C→D) is NOT circular and must work."""
+        file_map = {
+            'a.txt': '<include>b.txt</include>\n<include>c.txt</include>',
+            'b.txt': 'B\n<include>d.txt</include>',
+            'c.txt': 'C\n<include>d.txt</include>',
+            'd.txt': 'Shared',
+        }
+        with patch('builtins.open', mock_open()) as m:
+            m.side_effect = _make_mock_open(file_map)
+
+            result = preprocess(
+                '<include>a.txt</include>',
+                recursive=False,
+                double_curly_brackets=False,
+            )
+
+            assert 'B' in result
+            assert 'C' in result
+            # D is included twice (via B and via C) — that's fine, not circular
+            assert result.count('Shared') == 2