diff --git a/tests/test_e2e_issue_8_template_injection.py b/tests/test_e2e_issue_8_template_injection.py new file mode 100644 index 000000000..985d3407b --- /dev/null +++ b/tests/test_e2e_issue_8_template_injection.py @@ -0,0 +1,415 @@ +""" +E2E Test for Issue #8: Template injection bug in pdd change workflow + +This E2E test exercises the full `pdd change` CLI workflow to verify the bug where +templates with directives fail when the included content contains JSON braces. + +User-Facing Impact: +- Users run `pdd change` workflow +- Workflow processes steps 1-4 successfully +- Step 5 loads template with docs/prompting_guide.md +- If prompting_guide.md contains JSON examples with braces, the workflow crashes +- Error: "Context missing key for step 5: '\n \"type\"'" +- Users waste ~$1.27-$1.57 per failed attempt +- Workflow is completely blocked + +Root Cause (from Step 5 analysis): +1. Step 5's template contains: docs/prompting_guide.md +2. load_prompt_template() does NOT call preprocess() to expand includes +3. When .format(**context) is called at orchestrator.py:521, includes are still raw +4. Later, when includes are processed, JSON braces like {"type": "error"} are interpreted + as template variables +5. Since '\n "type"' doesn't exist in context, KeyError is raised + +This E2E test: +1. Simulates the full orchestrator workflow from the CLI perspective +2. Runs through step 5 with a template that has directives +3. Verifies the workflow doesn't crash with KeyError +4. Tests the actual user-facing command path, not just internal functions + +The test should FAIL on buggy code (KeyError at step 5) and PASS once the fix is applied. +""" + +import pytest +from pathlib import Path +from unittest.mock import patch, MagicMock, AsyncMock +import tempfile +import os + + +@pytest.fixture +def mock_repo(tmp_path): + """Create a mock repository structure with git and .pdd directory.""" + # Create .git to simulate a git repo + (tmp_path / ".git").mkdir() + + # Create .pdd directory + (tmp_path / ".pdd").mkdir() + + # Create pdd/prompts directory structure + prompts_dir = tmp_path / "pdd" / "prompts" + prompts_dir.mkdir(parents=True) + + # Create docs directory + docs_dir = tmp_path / "docs" + docs_dir.mkdir(parents=True) + + return tmp_path + + +@pytest.fixture +def step5_template_with_include(tmp_path): + """Create a simplified Step 5 template with directive.""" + template_content = """% Step 5: Documentation Changes Analysis + +You are analyzing documentation changes needed for the issue. + + +docs/prompting_guide.md + + +% Issue Details +- URL: {issue_url} +- Title: {issue_title} + +% Previous Steps +Step 1: {step1_output} +Step 2: {step2_output} +Step 3: {step3_output} +Step 4: {step4_output} + +% Task +Analyze what documentation changes are needed for this issue. +""" + + prompts_dir = tmp_path / "pdd" / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + + template_path = prompts_dir / "agentic_change_step5_docs_change_LLM.prompt" + template_path.write_text(template_content) + + return template_path + + +@pytest.fixture +def prompting_guide_with_json(tmp_path): + """Create prompting_guide.md with JSON content that triggers the bug.""" + # This is the exact content pattern that causes the bug + guide_content = """# PDD Prompting Guide + +## Example Output Format + +When reporting errors, use this format: + +```json +{ + "type": "error", + "message": "Description of the error", + "code": 500 +} +``` + +## Another Example + +Module structure: +```json +{ + "type": "module", + "name": "test_module" +} +``` +""" + + docs_dir = tmp_path / "docs" + docs_dir.mkdir(parents=True, exist_ok=True) + + guide_path = docs_dir / "prompting_guide.md" + guide_path.write_text(guide_content) + + return guide_path + + +class TestIssue8TemplateInjectionE2E: + """ + E2E tests for Issue #8: Verify pdd change workflow handles templates with includes. + + These tests exercise the full orchestrator code path that a user would hit when + running `pdd change`, including template loading, preprocessing, and formatting. + """ + + def test_orchestrator_step5_with_include_directive( + self, mock_repo, step5_template_with_include, prompting_guide_with_json + ): + """ + E2E Test: Orchestrator should process Step 5 template with without KeyError. + + This test simulates the exact user workflow from Issue #8: + 1. User runs pdd change workflow + 2. Steps 1-4 complete successfully + 3. Step 5 loads template with docs/prompting_guide.md + 4. prompting_guide.md contains JSON examples with braces + 5. orchestrator calls load_prompt_template() and then .format(**context) + + Expected behavior (after fix): + - load_prompt_template() preprocesses includes + - Braces in included JSON are escaped + - .format(**context) succeeds without KeyError + - Workflow continues to completion + + Bug behavior (Issue #8): + - load_prompt_template() returns raw template with unprocessed + - When includes are processed later, JSON braces are interpreted as variables + - KeyError: '\n "type"' is raised at orchestrator.py:521 + - Workflow fails with "Context missing key for step 5: '\n "type"'" + """ + from pdd.load_prompt_template import load_prompt_template + + # Change to the mock repo directory + old_cwd = os.getcwd() + try: + os.chdir(mock_repo) + + # Build the context that the orchestrator would pass to Step 5 + # This mimics agentic_change_orchestrator.py lines 401-413 + context = { + "issue_url": "https://github.com/Serhan-Asad/pdd/issues/8", + "issue_title": "[TEST] Bug: pdd change fails at step 5 due to template injection", + "repo_owner": "Serhan-Asad", + "repo_name": "pdd", + "issue_number": 8, + "issue_author": "Serhan-Asad", + "issue_content": "Test issue for template injection bug", + } + + # Add outputs from previous steps (steps 1-4) + context["step1_output"] = "Step 1: Duplicate check - No duplicates found" + context["step2_output"] = "Step 2: Documentation check - Not documented" + context["step3_output"] = "Step 3: Research - Issue is valid" + context["step4_output"] = "Step 4: Requirements clear - Ready to proceed" + + # Load the Step 5 template - step 5 uses "docs_change" as the step name + template_name = "agentic_change_step5_docs_change_LLM" + + # Mock the resolver to use our test template + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + mock_resolver_instance = MagicMock() + mock_resolver_instance.resolve_prompt_template.return_value = step5_template_with_include + mock_resolver.return_value = mock_resolver_instance + + prompt_template = load_prompt_template(template_name) + assert prompt_template is not None, f"Failed to load template: {template_name}" + + # BUG CHECK 1: The include should be processed (expanded) + # If it's not processed, we'll see the raw tag + if "docs/prompting_guide.md" in prompt_template: + pytest.fail( + "BUG DETECTED (Issue #8): load_prompt_template() returned template with " + "unprocessed directive.\n\n" + "Root cause: load_prompt_template() does NOT call preprocess().\n\n" + "Current behavior: Template contains raw 'docs/prompting_guide.md'\n" + "Expected behavior: Include should be expanded to the content of prompting_guide.md\n\n" + "This is the root cause of the template injection bug - includes are not processed,\n" + "leading to potential KeyError when the template is formatted." + ) + + # If we got here, includes SHOULD have been processed + # BUG CHECK 2: Verify the included content is present + if "PDD Prompting Guide" not in prompt_template: + pytest.fail( + "BUG DETECTED (Issue #8): Template doesn't contain included content.\n\n" + "The docs/prompting_guide.md directive was not expanded.\n" + "Expected: Included file content to be present in the template\n" + "Actual: No content from prompting_guide.md found\n\n" + "This confirms load_prompt_template() is NOT calling preprocess()." + ) + + # BUG CHECK 3: Format the template and check for KeyError + # This is where the orchestrator fails at line 521 + try: + formatted_prompt = prompt_template.format(**context) + except KeyError as e: + # Extract the key that caused the error + error_key = str(e.args[0]) if e.args else "unknown" + + pytest.fail( + f"BUG DETECTED (Issue #8): KeyError '{error_key}' raised when formatting " + f"Step 5 prompt template.\n\n" + f"This is the exact error users see when running 'pdd change':\n" + f" Error: Context missing key for step 5: {e}\n\n" + f"Root cause:\n" + f"1. Step 5 template has docs/prompting_guide.md\n" + f"2. prompting_guide.md contains JSON like {{\"type\": \"error\"}}\n" + f"3. load_prompt_template() preprocessed includes BUT didn't escape braces\n" + f"4. When .format(**context) runs, it interprets JSON braces as variables\n" + f"5. Since '{error_key}' is not in context, KeyError is raised\n\n" + f"Expected fix: preprocess() should escape braces in included content\n\n" + f"User impact:\n" + f" - Workflow fails at step 5 (100% failure rate)\n" + f" - ~$1.27-$1.57 wasted per attempt\n" + f" - No workaround available" + ) + + # Verify the formatted prompt is valid + assert formatted_prompt is not None, "Formatted prompt should not be None" + + # Verify context variables were substituted + assert "https://github.com/Serhan-Asad/pdd/issues/8" in formatted_prompt, \ + "issue_url should be substituted in template" + assert "Step 1: Duplicate check" in formatted_prompt, \ + "step1_output should be substituted in template" + + # Verify JSON from included file has proper braces (no KeyError means escaping worked) + # The JSON content should be present as literal text, not interpreted as variables + assert "type" in formatted_prompt, \ + "JSON content from included file should be present" + + finally: + os.chdir(old_cwd) + + def test_orchestrator_full_context_with_include( + self, mock_repo, step5_template_with_include, prompting_guide_with_json + ): + """ + E2E Test: Verify template formatting works with full orchestrator context. + + This test ensures that when load_prompt_template() properly preprocesses + includes, the template still works correctly with all the variables that + the orchestrator provides. + """ + from pdd.load_prompt_template import load_prompt_template + + old_cwd = os.getcwd() + try: + os.chdir(mock_repo) + + # Build a more complete context like the real orchestrator would + context = { + "issue_url": "https://github.com/Serhan-Asad/pdd/issues/8", + "issue_title": "[TEST] Template injection bug", + "repo_owner": "Serhan-Asad", + "repo_name": "pdd", + "issue_number": 8, + "issue_author": "Serhan-Asad", + "issue_content": "Template injection test", + "step1_output": "Duplicate check completed", + "step2_output": "Docs check completed", + "step3_output": "Research completed", + "step4_output": "Requirements clarified", + # Additional context that might be present + "pddrc_content": "pddrc configuration", + } + + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + mock_resolver_instance = MagicMock() + mock_resolver_instance.resolve_prompt_template.return_value = step5_template_with_include + mock_resolver.return_value = mock_resolver_instance + + template = load_prompt_template("agentic_change_step5_docs_change_LLM") + assert template is not None + + # BUG CHECK: Verify included content is present (confirms preprocessing happened) + if "PDD Prompting Guide" not in template: + pytest.fail( + "BUG DETECTED (Issue #8): Template doesn't contain included content.\n\n" + "The docs/prompting_guide.md directive was not expanded.\n" + "This confirms load_prompt_template() is NOT calling preprocess()." + ) + + # This should NOT raise KeyError + try: + formatted = template.format(**context) + except KeyError as e: + pytest.fail( + f"BUG DETECTED (Issue #8): Template formatting failed with KeyError: {e}\n\n" + f"This indicates that either:\n" + f"1. directives are not being preprocessed, OR\n" + f"2. Braces in included content are not being properly escaped\n\n" + f"The user would see this error when running 'pdd change' at step 5." + ) + + # Verify all substitutions worked + assert "https://github.com/Serhan-Asad/pdd/issues/8" in formatted + assert "Duplicate check completed" in formatted + assert "Requirements clarified" in formatted + + finally: + os.chdir(old_cwd) + + def test_multiple_includes_in_template(self, mock_repo): + """ + E2E Test: Verify templates with multiple directives work correctly. + + This is a regression test to ensure the fix handles: + - Multiple includes in one template + - Includes with different content types + - Proper escaping across all included content + """ + old_cwd = os.getcwd() + try: + os.chdir(mock_repo) + + # Create template with multiple includes + template_content = """% Multi-include template + + +docs/guide1.md + + + +docs/guide2.md + + +Context: {context_var} +""" + + prompts_dir = mock_repo / "pdd" / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + template_path = prompts_dir / "test_multi_include.prompt" + template_path.write_text(template_content) + + # Create two include files with JSON + docs_dir = mock_repo / "docs" + docs_dir.mkdir(parents=True, exist_ok=True) + + guide1 = docs_dir / "guide1.md" + guide1.write_text('# Guide 1\n```json\n{"section": "one"}\n```') + + guide2 = docs_dir / "guide2.md" + guide2.write_text('# Guide 2\n```json\n{"section": "two"}\n```') + + from pdd.load_prompt_template import load_prompt_template + + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + mock_resolver_instance = MagicMock() + mock_resolver_instance.resolve_prompt_template.return_value = template_path + mock_resolver.return_value = mock_resolver_instance + + template = load_prompt_template("test_multi_include") + assert template is not None + + # BUG CHECK: Verify includes were processed + if "Guide 1" not in template or "Guide 2" not in template: + pytest.fail( + "BUG DETECTED (Issue #8): Multiple directives not expanded.\n\n" + "load_prompt_template() should preprocess ALL includes in the template." + ) + + # Format with context + context = {"context_var": "test_value"} + + try: + formatted = template.format(**context) + except KeyError as e: + pytest.fail( + f"BUG: Multiple includes caused KeyError: {e}\n\n" + f"When templates have multiple directives, all must be " + f"preprocessed and all braces must be properly escaped." + ) + + # Verify variable substitution worked + assert "test_value" in formatted + + finally: + os.chdir(old_cwd) + + diff --git a/tests/test_issue_8_template_injection.py b/tests/test_issue_8_template_injection.py new file mode 100644 index 000000000..9f125a016 --- /dev/null +++ b/tests/test_issue_8_template_injection.py @@ -0,0 +1,293 @@ +""" +E2E Test for Issue #8: Template injection bug due to unprocessed directives + +This test suite verifies the bug where load_prompt_template() fails to preprocess +directives before returning, causing issues when included files contain content with braces. + +Root Cause (identified in Issue #8 Step 5): +1. Step 5's template contains: docs/prompting_guide.md +2. load_prompt_template() only reads the file - it does NOT call preprocess() to process includes +3. When the orchestrator later calls .format(**context), unprocessed includes remain in the template +4. If includes are eventually processed and contain JSON like {"type": "module"}, those braces + may cause issues depending on the processing order + +The Bug Location: +- pdd/load_prompt_template.py: load_prompt_template() function doesn't call preprocess() +- Expected: load_prompt_template() should call preprocess() to expand directives + +This test should FAIL on buggy code (include directives not processed) and PASS once +load_prompt_template() is fixed to call preprocess(). +""" + +import pytest +from pathlib import Path +from unittest.mock import patch, MagicMock +import os + + +class TestIssue8TemplateInjection: + """ + Tests for Issue #8: Verify template injection bug due to unprocessed includes. + + These tests exercise the load_prompt_template() function to verify that it properly + preprocesses directives before returning the template. + """ + + def test_load_prompt_template_should_preprocess_includes(self, tmp_path): + """ + Unit Test: load_prompt_template() should call preprocess() to expand directives. + + This is the PRIMARY test for Issue #8. It verifies that load_prompt_template() + preprocesses tags before returning. + + Expected behavior (after fix): + - load_prompt_template() calls preprocess() before returning + - directives are expanded + - Returned template has no tags + + Bug behavior (Issue #8): + - load_prompt_template() returns raw file content + - tags remain in the template + """ + # Create a minimal template with an include directive + prompts_dir = tmp_path / "pdd" / "prompts" + prompts_dir.mkdir(parents=True) + + template_content = "Before include\ntest.txt\nAfter include\nVariable: {myvar}" + template_path = prompts_dir / "test_template.prompt" + template_path.write_text(template_content) + + # Create the file to be included with JSON content that has braces + test_file = tmp_path / "test.txt" + test_file.write_text('INCLUDED: {"key": "value"}') + + # Set up environment + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + + # Load the template + from pdd.load_prompt_template import load_prompt_template + + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + mock_resolver_instance = MagicMock() + mock_resolver_instance.resolve_prompt_template.return_value = template_path + mock_resolver.return_value = mock_resolver_instance + + template = load_prompt_template("test_template") + assert template is not None + + # BUG CHECK: The include directive should NOT be in the returned template + # After the fix, preprocess() will have expanded it + if "" in template: + pytest.fail( + "BUG DETECTED (Issue #8): load_prompt_template() returned template with " + "unprocessed directive.\n\n" + "The function should call preprocess() to expand includes before returning.\n\n" + f"Template content:\n{template}\n\n" + "Expected includes to be expanded.\n" + f"Actual: template still contains '' tags" + ) + + # If we got here, includes were processed! Now verify the template works correctly + # Template variables like {myvar} should still work with .format() + try: + formatted = template.format(myvar="TEST_VALUE") + + # After formatting, we should have both the included content AND the variable substituted + assert "INCLUDED:" in formatted, "Include content should be present" + assert "TEST_VALUE" in formatted, "Template variable should be substituted" + + # The JSON from the included file should have single braces (properly escaped during include) + # After .format(), double braces {{ should become single braces { + assert '{"key": "value"}' in formatted, "JSON from included file should have single braces after formatting" + + except KeyError as e: + pytest.fail( + f"BUG: After loading template, .format() raised KeyError: {e}\n\n" + f"This means either template variables weren't preserved correctly during preprocessing,\n" + f"or braces in included content weren't properly escaped." + ) + + finally: + os.chdir(old_cwd) + + def test_step5_template_has_unprocessed_include(self, tmp_path): + """ + Regression Test: Verify that Step 5 template's directive is processed. + + This test simulates the exact scenario from Issue #8: + - Step 5 template contains docs/prompting_guide.md + - prompting_guide.md contains JSON examples + - load_prompt_template() should preprocess the include + + Bug behavior: tag remains in template + Expected behavior: Include is expanded and braces are escaped + """ + # Set up the exact file structure from the bug report + prompts_dir = tmp_path / "pdd" / "prompts" + prompts_dir.mkdir(parents=True) + + # Step 5 template (simplified version of the real one) + step5_template = """% Step 5: Documentation Changes + + +docs/prompting_guide.md + + +Issue: {issue_url} +Previous output: {step4_output} +""" + + template_path = prompts_dir / "agentic_change_step5_docs_change_LLM.prompt" + template_path.write_text(step5_template) + + # prompting_guide.md with JSON (the trigger from Issue #8) + docs_dir = tmp_path / "docs" + docs_dir.mkdir(parents=True) + + guide_content = """# Prompting Guide + +Example output format: +```json +{ + "type": "error", + "message": "test" +} +``` +""" + + guide_path = docs_dir / "prompting_guide.md" + guide_path.write_text(guide_content) + + # Set up environment + old_cwd = os.getcwd() + try: + os.chdir(tmp_path) + + # Load template + from pdd.load_prompt_template import load_prompt_template + + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + mock_resolver_instance = MagicMock() + mock_resolver_instance.resolve_prompt_template.return_value = template_path + mock_resolver.return_value = mock_resolver_instance + + template = load_prompt_template("agentic_change_step5_docs_change_LLM") + assert template is not None + + # BUG CHECK: Include should be processed + if "docs/prompting_guide.md" in template: + pytest.fail( + "BUG DETECTED (Issue #8): Step 5 template has unprocessed directive.\n\n" + "load_prompt_template() should have called preprocess() to expand the include.\n\n" + "This is the root cause of the template injection bug in Issue #8." + ) + + # If preprocessing worked, the guide content should be in the template + assert "Prompting Guide" in template, \ + "After preprocessing, included content should be in template" + + # Now verify formatting works without errors + context = { + "issue_url": "https://github.com/Serhan-Asad/pdd/issues/8", + "step4_output": "Step 4 completed", + } + + try: + formatted = template.format(**context) + + # Verify variables were substituted + assert "https://github.com/Serhan-Asad/pdd/issues/8" in formatted + assert "Step 4 completed" in formatted + + # Verify JSON from included file has proper single braces + assert '"type": "error"' in formatted + assert '"message": "test"' in formatted + + except KeyError as e: + pytest.fail( + f"BUG: Formatting failed with KeyError: {e}\n\n" + f"This means braces in the included content weren't properly escaped." + ) + + finally: + os.chdir(old_cwd) + + +class TestIssue8RootCauseVerification: + """ + Root cause verification tests for Issue #8. + + These tests verify the specific function behavior that causes the bug: + load_prompt_template() not calling preprocess(). + """ + + def test_load_prompt_template_should_call_preprocess(self): + """ + Specification Test: load_prompt_template() should call preprocess(). + + This test verifies through mocking that load_prompt_template() calls preprocess() + on the template content before returning it. + + Current behavior (BUG): + - load_prompt_template() only reads the file and returns raw content + - It does NOT call preprocess() + - This leaves directives unprocessed + + Expected behavior (AFTER FIX): + - load_prompt_template() imports and calls preprocess() + - directives are expanded + - Braces in included content are properly escaped + - The returned template is ready for .format(**context) + """ + from unittest.mock import mock_open + + # Create a mock template content + template_content = "Test template with file.txt" + + # Set up mocks + with patch('builtins.open', mock_open(read_data=template_content)): + with patch('pdd.load_prompt_template.get_default_resolver') as mock_resolver: + # Set up resolver to return a valid path + mock_resolver_instance = MagicMock() + mock_path = Path("/fake/path/template.prompt") + mock_resolver_instance.resolve_prompt_template.return_value = mock_path + mock_resolver.return_value = mock_resolver_instance + + # Try to patch preprocess - if it's not imported, this will fail + try: + with patch('pdd.load_prompt_template.preprocess') as mock_preprocess: + mock_preprocess.return_value = "Preprocessed content" + + from pdd.load_prompt_template import load_prompt_template + result = load_prompt_template("test_template") + + # Verify preprocess was called + if not mock_preprocess.called: + pytest.fail( + "BUG DETECTED (Issue #8): load_prompt_template() does NOT call preprocess().\n\n" + "Root cause: The function at pdd/load_prompt_template.py loads the file " + "but returns raw content without preprocessing directives.\n\n" + "Expected behavior: load_prompt_template() should call:\n" + " from pdd.preprocess import preprocess\n" + " preprocessed = preprocess(prompt_template)\n" + " return preprocessed\n\n" + "This causes templates with directives to fail when " + "included files contain JSON or other content with braces." + ) + + # After fix, result should be the preprocessed content + assert result == "Preprocessed content", \ + "load_prompt_template should return preprocessed content" + + except AttributeError: + # preprocess is not imported in load_prompt_template + pytest.fail( + "BUG DETECTED (Issue #8): load_prompt_template() does NOT import preprocess.\n\n" + "Root cause: The module pdd/load_prompt_template.py doesn't import preprocess from pdd.preprocess.\n\n" + "Expected fix:\n" + "1. Add import: from pdd.preprocess import preprocess\n" + "2. Call it before returning: return preprocess(prompt_template)\n\n" + "This is the core issue preventing directives from being processed." + )