From d076cee903deae43a92688100a38924916e68d27 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Thu, 12 Nov 2026 08:30:10 -0800 Subject: [PATCH] feat(skills): let authors edit and withdraw pending submissions (#51312) --- litellm/models/skill_submission.py | 12 +- litellm/proxy/_lazy_openapi_snapshot.json | 359 +++++++++++++++++- litellm/proxy/_types.py | 2 + .../skill_hub_mutations.py | 17 +- .../skill_hub_submissions.py | 181 ++++++++- litellm/proxy/proxy_server.py | 1 + .../skill_submission_repository.py | 88 ++++- litellm/types/proxy/skill_hub.py | 13 + ...laudeCodePluginsPanel.integration.test.tsx | 101 ++++- .../_components/ClaudeCodePluginsPanel.tsx | 1 + .../skills/_components/SkillSubmissions.tsx | 308 +++++++++------ .../skills/_components/add_plugin_form.tsx | 84 +++- .../src/components/networking.tsx | 32 ++ .../src/i18n/locales/en.json | 13 +- .../src/lib/http/schema.d.ts | 223 ++++++++++- .../database/test_skill_submissions.py | 57 +++ .../test_skill_hub_endpoints.py | 121 ++++++ ...t_request_body_decompression_middleware.py | 1 + .../test_request_size_limit_middleware.py | 1 + 19 files changed, 1468 insertions(+), 147 deletions(-) diff --git a/litellm/models/skill_submission.py b/litellm/models/skill_submission.py index 09d722fa69f..59587588a35 100644 --- a/litellm/models/skill_submission.py +++ b/litellm/models/skill_submission.py @@ -11,7 +11,7 @@ from litellm.models.plugin_upload import ( ) from litellm.types.proxy.skill_scanning import SkillScan -SubmissionStatus: TypeAlias = Literal["pending_review", "approved", "rejected"] +SubmissionStatus: TypeAlias = Literal["pending_review", "approved", "rejected", "withdrawn"] SubmissionOperation: TypeAlias = Literal["create", "update"] @@ -38,6 +38,7 @@ class SkillSubmission: reviewed_by: str | None reviewed_at: datetime | None rejection_reason: str | None + revision: int = 0 @dataclass(frozen=True, slots=True) @@ -99,6 +100,14 @@ class SkillSubmissionStore(Protocol): async def get_archive(self, submission_id: str, submitted_by: str | None) -> bytes | None: ... + async def edit( + self, submission_id: str, owner: str, revision: int, upload: PluginPublication + ) -> SkillSubmission | SubmissionMissing | SubmissionConflict | SubmissionLimitExceeded: ... + + async def withdraw( + self, submission_id: str, owner: str, revision: int + ) -> SkillSubmission | SubmissionMissing | SubmissionConflict: ... + async def review( self, submission_id: str, @@ -107,4 +116,5 @@ class SkillSubmissionStore(Protocol): reason: str | None, *, publication_base_url: str | None = None, + expected_revision: int | None = None, ) -> SubmissionReviewResult: ... diff --git a/litellm/proxy/_lazy_openapi_snapshot.json b/litellm/proxy/_lazy_openapi_snapshot.json index b99fc8b61e1..fa0c423fc25 100644 --- a/litellm/proxy/_lazy_openapi_snapshot.json +++ b/litellm/proxy/_lazy_openapi_snapshot.json @@ -59670,6 +59670,38 @@ "skill_hub": { "components": { "schemas": { + "Body_edit_submission_archive_skill_hub_submissions__submission_id__upload_put": { + "properties": { + "expected_revision": { + "minimum": 0.0, + "title": "Expected Revision", + "type": "integer" + }, + "file": { + "contentMediaType": "application/octet-stream", + "description": "Replacement Agent Skills ZIP with the same declared skill name", + "title": "File", + "type": "string" + }, + "version": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Version" + } + }, + "required": [ + "file", + "expected_revision" + ], + "title": "Body_edit_submission_archive_skill_hub_submissions__submission_id__upload_put", + "type": "object" + }, "Body_replace_skill_archive_skill_hub_skills__name__upload_put": { "properties": { "expected_updated_at": { @@ -60227,6 +60259,18 @@ "SkillHubRejection": { "additionalProperties": false, "properties": { + "expected_revision": { + "anyOf": [ + { + "minimum": 0.0, + "type": "integer" + }, + { + "type": "null" + } + ], + "title": "Expected Revision" + }, "reason": { "anyOf": [ { @@ -60536,6 +60580,12 @@ ], "title": "Base Updated At" }, + "can_edit": { + "default": false, + "description": "Whether the caller may edit or withdraw this pending submission", + "title": "Can Edit", + "type": "boolean" + }, "created_at": { "format": "date-time", "title": "Created At", @@ -60594,6 +60644,13 @@ ], "title": "Reviewed By" }, + "revision": { + "default": 0, + "description": "Proposal revision used to prevent stale edits and reviews", + "minimum": 0.0, + "title": "Revision", + "type": "integer" + }, "skill_name": { "title": "Skill Name", "type": "string" @@ -60602,7 +60659,8 @@ "enum": [ "pending_review", "approved", - "rejected" + "rejected", + "withdrawn" ], "title": "Status", "type": "string" @@ -60653,6 +60711,95 @@ "title": "SkillHubSubmissionDetail", "type": "object" }, + "SkillHubSubmissionEdit": { + "additionalProperties": false, + "properties": { + "description": { + "anyOf": [ + { + "maxLength": 1024, + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Description" + }, + "domain": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Domain" + }, + "expected_revision": { + "description": "Revision read from the submission detail", + "minimum": 0.0, + "title": "Expected Revision", + "type": "integer" + }, + "expected_updated_at": { + "anyOf": [ + { + "format": "date-time", + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Published updated_at read by the editor. A stale value returns 409", + "title": "Expected Updated At" + }, + "keywords": { + "anyOf": [ + { + "items": { + "type": "string" + }, + "type": "array" + }, + { + "type": "null" + } + ], + "title": "Keywords" + }, + "namespace": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Namespace" + }, + "version": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Explicit higher SemVer, or omit/null for an automatic patch bump", + "title": "Version" + } + }, + "required": [ + "expected_revision" + ], + "title": "SkillHubSubmissionEdit", + "type": "object" + }, "SkillHubSubmissionList": { "properties": { "count": { @@ -60679,6 +60826,22 @@ "title": "SkillHubSubmissionList", "type": "object" }, + "SkillHubSubmissionRevision": { + "additionalProperties": false, + "properties": { + "expected_revision": { + "description": "Revision read from the submission detail", + "minimum": 0.0, + "title": "Expected Revision", + "type": "integer" + } + }, + "required": [ + "expected_revision" + ], + "title": "SkillHubSubmissionRevision", + "type": "object" + }, "SkillHubVersion": { "properties": { "content_kind": { @@ -62118,7 +62281,8 @@ "enum": [ "pending_review", "approved", - "rejected" + "rejected", + "withdrawn" ], "type": "string" }, @@ -62188,7 +62352,7 @@ }, "/skill-hub/submissions/{submission_id}": { "get": { - "description": "Read an immutable proposal and its currently visible published skill.", + "description": "Read a proposal revision and its currently visible published skill.", "operationId": "get_submission_skill_hub_submissions__submission_id__get", "parameters": [ { @@ -62232,6 +62396,62 @@ "tags": [ "skill_hub" ] + }, + "put": { + "description": "Replace a pending proposal's metadata, preserving files. Only its original submitting user may edit.", + "operationId": "edit_submission_skill_hub_submissions__submission_id__put", + "parameters": [ + { + "in": "path", + "name": "submission_id", + "required": true, + "schema": { + "title": "Submission Id", + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SkillHubSubmissionEdit" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SkillHubSubmission" + } + } + }, + "description": "Successful Response" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + }, + "description": "Validation Error" + } + }, + "security": [ + { + "APIKeyHeader": [] + } + ], + "summary": "Edit Submission", + "tags": [ + "skill_hub" + ] } }, "/skill-hub/submissions/{submission_id}/approve": { @@ -62249,6 +62469,23 @@ } } ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "anyOf": [ + { + "$ref": "#/components/schemas/SkillHubSubmissionRevision" + }, + { + "type": "null" + } + ], + "title": "Body" + } + } + } + }, "responses": { "200": { "content": { @@ -62506,6 +62743,122 @@ "skill_hub" ] } + }, + "/skill-hub/submissions/{submission_id}/upload": { + "put": { + "description": "Replace a pending proposal's files using the upload limits. Only its original submitting user may edit.", + "operationId": "edit_submission_archive_skill_hub_submissions__submission_id__upload_put", + "parameters": [ + { + "in": "path", + "name": "submission_id", + "required": true, + "schema": { + "title": "Submission Id", + "type": "string" + } + } + ], + "requestBody": { + "content": { + "multipart/form-data": { + "schema": { + "$ref": "#/components/schemas/Body_edit_submission_archive_skill_hub_submissions__submission_id__upload_put" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SkillHubSubmission" + } + } + }, + "description": "Successful Response" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + }, + "description": "Validation Error" + } + }, + "security": [ + { + "APIKeyHeader": [] + } + ], + "summary": "Edit Submission Archive", + "tags": [ + "skill_hub" + ] + } + }, + "/skill-hub/submissions/{submission_id}/withdraw": { + "put": { + "description": "Withdraw a pending proposal without publishing it. Only its original submitting user may withdraw.", + "operationId": "withdraw_submission_skill_hub_submissions__submission_id__withdraw_put", + "parameters": [ + { + "in": "path", + "name": "submission_id", + "required": true, + "schema": { + "title": "Submission Id", + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SkillHubSubmissionRevision" + } + } + }, + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/SkillHubSubmission" + } + } + }, + "description": "Successful Response" + }, + "422": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/HTTPValidationError" + } + } + }, + "description": "Validation Error" + } + }, + "security": [ + { + "APIKeyHeader": [] + } + ], + "summary": "Withdraw Submission", + "tags": [ + "skill_hub" + ] + } } } }, diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 1eab9d80484..804ad744989 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -470,6 +470,8 @@ class LiteLLMRoutes(enum.Enum): "/skill-hub/submissions", "/skill-hub/submissions/{submission_id}", "/skill-hub/submissions/{submission_id}/archive", + "/skill-hub/submissions/{submission_id}/upload", + "/skill-hub/submissions/{submission_id}/withdraw", "/skill-hub/submissions/{submission_id}/approve", "/skill-hub/submissions/{submission_id}/reject", "/skill-hub/submissions/{submission_id}/scan", diff --git a/litellm/proxy/discovery_endpoints/skill_hub_mutations.py b/litellm/proxy/discovery_endpoints/skill_hub_mutations.py index 63c519923c6..fff81b8a90c 100644 --- a/litellm/proxy/discovery_endpoints/skill_hub_mutations.py +++ b/litellm/proxy/discovery_endpoints/skill_hub_mutations.py @@ -58,6 +58,15 @@ def submission_owner(caller: UserAPIKeyAuth) -> str | None | SkillMutationError: return caller.user_id +def submission_editor(caller: UserAPIKeyAuth) -> str | SkillMutationError: + error: Final = writer_error(caller) + if error is not None: + return error + if not caller.user_id or caller.user_id == DB_UNAVAILABLE_FALLBACK_USER_ID: + return SkillMutationError(403, "Editing submissions requires the original submitting user's identity.") + return caller.user_id + + async def archive_writer(caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)]) -> UserAPIKeyAuth: from litellm.proxy.proxy_server import proxy_logging_obj @@ -149,13 +158,19 @@ async def review_skill( approve: bool, reason: str | None, publication_base_url: str | None = None, + expected_revision: int | None = None, ) -> SubmissionReviewed | SkillMutationError: if not is_proxy_admin(caller): return SkillMutationError(403, "Only proxy admins may review skill submissions.") if submissions is None: return SkillMutationError(503, "Skills Hub requires a connected database") result: Final = await submissions.review( - submission_id, caller.user_id or "proxy-admin", approve, reason, publication_base_url=publication_base_url + submission_id, + caller.user_id or "proxy-admin", + approve, + reason, + publication_base_url=publication_base_url, + expected_revision=expected_revision, ) match result: case SubmissionMissing(): diff --git a/litellm/proxy/discovery_endpoints/skill_hub_submissions.py b/litellm/proxy/discovery_endpoints/skill_hub_submissions.py index c1bb422b2a0..884e15e72a0 100644 --- a/litellm/proxy/discovery_endpoints/skill_hub_submissions.py +++ b/litellm/proxy/discovery_endpoints/skill_hub_submissions.py @@ -1,27 +1,49 @@ import asyncio +from dataclasses import replace from typing import Annotated, Final -from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response +from fastapi import APIRouter, Depends, File, Form, HTTPException, Query, Request, Response, UploadFile -from litellm.models.plugin_upload import PluginPublication -from litellm.models.skill_submission import SkillSubmission, SkillSubmissionStore, SubmissionStatus +from litellm.models.plugin_upload import PluginPublication, PluginPublicationError, PluginUploadStore +from litellm.models.skill_submission import ( + SkillSubmission, + SkillSubmissionStore, + SubmissionConflict, + SubmissionLimitExceeded, + SubmissionMissing, + SubmissionStatus, +) from litellm.proxy._types import UserAPIKeyAuth from litellm.proxy.anthropic_endpoints.claude_code_endpoints.claude_code_skill_access import resolve_skill_visibility from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.deps.repositories import PluginUploadRepositoryDep, SkillSubmissionRepositoryDep from litellm.proxy.discovery_endpoints.agent_skills_archive import build_agent_skill_archive from litellm.proxy.discovery_endpoints.skill_hub_endpoints import ( + prepare_skill_publication, skill_metadata_response, skill_proposal_response, skill_publication_response, ) -from litellm.proxy.discovery_endpoints.skill_hub_mutations import SkillMutationError, review_skill, submission_owner +from litellm.proxy.discovery_endpoints.skill_hub_mutations import ( + SkillMutationError, + archive_writer, + publication_error, + review_skill, + submission_editor, + submission_owner, + write_target, +) +from litellm.proxy.discovery_endpoints.skill_import import MAX_IMPORT_BYTES +from litellm.repositories.plugin_publication import resolve_plugin_version from litellm.types.proxy.skill_hub import ( + SkillHubMetadata, SkillHubRejection, SkillHubReviewResponse, SkillHubSubmission, SkillHubSubmissionDetail, + SkillHubSubmissionEdit, SkillHubSubmissionList, + SkillHubSubmissionRevision, ) router: Final = APIRouter(prefix="/skill-hub/submissions", tags=["Skills Hub"]) @@ -40,7 +62,7 @@ def _owner(caller: UserAPIKeyAuth) -> str | None: return owner -def _response(request: Request, submission: SkillSubmission) -> SkillHubSubmission: +def _response(request: Request, submission: SkillSubmission, caller: UserAPIKeyAuth) -> SkillHubSubmission: return SkillHubSubmission( id=submission.id, skill_name=submission.plugin_name, @@ -55,6 +77,8 @@ def _response(request: Request, submission: SkillSubmission) -> SkillHubSubmissi reviewed_by=submission.reviewed_by, reviewed_at=submission.reviewed_at, rejection_reason=submission.rejection_reason, + revision=submission.revision, + can_edit=submission.status == "pending_review" and submission_editor(caller) == submission.submitted_by, ) @@ -71,7 +95,7 @@ async def list_submissions( owner: Final = _owner(caller) page: Final = await _store(submissions).list(owner, status, limit, offset) return SkillHubSubmissionList( - submissions=tuple(_response(request, row) for row in page.submissions), + submissions=tuple(_response(request, row, caller) for row in page.submissions), count=len(page.submissions), has_more=page.has_more, ) @@ -85,7 +109,7 @@ async def get_submission( uploads: PluginUploadRepositoryDep, caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], ) -> SkillHubSubmissionDetail: - """Read an immutable proposal and its currently visible published skill.""" + """Read a proposal revision and its currently visible published skill.""" owner: Final = _owner(caller) submission: Final = await _store(submissions).get(submission_id, owner) if submission is None: @@ -95,7 +119,7 @@ async def get_submission( current: Final = await uploads.get_metadata(submission.plugin_name) visible: Final = current is not None and (owner is None or (await resolve_skill_visibility(caller)).allows(current)) return SkillHubSubmissionDetail( - submission=_response(request, submission), + submission=_response(request, submission, caller), current_skill=skill_metadata_response(request, current) if current is not None and visible else None, ) @@ -143,11 +167,12 @@ async def _review( caller: UserAPIKeyAuth, approve: bool, reason: str | None, + expected_revision: int | None, ) -> SkillHubReviewResponse: from litellm.proxy.utils import get_custom_url result: Final = await review_skill( - submission_id, submissions, caller, approve, reason, get_custom_url(str(request.base_url)) + submission_id, submissions, caller, approve, reason, get_custom_url(str(request.base_url)), expected_revision ) if isinstance(result, SkillMutationError): raise HTTPException(result.status, result.message) @@ -164,7 +189,7 @@ async def _review( ) return SkillHubReviewResponse( action="approved" if approve else "rejected", - submission=_response(request, result.submission), + submission=_response(request, result.submission, caller), skill=None if result.plugin is None else skill_publication_response(request, publication, result.plugin), ) @@ -175,9 +200,12 @@ async def approve_submission( submission_id: str, submissions: SkillSubmissionRepositoryDep, caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], + body: SkillHubSubmissionRevision | None = None, ) -> SkillHubReviewResponse: """Publish a pending proposal and record its approval atomically. Only proxy admins may review.""" - return await _review(request, submission_id, submissions, caller, True, None) + return await _review( + request, submission_id, submissions, caller, True, None, None if body is None else body.expected_revision + ) @router.put("/{submission_id}/reject", response_model=SkillHubReviewResponse) @@ -189,4 +217,133 @@ async def reject_submission( body: SkillHubRejection | None = None, ) -> SkillHubReviewResponse: """Reject a pending proposal without changing the published skill. Only proxy admins may review.""" - return await _review(request, submission_id, submissions, caller, False, None if body is None else body.reason) + return await _review( + request, + submission_id, + submissions, + caller, + False, + None if body is None else body.reason, + None if body is None else body.expected_revision, + ) + + +def _editor(caller: UserAPIKeyAuth) -> str: + owner: Final = submission_editor(caller) + if isinstance(owner, SkillMutationError): + raise HTTPException(owner.status, owner.message) + return owner + + +def _changed( + result: SkillSubmission | SubmissionMissing | SubmissionConflict | SubmissionLimitExceeded, +) -> SkillSubmission: + if isinstance(result, SubmissionMissing): + raise HTTPException(404, "Skill submission not found") + if isinstance(result, SubmissionConflict): + raise HTTPException(409, result.message) + if isinstance(result, SubmissionLimitExceeded): + raise HTTPException(429, result.message) + return result + + +async def _editable( + submission_id: str, + owner: str, + store: SkillSubmissionStore, + revision: int, + version: str | None, + uploads: PluginUploadStore | None, + caller: UserAPIKeyAuth, +) -> SkillSubmission: + submission: Final = await store.get(submission_id, owner) + if submission is None: + raise HTTPException(404, "Skill submission not found") + if submission.status != "pending_review" or submission.revision != revision: + raise HTTPException(409, "This submission changed or is no longer pending. Reload it before trying again.") + existing: Final = await write_target( + submission.plugin_name, submission.operation == "update", uploads, caller, submission.base_updated_at + ) + if isinstance(existing, SkillMutationError): + raise HTTPException(existing.status, existing.message) + validation: Final = resolve_plugin_version( + version, None if existing is None else existing.version, updating=existing is not None + ) + if isinstance(validation, PluginPublicationError): + error: Final = publication_error(validation) + raise HTTPException(error.status, error.message) + return submission + + +@router.put("/{submission_id}", response_model=SkillHubSubmission) +async def edit_submission( + request: Request, + submission_id: str, + body: SkillHubSubmissionEdit, + submissions: SkillSubmissionRepositoryDep, + uploads: PluginUploadRepositoryDep, + caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], +) -> SkillHubSubmission: + """Replace a pending proposal's metadata, preserving files. Only its original submitting user may edit.""" + owner: Final = _editor(caller) + store: Final = _store(submissions) + submission: Final = await _editable( + submission_id, owner, store, body.expected_revision, body.version, uploads, caller + ) + publication: Final = PluginPublication( + name=submission.plugin_name, + version=body.version, + description=body.description, + manifest={ + **{ + key: value + for key, value in submission.proposal.manifest.items() + if key not in SkillHubMetadata.model_fields + }, + **body.model_dump(exclude={"expected_revision"}, exclude_none=True), + }, + created_by=owner, + ) + result: Final = await store.edit(submission_id, owner, body.expected_revision, publication) + return _response(request, _changed(result), caller) + + +@router.put("/{submission_id}/upload", response_model=SkillHubSubmission) +async def edit_submission_archive( + request: Request, + submission_id: str, + submissions: SkillSubmissionRepositoryDep, + uploads: PluginUploadRepositoryDep, + caller: Annotated[UserAPIKeyAuth, Depends(archive_writer)], + file: Annotated[UploadFile, File(description="Replacement Agent Skills ZIP with the same declared skill name")], + expected_revision: Annotated[int, Form(ge=0)], + version: Annotated[str | None, Form()] = None, +) -> SkillHubSubmission: + """Replace a pending proposal's files using the upload limits. Only its original submitting user may edit.""" + owner: Final = _editor(caller) + store: Final = _store(submissions) + submission: Final = await _editable(submission_id, owner, store, expected_revision, version, uploads, caller) + content: Final = await file.read(MAX_IMPORT_BYTES + 1) + if len(content) > MAX_IMPORT_BYTES: + raise HTTPException(413, "Skill ZIP must be 10 MiB or smaller") + archive: Final = await asyncio.to_thread(build_agent_skill_archive, content) + if archive is None or archive.declared_name != submission.plugin_name: + raise HTTPException(422, "Replacement SKILL.md must declare the existing skill name and a description") + prepared: Final = await prepare_skill_publication(request, archive, version, None, owner) + publication: Final = replace(prepared, manifest={**submission.proposal.manifest, **prepared.manifest}) + result: Final = await store.edit(submission_id, owner, expected_revision, publication) + return _response(request, _changed(result), caller) + + +@router.put("/{submission_id}/withdraw", response_model=SkillHubSubmission) +async def withdraw_submission( + request: Request, + submission_id: str, + body: SkillHubSubmissionRevision, + submissions: SkillSubmissionRepositoryDep, + caller: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], +) -> SkillHubSubmission: + """Withdraw a pending proposal without publishing it. Only its original submitting user may withdraw.""" + owner: Final = _editor(caller) + result: Final = await _store(submissions).withdraw(submission_id, owner, body.expected_revision) + return _response(request, _changed(result), caller) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 7bf9fcacde2..7fd114f9838 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -2993,6 +2993,7 @@ _ROUTE_MAX_REQUEST_SIZE_MB: Final[Mapping[tuple[str, str], float]] = MappingProx ("POST", "/claude-code/plugins/upload"): 11, ("POST", "/skill-hub/skills/upload"): 11, ("PUT", "/skill-hub/skills/{name}/upload"): 11, + ("PUT", "/skill-hub/submissions/{submission_id}/upload"): 11, } ) diff --git a/litellm/repositories/skill_submission_repository.py b/litellm/repositories/skill_submission_repository.py index 03821da042a..673fd7cca8f 100644 --- a/litellm/repositories/skill_submission_repository.py +++ b/litellm/repositories/skill_submission_repository.py @@ -7,7 +7,7 @@ from datetime import datetime, timezone from typing import TYPE_CHECKING, Final, Protocol, cast # noqa: TID251 # Prisma's generated delegates are dynamic from urllib.parse import urlsplit -from pydantic import BaseModel, ConfigDict, TypeAdapter, ValidationError +from pydantic import BaseModel, ConfigDict, Field, TypeAdapter, ValidationError from litellm.models.plugin_upload import ( PluginPublication, @@ -45,6 +45,7 @@ class _StoredProposal(BaseModel): manifest: Mapping[str, object] version: str | None + revision: int = Field(default=0, ge=0) def proposal(self, name: str) -> SkillProposal: return SkillProposal( @@ -92,6 +93,7 @@ class _SubmissionRow(BaseModel): reviewed_by=self.reviewed_by, reviewed_at=self.reviewed_at, rejection_reason=self.rejection_reason, + revision=stored.revision, ) @@ -283,6 +285,71 @@ class SkillSubmissionRepository: ) return None if row is None or row.archive_content is None else row.archive_content.decode() + async def edit( + self, submission_id: str, owner: str, revision: int, upload: PluginPublication + ) -> SkillSubmission | SubmissionMissing | SubmissionConflict | SubmissionLimitExceeded: + from prisma import Base64 + + async with self._database.tx() as transaction: + await transaction.query_raw( + "SELECT 1 AS locked FROM pg_advisory_xact_lock(hashtext('litellm.skill_submissions'), hashtext($1))", + owner, + ) + row: Final = await _owned_pending(transaction, submission_id, owner, revision) + if isinstance(row, (SubmissionMissing, SubmissionConflict)): + return row + if upload.name != row.plugin_name: + return SubmissionConflict("A submission's skill name cannot change.") + proposal_json: Final = _StoredProposal( + manifest=upload.manifest, version=upload.version, revision=revision + 1 + ).model_dump_json() + archive: Final = row.archive_content if upload.content is None else Base64.encode(upload.content) + usage: Final = _USAGE.validate_python( + await transaction.query_raw( + """SELECT COUNT(*) AS count, + COALESCE(SUM(octet_length(proposal_json) + + COALESCE(octet_length(archive_content), 0)), 0) AS storage_bytes + FROM "LiteLLM_SkillSubmission" WHERE submitted_by = $1 AND status = 'pending_review'""", + owner, + ) + )[0] + previous_bytes: Final = len(row.proposal_json.encode()) + ( + 0 if row.archive_content is None else len(row.archive_content.decode()) + ) + incoming_bytes: Final = len(proposal_json.encode()) + (0 if archive is None else len(archive.decode())) + if usage.storage_bytes - previous_bytes + incoming_bytes > _MAX_PENDING_BYTES: + return SubmissionLimitExceeded( + "Pending skill submissions cannot exceed 50 MiB of stored metadata and archives per user." + ) + updated: Final = await transaction.litellm_skillsubmission.update( + where={"id": submission_id}, + data={"proposal_json": proposal_json, "archive_content": archive, "scan_json": None}, + ) + return SubmissionMissing() if updated is None else _submission(updated) + + async def withdraw( + self, submission_id: str, owner: str, revision: int + ) -> SkillSubmission | SubmissionMissing | SubmissionConflict: + async with self._database.tx() as transaction: + row: Final = await _owned_pending(transaction, submission_id, owner, revision) + if isinstance(row, (SubmissionMissing, SubmissionConflict)): + return row + receipt: Final = _StoredProposal( + manifest={"name": row.plugin_name, "source": {}}, + version=None, + revision=revision, + ) + updated: Final = await transaction.litellm_skillsubmission.update( + where={"id": submission_id}, + data={ + "status": "withdrawn", + "proposal_json": receipt.model_dump_json(), + "archive_content": None, + "scan_json": None, + }, + ) + return SubmissionMissing() if updated is None else _submission(updated) + async def review( self, submission_id: str, @@ -291,6 +358,7 @@ class SkillSubmissionRepository: reason: str | None, *, publication_base_url: str | None = None, + expected_revision: int | None = None, ) -> SubmissionReviewResult: from prisma.errors import UniqueViolationError @@ -304,7 +372,9 @@ class SkillSubmissionRepository: return SubmissionMissing() submission: Final = _submission(row) if submission.status != "pending_review": - return SubmissionConflict("This submission has already been reviewed.") + return SubmissionConflict("This submission is no longer pending review.") + if submission.revision != (expected_revision if expected_revision is not None else 0): + return SubmissionConflict("This submission changed. Reload it before reviewing.") content: Final = None if row.archive_content is None else row.archive_content.decode() if approve and content is not None and not publication_base_url: return SubmissionConflict("Uploaded skill approval requires a trusted publication URL.") @@ -344,6 +414,20 @@ class SkillSubmissionRepository: return SubmissionConflict("A skill with this name is already published. Submit a new update proposal.") +async def _owned_pending( + transaction: SubmissionTransaction, submission_id: str, owner: str, revision: int +) -> "prisma_models.LiteLLM_SkillSubmission | SubmissionMissing | SubmissionConflict": + await transaction.query_raw('SELECT id FROM "LiteLLM_SkillSubmission" WHERE id = $1 FOR UPDATE', submission_id) + row: Final = await transaction.litellm_skillsubmission.find_unique(where={"id": submission_id}) + if row is None or not owner or row.submitted_by != owner: + return SubmissionMissing() + if row.status != "pending_review": + return SubmissionConflict("Only pending submissions can be edited or withdrawn.") + if _StoredProposal.model_validate_json(row.proposal_json).revision != revision: + return SubmissionConflict("This submission changed. Reload it before trying again.") + return row + + async def _publish_submission( transaction: PluginPublicationTransaction, row: "prisma_models.LiteLLM_SkillSubmission", diff --git a/litellm/types/proxy/skill_hub.py b/litellm/types/proxy/skill_hub.py index 9d866f51feb..e045914ead2 100644 --- a/litellm/types/proxy/skill_hub.py +++ b/litellm/types/proxy/skill_hub.py @@ -156,6 +156,8 @@ class SkillHubSubmission(LiteLLMBaseModel): reviewed_by: str | None reviewed_at: datetime | None rejection_reason: str | None + revision: int = Field(0, ge=0, description="Proposal revision used to prevent stale edits and reviews") + can_edit: bool = Field(False, description="Whether the caller may edit or withdraw this pending submission") class SkillHubSubmissionList(LiteLLMBaseModel): @@ -173,6 +175,16 @@ class SkillHubSubmissionDetail(LiteLLMBaseModel): current_skill: SkillHubSkill | None +class SkillHubSubmissionRevision(LiteLLMBaseModel): + model_config = ConfigDict(frozen=True, extra="forbid") + + expected_revision: int = Field(ge=0, description="Revision read from the submission detail") + + +class SkillHubSubmissionEdit(SkillHubMetadata): + expected_revision: int = Field(ge=0, description="Revision read from the submission detail") + + class SkillHubReviewResponse(LiteLLMBaseModel): model_config = ConfigDict(frozen=True) @@ -186,3 +198,4 @@ class SkillHubRejection(LiteLLMBaseModel): model_config = ConfigDict(frozen=True, extra="forbid") reason: str | None = Field(None, max_length=2000) + expected_revision: int | None = Field(None, ge=0) diff --git a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.integration.test.tsx b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.integration.test.tsx index 9a260bdfc65..6ee327da914 100644 --- a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.integration.test.tsx +++ b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.integration.test.tsx @@ -31,6 +31,8 @@ const published: SkillHubSkill = { const pending: SkillSubmission = { id: "submission-1", + revision: 0, + can_edit: false, skill_name: published.name, operation: "update", status: "pending_review", @@ -70,8 +72,9 @@ let reviewFailure: string | null; let directPublication: boolean; const respondToWrite = (path: string, body: RequestInit["body"]) => { + if (reviewFailure && path.startsWith("/skill-hub/submissions/")) + return Response.json({ detail: { error: reviewFailure } }, { status: 409 }); if (path.endsWith("/approve") || path.endsWith("/reject")) { - if (reviewFailure) return Response.json({ detail: { error: reviewFailure } }, { status: 409 }); const approved = path.endsWith("/approve"); const reason = typeof body === "string" ? JSON.parse(body).reason : null; submissions = submissions.map((submission) => @@ -88,6 +91,19 @@ const respondToWrite = (path: string, body: RequestInit["body"]) => { }; return Response.json(result); } + if (path === `/skill-hub/submissions/${pending.id}`) { + const metadata = typeof body === "string" ? JSON.parse(body) : {}; + submissions = submissions.map((item) => ({ + ...item, + revision: (item.revision ?? 0) + 1, + proposal: { ...item.proposal, ...metadata }, + })); + return Response.json(submissions[0]); + } + if (path.endsWith("/withdraw")) { + submissions = submissions.map((item) => ({ ...item, status: "withdrawn", can_edit: false })); + return Response.json(submissions[0]); + } if (directPublication) { catalog = [{ ...published, version: "3.0.0", description: "Admin-published instructions" }]; return Response.json(catalog[0]); @@ -573,7 +589,7 @@ it("saves an admin rejection reason for the author and keeps the published metad expect(writes[0]).toMatchObject({ path: `/skill-hub/submissions/${pending.id}/reject`, method: "PUT", - body: JSON.stringify({ reason: "Please include setup instructions" }), + body: JSON.stringify({ expected_revision: 0, reason: "Please include setup instructions" }), }); fireEvent.click(screen.getByRole("tab", { name: "Published skills" })); await waitFor(() => @@ -792,3 +808,84 @@ it("applies tag and scan filters across the catalog and resets pagination when s expect(catalogRequests.at(-1)?.searchParams.get("tag")).toBe("engineering"); expect(catalogRequests.at(-1)?.searchParams.get("security_scan_passed")).toBe("true"); }); + +it("lets an owner edit the viewed proposal and withdraw it with its current revision", async () => { + submissions = [{ ...pending, can_edit: true, revision: 3, proposal: { ...pending.proposal, version: "3.0.0" } }]; + renderWithProviders(, { + searchParams: "tab=submissions", + }); + fireEvent.click(await screen.findByRole("button", { name: "View details" })); + fireEvent.click(await screen.findByRole("button", { name: "Edit submission" })); + expect(screen.getByLabelText("Version override (optional)")).toHaveValue("3.0.0"); + expect( + screen.getByText( + `Current version ${published.version}. Leave blank for the next patch version, or enter a higher version`, + ), + ).toBeInTheDocument(); + fireEvent.change(await screen.findByLabelText("Description (Optional)"), { + target: { value: "Owner revised instructions" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Save submission" })); + expect(await screen.findByRole("cell", { name: "Owner revised instructions" })).toBeInTheDocument(); + expect(JSON.parse(String(writes[0].body))).toMatchObject({ + expected_revision: 3, + description: "Owner revised instructions", + }); + expect(catalog).toEqual([published]); + fireEvent.click(screen.getByRole("button", { name: "Withdraw submission" })); + expect(writes).toHaveLength(1); + fireEvent.click(await screen.findByRole("button", { name: "Confirm withdrawal" })); + expect(await screen.findByText("Withdrawn")).toBeInTheDocument(); + expect(writes[1]).toMatchObject({ + path: `/skill-hub/submissions/${pending.id}/withdraw`, + body: JSON.stringify({ expected_revision: 4 }), + }); + fireEvent.click(screen.getByRole("button", { name: "View details" })); + expect(screen.queryByRole("button", { name: "Edit submission" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Withdraw submission" })).not.toBeInTheDocument(); +}); + +it("keeps admin review separate from ownership and sends the displayed revision", async () => { + submissions = [{ ...pending, revision: 7, can_edit: false }]; + renderSubmissions(); + fireEvent.click(await screen.findByRole("button", { name: "Review" })); + expect(screen.queryByRole("button", { name: "Edit submission" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Withdraw submission" })).not.toBeInTheDocument(); + fireEvent.click(await screen.findByRole("button", { name: "Approve and publish" })); + expect(await screen.findByText("Approved")).toBeInTheDocument(); + expect(JSON.parse(String(writes[0].body))).toEqual({ expected_revision: 7 }); +}); + +it.each(["metadata", "upload"])( + "reloads a conflicted %s edit only when the author discards the draft", + async (mode) => { + submissions = [{ ...pending, can_edit: true }]; + renderWithProviders(, { + searchParams: "tab=submissions", + }); + fireEvent.click(await screen.findByRole("button", { name: "View details" })); + fireEvent.click(await screen.findByRole("button", { name: "Edit submission" })); + fireEvent.change(screen.getByLabelText("Description (Optional)"), { target: { value: "Unsaved draft" } }); + if (mode === "upload") { + fireEvent.click(screen.getByRole("radio", { name: "Upload ZIP" })); + fireEvent.change(screen.getByLabelText("Skill ZIP file"), { + target: { files: [new File(["replacement"], "skill.zip", { type: "application/zip" })] }, + }); + } + submissions = [ + { ...pending, can_edit: true, revision: 1, proposal: { ...pending.proposal, description: "Latest edit" } }, + ]; + reviewFailure = "This submission changed. Reload it before trying again."; + fireEvent.click(screen.getByRole("button", { name: "Save submission" })); + expect(await screen.findByRole("alert")).toHaveTextContent(reviewFailure); + expect(screen.getByRole("dialog", { name: "Edit submission" })).toBeInTheDocument(); + reviewFailure = null; + fireEvent.click(screen.getByRole("button", { name: "Discard draft and reload submission" })); + expect(await screen.findByRole("cell", { name: "Latest edit" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Edit submission" })); + expect(screen.getByLabelText("Description (Optional)")).toHaveValue("Latest edit"); + fireEvent.click(screen.getByRole("button", { name: "Save submission" })); + await waitFor(() => expect(writes).toHaveLength(2)); + expect(JSON.parse(String(writes[1].body))).toMatchObject({ expected_revision: 1, description: "Latest edit" }); + }, +); diff --git a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.tsx b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.tsx index d0ca11f829a..c17246fbe2e 100644 --- a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.tsx +++ b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/ClaudeCodePluginsPanel.tsx @@ -245,6 +245,7 @@ const SkillsPanelContent: React.FC = ({ accessToken={accessToken} queryScope={queryScope} canReview={isAdmin} + canWrite={canWrite} adminView={adminView} onChanged={refreshSkills} /> diff --git a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/SkillSubmissions.tsx b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/SkillSubmissions.tsx index 8d20855e4f4..9865610db4c 100644 --- a/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/SkillSubmissions.tsx +++ b/litellm/ui/litellm-dashboard/src/app/(dashboard)/skills/_components/SkillSubmissions.tsx @@ -7,6 +7,7 @@ import { getSkillSubmission, listSkillSubmissions, reviewSkillSubmission, + withdrawSkillSubmission, } from "@/components/networking"; import type { SkillHubSkill, SkillSubmission } from "@/components/claude_code_plugins/types"; import { Badge } from "@/components/ui/badge"; @@ -16,6 +17,16 @@ import { Textarea } from "@/components/ui/textarea"; import { useUrlTableState, type UrlTableStateOptions } from "@/components/shared/DataTable/useUrlTableState"; import SkillScanPanel from "./SkillScanPanel"; import { toast } from "@/lib/toast"; +import AddPluginForm from "./add_plugin_form"; +import { + AlertDialog, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "@/components/ui/alert-dialog"; const METADATA_FIELDS = ["name", "version", "description", "source", "keywords", "domain", "namespace"] as const; @@ -28,6 +39,7 @@ const STATUS_CLASSES = { pending_review: "border-warning/30 text-warning", approved: "border-success/30 text-success", rejected: "border-destructive/30 text-destructive", + withdrawn: "border-muted text-muted-foreground", }; const SUBMISSION_TABLE_STATE: UrlTableStateOptions = { @@ -82,51 +94,63 @@ function SubmissionSummary({ submission }: { submission: SkillSubmission }) { function SubmissionComparison({ submission, current, + onDownload, + downloading, }: { submission: SkillSubmission; current?: SkillHubSkill | null; + onDownload: () => Promise; + downloading: boolean; }) { const { t } = useTranslation(); return ( -
- - - - - - - - - - {METADATA_FIELDS.map((field) => ( - - + <> + {!current &&

{t("skills.submissions.noCurrent")}

} +
+
{t("skills.submissions.field")}{t("skills.submissions.current")}{t("skills.submissions.proposed")}
{t(`skills.submissions.fields.${field}`)}
+ + + + + + + + + {METADATA_FIELDS.map((field) => ( + + + + + + ))} + + + - - ))} - - - - - - -
{t("skills.submissions.field")}{t("skills.submissions.current")}{t("skills.submissions.proposed")}
{t(`skills.submissions.fields.${field}`)} + 
+                    {displayValue(current?.[field]) || t("skills.submissions.notSet")}
+
+ 		+ 
+                    {field === "version" && submission.operation === "update" && !submission.proposal.version
+                      ? t("skills.submissions.nextPatchVersion")
+                      : displayValue(submission.proposal[field]) || t("skills.submissions.notSet")}
+
+
{t("skills.submissions.fields.agent_skill")}{t("skills.submissions.descriptorUnavailable")} 
-                  {displayValue(current?.[field]) || t("skills.submissions.notSet")}
-
- 		- 
-                  {field === "version" && submission.operation === "update" && !submission.proposal.version
-                    ? t("skills.submissions.nextPatchVersion")
-                    : displayValue(submission.proposal[field]) || t("skills.submissions.notSet")}
+                  {displayValue(submission.proposal.agent_skill) || t("skills.submissions.notSet")}
{t("skills.submissions.fields.agent_skill")}{t("skills.submissions.descriptorUnavailable")} - 
-                {displayValue(submission.proposal.agent_skill) || t("skills.submissions.notSet")}
-
-
-
+ + + + {submission.has_archive && submission.proposal.agent_skill && ( + + )} + ); } @@ -135,33 +159,46 @@ interface SubmissionReviewProps { accessToken: string; queryScope: QueryKey; canReview: boolean; + canWrite: boolean; onClose: () => void; onChanged: () => Promise; } -function SubmissionReview({ id, accessToken, queryScope, canReview, onClose, onChanged }: SubmissionReviewProps) { +function SubmissionReview({ + id, + accessToken, + queryScope, + canReview, + canWrite, + onClose, + onChanged, +}: SubmissionReviewProps) { const { t } = useTranslation(); const [reason, setReason] = useState(""); const [busy, setBusy] = useState(false); const [downloading, setDownloading] = useState(false); const [error, setError] = useState(null); - const detail = useQuery({ + const [editing, setEditing] = useState(null); + const [withdrawing, setWithdrawing] = useState(null); + const detailOptions = { queryKey: [...queryScope, "submission", id], queryFn: () => getSkillSubmission(accessToken, id), - }); + refetchOnWindowFocus: false, + refetchOnReconnect: false, + }; + const detail = useQuery(detailOptions); const { submission, current_skill: current } = detail.data ?? {}; const canDecide = canReview && submission?.status === "pending_review"; + const actionsDisabled = busy || detail.isFetching || detail.isError; const review = async (action: "approve" | "reject") => { - if (!canDecide || busy) return; + if (!canDecide || !submission || busy) return; setBusy(true); setError(null); try { - await reviewSkillSubmission( - accessToken, - id, - action, - action === "reject" ? { reason: reason.trim() || null } : undefined, - ); + await reviewSkillSubmission(accessToken, id, action, { + expected_revision: submission.revision ?? 0, + ...(action === "reject" ? { reason: reason.trim() || null } : {}), + }); onClose(); await onChanged(); toast.success(t(action === "approve" ? "skills.submissions.approved" : "skills.submissions.rejected")); @@ -172,6 +209,24 @@ function SubmissionReview({ id, accessToken, queryScope, canReview, onClose, onC setBusy(false); } }; + const withdraw = async () => { + if (!withdrawing || busy) return; + setBusy(true); + setError(null); + try { + await withdrawSkillSubmission(accessToken, withdrawing); + setWithdrawing(null); + onClose(); + await onChanged(); + toast.success(t("skills.submissions.withdrawn")); + } catch (err) { + setError(err instanceof Error ? err.message : t("skills.submissions.actionFailed")); + setWithdrawing(null); + await onChanged(); + } finally { + setBusy(false); + } + }; const download = async () => { if (!submission || downloading) return; setDownloading(true); @@ -185,73 +240,111 @@ function SubmissionReview({ id, accessToken, queryScope, canReview, onClose, onC } }; return ( - !open && !busy && onClose()}> - - - {t("skills.submissions.reviewTitle", { name: submission?.skill_name ?? "" })} - - {detail.isPending &&

{t("skills.submissions.loading")}

} - {detail.isError && ( -
- {t("skills.submissions.loadFailed")} - -
- )} - {submission && ( - <> - - {!current &&

{t("skills.submissions.noCurrent")}

} - - {submission.has_archive && submission.proposal.agent_skill && ( - - )} - {canReview && ( - + )} + {submission && ( + <> + + - )} - {canDecide && ( - <> - -