Skip to content

Conversation

@thrix
Copy link
Contributor

@thrix thrix commented Jul 8, 2025

When tree_path is '.', the current code constructs invalid paths like '/path/to/repo./' which causes "Invalid directory path" errors.

This fix adds special handling for '.' (current directory) to avoid path modification while still handling .git suffix removal properly.

Fixes #19

🤖 Generated with Claude Code

When tree_path is '.', the current code constructs invalid paths like
'/path/to/repo./' which causes "Invalid directory path" errors.

This fix adds special handling for '.' (current directory) to avoid
path modification while still handling .git suffix removal properly.

Fixes #19

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@codiumai-pr-agent-free
Copy link

Title

Fix handling of '.' as tree_path in get_tree function


User description

When tree_path is '.', the current code constructs invalid paths like '/path/to/repo./' which causes "Invalid directory path" errors.

This fix adds special handling for '.' (current directory) to avoid path modification while still handling .git suffix removal properly.

Fixes #19

🤖 Generated with Claude Code


PR Type

Bug fix


Description

  • Fix handling of '.' as tree_path to prevent invalid directory paths

  • Add special case for current directory without path modification

  • Preserve .git suffix removal for dot path handling

  • Add comprehensive unit tests for dot path scenarios


Changes diagram

flowchart LR
  A["tree_path input"] --> B{"tree_path == '.'?"}
  B -->|Yes| C["Handle current directory"]
  B -->|No| D["Original path logic"]
  C --> E{"path has .git suffix?"}
  E -->|Yes| F["Remove .git suffix"]
  E -->|No| G["Use path as-is"]
  D --> H["Append '/' and process"]
  F --> I["Return processed path"]
  G --> I
  H --> I
Loading

Changes walkthrough 📝

Relevant files
Bug fix
service.py
Fix dot path handling in get_tree                                               

src/tmt_web/service.py

  • Add special handling for tree_path == "." to avoid invalid path
    construction
  • Preserve .git suffix removal logic for dot path case
  • Restructure conditional logic to handle current directory separately
  • +5/-1     
    Tests
    test_service.py
    Add unit tests for dot path scenarios                                       

    tests/unit/test_service.py

  • Add test for '.' as tree_path without .git suffix
  • Add test for '.' as tree_path with .git suffix handling
  • Verify as_posix() is not called for dot path cases
  • +32/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @codiumai-pr-agent-free
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    19 - Fully compliant

    Compliant requirements:

    • Fix the handling of '.' as path parameter which currently results in an error
    • Support '.' as a valid path value since it's the default
    • Fix the specific error: "Invalid yaml syntax: Invalid directory path: /var/tmp/tmt-web/2361321475348135807."
    • Fix the issue in the code related to path handling in service.py

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    @codiumai-pr-agent-free
    Copy link

    codiumai-pr-agent-free bot commented Jul 8, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Handle empty string paths
    Suggestion Impact:The commit restructured the code to handle path modifications more efficiently, and while it doesn't directly use the suggested 'or tree_path == ""' condition, it achieves the same goal by restructuring the logic. The new code handles the .git suffix removal first, then checks if tree_path is not None and not '.', which implicitly handles empty strings correctly.

    code diff:

    -    if tree_path == ".":
    -        # Handle current directory case - no path modification needed
    -        if path.suffix == ".git":
    -            path = path.with_suffix("")
    -    elif tree_path is not None:
    +    # Remove .git suffix if present
    +    if path.suffix == ".git":
    +        path = path.with_suffix("")
    +
    +    # If path is set, construct a path to the tmt Tree
    +    if tree_path is not None and tree_path != ".":
             tree_path += "/"
    -        # If path is set, construct a path to the tmt Tree
    -        if path.suffix == ".git":
    -            path = path.with_suffix("")
             path = Path(path.as_posix() + tree_path)

    The current implementation doesn't handle the case when tree_path is an empty
    string, which could lead to unexpected behavior. Consider adding a check for
    empty string alongside the "." check to ensure consistent handling of both
    cases.

    src/tmt_web/service.py [45-53]

    -if tree_path == ".":
    +if tree_path == "." or tree_path == "":
         # Handle current directory case - no path modification needed
         if path.suffix == ".git":
             path = path.with_suffix("")
     elif tree_path is not None:
         tree_path += "/"
         # If path is set, construct a path to the tmt Tree
         if path.suffix == ".git":
             path = path.with_suffix("")

    [Suggestion processed]

    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that an empty string for tree_path could be a valid case for the repository root, and it should be handled consistently with the . path to improve robustness.

    Low
    General
    Remove duplicated code
    Suggestion Impact:The commit implemented the suggestion by moving the .git suffix removal logic outside the conditional blocks, exactly as suggested. The code now checks for and removes the .git suffix once, before handling the tree_path conditions.

    code diff:

    -    if tree_path == ".":
    -        # Handle current directory case - no path modification needed
    -        if path.suffix == ".git":
    -            path = path.with_suffix("")
    -    elif tree_path is not None:
    +    # Remove .git suffix if present
    +    if path.suffix == ".git":
    +        path = path.with_suffix("")
    +
    +    # If path is set, construct a path to the tmt Tree
    +    if tree_path is not None and tree_path != ".":
             tree_path += "/"
    -        # If path is set, construct a path to the tmt Tree
    -        if path.suffix == ".git":
    -            path = path.with_suffix("")

    The code duplicates the logic for handling .git suffix in both branches. Extract
    this common logic outside the conditional blocks to reduce duplication and
    improve maintainability.

    src/tmt_web/service.py [45-53]

    +# Remove .git suffix regardless of tree_path
    +if path.suffix == ".git":
    +    path = path.with_suffix("")
    +    
     if tree_path == ".":
         # Handle current directory case - no path modification needed
    -    if path.suffix == ".git":
    -        path = path.with_suffix("")
    +    pass
     elif tree_path is not None:
         tree_path += "/"
         # If path is set, construct a path to the tmt Tree
    -    if path.suffix == ".git":
    -        path = path.with_suffix("")

    [Suggestion processed]

    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that the logic to strip the .git suffix is duplicated and proposes a good refactoring to remove this duplication, which improves code maintainability.

    Low
    • Update

    @thrix thrix requested a review from martinhoyer July 8, 2025 15:58
    @psss
    Copy link
    Contributor

    psss commented Oct 14, 2025

    Seems like a easy review fix, forgotten, tentatively adding to the next sprint.

    @psss psss added this to planning Oct 14, 2025
    @github-project-automation github-project-automation bot moved this to backlog in planning Oct 14, 2025
    @psss psss moved this from backlog to review in planning Oct 14, 2025
    Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
    @thrix thrix requested a review from LecrisUT November 3, 2025 13:29
    Copy link

    @tcornell-bus tcornell-bus left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    LGTM

    @happz happz merged commit 37be50c into main Nov 3, 2025
    3 checks passed
    @happz happz deleted the fix/dot-path-handling branch November 3, 2025 21:34
    @github-project-automation github-project-automation bot moved this from review to done in planning Nov 3, 2025
    @codiumai-pr-agent-free
    Copy link

    PR Compliance Guide 🔍

    Below is a summary of compliance checks for this PR:

    Security Compliance
    🟢
    No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
    Ticket Compliance
    🟢
    🎫 #19
    🟢 Fix handling of '.' as path parameter
    Ensure '.' is treated as a valid path value
    Fix the "Invalid directory path" error when '.' is provided as path
    Codebase Duplication Compliance
    Codebase context is not defined

    Follow the guide to enable codebase context checks.

    Custom Compliance
    🟢
    Generic: Comprehensive Audit Trails

    Objective: To create a detailed and reliable record of critical system actions for security analysis
    and compliance.

    Status: Passed

    Generic: Meaningful Naming and Self-Documenting Code

    Objective: Ensure all identifiers clearly express their purpose and intent, making code
    self-documenting

    Status: Passed

    Generic: Robust Error Handling and Edge Case Management

    Objective: Ensure comprehensive error handling that provides meaningful context and graceful
    degradation

    Status: Passed

    Generic: Secure Error Handling

    Objective: To prevent the leakage of sensitive system information through error messages while
    providing sufficient detail for internal debugging.

    Status: Passed

    Generic: Secure Logging Practices

    Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
    information like PII, PHI, or cardholder data.

    Status: Passed

    Generic: Security-First Input Validation and Data Handling

    Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
    vulnerabilities

    Status: Passed

    Compliance status legend 🟢 - Fully Compliant
    🟡 - Partial Compliant
    🔴 - Not Compliant
    ⚪ - Requires Further Human Verification
    🏷️ - Compliance label

    The managed version of the open source project PR-Agent is sunsetting on the 1st December 2025. The commercial version of this project will remain available and free to use as a hosted service. Install Qodo.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    Status: done

    Development

    Successfully merging this pull request may close these issues.

    Correctly handle . in path

    6 participants