Skip to content

Implement PUT /v1/entries/{type}/{name}/claims endpoint#720

Merged
rdimitrov merged 4 commits intomainfrom
rdimitrov/implement-put-entry-claims
Apr 20, 2026
Merged

Implement PUT /v1/entries/{type}/{name}/claims endpoint#720
rdimitrov merged 4 commits intomainfrom
rdimitrov/implement-put-entry-claims

Conversation

@rdimitrov
Copy link
Copy Markdown
Member

Summary

  • Implements the PUT /v1/entries/{type}/{name}/claims endpoint that was previously returning 501
  • Allows updating claims on a published entry (MCP server or skill) within the managed source
  • Enforces JWT-based authorization: caller's claims must cover both existing and new claims; super-admins bypass
  • Returns 204 on success, 400 for validation errors, 403 for insufficient claims, 404 if entry not found

Changes

  • SQL: Added UpdateRegistryEntryClaims query with nullable claims and :execrows for not-found detection
  • Service interface: Added UpdateEntryClaims method and WithEntryType option
  • Service implementation: New impl_entries.go with serializable transaction, managed source lookup, and dual claims validation
  • API handler: Replaced 501 stub with full implementation including entry type validation and error mapping
  • Tests: 9 handler unit tests + 9 service-layer integration tests covering authorization, not-found, clear-claims, super-admin bypass, and skill-type scenarios
  • Removed obsolete TestV1StubEndpoints (only tested the 501 stub)

Test plan

  • task build — compiles cleanly
  • task lint-fix — 0 issues
  • task test — all unit and integration tests pass (including 18 new tests)
  • End-to-end: docker compose (anonymous mode) — set/update/clear claims, not-found, bad type, skill type
  • End-to-end: native server + mock OIDC (OAuth mode) — authorized update, cross-org rejection (403), JWT scope enforcement (403), no-auth (401), super-admin bypass (204)

Fixes #638

🤖 Generated with Claude Code

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 14, 2026

Codecov Report

❌ Patch coverage is 70.90909% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.47%. Comparing base (e9ccced) to head (495b340).

Files with missing lines Patch % Lines
internal/service/db/impl_entries.go 71.08% 15 Missing and 9 partials ⚠️
internal/service/mocks/mock_service.go 0.00% 12 Missing ⚠️
internal/db/sqlc/registry_entries.sql.go 0.00% 10 Missing ⚠️
internal/api/v1/entries.go 93.93% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #720      +/-   ##
==========================================
+ Coverage   59.80%   60.47%   +0.67%     
==========================================
  Files         106      109       +3     
  Lines       10306    10467     +161     
==========================================
+ Hits         6163     6330     +167     
+ Misses       3600     3583      -17     
- Partials      543      554      +11     

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

rdimitrov and others added 3 commits April 20, 2026 17:45
Replace the 501 stub with a full implementation that allows updating
claims on a published entry (MCP server or skill) within the managed
source. The caller's JWT claims must cover both the entry's existing
claims and the new claims being set; super-admins bypass these checks.

Adds the `UpdateRegistryEntryClaims` SQL query, `UpdateEntryClaims`
service method with serializable transaction, and the `WithEntryType`
option. Includes 9 handler unit tests and 9 service-layer integration
tests covering authorization, not-found, clear-claims, super-admin
bypass, and skill-type scenarios.

Fixes #638

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three places previously carried their own `server`/`skill` switch: the
`updateEntryClaims` handler, `UpdateEntryClaimsOptions.setEntryType`,
and `mapEntryType`. Introduce `service.ValidateEntryType` and the
`EntryTypeServer` / `EntryTypeSkill` constants as the single source of
truth, and rewrite all three call sites to use them. `WithEntryType`
drops its redundant empty-string check and defers entirely to the
setter, which now fully validates the input.

Map `service.ErrNoManagedSource` to `503 Service Unavailable` with a
targeted message instead of `500 Internal Server Error`. The managed
source not being configured is a precondition failure, not an internal
error; 503 signals the same to clients without misrepresenting the
cause. The publish and delete handlers still return 500 for the same
error — normalizing those is out of scope for this PR and should be
handled as a follow-up to stay consistent.

Add unit tests for `options_entries.go` covering `WithEntryType` (valid,
empty, unknown, wrong case, wrong option type), `WithName`,
`WithClaims`, `WithJWTClaims`, and multi-setter composition, plus direct
tests for `ValidateEntryType`. Codecov previously reported 0% coverage
on `options_entries.go`.

Regenerate Swagger docs to reflect the new 503 response.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rdimitrov rdimitrov force-pushed the rdimitrov/implement-put-entry-claims branch from e48552a to 05fc5b5 Compare April 20, 2026 14:45
Update test fixtures to use the current upstream registry schema format
which requires `meta`, `data` wrapper fields, and at least one server
entry. The old format was rejected after the ToolHive format drop in
127c8c8.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@lujunsan lujunsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@blkt blkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could have been split into at least 3 PRs.

Comment thread internal/service/entry_types.go
@rdimitrov rdimitrov merged commit 5803fd2 into main Apr 20, 2026
29 of 30 checks passed
@rdimitrov rdimitrov deleted the rdimitrov/implement-put-entry-claims branch April 20, 2026 16:15
rdimitrov added a commit that referenced this pull request Apr 20, 2026
## Summary

- Drop the standalone `service.ValidateEntryType` helper and inline the
switch directly into `setEntryType`, so `WithEntryType` is the single
validation site for entry type strings (addresses @blkt's review nit on
#720).
- Add an `ErrInvalidEntryType` sentinel so the handler can still map
invalid input to HTTP 400 after validation moves inside the service
layer.
- Simplify `mapEntryType` — drop the redundant pre-check and let the
switch default return the same sentinel.

Fixes #736

## Test plan

- [x] `task build`
- [x] `task lint-fix` — 0 issues
- [x] `task test` — all unit and integration tests pass, including
updated `TestWithEntryType` (asserts `ErrorIs(err,
ErrInvalidEntryType)`) and the handler's `unsupported entry type` case
(now exercises the service error path with a mock returning
`ErrInvalidEntryType`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

As a user with the manageEntries role, I want to update claims for a published entry, so that I can modify claim data in the managed source

4 participants