Skip to content

feat(git): add support for git SSH URLs#375

Closed
myysy wants to merge 5 commits intovolcengine:mainfrom
myysy:fea/support_ssh_add_repo
Closed

feat(git): add support for git SSH URLs#375
myysy wants to merge 5 commits intovolcengine:mainfrom
myysy:fea/support_ssh_add_repo

Conversation

@myysy
Copy link
Copy Markdown
Collaborator

@myysy myysy commented Mar 2, 2026 •
edited
Loading

  • Add support for git@, ssh://, and git:// URLs in resource processing
  • Enhance error messages for common git failures
  • Validate git SSH URI format

Description

Related Issue

#317

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@myysy
feat(git): add support for git SSH URLs and improve error handling
bdf89c3 
- Add support for git@, ssh://, and git:// URLs in resource processing
- Enhance error messages for common git failures
- Validate git SSH URI format
- Update help text and documentation for new URL formats
- Add fallback to Python CLI when binary is not found
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 2, 2026 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@myysy
style: remove trailing whitespace and clean up code formatting
2d615c4 
Clean up code by removing trailing whitespace and improving list comprehension formatting for better readability
@qin-ctx qin-ctx self-requested a review March 2, 2026 09:24
qin-ctx
qin-ctx reviewed Mar 2, 2026
View reviewed changes
myysy added 3 commits March 2, 2026 21:12
@myysy
feat(code): improve git error handling and URL parsing
c895e73 
- Add git command to error message for better debugging
- Rename is_code_hosting_url to is_code_repository_root_url for clarity
- Simplify parse_code_hosting_url to handle only org/repo format
- Add new stricter is_code_repository_root_url function
- Add comprehensive tests for URL parsing and validation
@myysy
test(code_hosting_utils): improve test readability with line breaks
7143460 
Refactor test assertions and module path declarations to use multiple lines for better readability while maintaining the same test functionality
@myysy
refactor: remove unused curses.error import
1739f14
@r266-tech r266-tech mentioned this pull request Mar 2, 2026
Merged
@r266-tech
Copy link
Copy Markdown
Contributor

r266-tech commented Mar 2, 2026

Hi @myysy, I've opened an alternative PR #385 that addresses the same issue (#317) while incorporating @qin-ctx's review feedback:

  1. Regression-safe routing: Introduced is_git_repo_url() that uses strict path checking — HTTPS URLs must have exactly 2 path segments (/owner/repo) to be treated as repos. This prevents /issues/, /pull/, /blob/ URLs from being routed to CodeRepositoryParser.

  2. Error messages preserved: _run_git() keeps command info in the format: {user_msg} Command: git {args}. Details: {error}

  3. SSH/HTTP consistency: Both branches in parse_code_hosting_url() take only the first 2 path segments.

  4. 20 pytest tests covering all new functions including regression tests.

  5. No unnecessary try/except: Clean git@ handling with explicit returns.

Happy to discuss or collaborate on merging efforts!

MaojiaSheng pushed a commit that referenced this pull request Mar 2, 2026
@r266-tech
fix(git): support git@ SSH URLs with regression-safe repo detection (#…
6aa8e7d 
…317) (#385)

- Add git@ SSH URL parsing to parse_code_hosting_url()
- Add git@ support to is_code_hosting_url() (clean, no try/except)
- Add validate_git_ssh_uri() for early format validation
- Add is_git_repo_url() for strict cloneable-repo detection:
  * git@/ssh://git:// protocols: always repo if domain matches
  * https URLs: require exactly /owner/repo path (2 segments)
  * Prevents regression: /issues/, /pull/, /blob/ etc. are NOT repos
- Update media_processor routing to use CodeRepositoryParser for git repos
- Update CLI resource handler to recognize git@ URLs
- Update CodeRepositoryParser.parse() to handle git@ directly
- Enhance _run_git() errors: keep command info, add friendly messages
- Add 20 comprehensive tests covering all new functions

Fixes #317
Addresses review comments from PR #375 (regression risk, test coverage,
error message preservation, SSH/HTTP path consistency)

Co-authored-by: r266-tech <r266-tech@users.noreply.github.com>
@MaojiaSheng
Copy link
Copy Markdown
Collaborator

MaojiaSheng commented Mar 2, 2026

oops this pr is duplicated with #385

@MaojiaSheng MaojiaSheng closed this Mar 3, 2026
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qin-ctx qin-ctx qin-ctx left review comments

Assignees

No one assigned

Labels

None yet

Projects

OpenViking project
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@myysy @CLAassistant @r266-tech @MaojiaSheng @qin-ctx