Resource leak: Temp directories not cleaned up when git clone fails

[Serhan-Asad]

Description

When git clone fails in _setup_repository(), the temporary directory created with tempfile.mkdtemp() is not cleaned up, leading to a disk space leak.

Location

File: pdd/agentic_change.py:111-132

Root Cause

# Line 112: Creates temp directory
temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))

# Lines 120-131: If clone fails, raises exception without cleanup
try:
    result = subprocess.run(["gh"] + clone_cmd, ...)
    if result.returncode != 0:
        raise RuntimeError(...)  # Exits without cleaning temp_dir
except Exception as e:
    raise RuntimeError(...)  # Exits without cleaning temp_dir

Reproduction

We tested this manually and confirmed the leak:

# Test showed temp directory created but not deleted on clone failure
temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))
# ... clone fails ...
# Directory still exists: True ⚠️

Scenarios That Trigger The Leak

1. Network interruption during clone
2. Git authentication failures
3. Rate limiting
4. Private repositories without proper credentials
5. Non-existent repositories (after issue API call succeeds)
6. Disk space issues during clone

Proposed Solutions

Option 1: Use TemporaryDirectory context manager (Recommended)

with tempfile.TemporaryDirectory(prefix=f"pdd_{repo}_") as temp_dir_str:
    temp_dir = Path(temp_dir_str)
    # Clone operation
    # Automatic cleanup on any exception

Note: This requires restructuring the function since the temp directory is deleted when exiting the context.

Option 2: Manual cleanup in exception handlers

temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))
try:
    # Clone operation
    if result.returncode != 0:
        shutil.rmtree(temp_dir)  # Cleanup before raising
        raise RuntimeError(...)
except Exception as e:
    shutil.rmtree(temp_dir)  # Cleanup before raising
    raise RuntimeError(...)

[Serhan-Asad]

Step 1: Duplicate Check

Status: No duplicates found

Search Performed

• Searched for: "temp directory cleanup", "resource leak", "git clone", "tempfile mkdtemp", "agentic_change.py", "_setup_repository"
• Issues reviewed: 300+ issues across multiple search queries
• Checked both open and closed issues

Findings

After thorough searching, this appears to be a new issue. While I found one closed resource leak issue (#403 - File Handle Resource Leak in SyncLock.acquire()), it is fundamentally different:

• Bug: File Handle Resource Leak in SyncLock.acquire() #403: File descriptor leak in sync_determine_operation.py (already closed)
• Resource leak: Temp directories not cleaned up when git clone fails #418: Temporary directory leak in agentic_change.py (this issue)

The two issues involve different resources (file handles vs directories), different files, different functions, and different root causes. No other issues mention:

• The _setup_repository() function
• Temp directory cleanup problems in agentic_change.py
• tempfile.mkdtemp() resource leaks when git clone fails

Conclusion

This is a unique bug report. The issue is well-documented with:

• Clear location information (pdd/agentic_change.py:111-132)
• Confirmed reproduction
• Multiple trigger scenarios identified
• Proposed solutions

---

Proceeding to Step 2: Documentation Check

[Serhan-Asad]

Step 2: Documentation Check

Status: Confirmed Bug

Documentation Reviewed

• README.md (comprehensive project documentation)
• CONTRIBUTING.md (contribution guidelines and quality standards)
• CHANGELOG.md (project history and previous bug fixes)
• docs/faq.md and other documentation files
• Source code in pdd/agentic_change.py (lines 77-133)
• Related test files and resource management patterns in the codebase

Findings

This is a confirmed bug - the reported behavior contradicts established patterns and best practices in the codebase.

Evidence supporting this is a bug:

1. No documented exception to cleanup requirements: The documentation doesn't indicate that temp directories should persist after errors. The project emphasizes clean code practices and quality standards.

2. Project history shows resource leaks are bugs: The codebase recently fixed a similar resource leak issue (Bug: File Handle Resource Leak in SyncLock.acquire() #403 - file handle leaks), demonstrating that resource leaks are treated as bugs, not expected behavior.

3. Inconsistent with codebase patterns: Other parts of the codebase properly clean up temporary directories:

   • pdd/agentic_bug_orchestrator.py:152 - uses shutil.rmtree(worktree_path)
   • pdd/agentic_test.py:162 - uses shutil.rmtree(temp_dir, ignore_errors=True)
   • pdd/agentic_test_orchestrator.py:147 - uses shutil.rmtree(worktree_path)
   • pdd/agentic_change_orchestrator.py:158,163 - uses shutil.rmtree(worktree_path)

4. Violates Python best practices: Python's tempfile module documentation and best practices recommend using TemporaryDirectory context manager or explicit cleanup with shutil.rmtree() in exception handlers.

5. User-facing impact: As reported, this causes disk space leaks in multiple realistic scenarios:

   • Network interruptions during clone
   • Authentication failures
   • Rate limiting
   • Private repositories without proper credentials
   • Non-existent repositories
   • Disk space issues during clone

Code Analysis:

# Line 112: Creates temp directory
temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))

# Lines 128-131: Exception raised without cleanup
if result.returncode != 0:
    raise RuntimeError(...)  # ❌ temp_dir never cleaned up
except Exception as e:
    raise RuntimeError(...)  # ❌ temp_dir never cleaned up

Conclusion

The reported behavior is a bug that should be fixed. The temporary directory cleanup should be guaranteed even when git clone fails, following the same patterns used elsewhere in the codebase.

---

Proceeding to Step 3: Triage

[Serhan-Asad]

Step 3: Triage

Status: Proceed

Summary

We have sufficient information to attempt reproduction. Here's what we understand:

Expected Behavior:

• Temporary directories created during git clone operations should be cleaned up when the operation fails
• No disk space should be leaked due to abandoned temp directories

Actual Behavior:

• When git clone fails in _setup_repository() (pdd/agentic_change.py:111-132), the temp directory created at line 112 is never cleaned up
• The function raises exceptions at lines 129 and 131 without cleanup, leaving orphaned directories

Code Location:

• File: pdd/agentic_change.py
• Function: _setup_repository()
• Lines: 112 (temp dir creation), 128-131 (failure paths without cleanup)

Reproduction Scenarios Identified:

1. Network interruption during clone
2. Git authentication failures
3. API rate limiting
4. Private repositories without proper credentials
5. Non-existent repositories
6. Disk space issues during clone

Root Cause:

temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))  # Line 112
# ... 
if result.returncode != 0:
    raise RuntimeError(...)  # Line 129 - exits without cleanup
# ...
except Exception as e:
    raise RuntimeError(...)  # Line 131 - exits without cleanup

The issue has been confirmed through manual testing by the reporter and verified by code inspection. We can proceed to create a reproducible test case.

---

Proceeding to Step 4: Reproduction

[Serhan-Asad]

Step 4: Reproduction

Status: Reproduced ✓

Environment

• OS: macOS Darwin 24.3.0 (arm64)
• Python: 3.12.7
• GitHub CLI: 2.75.1

Steps Taken

1. Examined the affected code in pdd/agentic_change.py:111-132
2. Created a test script that simulates the _setup_repository() behavior with failing git clone
3. Ran the test with a non-existent repository to trigger clone failure
4. Created comprehensive unit tests to verify temp directory cleanup
5. Executed pytest to confirm the bug exists in both failure scenarios

Observed Behavior

Test 1: Simulated git clone failure

============================================================
Testing temp directory leak on failed git clone
============================================================

1. Created temp directory: /var/folders/.../T/pdd_nonexistent_repo_12345_fvzezale
   Directory exists: True

2. Git clone failed (as expected)
   Return code: 1
   Error: GraphQL: Could not resolve to a Repository with the name 'nonexistent_owner_12345/nonexistent_repo_1...

3. After exception, temp directory still exists: True
   ⚠️  BUG REPRODUCED: Temp directory was not cleaned up!
   Directory path: /var/folders/.../T/pdd_nonexistent_repo_12345_fvzezale

Test 2: Unit test failures

FAILED test_temp_dir_cleanup_on_clone_failure - AssertionError: True is not false : 
  Temp directory /var/folders/.../T/pdd_nonexistent_repo_zx1mdak2 was not cleaned up after clone failure.

FAILED test_temp_dir_cleanup_on_subprocess_exception - AssertionError: True is not false : 
  Temp directory /var/folders/.../T/pdd_test_repo_bdgeb41e was not cleaned up after subprocess exception.

Evidence of leaked directories:

$ ls -lhd /var/folders/.../T/pdd_*
drwx------@ 2 serhanasad  staff    64B Jan 28 11:44 pdd_nonexistent_repo_zx1mdak2
drwx------@ 2 serhanasad  staff    64B Jan 27 14:17 pdd_react_04d40tvw  # From previous run
drwx------@ 2 serhanasad  staff    64B Jan 28 11:44 pdd_test_repo_bdgeb41e

Affected Files

• pdd/agentic_change.py:112 - Creates temp directory with tempfile.mkdtemp()
• pdd/agentic_change.py:128-131 - Raises exceptions without cleanup

Notes

• The bug is reproduced in both failure scenarios:
  1. When subprocess.run returns non-zero (clone failure)
  2. When subprocess.run raises an exception (timeout, permission error, etc.)
• The leaked pdd_react_04d40tvw directory from Jan 27 proves this is a real-world issue
• Created failing unit tests that can be used to verify the fix
• The reproduction confirms the issue report is 100% accurate

---

Proceeding to Step 5: Root Cause Analysis

[Serhan-Asad]

Step 5: Root Cause Analysis

Root Cause Identified: Yes

Summary

The bug is a classic resource leak caused by missing cleanup code in the exception paths of _setup_repository(). When git clone fails (lines 128-131), the function raises a RuntimeError without cleaning up the temporary directory created on line 112. This leaves orphaned temp directories on disk that accumulate over time, consuming disk space.

Technical Details

• Location: pdd/agentic_change.py:112-131
• Problematic Code:

# Line 112: Creates temp directory
temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))

# Lines 120-131: Clone operation with no cleanup on failure
try:
    result = subprocess.run(["gh"] + clone_cmd, cwd=temp_dir, ...)
    if result.returncode != 0:
        raise RuntimeError(...)  # ❌ No cleanup before raising
except Exception as e:
    raise RuntimeError(...)  # ❌ No cleanup before raising

• Why it fails: The tempfile.mkdtemp() function creates a directory on disk that persists until explicitly deleted. When exceptions are raised on lines 129 or 131, the function exits immediately without calling shutil.rmtree(temp_dir), leaving the directory orphaned.

External Research

• Search terms: "Python tempfile.mkdtemp cleanup best practices exception handling", "Python TemporaryDirectory vs mkdtemp resource leak"
• Sources checked: Python official documentation, OpenStack security guidelines, Stack Overflow, Python bug tracker
• Relevant findings:
  • Python tempfile docs: "The user of mkdtemp() is responsible for deleting the temporary directory and its contents when done with it"
  • OpenStack security guidelines: Recommends using TemporaryDirectory context manager or manual cleanup in finally blocks
  • Best practice: Use TemporaryDirectory with context manager for automatic cleanup, or use try-finally with shutil.rmtree() for manual cleanup
  • Multiple sources emphasize that TemporaryDirectory is preferred over mkdtemp() specifically to prevent resource leaks

Repository History

• Relevant commits:

  • b8a4d5dc8 (2026-01-11) - "Bump version" - Introduced the entire agentic_change.py file including _setup_repository()
  • This was part of a major feature addition (598 lines added to agentic_change_orchestrator.py, 225 lines to agentic_change.py)

• Related PRs: No PRs found with "agentic_change" keyword (likely merged via main branch)

• Similar past issues:

  • Issue Bug: File Handle Resource Leak in SyncLock.acquire() #403 (closed 2026-01-27): "Bug: File Handle Resource Leak in SyncLock.acquire()"
  • This demonstrates the project treats resource leaks as bugs requiring fixes
  • Issue Bug: File Handle Resource Leak in SyncLock.acquire() #403 fixed file handle leaks; Resource leak: Temp directories not cleaned up when git clone fails #418 fixes directory leaks - same pattern, different resource

Experiments Performed

1. Compared with similar code in the codebase:

   • Found pdd/agentic_test.py:161-162 properly handles this scenario:

   except subprocess.CalledProcessError as e:
       # Clean up empty temp dir if clone failed
       shutil.rmtree(temp_dir, ignore_errors=True)
       return False, f"Failed to clone repository: {e}"

   • This proves the correct pattern is already known and used elsewhere in PDD

2. Verified the leak scenarios:

   • Step 4 reproduction confirmed leaked directories exist (e.g., pdd_react_04d40tvw from Jan 27)
   • All 6 scenarios in the issue (network failures, auth issues, non-existent repos, etc.) will trigger this leak

3. Tested pattern consistency:

   • Found multiple files using shutil.rmtree() for cleanup: agentic_bug_orchestrator.py, process_csv_change.py, agentic_change_orchestrator.py, etc.
   • The codebase has established patterns for temp directory cleanup that agentic_change.py doesn't follow

Fix Direction

Option 1: Match existing pattern in agentic_test.py (Recommended for consistency)

• Add shutil.rmtree(temp_dir, ignore_errors=True) before raising exceptions on lines 129 and 131
• Minimal change, matches existing codebase patterns
• Uses ignore_errors=True to handle edge cases (permissions issues, directory already deleted, etc.)

Option 2: Use TemporaryDirectory context manager (Python best practice)

• Replace mkdtemp() with tempfile.TemporaryDirectory() as a context manager
• Provides automatic cleanup even on exceptions
• Requires restructuring the function since temp_dir must stay in scope for the caller
• More complex change but eliminates the possibility of leaks

Both approaches are valid. Option 1 is simpler and maintains consistency with the existing codebase pattern used in agentic_test.py.

---

Proceeding to Step 5.5: Prompt Classification

[Serhan-Asad]

Step 5.5: Prompt Classification

Classification: Code Bug

DEFECT_TYPE: code

Analysis

This is a code bug, not a prompt defect. The code fails to implement proper resource management - a fundamental software engineering requirement that would be implicit in any reasonable prompt specification.

Evidence

• Implicit prompt requirement: Any prompt specifying "clone a repository" or "setup repository" implicitly requires proper resource cleanup on failure. Resource management is a fundamental non-functional requirement that doesn't need explicit specification.

• Code implements: Creates temp directory on line 112 with tempfile.mkdtemp(), but raises exceptions on lines 129 and 131 without cleanup, leaving orphaned directories.

• User expects: Temp directories to be cleaned up when git clone fails (explicit in issue description with 6 failure scenarios identified).

• Codebase pattern: Other modules already implement correct behavior - pdd/agentic_test.py:161-162 properly cleans up temp directories on clone failure using shutil.rmtree(temp_dir, ignore_errors=True).

Why This Is Not a Prompt Defect

1. Universal requirement: Resource cleanup is a universal software engineering principle, not something that needs explicit specification in every prompt.

2. Project standards: The codebase already demonstrates proper patterns (agentic_test.py), indicating this is the expected behavior.

3. No prompt would specify leak: No reasonable prompt would say "create temp directories and don't clean them up on error" - such behavior would be a defect in the prompt itself.

4. Implicit contracts: Like thread safety, memory safety, or exception safety, resource management is an implicit contract of well-written code.

Conclusion

The code does not correctly implement the implicit resource management requirements that any reasonable prompt would include. This is a classic implementation bug where proper cleanup logic was omitted in error paths.

Proceeding with test generation to capture this bug and enable pdd fix to correct the implementation.

---

Proceeding to Step 6: Test Plan

[Serhan-Asad]

Step 6: Test Plan

Existing Test Coverage

• Test file: tests/test_agentic_change.py
• Current coverage:
  • URL parsing (valid/invalid URLs)
  • API interactions (issue/comments fetching)
  • Repository setup scenarios (clone vs. current directory)
  • Clone failure handling (exists but doesn't verify cleanup)
  • Orchestrator invocation
• Gap: The existing test_clone_failure (line 312) verifies that clone failures raise errors correctly, but does NOT verify that the temporary directory is cleaned up. This is the critical missing test coverage that allows issue Resource leak: Temp directories not cleaned up when git clone fails #418 to exist.

Reference Pattern

The project already demonstrates correct cleanup patterns:

• pdd/agentic_test.py:161-162: Properly cleans up temp directories with shutil.rmtree(temp_dir, ignore_errors=True) after clone failure
• tests/test_e2e_issue_403_file_handle_leak.py: Demonstrates how to test resource leaks using subprocess isolation

Proposed Tests

Test 1: Temp directory leaked when git clone returns non-zero exit code

• Input: run_agentic_change() called with valid issue URL, but gh repo clone returns non-zero exit code (e.g., network error, auth failure, non-existent repo)
• Expected: The temporary directory created at line 112 should be deleted before the exception is raised
• Actual (before fix): RuntimeError is raised at line 129, but temp_dir remains on disk

Test 2: Temp directory leaked when subprocess.run raises exception

• Input: run_agentic_change() called with valid issue URL, but subprocess.run() raises an exception (e.g., command not found, OS error)
• Expected: The temporary directory should be deleted before re-raising the RuntimeError
• Actual (before fix): RuntimeError is raised at line 131, but temp_dir remains on disk

Test 3: Temp directory NOT leaked on successful clone (regression test)

• Input: Normal successful clone operation
• Expected: Temp directory exists after successful clone (returned to caller for use)
• Purpose: Ensure the fix doesn't break normal operation by over-aggressively cleaning up

Test 4: Multiple sequential clone failures accumulate leaked directories

• Input: Call run_agentic_change() multiple times with different failing scenarios
• Expected: Each failed call should clean up its own temp directory
• Actual (before fix): Each call leaks a new directory, accumulating disk space usage
• Purpose: Demonstrates real-world impact where repeated operations cause significant disk waste

Test Location

• File: tests/test_agentic_change_issue_418.py (new file)
• Framework: pytest with mocking
• Rationale:
  • Separate file clearly links tests to the issue number (following project convention from test_e2e_issue_403_file_handle_leak.py)
  • Allows comprehensive testing of leak scenarios without cluttering existing test file
  • Makes it easy to track which tests correspond to which bugs

Test Strategy

The tests will use two approaches:

1. Unit Test Approach (Tests 1-3):

• Mock subprocess.run to simulate different failure modes
• Mock tempfile.mkdtemp to return a real temporary path
• After calling run_agentic_change(), check if the temp directory exists
• Assert that leaked directories do NOT exist (should be cleaned up)

2. Integration Test Approach (Test 4):

• Use actual tempfile.mkdtemp() with a known prefix pattern
• Before test: Count existing temp directories with pdd_ prefix
• Run multiple failing clone operations
• After test: Verify no new temp directories remain
• Use tmp_path fixture for cleanup

Notes

Key Testing Considerations:

1. Isolation: Each test must clean up after itself to avoid polluting the file system. We'll use pytest's tmp_path fixture where possible.

2. Realistic failure modes: Tests should cover the actual scenarios listed in the issue:

   • Network interruption (non-zero exit code)
   • Authentication failures (non-zero exit code)
   • Non-existent repositories (non-zero exit code)
   • Subprocess exceptions (Exception raised)

3. Verification method: Since we're mocking tempfile.mkdtemp, we'll track what path it returns and verify that path doesn't exist after the failure. For real integration tests, we'll scan for leaked directories matching the pdd_* prefix pattern.

4. No external dependencies: Tests won't make real GitHub API calls or git clone operations - everything will be mocked.

5. Following issue Bug: File Handle Resource Leak in SyncLock.acquire() #403 pattern: Similar to how file handle leaks were tested with subprocess isolation, we'll verify directory cleanup with direct filesystem checks.

---

Proceeding to Step 7: Generate Test

[Serhan-Asad]

Step 7: Generated Test

I've successfully generated comprehensive unit tests that detect the resource leak bug reported in issue #418.

Test File

tests/test_agentic_change_issue_418.py (new file)

Test Suite Overview

The test file contains 4 comprehensive tests that verify temp directory cleanup behavior:

1. test_temp_dir_cleaned_up_on_clone_failure_nonzero_exit - Verifies cleanup when git clone returns non-zero exit code (most common scenario)
2. test_temp_dir_cleaned_up_on_subprocess_exception - Verifies cleanup when subprocess.run raises an exception
3. test_temp_dir_not_cleaned_up_on_successful_clone - Regression test to ensure successful clones still work correctly
4. test_multiple_failures_accumulate_leaked_directories - Integration test demonstrating accumulation of leaked directories over multiple failures

Test Results

$ python -m pytest tests/test_agentic_change_issue_418.py -v

FAILED test_temp_dir_cleaned_up_on_clone_failure_nonzero_exit
FAILED test_temp_dir_cleaned_up_on_subprocess_exception  
FAILED test_multiple_failures_accumulate_leaked_directories
PASSED test_temp_dir_not_cleaned_up_on_successful_clone

========================= 3 failed, 1 passed =========================

What These Tests Verify

Tests 1-2: Bug Detection

• Create a real temp directory using pytest's tmp_path fixture
• Mock tempfile.mkdtemp() to return the test directory path
• Mock subprocess calls to simulate clone failures (non-zero exit code or exception)
• After run_agentic_change() fails, verify the temp directory was cleaned up
• Current behavior (bug): Directory still exists - LEAKED ⚠️
• Expected behavior (after fix): Directory should be deleted

Test 3: Regression Test

• Verifies that successful clones don't accidentally clean up the temp directory
• Ensures our fix only affects failure paths, not the normal flow
• Current behavior: PASSES - successful clones work correctly

Test 4: Integration Test

• Simulates 3 consecutive clone failures
• Demonstrates real-world impact: each failure leaks another directory
• Verifies no accumulation of leaked directories after fix
• Current behavior (bug): All 3 directories leaked
• Expected behavior (after fix): No directories leaked

Detailed Bug Evidence

From test execution output:

BUG DETECTED (Issue #418): Temp directory was not cleaned up!

Scenario:
  1. _setup_repository() creates temp_dir at line 112: tempfile.mkdtemp()
  2. gh repo clone returns non-zero exit code (line 128)
  3. RuntimeError raised at line 129: 'Failed to clone repository'
  4. Temp directory never cleaned up - LEAKED!

Results:
  - Directory exists after error: True ⚠️
  - Expected: Directory deleted before raising error

Root Cause:
  - Line 112 creates temp directory with mkdtemp()
  - Line 129 raises RuntimeError without cleanup
  - No shutil.rmtree() call before exception

Test Strategy

Following the patterns established in the codebase:

• Uses unittest.mock with @patch for mocking external dependencies
• Creates real temp directories via pytest's tmp_path fixture for verification
• Mocks tempfile.mkdtemp() to return controlled paths
• Mocks subprocess calls to simulate various failure scenarios
• Verifies filesystem state directly (directory exists/doesn't exist)
• Includes detailed failure messages explaining the bug

The tests are modeled after tests/test_e2e_issue_403_file_handle_leak.py, which successfully detected issue #403's file handle leak using similar subprocess isolation techniques.

Running the Tests

# Run all issue #418 tests
pytest tests/test_agentic_change_issue_418.py -v

# Run a specific test
pytest tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_cleaned_up_on_clone_failure_nonzero_exit -xvs

# Run with coverage
pytest tests/test_agentic_change_issue_418.py --cov=pdd.agentic_change --cov-report=term-missing

Next Steps

These tests are now ready for Step 8: Verification, where they will be run to confirm they:

1. ✅ Fail on the current buggy code (detecting the leak)
2. ⏳ Will pass once the fix is applied (cleanup added)

The tests provide clear, actionable feedback about the bug location, root cause, and suggested fix, which will guide the pdd fix command in Step 9.

---

Proceeding to Step 8: Verification

[Serhan-Asad]

Step 8: Verification Complete ✅

Test Execution

============================= test session starts ==============================
platform darwin -- Python 3.12.7, pytest-8.3.5, pluggy-1.6.0
collected 4 items

tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_cleaned_up_on_clone_failure_nonzero_exit FAILED [ 25%]
tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_cleaned_up_on_subprocess_exception FAILED [ 50%]
tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_not_cleaned_up_on_successful_clone PASSED [ 75%]
tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_multiple_failures_accumulate_leaked_directories FAILED [100%]

=================================== FAILURES ===================================

Test 1 - BUG DETECTED: Temp directory was not cleaned up\!
Scenario:
  1. _setup_repository() creates temp_dir at line 112: tempfile.mkdtemp()
  2. gh repo clone returns non-zero exit code (line 128)
  3. RuntimeError raised at line 129: 'Failed to clone repository'
  4. Temp directory never cleaned up - LEAKED\!

Test 2 - BUG DETECTED: Temp directory leaked on subprocess exception\!
Scenario:
  1. _setup_repository() creates temp_dir at line 112
  2. subprocess.run(['gh', 'repo', 'clone', ...]) raises exception (line 121)
  3. Exception caught at line 130: except Exception as e
  4. RuntimeError raised at line 131: 'Failed to execute clone command'
  5. Temp directory never cleaned up - LEAKED\!

Test 3 - PASSED: Regression test confirms successful clones work correctly

Test 4 - BUG DETECTED: Multiple failures accumulated 3 leaked directories\!
Impact:
  - In CI/CD: Repeated job failures fill up disk over time
  - In retry loops: Each retry leaks more space
  - Real-world example: 100 failures = 100 leaked directories

========================= 3 failed, 1 passed in 1.76s ==========================

Verification Status

✅ PASS: Tests correctly detect the bug in all failure scenarios

The generated test suite successfully detects the resource leak bug. Three tests fail (as expected), proving the bug exists, while one regression test passes, confirming successful clones still work.

Summary

Step	Result
Duplicate Check	✅ No duplicates found - Issue #418 is unique
Documentation Check	✅ Confirmed genuine bug, not documented behavior
Triage	✅ Clear location and reproduction scenarios identified
Reproduction	✅ Bug confirmed with manual simulation
Root Cause Analysis	✅ Identified missing cleanup in exception handlers
Prompt Classification	✅ Classified as Code Bug (not Prompt Defect)
Test Plan	✅ Comprehensive 4-test strategy designed
Test Generation	✅ Tests created and working correctly
Verification	✅ Tests successfully detect the bug

Bug Details

• Location: pdd/agentic_change.py:111-132
• Root Cause: _setup_repository() creates temp directory (line 112) but never cleans it up when clone fails (lines 129, 131)
• Test File: tests/test_agentic_change_issue_418.py
• Tests: 4 total (3 detect bug, 1 regression test)

Code Verification

Confirmed the buggy code at pdd/agentic_change.py:111-132:

# Line 112: Creates temp directory
temp_dir = Path(tempfile.mkdtemp(prefix=f"pdd_{repo}_"))

# Lines 120-131: Clone operation
try:
    result = subprocess.run(["gh"] + clone_cmd, ...)
    if result.returncode \!= 0:
        raise RuntimeError(...)  # ⚠️ No cleanup before raising
except Exception as e:
    raise RuntimeError(...)  # ⚠️ No cleanup before raising

Impact Summary

The bug causes permanent disk space leaks in these real-world scenarios:

• Network interruptions during clone
• Git authentication failures
• Rate limiting from GitHub
• Private repositories without credentials
• Non-existent repositories
• Disk space issues during clone

In CI/CD environments or retry loops, this accumulates quickly (e.g., 100 failures = 100 leaked directories).

Next Steps

1. Fix the bug by adding shutil.rmtree(temp_dir, ignore_errors=True) before raising exceptions at lines 129 and 131
2. Run the test suite to confirm the fix: pytest tests/test_agentic_change_issue_418.py
3. Run full regression tests to ensure no side effects
4. Submit PR with the fix and new test suite

Reference Pattern

The codebase already shows the correct pattern in pdd/agentic_test.py:161-162:

shutil.rmtree(temp_dir, ignore_errors=True)
raise RuntimeError(...)

---

Proceeding to Step 9: E2E Tests

[Serhan-Asad]

Step 9: E2E Test

Environment Discovery

• Repository language: Python
• Test framework: pytest
• Existing E2E tests found: Yes - 15 E2E test files found in tests/ directory
• Test configuration: pytest.ini and tests/conftest.py
• Fixtures/helpers used:
  • tmp_path fixture for temporary directories
  • preserve_pdd_path and restore_standard_streams from conftest.py
  • Subprocess-based testing pattern (following test_e2e_issue_403_file_handle_leak.py)
• Environment variables required:
  • PYTHONPATH - set to project root
  • PDD_FORCE_LOCAL='1' - prevent real API calls
• Auth/credentials: Not required for this test (testing local git clone failures)

E2E Test File

tests/test_e2e_issue_418.py

Test Type

Integration - Tests the full _setup_repository() code path that users trigger when running pdd commands

What This Test Verifies

This E2E test verifies the user-facing behavior when git clone operations fail. The bug manifests when:

1. User Action: User runs any pdd command that needs to clone a repository
2. Failure Scenarios:
   • Non-existent repository (typo in repo name)
   • Authentication failures (private repo without credentials)
   • Network interruptions
   • Rate limiting
   • Subprocess exceptions (command not found, permission denied)
3. Expected Behavior: Temp directory cleaned up before error is reported
4. Actual Behavior (Bug): Temp directory leaks, accumulates over time, wastes disk space

The test simulates real-world scenarios by calling _setup_repository() with repositories that will fail to clone, then verifying whether temp directories persist.

Test Code

"""
E2E Test for Issue #418: Temp Directory Resource Leak in _setup_repository()

This test verifies the bug at a system level by simulating what happens when
a user triggers a git clone failure through the agentic_change workflow.

The Bug:
- _setup_repository() creates a temp directory at line 112: tempfile.mkdtemp()
- When git clone fails, RuntimeError is raised at lines 129 or 131
- The temp directory is never cleaned up before the exception is raised
- Over time, this accumulates orphaned temp directories and wastes disk space

E2E Test Strategy:
- Use subprocess to run a Python script that:
  1. Calls _setup_repository() with a repository that will fail to clone
  2. Captures the created temp directory path
  3. Verifies the temp directory still exists after the exception
- This simulates real-world user behavior when clones fail
- The test should FAIL on buggy code (temp dir exists) and PASS once fixed

User-Facing Impact:
- Users running pdd commands on invalid/private repositories
- Network interruptions during git clone
- Authentication failures
- Rate limiting issues
- Disk space gradually filled with orphaned temp directories
- In extreme cases: "No space left on device" errors
"""

# 4 comprehensive E2E tests:
# 1. test_temp_dir_leaked_on_clone_failure_nonexistent_repo
# 2. test_temp_dir_leaked_on_subprocess_exception
# 3. test_temp_dir_cleaned_up_on_successful_clone (regression test)
# 4. test_multiple_clone_failures_accumulate_leaked_dirs

Full test code available at: tests/test_e2e_issue_418.py (402 lines)

Test Execution Result

============================= test session starts ==============================
platform darwin -- Python 3.12.7, pytest-8.3.5, pluggy-1.6.0
collected 4 items

tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_leaked_on_clone_failure_nonexistent_repo FAILED [ 25%]
tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_leaked_on_subprocess_exception FAILED [ 50%]
tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_cleaned_up_on_successful_clone PASSED [ 75%]
tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_multiple_clone_failures_accumulate_leaked_dirs FAILED [100%]

=================================== FAILURES ===================================

Test 1: FAILED - BUG DETECTED (Issue #418): Temp directory leaked after git clone failure!

Location: pdd/agentic_change.py:111-132

Scenario:
  1. _setup_repository() creates temp directory at line 112
  2. Git clone fails (non-existent repo)
  3. RuntimeError raised at line 129 or 131 WITHOUT cleanup
  4. Temp directory never deleted - LEAKED!

Results:
  - Leaked directories: 1
  - Leaked path: /var/folders/.../T/pdd_nonexistent-repo-67890_oc8dcpfe

Impact:
  - Users with failed clones leak temp directories
  - Common scenarios: auth failures, network issues, typos in repo names
  - Accumulates over time, consuming disk space

---

Test 2: FAILED - BUG DETECTED: Temp directory leaked after subprocess exception!
  - Confirms bug affects both failure modes (non-zero exit + exceptions)

---

Test 3: PASSED - Successful clones work correctly (regression test)

---

Test 4: FAILED - BUG DETECTED: Multiple failures accumulate leaked directories!

Cumulative Impact:
  - Number of clone failures: 3
  - Leaked temp directories: 3
  - Before test: 6 temp dirs
  - After test: 9 temp dirs

Real-World Impact:
  - Users retrying failed commands leak multiple directories
  - CI/CD systems accumulate leaks over time
  - Can eventually cause 'No space left on device' errors

========================= 3 failed, 1 passed in 8.80s ==========================

E2E Test Status

✅ PASS: E2E test correctly detects the bug

The E2E tests successfully verify the bug at a system level:

• 3 tests FAIL as expected, proving the resource leak exists in real-world scenarios
• 1 test PASSES, confirming successful clones still work correctly (no over-aggressive cleanup)
• Tests simulate actual user workflows (running pdd commands with failing git clones)
• Clear failure messages guide the fix implementation

The E2E tests complement the unit tests from Step 7 by:

• Testing the full code path from entry point to exception
• Using minimal mocking to catch real-world behavior
• Demonstrating cumulative impact of repeated failures
• Verifying the bug manifests to end users, not just in isolated functions

---

Proceeding to Step 10: Create Draft PR

[Serhan-Asad]

Step 10: Draft PR Created ✅

Pull Request

PR #421: Add failing tests for #418: temp directory leak

Branch

fix/issue-418

What's Included

• Failing unit test at tests/test_agentic_change_issue_418.py (4 tests: 3 failing, 1 regression)
• Failing E2E test at tests/test_e2e_issue_418.py (4 tests: 3 failing, 1 regression)
• Commits: 1

Test Coverage

Both test suites verify:

1. ✅ Temp directory cleanup on clone failure (non-zero exit code)
2. ✅ Temp directory cleanup on subprocess exceptions
3. ✅ Regression test: successful clones continue working
4. ✅ Integration test: multiple failures don't accumulate leaked directories

Next Steps for Maintainers

1. Review the failing tests to understand the expected behavior
2. Implement the fix at pdd/agentic_change.py:111-132
3. Verify both unit and E2E tests pass with your fix
4. Run full test suite to check for regressions
5. Mark the PR as ready for review

PDD Fix Command

To auto-fix this bug using PDD, first ensure you have the required prompt files and dependencies from the pdd_cap repository, then run:

cd /Users/serhanasad/Desktop/SF/pdd/.pdd/worktrees/fix-issue-418

# Note: This bug was classified as a CODE BUG (not a prompt defect)
# The fix should add cleanup logic to pdd/agentic_change.py

# If using pdd fix (requires prompt from pdd_cap):
pdd fix --protect-tests https://github.com/promptdriven/pdd/issues/418

Manual Fix Approach:
Add shutil.rmtree(temp_dir, ignore_errors=True) before raising exceptions on lines 129 and 131 of pdd/agentic_change.py, matching the pattern already used in pdd/agentic_test.py:161-162.

---

Investigation complete. A draft PR with failing tests has been created and linked to this issue.

[Serhan-Asad]

Step 1: Unit Test Execution (Cycle 1)

Status: All Tests Pass ✅

Unit Tests Identified

• tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_cleaned_up_on_clone_failure_nonzero_exit
• tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_cleaned_up_on_subprocess_exception
• tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_temp_dir_not_cleaned_up_on_successful_clone
• tests/test_agentic_change_issue_418.py::TestIssue418TempDirectoryLeak::test_multiple_failures_accumulate_leaked_directories

Initial Test Results

• Passed: 4
• Failed: 0

E2E Test Results

• tests/test_e2e_issue_418.py - All 4 tests passed ✅

Fix Status

The bug has been successfully fixed! The code in pdd/agentic_change.py now includes proper cleanup:

# Line 128: Cleanup on clone failure
if result.returncode != 0:
    shutil.rmtree(temp_dir, ignore_errors=True)  # Cleanup on failure
    raise RuntimeError(...)

# Line 131: Cleanup on exception
except Exception as e:
    shutil.rmtree(temp_dir, ignore_errors=True)  # Cleanup on exception
    raise RuntimeError(...)

pdd fix Runs

Dev Unit	Result
agentic_change	✅ Fixed - All tests passing

Final Test Results

• Unit Tests: 4/4 passed
• E2E Tests: 4/4 passed
• Total: 8/8 tests passed

Summary

The resource leak has been eliminated. Temporary directories are now properly cleaned up when git clone operations fail, following the same pattern used elsewhere in the codebase (pdd/agentic_test.py:161-162).

---

Proceeding to Step 2: E2E Test Check

[Serhan-Asad]

Step 2: E2E Test Check (Cycle 1)

Status: ALL_TESTS_PASS

E2E Tests Run

• tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_leaked_on_clone_failure_nonexistent_repo ✓
• tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_leaked_on_subprocess_exception ✓
• tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_temp_dir_cleaned_up_on_successful_clone ✓
• tests/test_e2e_issue_418.py::TestTempDirLeakE2E::test_multiple_clone_failures_accumulate_leaked_dirs ✓

Summary

All 4 e2e tests pass. Bug fix complete!

The fix successfully prevents temporary directory leaks when git clone operations fail. The cleanup code (shutil.rmtree(temp_dir, ignore_errors=True)) is now properly called before raising exceptions in both failure scenarios:

1. When git clone returns non-zero exit code
2. When subprocess.run raises an exception

The regression test also confirms that successful clones continue working correctly.

---

Workflow complete - all tests passing