Split skillsvc into per-concern files

[samuv]

Summary

• pkg/skills/skillsvc/skillsvc.go had grown to 1983 lines and skillsvc_test.go to 3912 lines, mixing ten distinct concerns (service construction, OCI install, git install, extraction, client resolution, registry lookup, content fetching, build/push, uninstall, list/info).
• Split the production file into 13 focused files by concern and the test file into matching per-area test files, all within the existing skillsvc package. Because everything stays in one Go package, there is no circular-import risk and no exported API surface changes.
• This is a pure file-level reorganisation — no behavioural changes, no renames, no signature changes.

Type of change

• Refactor (non-breaking)

Test plan

• Unit tests (task test / go test ./pkg/skills/skillsvc/...)
• Linting (golangci-lint run ./pkg/skills/skillsvc/... — 0 issues)
• Full repo build (go build ./...)

Changes

File	Change
pkg/skills/skillsvc/service.go	Option, With* constructors, SkillLookup interface, service struct, skillLock, New
pkg/skills/skillsvc/install.go	Install dispatch, installByName, installFromRegistryLookup, installAndRegister, registerSkillInGroup
pkg/skills/skillsvc/install_oci.go	installFromOCI, resolveFromLocalStore, OCI pull timeouts and size limits
pkg/skills/skillsvc/install_git.go	installFromGit + apply/write/persist stages
pkg/skills/skillsvc/install_extraction.go	installWithExtraction, no-op / upgrade / fresh paths, buildInstalledSkill
pkg/skills/skillsvc/clients.go	Client resolution, validation, and dedup helpers
pkg/skills/skillsvc/oci.go	OCI reference parsing, extractOCIContent, isSkillArtifact
pkg/skills/skillsvc/registry.go	Registry name lookup and buildGitReferenceFromRegistryURL
pkg/skills/skillsvc/content.go	GetContent from git / OCI / URL
pkg/skills/skillsvc/list.go	List, Info
pkg/skills/skillsvc/uninstall.go	Uninstall
pkg/skills/skillsvc/build.go	Validate, Build, Push, ListBuilds, DeleteBuild
pkg/skills/skillsvc/scope.go	normalizeProjectRoot, defaultScope
pkg/skills/skillsvc/testhelpers_test.go	Shared test fixtures (makeLayerData, buildTestArtifact, buildManifestWithLayerSize, …)
pkg/skills/skillsvc/service_test.go	TestNewWithZeroOptions, TestList, TestListFiltersByGroup
pkg/skills/skillsvc/install_test.go	TestInstallPlainNameNotFound, TestInstallWithExtraction, TestInstallAddsSkillToGroup
pkg/skills/skillsvc/install_oci_test.go	TestInstallFromOCI, TestInstallFromLocalStore, TestInstallQualifiedNameOCIFallback
pkg/skills/skillsvc/install_git_test.go	TestInstallFromGit, TestInstallFromGitGroupRegistrationRollback, TestInstallFromRegistryGitFallback
pkg/skills/skillsvc/install_registry_test.go	TestInstallFromRegistry, TestBuildGitReferenceFromRegistryURL, TestSplitQualifiedName, TestResolveRegistryPackagesSubfolder
pkg/skills/skillsvc/uninstall_test.go	TestUninstall, TestConcurrentInstallAndUninstall, TestUninstallRemovesSkillFromGroups
pkg/skills/skillsvc/info_test.go	TestInfo
pkg/skills/skillsvc/content_test.go	TestGetContent
pkg/skills/skillsvc/build_test.go	TestValidate, TestBuild, TestPush, TestListBuilds, TestDeleteBuild, TestValidateOCITagOrReference
pkg/skills/skillsvc/oci_test.go	TestQualifiedOCIRef
pkg/skills/skillsvc/skillsvc.go	Removed
pkg/skills/skillsvc/skillsvc_test.go	Removed

Does this introduce a user-facing change?

No — this is a pure file-level reorganisation within an internal package. No exported API, behaviour, or logic changes.

Large PR Justification

This refactor must be atomic:

• It's a pure file-move with zero behavioural changes. Every function, type, and constant was moved verbatim — no renames, no signature changes, no logic edits. The line count is inflated because git sees deletions of skillsvc.go (1983 lines) and skillsvc_test.go (3912 lines) alongside the identical content reappearing in 13 production files and 11 test files. Net semantic diff is ~0.
• It cannot be split incrementally. Go requires every symbol in a package to be defined exactly once. Moving functions one-by-one across commits would either (a) break compilation at every intermediate step, or (b) require temporary duplication that defeats the purpose of the refactor. The two commits on this branch (Split skillsvc.go… and Split skillsvc_test.go…) are the minimum-granularity split that keeps go build green at every commit.
• No caller outside the package is touched. The skillsvc package's exported API (New, Option, With*, SkillLookup) is unchanged, so no downstream code is affected.
• Review strategy: review with git diff --find-renames=40% or GitHub's "Split" view — most files show as renames/moves rather than true additions. A good sanity check is git diff main...HEAD -- pkg/skills/skillsvc/ | grep -c '^[+-]' vs. the same command filtered to lines that aren't pure moves.

[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.

Large PR justification has been provided. Thank you!

[github-actions]

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

[codecov]

Codecov Report

❌ Patch coverage is 81.68880% with 193 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.40%. Comparing base (8504427) to head (b2cc066).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/skills/skillsvc/install_git.go	58.11%	40 Missing and 9 partials ⚠️
pkg/skills/skillsvc/oci.go	57.64%	25 Missing and 11 partials ⚠️
pkg/skills/skillsvc/content.go	74.16%	24 Missing and 7 partials ⚠️
pkg/skills/skillsvc/clients.go	75.83%	19 Missing and 10 partials ⚠️
pkg/skills/skillsvc/install_extraction.go	87.73%	8 Missing and 5 partials ⚠️
pkg/skills/skillsvc/build.go	90.00%	7 Missing and 5 partials ⚠️
pkg/skills/skillsvc/uninstall.go	80.00%	4 Missing and 3 partials ⚠️
pkg/skills/skillsvc/install.go	93.81%	5 Missing and 1 partial ⚠️
pkg/skills/skillsvc/install_oci.go	93.42%	4 Missing and 1 partial ⚠️
pkg/skills/skillsvc/registry.go	93.82%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4947      +/-   ##
==========================================
- Coverage   69.43%   69.40%   -0.03%     
==========================================
  Files         539      551      +12     
  Lines       55750    55750              
==========================================
- Hits        38708    38692      -16     
- Misses      14067    14085      +18     
+ Partials     2975     2973       -2     

☔ 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.

[jhrozek]

LGTM. Verified the mechanical claim: symbol counts match (73 prod symbols, 28 Test* funcs, 34 total test funcs), SPDX headers and imports are consistent across the new files, and go build, go vet, go test, golangci-lint are all green on the package.