Wire git resolver into skill install flow

[JAORMX]

Summary

The gitresolver package was fully implemented but had no connection to the skill install path. Users couldn't actually install skills from git repositories — the resolver sat unused.

This change integrates the git resolver so thv skill install supports git-based skill sources, both via explicit git:// prefix and through registry entries that specify a git package type.

• Add WithGitResolver option and installFromGit method to skillsvc to handle git-based installs
• Detect git:// prefix in Install() before falling through to OCI reference check
• Extend resolveFromRegistry to handle "git" package types alongside OCI
• Add dev-mode SSRF bypass in validateHost for E2E testability
• Generate mock for gitresolver.Resolver interface
• Add unit tests for all git install paths (fresh, upgrade, no-op, errors)
• Add E2E tests with a local dumb-HTTP git server infrastructure

Type of change

• New feature

Test plan

• Unit tests (task test)
• E2E tests (task test-e2e)

Changes

File	Change
pkg/api/server.go	Wire gitresolver.NewResolver() into server builder
pkg/skills/gitresolver/reference.go	Add IsGitReference helper, export ParseReference
pkg/skills/gitresolver/resolver.go	Add Resolver interface declaration for mock generation
pkg/skills/gitresolver/mocks/mock_resolver.go	Generated mock for Resolver interface
pkg/skills/skillsvc/skillsvc.go	Add WithGitResolver, installFromGit, git-aware registry resolution
pkg/skills/skillsvc/skillsvc_test.go	Unit tests for git install paths
test/e2e/api_skills_git_test.go	E2E tests with local dumb-HTTP git server

Does this introduce a user-facing change?

Yes — users can now install skills from git repositories using thv skill install git:// or via registry entries with a git package type.

Large PR Justification

• Generated code that cannot be split: mock for gitresolver.Resolver (57 lines)
• Test code that depends on the feature: unit tests (350 lines) and E2E tests (471 lines) exercise the new install paths and cannot be merged independently
• Actual production code is ~270 lines across 4 files, well within the 400-line limit when excluding tests and generated code

Special notes for reviewers

• The validateHost function has a dev-mode bypass (THV_DEV_SKIP_SSRF_CHECK) to allow E2E tests to use localhost git servers. This is gated behind a build/env check and never active in production.
• The E2E test spins up a temporary bare git repo served over dumb HTTP — no external network access needed.

Generated with Claude Code

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 80.58824% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.33%. Comparing base (ccb98c3) to head (cacd421).

Files with missing lines	Patch %	Lines
pkg/skills/skillsvc/skillsvc.go	81.16%	20 Missing and 9 partials ⚠️
pkg/api/server.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4338      +/-   ##
==========================================
+ Coverage   68.95%   69.33%   +0.37%     
==========================================
  Files         479      479              
  Lines       48489    48592     +103     
==========================================
+ Hits        33438    33692     +254     
+ Misses      12317    12304      -13     
+ Partials     2734     2596     -138     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

Large PR justification has been provided. Thank you!

[aponcedeleonch]

Review Comments

Potential inconsistent state on group registration failure

In Install() (skillsvc.go:221-226), if installFromGit succeeds (files written + DB record created) but registerSkillInGroup fails, the skill is installed on disk and in the DB but not added to the group. This leaves the system in an inconsistent state — the skill exists but isn't in the expected group.

Note: this is the same pattern used by the OCI install path, so it's not a regression introduced by this PR — but worth tracking as a follow-up improvement for both paths.

Missing unit test coverage

• No unit tests for validateHost with the SSRF bypass enabled/disabled (both branches of isDevMode()). Since this is security-critical code, it would be good to have tests using t.Setenv("TOOLHIVE_DEV", "true") and without it to verify the SSRF protection logic works correctly in production configuration.
• No unit tests for buildGitReferenceFromRegistryURL edge cases (e.g., http:// URL silent promotion to https://, already prefixed with git://).

[aponcedeleonch]

If installFromGit succeeds (files written + DB record created) but registerSkillInGroup fails, the skill is installed on disk and in the DB but not added to the group. This leaves the system in an inconsistent state — the skill exists but isn't in the expected group.

Note: this is the same pattern used by the OCI install path, so it's not a regression introduced by this PR — but worth tracking as a follow-up improvement for both paths.

[JAORMX]

Good catch — addressed this directly rather than deferring. Added installAndRegister helper that wraps registerSkillInGroup with a best-effort DB rollback (store.Delete) on failure. This covers all 5 call sites (git direct, OCI direct, installByName, and both registry lookup paths).

If group registration fails, the DB record is removed so a retry starts fresh rather than leaving an orphaned skill record. Files on disk are left in place (a retry with the same content will detect them).

Also added TestInstallFromGitGroupRegistrationRollback to verify the rollback behavior on the git install path, and updated the existing "group registration error" test to assert the Delete call.

[aponcedeleonch]

Missing unit tests for validateHost covering both branches of isDevMode(). Since this is security-critical SSRF protection, it would be good to have tests using t.Setenv("TOOLHIVE_DEV", "true") and without it to verify the protection works correctly in production configuration.

Also missing: unit tests for buildGitReferenceFromRegistryURL edge cases (e.g., http:// URL silent promotion to https://, already prefixed with git://).

[JAORMX]

Added both:

TestParseGitReferenceDevMode in reference_test.go — covers validateHost with TOOLHIVE_DEV=true:

• 5 success cases: localhost, 127.0.0.1, 10.x, 192.168.x, and localhost-with-port all allowed in dev mode (returning http:// URLs)
• 3 error cases: empty host, no repo path, single path component still rejected in dev mode
• Uses t.Setenv in a separate non-parallel test function (//nolint:paralleltest)

TestBuildGitReferenceFromRegistryURL in skillsvc_test.go — covers edge cases:

• https:// → git:// conversion (happy path)
• http:// silent promotion to git://
• git:// pass-through
• https:// with nested path/ref/fragment
• Error cases: empty git ref, unsupported ftp:// scheme, bare string without scheme, missing repo path

[JAORMX]

Rebased onto main and addressed all review feedback in 3106985:

Inconsistent state fix — Rather than deferring, fixed it directly. Added installAndRegister helper that wraps all 5 registerSkillInGroup call sites with a best-effort store.Delete rollback. If group registration fails after a successful install, the DB record is removed so a retry starts fresh instead of leaving an orphaned skill.

New tests:

• TestParseGitReferenceDevMode — covers validateHost with TOOLHIVE_DEV=true (5 success cases for localhost/private IPs allowed, 3 error cases for non-SSRF validation still enforced)
• TestBuildGitReferenceFromRegistryURL — covers URL scheme conversion edge cases (https→git, http silent promotion, git passthrough, error cases)
• TestInstallFromGitGroupRegistrationRollback — verifies the DB rollback on group registration failure for the git install path
• Updated existing "group registration error" test to assert the Delete rollback call