fix: add missing DELETE endpoint for skills by optimus-fulcria · Pull Request #286 · agentregistry-dev/agentregistry

optimus-fulcria · 2026-03-06T02:05:48Z

Description

Adds the missing DELETE /v0/skills/{skillName}/versions/{version} endpoint to the registry API.

Motivation: The arctl skill delete CLI command and client were already implemented, but the server had no corresponding endpoint, causing 404 errors.

What changed:

Added DELETE handler in internal/registry/api/handlers/v0/skills.go with proper huma error codes (huma.Error404NotFound, huma.Error401Unauthorized, huma.Error403Forbidden)
Added DeleteSkill(ctx, tx, name, version) to the database layer — deletes the skill row and promotes the next latest version in a transaction
Added DeleteSkill to service interface and fake registry for testing
Regenerated OpenAPI spec and TypeScript client
Added e2e tests covering the full delete lifecycle

Fixes #223

Change Type

/kind fix

Changelog

Add DELETE endpoint for removing skill versions from the registry

Additional Notes

Test plan

go build ./... passes
go vet -tags e2e ./e2e/... passes
E2e tests added in e2e/skill_publish_test.go:
- TestSkillDelete: publish → verify exists → delete via CLI → confirm 404 → re-delete fails
- TestSkillDeletePromotesLatest: publish v0.0.1 and v0.0.2 → verify latest is v0.0.2 → delete v0.0.2 → verify latest promoted to v0.0.1
- TestSkillDeleteNotFound: HTTP DELETE on non-existent skill returns 404
Database deletion handles is_latest promotion in a transaction
Uses proper huma error codes (404, 401, 403) instead of generic errors

peterj · 2026-03-06T18:31:50Z

please update the PR template and add tests (probably e2e would make sense -- we have some skill tests already in the e2e folder)

peterj

please update the PR template and add tests (probably e2e would make sense -- we have some skill tests already in the e2e folder)

optimus-fulcria · 2026-03-06T23:10:41Z

PR template updated. I'll add e2e tests - looking at the existing e2e/skill tests as a pattern.

github-actions · 2026-03-06T23:10:46Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-06T23:12:23Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-06T23:12:37Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

optimus-fulcria · 2026-03-06T23:35:01Z

Both items addressed:

PR template updated.
E2e test added in 24d3b34 (TestSkillDelete in skill_publish_test.go): publishes a skill via --github, verifies 200, deletes via CLI, confirms 404.

optimus-fulcria · 2026-03-07T00:10:38Z

Done in 24d3b34: added TestSkillDelete e2e test that publishes a skill, verifies it exists, deletes it via the DELETE endpoint, then confirms 404.

github-actions · 2026-03-07T00:10:48Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-07T00:12:25Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-07T00:12:46Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

optimus-fulcria · 2026-03-07T00:12:46Z

@peterj Updated per your feedback:

PR template updated - Body now follows the .github/PULL_REQUEST_TEMPLATE.md format with Description, Change Type, Changelog, and Test plan sections.
E2e tests enhanced (18e2bcd) - Added two more test cases on top of the existing TestSkillDelete:
- delete_again_fails: verifies that deleting an already-deleted skill returns a non-zero exit code
- TestSkillDeleteNotFound: verifies that the HTTP DELETE API returns 404 for a skill that was never published

All three e2e tests together cover: publish -> verify exists -> delete via CLI -> confirm 404 -> idempotent re-delete fails -> direct HTTP DELETE on non-existent returns 404.

github-actions · 2026-03-07T01:05:19Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-07T01:39:57Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

inFocus7

overall lgtm, two small requests and then it's good on my end

internal/registry/api/handlers/v0/skills.go

inFocus7 · 2026-03-09T22:13:00Z

e2e/skill_publish_test.go

+
+// TestSkillDelete tests publishing a skill and then deleting it via the
+// DELETE /v0/skills/{name}/versions/{version} endpoint.
+func TestSkillDelete(t *testing.T) {


can we add a test that we correctly handle promoting a previously-published skill as latest when the current latest is deleted?

github-actions · 2026-03-11T10:39:54Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

optimus-fulcria · 2026-03-11T11:39:44Z

Addressed both review items:

Error codes (per @inFocus7): Now returns proper HTTP status codes — 404 for not found, 401 for unauthenticated, 403 for forbidden — instead of lumping all into 404.
Latest promotion test: Added TestSkillDeletePromotesLatest — publishes v0.0.1 and v0.0.2, deletes v0.0.2 (current latest), then verifies "latest" resolves to v0.0.1.

github-actions · 2026-03-11T11:39:49Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

optimus-fulcria · 2026-03-11T14:08:45Z

PR template updated, e2e tests added:

TestSkillDelete: full publish → delete → verify 404 lifecycle
TestSkillDeleteNotFound: verifies 404 on non-existent skill
Latest version promotion tested when deleting the latest version

All CI checks pass. Ready for re-review.

github-actions · 2026-03-11T16:10:00Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

github-actions · 2026-03-11T16:10:01Z

You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks!

The skill delete CLI command and client were implemented but the server had no corresponding DELETE endpoint, causing 404 errors. Add the endpoint at all layers: database, service, fake registry, and API handler. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Publishes a skill, verifies it exists, deletes it via CLI, and confirms it returns 404 afterward. Tests the DELETE endpoint added for agentregistry-dev#223. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add two additional e2e test cases for skill deletion: - delete_again_fails: verifies that deleting an already-deleted skill returns a non-zero exit code (idempotency check) - TestSkillDeleteNotFound: verifies that the HTTP DELETE API returns 404 when targeting a skill that was never published Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The exists_before_delete test was hitting /skills/{name} which doesn't exist - corrected to /skills/{name}/versions/{version}. Regenerated openapi.yaml and TypeScript client to include the DELETE endpoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Address review feedback: - Return 401/403 for auth errors instead of lumping into 404 - Add e2e test verifying latest promotion when current latest is deleted Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- Replace deprecated --github flag with --git (renamed in agentregistry-dev#277) - Use createSkillDir helper instead of inline dir creation - Add t.Cleanup to delete remaining versions in promote test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

timflannagan

Approach largely LGTM. I think we could add some unit tests here, but I can tackle in a follow-up.

timflannagan · 2026-03-11T19:39:05Z

(Had to force push to fix a merge conflict)

stale

github-actions bot added do-not-merge/description-invalid do-not-merge/kind-invalid do-not-merge/release-note-invalid labels Mar 6, 2026

peterj previously requested changes Mar 6, 2026

View reviewed changes

github-actions bot added kind/fix release-note and removed do-not-merge/description-invalid do-not-merge/kind-invalid do-not-merge/release-note-invalid labels Mar 6, 2026

inFocus7 reviewed Mar 9, 2026

View reviewed changes

timflannagan mentioned this pull request Mar 10, 2026

Improve daemon lifecycle management #307

Open

Optimus (AI Agent) and others added 6 commits March 11, 2026 15:35

test(e2e): add TestSkillDelete for skill deletion endpoint

c2c52b3

Publishes a skill, verifies it exists, deletes it via CLI, and confirms it returns 404 afterward. Tests the DELETE endpoint added for agentregistry-dev#223. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix: return proper HTTP status codes and add latest promotion test

020f62e

Address review feedback: - Return 401/403 for auth errors instead of lumping into 404 - Add e2e test verifying latest promotion when current latest is deleted Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

timflannagan force-pushed the fix/skill-delete-endpoint-223 branch from 9d764b2 to d4f022e Compare March 11, 2026 19:36

timflannagan approved these changes Mar 11, 2026

View reviewed changes

timflannagan added this pull request to the merge queue Mar 11, 2026

Merged via the queue into agentregistry-dev:main with commit 938806e Mar 11, 2026
7 checks passed

timflannagan mentioned this pull request Mar 12, 2026

Improve skill deletion unit testing #336

Open

Conversation

optimus-fulcria commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change Type

Changelog

Additional Notes

Test plan

Uh oh!

peterj commented Mar 6, 2026

Uh oh!

peterj left a comment

Choose a reason for hiding this comment

Uh oh!

optimus-fulcria commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026

Uh oh!

github-actions bot commented Mar 6, 2026

Uh oh!

optimus-fulcria commented Mar 6, 2026

Uh oh!

optimus-fulcria commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

optimus-fulcria commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

github-actions bot commented Mar 7, 2026

Uh oh!

inFocus7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

inFocus7 Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

optimus-fulcria commented Mar 11, 2026

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

optimus-fulcria commented Mar 11, 2026

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

github-actions bot commented Mar 11, 2026

Uh oh!

timflannagan left a comment

Choose a reason for hiding this comment

Uh oh!

timflannagan commented Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

optimus-fulcria commented Mar 6, 2026 •

edited

Loading