Skip to content

feat: Add support for parsing full GitHub URLs#192

Open
hoducha wants to merge 4 commits intocomputerlovetech:mainfrom
hoducha:main
Open

feat: Add support for parsing full GitHub URLs#192
hoducha wants to merge 4 commits intocomputerlovetech:mainfrom
hoducha:main

Conversation

@hoducha
Copy link
Copy Markdown
Contributor

@hoducha hoducha commented Mar 15, 2026

Problem

Running agr add with a full GitHub URL fails:

❯ agr add https://github.com/user/repo/tree/main/skills/my-skill
Failed: Invalid handle '...': too many path segments (expected user/name or user/repo/name)

Users often copy and paste URLs directly from the GitHub web interface. Additionally, some shared skills are located outside of the repository's root directory. However, the parse_handle function only accepts the shortened URL format in the form of user/repo/skill.

Solution

Detects GitHub URLs and normalizes them to the standard handle format before the existing parsing logic runs.

Supported URL patterns:

https://github.com/user/repo/tree/branch/path/to/skill → user/repo/skill
https://github.com/user/repo/tree/branch → user/repo
https://github.com/user/repo → user/repo
Also handles /blob/... URLs, trailing slashes, and http://

A fast-path check ("://" not in ref) skips urlparse entirely for normal handles.

Copy link
Copy Markdown
Collaborator

@kasperjunge kasperjunge left a comment

Choose a reason for hiding this comment

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

Review feedback: slash-in-branch-name edge case

After deeper analysis, the skill_path[-1] approach is actually correct for the real-world use case — users copy a URL pointing to a skill directory, so the last segment is always the skill name, regardless of slashes in the branch.

Suggestion: Add a comment in _try_parse_github_url documenting this known limitation:

if len(path_parts) >= 4 and path_parts[2] in ("tree", "blob"):
    # Note: we can't reliably separate the branch name from the skill
    # path when branches contain slashes (e.g. feature/foo). Since skill
    # discovery is name-based (recursive SKILL.md search), we only need
    # the last segment as the skill name — which skill_path[-1] gives us
    # correctly even with slashed branches.
    skill_path = path_parts[4:]

Also consider adding a test that documents this works:

def test_url_with_slashed_branch(self):
    """Slashed branch names still extract the correct skill name."""
    h = parse_handle("https://github.com/user/repo/tree/feature/login/skills/sample")
    assert h.name == "sample"
    assert h.repo == "repo"

Otherwise the implementation looks good — the "://" fast-path skip and the overall integration approach are clean. 👍

🤖 Generated with Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants