From dd6ee64f23e4de8bf2781e043af53c2a9fbd28fb Mon Sep 17 00:00:00 2001 From: Goon Date: Fri, 12 Jun 2026 15:38:23 +0700 Subject: [PATCH] fix(skills): harden evolution mutations --- internal/http/skills.go | 23 ++++- internal/http/skills_evolution.go | 122 ++++++++++++++++--------- internal/http/skills_evolution_test.go | 110 +++++++++++++++++++++- internal/i18n/catalog_en.go | 14 ++- internal/i18n/catalog_vi.go | 14 ++- internal/i18n/catalog_zh.go | 14 ++- internal/i18n/keys.go | 14 ++- 7 files changed, 247 insertions(+), 64 deletions(-) diff --git a/internal/http/skills.go b/internal/http/skills.go index 92eea618..c4224329 100644 --- a/internal/http/skills.go +++ b/internal/http/skills.go @@ -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. diff --git a/internal/http/skills_evolution.go b/internal/http/skills_evolution.go index 3025fe51..59770e42 100644 --- a/internal/http/skills_evolution.go +++ b/internal/http/skills_evolution.go @@ -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 diff --git a/internal/http/skills_evolution_test.go b/internal/http/skills_evolution_test.go index 032bedcb..7b34e424 100644 --- a/internal/http/skills_evolution_test.go +++ b/internal/http/skills_evolution_test.go @@ -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 } diff --git a/internal/i18n/catalog_en.go b/internal/i18n/catalog_en.go index 37c11706..920c8b78 100644 --- a/internal/i18n/catalog_en.go +++ b/internal/i18n/catalog_en.go @@ -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'", diff --git a/internal/i18n/catalog_vi.go b/internal/i18n/catalog_vi.go index 194834be..266e1050 100644 --- a/internal/i18n/catalog_vi.go +++ b/internal/i18n/catalog_vi.go @@ -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'", diff --git a/internal/i18n/catalog_zh.go b/internal/i18n/catalog_zh.go index c1989d84..5a47157e 100644 --- a/internal/i18n/catalog_zh.go +++ b/internal/i18n/catalog_zh.go @@ -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'", diff --git a/internal/i18n/keys.go b/internal/i18n/keys.go index 34442601..d57ce781 100644 --- a/internal/i18n/keys.go +++ b/internal/i18n/keys.go @@ -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"