Skip to content

fix: Use GitHub URL instead of local one#297

Merged
kami619 merged 1 commit intoambient-code:mainfrom
mprpic:fix-broken-repo-urls
Feb 18, 2026
Merged

fix: Use GitHub URL instead of local one#297
kami619 merged 1 commit intoambient-code:mainfrom
mprpic:fix-broken-repo-urls

Conversation

@mprpic
Copy link
Contributor

@mprpic mprpic commented Feb 18, 2026

When the assessment is generated, it adds the URL configured in the user's local copy of the repo, which can be an SSH URL. This then gets rendered as is in the leaderboard, causing broken URLs.

Since submissions can only be created for GitHub URLs, add an explicit URL using https in the generated leaderboard.

Description

When the assessment is generated, it adds the URL configured in the user's local copy of the repo, which can be an SSH URL. This then gets rendered as is in the leaderboard, causing broken URLs.

Since submissions can only be created for GitHub URLs, add an explicit URL using https in the generated leaderboard.

This fixes the currently broken links in the top 10 view at https://ambient-code.github.io/agentready/.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Changes Made

  • Update to leaderboard generator script

Testing

Ran the leaderboard generation script and it correctly fixed the URLs:

@@ -42,7 +42,7 @@
       "language": "Unknown",
       "size": "Unknown",
       "last_updated": "2026-02-17",
-      "url": "git@github.com:opendatahub-io/ai-helpers.git",
+      "url": "https://github.com/opendatahub-io/ai-helpers",
       "agentready_version": "2.27.3",
       "research_version": "1.0.1",
       "history": [

When the assessment is generated, it adds the URL configured in the
user's local copy of the repo, which can be an SSH URL. This then gets
rendered as is in the leaderboard, causing broken URLs.

Since submissions can only be created for GitHub URLs, add an explicit
URL using https in the generated leaderboard.

Signed-off-by: Martin Prpič <mprpic@redhat.com>
@github-actions
Copy link
Contributor

📈 Test Coverage Report

Branch Coverage
This PR 66.0%
Main 66.0%
Diff ✅ +0%

Coverage calculated from unit tests only

@github-actions
Copy link
Contributor

🤖 AgentReady Code Review

PR Status: 2 issues found (0 🔴 Critical, 1 🟡 Major, 1 🔵 Minor)
Score Impact: Current 80.0/100 → 80.5/100 if all issues fixed (+0.5 points)
Certification: Gold → Gold (maintained)


✅ Summary: APPROVED with Recommendations

This PR successfully fixes a real production issue (broken SSH URLs in the leaderboard) with a clean, minimal change. The fix actually improves security by using validated data instead of user-provided URLs. However, the review identified pre-existing validation gaps that should be addressed in a follow-up PR.


🟡 Major Issues (Pre-existing, not introduced by this PR)

1. URL Construction Lacks Input Validation

Attribute: Security Best Practices (Tier 2)
Confidence: 85%
Score Impact: −0.3 points
Location: scripts/generate-leaderboard-data.py:98

Issue Details:
The script constructs URLs from repo_name without validating the format. While repo_name is derived from filesystem paths (making this low-risk in production), the code doesn't verify the expected org/repo format before string operations.

Potential failure scenarios:

  • Lines 91-92 split repo_name by "/" assuming exactly 2 parts
  • If repo_name is malformed (e.g., "single-part" or "org/repo/extra"), this causes IndexError
  • Current error handling (line 150) catches this but provides generic error message

Note: The AgentReady codebase has comprehensive security utilities in src/agentready/utils/security.py (including validate_url(), validate_path(), sanitize_for_html()) that could be leveraged here.

Remediation: Add validation before URL construction (follow-up PR recommended)


🔵 Minor Issues

2. Script Lacks Unit Tests

Attribute: Test Coverage (Tier 2)
Confidence: 90%
Score Impact: −0.2 points
Location: scripts/generate-leaderboard-data.py (entire file)

Issue Details:
The script has no associated unit tests. While CLAUDE.md test requirements explicitly target "new assessors" and core library code, adding tests for utility scripts would align with the ">80% coverage" goal.

PR Author's Note: The author acknowledged this in the PR description: "Tests seem to currently be broken, I can submit a fix for that as a secondary PR"

This is acceptable because:

  • Script is a data generation utility, not core library code
  • Script is validated indirectly via automated leaderboard workflow
  • All CI checks pass
  • CLAUDE.md test requirements focus on assessors/reporters

✅ What This PR Does Well

  1. Fixes Real Production Issue: Transforms broken SSH URLs to clickable HTTPS URLs

  2. Improves Security: Uses validated repo_name from filesystem structure instead of potentially malicious user-provided URLs

  3. Excellent Commit Message: Follows conventional commits perfectly with clear explanation of the "why"

  4. Completes URL Handling Trilogy:

  5. All CI Checks Pass: Black, isort, Ruff, full test suite, CodeQL, macOS compatibility


📊 Detailed Analysis

Historical Context:

  • This is the third PR in a series addressing URL handling issues
  • Leaderboard currently contains 5+ SSH URLs causing broken links
  • The workflow already normalizes URLs in validation; this extends that pattern to data generation

Code Change:
Line 98 changed from using potentially SSH-formatted URL from git config to constructing canonical HTTPS URL from validated org/repo names.

Why This Is Safe:

  • repo_name comes from filesystem path parsing (lines 38-40)
  • Already validated by leaderboard submission workflow
  • Consistent with GitHub-only submission policy

Summary

  • Auto-Fix Candidates: 0 critical issues
  • Manual Review: 2 issues (pre-existing, not blocking)
  • Total Score Improvement Potential: +0.5 points if validation added in follow-up
  • AgentReady Assessment: Run agentready assess . after fixes to verify score

Recommendation: ✅ Approve and merge. Consider adding validation and tests in follow-up PR as mentioned by the author.


🤖 Generated with Claude Code

If this review was useful, react with 👍. Otherwise, react with 👎.

@kami619 kami619 merged commit 5abc7c2 into ambient-code:main Feb 18, 2026
10 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 18, 2026
# [2.28.0](v2.27.5...v2.28.0) (2026-02-18)

### Bug Fixes

* Use GitHub URL instead of local one ([#297](#297)) ([5abc7c2](5abc7c2))

### Features

* add feast-dev/feast to leaderboard ([#293](#293)) ([c894ce9](c894ce9))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 2.28.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments