Skip to content

Install local skill builds by name when tag differs#5182

Open
samuv wants to merge 2 commits intomainfrom
oci-local-resolver
Open

Install local skill builds by name when tag differs#5182
samuv wants to merge 2 commits intomainfrom
oci-local-resolver

Conversation

@samuv
Copy link
Copy Markdown
Contributor

@samuv samuv commented May 4, 2026

Summary

  • POST /api/v1beta/skills with {"name":"<skill-name>","scope":"user","version":"<tag-or-version>"} returned 404 Not Found ("not found in local store or registry; install by OCI reference: …") when the only matching artifact was a local build tagged with something other than the skill name — e.g. a build produced by thv skill build --tag v0.0.1. The build was clearly visible in GET /api/v1beta/skills/builds (which exposes name, tag, version, digest), but install-by-name only resolved a literal tag in the local OCI store, so the skill name was effectively unbindable to that build.
  • The version field in the request was also being interpreted strictly as cfg.Version (the artifact's SKILL.md frontmatter version). Many local builds have an empty cfg.Version while carrying a meaningful OCI tag, and the existing OCI-name install path already treats version as the tag — so a caller passing the tag string from GET /skills/builds as version was silently filtered out.
  • Two-commit fix: a pure refactor that extracts the install-options hydration block, then the actual change — resolveFromLocalStore now falls back to scanning local-build-marked tags by declared skill name (and version, when specified) when the direct tag lookup misses or its version disagrees with the request. The version filter matches against either cfg.Version or the local store tag. Multiple matches resolve via a tag-equals-name tie-breaker; otherwise the API returns 409 Conflict with the candidate list.

Resolution flow after the change

flowchart TD
    in[POST /skills name + optional version] --> direct{Direct tag lookup of name}
    direct -->|"found, name matches, version ok"| useDirect[Use direct tag]
    direct -->|"found, name mismatches"| err422[422 supply-chain - existing]
    direct -->|"found, version mismatches cfg"| scan[Scan local-build tags]
    direct -->|not found| scan
    scan --> filter{"Filter: cfg.Name == name AND (version empty OR cfg.Version == version OR tag == version)"}
    filter -->|0 matches| reg[Fall through to registry - existing]
    filter -->|1 match| useScan[Use that match]
    filter -->|multiple matches| tie{Any tag equals name}
    tie -->|yes| useTagged[Use tag-named build]
    tie -->|no| ambig[409 Conflict - list candidates]
Loading
  1. Direct tag lookup of opts.Name in the local OCI store (existing fast path):
  • Hit, name matches, version satisfied → use it.
  • Hit, name mismatches → 422 Unprocessable Entity (existing supply-chain check).
  • Hit, name matches but version disagrees with cfg.Version → fall through to scan.
  • Miss → fall through to scan.
  1. Local-build scan filters tags by cfg.Name == opts.Name AND (opts.Version empty OR cfg.Version == opts.Version OR tag == opts.Version):
    • 0 matches → fall through to the existing registry lookup (preserving the 404/registry-resolved paths).
    • 1 match → use it.
    • Multiple matches → prefer the one whose tag equals the skill name; otherwise return 409 Conflict listing each candidate's (tag, version) so callers can disambiguate by passing version.
      The scan is gated on the dev.stacklok.toolhive.local-build descriptor annotation, so only artifacts produced by thv skill build are considered — pulled artifacts cached in the local OCI store stay invisible, matching ListBuilds. Reference on the resulting InstalledSkill defaults to the resolved local store tag (e.g. v0.0.1) rather than the skill name, so a later thv skill push --reference <tag> can re-resolve the artifact.

Type of change

  • Bug fix (install-by-name now sees local builds whose tag is not the skill name; version accepts either the artifact's declared version or the local store tag)

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix — 0 issues)
  • Seven new TestInstallFromLocalStore cases assert the new behavior end-to-end via svc.Install:
    • scan resolves by skill name when tag differs — the original "not found" reproducer.
    • scan version matches local store tag — the second reproducer: cfg.Version differs from the displayed tag, caller passes the tag as version.
    • scan version filter selects matching build — two builds, same name, different cfg.Versions; matching version wins.
    • scan ambiguous matches return 409 with candidate list — two builds with same name+version, no tag-equals-name winner.
    • scan version mismatch falls through to registry — name matches but neither cfg.Version nor tag matches; falls through to the existing registry-lookup 404.
    • direct version mismatch falls through to scan — direct tag matches name but not version; scan locates the sibling build with the right version.
    • scan ignores non-skill local-build-marked tags — defensive: a local-build-marked entry whose artifactType is not the skill type is skipped.
  • Tightened the table runner so wantErr is asserted alongside wantCode; this also tightens the existing name mismatch in local artifact and corrupt manifest propagates error cases (no behavior change, just stricter assertions).
  • Manual reproduction against thv serve: POST /api/v1beta/skills {"name":"<skill-name>","scope":"user","version":"v0.0.1"} against a local store containing only tag v0.0.1 (with empty cfg.Version) — installs successfully.

Changes

File Change
pkg/skills/skillsvc/install_oci.go Extract hydrateOptsFromLocalBuild. Split resolveFromLocalStore into tryDirectLocalTag + tryLocalBuildScan. Add localBuildMatch, findLocalBuildsByName, pickLocalBuildMatch, ambiguousLocalBuildError. Direct path now falls through to scan when a caller-supplied version disagrees. Scan version filter accepts either cfg.Version or the local store tag. New imports: github.com/opencontainers/go-digest, ociskills "github.com/stacklok/toolhive-core/oci/skills".
pkg/skills/skillsvc/install_oci_test.go Seven new TestInstallFromLocalStore cases; runner asserts wantErr when wantCode is set.

Does this introduce a user-facing change?

Yes — bug-fix only.

  • Callers that pass a plain skill name (with or without version) on POST /api/v1beta/skills now resolve to a local build whose declared skill name matches, regardless of the OCI tag the build was produced under. This unblocks the natural workflow of thv skill build --tag <version> followed by install-by-name from the studio UI / API.
  • The version field is now matched against either the artifact's declared version (cfg.Version) or the local store tag, matching what GET /skills/builds displays and the existing OCI-name install behavior.
  • New 409 Conflict response when the local store has multiple builds for the same skill name and the call did not supply a version to disambiguate (the response body lists each candidate's tag and version).
  • Existing happy paths (direct tag matches name, OCI references, registry lookup, supply-chain 422) are unchanged.
    Commit commands are unchanged from the previous turn. Want me to run them?

@samuv samuv self-assigned this May 4, 2026
@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label May 4, 2026
@samuv samuv requested a review from JAORMX as a code owner May 4, 2026 15:46
@samuv samuv changed the title Oci local resolver Install local skill builds by name when tag differs May 4, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 73.80952% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.72%. Comparing base (729e56a) to head (4718c99).

Files with missing lines Patch % Lines
pkg/skills/skillsvc/install_oci.go 73.80% 13 Missing and 9 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5182   +/-   ##
=======================================
  Coverage   67.72%   67.72%           
=======================================
  Files         607      607           
  Lines       61984    62063   +79     
=======================================
+ Hits        41978    42033   +55     
- Misses      16845    16859   +14     
- Partials     3161     3171   +10     

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

@samuv samuv force-pushed the oci-local-resolver branch from 7b5e147 to 78171d6 Compare May 5, 2026 07:57
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 5, 2026
@samuv samuv force-pushed the oci-local-resolver branch from 78171d6 to 7c23e6c Compare May 5, 2026 08:44
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 5, 2026
@samuv samuv force-pushed the oci-local-resolver branch from 7c23e6c to a25692f Compare May 5, 2026 16:26
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 5, 2026
samuv added 2 commits May 6, 2026 10:58
Pull the LayerData/Digest/Reference/Version assignment block out of resolveFromLocalStore into a small hydrateOptsFromLocalBuild helper so a follow-up can share it between the existing direct-tag path and a new local-build name-scan path. No behavior change.
A local skill build tagged with something other than its declared skill name (e.g. thv skill build --tag v0.0.1) was not findable via POST /api/v1beta/skills with a plain name + version body: install-by-name did a literal tag lookup against the local OCI store, so the only resolvable handle was the OCI tag string. Callers got 404 "not found in local store or registry" even though the build was visible in GET /skills/builds.

Extend resolveFromLocalStore so that when the direct tag lookup misses, or when a caller-supplied version disagrees with the direct match, it walks the local-build-marked tags for one whose declared skill name matches. The version filter accepts either the artifact's cfg.Version or the local store tag, mirroring the OCI-name install path that already treats version as the tag and matching the GET /skills/builds output where many artifacts carry a tag like "v0.0.1" with an empty SKILL.md version. With multiple matches, prefer the build whose tag equals the skill name; otherwise return 409 Conflict listing each candidate's tag and version.

Add table-driven cases covering: scan resolves by skill name when tag differs, version matches the local store tag (the user-facing reproducer), version filter narrows multiple matches, version mismatch falls through to registry, direct version mismatch falls through to scan, ambiguous matches return 409, and non-skill local-build-marked tags are ignored. Tighten the test runner so wantErr is also asserted when wantCode is set.
@samuv samuv force-pushed the oci-local-resolver branch from a25692f to 4718c99 Compare May 6, 2026 08:58
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant