Update E2E tests for registry-based skill resolution by JAORMX · Pull Request #4202 · stacklok/toolhive

JAORMX · 2026-03-18T11:58:03Z

Summary

Plain name installs without a prior build now return 404 instead of creating a dead-end "pending" record (Resolve plain skill names from registry during install #4200). All E2E tests that relied on the pending path needed updating to use the build-then-install flow.
Adds a buildAndInstallSkill helper to reduce boilerplate across API tests
Adds a test verifying the 404 not-found behavior for unresolvable plain names

Depends on #4200

Type of change

Bug fix

Test plan

Linting (task lint-fix)
E2E tests (task test-e2e) -- requires Resolve plain skill names from registry during install #4200 to be merged first

Changes

File	Change
`test/e2e/api_skills_test.go`	Add `buildAndInstallSkill` helper; replace pending-status test with 404 test; update all tests that installed by plain name to build first (install, list, info, uninstall, group integration, overwrite protection, lifecycle)
`test/e2e/cli_skills_test.go`	Update install-and-list, info, and uninstall tests to build before installing; update lifecycle test comment

Special notes for reviewers

Also depends on Default skills to the "default" group on install #4201 (default group for skills) for group integration tests to pass in CI. Without that fix, skills installed without --group won't be in any group, but the group integration tests explicitly pass a group so they are not affected.

Generated with Claude Code

codecov · 2026-03-18T12:17:35Z

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.75%. Comparing base (66a114b) to head (eea80f1).
⚠️ Report is 1 commits behind head on jaosorior/skill-install-registry-lookup.

Files with missing lines	Patch %	Lines
pkg/api/server.go	0.00%	6 Missing ⚠️

Additional details and impacted files

@@                           Coverage Diff                            @@
##           jaosorior/skill-install-registry-lookup    #4202   +/-   ##
========================================================================
  Coverage                                    68.74%   68.75%           
========================================================================
  Files                                          468      468           
  Lines                                        47174    47179    +5     
========================================================================
+ Hits                                         32432    32437    +5     
- Misses                                       12035    12037    +2     
+ Partials                                      2707     2705    -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.

Plain name installs without a prior build now return 404 instead of creating a dead-end "pending" record. Update all E2E tests that relied on the pending path to use the build-then-install flow, and add a test verifying the 404 behavior for unresolvable names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Exercises the full registry lookup chain end-to-end: 1. Start an in-process OCI registry (go-containerregistry/pkg/registry) 2. Build a skill and push it to the in-process registry 3. Create an upstream-format registry JSON with a skill entry pointing to the in-process registry 4. Configure the thv server to use that registry file 5. Install by plain name and verify the skill resolves through the registry, pulls from OCI, and installs with full metadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Two fixes for CI failures: 1. Enable plain HTTP for the OCI registry client when TOOLHIVE_DEV=true so E2E tests can push/pull to an in-process httptest registry without TLS. 2. Update duplicate/overwrite tests to expect 201 (idempotent no-op) instead of 409 Conflict. With real build-then-install, reinstalling the same digest is correctly idempotent. The old 409 was an artifact of the removed pending-install path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The skill push API returns 204 No Content on success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The upstream registry JSON schema validation rejects the test fixture. The in-process OCI registry, plain HTTP push, and registry update plumbing all work (verified in prior CI runs), but the upstream format schema requires fields beyond what the test provides. Mark as pending until the exact schema requirements are debugged. All other E2E tests (48/49) pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The upstream registry file validator requires at least one server or group. Our test JSON had only skills with an empty servers array, which was rejected with 502. Add a minimal dummy server entry to satisfy the validation while keeping the focus on skill resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update E2E tests for registry-based skill resolution Plain name installs without a prior build now return 404 instead of creating a dead-end "pending" record. Update all E2E tests that relied on the pending path to use the build-then-install flow, and add a test verifying the 404 behavior for unresolvable names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add E2E test for registry-based skill name resolution Exercises the full registry lookup chain end-to-end: 1. Start an in-process OCI registry (go-containerregistry/pkg/registry) 2. Build a skill and push it to the in-process registry 3. Create an upstream-format registry JSON with a skill entry pointing to the in-process registry 4. Configure the thv server to use that registry file 5. Install by plain name and verify the skill resolves through the registry, pulls from OCI, and installs with full metadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix E2E test failures: plain HTTP for dev, idempotent installs Two fixes for CI failures: 1. Enable plain HTTP for the OCI registry client when TOOLHIVE_DEV=true so E2E tests can push/pull to an in-process httptest registry without TLS. 2. Update duplicate/overwrite tests to expect 201 (idempotent no-op) instead of 409 Conflict. With real build-then-install, reinstalling the same digest is correctly idempotent. The old 409 was an artifact of the removed pending-install path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix push assertion: expect 204 No Content, not 200 The skill push API returns 204 No Content on success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Temporarily skip registry lookup E2E test The upstream registry JSON schema validation rejects the test fixture. The in-process OCI registry, plain HTTP push, and registry update plumbing all work (verified in prior CI runs), but the upstream format schema requires fields beyond what the test provides. Mark as pending until the exact schema requirements are debugged. All other E2E tests (48/49) pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix registry lookup E2E: add dummy server to pass validation The upstream registry file validator requires at least one server or group. Our test JSON had only skills with an empty servers array, which was rejected with 502. Add a minimal dummy server entry to satisfy the validation while keeping the focus on skill resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Resolve plain skill names from registry during install When `thv skill install <name>` does not find the skill in the local OCI store, the code previously created a dead-end "pending" DB record. Now it queries the configured skill registry/index for an exact name match and, if found, extracts the OCI reference and installs from it. If no match is found anywhere, a 404 with an actionable error message is returned. This is Phase 2 of #4015. The resolution cascade is now: 1. Local OCI store (from `thv skill build`) 2. Registry/index lookup (new) 3. Actionable 404 error (replaces silent pending record) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use lazy skill lookup to pick up registry config changes The SkillLookup was created once at server startup from the config at that point. If the registry was changed at runtime (via the API or thv config set-registry), the skill lookup still used the old provider, so plain-name installs could not find skills in the new registry. Replace the eager buildSkillLookupOption with a lazySkillLookup that calls GetDefaultProviderWithConfig on each SearchSkills invocation. After ResetDefaultProvider (called on every registry config change), the next lookup creates a fresh provider from the current config. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update E2E tests for registry-based skill resolution (#4202) * Update E2E tests for registry-based skill resolution Plain name installs without a prior build now return 404 instead of creating a dead-end "pending" record. Update all E2E tests that relied on the pending path to use the build-then-install flow, and add a test verifying the 404 behavior for unresolvable names. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add E2E test for registry-based skill name resolution Exercises the full registry lookup chain end-to-end: 1. Start an in-process OCI registry (go-containerregistry/pkg/registry) 2. Build a skill and push it to the in-process registry 3. Create an upstream-format registry JSON with a skill entry pointing to the in-process registry 4. Configure the thv server to use that registry file 5. Install by plain name and verify the skill resolves through the registry, pulls from OCI, and installs with full metadata Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix E2E test failures: plain HTTP for dev, idempotent installs Two fixes for CI failures: 1. Enable plain HTTP for the OCI registry client when TOOLHIVE_DEV=true so E2E tests can push/pull to an in-process httptest registry without TLS. 2. Update duplicate/overwrite tests to expect 201 (idempotent no-op) instead of 409 Conflict. With real build-then-install, reinstalling the same digest is correctly idempotent. The old 409 was an artifact of the removed pending-install path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix push assertion: expect 204 No Content, not 200 The skill push API returns 204 No Content on success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Temporarily skip registry lookup E2E test The upstream registry JSON schema validation rejects the test fixture. The in-process OCI registry, plain HTTP push, and registry update plumbing all work (verified in prior CI runs), but the upstream format schema requires fields beyond what the test provides. Mark as pending until the exact schema requirements are debugged. All other E2E tests (48/49) pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix registry lookup E2E: add dummy server to pass validation The upstream registry file validator requires at least one server or group. Our test JSON had only skills with an empty servers array, which was rejected with 502. Add a minimal dummy server entry to satisfy the validation while keeping the focus on skill resolution. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Document selection policy for registry package resolution Add a comment explaining that the first OCI package in registry order is chosen when exactly one skill matches, addressing review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix Skills API E2E test polluting api-clients CI job The Skills API Describe had the `api-clients` label, causing it to run in the api-clients CI job. The registry lookup test modified the shared config directory's registry to a test-only file but never restored it. Subsequent client tests failed with 404 because "osv" could no longer be resolved from the corrupted registry. Move Skills API tests to the `api-registry` label where registry mutations are expected, and add DeferCleanup to reset the registry config after the test (matching the pattern in api_registry_test.go). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JAORMX requested review from ChrisJBurns, amirejaz, jhrozek, lujunsan, rdimitrov and yrobla as code owners March 18, 2026 11:58

github-actions bot added the size/S Small PR: 100-299 lines changed label Mar 18, 2026

github-actions bot added size/S Small PR: 100-299 lines changed size/M Medium PR: 300-599 lines changed and removed size/S Small PR: 100-299 lines changed size/M Medium PR: 300-599 lines changed labels Mar 18, 2026

JAORMX force-pushed the jaosorior/skill-install-e2e-registry-lookup branch from e1f26f2 to 452461e Compare March 18, 2026 14:43

github-actions bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Mar 18, 2026

JAORMX force-pushed the jaosorior/skill-install-registry-lookup branch from 0fe2e7a to 66a114b Compare March 18, 2026 14:55

JAORMX and others added 6 commits March 18, 2026 16:55

Fix push assertion: expect 204 No Content, not 200

70d2ec5

The skill push API returns 204 No Content on success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JAORMX force-pushed the jaosorior/skill-install-e2e-registry-lookup branch from 452461e to eea80f1 Compare March 18, 2026 14:55

github-actions bot removed the size/M Medium PR: 300-599 lines changed label Mar 18, 2026

github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 18, 2026

JAORMX merged commit 54f40ba into jaosorior/skill-install-registry-lookup Mar 19, 2026
117 of 122 checks passed

JAORMX deleted the jaosorior/skill-install-e2e-registry-lookup branch March 19, 2026 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update E2E tests for registry-based skill resolution#4202

Update E2E tests for registry-based skill resolution#4202
JAORMX merged 6 commits intojaosorior/skill-install-registry-lookupfrom
jaosorior/skill-install-e2e-registry-lookup

JAORMX commented Mar 18, 2026

Uh oh!

codecov bot commented Mar 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

JAORMX commented Mar 18, 2026

Summary

Type of change

Test plan

Changes

Special notes for reviewers

Uh oh!

codecov bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov bot commented Mar 18, 2026 •

edited

Loading