mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 14:16:08 +00:00
fix(skills): harden evolution mutations
This commit is contained in:
1 parent
1e0ea7615d
commit
dd6ee64f23
7 files changed
+247
-64
No files matched your search
+18
-5
@@ -103,11 +103,11 @@ func (h *SkillsHandler) RegisterRoutes(mux *http.ServeMux) {
|
||||
mux.HandleFunc("POST /v1/skills/upload", h.adminMiddleware(h.handleUpload))
|
||||
mux.HandleFunc("PUT /v1/skills/{id}", h.adminMiddleware(h.handleUpdate))
|
||||
mux.HandleFunc("DELETE /v1/skills/{id}", h.adminMiddleware(h.handleDelete))
|
||||
mux.HandleFunc("PATCH /v1/skills/{id}/evolution", h.adminMiddleware(h.handlePatchEvolution))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions", h.adminMiddleware(h.handleCreateSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/approve", h.adminMiddleware(h.handleApproveSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/reject", h.adminMiddleware(h.handleRejectSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/apply", h.adminMiddleware(h.handleApplySkillSuggestion))
|
||||
mux.HandleFunc("PATCH /v1/skills/{id}/evolution", h.tenantAdminMiddleware(h.handlePatchEvolution))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions", h.tenantAdminMiddleware(h.handleCreateSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/approve", h.tenantAdminMiddleware(h.handleApproveSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/reject", h.tenantAdminMiddleware(h.handleRejectSkillSuggestion))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/evolution/suggestions/{suggestionID}/apply", h.tenantAdminMiddleware(h.handleApplySkillSuggestion))
|
||||
mux.HandleFunc("GET /v1/skills/{id}/dependencies", h.adminMiddleware(h.handleSkillDependenciesStatus))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/dependencies/scan", h.adminMiddleware(h.handleSkillDependenciesStatus))
|
||||
mux.HandleFunc("POST /v1/skills/{id}/dependencies/check", h.adminMiddleware(h.handleSkillDependenciesStatus))
|
||||
@@ -154,6 +154,19 @@ func (h *SkillsHandler) adminMiddleware(next http.HandlerFunc) http.HandlerFunc
|
||||
return requireAuth(permissions.RoleAdmin, next)
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) tenantAdminMiddleware(next http.HandlerFunc) http.HandlerFunc {
|
||||
return h.adminMiddleware(func(w http.ResponseWriter, r *http.Request) {
|
||||
if store.IsMasterScope(r.Context()) {
|
||||
next(w, r)
|
||||
return
|
||||
}
|
||||
if !requireTenantAdmin(w, r, h.tenantStore) {
|
||||
return
|
||||
}
|
||||
next(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// requireMasterTenant rejects requests from non-master tenants.
|
||||
// System skill management (install packages, rescan deps) is a server-wide operation
|
||||
// that should only be accessible to the master tenant or cross-tenant admins.
|
||||
|
||||
@@ -14,11 +14,21 @@ import (
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/i18n"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/permissions"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/skills"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
var (
|
||||
errSkillEvolutionSkillNotFound = errors.New("skill not found")
|
||||
errSkillEvolutionSystemMutation = errors.New("system skill mutation is blocked")
|
||||
errSkillEvolutionInvalidDraftPatch = errors.New("invalid draft_patch")
|
||||
errSkillEvolutionDraftPatchRequired = errors.New("draft_patch requires content or find/replace")
|
||||
errSkillEvolutionFindTextNotFound = errors.New("find text not found in target file")
|
||||
errSkillEvolutionGuardPolicyViolation = errors.New("skill guard policy violation")
|
||||
)
|
||||
|
||||
type skillEvolutionPatchRequest struct {
|
||||
Enabled *bool `json:"enabled"`
|
||||
Mode string `json:"mode"`
|
||||
@@ -42,30 +52,34 @@ type skillDraftPatch struct {
|
||||
Content *string `json:"content"`
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) evolutionConfigured(w http.ResponseWriter) bool {
|
||||
func (h *SkillsHandler) evolutionConfigured(w http.ResponseWriter, r *http.Request) bool {
|
||||
if h.evolutionStore == nil {
|
||||
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": "skill evolution store not configured"})
|
||||
writeSkillEvolutionError(w, r, http.StatusServiceUnavailable, i18n.MsgSkillEvolutionNotConfigured)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func writeSkillEvolutionError(w http.ResponseWriter, r *http.Request, status int, key string, args ...any) {
|
||||
writeJSON(w, status, map[string]string{"error": i18n.T(store.LocaleFromContext(r.Context()), key, args...)})
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) skillIDFromRequest(w http.ResponseWriter, r *http.Request) (uuid.UUID, store.SkillInfo, bool) {
|
||||
id, err := uuid.Parse(r.PathValue("id"))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid skill id"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgInvalidID, "skill")
|
||||
return uuid.Nil, store.SkillInfo{}, false
|
||||
}
|
||||
info, ok := h.skills.GetSkillByID(r.Context(), id)
|
||||
if !ok {
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "skill not found"})
|
||||
writeSkillEvolutionError(w, r, http.StatusNotFound, i18n.MsgNotFound, "skill", id.String())
|
||||
return uuid.Nil, store.SkillInfo{}, false
|
||||
}
|
||||
return id, info, true
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleGetEvolution(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -74,14 +88,14 @@ func (h *SkillsHandler) handleGetEvolution(w http.ResponseWriter, r *http.Reques
|
||||
}
|
||||
settings, err := h.evolutionStore.GetSettings(r.Context(), skillID)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, settings)
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handlePatchEvolution(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -94,7 +108,7 @@ func (h *SkillsHandler) handlePatchEvolution(w http.ResponseWriter, r *http.Requ
|
||||
}
|
||||
current, err := h.evolutionStore.GetSettings(r.Context(), skillID)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
enabled := current.Enabled
|
||||
@@ -109,7 +123,7 @@ func (h *SkillsHandler) handlePatchEvolution(w http.ResponseWriter, r *http.Requ
|
||||
mode = store.SkillEvolutionModeSuggestOnly
|
||||
}
|
||||
if mode != store.SkillEvolutionModeSuggestOnly && mode != store.SkillEvolutionModeAutoAnalyze {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid evolution mode"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgInvalidEvolutionMode)
|
||||
return
|
||||
}
|
||||
updated, err := h.evolutionStore.UpsertSettings(r.Context(), store.SkillEvolutionSettings{
|
||||
@@ -118,7 +132,7 @@ func (h *SkillsHandler) handlePatchEvolution(w http.ResponseWriter, r *http.Requ
|
||||
Mode: mode,
|
||||
})
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
action := "skill.evolve.mode_updated"
|
||||
@@ -134,7 +148,7 @@ func (h *SkillsHandler) handlePatchEvolution(w http.ResponseWriter, r *http.Requ
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleGetSkillMetrics(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -149,7 +163,7 @@ func (h *SkillsHandler) handleGetSkillMetrics(w http.ResponseWriter, r *http.Req
|
||||
}
|
||||
stats, err := h.evolutionStore.AggregateUsage(r.Context(), skillID, since)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
if !permissions.HasMinRole(resolveAuth(r).Role, permissions.RoleAdmin) {
|
||||
@@ -160,7 +174,7 @@ func (h *SkillsHandler) handleGetSkillMetrics(w http.ResponseWriter, r *http.Req
|
||||
|
||||
func (h *SkillsHandler) handleGetSkillActivity(w http.ResponseWriter, r *http.Request) {
|
||||
if h.activityStore == nil {
|
||||
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": "activity store not configured"})
|
||||
writeSkillEvolutionError(w, r, http.StatusServiceUnavailable, i18n.MsgActivityStoreNotConfigured)
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -173,14 +187,14 @@ func (h *SkillsHandler) handleGetSkillActivity(w http.ResponseWriter, r *http.Re
|
||||
Limit: clampLimit(r.URL.Query().Get("limit"), 50, 200),
|
||||
})
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"activity": items})
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleListSkillSuggestions(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -189,7 +203,7 @@ func (h *SkillsHandler) handleListSkillSuggestions(w http.ResponseWriter, r *htt
|
||||
}
|
||||
items, err := h.evolutionStore.ListSuggestions(r.Context(), skillID, r.URL.Query().Get("status"), clampLimit(r.URL.Query().Get("limit"), 50, 200))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
if !permissions.HasMinRole(resolveAuth(r).Role, permissions.RoleAdmin) {
|
||||
@@ -204,7 +218,7 @@ func (h *SkillsHandler) handleListSkillSuggestions(w http.ResponseWriter, r *htt
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleCreateSkillSuggestion(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -216,14 +230,14 @@ func (h *SkillsHandler) handleCreateSkillSuggestion(w http.ResponseWriter, r *ht
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.SuggestionType) == "" {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "suggestion_type is required"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgRequired, "suggestion_type")
|
||||
return
|
||||
}
|
||||
target := strings.TrimSpace(req.TargetFile)
|
||||
if target != "" {
|
||||
clean, err := skills.ValidateSkillTargetPath(target, true)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgInvalidRequest, err.Error())
|
||||
return
|
||||
}
|
||||
target = clean
|
||||
@@ -239,7 +253,7 @@ func (h *SkillsHandler) handleCreateSkillSuggestion(w http.ResponseWriter, r *ht
|
||||
CreatedByActorID: store.ActorIDFromContext(r.Context()),
|
||||
})
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
h.logSkillActivity(r, "skill.suggestion.created", skillID, map[string]any{"suggestion_id": created.ID.String(), "type": created.SuggestionType})
|
||||
@@ -255,7 +269,7 @@ func (h *SkillsHandler) handleRejectSkillSuggestion(w http.ResponseWriter, r *ht
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleSuggestionStatus(w http.ResponseWriter, r *http.Request, status, action string) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, _, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -264,21 +278,21 @@ func (h *SkillsHandler) handleSuggestionStatus(w http.ResponseWriter, r *http.Re
|
||||
}
|
||||
suggestionID, err := uuid.Parse(r.PathValue("suggestionID"))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid suggestion id"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgInvalidID, "suggestion")
|
||||
return
|
||||
}
|
||||
sg, err := h.evolutionStore.GetSuggestion(r.Context(), suggestionID)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
if sg == nil || sg.SkillID != skillID {
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "suggestion not found"})
|
||||
writeSkillEvolutionError(w, r, http.StatusNotFound, i18n.MsgNotFound, "suggestion", suggestionID.String())
|
||||
return
|
||||
}
|
||||
updated, err := h.evolutionStore.UpdateSuggestionStatus(r.Context(), suggestionID, status, "user", store.ActorIDFromContext(r.Context()))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
h.logSkillActivity(r, action, skillID, map[string]any{"suggestion_id": suggestionID.String()})
|
||||
@@ -286,7 +300,7 @@ func (h *SkillsHandler) handleSuggestionStatus(w http.ResponseWriter, r *http.Re
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) handleApplySkillSuggestion(w http.ResponseWriter, r *http.Request) {
|
||||
if !h.evolutionConfigured(w) {
|
||||
if !h.evolutionConfigured(w, r) {
|
||||
return
|
||||
}
|
||||
skillID, info, ok := h.skillIDFromRequest(w, r)
|
||||
@@ -294,23 +308,23 @@ func (h *SkillsHandler) handleApplySkillSuggestion(w http.ResponseWriter, r *htt
|
||||
return
|
||||
}
|
||||
if info.IsSystem {
|
||||
writeJSON(w, http.StatusForbidden, map[string]string{"error": "system skill mutation is blocked"})
|
||||
writeSkillEvolutionError(w, r, http.StatusForbidden, i18n.MsgSystemSkillMutationBlocked)
|
||||
return
|
||||
}
|
||||
suggestionID, err := uuid.Parse(r.PathValue("suggestionID"))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid suggestion id"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgInvalidID, "suggestion")
|
||||
return
|
||||
}
|
||||
var req skillSuggestionApplyRequest
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
sg, err := h.evolutionStore.GetSuggestion(r.Context(), suggestionID)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
if sg == nil || sg.SkillID != skillID {
|
||||
writeJSON(w, http.StatusNotFound, map[string]string{"error": "suggestion not found"})
|
||||
writeSkillEvolutionError(w, r, http.StatusNotFound, i18n.MsgNotFound, "suggestion", suggestionID.String())
|
||||
return
|
||||
}
|
||||
if sg.Status == store.SkillSuggestionStatusApplied && sg.AppliedVersion != nil {
|
||||
@@ -319,18 +333,18 @@ func (h *SkillsHandler) handleApplySkillSuggestion(w http.ResponseWriter, r *htt
|
||||
}
|
||||
if sg.Status != store.SkillSuggestionStatusApproved {
|
||||
if !req.Approve {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "suggestion must be approved before apply"})
|
||||
writeSkillEvolutionError(w, r, http.StatusBadRequest, i18n.MsgSuggestionMustBeApproved)
|
||||
return
|
||||
}
|
||||
sg, err = h.evolutionStore.UpdateSuggestionStatus(r.Context(), suggestionID, store.SkillSuggestionStatusApproved, "user", store.ActorIDFromContext(r.Context()))
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
|
||||
writeSkillEvolutionError(w, r, http.StatusInternalServerError, i18n.MsgInternalError, err.Error())
|
||||
return
|
||||
}
|
||||
}
|
||||
applied, err := h.applySkillSuggestionPatch(r, skillID, sg)
|
||||
if err != nil {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": h.skillEvolutionApplyErrorMessage(r, skillID, err)})
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, applied)
|
||||
@@ -339,10 +353,10 @@ func (h *SkillsHandler) handleApplySkillSuggestion(w http.ResponseWriter, r *htt
|
||||
func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.UUID, sg *store.SkillImprovementSuggestion) (*store.SkillImprovementSuggestion, error) {
|
||||
currentDir, slug, oldVersion, isSystem, ok := h.skills.GetSkillFilePath(r.Context(), skillID)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("skill not found")
|
||||
return nil, errSkillEvolutionSkillNotFound
|
||||
}
|
||||
if isSystem {
|
||||
return nil, fmt.Errorf("system skill mutation is blocked")
|
||||
return nil, errSkillEvolutionSystemMutation
|
||||
}
|
||||
target := sg.TargetFile
|
||||
if strings.TrimSpace(target) == "" {
|
||||
@@ -355,11 +369,11 @@ func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.
|
||||
var patch skillDraftPatch
|
||||
if len(sg.DraftPatch) > 0 {
|
||||
if err := json.Unmarshal(sg.DraftPatch, &patch); err != nil {
|
||||
return nil, fmt.Errorf("invalid draft_patch: %w", err)
|
||||
return nil, fmt.Errorf("%w: %v", errSkillEvolutionInvalidDraftPatch, err)
|
||||
}
|
||||
}
|
||||
if patch.Content == nil && patch.Find == "" {
|
||||
return nil, fmt.Errorf("draft_patch requires content or find/replace")
|
||||
return nil, errSkillEvolutionDraftPatchRequired
|
||||
}
|
||||
newVersion, commitLock, err := h.skills.GetNextVersionLocked(r.Context(), slug)
|
||||
if err != nil {
|
||||
@@ -372,10 +386,10 @@ func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.
|
||||
if err := copyDir(currentDir, tmpDir); err != nil {
|
||||
return nil, fmt.Errorf("stage version: %w", err)
|
||||
}
|
||||
cleanup := true
|
||||
removeDestOnError := true
|
||||
defer func() {
|
||||
if cleanup {
|
||||
_ = os.RemoveAll(tmpDir)
|
||||
_ = os.RemoveAll(tmpDir)
|
||||
if removeDestOnError {
|
||||
_ = os.RemoveAll(destDir)
|
||||
}
|
||||
}()
|
||||
@@ -392,14 +406,14 @@ func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.
|
||||
nextContent = string(currentBytes)
|
||||
replaced := strings.Replace(nextContent, patch.Find, patch.Replace, 1)
|
||||
if replaced == nextContent {
|
||||
return nil, fmt.Errorf("find text not found in target file")
|
||||
return nil, errSkillEvolutionFindTextNotFound
|
||||
}
|
||||
nextContent = replaced
|
||||
}
|
||||
if cleanTarget == "SKILL.md" {
|
||||
violations, safe := skills.GuardSkillContent(nextContent)
|
||||
if !safe {
|
||||
return nil, errors.New(skills.FormatGuardViolations(violations))
|
||||
return nil, fmt.Errorf("%w: %s", errSkillEvolutionGuardPolicyViolation, skills.FormatGuardViolations(violations))
|
||||
}
|
||||
}
|
||||
if err := os.MkdirAll(filepath.Dir(targetPath), 0755); err != nil {
|
||||
@@ -425,6 +439,9 @@ func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.
|
||||
}); err != nil {
|
||||
return nil, fmt.Errorf("update skill: %w", err)
|
||||
}
|
||||
// The active skill row now points at destDir; keep files even if
|
||||
// version history or suggestion metadata fails after this point.
|
||||
removeDestOnError = false
|
||||
changedFiles, _ := json.Marshal([]string{cleanTarget})
|
||||
if _, err := h.evolutionStore.CreateSkillVersion(r.Context(), store.SkillVersion{
|
||||
SkillID: skillID,
|
||||
@@ -448,10 +465,29 @@ func (h *SkillsHandler) applySkillSuggestionPatch(r *http.Request, skillID uuid.
|
||||
"new_version": newVersion,
|
||||
"content_hash": hash,
|
||||
})
|
||||
cleanup = false
|
||||
return applied, nil
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) skillEvolutionApplyErrorMessage(r *http.Request, skillID uuid.UUID, err error) string {
|
||||
locale := store.LocaleFromContext(r.Context())
|
||||
switch {
|
||||
case errors.Is(err, errSkillEvolutionSkillNotFound):
|
||||
return i18n.T(locale, i18n.MsgNotFound, "skill", skillID.String())
|
||||
case errors.Is(err, errSkillEvolutionSystemMutation):
|
||||
return i18n.T(locale, i18n.MsgSystemSkillMutationBlocked)
|
||||
case errors.Is(err, errSkillEvolutionInvalidDraftPatch):
|
||||
return i18n.T(locale, i18n.MsgInvalidDraftPatch, strings.TrimPrefix(err.Error(), errSkillEvolutionInvalidDraftPatch.Error()+": "))
|
||||
case errors.Is(err, errSkillEvolutionDraftPatchRequired):
|
||||
return i18n.T(locale, i18n.MsgDraftPatchRequired)
|
||||
case errors.Is(err, errSkillEvolutionFindTextNotFound):
|
||||
return i18n.T(locale, i18n.MsgFindTextNotFound)
|
||||
case errors.Is(err, errSkillEvolutionGuardPolicyViolation):
|
||||
return i18n.T(locale, i18n.MsgInvalidRequest, strings.TrimPrefix(err.Error(), errSkillEvolutionGuardPolicyViolation.Error()+": "))
|
||||
default:
|
||||
return i18n.T(locale, i18n.MsgInvalidRequest, err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func (h *SkillsHandler) logSkillActivity(r *http.Request, action string, skillID uuid.UUID, details map[string]any) {
|
||||
if h.activityStore == nil {
|
||||
return
|
||||
|
||||
@@ -3,16 +3,19 @@ package http
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/crypto"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
)
|
||||
|
||||
@@ -68,6 +71,95 @@ func TestApplySkillSuggestionPatchCreatesNewReferenceFile(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplySkillSuggestionPatchKeepsActiveFilesWhenVersionRecordFails(t *testing.T) {
|
||||
handler, skillStore, ctx, root := newTestUploadHandler(t)
|
||||
evolution := &skillEvolutionStoreStub{createVersionErr: errors.New("version store unavailable")}
|
||||
handler.SetEvolutionStore(evolution, nil)
|
||||
|
||||
currentDir := filepath.Join(root, "skills-store", "failure-skill", "1")
|
||||
if err := os.MkdirAll(currentDir, 0755); err != nil {
|
||||
t.Fatalf("mkdir current skill: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(currentDir, "SKILL.md"), []byte(skillMarkdown("Failure Skill", "failure-skill")), 0644); err != nil {
|
||||
t.Fatalf("write skill: %v", err)
|
||||
}
|
||||
skillID := skillStore.seedCustomSkill("failure-skill", currentDir, "active", nil)
|
||||
content := "Keep active files when metadata recording fails.\n"
|
||||
patch, err := json.Marshal(skillDraftPatch{Content: &content})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal draft patch: %v", err)
|
||||
}
|
||||
suggestion := &store.SkillImprovementSuggestion{
|
||||
ID: uuid.New(),
|
||||
SkillID: skillID,
|
||||
TargetFile: "references/recovery.md",
|
||||
DraftPatch: patch,
|
||||
Status: store.SkillSuggestionStatusApproved,
|
||||
SuggestionType: "skill_reference_add",
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodPost, "/v1/skills/"+skillID.String()+"/evolution/suggestions/"+suggestion.ID.String()+"/apply", nil).WithContext(ctx)
|
||||
|
||||
_, err = handler.applySkillSuggestionPatch(req, skillID, suggestion)
|
||||
if err == nil || !strings.Contains(err.Error(), "record skill version") {
|
||||
t.Fatalf("error = %v, want record skill version failure", err)
|
||||
}
|
||||
|
||||
newDir := filepath.Join(root, "skills-store", "failure-skill", "2")
|
||||
if _, err := os.Stat(newDir); err != nil {
|
||||
t.Fatalf("new active skill dir missing after metadata failure: %v", err)
|
||||
}
|
||||
created, err := os.ReadFile(filepath.Join(newDir, "references", "recovery.md"))
|
||||
if err != nil {
|
||||
t.Fatalf("read preserved reference: %v", err)
|
||||
}
|
||||
if string(created) != content {
|
||||
t.Fatalf("preserved reference = %q, want %q", created, content)
|
||||
}
|
||||
info, ok := skillStore.GetSkillByID(ctx, skillID)
|
||||
if !ok {
|
||||
t.Fatal("skill not found after apply failure")
|
||||
}
|
||||
if info.BaseDir != newDir || info.Version != 2 {
|
||||
t.Fatalf("skill active target = (%q, v%d), want (%q, v2)", info.BaseDir, info.Version, newDir)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillEvolutionMutationRouteRejectsNonTenantAdmin(t *testing.T) {
|
||||
handler, skillStore, _, root := newTestUploadHandler(t)
|
||||
tenantID := uuid.New()
|
||||
skillID := skillStore.seedCustomSkillForTenant(tenantID, "evolution-skill", filepath.Join(root, "tenants", tenantID.String(), "skills-store", "evolution-skill", "1"), "active", nil)
|
||||
tenants := newMockTenantStore()
|
||||
tenants.addTenant(tenantID, "tenant-a")
|
||||
tenants.setUserRole(tenantID, "scope-admin", store.TenantRoleMember)
|
||||
handler.tenantStore = tenants
|
||||
evolution := &skillEvolutionStoreStub{}
|
||||
handler.SetEvolutionStore(evolution, nil)
|
||||
rawKey := "evolution-admin"
|
||||
setupTestCache(t, map[string]*store.APIKeyData{
|
||||
crypto.HashAPIKey(rawKey): {
|
||||
ID: uuid.New(),
|
||||
Scopes: []string{"operator.admin"},
|
||||
TenantID: tenantID,
|
||||
OwnerID: "scope-admin",
|
||||
},
|
||||
})
|
||||
|
||||
mux := http.NewServeMux()
|
||||
handler.RegisterRoutes(mux)
|
||||
req := httptest.NewRequest(http.MethodPatch, "/v1/skills/"+skillID.String()+"/evolution", strings.NewReader(`{"enabled":true}`))
|
||||
req.Header.Set("Authorization", "Bearer "+rawKey)
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
mux.ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Fatalf("status = %d, body = %s, want 403", w.Code, w.Body.String())
|
||||
}
|
||||
if evolution.settingsUpdates != 0 {
|
||||
t.Fatalf("settings updates = %d, want 0", evolution.settingsUpdates)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSuggestionStatusDoesNotUpdateDifferentSkillSuggestion(t *testing.T) {
|
||||
handler, skillStore, ctx, root := newTestUploadHandler(t)
|
||||
requestSkillID := skillStore.seedCustomSkill("request-skill", filepath.Join(root, "skills-store", "request-skill", "1"), "active", nil)
|
||||
@@ -104,9 +196,12 @@ func TestHandleSuggestionStatusDoesNotUpdateDifferentSkillSuggestion(t *testing.
|
||||
}
|
||||
|
||||
type skillEvolutionStoreStub struct {
|
||||
versions []store.SkillVersion
|
||||
suggestions map[uuid.UUID]*store.SkillImprovementSuggestion
|
||||
statusUpdates int
|
||||
versions []store.SkillVersion
|
||||
suggestions map[uuid.UUID]*store.SkillImprovementSuggestion
|
||||
statusUpdates int
|
||||
settingsUpdates int
|
||||
createVersionErr error
|
||||
markAppliedErr error
|
||||
}
|
||||
|
||||
func (s *skillEvolutionStoreStub) GetSettings(context.Context, uuid.UUID) (*store.SkillEvolutionSettings, error) {
|
||||
@@ -114,7 +209,8 @@ func (s *skillEvolutionStoreStub) GetSettings(context.Context, uuid.UUID) (*stor
|
||||
}
|
||||
|
||||
func (s *skillEvolutionStoreStub) UpsertSettings(context.Context, store.SkillEvolutionSettings) (*store.SkillEvolutionSettings, error) {
|
||||
return nil, nil
|
||||
s.settingsUpdates++
|
||||
return &store.SkillEvolutionSettings{}, nil
|
||||
}
|
||||
|
||||
func (s *skillEvolutionStoreStub) RecordUsage(context.Context, store.SkillUsageMetric) error {
|
||||
@@ -167,6 +263,9 @@ func (s *skillEvolutionStoreStub) UpdateSuggestionStatus(_ context.Context, id u
|
||||
}
|
||||
|
||||
func (s *skillEvolutionStoreStub) MarkSuggestionApplied(_ context.Context, id uuid.UUID, version int, actorType, actorID string) (*store.SkillImprovementSuggestion, error) {
|
||||
if s.markAppliedErr != nil {
|
||||
return nil, s.markAppliedErr
|
||||
}
|
||||
return &store.SkillImprovementSuggestion{
|
||||
ID: id,
|
||||
Status: store.SkillSuggestionStatusApplied,
|
||||
@@ -177,6 +276,9 @@ func (s *skillEvolutionStoreStub) MarkSuggestionApplied(_ context.Context, id uu
|
||||
}
|
||||
|
||||
func (s *skillEvolutionStoreStub) CreateSkillVersion(_ context.Context, version store.SkillVersion) (*store.SkillVersion, error) {
|
||||
if s.createVersionErr != nil {
|
||||
return nil, s.createVersionErr
|
||||
}
|
||||
s.versions = append(s.versions, version)
|
||||
return &version, nil
|
||||
}
|
||||
|
||||
@@ -133,9 +133,17 @@ func init() {
|
||||
MsgCannotRemoveLastWriter: "cannot remove the last file writer",
|
||||
|
||||
// Skills
|
||||
MsgSkillsUpdateNotSupported: "skills.update not supported for file-based skills",
|
||||
MsgCannotResolveSkillID: "cannot resolve skill ID for file-based skill",
|
||||
MsgInvalidVisibility: "invalid visibility %q: must be one of private, public",
|
||||
MsgSkillsUpdateNotSupported: "skills.update not supported for file-based skills",
|
||||
MsgCannotResolveSkillID: "cannot resolve skill ID for file-based skill",
|
||||
MsgInvalidVisibility: "invalid visibility %q: must be one of private, public",
|
||||
MsgSkillEvolutionNotConfigured: "skill evolution store is not configured",
|
||||
MsgActivityStoreNotConfigured: "activity store is not configured",
|
||||
MsgInvalidEvolutionMode: "invalid evolution mode",
|
||||
MsgSystemSkillMutationBlocked: "system skill mutation is blocked",
|
||||
MsgSuggestionMustBeApproved: "suggestion must be approved before apply",
|
||||
MsgInvalidDraftPatch: "invalid draft_patch: %s",
|
||||
MsgDraftPatchRequired: "draft_patch requires content or find/replace",
|
||||
MsgFindTextNotFound: "find text not found in target file",
|
||||
|
||||
// Logs
|
||||
MsgInvalidLogAction: "action must be 'start' or 'stop'",
|
||||
|
||||
@@ -133,9 +133,17 @@ func init() {
|
||||
MsgCannotRemoveLastWriter: "không thể xóa người quản lý cuối cùng",
|
||||
|
||||
// Skills
|
||||
MsgSkillsUpdateNotSupported: "skills.update không được hỗ trợ với skill dựa trên tệp",
|
||||
MsgCannotResolveSkillID: "không thể xác định ID skill dựa trên tệp",
|
||||
MsgInvalidVisibility: "visibility không hợp lệ %q: phải là private hoặc public",
|
||||
MsgSkillsUpdateNotSupported: "skills.update không được hỗ trợ với skill dựa trên tệp",
|
||||
MsgCannotResolveSkillID: "không thể xác định ID skill dựa trên tệp",
|
||||
MsgInvalidVisibility: "visibility không hợp lệ %q: phải là private hoặc public",
|
||||
MsgSkillEvolutionNotConfigured: "kho lưu skill evolution chưa được cấu hình",
|
||||
MsgActivityStoreNotConfigured: "kho lưu activity chưa được cấu hình",
|
||||
MsgInvalidEvolutionMode: "evolution mode không hợp lệ",
|
||||
MsgSystemSkillMutationBlocked: "không được phép sửa skill hệ thống",
|
||||
MsgSuggestionMustBeApproved: "phải duyệt suggestion trước khi apply",
|
||||
MsgInvalidDraftPatch: "draft_patch không hợp lệ: %s",
|
||||
MsgDraftPatchRequired: "draft_patch cần content hoặc find/replace",
|
||||
MsgFindTextNotFound: "không tìm thấy đoạn cần thay trong tệp đích",
|
||||
|
||||
// Logs
|
||||
MsgInvalidLogAction: "action phải là 'start' hoặc 'stop'",
|
||||
|
||||
@@ -133,9 +133,17 @@ func init() {
|
||||
MsgCannotRemoveLastWriter: "无法移除最后一个文件管理员",
|
||||
|
||||
// Skills
|
||||
MsgSkillsUpdateNotSupported: "基于文件的Skill不支持 skills.update",
|
||||
MsgCannotResolveSkillID: "无法解析基于文件的Skill ID",
|
||||
MsgInvalidVisibility: "无效的 visibility %q:必须为 private 或 public",
|
||||
MsgSkillsUpdateNotSupported: "基于文件的Skill不支持 skills.update",
|
||||
MsgCannotResolveSkillID: "无法解析基于文件的Skill ID",
|
||||
MsgInvalidVisibility: "无效的 visibility %q:必须为 private 或 public",
|
||||
MsgSkillEvolutionNotConfigured: "技能演进存储未配置",
|
||||
MsgActivityStoreNotConfigured: "活动存储未配置",
|
||||
MsgInvalidEvolutionMode: "无效的 evolution mode",
|
||||
MsgSystemSkillMutationBlocked: "禁止修改系统Skill",
|
||||
MsgSuggestionMustBeApproved: "应用前必须先批准建议",
|
||||
MsgInvalidDraftPatch: "无效的 draft_patch:%s",
|
||||
MsgDraftPatchRequired: "draft_patch 需要 content 或 find/replace",
|
||||
MsgFindTextNotFound: "在目标文件中未找到要替换的文本",
|
||||
|
||||
// Logs
|
||||
MsgInvalidLogAction: "action 必须是 'start' 或 'stop'",
|
||||
|
||||
+11
-3
@@ -134,9 +134,17 @@ const (
|
||||
MsgCannotRemoveLastWriter = "error.cannot_remove_last_writer" // "cannot remove the last file writer"
|
||||
|
||||
// --- Skills ---
|
||||
MsgSkillsUpdateNotSupported = "error.skills_update_not_supported" // "skills.update not supported for file-based skills"
|
||||
MsgCannotResolveSkillID = "error.cannot_resolve_skill_id" // "cannot resolve skill ID for file-based skill"
|
||||
MsgInvalidVisibility = "error.invalid_visibility" // "invalid visibility %q: must be one of private, public"
|
||||
MsgSkillsUpdateNotSupported = "error.skills_update_not_supported" // "skills.update not supported for file-based skills"
|
||||
MsgCannotResolveSkillID = "error.cannot_resolve_skill_id" // "cannot resolve skill ID for file-based skill"
|
||||
MsgInvalidVisibility = "error.invalid_visibility" // "invalid visibility %q: must be one of private, public"
|
||||
MsgSkillEvolutionNotConfigured = "error.skill_evolution_not_configured" // "skill evolution store is not configured"
|
||||
MsgActivityStoreNotConfigured = "error.activity_store_not_configured" // "activity store is not configured"
|
||||
MsgInvalidEvolutionMode = "error.invalid_evolution_mode" // "invalid evolution mode"
|
||||
MsgSystemSkillMutationBlocked = "error.system_skill_mutation_blocked" // "system skill mutation is blocked"
|
||||
MsgSuggestionMustBeApproved = "error.suggestion_must_be_approved" // "suggestion must be approved before apply"
|
||||
MsgInvalidDraftPatch = "error.invalid_draft_patch" // "invalid draft_patch: %s"
|
||||
MsgDraftPatchRequired = "error.draft_patch_required" // "draft_patch requires content or find/replace"
|
||||
MsgFindTextNotFound = "error.find_text_not_found" // "find text not found in target file"
|
||||
|
||||
// --- Package updates (Phase 4+5) ---
|
||||
MsgPackageNotInstalled = "packages.update.not_installed" // "Package {name} is not installed"
|
||||
|
||||
Reference in new issue
Block a user