Skip to content

Commit c7a6728

Browse files
ptoneScion Agent (skill-bank-publish-fix)
andauthored
fix(hub): make skill version publish idempotent for draft versions (#451)
When publishing a skill version, if the draft creation (step 1) succeeds but upload/finalize (steps 2-3) fail or are interrupted, retrying from step 1 would hit a UNIQUE constraint and return 409 Conflict, leaving the user stuck. Now when a version with the same number already exists as a draft, the endpoint returns the existing draft with fresh upload URLs instead of rejecting. Published/deprecated/archived versions still return 409. Co-authored-by: Scion Agent (skill-bank-publish-fix) <agent@scion.dev>
1 parent 4bea964 commit c7a6728

1 file changed

Lines changed: 39 additions & 20 deletions

File tree

pkg/hub/skill_handlers.go

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package hub
1616

1717
import (
1818
"context"
19+
"errors"
1920
"fmt"
2021
"net/http"
2122
"strconv"
@@ -697,34 +698,52 @@ func (s *Server) publishSkillVersion(w http.ResponseWriter, r *http.Request, ski
697698
return
698699
}
699700

700-
// Check for existing published version (immutability)
701+
// Check for an existing version with this number.
702+
// - Draft: reuse it (idempotent retry of an incomplete publish).
703+
// - Published/deprecated/archived: reject as conflict.
704+
var sv *store.SkillVersion
701705
existing, err := s.store.GetSkillVersionByNumber(ctx, skillID, req.Version)
702-
if err == nil && existing.Status == store.SkillVersionStatusPublished {
703-
writeError(w, http.StatusConflict, "conflict",
704-
fmt.Sprintf("version %s is already published and immutable; publish a new version instead", req.Version), nil)
706+
if err != nil && !errors.Is(err, store.ErrNotFound) {
707+
writeErrorFromErr(w, err, "")
705708
return
706709
}
707-
708-
// Create draft version
709-
sv := &store.SkillVersion{
710-
ID: api.NewUUID(),
711-
SkillID: skillID,
712-
Version: req.Version,
713-
Status: store.SkillVersionStatusDraft,
710+
if err == nil {
711+
switch existing.Status {
712+
case store.SkillVersionStatusDraft:
713+
sv = existing
714+
case store.SkillVersionStatusPublished:
715+
writeError(w, http.StatusConflict, "conflict",
716+
fmt.Sprintf("version %s is already published and immutable; publish a new version instead", req.Version), nil)
717+
return
718+
default:
719+
writeError(w, http.StatusConflict, "conflict",
720+
fmt.Sprintf("version %s already exists with status %q", req.Version, existing.Status), nil)
721+
return
722+
}
714723
}
715724

716-
if identity := GetIdentityFromContext(ctx); identity != nil {
717-
sv.PublisherID = identity.ID()
718-
}
725+
if sv == nil {
726+
// Create new draft version
727+
sv = &store.SkillVersion{
728+
ID: api.NewUUID(),
729+
SkillID: skillID,
730+
Version: req.Version,
731+
Status: store.SkillVersionStatusDraft,
732+
}
719733

720-
if err := s.store.CreateSkillVersion(ctx, sv); err != nil {
721-
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
722-
writeError(w, http.StatusConflict, "conflict",
723-
fmt.Sprintf("version %s already exists for this skill", req.Version), nil)
734+
if identity := GetIdentityFromContext(ctx); identity != nil {
735+
sv.PublisherID = identity.ID()
736+
}
737+
738+
if err := s.store.CreateSkillVersion(ctx, sv); err != nil {
739+
if strings.Contains(err.Error(), "UNIQUE constraint failed") {
740+
writeError(w, http.StatusConflict, "conflict",
741+
fmt.Sprintf("version %s already exists for this skill", req.Version), nil)
742+
return
743+
}
744+
writeErrorFromErr(w, err, "")
724745
return
725746
}
726-
writeErrorFromErr(w, err, "")
727-
return
728747
}
729748

730749
response := PublishVersionResponse{

0 commit comments

Comments
 (0)